diff --git a/key-wallet/src/bip32.rs b/key-wallet/src/bip32.rs index 041794a1d..f48bcaee7 100644 --- a/key-wallet/src/bip32.rs +++ b/key-wallet/src/bip32.rs @@ -120,6 +120,15 @@ impl zeroize::Zeroize for ChainCode { } } +// Manual implementation of Zeroize for Fingerprint, so extended-key types in +// other modules (BLS, Ed25519) can wipe their derivation metadata too. +// Note: Fingerprint is Copy, so this won't prevent copies in registers/stack +impl zeroize::Zeroize for Fingerprint { + fn zeroize(&mut self) { + zeroize::Zeroize::zeroize(&mut self.0); + } +} + impl fmt::LowerHex for ChainCode { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { for &byte in &self.0 { diff --git a/key-wallet/src/derivation_bls_bip32.rs b/key-wallet/src/derivation_bls_bip32.rs index 7c76caca1..ec85f0333 100644 --- a/key-wallet/src/derivation_bls_bip32.rs +++ b/key-wallet/src/derivation_bls_bip32.rs @@ -78,6 +78,33 @@ pub struct ExtendedBLSPrivKey { pub chain_code: ChainCode, } +// Hand-written (not `#[derive(Zeroize)]`): `BlsSecretKey` has no `Zeroize` +// impl of its own, but its inner scalar (public field `0`) does, so we wipe +// the value field by field. `Drop` (below) calls this, so the key is wiped +// automatically on scope exit with no caller action required. +// Cf. `ExtendedPrivKey` in `bip32`. +impl zeroize::Zeroize for ExtendedBLSPrivKey { + fn zeroize(&mut self) { + // Secret key material. + self.private_key.0.zeroize(); + self.chain_code.zeroize(); + // Derivation metadata — cleared too so the whole value is wiped. + self.depth.zeroize(); + self.parent_fingerprint.zeroize(); + self.child_number = ChildNumber::Normal { + index: 0, + }; + self.network = Network::Mainnet; // repr(u8)=0 discriminant, the "zero" value + } +} + +impl Drop for ExtendedBLSPrivKey { + fn drop(&mut self) { + use zeroize::Zeroize; + self.zeroize(); + } +} + impl ExtendedBLSPrivKey { /// Create a new master key from a seed pub fn new_master(network: Network, seed: &[u8]) -> Result { @@ -1213,4 +1240,21 @@ mod tests { assert_eq!(child_hardened.parent_fingerprint, master.fingerprint()); assert_eq!(child_unhardened.parent_fingerprint, master.fingerprint()); } + + #[test] + fn test_zeroize_clears_key_material() { + use zeroize::Zeroize; + + let seed = [42u8; 32]; + let mut key = ExtendedBLSPrivKey::new_master(Network::Testnet, &seed).unwrap(); + assert_ne!(key.private_key.to_be_bytes(), [0u8; 32]); + assert_ne!(key.chain_code.as_ref(), &[0u8; 32]); + + key.zeroize(); + + assert_eq!(key.private_key.to_be_bytes(), [0u8; 32]); + assert_eq!(key.chain_code.as_ref(), &[0u8; 32]); + assert_eq!(key.depth, 0); + assert_eq!(key.parent_fingerprint, Fingerprint::default()); + } } diff --git a/key-wallet/src/derivation_slip10.rs b/key-wallet/src/derivation_slip10.rs index b24f1c91f..804154003 100644 --- a/key-wallet/src/derivation_slip10.rs +++ b/key-wallet/src/derivation_slip10.rs @@ -26,7 +26,7 @@ pub use dashcore::ed25519_dalek::VerifyingKey as Ed25519PublicKey; pub use crate::bip32::DerivationPath; /// Extended Ed25519 private key for SLIP-0010 -#[derive(Clone, PartialEq, Eq)] +#[derive(Clone)] pub struct ExtendedEd25519PrivKey { /// Network this key is for pub network: Network, @@ -42,6 +42,52 @@ pub struct ExtendedEd25519PrivKey { pub chain_code: ChainCode, } +// Hand-written instead of derived so the raw secret bytes are compared in +// constant time (a derived `==` on `[u8; 32]` short-circuits on the first +// differing byte, leaking how much of the secret matches). +impl PartialEq for ExtendedEd25519PrivKey { + fn eq(&self, other: &Self) -> bool { + let mut diff = 0u8; + for (a, b) in self.private_key.iter().zip(other.private_key.iter()) { + diff |= a ^ b; + } + // black_box keeps the compiler from turning the fold back into an + // early-exit comparison. + core::hint::black_box(diff) == 0 + && self.network == other.network + && self.depth == other.depth + && self.parent_fingerprint == other.parent_fingerprint + && self.child_number == other.child_number + && self.chain_code == other.chain_code + } +} + +impl Eq for ExtendedEd25519PrivKey {} + +// `Drop` (below) calls this, so the key is wiped automatically on scope exit +// with no caller action required. Cf. `ExtendedPrivKey` in `bip32`. +impl zeroize::Zeroize for ExtendedEd25519PrivKey { + fn zeroize(&mut self) { + // Secret key material. + self.private_key.zeroize(); + self.chain_code.zeroize(); + // Derivation metadata — cleared too so the whole value is wiped. + self.depth.zeroize(); + self.parent_fingerprint.zeroize(); + self.child_number = ChildNumber::Normal { + index: 0, + }; + self.network = Network::Mainnet; // repr(u8)=0 discriminant, the "zero" value + } +} + +impl Drop for ExtendedEd25519PrivKey { + fn drop(&mut self) { + use zeroize::Zeroize; + self.zeroize(); + } +} + impl ExtendedEd25519PrivKey { /// Create a new master key from seed pub fn new_master(network: Network, seed: &[u8]) -> Result { @@ -741,4 +787,37 @@ mod test { hex::encode(private_key) } + + #[test] + fn test_zeroize_clears_key_material() { + use zeroize::Zeroize; + + let seed = hex::decode(CASE_1_SEED).unwrap(); + let mut key = ExtendedEd25519PrivKey::new_master(Network::Mainnet, &seed).unwrap(); + assert_ne!(key.private_key, [0u8; 32]); + assert_ne!(key.chain_code.as_ref(), &[0u8; 32]); + + key.zeroize(); + + assert_eq!(key.private_key, [0u8; 32]); + assert_eq!(key.chain_code.as_ref(), &[0u8; 32]); + assert_eq!(key.depth, 0); + assert_eq!(key.parent_fingerprint, Fingerprint::default()); + } + + #[test] + fn test_partial_eq_matches_field_equality() { + let seed = hex::decode(CASE_1_SEED).unwrap(); + let master = ExtendedEd25519PrivKey::new_master(Network::Mainnet, &seed).unwrap(); + + assert_eq!(master, master.clone()); + + let child = master.ckd_priv(ChildNumber::from_hardened_idx(0).unwrap()).unwrap(); + assert_ne!(master, child); + + // Same secret but different metadata must not compare equal + let mut other_network = master.clone(); + other_network.network = Network::Testnet; + assert_ne!(master, other_network); + } }