Skip to content

Commit b9eefea

Browse files
committed
Add documentation on side channel attacks to SecretKey
We recently added a constant time `eq` implementation however library users can inadvertently bypass this protection if they use `AsRef`. To help prevent this add documentation to the `SecretKey` and also to the `AsRef` implementation.
1 parent 7cf3c6c commit b9eefea

File tree

1 file changed

+15
-1
lines changed

1 file changed

+15
-1
lines changed

src/key.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ use crate::{hashes, ThirtyTwoByteHash};
3636

3737
/// Secret 256-bit key used as `x` in an ECDSA signature.
3838
///
39+
/// # Side channel attacks
40+
///
41+
/// We have attempted to reduce the side channel attack surface by implementing a constant time `eq`
42+
/// method. For similar reasons we explicitly do not implement `PartialOrd`, `Ord`, or `Hash` on
43+
/// `SecretKey`. If you really want to order secrets keys then you can use `AsRef` to get at the
44+
/// underlying bytes and compare them - however this is almost certainly a bad idea.
45+
///
3946
/// # Serde support
4047
///
4148
/// Implements de/serialization with the `serde` feature enabled. We treat the byte value as a tuple
@@ -73,7 +80,14 @@ impl PartialEq for SecretKey {
7380
impl Eq for SecretKey {}
7481

7582
impl AsRef<[u8; constants::SECRET_KEY_SIZE]> for SecretKey {
76-
/// Gets a reference to the underlying array
83+
/// Gets a reference to the underlying array.
84+
///
85+
/// # Side channel attacks
86+
///
87+
/// Using ordering functions (`PartialOrd`/`Ord`) on a reference to secret keys leaks data
88+
/// because the implementations are not constant time. Doing so will make your code vulnerable
89+
/// to side channel attacks. [`SecretKey::eq`] is implemented using a constant time algorithm,
90+
/// please consider using it to do comparisons of secret keys.
7791
#[inline]
7892
fn as_ref(&self) -> &[u8; constants::SECRET_KEY_SIZE] {
7993
let &SecretKey(ref dat) = self;

0 commit comments

Comments
 (0)