From 290b36f7eb761a511133f1edc26693892cb4fbba Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 24 Jan 2025 13:44:05 -0800 Subject: [PATCH] arithmetic: Rewrite `limbs_reduce_once`. Move the function to `arithmetic` from `limb`. This is step towards moving all arithmetic out of `limb`. Change the signature so that the reduction is done separately instead of in-place. It was already being done separately in `bigint` and it costs very little, if anything, to do the same in the other caller in `ec`. Optimize the implementation to take advantage of the fact that `r` and `a` do not alias each other. To do so, replace `LIMBS_reduce_once` with two separate helper functions, `LIMBS_sub` and `LIMBS_cmov`. As part of doing this, ensure we're not passing any empty slices to the relevant C code. ``` git difftool HEAD^1:src/limb.rs src/arithmetic/limbs/reduce_once.rs ``` --- build.rs | 3 +- crypto/limbs/limbs.c | 36 ++++++++++-------------- crypto/limbs/limbs.h | 1 - src/arithmetic.rs | 2 +- src/arithmetic/bigint.rs | 6 ++-- src/arithmetic/limbs/fallback/cmov.rs | 38 +++++++++++++++++++++++++ src/arithmetic/limbs/fallback/mod.rs | 16 +++++++++++ src/arithmetic/limbs/fallback/sub.rs | 40 +++++++++++++++++++++++++++ src/arithmetic/limbs/mod.rs | 6 ++++ src/arithmetic/limbs/reduce_once.rs | 36 ++++++++++++++++++++++++ src/ec/suite_b/ops.rs | 26 +++++++++-------- src/limb.rs | 13 --------- 12 files changed, 171 insertions(+), 52 deletions(-) create mode 100644 src/arithmetic/limbs/fallback/cmov.rs create mode 100644 src/arithmetic/limbs/fallback/mod.rs create mode 100644 src/arithmetic/limbs/fallback/sub.rs create mode 100644 src/arithmetic/limbs/reduce_once.rs diff --git a/build.rs b/build.rs index eea7799082..a4190cc2d5 100644 --- a/build.rs +++ b/build.rs @@ -878,9 +878,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 d0027d400d..d093f1520c 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.rs b/src/arithmetic.rs index 94df7e461b..fcdc28b227 100644 --- a/src/arithmetic.rs +++ b/src/arithmetic.rs @@ -24,7 +24,7 @@ mod constant; pub mod bigint; pub(crate) mod inout; -mod limbs; +pub mod limbs; mod limbs512; pub mod montgomery; diff --git a/src/arithmetic/bigint.rs b/src/arithmetic/bigint.rs index 8d16890749..272153373f 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::{inout::AliasingSlices3, limbs512, montgomery::*, LimbSliceError, MAX_LIMBS}; +use super::{inout::AliasingSlices3, limbs::*, limbs512, montgomery::*, LimbSliceError, MAX_LIMBS}; use crate::{ bits::BitLength, c, @@ -213,8 +213,8 @@ pub fn elem_reduced_once( other_modulus_len_bits: BitLength, ) -> Elem { assert_eq!(m.len_bits(), other_modulus_len_bits); - r.limbs.copy_from_slice(&a.limbs); - limb::limbs_reduce_once(&mut r.limbs, m.limbs()) + + limbs_reduce_once(&mut r.limbs, &a.limbs, m.limbs()) .unwrap_or_else(unwrap_impossible_len_mismatch_error); Elem { limbs: r.limbs, diff --git a/src/arithmetic/limbs/fallback/cmov.rs b/src/arithmetic/limbs/fallback/cmov.rs new file mode 100644 index 0000000000..dc054e3378 --- /dev/null +++ b/src/arithmetic/limbs/fallback/cmov.rs @@ -0,0 +1,38 @@ +// 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::super::super::inout::AliasingSlices3; +use crate::{c, error::LenMismatchError, limb::*}; +use core::num::NonZeroUsize; + +// `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: NonZeroUsize, +) -> Result<(), LenMismatchError> { + prefixed_extern! { + // r, a, and/or b may alias. + fn LIMBS_select( + cond: Limb, + r: *mut Limb, + a: *const Limb, + b: *const Limb, + num_limbs: c::NonZero_size_t); + } + (r, a).with_non_dangling_non_null_pointers_rab(num_limbs, |r, a, b| unsafe { + LIMBS_select(cond, r, b, a, num_limbs) + }) +} diff --git a/src/arithmetic/limbs/fallback/mod.rs b/src/arithmetic/limbs/fallback/mod.rs new file mode 100644 index 0000000000..03521f8d5c --- /dev/null +++ b/src/arithmetic/limbs/fallback/mod.rs @@ -0,0 +1,16 @@ +// 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. + +pub(super) mod cmov; +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..69d5d43d3a --- /dev/null +++ b/src/arithmetic/limbs/fallback/sub.rs @@ -0,0 +1,40 @@ +// 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::super::super::inout::AliasingSlices3; +use crate::{c, error::LenMismatchError, limb::*}; +use core::num::NonZeroUsize; + +/// 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( + a_high: Limb, + in_out: impl AliasingSlices3, + num_limbs: NonZeroUsize, +) -> Result { + prefixed_extern! { + // `r`, 'a', and/or `b` may alias. + fn LIMBS_sub( + a_high: Limb, + r: *mut Limb, + a: *const Limb, + b: *const Limb, + num_limbs: c::NonZero_size_t) + -> Limb; + } + in_out.with_non_dangling_non_null_pointers_rab(num_limbs, |r, a, b| unsafe { + LIMBS_sub(a_high, r, a, b, num_limbs) + }) +} diff --git a/src/arithmetic/limbs/mod.rs b/src/arithmetic/limbs/mod.rs index 0dc6af36c3..e0a4b173df 100644 --- a/src/arithmetic/limbs/mod.rs +++ b/src/arithmetic/limbs/mod.rs @@ -12,5 +12,11 @@ // OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN // CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +mod fallback; +mod reduce_once; + pub(super) mod aarch64; 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..bd0c2901f6 --- /dev/null +++ b/src/arithmetic/limbs/reduce_once.rs @@ -0,0 +1,36 @@ +// 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::*}; +use core::num::NonZeroUsize; + +/// Equivalent to `if (r >= m) { r -= m; }` +#[inline] +pub fn limbs_reduce_once(r: &mut [Limb], a: &[Limb], m: &[Limb]) -> Result<(), 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( + a_high: Limb, + r: &mut [Limb], + a: &[Limb], + m: &[Limb], + num_limbs: NonZeroUsize, +) -> Result<(), LenMismatchError> { + #[allow(clippy::useless_asref)] + let borrow = limbs_sub(a_high, (r.as_mut(), a, m), num_limbs)?; + limbs_cmov(borrow, r, a, num_limbs) +} diff --git a/src/ec/suite_b/ops.rs b/src/ec/suite_b/ops.rs index ff3b425692..033ad7f7f1 100644 --- a/src/ec/suite_b/ops.rs +++ b/src/ec/suite_b/ops.rs @@ -14,7 +14,7 @@ use crate::{ arithmetic::limbs_from_hex, - arithmetic::montgomery::*, + arithmetic::{limbs::*, montgomery::*}, bb::LeakyWord, cpu, error::{self, LenMismatchError}, @@ -491,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; + limbs_reduce_once( + &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, @@ -573,14 +577,12 @@ 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)?; let mut r = Scalar::zero(); - { - let r = &mut r.limbs[..num_limbs]; - parse_big_endian_and_pad_consttime(bytes, r)?; - limbs_reduce_once(r, &n.limbs[..num_limbs]) - .unwrap_or_else(unwrap_impossible_len_mismatch_error); - } - + limbs_reduce_once(&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 1153571ab8..692bda6c7c 100644 --- a/src/limb.rs +++ b/src/limb.rs @@ -152,19 +152,6 @@ pub fn limbs_minimal_bits(a: &[Limb]) -> bits::BitLength { bits::BitLength::from_bits(0) } -/// Equivalent to `if (r >= m) { r -= m; }` -#[inline] -pub fn limbs_reduce_once(r: &mut [Limb], m: &[Limb]) -> Result<(), LenMismatchError> { - prefixed_extern! { - fn LIMBS_reduce_once(r: *mut Limb, m: *const Limb, num_limbs: c::NonZero_size_t); - } - let num_limbs = NonZeroUsize::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,