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.
This commit is contained in:
Martin Habovstiak 2024-08-30 06:20:30 +02:00
parent 94c0614bda
commit be13397570
2 changed files with 14 additions and 12 deletions

View File

@ -11,7 +11,7 @@ use alloc::vec;
use alloc::vec::Vec; use alloc::vec::Vec;
use core::fmt; use core::fmt;
use crate::{GeneralHash, HashEngine, Hmac, HmacEngine}; use crate::{GeneralHash, HashEngine, Hmac, HmacEngine, IsByteArray};
/// Output keying material max length multiple. /// Output keying material max length multiple.
const MAX_OUTPUT_BLOCKS: usize = 255; const MAX_OUTPUT_BLOCKS: usize = 255;
@ -54,14 +54,14 @@ where
/// but the info must be independent from the ikm for security. /// but the info must be independent from the ikm for security.
pub fn expand(&self, info: &[u8], okm: &mut [u8]) -> Result<(), MaxLengthError> { 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. // Length of output keying material in bytes must be less than 255 * hash length.
if okm.len() > (MAX_OUTPUT_BLOCKS * T::LEN) { if okm.len() > (MAX_OUTPUT_BLOCKS * T::Bytes::LEN) {
return Err(MaxLengthError { max: MAX_OUTPUT_BLOCKS * T::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. // Counter starts at "1" based on RFC5869 spec and is committed to in the hash.
let mut counter = 1u8; let mut counter = 1u8;
// Ceiling calculation for the total number of blocks (iterations) required for the expand. // 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 { while counter <= total_blocks as u8 {
let mut hmac_engine: HmacEngine<T> = HmacEngine::new(self.prk.as_ref()); let mut hmac_engine: HmacEngine<T> = HmacEngine::new(self.prk.as_ref());
@ -69,18 +69,18 @@ where
// First block does not have a previous block, // First block does not have a previous block,
// all other blocks include last block in the HMAC input. // all other blocks include last block in the HMAC input.
if counter != 1u8 { if counter != 1u8 {
let previous_start_index = (counter as usize - 2) * T::LEN; let previous_start_index = (counter as usize - 2) * T::Bytes::LEN;
let previous_end_index = (counter as usize - 1) * T::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(&okm[previous_start_index..previous_end_index]);
} }
hmac_engine.input(info); hmac_engine.input(info);
hmac_engine.input(&[counter]); hmac_engine.input(&[counter]);
let t = Hmac::from_engine(hmac_engine); 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. // Last block might not take full hash length.
let end_index = 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)]); okm[start_index..end_index].copy_from_slice(&t.as_ref()[0..(end_index - start_index)]);

View File

@ -72,10 +72,11 @@ impl<T: GeneralHash> HmacEngine<T> {
if key.len() > T::Engine::BLOCK_SIZE { if key.len() > T::Engine::BLOCK_SIZE {
let hash = <T as GeneralHash>::hash(key); let hash = <T as GeneralHash>::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; *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; *b_o ^= *b_h;
} }
} else { } else {
@ -119,7 +120,8 @@ impl<T: GeneralHash> fmt::LowerHex for Hmac<T> {
} }
impl<T: GeneralHash> convert::AsRef<[u8]> for Hmac<T> { impl<T: GeneralHash> convert::AsRef<[u8]> for Hmac<T> {
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<T: GeneralHash> GeneralHash for Hmac<T> { impl<T: GeneralHash> GeneralHash for Hmac<T> {
@ -127,7 +129,7 @@ impl<T: GeneralHash> GeneralHash for Hmac<T> {
fn from_engine(mut e: HmacEngine<T>) -> Hmac<T> { fn from_engine(mut e: HmacEngine<T>) -> Hmac<T> {
let ihash = T::from_engine(e.iengine); 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); let ohash = T::from_engine(e.oengine);
Hmac(ohash) Hmac(ohash)
} }