From 9932b5ab215e7806a86a32d41adf2ec0b4982b93 Mon Sep 17 00:00:00 2001 From: Claudia Richoux Date: Thu, 26 Jan 2023 01:21:33 -0500 Subject: [PATCH 1/2] adding a feature, making clippy shut up --- Cargo.lock | 7 +++++++ blake2/Cargo.toml | 2 ++ tiger/src/lib.rs | 4 ++-- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d851c17b4..9a05204d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8,6 +8,7 @@ version = "0.10.6" dependencies = [ "digest", "hex-literal", + "zeroize", ] [[package]] @@ -316,3 +317,9 @@ checksum = "4b0930846e800a97c78fd09a494b25d1f0780be9face03b7a05151e3104a8284" dependencies = [ "cc", ] + +[[package]] +name = "zeroize" +version = "1.5.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c394b5bd0c6f669e7275d9c20aa90ae064cb22e75a1cad54e1b34088034b149f" diff --git a/blake2/Cargo.toml b/blake2/Cargo.toml index 11c00ac01..92d1f67b0 100644 --- a/blake2/Cargo.toml +++ b/blake2/Cargo.toml @@ -13,6 +13,7 @@ categories = ["cryptography", "no-std"] [dependencies] digest = { version = "0.10.3", features = ["mac"] } +zeroize = { version = "1.4.0", optional = true} [dev-dependencies] digest = { version = "0.10.3", features = ["dev"] } @@ -26,3 +27,4 @@ simd = [] simd_opt = ["simd"] simd_asm = ["simd_opt"] size_opt = [] # Optimize for code size. Removes some `inline(always)` +zeroize = ["dep:zeroize"] # Enable zeroize crate support diff --git a/tiger/src/lib.rs b/tiger/src/lib.rs index a5f1f8d80..ed2830a7d 100644 --- a/tiger/src/lib.rs +++ b/tiger/src/lib.rs @@ -91,7 +91,7 @@ impl UpdateCore for TigerCore { impl FixedOutputCore for TigerCore { #[inline] fn finalize_fixed_core(&mut self, buffer: &mut Buffer, out: &mut Output) { - let bs = Self::BlockSize::U64 as u64; + let bs = Self::BlockSize::U64; let pos = buffer.get_pos() as u64; let bit_len = 8 * (pos + bs * self.block_len); @@ -165,7 +165,7 @@ impl UpdateCore for Tiger2Core { impl FixedOutputCore for Tiger2Core { #[inline] fn finalize_fixed_core(&mut self, buffer: &mut Buffer, out: &mut Output) { - let bs = Self::BlockSize::U64 as u64; + let bs = Self::BlockSize::U64; let pos = buffer.get_pos() as u64; let bit_len = 8 * (pos + bs * self.block_len); From 61166a8075219376d46be3996f9e72998232e329 Mon Sep 17 00:00:00 2001 From: Claudia Richoux Date: Thu, 26 Jan 2023 02:16:05 -0500 Subject: [PATCH 2/2] doesn't quite work, pending adding a zeroize to blockbuffer --- Cargo.lock | 68 +++++++++++++++++++++++++++++++++++++++ blake2/Cargo.toml | 2 +- blake2/src/lib.rs | 5 ++- blake2/src/macros.rs | 27 ++++++++++++++++ blake2/src/simd/simdty.rs | 12 +++++++ 5 files changed, 112 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9a05204d6..815f65da9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -193,6 +193,24 @@ version = "0.5.19" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dbf0c48bc1d91375ae5c3cd81e3722dff1abcf81a30960240640d223f59fe0e5" +[[package]] +name = "proc-macro2" +version = "1.0.50" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ef7d57beacfaf2d8aee5937dab7b7f28de3cb8b1828479bb5de2a7106f2bae2" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quote" +version = "1.0.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8856d8364d252a14d474036ea1358d63c9e6965c8e5c1885c18f73d70bff9c7b" +dependencies = [ + "proc-macro2", +] + [[package]] name = "ripemd" version = "0.1.3" @@ -280,6 +298,29 @@ version = "2.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6bdef32e8150c2a081110b42772ffe7d7c9032b606bc226c8260fd97e0976601" +[[package]] +name = "syn" +version = "1.0.107" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1f4064b5b16e03ae50984a5a8ed5d4f8803e6bc1fd170a3cda91a1be4b18e3f5" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "synstructure" +version = "0.12.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f36bdaa60a83aca3921b5259d5400cbf5e90fc51931376a9bd4a0eb79aa7210f" +dependencies = [ + "proc-macro2", + "quote", + "syn", + "unicode-xid", +] + [[package]] name = "tiger" version = "0.2.1" @@ -294,6 +335,18 @@ version = "1.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "497961ef93d974e23eb6f433eb5fe1b7930b659f06d12dec6fc44a8f554c0bba" +[[package]] +name = "unicode-ident" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "84a22b9f218b40614adcb3f4ff08b703773ad44fa9423e4e0d346d5db86e4ebc" + +[[package]] +name = "unicode-xid" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f962df74c8c05a667b5ee8bcf162993134c104e96440b663c8daa176dc772d8c" + [[package]] name = "version_check" version = "0.9.4" @@ -323,3 +376,18 @@ name = "zeroize" version = "1.5.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c394b5bd0c6f669e7275d9c20aa90ae064cb22e75a1cad54e1b34088034b149f" +dependencies = [ + "zeroize_derive", +] + +[[package]] +name = "zeroize_derive" +version = "1.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44bf07cb3e50ea2003396695d58bf46bc9887a1f362260446fad6bc4e79bd36c" +dependencies = [ + "proc-macro2", + "quote", + "syn", + "synstructure", +] diff --git a/blake2/Cargo.toml b/blake2/Cargo.toml index 92d1f67b0..c95650042 100644 --- a/blake2/Cargo.toml +++ b/blake2/Cargo.toml @@ -13,7 +13,7 @@ categories = ["cryptography", "no-std"] [dependencies] digest = { version = "0.10.3", features = ["mac"] } -zeroize = { version = "1.4.0", optional = true} +zeroize = { version = "1.4.0", features = ["zeroize_derive"], optional = true} [dev-dependencies] digest = { version = "0.10.3", features = ["dev"] } diff --git a/blake2/src/lib.rs b/blake2/src/lib.rs index 85a6b1c28..6306545fc 100644 --- a/blake2/src/lib.rs +++ b/blake2/src/lib.rs @@ -82,7 +82,7 @@ extern crate std; pub use digest::{self, Digest}; -use core::{convert::TryInto, fmt, marker::PhantomData, ops::Div}; +use core::{convert::TryInto, fmt, marker::PhantomData, ops::Div, borrow::BorrowMut}; use digest::{ block_buffer::{Lazy, LazyBuffer}, consts::{U128, U32, U4, U64}, @@ -99,6 +99,9 @@ use digest::{ #[cfg(feature = "reset")] use digest::{FixedOutputReset, Reset}; +#[cfg(feature = "zeroize")] +use zeroize::{Zeroize, ZeroizeOnDrop}; + mod as_bytes; mod consts; diff --git a/blake2/src/macros.rs b/blake2/src/macros.rs index 917a212c8..9d1cc1d55 100644 --- a/blake2/src/macros.rs +++ b/blake2/src/macros.rs @@ -5,6 +5,7 @@ macro_rules! blake2_impl { $vardoc:expr, $doc:expr, ) => { #[derive(Clone)] + #[cfg_attr(feature = "zeroize", derive(Zeroize))] #[doc=$vardoc] pub struct $name { h: [$vec; 2], @@ -285,12 +286,16 @@ macro_rules! blake2_mac_impl { salt: &[u8], persona: &[u8], ) -> Result { + // since key is passed by reference, it is the caller's responsibility to zeroize it. + // TODO (reviewer): is this true? is this the right way to do it? let kl = key.len(); let bs = <$hash as BlockSizeUser>::BlockSize::USIZE; let qbs = bs / 4; if kl > bs || salt.len() > qbs || persona.len() > qbs { return Err(InvalidLength); } + // padded_key gets owned by buffer, so is zeroized on drop when zeroize is enabled. + // TODO (reviewer): is this true? is this the right way to do it? let mut padded_key = Block::<$hash>::default(); padded_key[..kl].copy_from_slice(key); Ok(Self { @@ -321,16 +326,20 @@ macro_rules! blake2_mac_impl { LeEq: NonZero, { #[inline] + /// caller is responsible for zeroizing the key. fn new(key: &Key) -> Self { Self::new_from_slice(key).expect("Key has correct length") } #[inline] + /// caller is responsible for zeroizing the key. fn new_from_slice(key: &[u8]) -> Result { let kl = key.len(); if kl > ::KeySize::USIZE { return Err(InvalidLength); } + // padded_key gets owned by buffer, so is zeroized on drop when zeroize is enabled. + // TODO (reviewer): is this true? is this the right way to do it? let mut padded_key = Block::<$hash>::default(); padded_key[..kl].copy_from_slice(key); Ok(Self { @@ -378,6 +387,7 @@ macro_rules! blake2_mac_impl { let mut full_res = Default::default(); core.finalize_variable_core(buffer, &mut full_res); out.copy_from_slice(&full_res[..OutSize::USIZE]); + // self including core is zeroized on drop } } @@ -390,6 +400,9 @@ macro_rules! blake2_mac_impl { fn reset(&mut self) { self.core.reset(); let kl = self.key_block.len(); + // zeroize the buffer before you replace it (feeling pretty confident about this one?) + self.buffer.zeroize(); + // TODO (reviewer) does padded key need to be zeroized here? I think not but just flagging the possibility let mut padded_key = Block::<$hash>::default(); padded_key[..kl].copy_from_slice(&self.key_block); self.buffer = LazyBuffer::new(&padded_key); @@ -428,5 +441,19 @@ macro_rules! blake2_mac_impl { write!(f, "{}{} {{ ... }}", stringify!($name), OutSize::USIZE) } } + + #[cfg(feature = "zeroize")] + impl Drop for $name + where + OutSize: ArrayLength + IsLessOrEqual<$max_size>, + LeEq: NonZero, + { + fn drop(&mut self) { + self.core.zeroize(); + self.buffer.zeroize(); + #[cfg(feature = "reset")] + self.key_block.zeroize(); + } + } }; } diff --git a/blake2/src/simd/simdty.rs b/blake2/src/simd/simdty.rs index 008b8b48c..ecdfd345b 100644 --- a/blake2/src/simd/simdty.rs +++ b/blake2/src/simd/simdty.rs @@ -8,6 +8,8 @@ #![allow(dead_code, non_camel_case_types)] use crate::as_bytes::Safe; +#[cfg(feature = "zeroize")] +use zeroize::Zeroize; #[cfg(feature = "simd")] macro_rules! decl_simd { @@ -75,3 +77,13 @@ unsafe impl Safe for Simd4 {} unsafe impl Safe for Simd8 {} unsafe impl Safe for Simd16 {} unsafe impl Safe for Simd32 {} + +#[cfg(feature = "zeroize")] +impl Zeroize for Simd4 { + fn zeroize(&mut self) { + self.0.zeroize(); + self.1.zeroize(); + self.2.zeroize(); + self.3.zeroize(); + } +}