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

error handling: Refactor how "Input too long" errors are handled. #2202

Merged
merged 1 commit into from
Dec 31, 2024
Merged
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
8 changes: 5 additions & 3 deletions src/aead/gcm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use self::ffi::{Block, BLOCK_LEN, ZERO_BLOCK};
use super::{aes_gcm, Aad};
use crate::{
bits::{BitLength, FromByteLen as _},
error,
error::{self, InputTooLongError},
polyfill::{sliceutil::overwrite_at_start, NotSend},
};
use cfg_if::cfg_if;
Expand Down Expand Up @@ -57,8 +57,10 @@ impl<'key, K: Gmult> Context<'key, K> {
if in_out_len > aes_gcm::MAX_IN_OUT_LEN {
return Err(error::Unspecified);
}
let in_out_len = BitLength::from_byte_len(in_out_len)?;
let aad_len = BitLength::from_byte_len(aad.as_ref().len())?;
let in_out_len =
BitLength::from_byte_len(in_out_len).map_err(error::erase::<InputTooLongError>)?;
let aad_len = BitLength::from_byte_len(aad.as_ref().len())
.map_err(error::erase::<InputTooLongError>)?;

// NIST SP800-38D Section 5.2.1.1 says that the maximum AAD length is
// 2**64 - 1 bits, i.e. BitLength<u64>::MAX, so we don't need to do an
Expand Down
23 changes: 11 additions & 12 deletions src/bits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

//! Bit lengths.

use crate::{error, polyfill};
use crate::{error::InputTooLongError, polyfill};

/// The length of something, in bits.
///
Expand All @@ -27,36 +27,35 @@ pub(crate) trait FromByteLen<T>: Sized {
/// Constructs a `BitLength` from the given length in bytes.
///
/// Fails if `bytes * 8` is too large for a `T`.
fn from_byte_len(bytes: T) -> Result<Self, error::Unspecified>;
fn from_byte_len(bytes: T) -> Result<Self, InputTooLongError<T>>;
}

impl FromByteLen<usize> for BitLength<usize> {
#[inline]
fn from_byte_len(bytes: usize) -> Result<Self, error::Unspecified> {
fn from_byte_len(bytes: usize) -> Result<Self, InputTooLongError> {
match bytes.checked_mul(8) {
Some(bits) => Ok(Self(bits)),
None => Err(error::Unspecified),
None => Err(InputTooLongError::new(bytes)),
}
}
}

impl FromByteLen<u64> for BitLength<u64> {
#[inline]
fn from_byte_len(bytes: u64) -> Result<Self, error::Unspecified> {
fn from_byte_len(bytes: u64) -> Result<Self, InputTooLongError<u64>> {
match bytes.checked_mul(8) {
Some(bits) => Ok(Self(bits)),
None => Err(error::Unspecified),
None => Err(InputTooLongError::new(bytes)),
}
}
}

impl FromByteLen<usize> for BitLength<u64> {
#[inline]
fn from_byte_len(bytes: usize) -> Result<Self, error::Unspecified> {
let bytes = polyfill::u64_from_usize(bytes);
match bytes.checked_mul(8) {
fn from_byte_len(bytes: usize) -> Result<Self, InputTooLongError<usize>> {
match polyfill::u64_from_usize(bytes).checked_mul(8) {
Some(bits) => Ok(Self(bits)),
None => Err(error::Unspecified),
None => Err(InputTooLongError::new(bytes)),
}
}
}
Expand Down Expand Up @@ -102,8 +101,8 @@ impl BitLength<usize> {

#[cfg(feature = "alloc")]
#[inline]
pub(crate) fn try_sub_1(self) -> Result<Self, error::Unspecified> {
let sum = self.0.checked_sub(1).ok_or(error::Unspecified)?;
pub(crate) fn try_sub_1(self) -> Result<Self, crate::error::Unspecified> {
let sum = self.0.checked_sub(1).ok_or(crate::error::Unspecified)?;
Ok(Self(sum))
}
}
Expand Down
20 changes: 12 additions & 8 deletions src/digest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
};
use crate::{
bits::{BitLength, FromByteLen as _},
cpu, debug, error,
cpu, debug,
error::{self, InputTooLongError},
polyfill::{self, slice, sliceutil},
};
use core::num::Wrapping;
Expand Down Expand Up @@ -80,12 +81,15 @@
num_pending: usize,
cpu_features: cpu::Features,
) -> Result<Digest, FinishError> {
let completed_bytes = self
let completed_bits = self
.completed_bytes
.checked_add(polyfill::u64_from_usize(num_pending))
.ok_or_else(|| FinishError::too_much_input(self.completed_bytes))?;
let completed_bits = BitLength::from_byte_len(completed_bytes)
.map_err(|_: error::Unspecified| FinishError::too_much_input(self.completed_bytes))?;
.ok_or_else(|| {
// Choosing self.completed_bytes here is lossy & somewhat arbitrary.
InputTooLongError::new(self.completed_bytes)

Check warning on line 89 in src/digest.rs

View check run for this annotation

Codecov / codecov/patch

src/digest.rs#L88-L89

Added lines #L88 - L89 were not covered by tests
})
.and_then(BitLength::from_byte_len)
.map_err(FinishError::input_too_long)?;

let block_len = self.algorithm.block_len();
let block = &mut block[..block_len];
Expand Down Expand Up @@ -143,16 +147,16 @@

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

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

// unreachable
Expand Down
39 changes: 39 additions & 0 deletions src/error/input_too_long.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 InputTooLongError<T = usize> {
/// Note that this might not actually be the (exact) length of the input,
/// and its units might be lost. For example, it could be any of the
/// following:
///
/// * The length in bytes of the entire input.
/// * The length in bytes of some *part* of the input.
/// * A bit length.
/// * A length in terms of "blocks" or other grouping of input values.
/// * Some intermediate quantity that was used when checking the input
/// length.
/// * Some arbitrary value.
#[allow(dead_code)]
imprecise_input_length: T,
}

impl<T> InputTooLongError<T> {
#[cold]
#[inline(never)]
pub(crate) fn new(imprecise_input_length: T) -> Self {
Self {
imprecise_input_length,
}
}
}
12 changes: 6 additions & 6 deletions src/error/into_unspecified.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,19 @@
use crate::error::{KeyRejected, Unspecified};

impl From<untrusted::EndOfInput> for Unspecified {
fn from(_: untrusted::EndOfInput) -> Self {
Self
fn from(source: untrusted::EndOfInput) -> Self {
super::erase(source)
}
}

impl From<core::array::TryFromSliceError> for Unspecified {
fn from(_: core::array::TryFromSliceError) -> Self {
Self
fn from(source: core::array::TryFromSliceError) -> Self {
super::erase(source)
}
}

impl From<KeyRejected> for Unspecified {
fn from(_: KeyRejected) -> Self {
Self
fn from(source: KeyRejected) -> Self {
super::erase(source)
}
}
9 changes: 9 additions & 0 deletions src/error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@

pub use self::{key_rejected::KeyRejected, unspecified::Unspecified};

pub(crate) use self::input_too_long::InputTooLongError;

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

#[cold]
#[inline(never)]
pub(crate) fn erase<T>(_: T) -> Unspecified {
Unspecified
}
8 changes: 5 additions & 3 deletions src/rsa/public_modulus.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::{
arithmetic::{bigint, montgomery::RR},
bits::{self, FromByteLen as _},
cpu, error,
cpu,
error::{self, InputTooLongError},
rsa::N,
};
use core::ops::RangeInclusive;
Expand Down Expand Up @@ -45,8 +46,9 @@ impl PublicModulus {
// the public modulus to be exactly 2048 or 3072 bits, but we are more
// flexible to be compatible with other commonly-used crypto libraries.
assert!(min_bits >= MIN_BITS);
let bits_rounded_up =
bits::BitLength::from_byte_len(bits.as_usize_bytes_rounded_up()).unwrap(); // TODO: safe?
let bits_rounded_up = bits::BitLength::from_byte_len(bits.as_usize_bytes_rounded_up())
.map_err(error::erase::<InputTooLongError>)
.unwrap(); // TODO: safe?
if bits_rounded_up < min_bits {
return Err(error::KeyRejected::too_small());
}
Expand Down
7 changes: 5 additions & 2 deletions src/rsa/verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ use super::{
};
use crate::{
bits::{self, FromByteLen as _},
cpu, digest, error, sealed, signature,
cpu, digest,
error::{self, InputTooLongError},
sealed, signature,
};

impl signature::VerificationAlgorithm for RsaParameters {
Expand Down Expand Up @@ -201,7 +203,8 @@ pub(crate) fn verify_rsa_(
cpu_features: cpu::Features,
) -> Result<(), error::Unspecified> {
let max_bits: bits::BitLength =
bits::BitLength::from_byte_len(PUBLIC_KEY_PUBLIC_MODULUS_MAX_LEN)?;
bits::BitLength::from_byte_len(PUBLIC_KEY_PUBLIC_MODULUS_MAX_LEN)
.map_err(error::erase::<InputTooLongError>)?;

// XXX: FIPS 186-4 seems to indicate that the minimum
// exponent value is 2**16 + 1, but it isn't clear if this is just for
Expand Down
51 changes: 24 additions & 27 deletions src/tests/bits_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,55 +13,52 @@
// CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

use crate::{
bits::{BitLength, FromByteLen as _},
polyfill::u64_from_usize,
{
bits::{BitLength, FromByteLen as _},
error,
},
};

#[test]
fn test_from_byte_len_overflow() {
const USIZE_MAX_VALID_BYTES: usize = usize::MAX / 8;

// Maximum valid input for BitLength<usize>.
{
let bits = BitLength::<usize>::from_byte_len(USIZE_MAX_VALID_BYTES).unwrap();
assert_eq!(bits.as_usize_bytes_rounded_up(), USIZE_MAX_VALID_BYTES);
assert_eq!(bits.as_bits(), usize::MAX & !0b111);
match BitLength::<usize>::from_byte_len(USIZE_MAX_VALID_BYTES) {
Ok(bits) => {
assert_eq!(bits.as_usize_bytes_rounded_up(), USIZE_MAX_VALID_BYTES);
assert_eq!(bits.as_bits(), usize::MAX & !0b111);
}
Err(_) => unreachable!(),

Check warning on line 30 in src/tests/bits_tests.rs

View check run for this annotation

Codecov / codecov/patch

src/tests/bits_tests.rs#L30

Added line #L30 was not covered by tests
}

// Minimum invalid usize input for BitLength<usize>.
assert_eq!(
BitLength::<usize>::from_byte_len(USIZE_MAX_VALID_BYTES + 1),
Err(error::Unspecified)
);
assert!(BitLength::<usize>::from_byte_len(USIZE_MAX_VALID_BYTES + 1).is_err());

// Minimum invalid usize input for BitLength<u64> on 64-bit targets.
{
let bits = BitLength::<u64>::from_byte_len(USIZE_MAX_VALID_BYTES + 1);
let r = BitLength::<u64>::from_byte_len(USIZE_MAX_VALID_BYTES + 1);
if cfg!(target_pointer_width = "64") {
assert_eq!(bits, Err(error::Unspecified));
assert!(r.is_err());
} else {
let bits = bits.unwrap();
assert_eq!(
bits.as_bits(),
(u64_from_usize(USIZE_MAX_VALID_BYTES) + 1) * 8
);
match r {
Ok(bits) => {
assert_eq!(
bits.as_bits(),
(u64_from_usize(USIZE_MAX_VALID_BYTES) + 1) * 8
);
}
Err(_) => unreachable!(),

Check warning on line 49 in src/tests/bits_tests.rs

View check run for this annotation

Codecov / codecov/patch

src/tests/bits_tests.rs#L49

Added line #L49 was not covered by tests
}
}
}

const U64_MAX_VALID_BYTES: u64 = u64::MAX / 8;

// Maximum valid u64 input for BitLength<u64>.
{
let bits = BitLength::<u64>::from_byte_len(U64_MAX_VALID_BYTES).unwrap();
assert_eq!(bits.as_bits(), u64::MAX & !0b111);
}
match BitLength::<u64>::from_byte_len(U64_MAX_VALID_BYTES) {
Ok(bits) => assert_eq!(bits.as_bits(), u64::MAX & !0b111),
Err(_) => unreachable!(),

Check warning on line 59 in src/tests/bits_tests.rs

View check run for this annotation

Codecov / codecov/patch

src/tests/bits_tests.rs#L59

Added line #L59 was not covered by tests
};

// Minimum invalid usize input for BitLength<u64> on 64-bit targets.
{
let bits = BitLength::<u64>::from_byte_len(U64_MAX_VALID_BYTES + 1);
assert_eq!(bits, Err(error::Unspecified));
}
assert!(BitLength::<u64>::from_byte_len(U64_MAX_VALID_BYTES + 1).is_err());
}