From b794f56834e2bd7e29ab8acfd90429083b4e8f25 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Sun, 19 Jan 2025 13:37:29 -0800 Subject: [PATCH] arithmetic internals: Clarify memory safety of calls to `bn_mul_mont`. Replace `debug_asesrt!`-based checking with proper error checking. The error cases will never be reached because the callers already ensured that the slices are the correct lengths, but this is more clearly correct. The previous step defining the `InOut` type didn't work out so well, so replace `InOut` with `AliasingSlices` that does the same thing. The cost is more monomorphization, but that will become moot soon, and it already isn't too bad since there are only three cases to consider. It does help reduce the number of length checks that end up getting generated. --- src/arithmetic.rs | 19 +++++--- src/arithmetic/bigint.rs | 50 +++++++++++--------- src/arithmetic/ffi.rs | 92 ++++++++++++++++++++++++++++++++++++ src/arithmetic/inout.rs | 63 ++++++++++++++++++++++-- src/arithmetic/montgomery.rs | 64 ++++++++----------------- 5 files changed, 210 insertions(+), 78 deletions(-) create mode 100644 src/arithmetic/ffi.rs diff --git a/src/arithmetic.rs b/src/arithmetic.rs index 87ae90740..fb35cfefa 100644 --- a/src/arithmetic.rs +++ b/src/arithmetic.rs @@ -12,27 +12,34 @@ // OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN // CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -use crate::limb::LIMB_BITS; +pub(crate) use self::{constant::limbs_from_hex, limb_slice_error::LimbSliceError}; +use crate::{error::LenMismatchError, limb::LIMB_BITS}; + +#[macro_use] +mod ffi; mod constant; #[cfg(feature = "alloc")] pub mod bigint; -mod inout; +pub(crate) mod inout; pub mod montgomery; mod n0; // The minimum number of limbs allowed for any `&[Limb]` operation. // -// This must be at least 4 for bn_mul_mont to work, at least on x86. -// // TODO: Use `256 / LIMB_BITS` so that the limit is independent of limb size. -#[allow(dead_code)] // XXX: Presently only used by `bigint`. pub const MIN_LIMBS: usize = 4; // The maximum number of limbs allowed for any `&[Limb]` operation. pub const MAX_LIMBS: usize = 8192 / LIMB_BITS; -pub use self::{constant::limbs_from_hex, inout::InOut}; +cold_exhaustive_error! { + enum limb_slice_error::LimbSliceError { + len_mismatch => LenMismatch(LenMismatchError), + too_short => TooShort(usize), + too_long => TooLong(usize), + } +} diff --git a/src/arithmetic/bigint.rs b/src/arithmetic/bigint.rs index 7d68f2dbd..7f4bc3ab8 100644 --- a/src/arithmetic/bigint.rs +++ b/src/arithmetic/bigint.rs @@ -42,7 +42,7 @@ pub(crate) use self::{ modulusvalue::OwnedModulusValue, private_exponent::PrivateExponent, }; -use super::{montgomery::*, InOut, MAX_LIMBS}; +use super::{montgomery::*, LimbSliceError, MAX_LIMBS}; use crate::{ bits::BitLength, c, error, @@ -99,13 +99,8 @@ fn from_montgomery_amm(limbs: BoxedLimbs, m: &Modulus) -> Elem( where (E, E): ProductEncoding, { - limbs_square_mont(&mut a.limbs, m.limbs(), m.n0(), m.cpu_features()); + limbs_square_mont(&mut a.limbs, m.limbs(), m.n0(), m.cpu_features()) + .unwrap_or_else(unwrap_impossible_limb_slice_error); Elem { limbs: a.limbs, encoding: PhantomData, @@ -479,13 +475,8 @@ pub fn elem_exp_consttime( let src1 = entry(previous, src1, num_limbs); let src2 = entry(previous, src2, num_limbs); let dst = entry_mut(rest, 0, num_limbs); - limbs_mul_mont( - InOut::Disjoint(dst, src1), - src2, - m.limbs(), - m.n0(), - m.cpu_features(), - ); + limbs_mul_mont((dst, src1, src2), m.limbs(), m.n0(), m.cpu_features()) + .map_err(error::erase::)?; } let tmp = m.zero(); @@ -649,15 +640,16 @@ pub fn elem_exp_consttime( mut i: LeakyWindow, num_limbs: usize, cpu_features: cpu::Features, - ) { + ) -> Result<(), LimbSliceError> { loop { scatter(table, acc, i, num_limbs); i *= 2; if i >= TABLE_ENTRIES as LeakyWindow { break; } - limbs_square_mont(acc, m_cached, n0, cpu_features); + limbs_square_mont(acc, m_cached, n0, cpu_features)?; } + Ok(()) } // All entries in `table` will be Montgomery encoded. @@ -670,7 +662,8 @@ pub fn elem_exp_consttime( acc.copy_from_slice(base_cached); // Fill in entries 1, 2, 4, 8, 16. - scatter_powers_of_2(table, acc, m_cached, n0, 1, num_limbs, cpu_features); + scatter_powers_of_2(table, acc, m_cached, n0, 1, num_limbs, cpu_features) + .map_err(error::erase::)?; // Fill in entries 3, 6, 12, 24; 5, 10, 20, 30; 7, 14, 28; 9, 18; 11, 22; 13, 26; 15, 30; // 17; 19; 21; 23; 25; 27; 29; 31. for i in (3..(TABLE_ENTRIES as LeakyWindow)).step_by(2) { @@ -683,7 +676,8 @@ pub fn elem_exp_consttime( Window::from(i - 1), // Not secret num_limbs, ); - scatter_powers_of_2(table, acc, m_cached, n0, i, num_limbs, cpu_features); + scatter_powers_of_2(table, acc, m_cached, n0, i, num_limbs, cpu_features) + .map_err(error::erase::)?; } let acc = limb::fold_5_bit_windows( @@ -729,6 +723,16 @@ pub fn elem_verify_equal_consttime( } } +#[cold] +#[inline(never)] +fn unwrap_impossible_limb_slice_error(err: LimbSliceError) { + match err { + LimbSliceError::LenMismatch(_) => unreachable!(), + LimbSliceError::TooShort(_) => unreachable!(), + LimbSliceError::TooLong(_) => unreachable!(), + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/arithmetic/ffi.rs b/src/arithmetic/ffi.rs new file mode 100644 index 000000000..b3566546d --- /dev/null +++ b/src/arithmetic/ffi.rs @@ -0,0 +1,92 @@ +// Copyright 2024-2025 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::AliasingSlices, n0::N0, LimbSliceError, MAX_LIMBS, MIN_LIMBS}; +use crate::{c, limb::Limb, polyfill::usize_from_u32}; +use core::mem::size_of; + +const _MAX_LIMBS_ADDRESSES_MEMORY_SAFETY_ISSUES: () = { + // BoringSSL's limit: 8 kiloBYTES. + const BN_MONTGOMERY_MAX_WORDS: usize = (8 * 1092) / size_of::(); + assert!(MAX_LIMBS <= BN_MONTGOMERY_MAX_WORDS); + + // Some 64-bit assembly implementations were written to take `len` as a + // `c_int`, so they zero out the undefined top half of `len` to convert it + // to a `usize`. But, others don't. + assert!(MAX_LIMBS <= usize_from_u32(u32::MAX)); +}; + +macro_rules! bn_mul_mont_ffi { + ( $in_out:expr, $n:expr, $n0:expr, $cpu:expr, + unsafe { ($MIN_LEN:expr, $Cpu:ty) => $f:ident }) => {{ + use crate::{c, limb::Limb}; + prefixed_extern! { + // `r` and/or 'a' and/or 'b' may alias. + // XXX: BoringSSL declares these functions to return `int`. + fn $f( + r: *mut Limb, + a: *const Limb, + b: *const Limb, + n: *const Limb, + n0: &N0, + len: c::size_t, + ); + } + unsafe { + crate::arithmetic::ffi::bn_mul_mont_ffi::<$Cpu, { $MIN_LEN }>( + $in_out, $n, $n0, $cpu, $f, + ) + } + }}; +} + +#[inline] +pub(super) unsafe fn bn_mul_mont_ffi( + mut in_out: impl AliasingSlices, + n: &[Limb], + n0: &N0, + cpu: Cpu, + f: unsafe extern "C" fn( + r: *mut Limb, + a: *const Limb, + b: *const Limb, + n: *const Limb, + n0: &N0, + len: c::size_t, + ), +) -> Result<(), LimbSliceError> { + /// The x86 implementation of `bn_mul_mont`, at least, requires at least 4 + /// limbs. For a long time we have required 4 limbs for all targets, though + /// this may be unnecessary. + const _MIN_LIMBS_AT_LEAST_4: () = assert!(MIN_LIMBS >= 4); + // We haven't tested shorter lengths. + assert!(MIN_LEN >= MIN_LIMBS); + if n.len() < MIN_LEN { + return Err(LimbSliceError::too_short(n.len())); + } + + // Avoid stack overflow from the alloca inside. + if n.len() > MAX_LIMBS { + return Err(LimbSliceError::too_long(n.len())); + } + + in_out + .with_pointers(n.len(), |r, a, b| { + let len = n.len(); + let n = n.as_ptr(); + let _: Cpu = cpu; + unsafe { f(r, a, b, n, n0, len) }; + }) + .map_err(LimbSliceError::len_mismatch) +} diff --git a/src/arithmetic/inout.rs b/src/arithmetic/inout.rs index daafb4f5a..cd57e425c 100644 --- a/src/arithmetic/inout.rs +++ b/src/arithmetic/inout.rs @@ -12,8 +12,63 @@ // OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN // CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -pub enum InOut<'io, T: ?Sized> { - InPlace(&'io mut T), - #[cfg_attr(target_arch = "x86_64", allow(dead_code))] - Disjoint(&'io mut T, &'io T), +pub(crate) use crate::error::LenMismatchError; + +pub(crate) trait AliasingSlices { + fn with_pointers( + &mut self, + expected_len: usize, + f: impl FnOnce(*mut T, *const T, *const T) -> R, + ) -> Result; +} + +impl AliasingSlices for &mut [T] { + fn with_pointers( + &mut self, + expected_len: usize, + f: impl FnOnce(*mut T, *const T, *const T) -> R, + ) -> Result { + let r = self; + if r.len() != expected_len { + return Err(LenMismatchError::new(r.len())); + } + Ok(f(r.as_mut_ptr(), r.as_ptr(), r.as_ptr())) + } +} + +impl AliasingSlices for (&mut [T], &[T]) { + fn with_pointers( + &mut self, + expected_len: usize, + f: impl FnOnce(*mut T, *const T, *const T) -> R, + ) -> Result { + let (r, a) = self; + if r.len() != expected_len { + return Err(LenMismatchError::new(r.len())); + } + if a.len() != expected_len { + return Err(LenMismatchError::new(a.len())); + } + Ok(f(r.as_mut_ptr(), r.as_ptr(), a.as_ptr())) + } +} + +impl AliasingSlices for (&mut [T], &[T], &[T]) { + fn with_pointers( + &mut self, + expected_len: usize, + f: impl FnOnce(*mut T, *const T, *const T) -> R, + ) -> Result { + let (r, a, b) = self; + if r.len() != expected_len { + return Err(LenMismatchError::new(r.len())); + } + if a.len() != expected_len { + return Err(LenMismatchError::new(a.len())); + } + if b.len() != expected_len { + return Err(LenMismatchError::new(b.len())); + } + Ok(f(r.as_mut_ptr(), a.as_ptr(), b.as_ptr())) + } } diff --git a/src/arithmetic/montgomery.rs b/src/arithmetic/montgomery.rs index 7c115b559..6d9a83483 100644 --- a/src/arithmetic/montgomery.rs +++ b/src/arithmetic/montgomery.rs @@ -1,4 +1,4 @@ -// Copyright 2017-2023 Brian Smith. +// Copyright 2017-2025 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,7 +12,8 @@ // OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN // CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -pub use super::{n0::N0, InOut}; +pub use super::n0::N0; +use super::{inout::AliasingSlices, LimbSliceError, MIN_LIMBS}; use crate::cpu; // Indicates that the element is not encoded; there is no *R* factor @@ -113,25 +114,15 @@ impl ProductEncoding for (RRR, RInverse) { use crate::{bssl, c, limb::Limb}; #[inline(always)] -pub(super) fn limbs_mul_mont(ra: InOut<[Limb]>, b: &[Limb], n: &[Limb], n0: &N0, _: cpu::Features) { - // XXX/TODO: All the `debug_assert_eq!` length checking needs to be - // replaced with enforcement that happens regardless of debug mode. - let (r, a) = match ra { - InOut::InPlace(r) => { - debug_assert_eq!(r.len(), n.len()); - (r.as_mut_ptr(), r.as_ptr()) - } - InOut::Disjoint(r, a) => { - debug_assert_eq!(r.len(), n.len()); - debug_assert_eq!(a.len(), n.len()); - (r.as_mut_ptr(), a.as_ptr()) - } - }; - debug_assert_eq!(b.len(), n.len()); - let b = b.as_ptr(); - let num_limbs = n.len(); - let n = n.as_ptr(); - unsafe { bn_mul_mont(r, a, b, n, n0, num_limbs) } +pub(super) fn limbs_mul_mont( + in_out: impl AliasingSlices, + n: &[Limb], + n0: &N0, + cpu: cpu::Features, +) -> Result<(), LimbSliceError> { + bn_mul_mont_ffi!(in_out, n, n0, cpu, unsafe { + (MIN_LIMBS, cpu::Features) => bn_mul_mont + }) } #[cfg(not(any( @@ -242,31 +233,14 @@ prefixed_extern! { fn limbs_mul_add_limb(r: *mut Limb, a: *const Limb, b: Limb, num_limbs: c::size_t) -> Limb; } -#[cfg(any( - all(target_arch = "aarch64", target_endian = "little"), - all(target_arch = "arm", target_endian = "little"), - target_arch = "x86_64", - target_arch = "x86" -))] -prefixed_extern! { - // `r` and/or 'a' and/or 'b' may alias. - fn bn_mul_mont( - r: *mut Limb, - a: *const Limb, - b: *const Limb, - n: *const Limb, - n0: &N0, - num_limbs: c::size_t, - ); -} - /// r = r**2 -pub(super) fn limbs_square_mont(r: &mut [Limb], n: &[Limb], n0: &N0, _cpu: cpu::Features) { - debug_assert_eq!(r.len(), n.len()); - let r = r.as_mut_ptr(); - let num_limbs = n.len(); - let n = n.as_ptr(); - unsafe { bn_mul_mont(r, r, r, n, n0, num_limbs) } +pub(super) fn limbs_square_mont( + r: &mut [Limb], + n: &[Limb], + n0: &N0, + cpu: cpu::Features, +) -> Result<(), LimbSliceError> { + limbs_mul_mont(r, n, n0, cpu) } #[cfg(test)]