From 94c0614bda8bca6077e76fbdd1414e32119a02d5 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Fri, 30 Aug 2024 05:49:52 +0200 Subject: [PATCH 1/2] Enforce that `Hash::Bytes` is an array In the future we would like to guarantee the correctness of `LEN` which is currently not entirely possible, so this at least adds a sealed trait enforcing the `Bytes` type to be an array. Consumers concerned about the validity of the length can access the `LEN` constant on `Bytes` instead to get the correct length of the array. --- hashes/src/hmac.rs | 1 - hashes/src/internal_macros.rs | 1 - hashes/src/lib.rs | 21 +++++++++++++++++++-- hashes/src/util.rs | 1 - 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/hashes/src/hmac.rs b/hashes/src/hmac.rs index 7b8c1a401..9de980d04 100644 --- a/hashes/src/hmac.rs +++ b/hashes/src/hmac.rs @@ -135,7 +135,6 @@ impl GeneralHash for Hmac { impl Hash for Hmac { type Bytes = T::Bytes; - const LEN: usize = T::LEN; fn from_slice(sl: &[u8]) -> Result, FromSliceError> { T::from_slice(sl).map(Hmac) } diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs index a105a66c3..3e362d23f 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -108,7 +108,6 @@ macro_rules! hash_trait_impls { impl<$($gen: $gent),*> crate::Hash for Hash<$($gen),*> { type Bytes = [u8; $bits / 8]; - const LEN: usize = $bits / 8; const DISPLAY_BACKWARD: bool = $reverse; fn from_slice(sl: &[u8]) -> $crate::_export::_core::result::Result, $crate::FromSliceError> { diff --git a/hashes/src/lib.rs b/hashes/src/lib.rs index 897f97b8f..df2be7556 100644 --- a/hashes/src/lib.rs +++ b/hashes/src/lib.rs @@ -274,10 +274,10 @@ pub trait Hash: + convert::AsRef<[u8]> { /// The byte array that represents the hash internally. - type Bytes: hex::FromHex + Copy; + type Bytes: hex::FromHex + Copy + IsByteArray /* is still unsupported by Rust */; /// Length of the hash, in bytes. - const LEN: usize; + const LEN: usize = Self::Bytes::LEN; /// Copies a byte slice into a hash object. fn from_slice(sl: &[u8]) -> Result; @@ -297,6 +297,23 @@ pub trait Hash: fn from_byte_array(bytes: Self::Bytes) -> Self; } +/// Ensures that a type is an array. +pub trait IsByteArray: AsRef<[u8]> + sealed::IsByteArray { + /// The length of the array. + const LEN: usize; +} + +impl IsByteArray for [u8; N] { + const LEN: usize = N; +} + +mod sealed { + #[doc(hidden)] + pub trait IsByteArray { } + + impl IsByteArray for [u8; N] { } +} + /// Attempted to create a hash from an invalid length slice. #[derive(Debug, Clone, PartialEq, Eq)] pub struct FromSliceError { diff --git a/hashes/src/util.rs b/hashes/src/util.rs index 58d08d9f1..2956415d6 100644 --- a/hashes/src/util.rs +++ b/hashes/src/util.rs @@ -231,7 +231,6 @@ macro_rules! hash_newtype { impl $crate::Hash for $newtype { type Bytes = <$hash as $crate::Hash>::Bytes; - const LEN: usize = <$hash as $crate::Hash>::LEN; const DISPLAY_BACKWARD: bool = $crate::hash_newtype_get_direction!($hash, $(#[$($type_attrs)*])*); #[inline] From be133975703f3a62d37fffc554f6d868d445b0a1 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Fri, 30 Aug 2024 06:20:30 +0200 Subject: [PATCH 2/2] Make hmac & hkdf more robust against buggy `Hash` Use the newly added requirement that `Hash::Bytes` is an array to protect the implementation of hmac and hkdf against implementations that would accidentally return slices of different sizes from the `AsRef` impl. --- hashes/src/hkdf.rs | 16 ++++++++-------- hashes/src/hmac.rs | 10 ++++++---- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/hashes/src/hkdf.rs b/hashes/src/hkdf.rs index 1de1cd616..db7a624a0 100644 --- a/hashes/src/hkdf.rs +++ b/hashes/src/hkdf.rs @@ -11,7 +11,7 @@ use alloc::vec; use alloc::vec::Vec; use core::fmt; -use crate::{GeneralHash, HashEngine, Hmac, HmacEngine}; +use crate::{GeneralHash, HashEngine, Hmac, HmacEngine, IsByteArray}; /// Output keying material max length multiple. const MAX_OUTPUT_BLOCKS: usize = 255; @@ -54,14 +54,14 @@ where /// but the info must be independent from the ikm for security. pub fn expand(&self, info: &[u8], okm: &mut [u8]) -> Result<(), MaxLengthError> { // Length of output keying material in bytes must be less than 255 * hash length. - if okm.len() > (MAX_OUTPUT_BLOCKS * T::LEN) { - return Err(MaxLengthError { max: MAX_OUTPUT_BLOCKS * T::LEN }); + if okm.len() > (MAX_OUTPUT_BLOCKS * T::Bytes::LEN) { + return Err(MaxLengthError { max: MAX_OUTPUT_BLOCKS * T::Bytes::LEN }); } // Counter starts at "1" based on RFC5869 spec and is committed to in the hash. let mut counter = 1u8; // Ceiling calculation for the total number of blocks (iterations) required for the expand. - let total_blocks = (okm.len() + T::LEN - 1) / T::LEN; + let total_blocks = (okm.len() + T::Bytes::LEN - 1) / T::Bytes::LEN; while counter <= total_blocks as u8 { let mut hmac_engine: HmacEngine = HmacEngine::new(self.prk.as_ref()); @@ -69,18 +69,18 @@ where // First block does not have a previous block, // all other blocks include last block in the HMAC input. if counter != 1u8 { - let previous_start_index = (counter as usize - 2) * T::LEN; - let previous_end_index = (counter as usize - 1) * T::LEN; + let previous_start_index = (counter as usize - 2) * T::Bytes::LEN; + let previous_end_index = (counter as usize - 1) * T::Bytes::LEN; hmac_engine.input(&okm[previous_start_index..previous_end_index]); } hmac_engine.input(info); hmac_engine.input(&[counter]); let t = Hmac::from_engine(hmac_engine); - let start_index = (counter as usize - 1) * T::LEN; + let start_index = (counter as usize - 1) * T::Bytes::LEN; // Last block might not take full hash length. let end_index = - if counter == (total_blocks as u8) { okm.len() } else { counter as usize * T::LEN }; + if counter == (total_blocks as u8) { okm.len() } else { counter as usize * T::Bytes::LEN }; okm[start_index..end_index].copy_from_slice(&t.as_ref()[0..(end_index - start_index)]); diff --git a/hashes/src/hmac.rs b/hashes/src/hmac.rs index 9de980d04..9c0741e60 100644 --- a/hashes/src/hmac.rs +++ b/hashes/src/hmac.rs @@ -72,10 +72,11 @@ impl HmacEngine { if key.len() > T::Engine::BLOCK_SIZE { let hash = ::hash(key); - for (b_i, b_h) in ipad.iter_mut().zip(hash.as_ref()) { + let hash = hash.as_byte_array().as_ref(); + for (b_i, b_h) in ipad.iter_mut().zip(hash) { *b_i ^= *b_h; } - for (b_o, b_h) in opad.iter_mut().zip(hash.as_ref()) { + for (b_o, b_h) in opad.iter_mut().zip(hash) { *b_o ^= *b_h; } } else { @@ -119,7 +120,8 @@ impl fmt::LowerHex for Hmac { } impl convert::AsRef<[u8]> for Hmac { - fn as_ref(&self) -> &[u8] { self.0.as_ref() } + // Calling as_byte_array is more reliable + fn as_ref(&self) -> &[u8] { self.0.as_byte_array().as_ref() } } impl GeneralHash for Hmac { @@ -127,7 +129,7 @@ impl GeneralHash for Hmac { fn from_engine(mut e: HmacEngine) -> Hmac { let ihash = T::from_engine(e.iengine); - e.oengine.input(ihash.as_ref()); + e.oengine.input(ihash.as_byte_array().as_ref()); let ohash = T::from_engine(e.oengine); Hmac(ohash) }