diff --git a/build/build.rs b/build/build.rs index 3f5ca4b355..5956c391af 100644 --- a/build/build.rs +++ b/build/build.rs @@ -739,9 +739,10 @@ fn prefix_all_symbols(pp: char, prefix_prefix: &str, prefix: &str) -> String { "LIMBS_are_zero", "LIMBS_equal", "LIMBS_less_than", - "LIMBS_reduce_once", + "LIMBS_select", "LIMBS_select_512_32", "LIMBS_shl_mod", + "LIMBS_sub", "LIMBS_sub_mod", "LIMBS_window5_split_window", "LIMBS_window5_unsplit_window", diff --git a/crypto/limbs/limbs.c b/crypto/limbs/limbs.c index 679f6ad015..adae406def 100644 --- a/crypto/limbs/limbs.c +++ b/crypto/limbs/limbs.c @@ -61,27 +61,6 @@ Limb LIMBS_less_than(const Limb a[], const Limb b[], size_t num_limbs) { return constant_time_is_nonzero_w(borrow); } -/* if (r >= m) { r -= m; } */ -void LIMBS_reduce_once(Limb r[], const Limb m[], size_t num_limbs) { - debug_assert_nonsecret(num_limbs >= 1); - /* This could be done more efficiently if we had |num_limbs| of extra space - * available, by storing |r - m| and then doing a conditional copy of either - * |r| or |r - m|. But, in order to operate in constant space, with an eye - * towards this function being used in RSA in the future, we do things a - * slightly less efficient way. */ - Limb lt = LIMBS_less_than(r, m, num_limbs); - Carry borrow = - limb_sub(&r[0], r[0], constant_time_select_w(lt, 0, m[0])); - for (size_t i = 1; i < num_limbs; ++i) { - /* XXX: This is probably particularly inefficient because the operations in - * constant_time_select affect the carry flag, so there will likely be - * loads and stores of |borrow|. */ - borrow = - limb_sbb(&r[i], r[i], constant_time_select_w(lt, 0, m[i]), borrow); - } - dev_assert_secret(borrow == 0); -} - void LIMBS_add_mod(Limb r[], const Limb a[], const Limb b[], const Limb m[], size_t num_limbs) { Limb overflow1 = @@ -94,6 +73,14 @@ void LIMBS_add_mod(Limb r[], const Limb a[], const Limb b[], const Limb m[], } } +// r, `a` and/or `b` may alias. +Limb LIMBS_sub(Limb a_high, Limb r[], const Limb a[], const Limb b[], size_t num_limbs) { + debug_assert_nonsecret(num_limbs >= 1); + Carry underflow = limbs_sub(r, a, b, num_limbs); + limb_sbb(&a_high, a_high, 0, underflow); + return a_high; +} + void LIMBS_sub_mod(Limb r[], const Limb a[], const Limb b[], const Limb m[], size_t num_limbs) { Limb underflow = @@ -122,6 +109,13 @@ void LIMBS_shl_mod(Limb r[], const Limb a[], const Limb m[], size_t num_limbs) { } } +// r, a, and/or b may alias. +void LIMBS_select(Limb cond, Limb r[], const Limb a[], const Limb b[], size_t num_limbs) { + for (size_t i = 0; i < num_limbs; ++i) { + r[i] = constant_time_select_w(cond, a[i], b[i]); + } +} + int LIMBS_select_512_32(Limb r[], const Limb table[], size_t num_limbs, crypto_word_t index) { if (num_limbs % (512 / LIMB_BITS) != 0) { diff --git a/crypto/limbs/limbs.h b/crypto/limbs/limbs.h index 0cf83dd651..4056f4a786 100644 --- a/crypto/limbs/limbs.h +++ b/crypto/limbs/limbs.h @@ -27,7 +27,6 @@ typedef crypto_word_t Limb; Limb LIMBS_are_zero(const Limb a[], size_t num_limbs); Limb LIMBS_equal(const Limb a[], const Limb b[], size_t num_limbs); -void LIMBS_reduce_once(Limb r[], const Limb m[], size_t num_limbs); void LIMBS_add_mod(Limb r[], const Limb a[], const Limb b[], const Limb m[], size_t num_limbs); void LIMBS_sub_mod(Limb r[], const Limb a[], const Limb b[], const Limb m[], diff --git a/src/arithmetic/bigint/elem.rs b/src/arithmetic/bigint/elem.rs index 90e60a9df6..ae209ab6c8 100644 --- a/src/arithmetic/bigint/elem.rs +++ b/src/arithmetic/bigint/elem.rs @@ -16,7 +16,7 @@ use crate::polyfill::prelude::*; use super::{ - super::{MAX_LIMBS, montgomery::*}, + super::{MAX_LIMBS, limbs, montgomery::*}, IntoMont, Mont, Uninit, boxed_limbs::BoxedLimbs, unwrap_impossible_len_mismatch_error, unwrap_impossible_limb_slice_error, @@ -194,13 +194,9 @@ impl Uninit { other_modulus_len_bits: BitLength, ) -> Elem { assert_eq!(m.len_bits(), other_modulus_len_bits); - // TODO: We should add a variant of `limbs_reduced_once` that does the - // reduction out-of-place, to eliminate this copy. - let mut r = self - .write_copy_of_slice_checked(a.limbs.as_ref()) - .unwrap_or_else(unwrap_impossible_len_mismatch_error); - limb::limbs_reduce_once(r.as_mut(), m.limbs()) - .unwrap_or_else(unwrap_impossible_len_mismatch_error); + let r = self + .write_fully_with(|r| limbs::limbs_reduce_once(r, a.leak_limbs_less_safe(), m.limbs())) + .unwrap_or_else(|LenMismatchError { .. }| unreachable!()); Elem::::assume_in_range_and_encoded_less_safe(r) } diff --git a/src/arithmetic/limbs/fallback/cmov.rs b/src/arithmetic/limbs/fallback/cmov.rs new file mode 100644 index 0000000000..ec388cf238 --- /dev/null +++ b/src/arithmetic/limbs/fallback/cmov.rs @@ -0,0 +1,44 @@ +// Copyright 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 AUTHOR DISCLAIMS ALL WARRANTIES +// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR 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 crate::{c, error::LenMismatchError, limb::*}; +use core::num::NonZero; + +// `if cond { r = a; }`, assuming `cond` is 0 (false) or 0xff..ff (true). +pub fn limbs_cmov( + cond: Limb, + r: &mut [Limb], + a: &[Limb], + num_limbs: NonZero, +) -> Result<(), LenMismatchError> { + prefixed_extern! { + // r, a, and/or b may alias. + unsafe fn LIMBS_select( + cond: Limb, + r: *mut Limb, + a: *const Limb, + b: *const Limb, + num_limbs: NonZero); + } + if r.len() != num_limbs.get() { + return Err(LenMismatchError::new(r.len())); + } + if a.len() != num_limbs.get() { + return Err(LenMismatchError::new(a.len())); + } + unsafe { + LIMBS_select(cond, r.as_mut_ptr(), a.as_ptr(), r.as_ptr(), num_limbs); + } + Ok(()) +} diff --git a/src/arithmetic/limbs/fallback/mod.rs b/src/arithmetic/limbs/fallback/mod.rs index acc5dd81ae..f3ded0adcd 100644 --- a/src/arithmetic/limbs/fallback/mod.rs +++ b/src/arithmetic/limbs/fallback/mod.rs @@ -12,4 +12,6 @@ // OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN // CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +pub(super) mod cmov; pub(in super::super) mod mont; +pub(super) mod sub; diff --git a/src/arithmetic/limbs/fallback/sub.rs b/src/arithmetic/limbs/fallback/sub.rs new file mode 100644 index 0000000000..1f311c5569 --- /dev/null +++ b/src/arithmetic/limbs/fallback/sub.rs @@ -0,0 +1,46 @@ +// Copyright 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 AUTHOR DISCLAIMS ALL WARRANTIES +// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR 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 crate::{ + c, + error::LenMismatchError, + limb::*, + polyfill::{StartMutPtr, slice::AliasingSlices}, +}; +use core::num::NonZero; + +/// r = a - b, where `a` is considered to have `a_high` appended to it as its most +/// significant word. The new value of `a_high` is returned. +#[inline] +pub(in super::super) fn limbs_sub<'o>( + a_high: Limb, + in_out: impl AliasingSlices<'o, Limb, 2>, + num_limbs: NonZero, +) -> Result<(&'o mut [Limb], Limb), LenMismatchError> { + prefixed_extern! { + // `r`, 'a', and/or `b` may alias. + unsafe fn LIMBS_sub( + a_high: Limb, + r: *mut Limb, + a: *const Limb, + b: *const Limb, + num_limbs: NonZero) + -> Limb; + } + in_out.with_non_dangling_non_null_pointers(num_limbs, |mut r, [a, b]| { + let borrow = unsafe { LIMBS_sub(a_high, r.start_mut_ptr(), a, b, num_limbs) }; + let r = unsafe { r.deref_unchecked().assume_init() }; + (r, borrow) + }) +} diff --git a/src/arithmetic/limbs/mod.rs b/src/arithmetic/limbs/mod.rs index 54d88f1318..e3aa8b68f0 100644 --- a/src/arithmetic/limbs/mod.rs +++ b/src/arithmetic/limbs/mod.rs @@ -14,4 +14,8 @@ pub(super) mod aarch64; pub(super) mod fallback; +pub(super) mod reduce_once; pub(super) mod x86_64; + +pub(crate) use self::reduce_once::limbs_reduce_once; +use fallback::{cmov::limbs_cmov, sub::limbs_sub}; diff --git a/src/arithmetic/limbs/reduce_once.rs b/src/arithmetic/limbs/reduce_once.rs new file mode 100644 index 0000000000..f2c642a5f0 --- /dev/null +++ b/src/arithmetic/limbs/reduce_once.rs @@ -0,0 +1,41 @@ +// Copyright 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 AUTHOR DISCLAIMS ALL WARRANTIES +// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR 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::*; +use crate::{error::LenMismatchError, limb::*, polyfill::slice::Uninit}; +use core::num::NonZeroUsize; + +/// Equivalent to `if (r >= m) { r -= m; }` +#[inline] +pub fn limbs_reduce_once<'r>( + r: Uninit<'r, Limb>, + a: &[Limb], + m: &[Limb], +) -> Result<&'r mut [Limb], LenMismatchError> { + let num_limbs = NonZeroUsize::new(m.len()).ok_or_else(|| LenMismatchError::new(m.len()))?; + reduce_once(0, r, a, m, num_limbs) +} + +fn reduce_once<'r>( + a_high: Limb, + r: Uninit<'r, Limb>, + a: &[Limb], + m: &[Limb], + num_limbs: NonZeroUsize, +) -> Result<&'r mut [Limb], LenMismatchError> { + #[allow(clippy::useless_asref)] + let (r, borrow) = limbs_sub(a_high, (r, a, m), num_limbs)?; + limbs_cmov(borrow, r, a, num_limbs)?; + Ok(r) +} diff --git a/src/arithmetic/mod.rs b/src/arithmetic/mod.rs index b4824a01ea..fa595daccf 100644 --- a/src/arithmetic/mod.rs +++ b/src/arithmetic/mod.rs @@ -24,7 +24,7 @@ mod constant; pub mod bigint; mod exp_vartime; -mod limbs; +pub mod limbs; mod limbs512; pub mod montgomery; diff --git a/src/ec/suite_b/ops/mod.rs b/src/ec/suite_b/ops/mod.rs index 67c22c8f58..af139cd422 100644 --- a/src/ec/suite_b/ops/mod.rs +++ b/src/ec/suite_b/ops/mod.rs @@ -14,7 +14,7 @@ use crate::{ arithmetic::limbs_from_hex, - arithmetic::montgomery::*, + arithmetic::{limbs::*, montgomery::*}, bb::LeakyWord, cpu, error::{self, LenMismatchError}, @@ -22,6 +22,7 @@ use crate::{ }; use core::marker::PhantomData; +use crate::polyfill::slice::Uninit; use elem::{mul_mont, unary_op, unary_op_assign, unary_op_from_binary_op_assign}; /// A field element, i.e. an element of ℤ/qℤ for the curve's field modulus @@ -490,11 +491,15 @@ fn twin_mul_inefficient( // This assumes n < q < 2*n. impl Modulus { - pub fn elem_reduced_to_scalar(&self, elem: &Elem) -> Scalar { + pub fn elem_reduced_to_scalar(&self, a: &Elem) -> Scalar { let num_limbs = self.num_limbs.into(); - let mut r_limbs = elem.limbs; - limbs_reduce_once(&mut r_limbs[..num_limbs], &self.limbs[..num_limbs]) - .unwrap_or_else(unwrap_impossible_len_mismatch_error); + let mut r_limbs = a.limbs; + let _: &mut [Limb] = limbs_reduce_once( + Uninit::from_mut(&mut r_limbs[..num_limbs]), + &a.limbs[..num_limbs], + &self.limbs[..num_limbs], + ) + .unwrap_or_else(unwrap_impossible_len_mismatch_error); Scalar { limbs: r_limbs, m: PhantomData, @@ -572,14 +577,17 @@ pub(super) fn scalar_parse_big_endian_partially_reduced_variable_consttime( bytes: untrusted::Input, ) -> Result { let num_limbs = n.num_limbs.into(); + let mut unreduced = [0; elem::NumLimbs::MAX]; + let unreduced = &mut unreduced[..num_limbs]; + parse_big_endian_and_pad_consttime(bytes, unreduced) + .map_err(error::erase::)?; let mut r = Scalar::zero(); - { - let r = &mut r.limbs[..num_limbs]; - parse_big_endian_and_pad_consttime(bytes, r).map_err(error::erase::)?; - limbs_reduce_once(r, &n.limbs[..num_limbs]) - .unwrap_or_else(unwrap_impossible_len_mismatch_error); - } - + let _: &mut _ = limbs_reduce_once( + Uninit::from_mut(&mut r.limbs[..num_limbs]), + unreduced, + &n.limbs[..num_limbs], + ) + .map_err(error::erase::)?; Ok(r) } diff --git a/src/limb.rs b/src/limb.rs index 2468812447..6969bcf317 100644 --- a/src/limb.rs +++ b/src/limb.rs @@ -123,19 +123,6 @@ pub fn verify_limbs_equal_1_leak_bit(a: &[Limb]) -> Result<(), error::Unspecifie Err(error::Unspecified) } -/// Equivalent to `if (r >= m) { r -= m; }` -#[inline] -pub fn limbs_reduce_once(r: &mut [Limb], m: &[Limb]) -> Result<(), LenMismatchError> { - prefixed_extern! { - unsafe fn LIMBS_reduce_once(r: *mut Limb, m: *const Limb, num_limbs: NonZero); - } - let num_limbs = NonZero::new(r.len()).ok_or_else(|| LenMismatchError::new(m.len()))?; - let r = r.as_mut_ptr(); // Non-dangling because num_limbs is non-zero. - let m = m.as_ptr(); // Non-dangling because num_limbs is non-zero. - unsafe { LIMBS_reduce_once(r, m, num_limbs) }; - Ok(()) -} - #[derive(Clone, Copy, PartialEq)] pub enum AllowZero { No, diff --git a/src/polyfill/uninit_slice.rs b/src/polyfill/uninit_slice.rs index acd0de49c8..d59225228f 100644 --- a/src/polyfill/uninit_slice.rs +++ b/src/polyfill/uninit_slice.rs @@ -169,14 +169,24 @@ impl<'target, E: Copy> Uninit<'target, E> { // somebody might then unsoundly write `uninit` into it without using `unsafe`. // We avoid that problem here because `Uninit` never writes `uninit` and it // never exposes a `MaybeUninit` (mutable) reference externally. -impl<'target, E> AliasedUninit<'target, E> { +impl<'target, E> Uninit<'target, E> { pub fn from_mut(target: &'target mut [E]) -> Self { let target: &'target mut [E] = target; let target: *mut [E] = target; let target: *mut [MaybeUninit] = target as *mut [MaybeUninit]; - let _target: &'target mut [MaybeUninit] = unsafe { &mut *target }; + let target: &'target mut [MaybeUninit] = unsafe { &mut *target }; + Self { target } + } +} + +// Generally it isn't safe to cast `mut T` to `mut MaybeUninit` because +// somebody might then unsoundly write `uninit` into it without using `unsafe`. +// We avoid that problem here because `Uninit` never writes `uninit` and it +// never exposes a `MaybeUninit` (mutable) reference externally. +impl<'target, E> AliasedUninit<'target, E> { + pub fn from_mut(target: &'target mut [E]) -> Self { Self { - target, + target: Uninit::from_mut(target).target, _a: PhantomData, } }