Skip to content

Commit

Permalink
Obfuscate SharedSecret when printing
Browse files Browse the repository at this point in the history
Currently printing the `SharedSecret` using `Display` or `Debug` prints
the real secret, this is sub-optimal. We have a solution for other
secrets in the project where printing is obfuscated and we provide a
`display_secret` method for explicitly printing.

Mirror the logic for other secrets and obfuscate the `SharedSecret` when printing.

Includes improvements to rustdoc for the examples of all three secret
printing types.
  • Loading branch information
tcharding committed Feb 18, 2022
1 parent 5af56ec commit 6788d4b
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 13 deletions.
10 changes: 9 additions & 1 deletion src/ecdh.rs
Expand Up @@ -39,8 +39,9 @@ use secp256k1_sys::types::{c_int, c_uchar, c_void};
/// assert_eq!(sec1, sec2);
/// # }
// ```
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct SharedSecret([u8; 32]);
impl_display_secret!(SharedSecret);

impl SharedSecret {
/// Creates a new shared secret from a pubkey and secret key.
Expand All @@ -62,6 +63,13 @@ impl SharedSecret {
debug_assert_eq!(res, 1);
SharedSecret(buf)
}

/// Serializes the secret key as byte value.
// We just use this for interoperability with the display secret infrastructure (see `secret.rs`).
#[inline]
pub(crate) fn serialize_secret(&self) -> [u8; 32] {
self.0
}
}

impl Borrow<[u8]> for SharedSecret {
Expand Down
63 changes: 51 additions & 12 deletions src/secret.rs
Expand Up @@ -16,6 +16,7 @@

use ::core::fmt;
use ::{SecretKey, KeyPair, to_hex};
use ecdh::SharedSecret;
use constants::SECRET_KEY_SIZE;

macro_rules! impl_display_secret {
Expand Down Expand Up @@ -113,28 +114,26 @@ impl SecretKey {
/// little-endian hexadecimal string using the provided formatter.
///
/// This is the only method that outputs the actual secret key value, and, thus,
/// should be used with extreme precaution.
/// should be used with extreme caution.
///
/// # Example
/// # Examples
///
/// ```
/// # #[cfg(all(feature = "std", not(feature = "bitcoin_hashes")))] {
/// use secp256k1::ONE_KEY;
/// let key = ONE_KEY;
/// // Normal display hides value
/// assert_eq!(
/// "SecretKey(#2518682f7819fb2d)",
/// format!("{:?}", key)
/// );
/// let key = secp256k1::ONE_KEY;
///
/// // Normal display hides value.
/// assert_eq!("SecretKey(#2518682f7819fb2d)", format!("{:?}", key));
///
/// // Here we explicitly display the secret value:
/// assert_eq!(
/// "0000000000000000000000000000000000000000000000000000000000000001",
/// format!("{}", key.display_secret())
/// );
/// // Also, we can explicitly display with `Debug`:
/// assert_eq!(
/// "DisplaySecret(\"0000000000000000000000000000000000000000000000000000000000000001\")",
/// format!("{:?}", key.display_secret())
/// );
/// format!("{:?}", key.display_secret()),
/// format!("DisplaySecret(\"{}\")", key.display_secret()));
/// # }
/// ```
#[inline]
Expand Down Expand Up @@ -172,6 +171,7 @@ impl KeyPair {
/// "0000000000000000000000000000000000000000000000000000000000000001",
/// format!("{}", key.display_secret())
/// );
/// // Also, we can explicitly display with `Debug`:
/// assert_eq!(
/// "DisplaySecret(\"0000000000000000000000000000000000000000000000000000000000000001\")",
/// format!("{:?}", key.display_secret())
Expand All @@ -183,3 +183,42 @@ impl KeyPair {
DisplaySecret { secret: self.serialize_secret() }
}
}

impl SharedSecret {
/// Formats the explicit byte value of the shared secret kept inside the type as a
/// little-endian hexadecimal string using the provided formatter.
///
/// This is the only method that outputs the actual shared secret value, and, thus,
/// should be used with extreme caution.
///
/// # Examples
///
/// ```
/// # #[cfg(all(feature = "std", not(feature = "bitcoin_hashes")))] {
/// # use std::str::FromStr;
/// # use secp256k1::{SecretKey, PublicKey};
/// use secp256k1::ecdh::SharedSecret;
///
/// # let pk = PublicKey::from_slice(&[3, 23, 183, 225, 206, 31, 159, 148, 195, 42, 67, 115, 146, 41, 248, 140, 11, 3, 51, 41, 111, 180, 110, 143, 114, 134, 88, 73, 198, 174, 52, 184, 78]).expect("hard coded slice should parse correctly");
/// # let sk = SecretKey::from_str("57f0148f94d13095cfda539d0da0d1541304b678d8b36e243980aab4e1b7cead").unwrap();
///
/// let secret = SharedSecret::new(&pk, &sk);
/// // Normal display hides the value.
/// assert_eq!(format!("{:?}", secret), "SharedSecret(#2f7b793c9a1de4bf)");
///
/// // Here we explicitly display the secret value:
/// assert_eq!(
/// format!("{}", secret.display_secret()),
/// "cf05ae7da039ddce6d56dd57d3000c6dd91c6f1695eae47e05389f11e2467043"
/// );
/// // Also, we can explicitly display with `Debug`:
/// assert_eq!(
/// format!("{:?}", secret.display_secret()),
/// format!("DisplaySecret(\"{}\")", secret.display_secret()));
/// # }
/// ```
#[inline]
pub fn display_secret(&self) -> DisplaySecret {
DisplaySecret { secret: self.serialize_secret() }
}
}

0 comments on commit 6788d4b

Please sign in to comment.