From 532ccc0f583d601d7ba2bb7f98f9b7355f7f5c4f Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Tue, 25 Oct 2022 09:47:24 +0200 Subject: [PATCH] feat: add `Zeroize` support to key types, and create new shared secret type (#137) * Add `Zeroize` support to key types * Use zeroize-on-drop macro * Ensure ECDH secrets are zeroized on drop * Add new DHKE shared secret type --- src/dhke.rs | 48 +++++++++++++++++++++++++++++++ src/keys.rs | 8 ------ src/lib.rs | 1 + src/ristretto/ristretto_keys.rs | 51 +++++++++++++++++++++------------ 4 files changed, 82 insertions(+), 26 deletions(-) create mode 100644 src/dhke.rs diff --git a/src/dhke.rs b/src/dhke.rs new file mode 100644 index 00000000..07859a1c --- /dev/null +++ b/src/dhke.rs @@ -0,0 +1,48 @@ +// Copyright 2022 The Tari Project +// SPDX-License-Identifier: BSD-3-Clause + +//! The robotic innards of a Diffie-Hellman key exchange (DHKE) producing a shared secret. +//! Even though the result of a DHKE is the same type as a public key, it is typically treated as a secret value. +//! To make this work more safely, we ensure that a DHKE result is cleared after use (but beware of subsequent copies or moves). +//! Because a DHKE shared secret is intended to be used in further key derivation, the only visibility into it is as a byte array; it's not possible to directly extract the underlying public key type, and you probably shouldn't clone the byte array without a very good reason. +//! If you need the underlying public key itself, you probably should be using something else. + +use std::ops::Mul; + +use zeroize::Zeroize; + +use crate::keys::PublicKey; + +pub struct DiffieHellmanSharedSecret

(P) +where P: Zeroize; + +impl

DiffieHellmanSharedSecret

+where + P: PublicKey + Zeroize, + for<'a> &'a

::K: Mul<&'a P, Output = P>, +{ + /// Perform a Diffie-Hellman key exchange + pub fn new(sk: &P::K, pk: &P) -> Self { + Self(sk * pk) + } + + /// Get the shared secret as a byte array + pub fn as_bytes(&self) -> &[u8] { + self.0.as_bytes() + } +} + +impl

Zeroize for DiffieHellmanSharedSecret

where P: Zeroize { + /// Zeroize the shared secret's underlying public key + fn zeroize(&mut self) { + self.0.zeroize(); + } +} + +impl

Drop for DiffieHellmanSharedSecret

where P: Zeroize { + /// Zeroize the shared secret when out of scope or otherwise dropped + fn drop(&mut self) { + self.zeroize(); + } +} + diff --git a/src/keys.rs b/src/keys.rs index ba62b982..893721a7 100644 --- a/src/keys.rs +++ b/src/keys.rs @@ -68,11 +68,3 @@ pub trait PublicKey: (k, pk) } } - -/// This trait provides a common mechanism to calculate a shared secret using the private and public key of two parties -pub trait DiffieHellmanSharedSecret: ByteArray + Clone + PartialEq + Eq + Add + Default { - /// The type of public key - type PK: PublicKey; - /// Generate a shared secret from one party's private key and another party's public key - fn shared_secret(k: &::K, pk: &Self::PK) -> Self::PK; -} diff --git a/src/lib.rs b/src/lib.rs index 6135ebe8..cd20fa1a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,6 +9,7 @@ extern crate lazy_static; #[macro_use] mod macros; pub mod commitment; +pub mod dhke; pub mod hash; pub mod hashing; pub mod keys; diff --git a/src/ristretto/ristretto_keys.rs b/src/ristretto/ristretto_keys.rs index a6500df0..bf3740e9 100644 --- a/src/ristretto/ristretto_keys.rs +++ b/src/ristretto/ristretto_keys.rs @@ -26,7 +26,7 @@ use zeroize::Zeroize; use crate::{ errors::HashingError, hashing::{DerivedKeyDomain, DomainSeparatedHasher, DomainSeparation}, - keys::{DiffieHellmanSharedSecret, PublicKey, SecretKey}, + keys::{PublicKey, SecretKey}, }; /// The [SecretKey](trait.SecretKey.html) implementation for [Ristretto](https://ristretto.group) is a thin wrapper @@ -49,7 +49,8 @@ use crate::{ /// let _k2 = RistrettoSecretKey::from_hex(&"100000002000000030000000040000000"); /// let _k3 = RistrettoSecretKey::random(&mut rng); /// ``` -#[derive(Eq, Clone, Default)] +#[derive(Eq, Clone, Default, Zeroize)] +#[zeroize(drop)] pub struct RistrettoSecretKey(pub(crate) Scalar); const SCALAR_LENGTH: usize = 32; @@ -67,13 +68,6 @@ impl SecretKey for RistrettoSecretKey { } } -impl Drop for RistrettoSecretKey { - /// Clear the secret key value in memory when it goes out of scope - fn drop(&mut self) { - self.0.zeroize() - } -} - //------------------------------------- Ristretto Secret Key ByteArray ---------------------------------------------// impl ByteArray for RistrettoSecretKey { @@ -280,6 +274,18 @@ impl RistrettoPublicKey { } } +impl Zeroize for RistrettoPublicKey { + /// Zeroizes both the point and (if it exists) the compressed point + fn zeroize(&mut self) { + self.point.zeroize(); + + // Need to empty the cell + if let Some(mut compressed) = self.compressed.take() { + compressed.zeroize(); + } + } +} + //--------------------------------------- Ristretto Hashing Applications ------------------------------------------// /// The Domain Separation Tag type for the KDF algorithm, version 1 @@ -330,15 +336,6 @@ impl PublicKey for RistrettoPublicKey { } } -impl DiffieHellmanSharedSecret for RistrettoPublicKey { - type PK = RistrettoPublicKey; - - /// Generate a shared secret from one party's private key and another party's public key - fn shared_secret(k: &::K, pk: &Self::PK) -> Self::PK { - k * pk - } -} - // Requires custom Hashable implementation for RistrettoPublicKey as CompressedRistretto doesnt implement this trait impl Hashable for RistrettoPublicKey { fn hash(&self) -> Vec { @@ -830,4 +827,22 @@ mod test { let visible = format!("{:?}", key.reveal()); assert!(visible.contains("016c")); } + + #[test] + fn zeroize_test() { + let mut rng = rand::thread_rng(); + let zeros = [0u8; 32]; + + // Zeroize scalar + let mut s = RistrettoSecretKey::random(&mut rng); + s.zeroize(); + assert_eq!(s.as_bytes(), &zeros); + + // Zeroize point + let mut p = RistrettoPublicKey::from_secret_key(&RistrettoSecretKey::random(&mut rng)); + p.zeroize(); + assert_eq!(p.compressed.get(), None); // no compressed point yet + assert_eq!(p.as_bytes(), &zeros); // this compresses the point + assert_eq!(p.compressed.get().unwrap().as_bytes(), &zeros); // check directly for good measure + } }