From f6a293b445c6667c0c884c66c9c53b102867646f Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Sun, 23 Jun 2024 12:42:51 -0700 Subject: [PATCH] aes-gcm: Provide more descriptive errors in internal API. --- src/aead/aes_gcm.rs | 94 ++++++++++++++++++++++++++++++++++--------- src/aead/algorithm.rs | 4 +- src/aead/gcm.rs | 34 +++++++++++----- 3 files changed, 101 insertions(+), 31 deletions(-) diff --git a/src/aead/aes_gcm.rs b/src/aead/aes_gcm.rs index ef7f0db2d8..41ca71d8b6 100644 --- a/src/aead/aes_gcm.rs +++ b/src/aead/aes_gcm.rs @@ -120,7 +120,7 @@ pub(super) fn seal( nonce: Nonce, aad: Aad<&[u8]>, in_out: &mut [u8], -) -> Result { +) -> Result { let (tag_iv, ctr) = Counter::one_two(nonce); #[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))] @@ -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 @@ -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!(), @@ -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(); @@ -247,8 +249,9 @@ fn seal_strided Result { - let mut auth = gcm::Context::new(gcm_key, aad, in_out.len())?; +) -> Result { + 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); @@ -256,7 +259,7 @@ fn seal_strided {} Err(_) => unreachable!(), @@ -273,11 +276,11 @@ fn seal_finish( remainder: &mut [u8], ctr: Counter, tag_iv: aes::Iv, -) -> Result { +) -> Result { 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); @@ -287,6 +290,27 @@ fn seal_finish( 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) + } + + #[cold] + #[inline(never)] + fn counter_overflow(counter_overflow_error: CounterOverflowError) -> Self { + Self::CounterOverflow(counter_overflow_error) + } +} + #[inline(never)] pub(super) fn open( Key(key): &Key, @@ -294,10 +318,10 @@ pub(super) fn open( aad: Aad<&[u8]>, in_out: &mut [u8], src: RangeFrom, -) -> Result { +) -> Result { // 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); @@ -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( @@ -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( @@ -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; @@ -455,11 +481,11 @@ fn open_strided, mut ctr: Counter, tag_iv: aes::Iv, -) -> Result { - let input = in_out.get(src.clone()).ok_or(error::Unspecified)?; +) -> Result { + 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; @@ -483,7 +509,7 @@ fn open_strided( src: RangeFrom, ctr: Counter, tag_iv: aes::Iv, -) -> Result { - let iv = ctr.try_into_iv().map_err(|_| error::Unspecified)?; +) -> Result { + 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); @@ -531,6 +557,34 @@ fn finish( 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) + } + + #[cold] + #[inline(never)] + fn counter_overflow(counter_overflow_error: CounterOverflowError) -> Self { + Self::CounterOverflow(counter_overflow_error) + } + + #[cold] + #[inline(never)] + fn invalid_src() -> Self { + Self::InvalidSrc + } +} + 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 diff --git a/src/aead/algorithm.rs b/src/aead/algorithm.rs index 1556cf5dde..a5facc0a08 100644 --- a/src/aead/algorithm.rs +++ b/src/aead/algorithm.rs @@ -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( @@ -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]. diff --git a/src/aead/gcm.rs b/src/aead/gcm.rs index 7fcfd88d86..35149b2f91 100644 --- a/src/aead/gcm.rs +++ b/src/aead/gcm.rs @@ -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; @@ -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 { + pub(crate) fn new(key: &'key K, aad: Aad<&[u8]>, in_out_len: usize) -> Result { if in_out_len > aes_gcm::MAX_IN_OUT_LEN { - return Err(error::Unspecified); + return Err(Error::in_out_too_long()); } - 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::MAX, so we don't need to do an @@ -139,6 +136,25 @@ impl Context<'_, K> { } } +pub(super) enum Error { + AadTooLong(()), + InOutTooLong(()), +} + +impl Error { + #[cold] + #[inline(never)] + fn aad_too_long() -> Self { + Self::AadTooLong(()) + } + + #[cold] + #[inline(never)] + fn in_out_too_long() -> Self { + Self::InOutTooLong(()) + } +} + pub(super) trait Gmult { fn gmult(&self, xi: &mut Xi); }