From 923d6b122d0247bc98c47638663f11e3bb709484 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 14 Jun 2024 13:55:52 -0700 Subject: [PATCH] aes: Split FFI helpers into their own submodule. Make it clearer which types are used in the FFI and start separating the (unsafe) code for dealing with the C/assembly implementations from the (mostly safe) rest of the code. `git difftool HEAD^1:src/aead/aes.rs src/aead/aes/ffi.rs`. --- src/aead/aes.rs | 227 ++++++++------------------------------------ src/aead/aes/ffi.rs | 211 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 250 insertions(+), 188 deletions(-) create mode 100644 src/aead/aes/ffi.rs diff --git a/src/aead/aes.rs b/src/aead/aes.rs index 3192cccb08..8dd993d8db 100644 --- a/src/aead/aes.rs +++ b/src/aead/aes.rs @@ -1,4 +1,4 @@ -// Copyright 2018 Brian Smith. +// Copyright 2018-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 @@ -12,156 +12,26 @@ // OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN // CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -use super::{nonce::Nonce, quic::Sample, NONCE_LEN}; -use crate::{bits::BitLength, c, constant_time, cpu, error, polyfill::slice}; -use core::{num::NonZeroUsize, ops::RangeFrom}; +use super::{quic::Sample, Nonce, NONCE_LEN}; +use crate::{constant_time, cpu, error}; +use cfg_if::cfg_if; +use core::ops::RangeFrom; -#[derive(Clone)] -pub(super) struct Key { - inner: AES_KEY, -} - -// SAFETY: -// * The function `$name` must read `bits` bits from `user_key`; `bits` will -// always be a valid AES key length, i.e. a whole number of bytes. -// * `$name` must set `key.rounds` to the value expected by the corresponding -// encryption/decryption functions and return 0, or otherwise must return -// non-zero to indicate failure. -// * `$name` may inspect CPU features. -// -// In BoringSSL, the C prototypes for these are in -// crypto/fipsmodule/aes/internal.h. -macro_rules! set_encrypt_key { - ( $name:ident, $key_bytes:expr, $key:expr, $cpu_features:expr ) => {{ - prefixed_extern! { - fn $name(user_key: *const u8, bits: BitLength, key: *mut AES_KEY) -> c::int; - } - set_encrypt_key($name, $key_bytes, $key, $cpu_features) - }}; -} +pub(super) use ffi::Counter; +#[macro_use] +mod ffi; -#[inline] -unsafe fn set_encrypt_key( - f: unsafe extern "C" fn(*const u8, BitLength, *mut AES_KEY) -> c::int, - bytes: KeyBytes<'_>, - key: &mut AES_KEY, - _cpu_features: cpu::Features, -) -> Result<(), error::Unspecified> { - let (bytes, key_bits) = match bytes { - KeyBytes::AES_128(bytes) => (&bytes[..], BitLength::from_bits(128)), - KeyBytes::AES_256(bytes) => (&bytes[..], BitLength::from_bits(256)), - }; - - // Unusually, in this case zero means success and non-zero means failure. - if 0 == unsafe { f(bytes.as_ptr(), key_bits, key) } { - debug_assert_ne!(key.rounds, 0); // Sanity check initialization. - Ok(()) +cfg_if! { + if #[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))] { + pub(super) use ffi::AES_KEY; } else { - Err(error::Unspecified) - } -} - -macro_rules! encrypt_block { - ($name:ident, $block:expr, $key:expr) => {{ - prefixed_extern! { - fn $name(a: &Block, r: *mut Block, key: &AES_KEY); - } - encrypt_block_($name, $block, $key) - }}; -} - -#[inline] -fn encrypt_block_( - f: unsafe extern "C" fn(&Block, *mut Block, &AES_KEY), - a: Block, - key: &Key, -) -> Block { - let mut result = core::mem::MaybeUninit::uninit(); - unsafe { - f(&a, result.as_mut_ptr(), &key.inner); - result.assume_init() + use ffi::AES_KEY; } } -/// SAFETY: -/// * The caller must ensure that `$key` was initialized with the -/// `set_encrypt_key!` invocation that `$name` requires. -/// * The caller must ensure that fhe function `$name` satisfies the conditions -/// for the `f` parameter to `ctr32_encrypt_blocks`. -macro_rules! ctr32_encrypt_blocks { - ($name:ident, $in_out:expr, $src:expr, $key:expr, $ctr:expr, $cpu_features:expr ) => {{ - prefixed_extern! { - fn $name( - input: *const [u8; BLOCK_LEN], - output: *mut [u8; BLOCK_LEN], - blocks: c::NonZero_size_t, - key: &AES_KEY, - ivec: &Counter, - ); - } - ctr32_encrypt_blocks($name, $in_out, $src, $key, $ctr, $cpu_features) - }}; -} - -/// SAFETY: -/// * `f` must not read more than `blocks` blocks from `input`. -/// * `f` must write exactly `block` blocks to `output`. -/// * In particular, `f` must handle blocks == 0 without reading from `input` -/// or writing to `output`. -/// * `f` must support the input overlapping with the output exactly or -/// with any nonnegative offset `n` (i.e. `input == output.add(n)`); -/// `f` does NOT need to support the cases where input < output. -/// * `key` must have been initialized with the `set_encrypt_key!` invocation -/// that corresponds to `f`. -/// * `f` may inspect CPU features. -#[inline] -unsafe fn ctr32_encrypt_blocks( - f: unsafe extern "C" fn( - input: *const [u8; BLOCK_LEN], - output: *mut [u8; BLOCK_LEN], - blocks: c::NonZero_size_t, - key: &AES_KEY, - ivec: &Counter, - ), - in_out: &mut [u8], - src: RangeFrom, - key: &AES_KEY, - ctr: &mut Counter, - cpu_features: cpu::Features, -) { - let (input, leftover) = slice::as_chunks(&in_out[src]); - debug_assert_eq!(leftover.len(), 0); - - let blocks = match NonZeroUsize::new(input.len()) { - Some(blocks) => blocks, - None => { - return; - } - }; - - let blocks_u32: u32 = blocks.get().try_into().unwrap(); - - let input = input.as_ptr(); - let output: *mut [u8; BLOCK_LEN] = in_out.as_mut_ptr().cast(); - - let _: cpu::Features = cpu_features; - - // SAFETY: - // * `input` points to `blocks` blocks. - // * `output` points to space for `blocks` blocks to be written. - // * input == output.add(n), where n == src.start, and the caller is - // responsible for ensuing this sufficient for `f` to work correctly. - // * The caller is responsible for ensuring `f` can handle any value of - // `blocks` including zero. - // * The caller is responsible for ensuring `key` was initialized by the - // `set_encrypt_key!` invocation required by `f`. - // * CPU feature detection has been done so `f` can inspect - // CPU features. - unsafe { - f(input, output, blocks, key, ctr); - } - - ctr.increment_by_less_safe(blocks_u32); +#[derive(Clone)] +pub(super) struct Key { + inner: AES_KEY, } impl Key { @@ -170,17 +40,12 @@ impl Key { bytes: KeyBytes<'_>, cpu_features: cpu::Features, ) -> Result { - let mut key = AES_KEY { - rd_key: [0u32; 4 * (MAX_ROUNDS + 1)], - rounds: 0, - }; - - match detect_implementation(cpu_features) { + let key = match detect_implementation(cpu_features) { #[cfg(any(target_arch = "aarch64", target_arch = "x86_64", target_arch = "x86"))] // SAFETY: `aes_hw_set_encrypt_key` satisfies the `set_encrypt_key!` // contract for these target architectures. Implementation::HWAES => unsafe { - set_encrypt_key!(aes_hw_set_encrypt_key, bytes, &mut key, cpu_features)?; + set_encrypt_key!(aes_hw_set_encrypt_key, bytes, cpu_features) }, #[cfg(any( @@ -192,15 +57,15 @@ impl Key { // SAFETY: `vpaes_set_encrypt_key` satisfies the `set_encrypt_key!` // contract for these target architectures. Implementation::VPAES_BSAES => unsafe { - set_encrypt_key!(vpaes_set_encrypt_key, bytes, &mut key, cpu_features)?; + set_encrypt_key!(vpaes_set_encrypt_key, bytes, cpu_features) }, // SAFETY: `aes_nohw_set_encrypt_key` satisfies the `set_encrypt_key!` // contract. Implementation::NOHW => unsafe { - set_encrypt_key!(aes_nohw_set_encrypt_key, bytes, &mut key, cpu_features)?; + set_encrypt_key!(aes_nohw_set_encrypt_key, bytes, cpu_features) }, - }; + }?; Ok(Self { inner: key }) } @@ -218,9 +83,9 @@ impl Key { // `encrypt_iv_xor_block` calls `encrypt_block` on `target_arch = "x86"`. #[cfg(target_arch = "x86")] - Implementation::VPAES_BSAES => encrypt_block!(vpaes_encrypt, a, self), + Implementation::VPAES_BSAES => unsafe { encrypt_block!(vpaes_encrypt, a, &self.inner) }, - Implementation::NOHW => encrypt_block!(aes_nohw_encrypt, a, self), + Implementation::NOHW => unsafe { encrypt_block!(aes_nohw_encrypt, a, &self.inner) }, } } @@ -240,7 +105,8 @@ impl Key { _ => false, }; if use_ctr32 { - let mut ctr = Counter(iv.0); // We're only doing one block so this is OK. + // We're only doing one block so this is OK. + let mut ctr = Counter::from_iv_less_safe(iv); self.ctr32_encrypt_within(&mut block, 0.., &mut ctr, cpu_features); block } else { @@ -372,17 +238,6 @@ impl Key { } } -// Keep this in sync with AES_KEY in aes.h. -#[repr(C)] -#[derive(Clone)] -pub(super) struct AES_KEY { - pub rd_key: [u32; 4 * (MAX_ROUNDS + 1)], - pub rounds: c::uint, -} - -// Keep this in sync with `AES_MAXNR` in aes.h. -const MAX_ROUNDS: usize = 14; - pub const AES_128_KEY_LEN: usize = 128 / 8; pub const AES_256_KEY_LEN: usize = 256 / 8; @@ -391,10 +246,8 @@ pub enum KeyBytes<'a> { AES_256(&'a [u8; AES_256_KEY_LEN]), } -/// nonce || big-endian counter. -#[repr(transparent)] -pub(super) struct Counter([u8; BLOCK_LEN]); - +// `Counter` is `ffi::Counter` as its representation is dictated by its use in +// the FFI. impl Counter { pub fn one(nonce: Nonce) -> Self { let mut value = [0u8; BLOCK_LEN]; @@ -403,13 +256,21 @@ impl Counter { Self(value) } + pub(super) fn from_iv_less_safe(iv: Iv) -> Self { + Self(iv.0) + } + pub fn increment(&mut self) -> Iv { let iv = Iv(self.0); self.increment_by_less_safe(1); iv } - fn increment_by_less_safe(&mut self, increment_by: u32) { + pub fn into(self) -> Iv { + Iv(self.0) + } + + pub(super) fn increment_by_less_safe(&mut self, increment_by: u32) { let [.., c0, c1, c2, c3] = &mut self.0; let old_value: u32 = u32::from_be_bytes([*c0, *c1, *c2, *c3]); let new_value = old_value + increment_by; @@ -422,12 +283,6 @@ impl Counter { /// Intentionally not `Clone` to ensure each is used only once. pub struct Iv(Block); -impl From for Iv { - fn from(counter: Counter) -> Self { - Self(counter.0) - } -} - impl Iv { /// "Less safe" because it defeats attempts to use the type system to prevent reuse of the IV. #[inline] @@ -525,22 +380,18 @@ unsafe fn bsaes_ctr32_encrypt_blocks_with_vpaes_key( fn vpaes_encrypt_key_to_bsaes(bsaes_key: *mut AES_KEY, vpaes_key: &AES_KEY); } - let mut bsaes_key = AES_KEY { - rd_key: [0u32; 4 * (MAX_ROUNDS + 1)], - rounds: 0, - }; // SAFETY: // * The caller ensures `vpaes_key` was initialized by // `vpaes_set_encrypt_key`. // * `bsaes_key was zeroed above, and `vpaes_encrypt_key_to_bsaes` // is assumed to initialize `bsaes_key`. - unsafe { - vpaes_encrypt_key_to_bsaes(&mut bsaes_key, vpaes_key); - } + let bsaes_key = + unsafe { AES_KEY::derive(vpaes_encrypt_key_to_bsaes, &vpaes_key, cpu_features) }; + // The code for `vpaes_encrypt_key_to_bsaes` notes "vpaes stores one // fewer round count than bsaes, but the number of keys is the same," // so use this as a sanity check. - debug_assert_eq!(bsaes_key.rounds, vpaes_key.rounds + 1); + debug_assert_eq!(bsaes_key.rounds(), vpaes_key.rounds() + 1); // SAFETY: // * `bsaes_key` is in bsaes format after calling diff --git a/src/aead/aes/ffi.rs b/src/aead/aes/ffi.rs new file mode 100644 index 0000000000..5248fbe228 --- /dev/null +++ b/src/aead/aes/ffi.rs @@ -0,0 +1,211 @@ +// Copyright 2018-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::{Block, KeyBytes, BLOCK_LEN}; +use crate::{bits::BitLength, c, cpu, error, polyfill::slice}; +use core::{num::NonZeroUsize, ops::RangeFrom}; + +/// nonce || big-endian counter. +#[repr(transparent)] +pub(in super::super) struct Counter(pub(super) [u8; BLOCK_LEN]); + +// Keep this in sync with AES_KEY in aes.h. +#[repr(C)] +#[derive(Clone)] +pub(in super::super) struct AES_KEY { + pub rd_key: [u32; 4 * (MAX_ROUNDS + 1)], + pub rounds: c::uint, +} + +// Keep this in sync with `AES_MAXNR` in aes.h. +const MAX_ROUNDS: usize = 14; + +impl AES_KEY { + #[inline] + pub(super) unsafe fn new( + f: unsafe extern "C" fn(*const u8, BitLength, *mut AES_KEY) -> c::int, + bytes: KeyBytes<'_>, + _cpu_features: cpu::Features, + ) -> Result { + let mut key = Self { + rd_key: [0; 4 * (MAX_ROUNDS + 1)], + rounds: 0, + }; + + let (bytes, key_bits) = match bytes { + KeyBytes::AES_128(bytes) => (&bytes[..], BitLength::from_bits(128)), + KeyBytes::AES_256(bytes) => (&bytes[..], BitLength::from_bits(256)), + }; + + // Unusually, in this case zero means success and non-zero means failure. + if 0 == unsafe { f(bytes.as_ptr(), key_bits, &mut key) } { + debug_assert_ne!(key.rounds, 0); // Sanity check initialization. + Ok(key) + } else { + Err(error::Unspecified) + } + } +} + +#[cfg(target_arch = "arm")] +impl AES_KEY { + pub(super) unsafe fn derive( + f: for<'a> unsafe extern "C" fn(*mut AES_KEY, &'a AES_KEY), + src: &Self, + _cpu_features: cpu::Features, + ) -> Self { + let mut r = AES_KEY { + rd_key: [0u32; 4 * (MAX_ROUNDS + 1)], + rounds: 0, + }; + unsafe { f(&mut r, src) }; + r + } + + pub(super) fn rounds(&self) -> u32 { + self.rounds + } +} + +// SAFETY: +// * The function `$name` must read `bits` bits from `user_key`; `bits` will +// always be a valid AES key length, i.e. a whole number of bytes. +// * `$name` must set `key.rounds` to the value expected by the corresponding +// encryption/decryption functions and return 0, or otherwise must return +// non-zero to indicate failure. +// * `$name` may inspect CPU features. +// +// In BoringSSL, the C prototypes for these are in +// crypto/fipsmodule/aes/internal.h. +macro_rules! set_encrypt_key { + ( $name:ident, $key_bytes:expr, $cpu_features:expr $(,)? ) => {{ + use crate::{bits::BitLength, c}; + prefixed_extern! { + fn $name(user_key: *const u8, bits: BitLength, key: *mut AES_KEY) -> c::int; + } + $crate::aead::aes::ffi::AES_KEY::new($name, $key_bytes, $cpu_features) + }}; +} + +macro_rules! encrypt_block { + ($name:ident, $block:expr, $key:expr) => {{ + use crate::aead::aes::{ffi::AES_KEY, Block}; + prefixed_extern! { + fn $name(a: &Block, r: *mut Block, key: &AES_KEY); + } + $key.encrypt_block($name, $block) + }}; +} + +impl AES_KEY { + #[inline] + pub(super) unsafe fn encrypt_block( + &self, + f: unsafe extern "C" fn(&Block, *mut Block, &AES_KEY), + a: Block, + ) -> Block { + let mut result = core::mem::MaybeUninit::uninit(); + unsafe { + f(&a, result.as_mut_ptr(), self); + result.assume_init() + } + } +} + +/// SAFETY: +/// * The caller must ensure that `$key` was initialized with the +/// `set_encrypt_key!` invocation that `$name` requires. +/// * The caller must ensure that fhe function `$name` satisfies the conditions +/// for the `f` parameter to `ctr32_encrypt_blocks`. +macro_rules! ctr32_encrypt_blocks { + ($name:ident, $in_out:expr, $src:expr, $key:expr, $ctr:expr, $cpu_features:expr ) => {{ + use crate::{ + aead::aes::{ffi::AES_KEY, Counter, BLOCK_LEN}, + c, + }; + prefixed_extern! { + fn $name( + input: *const [u8; BLOCK_LEN], + output: *mut [u8; BLOCK_LEN], + blocks: c::NonZero_size_t, + key: &AES_KEY, + ivec: &Counter, + ); + } + $key.ctr32_encrypt_blocks($name, $in_out, $src, $ctr, $cpu_features) + }}; +} + +impl AES_KEY { + /// SAFETY: + /// * `f` must not read more than `blocks` blocks from `input`. + /// * `f` must write exactly `block` blocks to `output`. + /// * In particular, `f` must handle blocks == 0 without reading from `input` + /// or writing to `output`. + /// * `f` must support the input overlapping with the output exactly or + /// with any nonnegative offset `n` (i.e. `input == output.add(n)`); + /// `f` does NOT need to support the cases where input < output. + /// * `key` must have been initialized with the `set_encrypt_key!` invocation + /// that corresponds to `f`. + /// * `f` may inspect CPU features. + #[inline] + pub(super) unsafe fn ctr32_encrypt_blocks( + &self, + f: unsafe extern "C" fn( + input: *const [u8; BLOCK_LEN], + output: *mut [u8; BLOCK_LEN], + blocks: c::NonZero_size_t, + key: &AES_KEY, + ivec: &Counter, + ), + in_out: &mut [u8], + src: RangeFrom, + ctr: &mut Counter, + cpu_features: cpu::Features, + ) { + let (input, leftover) = slice::as_chunks(&in_out[src]); + debug_assert_eq!(leftover.len(), 0); + + let blocks = match NonZeroUsize::new(input.len()) { + Some(blocks) => blocks, + None => { + return; + } + }; + + let blocks_u32: u32 = blocks.get().try_into().unwrap(); + + let input = input.as_ptr(); + let output: *mut [u8; BLOCK_LEN] = in_out.as_mut_ptr().cast(); + + let _: cpu::Features = cpu_features; + + // SAFETY: + // * `input` points to `blocks` blocks. + // * `output` points to space for `blocks` blocks to be written. + // * input == output.add(n), where n == src.start, and the caller is + // responsible for ensuing this sufficient for `f` to work correctly. + // * The caller is responsible for ensuring `f` can handle any value of + // `blocks` including zero. + // * The caller is responsible for ensuring `key` was initialized by the + // `set_encrypt_key!` invocation required by `f`. + // * CPU feature detection has been done so `f` can inspect + // CPU features. + unsafe { + f(input, output, blocks, self, ctr); + } + + ctr.increment_by_less_safe(blocks_u32); + } +}