From be133975703f3a62d37fffc554f6d868d445b0a1 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Fri, 30 Aug 2024 06:20:30 +0200 Subject: [PATCH] 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) }