Skip to content

Commit

Permalink
arithmetic: Rewrite limbs_reduce_once.
Browse files Browse the repository at this point in the history
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 non 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.
  • Loading branch information
briansmith committed Jan 24, 2025
1 parent a75be07 commit b68e556
Show file tree
Hide file tree
Showing 8 changed files with 94 additions and 44 deletions.
3 changes: 2 additions & 1 deletion build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -870,9 +870,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",
Expand Down
31 changes: 12 additions & 19 deletions crypto/limbs/limbs.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "../fipsmodule/bn/internal.h"
#include "limbs.inl"


/* XXX: We assume that the conversion from |Carry| to |Limb| is constant-time,
* but we haven't verified that assumption. TODO: Fix it so we don't need to
* make that assumption. */
Expand Down Expand Up @@ -61,25 +60,12 @@ 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) {
// `a` and `b` may alias.
Limb LIMBS_sub(Limb r[], const Limb a[], const Limb b[], 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);
Limb underflow =
constant_time_is_nonzero_w(limbs_sub(r, a, b, num_limbs));
return underflow;
}

void LIMBS_add_mod(Limb r[], const Limb a[], const Limb b[], const Limb m[],
Expand Down Expand Up @@ -122,6 +108,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 r[], const Limb a[], const Limb b[], size_t num_limbs, Limb cond) {
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) {
Expand Down
1 change: 0 additions & 1 deletion crypto/limbs/limbs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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[],
Expand Down
1 change: 1 addition & 0 deletions src/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ mod constant;
#[cfg(feature = "alloc")]
pub mod bigint;

pub mod add;
pub mod montgomery;

mod n0;
Expand Down
42 changes: 42 additions & 0 deletions src/arithmetic/add.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright 2015-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 crate::{c, error::LenMismatchError, limb::*};
use core::num::NonZeroUsize;

/// Equivalent to `r = if a < m { a } else { a - m };`
#[inline]
pub fn limbs_reduce_once(r: &mut [Limb], a: &[Limb], m: &[Limb]) -> Result<(), LenMismatchError> {
let underflow = limbs_sub(r, a, m)?;
limbs_cmov(r, a, underflow)
}

// `r = a - b`, returning true on underflow, false otherwise.
fn limbs_sub(r: &mut [Limb], a: &[Limb], b: &[Limb]) -> Result<LimbMask, LenMismatchError> {
prefixed_extern! {
// 'a' and `b` may alias.
fn LIMBS_sub(r: *mut Limb, a: *const Limb, b: *const Limb, num_limbs: c::NonZero_size_t)
-> LimbMask;
}
// XXX: LenMismatchError is not the best error to return for an empty `m`.
let num_limbs = NonZeroUsize::new(b.len()).ok_or_else(|| LenMismatchError::new(b.len()))?;
if r.len() != num_limbs.into() {
return Err(LenMismatchError::new(a.len()));

Check warning on line 35 in src/arithmetic/add.rs

View check run for this annotation

Codecov / codecov/patch

src/arithmetic/add.rs#L35

Added line #L35 was not covered by tests
}
if a.len() != num_limbs.into() {
return Err(LenMismatchError::new(a.len()));

Check warning on line 38 in src/arithmetic/add.rs

View check run for this annotation

Codecov / codecov/patch

src/arithmetic/add.rs#L38

Added line #L38 was not covered by tests
}
let underflow = unsafe { LIMBS_sub(r.as_mut_ptr(), a.as_ptr(), b.as_ptr(), num_limbs) };
Ok(underflow)
}
5 changes: 3 additions & 2 deletions src/arithmetic/bigint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub(crate) use self::{
modulusvalue::OwnedModulusValue,
private_exponent::PrivateExponent,
};
use super::{montgomery::*, LimbSliceError, MAX_LIMBS};
use super::{add::*, montgomery::*, LimbSliceError, MAX_LIMBS};
use crate::{
bits::BitLength,
c,
Expand Down Expand Up @@ -172,7 +172,8 @@ pub fn elem_reduced_once<A, M>(
assert_eq!(m.len_bits(), other_modulus_len_bits);

let mut r = a.limbs.clone();
limb::limbs_reduce_once_constant_time(&mut r, m.limbs());
limbs_reduce_once(&mut r, &a.limbs, m.limbs())
.unwrap_or_else(unwrap_impossible_len_mismatch_error);
Elem {
limbs: BoxedLimbs::new_unchecked(r.into_limbs()),
encoding: PhantomData,
Expand Down
24 changes: 14 additions & 10 deletions src/ec/suite_b/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use crate::{
arithmetic::limbs_from_hex,
arithmetic::montgomery::*,
arithmetic::{add::*, montgomery::*},
constant_time::LeakyWord,
cpu,
error::{self, LenMismatchError},
Expand Down Expand Up @@ -490,10 +490,15 @@ fn twin_mul_inefficient(

// This assumes n < q < 2*n.
impl Modulus<N> {
pub fn elem_reduced_to_scalar(&self, elem: &Elem<Unencoded>) -> Scalar<Unencoded> {
pub fn elem_reduced_to_scalar(&self, a: &Elem<Unencoded>) -> Scalar<Unencoded> {
let num_limbs = self.num_limbs.into();
let mut r_limbs = elem.limbs;
limbs_reduce_once_constant_time(&mut r_limbs[..num_limbs], &self.limbs[..num_limbs]);
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,
Expand Down Expand Up @@ -571,13 +576,12 @@ pub(super) fn scalar_parse_big_endian_partially_reduced_variable_consttime(
bytes: untrusted::Input,
) -> Result<Scalar, error::Unspecified> {
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_constant_time(r, &n.limbs[..num_limbs]);
}

limbs_reduce_once(&mut r.limbs[..num_limbs], unreduced, &n.limbs[..num_limbs])
.map_err(error::erase::<LenMismatchError>)?;
Ok(r)
}

Expand Down
31 changes: 20 additions & 11 deletions src/limb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
use crate::{
c, constant_time,
error::{self, LenMismatchError},
polyfill::{slice, usize_from_u32, ArrayFlatMap},
polyfill::{slice, usize_from_u32, AliasingSlices, ArrayFlatMap},
};
use core::num::NonZeroUsize;

Expand Down Expand Up @@ -154,16 +154,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_constant_time(r: &mut [Limb], m: &[Limb]) {
prefixed_extern! {
fn LIMBS_reduce_once(r: *mut Limb, m: *const Limb, num_limbs: c::size_t);
}
assert_eq!(r.len(), m.len());
unsafe { LIMBS_reduce_once(r.as_mut_ptr(), m.as_ptr(), m.len()) };
}

#[derive(Clone, Copy, PartialEq)]
pub enum AllowZero {
No,
Expand Down Expand Up @@ -376,6 +366,25 @@ pub(crate) fn limbs_negative_odd(r: &mut [Limb], a: &[Limb]) {
r[0] |= 1;
}

// `if cond { r = a; }`
pub fn limbs_cmov(r: &mut [Limb], a: &[Limb], cond: LimbMask) -> Result<(), LenMismatchError> {
prefixed_extern! {
// r, a, and/or b may alias.
fn LIMBS_select(
r: *mut Limb,
a: *const Limb,
b: *const Limb,
num_limbs: c::NonZero_size_t,
cond: LimbMask);
}
let len = r.len();
(r, a).with_pointers(len, |r, a, b| {
if let Some(num_limbs) = NonZeroUsize::new(len) {
unsafe { LIMBS_select(r, b, a, num_limbs, cond) }
}

Check warning on line 384 in src/limb.rs

View check run for this annotation

Codecov / codecov/patch

src/limb.rs#L384

Added line #L384 was not covered by tests
})
}

#[cfg(any(test, feature = "alloc"))]
prefixed_extern! {
fn LIMB_shr(a: Limb, shift: c::size_t) -> Limb;
Expand Down

0 comments on commit b68e556

Please sign in to comment.