Skip to content

Commit

Permalink
aes-gcm: Provide more descriptive errors in internal API.
Browse files Browse the repository at this point in the history
  • Loading branch information
briansmith committed Jun 24, 2024
1 parent 5250acd commit 4a24c88
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 31 deletions.
94 changes: 74 additions & 20 deletions src/aead/aes_gcm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ pub(super) fn seal(
nonce: Nonce,
aad: Aad<&[u8]>,
in_out: &mut [u8],
) -> Result<Tag, error::Unspecified> {
) -> Result<Tag, SealError> {
let (tag_iv, ctr) = Counter::one_two(nonce);

#[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))]
Expand All @@ -130,7 +130,8 @@ pub(super) fn seal(
#[cfg(target_arch = "x86_64")]
DynKey::AesHwClMulAvxMovbe(Combo { aes_key, gcm_key }) => {
use crate::c;
let mut auth = gcm::Context::new(gcm_key, aad, in_out.len())?;
let mut auth =
gcm::Context::new(gcm_key, aad, in_out.len()).map_err(SealError::from_gcm_error)?;
let (htable, xi) = auth.inner();
prefixed_extern! {
// `HTable` and `Xi` should be 128-bit aligned. TODO: Can we shrink `HTable`? The
Expand Down Expand Up @@ -168,7 +169,7 @@ pub(super) fn seal(
if let Some(whole_len) = NonZeroUsize::new(whole.len()) {
let iv_block = ctr
.increment_by(whole_len)
.map_err(|_: CounterOverflowError| error::Unspecified)?;
.map_err(SealError::counter_overflow)?;
match aes_key.ctr32_encrypt_within(slice::flatten_mut(whole), 0.., iv_block) {
Ok(()) => {}
Result::<_, InOutLenInconsistentWithIvBlockLenError>::Err(_) => unreachable!(),
Expand All @@ -182,7 +183,8 @@ pub(super) fn seal(
DynKey::AesHwClMul(Combo { aes_key, gcm_key }) => {
use crate::bits::BitLength;

let mut auth = gcm::Context::new(gcm_key, aad, in_out.len())?;
let mut auth =
gcm::Context::new(gcm_key, aad, in_out.len()).map_err(SealError::from_gcm_error)?;

let (whole, remainder) = slice::as_chunks_mut(in_out);
let whole_block_bits = auth.in_out_whole_block_bits();
Expand Down Expand Up @@ -247,16 +249,17 @@ fn seal_strided<A: aes::EncryptBlock + aes::EncryptCtr32, G: gcm::UpdateBlocks +
in_out: &mut [u8],
mut ctr: Counter,
tag_iv: aes::Iv,
) -> Result<Tag, error::Unspecified> {
let mut auth = gcm::Context::new(gcm_key, aad, in_out.len())?;
) -> Result<Tag, SealError> {
let mut auth =
gcm::Context::new(gcm_key, aad, in_out.len()).map_err(SealError::from_gcm_error)?;

let (whole, remainder) = slice::as_chunks_mut(in_out);

for chunk in whole.chunks_mut(CHUNK_BLOCKS) {
let chunk_len = NonZeroUsize::new(chunk.len()).unwrap(); // Guaranteed by chunks_mut
let iv_block = ctr
.increment_by(chunk_len)
.map_err(|_: CounterOverflowError| error::Unspecified)?;
.map_err(SealError::counter_overflow)?;
match aes_key.ctr32_encrypt_within(slice::flatten_mut(chunk), 0.., iv_block) {
Ok(_) => {}
Err(_) => unreachable!(),

Check warning on line 265 in src/aead/aes_gcm.rs

View check run for this annotation

Codecov / codecov/patch

src/aead/aes_gcm.rs#L265

Added line #L265 was not covered by tests
Expand All @@ -273,11 +276,11 @@ fn seal_finish<A: aes::EncryptBlock, G: gcm::Gmult>(
remainder: &mut [u8],
ctr: Counter,
tag_iv: aes::Iv,
) -> Result<Tag, error::Unspecified> {
) -> Result<Tag, SealError> {
if !remainder.is_empty() {
let mut input = ZERO_BLOCK;
overwrite_at_start(&mut input, remainder);
let iv = ctr.try_into_iv().map_err(|_| error::Unspecified)?;
let iv = ctr.try_into_iv().map_err(SealError::counter_overflow)?;
let mut output = aes_key.encrypt_iv_xor_block(iv, input);
output[remainder.len()..].fill(0);
auth.update_block(output);
Expand All @@ -287,17 +290,38 @@ fn seal_finish<A: aes::EncryptBlock, G: gcm::Gmult>(
Ok(finish(aes_key, auth, tag_iv))
}

#[non_exhaustive]
pub(super) enum SealError {
#[allow(dead_code)]
InputTooLong(gcm::Error),
CounterOverflow(CounterOverflowError),
}

impl SealError {
#[cold]
#[inline(never)]
fn from_gcm_error(error: gcm::Error) -> Self {
Self::InputTooLong(error)
}

Check warning on line 305 in src/aead/aes_gcm.rs

View check run for this annotation

Codecov / codecov/patch

src/aead/aes_gcm.rs#L303-L305

Added lines #L303 - L305 were not covered by tests

#[cold]
#[inline(never)]
fn counter_overflow(counter_overflow_error: CounterOverflowError) -> Self {
Self::CounterOverflow(counter_overflow_error)
}

Check warning on line 311 in src/aead/aes_gcm.rs

View check run for this annotation

Codecov / codecov/patch

src/aead/aes_gcm.rs#L309-L311

Added lines #L309 - L311 were not covered by tests
}

#[inline(never)]
pub(super) fn open(
Key(key): &Key,
nonce: Nonce,
aad: Aad<&[u8]>,
in_out: &mut [u8],
src: RangeFrom<usize>,
) -> Result<Tag, error::Unspecified> {
) -> Result<Tag, OpenError> {
// Check that `src` is in bounds.
#[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))]
let input = in_out.get(src.clone()).ok_or(error::Unspecified)?;
let input = in_out.get(src.clone()).ok_or_else(OpenError::invalid_src)?;

let (tag_iv, ctr) = Counter::one_two(nonce);

Expand All @@ -322,7 +346,8 @@ pub(super) fn open(
Xi: &mut gcm::Xi) -> c::size_t;
}

let mut auth = gcm::Context::new(gcm_key, aad, input.len())?;
let mut auth =
gcm::Context::new(gcm_key, aad, input.len()).map_err(OpenError::from_gcm_error)?;
let (htable, xi) = auth.inner();
let processed = unsafe {
aesni_gcm_decrypt(
Expand Down Expand Up @@ -353,7 +378,7 @@ pub(super) fn open(
let whole_len = if let Some(whole_len) = NonZeroUsize::new(whole.len()) {
let iv_block = ctr
.increment_by(whole_len)
.map_err(|_: CounterOverflowError| error::Unspecified)?;
.map_err(OpenError::counter_overflow)?;
auth.update_blocks(whole);
let whole_len = slice::flatten(whole).len();
match aes_key.ctr32_encrypt_within(
Expand Down Expand Up @@ -381,7 +406,8 @@ pub(super) fn open(
use crate::bits::BitLength;

let input_len = input.len();
let mut auth = gcm::Context::new(gcm_key, aad, input_len)?;
let mut auth =
gcm::Context::new(gcm_key, aad, input_len).map_err(OpenError::from_gcm_error)?;

let remainder_len = input_len % BLOCK_LEN;
let whole_len = input_len - remainder_len;
Expand Down Expand Up @@ -455,11 +481,11 @@ fn open_strided<A: aes::EncryptBlock + aes::EncryptCtr32, G: gcm::UpdateBlocks +
src: RangeFrom<usize>,
mut ctr: Counter,
tag_iv: aes::Iv,
) -> Result<Tag, error::Unspecified> {
let input = in_out.get(src.clone()).ok_or(error::Unspecified)?;
) -> Result<Tag, OpenError> {
let input = in_out.get(src.clone()).ok_or_else(OpenError::invalid_src)?;
let input_len = input.len();

let mut auth = gcm::Context::new(gcm_key, aad, input_len)?;
let mut auth = gcm::Context::new(gcm_key, aad, input_len).map_err(OpenError::from_gcm_error)?;

let remainder_len = input_len % BLOCK_LEN;
let whole_len = input_len - remainder_len;
Expand All @@ -483,7 +509,7 @@ fn open_strided<A: aes::EncryptBlock + aes::EncryptCtr32, G: gcm::UpdateBlocks +
};
let iv_block = ctr
.increment_by(num_blocks)
.map_err(|_| error::Unspecified)?;
.map_err(OpenError::counter_overflow)?;

auth.update_blocks(ciphertext);

Expand All @@ -510,8 +536,8 @@ fn open_finish<A: aes::EncryptBlock, G: gcm::Gmult>(
src: RangeFrom<usize>,
ctr: Counter,
tag_iv: aes::Iv,
) -> Result<Tag, error::Unspecified> {
let iv = ctr.try_into_iv().map_err(|_| error::Unspecified)?;
) -> Result<Tag, OpenError> {
let iv = ctr.try_into_iv().map_err(OpenError::counter_overflow)?;
shift::shift_partial((src.start, remainder), |remainder| {
let mut input = ZERO_BLOCK;
overwrite_at_start(&mut input, remainder);
Expand All @@ -531,6 +557,34 @@ fn finish<A: aes::EncryptBlock, G: gcm::Gmult>(
gcm_ctx.pre_finish(|pre_tag| Tag(aes_key.encrypt_iv_xor_block(tag_iv, pre_tag)))
}

#[non_exhaustive]
pub(super) enum OpenError {
#[allow(dead_code)]
InputTooLong(gcm::Error),
InvalidSrc,
CounterOverflow(CounterOverflowError),
}

impl OpenError {
#[cold]
#[inline(never)]
fn from_gcm_error(error: gcm::Error) -> Self {
Self::InputTooLong(error)
}

Check warning on line 573 in src/aead/aes_gcm.rs

View check run for this annotation

Codecov / codecov/patch

src/aead/aes_gcm.rs#L571-L573

Added lines #L571 - L573 were not covered by tests

#[cold]
#[inline(never)]
fn counter_overflow(counter_overflow_error: CounterOverflowError) -> Self {
Self::CounterOverflow(counter_overflow_error)
}

Check warning on line 579 in src/aead/aes_gcm.rs

View check run for this annotation

Codecov / codecov/patch

src/aead/aes_gcm.rs#L577-L579

Added lines #L577 - L579 were not covered by tests

#[cold]
#[inline(never)]
fn invalid_src() -> Self {
Self::InvalidSrc
}

Check warning on line 585 in src/aead/aes_gcm.rs

View check run for this annotation

Codecov / codecov/patch

src/aead/aes_gcm.rs#L583-L585

Added lines #L583 - L585 were not covered by tests
}

pub(super) const MAX_IN_OUT_LEN: usize = super::max_input_len(BLOCK_LEN, 2);

// [NIST SP800-38D] Section 5.2.1.1. Note that [RFC 5116 Section 5.1] and
Expand Down
4 changes: 2 additions & 2 deletions src/aead/algorithm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ fn aes_gcm_seal(
KeyInner::AesGcm(key) => key,
_ => unreachable!(),
};
aes_gcm::seal(key, nonce, aad, in_out)
aes_gcm::seal(key, nonce, aad, in_out).map_err(|_: aes_gcm::SealError| error::Unspecified)
}

pub(super) fn aes_gcm_open(
Expand All @@ -208,7 +208,7 @@ pub(super) fn aes_gcm_open(
KeyInner::AesGcm(key) => key,
_ => unreachable!(),
};
aes_gcm::open(key, nonce, aad, in_out, src)
aes_gcm::open(key, nonce, aad, in_out, src).map_err(|_: aes_gcm::OpenError| error::Unspecified)
}

/// ChaCha20-Poly1305 as described in [RFC 8439].
Expand Down
34 changes: 25 additions & 9 deletions src/aead/gcm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use self::ffi::{Block, BLOCK_LEN, ZERO_BLOCK};
use super::{aes_gcm, Aad};
use crate::{
bits::{BitLength, FromByteLen as _},
error,
polyfill::{sliceutil::overwrite_at_start, NotSend},
};
use cfg_if::cfg_if;
Expand Down Expand Up @@ -49,16 +48,14 @@ pub(super) struct Context<'key, K> {

impl<'key, K: Gmult> Context<'key, K> {
#[inline(always)]
pub(crate) fn new(
key: &'key K,
aad: Aad<&[u8]>,
in_out_len: usize,
) -> Result<Self, error::Unspecified> {
pub(crate) fn new(key: &'key K, aad: Aad<&[u8]>, in_out_len: usize) -> Result<Self, Error> {
if in_out_len > aes_gcm::MAX_IN_OUT_LEN {
return Err(error::Unspecified);
return Err(Error::in_out_too_long());

Check warning on line 53 in src/aead/gcm.rs

View check run for this annotation

Codecov / codecov/patch

src/aead/gcm.rs#L53

Added line #L53 was not covered by tests
}
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::in_out_too_long())?;
let aad_len =
BitLength::from_byte_len(aad.as_ref().len()).map_err(|_| Error::aad_too_long())?;

// 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 Expand Up @@ -139,6 +136,25 @@ impl<K: Gmult> Context<'_, K> {
}
}

pub(super) enum Error {
AadTooLong(()),
InOutTooLong(()),
}

impl Error {
#[cold]
#[inline(never)]
fn aad_too_long() -> Self {
Self::AadTooLong(())
}

Check warning on line 149 in src/aead/gcm.rs

View check run for this annotation

Codecov / codecov/patch

src/aead/gcm.rs#L147-L149

Added lines #L147 - L149 were not covered by tests

#[cold]
#[inline(never)]
fn in_out_too_long() -> Self {
Self::InOutTooLong(())
}

Check warning on line 155 in src/aead/gcm.rs

View check run for this annotation

Codecov / codecov/patch

src/aead/gcm.rs#L153-L155

Added lines #L153 - L155 were not covered by tests
}

pub(super) trait Gmult {
fn gmult(&self, xi: &mut Xi);
}
Expand Down

0 comments on commit 4a24c88

Please sign in to comment.