Adding an optional zeroize feature that enables ZeroizeOnDrop for blake2 hashers

blake2/Cargo.toml

@@ -13,6 +13,7 @@ categories = ["cryptography", "no-std"]
 
 [dependencies]
 digest = { version = "0.10.3", features = ["mac"] }
+zeroize = { version = "1.4.0", features = ["zeroize_derive"], 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

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;
 

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<Self, InvalidLength> {
+                // 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<OutSize, $max_size>: NonZero,
         {
             #[inline]
+            /// caller is responsible for zeroizing the key.
             fn new(key: &Key<Self>) -> 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<Self, InvalidLength> {
                 let kl = key.len();
                 if kl > <Self as KeySizeUser>::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<OutSize> Drop for $name<OutSize>
+        where
+            OutSize: ArrayLength<u8> + IsLessOrEqual<$max_size>,
+            LeEq<OutSize, $max_size>: NonZero,
+        {
+            fn drop(&mut self) {
+                self.core.zeroize();
+                self.buffer.zeroize();
+                #[cfg(feature = "reset")]
+                self.key_block.zeroize();
+            }
+        }
     };
 }

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<T: Safe> Safe for Simd4<T> {}
 unsafe impl<T: Safe> Safe for Simd8<T> {}
 unsafe impl<T: Safe> Safe for Simd16<T> {}
 unsafe impl<T: Safe> Safe for Simd32<T> {}
+
+#[cfg(feature = "zeroize")]
+impl<T: Zeroize> Zeroize for Simd4<T> {
+    fn zeroize(&mut self) {
+        self.0.zeroize();
+        self.1.zeroize();
+        self.2.zeroize();
+        self.3.zeroize();
+    }
+}

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<Self>, out: &mut Output<Self>) {
-        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<Self>, out: &mut Output<Self>) {
-        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);