Skip to content

Commit

Permalink
limb: Unmerge usize_from_u32 import from "alloc"-guarded use
Browse files Browse the repository at this point in the history
Since commit e88750b ("Define native word size as
`constant_time::Word`.") `usize_from_u32()` is unconditionally used
in and via global `const`s but was (likely accidentally by means
of `rust-analyzer`) merged into a `use` statement that's guarded by
`feature = "alloc"`.  When compiling this crate without that feature,
which happens with `default-features = false`, this fails to compile due
to missing the import.

Moving the import to the unguarded `use crate::` group solves this.
  • Loading branch information
MarijnS95 authored and briansmith committed Jan 13, 2025
1 parent 8d7a6f8 commit c5f5d1a
Show file tree
Hide file tree
Showing 5 changed files with 160 additions and 25 deletions.
5 changes: 4 additions & 1 deletion src/error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@
pub use self::{key_rejected::KeyRejected, unspecified::Unspecified};

pub(crate) use self::input_too_long::InputTooLongError;
pub(crate) use self::{
input_too_long::InputTooLongError, too_much_output_requested::TooMuchOutputRequestedError,
};

mod input_too_long;
mod into_unspecified;
mod key_rejected;
mod too_much_output_requested;
mod unspecified;

#[cold]
Expand Down
39 changes: 39 additions & 0 deletions src/error/too_much_output_requested.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright 2024 Brian Smith.
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
// copyright notice and this permission notice appear in all copies.
//
// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHORS DISCLAIM ALL WARRANTIES
// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY
// SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
// WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
// OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
// CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

pub struct TooMuchOutputRequestedError {
/// Note that this might not actually be the (exact) output length
/// requested, and its units might be lost. For example, it could be any of
/// the following:
///
/// * The length in bytes of the entire output.
/// * The length in bytes of some *part* of the output.
/// * A bit length.
/// * A length in terms of "blocks" or other grouping of output values.
/// * Some intermediate quantity that was used when checking the output
/// length.
/// * Some arbitrary value.
#[allow(dead_code)]
imprecise_output_length: usize,
}

impl TooMuchOutputRequestedError {
#[cold]
#[inline(never)]
pub(crate) fn new(imprecise_output_length: usize) -> Self {
Self {
imprecise_output_length,
}
}

Check warning on line 38 in src/error/too_much_output_requested.rs

View check run for this annotation

Codecov / codecov/patch

src/error/too_much_output_requested.rs#L34-L38

Added lines #L34 - L38 were not covered by tests
}
6 changes: 3 additions & 3 deletions src/hmac.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ impl Key {
.unwrap()
}

fn try_new(
pub(crate) fn try_new(
algorithm: Algorithm,
key_value: &[u8],
cpu_features: cpu::Features,
Expand Down Expand Up @@ -274,7 +274,7 @@ impl Key {
Algorithm(self.inner.algorithm)
}

fn sign(&self, data: &[u8], cpu: cpu::Features) -> Result<Tag, InputTooLongError> {
pub(crate) 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)
Expand Down Expand Up @@ -347,7 +347,7 @@ impl Context {
.unwrap()
}

fn try_sign(self, cpu_features: cpu::Features) -> Result<Tag, InputTooLongError> {
pub(crate) 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());
Expand Down
4 changes: 2 additions & 2 deletions src/limb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@
use crate::{
c, constant_time, error,
polyfill::{slice, ArrayFlatMap},
polyfill::{slice, usize_from_u32, ArrayFlatMap},
};

#[cfg(any(test, feature = "alloc"))]
use crate::{bits, polyfill::usize_from_u32};
use crate::bits;

#[cfg(feature = "alloc")]
use core::num::Wrapping;
Expand Down
131 changes: 112 additions & 19 deletions src/pbkdf2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,30 @@
//! assert!(db.verify_password("alice", "@74d7]404j|W}6u").is_ok());
//! }
use crate::{constant_time, digest, error, hmac};
use self::{derive_error::DeriveError, verify_error::VerifyError};
use crate::{
constant_time, cpu, digest,
error::{self, TooMuchOutputRequestedError},
hmac::{self, InputTooLongError},
polyfill::usize_from_u32,
};
use core::num::NonZeroU32;

/// A PBKDF2 algorithm.
#[derive(Clone, Copy, PartialEq, Eq)]
pub struct Algorithm(hmac::Algorithm);

impl Algorithm {
/// The maximum output length.
///
/// The value is `u32::MAX` times the output length of the underlying
/// HMAC/digest algorithm. (This is a fundamental limitation of the
/// PBKDF2 construction.)
fn max_output_len(&self) -> usize {
usize_from_u32(u32::MAX) * self.0.digest_algorithm().output_len()
}
}

/// PBKDF2 using HMAC-SHA1.
pub static PBKDF2_HMAC_SHA1: Algorithm = Algorithm(hmac::HMAC_SHA1_FOR_LEGACY_USE_ONLY);

Expand Down Expand Up @@ -150,15 +167,41 @@ pub static PBKDF2_HMAC_SHA512: Algorithm = Algorithm(hmac::HMAC_SHA512);
///
/// # Panics
///
/// `derive` panics if `out.len()` is larger than (2**32 - 1) * the digest
/// algorithm's output length, per the PBKDF2 specification.
/// Panics if `out.len() > u32::MAX * digest_alg.output_len()`, where
/// `digest_alg` is the underlying HMAC/digest algorithm.
///
/// Panics if `salt` is so astronomically gigantic that it isn't a valid input
/// to the underlying digest function.
///
/// Panics if `secret` is so astronomically gigantic that it isn't a valid
/// input to the underlying digest function.
pub fn derive(
algorithm: Algorithm,
iterations: NonZeroU32,
salt: &[u8],
secret: &[u8],
out: &mut [u8],
) {
let cpu = cpu::features();
try_derive(algorithm, iterations, salt, secret, out, cpu)
.map_err(error::erase::<DeriveError>)
.unwrap()
}

fn try_derive(
algorithm: Algorithm,
iterations: NonZeroU32,
salt: &[u8],
secret: &[u8],
out: &mut [u8],
cpu: cpu::Features,
) -> Result<(), DeriveError> {
if out.len() > algorithm.max_output_len() {
return Err(DeriveError::too_much_output_requested(
TooMuchOutputRequestedError::new(out.len()),
));

Check warning on line 202 in src/pbkdf2.rs

View check run for this annotation

Codecov / codecov/patch

src/pbkdf2.rs#L200-L202

Added lines #L200 - L202 were not covered by tests
}

let digest_alg = algorithm.0.digest_algorithm();
let output_len = digest_alg.output_len();

Expand All @@ -167,25 +210,40 @@ pub fn derive(
// hasn't been optimized to the same extent as fastpbkdf2. In particular,
// this implementation is probably doing a lot of unnecessary copying.

let secret = hmac::Key::new(algorithm.0, secret);
let secret =
hmac::Key::try_new(algorithm.0, secret, cpu).map_err(DeriveError::secret_too_long)?;

// Clear |out|.
out.fill(0);

let mut idx: u32 = 0;

for chunk in out.chunks_mut(output_len) {
idx = idx.checked_add(1).expect("derived key too long");
derive_block(&secret, iterations, salt, idx, chunk);
idx = idx.checked_add(1).ok_or_else(|| {
// Due to the `algorithm.max_output_len()` check above.
unreachable!()

Check warning on line 224 in src/pbkdf2.rs

View check run for this annotation

Codecov / codecov/patch

src/pbkdf2.rs#L223-L224

Added lines #L223 - L224 were not covered by tests
})?;
// If the salt is too long, then we'll detect this on the first
// iteration before we've written any output.
derive_block(&secret, iterations, salt, idx, chunk, cpu)
.map_err(DeriveError::salt_too_long)?;
}
Ok(())
}

fn derive_block(secret: &hmac::Key, iterations: NonZeroU32, salt: &[u8], idx: u32, out: &mut [u8]) {
fn derive_block(
secret: &hmac::Key,
iterations: NonZeroU32,
salt: &[u8],
idx: u32,
out: &mut [u8],
cpu: cpu::Features,
) -> Result<(), InputTooLongError> {
let mut ctx = hmac::Context::with_key(secret);
ctx.update(salt);
ctx.update(&u32::to_be_bytes(idx));

let mut u = ctx.sign();
let mut u = ctx.try_sign(cpu)?;

let mut remaining: u32 = iterations.into();
loop {
Expand All @@ -196,7 +254,28 @@ fn derive_block(secret: &hmac::Key, iterations: NonZeroU32, salt: &[u8], idx: u3
}
remaining -= 1;

u = hmac::sign(secret, u.as_ref());
// This will not fail, because the output of HMAC is never too long to
// be an input for the same algorithm, but we can't prove that with
// only locally-available information.
u = secret.sign(u.as_ref(), cpu)?
}
Ok(())
}

cold_exhaustive_error! {
enum derive_error::DeriveError {
secret_too_long => SecretTooLong(InputTooLongError),
salt_too_long => SaltTooLong(InputTooLongError),
too_much_output_requested => TooMuchOutputRequested(TooMuchOutputRequestedError),
}
}

cold_exhaustive_error! {
enum verify_error::VerifyError {
mismatch => Mismatch(()),
secret_too_long => SecretTooLong(InputTooLongError),
salt_too_long => SaltTooLong(InputTooLongError),
previously_derived_empty => PreviouslyDerivedEmpty(usize),
}
}

Expand All @@ -215,39 +294,53 @@ fn derive_block(secret: &hmac::Key, iterations: NonZeroU32, salt: &[u8], idx: u3
/// | `secret` | P (password)
/// | `previously_derived` | dk (derived key)
/// | `previously_derived.len()` | dkLen (derived key length)
///
/// # Panics
///
/// `verify` panics if `out.len()` is larger than (2**32 - 1) * the digest
/// algorithm's output length, per the PBKDF2 specification.
pub fn verify(
algorithm: Algorithm,
iterations: NonZeroU32,
salt: &[u8],
secret: &[u8],
previously_derived: &[u8],
) -> Result<(), error::Unspecified> {
let cpu = cpu::features();
try_verify(algorithm, iterations, salt, secret, previously_derived, cpu)
.map_err(error::erase::<VerifyError>)
}

fn try_verify(
algorithm: Algorithm,
iterations: NonZeroU32,
salt: &[u8],
secret: &[u8],
previously_derived: &[u8],
cpu: cpu::Features,
) -> Result<(), VerifyError> {
let digest_alg = algorithm.0.digest_algorithm();

if previously_derived.is_empty() {
return Err(error::Unspecified);
return Err(VerifyError::previously_derived_empty(0));
}

let mut derived_buf = [0u8; digest::MAX_OUTPUT_LEN];

let output_len = digest_alg.output_len();
let secret = hmac::Key::new(algorithm.0, secret);
let secret =
hmac::Key::try_new(algorithm.0, secret, cpu).map_err(VerifyError::secret_too_long)?;
let mut idx: u32 = 0;

let mut matches = 1;

for previously_derived_chunk in previously_derived.chunks(output_len) {
idx = idx.checked_add(1).expect("derived key too long");
idx = idx.checked_add(1).ok_or_else(|| {
// `previously_derived` is so gigantic that PBKDF2 couldn't
// have been used to compute it.
VerifyError::mismatch(())

Check warning on line 336 in src/pbkdf2.rs

View check run for this annotation

Codecov / codecov/patch

src/pbkdf2.rs#L334-L336

Added lines #L334 - L336 were not covered by tests
})?;

let derived_chunk = &mut derived_buf[..previously_derived_chunk.len()];
derived_chunk.fill(0);

derive_block(&secret, iterations, salt, idx, derived_chunk);
derive_block(&secret, iterations, salt, idx, derived_chunk, cpu)
.map_err(VerifyError::salt_too_long)?;

// XXX: This isn't fully constant-time-safe. TODO: Fix that.
#[allow(clippy::bool_to_int_with_if)]
Expand All @@ -264,7 +357,7 @@ pub fn verify(
}

if matches == 0 {
return Err(error::Unspecified);
return Err(VerifyError::mismatch(()));
}

Ok(())
Expand Down

0 comments on commit c5f5d1a

Please sign in to comment.