Skip to content

Commit

Permalink
digest/hmac: Clarify error handling.
Browse files Browse the repository at this point in the history
Computing an HMAC can only fail if the input is too long. The
other `FinishError` condition, where we failed to meet the
precondition for `BlockContext::try_finish()`, is obviously,
using information local to the function, unreachable.

Similarly, when there is only a single slice of bytes as input,
then the `InputTooLongError` should be the `usize` type, not
`u64`. The case where the u64 -> usize condition fails is not
interesting, so we use a saturating conversion instead of
giving ourselves the future work of proving that it is
unreachable.

Finally, remove the `From<FinishError> for error::Unspecified`
implementation, in favor of `error::erase`, following the new
convention.
  • Loading branch information
briansmith committed Jan 9, 2025
1 parent 826598e commit fa0c22b
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 24 deletions.
33 changes: 19 additions & 14 deletions src/digest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ use self::{
};
use crate::{
bits::{BitLength, FromByteLen as _},
cpu, debug,
error::{self, InputTooLongError},
cpu, debug, error,
polyfill::{self, slice, sliceutil},
};
use core::num::Wrapping;
Expand Down Expand Up @@ -145,17 +144,19 @@ impl BlockContext {
}
}

pub(crate) type InputTooLongError = error::InputTooLongError<u64>;

pub(crate) enum FinishError {
#[allow(dead_code)]
InputTooLong(InputTooLongError<u64>),
InputTooLong(InputTooLongError),
#[allow(dead_code)]
PendingNotAPartialBlock(usize),
}

impl FinishError {
#[cold]
#[inline(never)]
fn input_too_long(source: InputTooLongError<u64>) -> Self {
fn input_too_long(source: InputTooLongError) -> Self {
Self::InputTooLong(source)
}

Expand All @@ -170,12 +171,6 @@ impl FinishError {
}
}

impl From<FinishError> for error::Unspecified {
fn from(_: FinishError) -> Self {
Self
}
}

/// A context for multi-step (Init-Update-Finish) digest calculations.
///
/// # Examples
Expand Down Expand Up @@ -270,13 +265,23 @@ impl Context {
pub fn finish(self) -> Digest {
let cpu = cpu::features();
self.try_finish(cpu)
.map_err(error::Unspecified::from)
.map_err(error::erase::<InputTooLongError>)
.unwrap()
}

pub(crate) fn try_finish(mut self, cpu_features: cpu::Features) -> Result<Digest, FinishError> {
pub(crate) fn try_finish(
mut self,
cpu_features: cpu::Features,
) -> Result<Digest, InputTooLongError> {
self.block
.try_finish(&mut self.pending, self.num_pending, cpu_features)
.map_err(|err| match err {
FinishError::InputTooLong(i) => i,
FinishError::PendingNotAPartialBlock(_) => {
// Due to invariant.
unreachable!()
}
})
}

/// The algorithm that this context is using.
Expand Down Expand Up @@ -304,7 +309,7 @@ impl Context {
pub fn digest(algorithm: &'static Algorithm, data: &[u8]) -> Digest {
let cpu = cpu::features();
Digest::compute_from(algorithm, data, cpu)
.map_err(error::Unspecified::from)
.map_err(error::erase::<InputTooLongError>)
.unwrap()
}

Expand All @@ -322,7 +327,7 @@ impl Digest {
algorithm: &'static Algorithm,
data: &[u8],
cpu: cpu::Features,
) -> Result<Self, FinishError> {
) -> Result<Self, InputTooLongError> {
let mut ctx = Context::new(algorithm);
ctx.update(data);
ctx.try_finish(cpu)
Expand Down
49 changes: 39 additions & 10 deletions src/hmac.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,12 @@
use crate::{
constant_time, cpu,
digest::{self, Digest},
digest::{self, Digest, FinishError},
error, hkdf, rand,
};

pub(crate) use crate::digest::InputTooLongError;

/// An HMAC algorithm.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct Algorithm(&'static digest::Algorithm);
Expand Down Expand Up @@ -193,7 +195,7 @@ impl Key {
let mut key_bytes = [0; digest::MAX_OUTPUT_LEN];
let key_bytes = &mut key_bytes[..algorithm.0.output_len()];
fill(key_bytes)?;
Self::try_new(algorithm, key_bytes, cpu).map_err(error::Unspecified::from)
Self::try_new(algorithm, key_bytes, cpu).map_err(error::erase::<InputTooLongError>)
}

/// Construct an HMAC signing key using the given digest algorithm and key
Expand All @@ -217,15 +219,15 @@ impl Key {
/// removed in a future version of *ring*.
pub fn new(algorithm: Algorithm, key_value: &[u8]) -> Self {
Self::try_new(algorithm, key_value, cpu::features())
.map_err(error::Unspecified::from)
.map_err(error::erase::<InputTooLongError>)
.unwrap()
}

fn try_new(
algorithm: Algorithm,
key_value: &[u8],
cpu_features: cpu::Features,
) -> Result<Self, digest::FinishError> {
) -> Result<Self, InputTooLongError> {
let digest_alg = algorithm.0;
let mut key = Self {
inner: digest::BlockContext::new(digest_alg),
Expand Down Expand Up @@ -272,14 +274,16 @@ impl Key {
Algorithm(self.inner.algorithm)
}

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

fn verify(&self, data: &[u8], tag: &[u8], cpu: cpu::Features) -> Result<(), VerifyError> {
let computed = self.sign(data, cpu).map_err(VerifyError::DigestError)?;
let computed = self
.sign(data, cpu)
.map_err(VerifyError::InputTooLongError)?;
constant_time::verify_slices_are_equal(computed.as_ref(), tag)
.map_err(|_: error::Unspecified| VerifyError::Mismatch)
}
Expand Down Expand Up @@ -339,21 +343,40 @@ impl Context {
/// instead.
pub fn sign(self) -> Tag {
self.try_sign(cpu::features())
.map_err(error::Unspecified::from)
.map_err(error::erase::<InputTooLongError>)
.unwrap()
}

fn try_sign(self, cpu_features: cpu::Features) -> Result<Tag, digest::FinishError> {
fn try_sign(self, cpu_features: cpu::Features) -> Result<Tag, InputTooLongError> {
// Consequently, `num_pending` is valid.
debug_assert_eq!(self.inner.algorithm(), self.outer.algorithm);
debug_assert!(self.inner.algorithm().output_len() < self.outer.algorithm.block_len());

let inner = self.inner.try_finish(cpu_features)?;
let inner = inner.as_ref();
let num_pending = inner.len();
let buffer = &mut [0u8; digest::MAX_BLOCK_LEN];
const _BUFFER_IS_LARGE_ENOUGH_TO_HOLD_INNER: () =
assert!(digest::MAX_OUTPUT_LEN < digest::MAX_BLOCK_LEN);
buffer[..num_pending].copy_from_slice(inner);

self.outer
.try_finish(buffer, num_pending, cpu_features)
.map(Tag)
.map_err(|err| match err {
FinishError::InputTooLong(i) => {
// Unreachable, as we gave the inner context exactly the
// same input we gave the outer context, and
// `inner.try_finish` already succeeded. However, it is
// quite difficult to prove this, and we already return
// `InputTooLongError`, so just forward it along.
i
}
FinishError::PendingNotAPartialBlock(_) => {
// Follows from the assertions above.
unreachable!()
}
})
}
}

Expand All @@ -365,7 +388,7 @@ impl Context {
/// return value of `sign` to a tag. Use `verify` for verification instead.
pub fn sign(key: &Key, data: &[u8]) -> Tag {
key.sign(data, cpu::features())
.map_err(error::Unspecified::from)
.map_err(error::erase::<InputTooLongError>)
.unwrap()
}

Expand All @@ -382,8 +405,14 @@ pub fn verify(key: &Key, data: &[u8], tag: &[u8]) -> Result<(), error::Unspecifi
}

enum VerifyError {
// Theoretically somebody could have calculated a valid tag with a gigantic
// input that we do not support. If we were to support every theoretically
// valid input length, for *every* digest algorithm, then we could argue
// that hitting the input length limit implies a mismatch since nobody
// could have calculated such a tag with the given input.
#[allow(dead_code)]
DigestError(digest::FinishError),
InputTooLongError(InputTooLongError),

Mismatch,
}

Expand Down

0 comments on commit fa0c22b

Please sign in to comment.