From eedd63d5e9a4e055260b476177ee14972584a1bb Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 1 Aug 2022 14:53:34 -0400 Subject: [PATCH] tor-proto: Replace SecretBytes with SecretBuf. This does not yet make sure that `SecretBuf` is used where it _should_ be, but at least it ensures that most uses of `SecretBytes` will indeed act as intended, and make sure that whatever they contain is zeroized. It requires some corresponding changes to method calls for correctness and type conformance. --- crates/tor-proto/src/crypto/cell.rs | 10 +++--- crates/tor-proto/src/crypto/handshake.rs | 17 +++++----- crates/tor-proto/src/crypto/handshake/fast.rs | 20 ++++++----- .../tor-proto/src/crypto/handshake/hs_ntor.rs | 18 +++++----- crates/tor-proto/src/crypto/handshake/ntor.rs | 15 ++++----- .../tor-proto/src/crypto/handshake/ntor_v3.rs | 12 +++---- crates/tor-proto/src/crypto/ll/kdf.rs | 33 ++++++++++--------- crates/tor-proto/src/lib.rs | 3 -- 8 files changed, 65 insertions(+), 63 deletions(-) diff --git a/crates/tor-proto/src/crypto/cell.rs b/crates/tor-proto/src/crypto/cell.rs index 80f3a5481..1523475b6 100644 --- a/crates/tor-proto/src/crypto/cell.rs +++ b/crates/tor-proto/src/crypto/cell.rs @@ -48,7 +48,7 @@ pub(crate) trait CryptInit: Sized { /// Initialize this object from a key generator. fn construct(keygen: K) -> Result { let seed = keygen.expand(Self::seed_len())?; - Self::initialize(&seed) + Self::initialize(&seed[..]) } } @@ -393,9 +393,9 @@ pub(crate) mod tor1 { mod test { #![allow(clippy::unwrap_used)] use super::*; - use crate::SecretBytes; use rand::RngCore; use tor_basic_utils::test_rng::testing_rng; + use tor_bytes::SecretBuf; fn add_layers( cc_out: &mut OutboundClientCrypt, @@ -411,10 +411,8 @@ mod test { fn roundtrip() { // Take canned keys and make sure we can do crypto correctly. use crate::crypto::handshake::ShakeKeyGenerator as KGen; - fn s(seed: &[u8]) -> SecretBytes { - let mut s: SecretBytes = SecretBytes::new(Vec::new()); - s.extend(seed); - s + fn s(seed: &[u8]) -> SecretBuf { + seed.to_vec().into() } let seed1 = s(b"hidden we are free"); diff --git a/crates/tor-proto/src/crypto/handshake.rs b/crates/tor-proto/src/crypto/handshake.rs index d8e1b926b..5dfe61788 100644 --- a/crates/tor-proto/src/crypto/handshake.rs +++ b/crates/tor-proto/src/crypto/handshake.rs @@ -17,9 +17,10 @@ pub(crate) mod ntor; #[cfg(feature = "ntor_v3")] pub(crate) mod ntor_v3; -use crate::{Result, SecretBytes}; +use crate::Result; //use zeroize::Zeroizing; use rand_core::{CryptoRng, RngCore}; +use tor_bytes::SecretBuf; /// A ClientHandshake is used to generate a client onionskin and /// handle a relay onionskin. @@ -73,7 +74,7 @@ pub(crate) trait ServerHandshake { /// It can only be used once. pub(crate) trait KeyGenerator { /// Consume the key - fn expand(self, keylen: usize) -> Result; + fn expand(self, keylen: usize) -> Result; } /// Generates keys based on the KDF-TOR function. @@ -81,18 +82,18 @@ pub(crate) trait KeyGenerator { /// This is deprecated and shouldn't be used for new keys. pub(crate) struct TapKeyGenerator { /// Seed for the TAP KDF. - seed: SecretBytes, + seed: SecretBuf, } impl TapKeyGenerator { /// Create a key generator based on a provided seed - pub(crate) fn new(seed: SecretBytes) -> Self { + pub(crate) fn new(seed: SecretBuf) -> Self { TapKeyGenerator { seed } } } impl KeyGenerator for TapKeyGenerator { - fn expand(self, keylen: usize) -> Result { + fn expand(self, keylen: usize) -> Result { use crate::crypto::ll::kdf::{Kdf, LegacyKdf}; LegacyKdf::new(1).derive(&self.seed[..], keylen) } @@ -101,19 +102,19 @@ impl KeyGenerator for TapKeyGenerator { /// Generates keys based on SHAKE-256. pub(crate) struct ShakeKeyGenerator { /// Seed for the key generator - seed: SecretBytes, + seed: SecretBuf, } impl ShakeKeyGenerator { /// Create a key generator based on a provided seed #[allow(dead_code)] // We'll construct these for v3 onion services - pub(crate) fn new(seed: SecretBytes) -> Self { + pub(crate) fn new(seed: SecretBuf) -> Self { ShakeKeyGenerator { seed } } } impl KeyGenerator for ShakeKeyGenerator { - fn expand(self, keylen: usize) -> Result { + fn expand(self, keylen: usize) -> Result { use crate::crypto::ll::kdf::{Kdf, ShakeKdf}; ShakeKdf::new().derive(&self.seed[..], keylen) } diff --git a/crates/tor-proto/src/crypto/handshake/fast.rs b/crates/tor-proto/src/crypto/handshake/fast.rs index aaaabef21..45f297d33 100644 --- a/crates/tor-proto/src/crypto/handshake/fast.rs +++ b/crates/tor-proto/src/crypto/handshake/fast.rs @@ -7,6 +7,7 @@ use crate::util::ct::bytes_eq; use crate::{Error, Result}; use rand::{CryptoRng, RngCore}; +use tor_bytes::SecretBuf; use tor_error::into_internal; /// Number of bytes used for a "CREATE_FAST" handshake by the initiator. @@ -22,6 +23,9 @@ pub(crate) struct CreateFastClientState([u8; FAST_C_HANDSHAKE_LEN]); /// See module documentation; you probably don't want to use this. pub(crate) struct CreateFastClient; +/// How many bytes does this handshake use for its input seed? +const SECRET_INPUT_LEN: usize = 40; + impl super::ClientHandshake for CreateFastClient { type KeyType = (); type StateType = CreateFastClientState; @@ -41,9 +45,9 @@ impl super::ClientHandshake for CreateFastClient { if msg.len() != FAST_S_HANDSHAKE_LEN { return Err(Error::BadCircHandshakeAuth); } - let mut inp = Vec::new(); - inp.extend(&state.0[..]); - inp.extend(&msg[0..20]); + let mut inp = SecretBuf::with_capacity(SECRET_INPUT_LEN); + inp.extend_from_slice(&state.0[..]); + inp.extend_from_slice(&msg[0..20]); let kh_expect = LegacyKdf::new(0).derive(&inp[..], 20)?; @@ -51,7 +55,7 @@ impl super::ClientHandshake for CreateFastClient { return Err(Error::BadCircHandshakeAuth); } - Ok(super::TapKeyGenerator::new(inp.into())) + Ok(super::TapKeyGenerator::new(inp)) } } @@ -76,15 +80,15 @@ impl super::ServerHandshake for CreateFastServer { let mut reply = vec![0_u8; FAST_S_HANDSHAKE_LEN]; rng.fill_bytes(&mut reply[0..20]); - let mut inp = Vec::new(); - inp.extend(msg); - inp.extend(&reply[0..20]); + let mut inp = SecretBuf::with_capacity(SECRET_INPUT_LEN); + inp.extend_from_slice(msg); + inp.extend_from_slice(&reply[0..20]); let kh = LegacyKdf::new(0) .derive(&inp[..], 20) .map_err(into_internal!("Can't expand key"))?; reply[20..].copy_from_slice(&kh); - Ok((super::TapKeyGenerator::new(inp.into()), reply)) + Ok((super::TapKeyGenerator::new(inp), reply)) } } diff --git a/crates/tor-proto/src/crypto/handshake/hs_ntor.rs b/crates/tor-proto/src/crypto/handshake/hs_ntor.rs index b583b109f..8370c12b8 100644 --- a/crates/tor-proto/src/crypto/handshake/hs_ntor.rs +++ b/crates/tor-proto/src/crypto/handshake/hs_ntor.rs @@ -26,8 +26,8 @@ use crate::crypto::handshake::KeyGenerator; use crate::crypto::ll::kdf::{Kdf, ShakeKdf}; -use crate::{Error, Result, SecretBytes}; -use tor_bytes::{Reader, Writer}; +use crate::{Error, Result}; +use tor_bytes::{Reader, SecretBuf, Writer}; use tor_llcrypto::d::Sha3_256; use tor_llcrypto::pk::{curve25519, ed25519}; use tor_llcrypto::util::rand_compat::RngCompatExt; @@ -56,19 +56,19 @@ pub type Subcredential = [u8; 32]; /// expansion protocol specified in section "Key expansion" of rend-spec-v3.txt . pub struct HsNtorHkdfKeyGenerator { /// Secret data derived from the handshake, used as input to HKDF - seed: SecretBytes, + seed: SecretBuf, } impl HsNtorHkdfKeyGenerator { /// Create a new key generator to expand a given seed - pub fn new(seed: SecretBytes) -> Self { + pub fn new(seed: SecretBuf) -> Self { HsNtorHkdfKeyGenerator { seed } } } impl KeyGenerator for HsNtorHkdfKeyGenerator { /// Expand the seed into a keystream of 'keylen' size - fn expand(self, keylen: usize) -> Result { + fn expand(self, keylen: usize) -> Result { ShakeKdf::new().derive(&self.seed[..], keylen) } } @@ -473,7 +473,7 @@ fn get_rendezvous1_key_material( let hs_ntor_key_constant = &b"tor-hs-ntor-curve25519-sha3-256-1:hs_key_extract"[..]; // Start with rend_secret_hs_input - let mut secret_input = Zeroizing::new(Vec::new()); + let mut secret_input = SecretBuf::new(); secret_input .write(xy) // EXP(X,y) .and_then(|_| secret_input.write(xb)) // EXP(X,b) @@ -491,7 +491,7 @@ fn get_rendezvous1_key_material( let verify = hs_ntor_mac(&secret_input, hs_ntor_verify_constant)?; // Start building 'auth_input' - let mut auth_input = Zeroizing::new(Vec::new()); + let mut auth_input = SecretBuf::new(); auth_input .write(&verify) .and_then(|_| auth_input.write(auth_key)) // AUTH_KEY @@ -506,12 +506,12 @@ fn get_rendezvous1_key_material( let auth_input_mac = hs_ntor_mac(&auth_input, hs_ntor_mac_constant)?; // Now finish up with the KDF construction - let mut kdf_seed = Zeroizing::new(Vec::new()); + let mut kdf_seed = SecretBuf::new(); kdf_seed .write(&ntor_key_seed) .and_then(|_| kdf_seed.write(hs_ntor_expand_constant)) .map_err(into_internal!("Can't encode kdf-input for hs-ntor."))?; - let keygen = HsNtorHkdfKeyGenerator::new(Zeroizing::new(kdf_seed.to_vec())); + let keygen = HsNtorHkdfKeyGenerator::new(kdf_seed); Ok((keygen, auth_input_mac)) } diff --git a/crates/tor-proto/src/crypto/handshake/ntor.rs b/crates/tor-proto/src/crypto/handshake/ntor.rs index ded626501..848719066 100644 --- a/crates/tor-proto/src/crypto/handshake/ntor.rs +++ b/crates/tor-proto/src/crypto/handshake/ntor.rs @@ -2,8 +2,8 @@ use super::{KeyGenerator, RelayHandshakeError, RelayHandshakeResult}; use crate::util::ct; -use crate::{Error, Result, SecretBytes}; -use tor_bytes::{EncodeResult, Reader, Writer}; +use crate::{Error, Result}; +use tor_bytes::{EncodeResult, Reader, SecretBuf, Writer}; use tor_error::into_internal; use tor_llcrypto::d; use tor_llcrypto::pk::curve25519::*; @@ -11,7 +11,6 @@ use tor_llcrypto::pk::rsa::RsaIdentity; use digest::Mac; use rand_core::{CryptoRng, RngCore}; -use zeroize::Zeroizing; /// Client side of the Ntor handshake. pub(crate) struct NtorClient; @@ -104,18 +103,18 @@ pub(crate) struct NtorHandshakeState { pub(crate) struct NtorHkdfKeyGenerator { /// Secret key information derived from the handshake, used as input /// to HKDF - seed: SecretBytes, + seed: SecretBuf, } impl NtorHkdfKeyGenerator { /// Create a new key generator to expand a given seed - pub(crate) fn new(seed: SecretBytes) -> Self { + pub(crate) fn new(seed: SecretBuf) -> Self { NtorHkdfKeyGenerator { seed } } } impl KeyGenerator for NtorHkdfKeyGenerator { - fn expand(self, keylen: usize) -> Result { + fn expand(self, keylen: usize) -> Result { let ntor1_key = &b"ntor-curve25519-sha256-1:key_extract"[..]; let ntor1_expand = &b"ntor-curve25519-sha256-1:key_expand"[..]; use crate::crypto::ll::kdf::{Kdf, Ntor1Kdf}; @@ -211,7 +210,7 @@ fn ntor_derive( let ntor1_verify = &b"ntor-curve25519-sha256-1:verify"[..]; let server_string = &b"Server"[..]; - let mut secret_input = Zeroizing::new(Vec::new()); + let mut secret_input = SecretBuf::new(); secret_input.write(xy)?; // EXP(X,y) secret_input.write(xb)?; // EXP(X,b) secret_input.write(&server_pk.id)?; // ID @@ -228,7 +227,7 @@ fn ntor_derive( m.update(&secret_input[..]); m.finalize() }; - let mut auth_input: SecretBytes = Zeroizing::new(Vec::new()); + let mut auth_input: SecretBuf = SecretBuf::new(); auth_input.write_and_consume(verify)?; // verify auth_input.write(&server_pk.id)?; // ID auth_input.write(&server_pk.pk)?; // B diff --git a/crates/tor-proto/src/crypto/handshake/ntor_v3.rs b/crates/tor-proto/src/crypto/handshake/ntor_v3.rs index e6bf1a3e4..fe9e080eb 100644 --- a/crates/tor-proto/src/crypto/handshake/ntor_v3.rs +++ b/crates/tor-proto/src/crypto/handshake/ntor_v3.rs @@ -14,8 +14,8 @@ use super::{RelayHandshakeError, RelayHandshakeResult}; use crate::util::ct; -use crate::{Error, Result, SecretBytes}; -use tor_bytes::{EncodeResult, Reader, Writeable, Writer}; +use crate::{Error, Result}; +use tor_bytes::{EncodeResult, Reader, SecretBuf, Writeable, Writer}; use tor_error::into_internal; use tor_llcrypto::d::{Sha3_256, Shake256}; use tor_llcrypto::pk::{curve25519, ed25519::Ed25519Identity}; @@ -316,10 +316,10 @@ pub(crate) struct NtorV3KeyGenerator { } impl KeyGenerator for NtorV3KeyGenerator { - fn expand(mut self, keylen: usize) -> Result { - let mut ret = vec![0; keylen]; - self.reader.read(&mut ret); - Ok(Zeroizing::new(ret)) + fn expand(mut self, keylen: usize) -> Result { + let mut ret: SecretBuf = vec![0; keylen].into(); + self.reader.read(ret.as_mut()); + Ok(ret) } } diff --git a/crates/tor-proto/src/crypto/ll/kdf.rs b/crates/tor-proto/src/crypto/ll/kdf.rs index 98102ba8a..00b12713f 100644 --- a/crates/tor-proto/src/crypto/ll/kdf.rs +++ b/crates/tor-proto/src/crypto/ll/kdf.rs @@ -14,16 +14,16 @@ //! services, and is likely to be used by other places in the future. //! It is based on SHAKE-256. -use crate::{Error, Result, SecretBytes}; +use crate::{Error, Result}; use digest::{ExtendableOutput, Update, XofReader}; +use tor_bytes::SecretBuf; use tor_llcrypto::d::{Sha1, Sha256, Shake256}; - -use zeroize::Zeroizing; +use zeroize::Zeroize; /// A trait for a key derivation function. pub(crate) trait Kdf { /// Derive `n_bytes` of key data from some secret `seed`. - fn derive(&self, seed: &[u8], n_bytes: usize) -> Result; + fn derive(&self, seed: &[u8], n_bytes: usize) -> Result; } /// A legacy KDF, for use with TAP. @@ -58,22 +58,25 @@ impl LegacyKdf { } } impl Kdf for LegacyKdf { - fn derive(&self, seed: &[u8], n_bytes: usize) -> Result { + fn derive(&self, seed: &[u8], n_bytes: usize) -> Result { use digest::Digest; - let mut result = Zeroizing::new(Vec::with_capacity(n_bytes + Sha1::output_size())); + let mut result = SecretBuf::with_capacity(n_bytes + Sha1::output_size()); let mut k = self.idx; if n_bytes > Sha1::output_size() * (256 - (k as usize)) { return Err(Error::InvalidKDFOutputLength); } + let mut digest_output = Default::default(); while result.len() < n_bytes { let mut d = Sha1::new(); Digest::update(&mut d, seed); Digest::update(&mut d, &[k]); - result.extend(d.finalize()); + d.finalize_into(&mut digest_output); + result.extend_from_slice(&digest_output); k += 1; } + digest_output.zeroize(); result.truncate(n_bytes); Ok(result) @@ -88,11 +91,11 @@ impl<'a, 'b> Ntor1Kdf<'a, 'b> { } impl Kdf for Ntor1Kdf<'_, '_> { - fn derive(&self, seed: &[u8], n_bytes: usize) -> Result { + fn derive(&self, seed: &[u8], n_bytes: usize) -> Result { let hkdf = hkdf::Hkdf::::new(Some(self.t_key), seed); - let mut result = Zeroizing::new(vec![0; n_bytes]); - hkdf.expand(self.m_expand, &mut result[..]) + let mut result: SecretBuf = vec![0; n_bytes].into(); + hkdf.expand(self.m_expand, result.as_mut()) .map_err(|_| Error::InvalidKDFOutputLength)?; Ok(result) } @@ -105,11 +108,11 @@ impl ShakeKdf { } } impl Kdf for ShakeKdf { - fn derive(&self, seed: &[u8], n_bytes: usize) -> Result { + fn derive(&self, seed: &[u8], n_bytes: usize) -> Result { let mut xof = Shake256::default(); xof.update(seed); - let mut result = Zeroizing::new(vec![0; n_bytes]); - xof.finalize_xof().read(&mut result); + let mut result: SecretBuf = vec![0; n_bytes].into(); + xof.finalize_xof().read(result.as_mut()); Ok(result) } } @@ -144,7 +147,7 @@ mod test { #[test] fn testvec_tap_kdf() { // Taken from test_crypto.c in Tor, generated by a python script. - fn expand(b: &[u8]) -> SecretBytes { + fn expand(b: &[u8]) -> SecretBuf { LegacyKdf::new(0).derive(b, 100).unwrap() } @@ -199,7 +202,7 @@ mod test { #[test] fn testvec_ntor1_kdf() { // From Tor's test_crypto.c; generated with ntor_ref.py - fn expand(b: &[u8]) -> SecretBytes { + fn expand(b: &[u8]) -> SecretBuf { let t_key = b"ntor-curve25519-sha256-1:key_extract"; let m_expand = b"ntor-curve25519-sha256-1:key_expand"; Ntor1Kdf::new(&t_key[..], &m_expand[..]) diff --git a/crates/tor-proto/src/lib.rs b/crates/tor-proto/src/lib.rs index 144759646..b0ce15f3a 100644 --- a/crates/tor-proto/src/lib.rs +++ b/crates/tor-proto/src/lib.rs @@ -129,9 +129,6 @@ pub use util::skew::ClockSkew; pub use channel::params::ChannelsParams; -/// A vector of bytes that gets cleared when it's dropped. -type SecretBytes = zeroize::Zeroizing>; - /// A Result type for this crate. pub type Result = std::result::Result;