From 0062b9ef45a26ecb948496dce3737ed5ad30bf08 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Wed, 18 Dec 2024 17:30:23 -0800 Subject: [PATCH] hmac: Use non-panicking `Digest` API throughout. Make the nature of panics in the HMAC module clearer--`ring::hmac` APIs only panic when the input is too long. --- src/digest.rs | 17 +++++++++++--- src/hmac.rs | 61 +++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 59 insertions(+), 19 deletions(-) diff --git a/src/digest.rs b/src/digest.rs index 39427262a..23145ebdc 100644 --- a/src/digest.rs +++ b/src/digest.rs @@ -298,9 +298,10 @@ impl Context { /// # } /// ``` pub fn digest(algorithm: &'static Algorithm, data: &[u8]) -> Digest { - let mut ctx = Context::new(algorithm); - ctx.update(data); - ctx.finish() + let cpu = cpu::features(); + Digest::compute_from(algorithm, data, cpu) + .map_err(error::Unspecified::from) + .unwrap() } /// A calculated digest value. @@ -313,6 +314,16 @@ pub struct Digest { } impl Digest { + pub(crate) fn compute_from( + algorithm: &'static Algorithm, + data: &[u8], + cpu: cpu::Features, + ) -> Result { + let mut ctx = Context::new(algorithm); + ctx.update(data); + ctx.try_finish(cpu) + } + /// The algorithm that was used to calculate the digest value. #[inline(always)] pub fn algorithm(&self) -> &'static Algorithm { diff --git a/src/hmac.rs b/src/hmac.rs index a62dbd731..c864df7f7 100644 --- a/src/hmac.rs +++ b/src/hmac.rs @@ -109,7 +109,11 @@ //! [code for `ring::hkdf`]: //! https://github.com/briansmith/ring/blob/main/src/hkdf.rs -use crate::{constant_time, cpu, digest, error, hkdf, rand}; +use crate::{ + constant_time, cpu, + digest::{self, Digest}, + error, hkdf, rand, +}; /// An HMAC algorithm. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -139,7 +143,7 @@ pub static HMAC_SHA512: Algorithm = Algorithm(&digest::SHA512); /// /// For a given tag `t`, use `t.as_ref()` to get the tag value as a byte slice. #[derive(Clone, Copy, Debug)] -pub struct Tag(digest::Digest); +pub struct Tag(Digest); impl AsRef<[u8]> for Tag { #[inline] @@ -175,17 +179,21 @@ impl Key { algorithm: Algorithm, rng: &dyn rand::SecureRandom, ) -> Result { - Self::construct(algorithm, |buf| rng.fill(buf)) + Self::construct(algorithm, |buf| rng.fill(buf), cpu::features()) } - fn construct(algorithm: Algorithm, fill: F) -> Result + fn construct( + algorithm: Algorithm, + fill: F, + cpu: cpu::Features, + ) -> Result where F: FnOnce(&mut [u8]) -> Result<(), error::Unspecified>, { let mut key_bytes = [0; digest::MAX_OUTPUT_LEN]; let key_bytes = &mut key_bytes[..algorithm.0.output_len()]; fill(key_bytes)?; - Ok(Self::new(algorithm, key_bytes)) + Self::try_new(algorithm, key_bytes, cpu).map_err(error::Unspecified::from) } /// Construct an HMAC signing key using the given digest algorithm and key @@ -208,8 +216,16 @@ impl Key { /// `digest_alg.output_len * 8` bits. Support for such keys is likely to be /// removed in a future version of *ring*. pub fn new(algorithm: Algorithm, key_value: &[u8]) -> Self { - let cpu_features = cpu::features(); + Self::try_new(algorithm, key_value, cpu::features()) + .map_err(error::Unspecified::from) + .unwrap() + } + fn try_new( + algorithm: Algorithm, + key_value: &[u8], + cpu_features: cpu::Features, + ) -> Result { let digest_alg = algorithm.0; let mut key = Self { inner: digest::BlockContext::new(digest_alg), @@ -222,7 +238,7 @@ impl Key { let key_value = if key_value.len() <= block_len { key_value } else { - key_hash = digest::digest(digest_alg, key_value); + key_hash = Digest::compute_from(digest_alg, key_value, cpu_features)?; key_hash.as_ref() }; @@ -249,7 +265,7 @@ impl Key { let leftover = key.outer.update(padded_key, cpu_features); debug_assert_eq!(leftover.len(), 0); - key + Ok(key) } /// The digest algorithm for the key. @@ -257,6 +273,16 @@ impl Key { pub fn algorithm(&self) -> Algorithm { Algorithm(self.inner.algorithm) } + + fn try_sign( + &self, + data: &[u8], + cpu: cpu::Features, + ) -> Result { + let mut ctx = Context::with_key(self); + ctx.update(data); + ctx.try_sign(cpu) + } } impl hkdf::KeyType for Algorithm { @@ -267,7 +293,7 @@ impl hkdf::KeyType for Algorithm { impl From> for Key { fn from(okm: hkdf::Okm) -> Self { - Self::construct(*okm.len(), |buf| okm.fill(buf)).unwrap() + Self::construct(*okm.len(), |buf| okm.fill(buf), cpu::features()).unwrap() } } @@ -312,11 +338,12 @@ impl Context { /// the return value of `sign` to a tag. Use `verify` for verification /// instead. pub fn sign(self) -> Tag { - self.try_sign().map_err(error::Unspecified::from).unwrap() + self.try_sign(cpu::features()) + .map_err(error::Unspecified::from) + .unwrap() } - fn try_sign(self) -> Result { - let cpu_features = cpu::features(); + fn try_sign(self, cpu_features: cpu::Features) -> Result { let inner = self.inner.try_finish(cpu_features)?; let inner = inner.as_ref(); let num_pending = inner.len(); @@ -337,9 +364,9 @@ impl Context { /// It is generally not safe to implement HMAC verification by comparing the /// return value of `sign` to a tag. Use `verify` for verification instead. pub fn sign(key: &Key, data: &[u8]) -> Tag { - let mut ctx = Context::with_key(key); - ctx.update(data); - ctx.sign() + key.try_sign(data, cpu::features()) + .map_err(error::Unspecified::from) + .unwrap() } /// Calculates the HMAC of `data` using the signing key `key`, and verifies @@ -350,7 +377,9 @@ pub fn sign(key: &Key, data: &[u8]) -> Tag { /// /// The verification will be done in constant time to prevent timing attacks. pub fn verify(key: &Key, data: &[u8], tag: &[u8]) -> Result<(), error::Unspecified> { - constant_time::verify_slices_are_equal(sign(key, data).as_ref(), tag) + let cpu = cpu::features(); + let computed = key.try_sign(data, cpu)?; + constant_time::verify_slices_are_equal(computed.as_ref(), tag) } #[cfg(test)]