From 7ac72730133507b3597611a55d6e7848aece74e2 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 26 Mar 2025 18:45:13 +0000 Subject: [PATCH 1/2] [hashes] Disable fixed-time equality cmp when building for fuzzers Fuzzers want to break memcmp calls into separate comparisons for coverage monitoring, allowing them to not-quite-brute-force find inputs that fully match. Thus, we disable our fancy fixed-time comparison when built with the `hashes_fuzz` cfg. --- hashes/src/cmp.rs | 67 +++++++++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 29 deletions(-) diff --git a/hashes/src/cmp.rs b/hashes/src/cmp.rs index 3a72b389f..99ccea994 100644 --- a/hashes/src/cmp.rs +++ b/hashes/src/cmp.rs @@ -15,41 +15,50 @@ /// As of rust 1.31.0 disassembly looks completely within reason for this, see /// . pub fn fixed_time_eq(a: &[u8], b: &[u8]) -> bool { - assert!(a.len() == b.len()); - let count = a.len(); - let lhs = &a[..count]; - let rhs = &b[..count]; + #[cfg(hashes_fuzz)] + { + // Fuzzers want to break memcmp calls into separate comparisons for coverage monitoring, + // so we avoid our fancy fixed-time comparison below for fuzzers. + a == b + } + #[cfg(not(hashes_fuzz))] + { + assert!(a.len() == b.len()); + let count = a.len(); + let lhs = &a[..count]; + let rhs = &b[..count]; - let mut r: u8 = 0; - for i in 0..count { - let mut rs = unsafe { core::ptr::read_volatile(&r) }; - rs |= lhs[i] ^ rhs[i]; - unsafe { - core::ptr::write_volatile(&mut r, rs); + let mut r: u8 = 0; + for i in 0..count { + let mut rs = unsafe { core::ptr::read_volatile(&r) }; + rs |= lhs[i] ^ rhs[i]; + unsafe { + core::ptr::write_volatile(&mut r, rs); + } } - } - { - let mut t = unsafe { core::ptr::read_volatile(&r) }; - t |= t >> 4; - unsafe { - core::ptr::write_volatile(&mut r, t); + { + let mut t = unsafe { core::ptr::read_volatile(&r) }; + t |= t >> 4; + unsafe { + core::ptr::write_volatile(&mut r, t); + } } - } - { - let mut t = unsafe { core::ptr::read_volatile(&r) }; - t |= t >> 2; - unsafe { - core::ptr::write_volatile(&mut r, t); + { + let mut t = unsafe { core::ptr::read_volatile(&r) }; + t |= t >> 2; + unsafe { + core::ptr::write_volatile(&mut r, t); + } } - } - { - let mut t = unsafe { core::ptr::read_volatile(&r) }; - t |= t >> 1; - unsafe { - core::ptr::write_volatile(&mut r, t); + { + let mut t = unsafe { core::ptr::read_volatile(&r) }; + t |= t >> 1; + unsafe { + core::ptr::write_volatile(&mut r, t); + } } + unsafe { (::core::ptr::read_volatile(&r) & 1) == 0 } } - unsafe { (::core::ptr::read_volatile(&r) & 1) == 0 } } #[cfg(test)] From 2d9e240fb6ae13e6139713f9bb8ccb51e5dc0bff Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 28 Mar 2025 13:42:46 +0000 Subject: [PATCH 2/2] [hashes] Use `fixed_time_eq` for `Hmac::eq` When someone is checking if an `Hmac` is equal to some other `Hmac`, its fairly common for them to be doing a constant-time-ness-sensitive operation. Thus, here we default to our existing `fixed_time_eq` method for `PartialEq` on `Hamc`, rather than the naive Rust slice comparison. While we should consider doing the same for all hash types, we do not yet do so here. --- hashes/src/hmac/mod.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/hashes/src/hmac/mod.rs b/hashes/src/hmac/mod.rs index 2d70253fc..d69c3ccc2 100644 --- a/hashes/src/hmac/mod.rs +++ b/hashes/src/hmac/mod.rs @@ -15,8 +15,9 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; use crate::{Hash, HashEngine}; /// A hash computed from a RFC 2104 HMAC. Parameterized by the underlying hash function. -#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Copy, Clone, PartialOrd, Ord, Hash)] #[repr(transparent)] +#[allow(clippy::derived_hash_with_manual_eq)] pub struct Hmac(T); impl str::FromStr for Hmac { @@ -24,6 +25,14 @@ impl str::FromStr for Hmac { fn from_str(s: &str) -> Result { Ok(Hmac(str::FromStr::from_str(s)?)) } } +impl PartialEq for Hmac { + fn eq(&self, other: &Self) -> bool { + crate::cmp::fixed_time_eq(self.as_ref(), other.as_ref()) + } +} + +impl Eq for Hmac {} + /// Pair of underlying hash engines, used for the inner and outer hash of HMAC. #[derive(Debug, Clone)] pub struct HmacEngine {