Skip to content

Commit

Permalink
Use only non-panicking digest API internally.
Browse files Browse the repository at this point in the history
Expose `digest::try_finish` to the rest of the crate. Use it everywhere
digests are computed internally, avoiding `digest::finish` completely,
including transitive uses. This requires adding new non-panicking
variants of several APIs. For now, make those variants non-public,
until tests and documentation are updated.

This is a bit too extreme because we know some of these cases will
never fail as the input is known to be short. We will need to choose to
either just accept this, or introduce another new (internal?)
infallible API.
  • Loading branch information
briansmith committed Dec 18, 2024
1 parent 08832ab commit a2734b8
Show file tree
Hide file tree
Showing 12 changed files with 209 additions and 78 deletions.
25 changes: 18 additions & 7 deletions src/digest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,14 +264,14 @@ impl Context {
/// `finish` consumes the context so it cannot be (mis-)used after `finish`
/// has been called.
pub fn finish(self) -> Digest {
self.try_finish().unwrap()
self.try_finish(cpu::features())
.map_err(error::Unspecified::from)
.unwrap()
}

fn try_finish(mut self) -> Result<Digest, error::Unspecified> {
let cpu_features = cpu::features();
pub(crate) fn try_finish(mut self, cpu_features: cpu::Features) -> Result<Digest, FinishError> {
self.block
.try_finish(&mut self.pending, self.num_pending, cpu_features)
.map_err(error::Unspecified::from)
}

/// The algorithm that this context is using.
Expand All @@ -297,9 +297,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 @@ -312,6 +313,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
11 changes: 8 additions & 3 deletions src/ec/curve25519/ed25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,23 @@
//! EdDSA Signatures.
use super::ops::ELEM_LEN;
use crate::digest;
use crate::{cpu, digest};

pub mod signing;
pub mod verification;

/// The length of an Ed25519 public key.
pub const ED25519_PUBLIC_KEY_LEN: usize = ELEM_LEN;

pub fn eddsa_digest(signature_r: &[u8], public_key: &[u8], msg: &[u8]) -> digest::Digest {
fn eddsa_digest(
signature_r: &[u8],
public_key: &[u8],
msg: &[u8],
cpu: cpu::Features,
) -> Result<digest::Digest, digest::FinishError> {
let mut ctx = digest::Context::new(&digest::SHA512);
ctx.update(signature_r);
ctx.update(public_key);
ctx.update(msg);
ctx.finish()
ctx.try_finish(cpu)
}
30 changes: 21 additions & 9 deletions src/ec/curve25519/ed25519/signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl Ed25519KeyPair {
) -> Result<pkcs8::Document, error::Unspecified> {
let cpu_features = cpu::features();
let seed: [u8; SEED_LEN] = rand::generate(rng)?.expose();
let key_pair = Self::from_seed_(&seed, cpu_features);
let key_pair = Self::from_seed_(&seed, cpu_features)?;
Ok(pkcs8::wrap_key(
&PKCS8_TEMPLATE,
&seed[..],
Expand Down Expand Up @@ -167,28 +167,39 @@ impl Ed25519KeyPair {
let seed = seed
.try_into()
.map_err(|_| error::KeyRejected::invalid_encoding())?;
Ok(Self::from_seed_(seed, cpu::features()))
Self::from_seed_(seed, cpu::features())
}

fn from_seed_(seed: &Seed, cpu_features: cpu::Features) -> Self {
let h = digest::digest(&digest::SHA512, seed);
fn from_seed_(seed: &Seed, cpu_features: cpu::Features) -> Result<Self, error::KeyRejected> {
let h = digest::Digest::compute_from(&digest::SHA512, seed, cpu_features)
.map_err(|_: digest::FinishError| error::KeyRejected::unexpected_error())?;
let (private_scalar, private_prefix) = h.as_ref().split_at(SCALAR_LEN);

let private_scalar =
MaskedScalar::from_bytes_masked(private_scalar.try_into().unwrap()).into();

let a = ExtPoint::from_scalarmult_base_consttime(&private_scalar, cpu_features);

Self {
Ok(Self {
private_scalar,
private_prefix: private_prefix.try_into().unwrap(),
public_key: PublicKey(a.into_encoded_point(cpu_features)),
}
})
}

/// Returns the signature of the message `msg`.
pub fn sign(&self, msg: &[u8]) -> signature::Signature {
let cpu_features = cpu::features();
self.try_sign(msg, cpu_features)
.map_err(error::Unspecified::from)
.unwrap()
}

fn try_sign(
&self,
msg: &[u8],
cpu_features: cpu::Features,
) -> Result<signature::Signature, digest::FinishError> {
signature::Signature::new(|signature_bytes| {
prefixed_extern! {
fn x25519_sc_muladd(
Expand All @@ -205,13 +216,14 @@ impl Ed25519KeyPair {
let mut ctx = digest::Context::new(&digest::SHA512);
ctx.update(&self.private_prefix);
ctx.update(msg);
ctx.finish()
ctx.try_finish(cpu_features)?
};
let nonce = Scalar::from_sha512_digest_reduced(nonce);

let r = ExtPoint::from_scalarmult_base_consttime(&nonce, cpu_features);
signature_r.copy_from_slice(&r.into_encoded_point(cpu_features));
let hram_digest = eddsa_digest(signature_r, self.public_key.as_ref(), msg);
let hram_digest =
eddsa_digest(signature_r, self.public_key.as_ref(), msg, cpu_features)?;
let hram = Scalar::from_sha512_digest_reduced(hram_digest);
unsafe {
x25519_sc_muladd(
Expand All @@ -222,7 +234,7 @@ impl Ed25519KeyPair {
);
}

SIGNATURE_LEN
Ok(SIGNATURE_LEN)
})
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/ec/curve25519/ed25519/verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,12 @@ impl signature::VerificationAlgorithm for EdDSAParameters {
let mut a = ExtPoint::from_encoded_point_vartime(public_key)?;
a.invert_vartime();

let h_digest = eddsa_digest(signature_r, public_key, msg.as_slice_less_safe());
let h_digest = eddsa_digest(
signature_r,
public_key,
msg.as_slice_less_safe(),
cpu_features,
)?;
let h = Scalar::from_sha512_digest_reduced(h_digest);

let mut r = Point::new_at_infinity();
Expand Down
25 changes: 16 additions & 9 deletions src/ec/suite_b/ecdsa/signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ impl EcdsaKeyPair {
let d = private_key::private_key_as_scalar(n, &seed);
let d = alg.private_scalar_ops.to_mont(&d, cpu);

let nonce_key = NonceRandomKey::new(alg, &seed, rng)?;
let nonce_key = NonceRandomKey::new(alg, &seed, rng, cpu)?;
Ok(Self {
d,
nonce_key,
Expand All @@ -178,7 +178,7 @@ impl EcdsaKeyPair {
let cpu = cpu::features();

// Step 4 (out of order).
let h = digest::digest(self.alg.digest_alg, message);
let h = digest::Digest::compute_from(self.alg.digest_alg, message, cpu)?;

// Incorporate `h` into the nonce to hedge against faulty RNGs. (This
// is not an approved random number generator that is mandated in
Expand All @@ -198,10 +198,12 @@ impl EcdsaKeyPair {
rng: &dyn rand::SecureRandom,
message: &[u8],
) -> Result<signature::Signature, error::Unspecified> {
let cpu = cpu::features();

// Step 4 (out of order).
let h = digest::digest(self.alg.digest_alg, message);
let h = digest::Digest::compute_from(self.alg.digest_alg, message, cpu)?;

self.sign_digest(h, rng, cpu::features())
self.sign_digest(h, rng, cpu)
}

/// Returns the signature of message digest `h` using a "random" nonce
Expand Down Expand Up @@ -278,9 +280,9 @@ impl EcdsaKeyPair {
}

// Step 7 with encoding.
return Ok(signature::Signature::new(|sig_bytes| {
(self.alg.format_rs)(scalar_ops, &r, &s, sig_bytes)
}));
return signature::Signature::new(|sig_bytes| {
Ok((self.alg.format_rs)(scalar_ops, &r, &s, sig_bytes))
});
}

Err(error::Unspecified)
Expand All @@ -303,6 +305,8 @@ impl core::fmt::Debug for NonceRandom<'_> {

impl rand::sealed::SecureRandom for NonceRandom<'_> {
fn fill_impl(&self, dest: &mut [u8]) -> Result<(), error::Unspecified> {
let cpu = cpu::features();

// Use the same digest algorithm that will be used to digest the
// message. The digest algorithm's output is exactly the right size;
// this is checked below.
Expand Down Expand Up @@ -331,7 +335,7 @@ impl rand::sealed::SecureRandom for NonceRandom<'_> {

ctx.update(self.message_digest.as_ref());

let nonce = ctx.finish();
let nonce = ctx.try_finish(cpu)?;

// `copy_from_slice()` panics if the lengths differ, so we don't have
// to separately assert that the lengths are the same.
Expand All @@ -350,6 +354,7 @@ impl NonceRandomKey {
alg: &EcdsaSigningAlgorithm,
seed: &ec::Seed,
rng: &dyn rand::SecureRandom,
cpu: cpu::Features,
) -> Result<Self, error::KeyRejected> {
let mut rand = [0; digest::MAX_OUTPUT_LEN];
let rand = &mut rand[0..alg.curve.elem_scalar_seed_len];
Expand All @@ -363,7 +368,9 @@ impl NonceRandomKey {
let mut ctx = digest::Context::new(alg.digest_alg);
ctx.update(rand);
ctx.update(seed.bytes_less_safe());
Ok(Self(ctx.finish()))
ctx.try_finish(cpu)
.map(Self)
.map_err(|_: digest::FinishError| error::KeyRejected::unexpected_error())
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/ec/suite_b/ecdsa/verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl signature::VerificationAlgorithm for EcdsaVerificationAlgorithm {
let e = {
// NSA Guide Step 2: "Use the selected hash function to compute H =
// Hash(M)."
let h = digest::digest(self.digest_alg, msg.as_slice_less_safe());
let h = digest::Digest::compute_from(self.digest_alg, msg.as_slice_less_safe(), cpu)?;

// NSA Guide Step 3: "Convert the bit string H to an integer e as
// described in Appendix B.2."
Expand Down
47 changes: 40 additions & 7 deletions src/hkdf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
//!
//! [RFC 5869]: https://tools.ietf.org/html/rfc5869
use crate::{error, hmac};
use crate::{cpu, digest, error, hmac};

/// An HKDF algorithm.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -62,22 +62,42 @@ impl Salt {
/// Constructing a `Salt` is relatively expensive so it is good to reuse a
/// `Salt` object instead of re-constructing `Salt`s with the same value.
pub fn new(algorithm: Algorithm, value: &[u8]) -> Self {
Self(hmac::Key::new(algorithm.0, value))
Self::try_new(algorithm, value, cpu::features())
.map_err(error::Unspecified::from)
.unwrap()
}

pub(crate) fn try_new(
algorithm: Algorithm,
value: &[u8],
cpu: cpu::Features,
) -> Result<Self, digest::FinishError> {
hmac::Key::try_new(algorithm.0, value, cpu).map(Self)
}

/// The [HKDF-Extract] operation.
///
/// [HKDF-Extract]: https://tools.ietf.org/html/rfc5869#section-2.2
pub fn extract(&self, secret: &[u8]) -> Prk {
self.try_extract(secret, cpu::features())
.map_err(error::Unspecified::from)
.unwrap()
}

pub(crate) fn try_extract(
&self,
secret: &[u8],
cpu: cpu::Features,
) -> Result<Prk, digest::FinishError> {
// The spec says that if no salt is provided then a key of
// `digest_alg.output_len` bytes of zeros is used. But, HMAC keys are
// already zero-padded to the block length, which is larger than the output
// length of the extract step (the length of the digest). Consequently the
// `Key` constructor will automatically do the right thing for a
// zero-length string.
let salt = &self.0;
let prk = hmac::sign(salt, secret);
Prk(hmac::Key::new(salt.algorithm(), prk.as_ref()))
let prk = hmac::try_sign(salt, secret, cpu)?;
hmac::Key::try_new(salt.algorithm(), prk.as_ref(), cpu).map(Prk)
}

/// The algorithm used to derive this salt.
Expand Down Expand Up @@ -115,7 +135,18 @@ impl Prk {
/// intentionally wants to leak the PRK secret, e.g. to implement
/// `SSLKEYLOGFILE` functionality.
pub fn new_less_safe(algorithm: Algorithm, value: &[u8]) -> Self {
Self(hmac::Key::new(algorithm.hmac_algorithm(), value))
let cpu = cpu::features();
Self::try_new_less_safe(algorithm, value, cpu)
.map_err(error::Unspecified::from)
.unwrap()
}

Check warning on line 142 in src/hkdf.rs

View check run for this annotation

Codecov / codecov/patch

src/hkdf.rs#L138-L142

Added lines #L138 - L142 were not covered by tests

pub(crate) fn try_new_less_safe(
algorithm: Algorithm,
value: &[u8],
cpu: cpu::Features,
) -> Result<Self, digest::FinishError> {
hmac::Key::try_new(algorithm.hmac_algorithm(), value, cpu).map(Self)

Check warning on line 149 in src/hkdf.rs

View check run for this annotation

Codecov / codecov/patch

src/hkdf.rs#L144-L149

Added lines #L144 - L149 were not covered by tests
}

/// The [HKDF-Expand] operation.
Expand Down Expand Up @@ -181,7 +212,8 @@ impl<L: KeyType> Okm<'_, L> {
/// constructed.)
#[inline]
pub fn fill(self, out: &mut [u8]) -> Result<(), error::Unspecified> {
fill_okm(self.prk, self.info, out, self.len_cached)
let cpu = cpu::features();
fill_okm(self.prk, self.info, out, self.len_cached, cpu)
}
}

Expand All @@ -190,6 +222,7 @@ fn fill_okm(
info: &[&[u8]],
out: &mut [u8],
len: usize,
cpu: cpu::Features,
) -> Result<(), error::Unspecified> {
if out.len() != len {
return Err(error::Unspecified);
Expand All @@ -208,7 +241,7 @@ fn fill_okm(
}
ctx.update(&[n]);

let t = ctx.sign();
let t = ctx.try_sign(cpu)?;
let t = t.as_ref();

// Append `t` to the output.
Expand Down
Loading

0 comments on commit a2734b8

Please sign in to comment.