From a7ccc1c6adbd1a430fddc0cafcc9ff1d67547ec3 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Sun, 29 Dec 2024 13:39:00 -0800 Subject: [PATCH] aes_gcm: Refactor internal error reporting. Make it clearer when/why these operations fail. Help the optimizer optimize for the non-error cases with `#[cold]` annotations. --- src/aead.rs | 2 ++ src/aead/aes_gcm.rs | 66 +++++++++++++++++++++++++----------------- src/aead/algorithm.rs | 9 +++--- src/aead/gcm.rs | 12 ++++---- src/aead/inout.rs | 8 +---- src/aead/open_error.rs | 37 +++++++++++++++++++++++ 6 files changed, 89 insertions(+), 45 deletions(-) create mode 100644 src/aead/open_error.rs diff --git a/src/aead.rs b/src/aead.rs index 2760c76f23..b5915df374 100644 --- a/src/aead.rs +++ b/src/aead.rs @@ -26,6 +26,7 @@ use crate::{ polyfill::{u64_from_usize, usize_from_u64_saturated}, }; +use self::open_error::OpenError; pub use self::{ algorithm::{Algorithm, AES_128_GCM, AES_256_GCM, CHACHA20_POLY1305}, less_safe_key::LessSafeKey, @@ -179,6 +180,7 @@ mod gcm; mod inout; mod less_safe_key; mod nonce; +mod open_error; mod opening_key; mod poly1305; pub mod quic; diff --git a/src/aead/aes_gcm.rs b/src/aead/aes_gcm.rs index 2ba1526e9b..e482b57283 100644 --- a/src/aead/aes_gcm.rs +++ b/src/aead/aes_gcm.rs @@ -14,10 +14,11 @@ use super::{ aes::{self, Counter, BLOCK_LEN, ZERO_BLOCK}, - gcm, shift, Aad, InOut, Nonce, Tag, + gcm, shift, Aad, InOut, Nonce, OpenError, Tag, }; use crate::{ - cpu, error, + cpu, + error::{self, InputTooLongError}, polyfill::{slice, sliceutil::overwrite_at_start, usize_from_u64_saturated}, }; use core::ops::RangeFrom; @@ -117,7 +118,7 @@ pub(super) fn seal( nonce: Nonce, aad: Aad<&[u8]>, in_out: &mut [u8], -) -> Result { +) -> Result { let mut ctr = Counter::one(nonce); let tag_iv = ctr.increment(); @@ -162,7 +163,7 @@ pub(super) fn seal( let (whole, remainder) = slice::as_chunks_mut(ramaining); aes_key.ctr32_encrypt_within(InOut::in_place(slice::flatten_mut(whole)), &mut ctr); auth.update_blocks(whole); - seal_finish(aes_key, auth, remainder, ctr, tag_iv) + Ok(seal_finish(aes_key, auth, remainder, ctr, tag_iv)) } #[cfg(target_arch = "aarch64")] @@ -200,7 +201,7 @@ pub(super) fn seal( ) } } - seal_finish(aes_key, auth, remainder, ctr, tag_iv) + Ok(seal_finish(aes_key, auth, remainder, ctr, tag_iv)) } #[cfg(any(target_arch = "x86_64", target_arch = "x86"))] @@ -234,7 +235,7 @@ fn seal_strided Result { +) -> Result { let mut auth = gcm::Context::new(gcm_key, aad, in_out.len())?; let (whole, remainder) = slice::as_chunks_mut(in_out); @@ -244,7 +245,7 @@ fn seal_strided( @@ -253,7 +254,7 @@ fn seal_finish( remainder: &mut [u8], ctr: Counter, tag_iv: aes::Iv, -) -> Result { +) -> Tag { if !remainder.is_empty() { let mut input = ZERO_BLOCK; overwrite_at_start(&mut input, remainder); @@ -263,7 +264,7 @@ fn seal_finish( overwrite_at_start(remainder, &output); } - Ok(finish(aes_key, auth, tag_iv)) + finish(aes_key, auth, tag_iv) } #[inline(never)] @@ -273,9 +274,10 @@ pub(super) fn open( aad: Aad<&[u8]>, in_out_slice: &mut [u8], src: RangeFrom, -) -> Result { +) -> Result { #[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))] - let in_out = InOut::overlapping(in_out_slice, src.clone())?; + let in_out = + InOut::overlapping(in_out_slice, src.clone()).map_err(OpenError::src_index_error)?; let mut ctr = Counter::one(nonce); let tag_iv = ctr.increment(); @@ -299,7 +301,8 @@ pub(super) fn open( } let (input, output, len) = in_out.into_input_output_len(); - let mut auth = gcm::Context::new(gcm_key, aad, len)?; + let mut auth = + gcm::Context::new(gcm_key, aad, len).map_err(OpenError::input_too_long)?; let (htable, xi) = auth.inner(); let processed = unsafe { aesni_gcm_decrypt( @@ -331,14 +334,15 @@ pub(super) fn open( let whole_len = slice::flatten(whole).len(); // Decrypt any remaining whole blocks. - let whole = InOut::overlapping(&mut in_out[..(src.start + whole_len)], src.clone())?; + let whole = InOut::overlapping(&mut in_out[..(src.start + whole_len)], src.clone()) + .map_err(OpenError::src_index_error)?; aes_key.ctr32_encrypt_within(whole, &mut ctr); let in_out = match in_out.get_mut(whole_len..) { Some(partial) => partial, None => unreachable!(), }; - open_finish(aes_key, auth, in_out, src, ctr, tag_iv) + Ok(open_finish(aes_key, auth, in_out, src, ctr, tag_iv)) } #[cfg(target_arch = "aarch64")] @@ -347,7 +351,8 @@ pub(super) fn open( let (input, output, input_len) = in_out.into_input_output_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::input_too_long)?; let remainder_len = input_len % BLOCK_LEN; let whole_len = input_len - remainder_len; @@ -417,15 +422,16 @@ pub(super) fn open( fn open_strided( Combo { aes_key, gcm_key }: &Combo, aad: Aad<&[u8]>, - in_out: &mut [u8], + in_out_slice: &mut [u8], src: RangeFrom, mut ctr: Counter, tag_iv: aes::Iv, -) -> Result { - let input = in_out.get(src.clone()).ok_or(error::Unspecified)?; - let input_len = input.len(); +) -> Result { + let in_out = + InOut::overlapping(in_out_slice, src.clone()).map_err(OpenError::src_index_error)?; + let input_len = in_out.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::input_too_long)?; let remainder_len = input_len % BLOCK_LEN; let whole_len = input_len - remainder_len; @@ -440,7 +446,7 @@ fn open_strided( @@ -468,7 +482,7 @@ fn open_finish( src: RangeFrom, ctr: Counter, tag_iv: aes::Iv, -) -> Result { +) -> Tag { shift::shift_partial((src.start, remainder), |remainder| { let mut input = ZERO_BLOCK; overwrite_at_start(&mut input, remainder); @@ -476,7 +490,7 @@ fn open_finish( aes_key.encrypt_iv_xor_block(ctr.into(), input) }); - Ok(finish(aes_key, auth, tag_iv)) + finish(aes_key, auth, tag_iv) } fn finish( diff --git a/src/aead/algorithm.rs b/src/aead/algorithm.rs index 1556cf5dde..04b8323779 100644 --- a/src/aead/algorithm.rs +++ b/src/aead/algorithm.rs @@ -12,14 +12,13 @@ // OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN // CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -use crate::{constant_time, cpu, error, hkdf}; -use core::ops::RangeFrom; - use super::{ aes, aes_gcm, chacha20_poly1305, nonce::{Nonce, NONCE_LEN}, Aad, KeyInner, Tag, TAG_LEN, }; +use crate::{constant_time, cpu, error, hkdf}; +use core::ops::RangeFrom; impl hkdf::KeyType for &'static Algorithm { #[inline] @@ -193,7 +192,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(error::erase) } pub(super) fn aes_gcm_open( @@ -208,7 +207,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(error::erase) } /// ChaCha20-Poly1305 as described in [RFC 8439]. diff --git a/src/aead/gcm.rs b/src/aead/gcm.rs index 0940bf43b8..726bc9c681 100644 --- a/src/aead/gcm.rs +++ b/src/aead/gcm.rs @@ -16,7 +16,7 @@ use self::ffi::{Block, BLOCK_LEN, ZERO_BLOCK}; use super::{aes_gcm, Aad}; use crate::{ bits::{BitLength, FromByteLen as _}, - error::{self, InputTooLongError}, + error::InputTooLongError, polyfill::{sliceutil::overwrite_at_start, NotSend}, }; use cfg_if::cfg_if; @@ -53,14 +53,12 @@ impl<'key, K: Gmult> Context<'key, K> { key: &'key K, aad: Aad<&[u8]>, in_out_len: usize, - ) -> Result { + ) -> Result { if in_out_len > aes_gcm::MAX_IN_OUT_LEN { - return Err(error::Unspecified); + return Err(InputTooLongError::new(in_out_len)); } - let in_out_len = BitLength::from_byte_len(in_out_len) - .map_err(|_: InputTooLongError| error::Unspecified)?; - let aad_len = BitLength::from_byte_len(aad.as_ref().len()) - .map_err(|_: InputTooLongError| error::Unspecified)?; + let in_out_len = BitLength::from_byte_len(in_out_len)?; + let aad_len = BitLength::from_byte_len(aad.as_ref().len())?; // 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 diff --git a/src/aead/inout.rs b/src/aead/inout.rs index baa1c4adcf..05de0df505 100644 --- a/src/aead/inout.rs +++ b/src/aead/inout.rs @@ -12,7 +12,6 @@ // OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN // CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -use crate::error; use core::ops::RangeFrom; pub struct InOut<'i> { @@ -60,13 +59,8 @@ pub struct SrcIndexError(#[allow(dead_code)] RangeFrom); impl SrcIndexError { #[cold] + #[inline(never)] fn new(src: RangeFrom) -> Self { Self(src) } } - -impl From for error::Unspecified { - fn from(_: SrcIndexError) -> Self { - Self - } -} diff --git a/src/aead/open_error.rs b/src/aead/open_error.rs new file mode 100644 index 0000000000..d6ccb8fdcc --- /dev/null +++ b/src/aead/open_error.rs @@ -0,0 +1,37 @@ +// 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. + +use super::inout::SrcIndexError; +use crate::error::InputTooLongError; + +pub(super) enum OpenError { + #[allow(dead_code)] + SrcIndexError(SrcIndexError), + #[allow(dead_code)] + InputTooLong(InputTooLongError), +} + +impl OpenError { + #[cold] + #[inline(never)] + pub(super) fn src_index_error(source: SrcIndexError) -> Self { + Self::SrcIndexError(source) + } + + #[cold] + #[inline(never)] + pub(super) fn input_too_long(source: InputTooLongError) -> Self { + Self::InputTooLong(source) + } +}