Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hmac: Use non-panicking Digest API throughout. #2193

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions src/digest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -313,6 +314,16 @@ pub struct Digest {
}

impl Digest {
pub(crate) fn compute_from(
algorithm: &'static Algorithm,
data: &[u8],
cpu: cpu::Features,
) -> Result<Self, FinishError> {
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 {
Expand Down
57 changes: 41 additions & 16 deletions src/hmac.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -139,7 +143,7 @@
///
/// 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]
Expand Down Expand Up @@ -175,17 +179,21 @@
algorithm: Algorithm,
rng: &dyn rand::SecureRandom,
) -> Result<Self, error::Unspecified> {
Self::construct(algorithm, |buf| rng.fill(buf))
Self::construct(algorithm, |buf| rng.fill(buf), cpu::features())
}

fn construct<F>(algorithm: Algorithm, fill: F) -> Result<Self, error::Unspecified>
fn construct<F>(
algorithm: Algorithm,
fill: F,
cpu: cpu::Features,
) -> Result<Self, error::Unspecified>
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
Expand All @@ -208,8 +216,16 @@
/// `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<Self, digest::FinishError> {
let digest_alg = algorithm.0;
let mut key = Self {
inner: digest::BlockContext::new(digest_alg),
Expand All @@ -222,7 +238,7 @@
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()
};

Expand All @@ -249,14 +265,20 @@
let leftover = key.outer.update(padded_key, cpu_features);
debug_assert_eq!(leftover.len(), 0);

key
Ok(key)
}

/// The digest algorithm for the key.
#[inline]
pub fn algorithm(&self) -> Algorithm {
Algorithm(self.inner.algorithm)
}

fn try_sign(&self, data: &[u8], cpu: cpu::Features) -> Result<Tag, digest::FinishError> {
let mut ctx = Context::with_key(self);
ctx.update(data);
ctx.try_sign(cpu)
}
}

impl hkdf::KeyType for Algorithm {
Expand All @@ -267,7 +289,7 @@

impl From<hkdf::Okm<'_, Algorithm>> for Key {
fn from(okm: hkdf::Okm<Algorithm>) -> Self {
Self::construct(*okm.len(), |buf| okm.fill(buf)).unwrap()
Self::construct(*okm.len(), |buf| okm.fill(buf), cpu::features()).unwrap()

Check warning on line 292 in src/hmac.rs

View check run for this annotation

Codecov / codecov/patch

src/hmac.rs#L292

Added line #L292 was not covered by tests
}
}

Expand Down Expand Up @@ -312,11 +334,12 @@
/// 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<Tag, digest::FinishError> {
let cpu_features = cpu::features();
fn try_sign(self, cpu_features: cpu::Features) -> Result<Tag, digest::FinishError> {
let inner = self.inner.try_finish(cpu_features)?;
let inner = inner.as_ref();
let num_pending = inner.len();
Expand All @@ -337,9 +360,9 @@
/// 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
Expand All @@ -350,7 +373,9 @@
///
/// 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)]
Expand Down