Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Boxed uintlike coverage #436

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
13ca4be
Implemented and tested BoxedUint::bit_vartime
xuganyu96 Dec 15, 2023
f8ecad3
Ipmlemented trailing_ones and trailing_ones_vartime for BoxedUint
xuganyu96 Dec 15, 2023
ef2c7bb
Implemented div and sqrt for BoxedUint
xuganyu96 Dec 15, 2023
24792c3
Refactored shr_vartime and shl_vartime to return (Self, ConstChoice)
xuganyu96 Dec 15, 2023
97f2111
div_rem_limb_with_reciprocal and its dependencies
xuganyu96 Dec 15, 2023
83e065e
Boxed div by 2
xuganyu96 Dec 15, 2023
1d100c1
Gate boxed div_by_2 by alloc
xuganyu96 Dec 15, 2023
9666ad5
Share Reciprocal across Uint and BoxedUint
xuganyu96 Dec 15, 2023
d10f9fd
Fixed clippy and rust docs problem
xuganyu96 Dec 15, 2023
4aedbec
BoxedUint::is_nonzero returns Choice
xuganyu96 Dec 17, 2023
4db5f76
Refactored const_new to return (NonZero<BoxedUint>, Choice)
xuganyu96 Dec 17, 2023
8c839eb
replace select with conditional_select
xuganyu96 Dec 17, 2023
ce00645
Use ct_gt instead of gt
xuganyu96 Dec 17, 2023
a4f8125
Use crate::ConstantTimeSelect as a substitute for subtle::Conditional…
xuganyu96 Dec 18, 2023
1e6cb45
Use fully qualified namespace for div_by_2 to avoid confusion
xuganyu96 Dec 18, 2023
89171b5
Use ct_assign to avoid unnecessary cloning
xuganyu96 Dec 18, 2023
233b9e5
ConstCtOption<NonZero<Limb>>::expect
xuganyu96 Dec 18, 2023
97209b3
use overflowing_shl instead of shl_vartime; same for shr
xuganyu96 Dec 18, 2023
abbfd1f
Moved mulhilo and addhilo to crate::primitives
xuganyu96 Dec 20, 2023
49d72c5
Fixed import pathing for 32-bit targets
xuganyu96 Dec 20, 2023
d2d0e0f
Migrated from Residue to Monty
xuganyu96 Dec 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions benches/boxed_uint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ fn bench_shifts(c: &mut Criterion) {
group.bench_function("shl_vartime", |b| {
b.iter_batched(
|| BoxedUint::random(&mut OsRng, UINT_BITS),
|x| black_box(x.shl_vartime(UINT_BITS / 2 + 10)),
|x| black_box(x.overflowing_shl(UINT_BITS / 2 + 10).0),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change here? And if you're benchmarking overflowing_shl now, the string label should be changed too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sorry I think I got really confused between the various shl/shr functions.

BatchSize::SmallInput,
)
});
Expand All @@ -27,7 +27,7 @@ fn bench_shifts(c: &mut Criterion) {
group.bench_function("shr_vartime", |b| {
b.iter_batched(
|| BoxedUint::random(&mut OsRng, UINT_BITS),
|x| black_box(x.shr_vartime(UINT_BITS / 2 + 10)),
|x| black_box(x.overflowing_shr(UINT_BITS / 2 + 10).0),
BatchSize::SmallInput,
)
});
Expand Down
16 changes: 15 additions & 1 deletion src/const_choice.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use subtle::{Choice, CtOption};

use crate::{modular::BernsteinYangInverter, NonZero, Uint, Word};
use crate::{modular::BernsteinYangInverter, Limb, NonZero, Uint, Word};

/// A boolean value returned by constant-time `const fn`s.
// TODO: should be replaced by `subtle::Choice` or `CtOption`
Expand Down Expand Up @@ -305,6 +305,20 @@ impl<const LIMBS: usize> ConstCtOption<NonZero<Uint<LIMBS>>> {
}
}

impl ConstCtOption<NonZero<Limb>> {
/// Returns the contained value, consuming the `self` value.
///
/// # Panics
///
/// Panics if the value is none with a custom panic message provided by
/// `msg`.
#[inline]
pub const fn expect(self, msg: &str) -> NonZero<Limb> {
assert!(self.is_some.is_true_vartime(), "{}", msg);
self.value
}
}

impl<const SAT_LIMBS: usize, const UNSAT_LIMBS: usize>
ConstCtOption<BernsteinYangInverter<SAT_LIMBS, UNSAT_LIMBS>>
{
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ pub use crate::{
limb::{Limb, WideWord, Word},
non_zero::NonZero,
traits::*,
uint::div_limb::Reciprocal,
uint::reciprocal::Reciprocal,
uint::*,
wrapping::Wrapping,
};
Expand Down
26 changes: 25 additions & 1 deletion src/modular/boxed_monty_form.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ mod pow;
mod sub;

use super::{
div_by_2,
reduction::{montgomery_reduction_boxed, montgomery_reduction_boxed_mut},
Retrieve,
};
Expand Down Expand Up @@ -242,6 +243,18 @@ impl BoxedMontyForm {
debug_assert!(self.montgomery_form < self.params.modulus);
self.montgomery_form.clone()
}

/// Performs the modular division by 2, that is for given `x` returns `y`
/// such that `y * 2 = x mod p`. This means:
/// - if `x` is even, returns `x / 2`,
/// - if `x` is odd, returns `(x + p) / 2`
/// (since the modulus `p` in Montgomery form is always odd, this divides entirely).
pub fn div_by_2(&self) -> Self {
Self {
montgomery_form: div_by_2::boxed::div_by_2(&self.montgomery_form, &self.params.modulus),
params: self.params.clone(), // TODO: avoid clone?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There would be no need for clone if it was an inplace method, but if it creates a new number, I think cloning is unavoidable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When std is available they're stored as an Arc, so cloning is cheap (incrementing an atomic reference counter)

}
}
}

impl Retrieve for BoxedMontyForm {
Expand All @@ -263,7 +276,7 @@ fn convert_to_montgomery(integer: &mut BoxedUint, params: &BoxedMontyParams) {

#[cfg(test)]
mod tests {
use super::{BoxedMontyParams, BoxedUint};
use super::{BoxedMontyForm, BoxedMontyParams, BoxedUint};

#[test]
fn new_params_with_invalid_modulus() {
Expand All @@ -280,4 +293,15 @@ mod tests {
fn new_params_with_valid_modulus() {
BoxedMontyParams::new(BoxedUint::from(3u8)).unwrap();
}

#[test]
fn div_by_2() {
let params = BoxedMontyParams::new(BoxedUint::from(9u8)).unwrap();
let zero = BoxedMontyForm::zero(params.clone());
let one = BoxedMontyForm::one(params.clone());
let two = one.add(&one);

assert_eq!(zero.div_by_2(), zero);
assert_eq!(one.div_by_2().mul(&two), one);
}
}
20 changes: 20 additions & 0 deletions src/modular/div_by_2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,23 @@ pub(crate) fn div_by_2<const LIMBS: usize>(a: &Uint<LIMBS>, modulus: &Uint<LIMBS

Uint::<LIMBS>::select(&if_even, &if_odd, is_odd)
}

#[cfg(feature = "alloc")]
pub(crate) mod boxed {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think putting different variants of this function in modules is a bit of an overcomplication, perhaps just different suffixes would suffice.

use crate::{BoxedUint, ConstantTimeSelect};

pub(crate) fn div_by_2(a: &BoxedUint, modulus: &BoxedUint) -> BoxedUint {
debug_assert_eq!(a.bits_precision(), modulus.bits_precision());

let (mut half, is_odd) = a.shr1_with_carry();
let half_modulus = modulus.shr1();

let if_odd = half
.wrapping_add(&half_modulus)
.wrapping_add(&BoxedUint::one_with_precision(a.bits_precision()));

half.ct_assign(&if_odd, is_odd);

half
}
}
11 changes: 11 additions & 0 deletions src/modular/monty_form.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,4 +266,15 @@ mod test {
MontyParams::<LIMBS>::new(&Uint::from(2u8)).is_none()
))
}

#[test]
fn div_by_2() {
let params = MontyParams::new(&Uint::<1>::from(9u8)).unwrap();
let zero = MontyForm::zero(params.clone());
let one = MontyForm::one(params.clone());
let two = one.add(&one);

assert_eq!(zero.div_by_2(), zero);
assert_eq!(one.div_by_2().mul(&two), one);
}
}
1 change: 1 addition & 0 deletions src/uint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub(crate) mod mul;
mod mul_mod;
mod neg;
mod neg_mod;
pub mod reciprocal;
mod resize;
mod shl;
mod shr;
Expand Down
20 changes: 20 additions & 0 deletions src/uint/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ mod bits;
mod cmp;
mod ct;
mod div;
mod div_limb;
pub(crate) mod encoding;
mod from;
mod inv_mod;
Expand All @@ -19,6 +20,7 @@ mod neg;
mod neg_mod;
mod shl;
mod shr;
mod sqrt;
mod sub;
mod sub_mod;

Expand Down Expand Up @@ -92,6 +94,17 @@ impl BoxedUint {
.fold(Choice::from(1), |acc, limb| acc & limb.is_zero())
}

/// Returns the truthy value if `self`!=0 or the falsy value otherwise.
pub(crate) fn is_nonzero(&self) -> Choice {
let mut b = 0;
let mut i = 0;
while i < self.limbs.len() {
b |= self.limbs[i].0;
i += 1;
}
Limb(b).is_nonzero().into()
Comment on lines +99 to +105
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let mut b = 0;
let mut i = 0;
while i < self.limbs.len() {
b |= self.limbs[i].0;
i += 1;
}
Limb(b).is_nonzero().into()
let choice = Choice::from(1u8);
for limb in &self.limbs {
choice &= limb.ct_eq(&Limb::ZERO);
}
choice

Copy link
Contributor Author

@xuganyu96 xuganyu96 Dec 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just !self.is_zero()? The logic suggested above is basically what is_zero uses but backwards.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, we can do that, but other instances can be replaced with !x.is_zero() too. It makes less sense when const fn isn't required

}

/// Is this [`BoxedUint`] equal to one?
pub fn is_one(&self) -> Choice {
let mut iter = self.limbs.iter();
Expand Down Expand Up @@ -245,6 +258,13 @@ impl BoxedUint {
}

impl NonZero<BoxedUint> {
/// TODO: this is not really "const", but I need a way to return (value, choice) since
/// BoxedUint is not [`ConditionallySelectable`] so `CtChoice::map` and such does not work
pub fn const_new(n: BoxedUint) -> (Self, Choice) {
let nonzero = n.is_nonzero();
(Self(n), nonzero)
}
Comment on lines +261 to +266
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I factored these into methods on e.g. in this case BoxedUint: #484

You can use ConstCtChoice now.


/// Widen this type's precision to the given number of bits.
///
/// See [`BoxedUint::widen`] for more information, including panic conditions.
Expand Down
108 changes: 106 additions & 2 deletions src/uint/boxed/bits.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Bit manipulation functions.

use crate::{BoxedUint, Limb, Zero};
use crate::{BoxedUint, ConstChoice, Limb, Zero};
use subtle::{Choice, ConditionallySelectable, ConstantTimeEq};

impl BoxedUint {
Expand All @@ -24,6 +24,23 @@ impl BoxedUint {
Limb::BITS * n - leading_zeros
}

/// `floor(log2(self.bits_precision()))`.
pub(crate) fn log2_bits(&self) -> u32 {
u32::BITS - self.bits_precision().leading_zeros() - 1
}

/// Returns `true` if the bit at position `index` is set, `false` otherwise.
///
/// # Remarks
/// This operation is variable time with respect to `index` only.
pub fn bit_vartime(&self, index: u32) -> bool {
if index >= self.bits_precision() {
false
} else {
(self.limbs[(index / Limb::BITS) as usize].0 >> (index % Limb::BITS)) & 1 == 1
}
}

/// Calculate the number of bits needed to represent this number in variable-time with respect
/// to `self`.
pub fn bits_vartime(&self) -> u32 {
Expand Down Expand Up @@ -55,6 +72,45 @@ impl BoxedUint {
count
}

/// Calculate the number of trailing ones in the binary representation of this number.
pub fn trailing_ones(&self) -> u32 {
let limbs = self.as_limbs();

let mut count = 0;
let mut i = 0;
let mut nonmax_limb_not_encountered = ConstChoice::TRUE;
while i < limbs.len() {
let l = limbs[i];
let z = l.trailing_ones();
count += nonmax_limb_not_encountered.if_true_u32(z);
nonmax_limb_not_encountered =
nonmax_limb_not_encountered.and(ConstChoice::from_word_eq(l.0, Limb::MAX.0));
i += 1;
}

count
}

/// Calculate the number of trailing ones in the binary representation of this number,
/// variable time in `self`.
pub fn trailing_ones_vartime(&self) -> u32 {
let limbs = self.as_limbs();

let mut count = 0;
let mut i = 0;
while i < limbs.len() {
let l = limbs[i];
let z = l.trailing_ones();
count += z;
if z != Limb::BITS {
break;
}
i += 1;
}

count
}

/// Sets the bit at `index` to 0 or 1 depending on the value of `bit_value`.
pub(crate) fn set_bit(&mut self, index: u32, bit_value: Choice) {
let limb_num = (index / Limb::BITS) as usize;
Expand Down Expand Up @@ -84,7 +140,7 @@ mod tests {
fn uint_with_bits_at(positions: &[u32]) -> BoxedUint {
let mut result = BoxedUint::zero_with_precision(256);
for &pos in positions {
result |= BoxedUint::one_with_precision(256).shl_vartime(pos).unwrap();
result |= BoxedUint::one_with_precision(256).overflowing_shl(pos).0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason for the change here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another instance of me being confused. Will restore back to its original state.

}
result
}
Expand All @@ -101,6 +157,54 @@ mod tests {
assert_eq!(87, n2.bits());
}

#[test]
fn bit_vartime() {
let u = uint_with_bits_at(&[16, 48, 112, 127, 255]);
assert!(!u.bit_vartime(0));
assert!(!u.bit_vartime(1));
assert!(u.bit_vartime(16));
assert!(u.bit_vartime(127));
assert!(u.bit_vartime(255));
assert!(!u.bit_vartime(256));
assert!(!u.bit_vartime(260));
}

#[test]
fn trailing_ones() {
let u = !uint_with_bits_at(&[16, 79, 150]);
assert_eq!(u.trailing_ones(), 16);

let u = !uint_with_bits_at(&[79, 150]);
assert_eq!(u.trailing_ones(), 79);

let u = !uint_with_bits_at(&[150, 207]);
assert_eq!(u.trailing_ones(), 150);

let u = !uint_with_bits_at(&[0, 150, 207]);
assert_eq!(u.trailing_ones(), 0);

let u = !BoxedUint::zero_with_precision(256);
assert_eq!(u.trailing_ones(), 256);
}

#[test]
fn trailing_ones_vartime() {
let u = !uint_with_bits_at(&[16, 79, 150]);
assert_eq!(u.trailing_ones_vartime(), 16);

let u = !uint_with_bits_at(&[79, 150]);
assert_eq!(u.trailing_ones_vartime(), 79);

let u = !uint_with_bits_at(&[150, 207]);
assert_eq!(u.trailing_ones_vartime(), 150);

let u = !uint_with_bits_at(&[0, 150, 207]);
assert_eq!(u.trailing_ones_vartime(), 0);

let u = !BoxedUint::zero_with_precision(256);
assert_eq!(u.trailing_ones_vartime(), 256);
}

#[test]
fn set_bit() {
let mut u = uint_with_bits_at(&[16, 79, 150]);
Expand Down
22 changes: 22 additions & 0 deletions src/uint/boxed/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,28 @@ use subtle::{
Choice, ConditionallySelectable, ConstantTimeEq, ConstantTimeGreater, ConstantTimeLess,
};

impl BoxedUint {
/// Returns the Ordering between `self` and `rhs` in variable time.
pub fn cmp_vartime(&self, rhs: &Self) -> Ordering {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a feeling that using comparisons between limbs instead of subtraction may be faster. Put a TODO perhaps?

debug_assert_eq!(self.limbs.len(), rhs.limbs.len());
let mut i = self.limbs.len() - 1;
loop {
let (val, borrow) = self.limbs[i].sbb(rhs.limbs[i], Limb::ZERO);
if val.0 != 0 {
return if borrow.0 != 0 {
Ordering::Less
} else {
Ordering::Greater
};
}
if i == 0 {
return Ordering::Equal;
}
i -= 1;
}
}
}

impl ConstantTimeEq for BoxedUint {
#[inline]
fn ct_eq(&self, other: &Self) -> Choice {
Expand Down
Loading