From b8bb124f334c11a13816532c34523e5644d575ab Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Sat, 13 Nov 2021 14:30:29 -0700 Subject: [PATCH] Fix `random_mod` performance for small moduli; `NonZero` moduli Changes the modulus for `random_mod` to be `NonZero`. Changes the algorithm used by `random_mod` to generate a number that can be represented by the same number of bytes as the modulus. Such a number can still be larger than the modulus, but is much more likely not to overflow than a "full-width" number provided the modulus is small relative to the width. Closes #3 --- src/limb.rs | 7 ++++++ src/limb/add.rs | 20 ++++++++++------ src/limb/bits.rs | 8 +++++++ src/limb/mul.rs | 2 +- src/limb/rand.rs | 37 ++++++++++++++++++++++++----- src/limb/sub.rs | 22 ++++++++++------- src/traits.rs | 4 ++-- src/uint/bits.rs | 6 ++--- src/uint/rand.rs | 58 +++++++++++++++++++++++++++++++++++++++++---- src/uint/sub_mod.rs | 18 +++++++------- 10 files changed, 141 insertions(+), 41 deletions(-) create mode 100644 src/limb/bits.rs diff --git a/src/limb.rs b/src/limb.rs index f7cf8609..fe763fff 100644 --- a/src/limb.rs +++ b/src/limb.rs @@ -8,6 +8,7 @@ mod bit_and; mod bit_not; mod bit_or; mod bit_xor; +mod bits; mod cmp; mod encoding; mod from; @@ -99,6 +100,12 @@ impl Limb { /// Maximum value this [`Limb`] can express. pub const MAX: Self = Limb(Inner::MAX); + /// Size of the inner integer in bits. + pub const BIT_SIZE: usize = BIT_SIZE; + + /// Size of the inner integer in bytes. + pub const BYTE_SIZE: usize = BYTE_SIZE; + /// Return `a` if `c`!=0 or `b` if `c`==0. /// /// Const-friendly: we can't yet use `subtle` in `const fn` contexts. diff --git a/src/limb/add.rs b/src/limb/add.rs index 5d39341e..4c4c0f15 100644 --- a/src/limb/add.rs +++ b/src/limb/add.rs @@ -1,7 +1,7 @@ //! Limb addition use super::{Inner, Limb, Wide}; -use crate::{Encoding, Wrapping}; +use crate::Wrapping; use core::ops::{Add, AddAssign}; use subtle::CtOption; @@ -16,12 +16,6 @@ impl Limb { (Limb(ret as Inner), Limb((ret >> Self::BIT_SIZE) as Inner)) } - /// Perform wrapping addition, discarding overflow. - #[inline(always)] - pub const fn wrapping_add(&self, rhs: Self) -> Self { - Limb(self.0.wrapping_add(rhs.0)) - } - /// Perform checked addition, returning a [`CtOption`] which `is_some` only /// if the operation did not overflow. #[inline] @@ -29,6 +23,18 @@ impl Limb { let (result, carry) = self.adc(rhs, Limb::ZERO); CtOption::new(result, carry.is_zero()) } + + /// Perform saturating addition. + #[inline] + pub fn saturating_add(&self, rhs: Self) -> Self { + Limb(self.0.saturating_add(rhs.0)) + } + + /// Perform wrapping addition, discarding overflow. + #[inline(always)] + pub const fn wrapping_add(&self, rhs: Self) -> Self { + Limb(self.0.wrapping_add(rhs.0)) + } } impl Add for Wrapping { diff --git a/src/limb/bits.rs b/src/limb/bits.rs new file mode 100644 index 00000000..1dfc6391 --- /dev/null +++ b/src/limb/bits.rs @@ -0,0 +1,8 @@ +use super::{Limb, BIT_SIZE}; + +impl Limb { + /// Calculate the number of bits needed to represent this number. + pub const fn bits(self) -> usize { + BIT_SIZE - (self.0.leading_zeros() as usize) + } +} diff --git a/src/limb/mul.rs b/src/limb/mul.rs index b31601ef..c0ec0f3e 100644 --- a/src/limb/mul.rs +++ b/src/limb/mul.rs @@ -1,7 +1,7 @@ //! Limb multiplication use super::{Inner, Limb, Wide}; -use crate::{Encoding, Wrapping}; +use crate::Wrapping; use core::ops::{Mul, MulAssign}; use subtle::CtOption; diff --git a/src/limb/rand.rs b/src/limb/rand.rs index 4e43adab..e2bfc831 100644 --- a/src/limb/rand.rs +++ b/src/limb/rand.rs @@ -1,20 +1,45 @@ //! Random number generator support -use super::Limb; +use super::{Limb, BYTE_SIZE}; +use crate::{Encoding, NonZero, Random, RandomMod}; use rand_core::{CryptoRng, RngCore}; +use subtle::ConstantTimeLess; -impl Limb { - /// Generate a random limb +impl Random for Limb { #[cfg(target_pointer_width = "32")] #[cfg_attr(docsrs, doc(cfg(feature = "rand")))] - pub fn random(mut rng: impl CryptoRng + RngCore) -> Self { + fn random(mut rng: impl CryptoRng + RngCore) -> Self { Self(rng.next_u32()) } - /// Generate a random limb #[cfg(target_pointer_width = "64")] #[cfg_attr(docsrs, doc(cfg(feature = "rand")))] - pub fn random(mut rng: impl CryptoRng + RngCore) -> Self { + fn random(mut rng: impl CryptoRng + RngCore) -> Self { Self(rng.next_u64()) } } + +impl RandomMod for Limb { + fn random_mod(mut rng: impl CryptoRng + RngCore, modulus: &NonZero) -> Self { + let mut bytes = ::Repr::default(); + + // TODO(tarcieri): use `div_ceil` when available + // See: https://github.com/rust-lang/rust/issues/88581 + let mut n_bytes = modulus.bits() / 8; + + // Ensure the randomly generated value can always be larger than + // the modulus in order to ensure a uniform distribution + if n_bytes < BYTE_SIZE { + n_bytes += 1; + } + + loop { + rng.fill_bytes(&mut bytes[..n_bytes]); + let n = Limb::from_le_bytes(bytes); + + if n.ct_lt(modulus).into() { + return n; + } + } + } +} diff --git a/src/limb/sub.rs b/src/limb/sub.rs index 1e1aa375..5c818ad8 100644 --- a/src/limb/sub.rs +++ b/src/limb/sub.rs @@ -1,7 +1,7 @@ //! Limb subtraction use super::{Inner, Limb, Wide}; -use crate::{Encoding, Wrapping}; +use crate::Wrapping; use core::ops::{Sub, SubAssign}; use subtle::CtOption; @@ -16,13 +16,6 @@ impl Limb { (Limb(ret as Inner), Limb((ret >> Self::BIT_SIZE) as Inner)) } - /// Perform wrapping subtraction, discarding underflow and wrapping around - /// the boundary of the type. - #[inline(always)] - pub const fn wrapping_sub(&self, rhs: Self) -> Self { - Limb(self.0.wrapping_sub(rhs.0)) - } - /// Perform checked subtraction, returning a [`CtOption`] which `is_some` /// only if the operation did not overflow. #[inline] @@ -30,6 +23,19 @@ impl Limb { let (result, underflow) = self.sbb(rhs, Limb::ZERO); CtOption::new(result, underflow.is_zero()) } + + /// Perform saturating subtraction. + #[inline] + pub fn saturating_sub(&self, rhs: Self) -> Self { + Limb(self.0.saturating_sub(rhs.0)) + } + + /// Perform wrapping subtraction, discarding underflow and wrapping around + /// the boundary of the type. + #[inline(always)] + pub const fn wrapping_sub(&self, rhs: Self) -> Self { + Limb(self.0.wrapping_sub(rhs.0)) + } } impl Sub for Wrapping { diff --git a/src/traits.rs b/src/traits.rs index 2c5cd9ff..46fd81c1 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -68,7 +68,7 @@ pub trait Random: Sized { /// Modular random number generation support. #[cfg(feature = "rand")] #[cfg_attr(docsrs, doc(cfg(feature = "rand")))] -pub trait RandomMod: Sized { +pub trait RandomMod: Sized + Zero { /// Generate a cryptographically secure random number which is less than /// a given `modulus`. /// @@ -80,7 +80,7 @@ pub trait RandomMod: Sized { /// issue so long as the underlying random number generator is truly a /// [`CryptoRng`], where previous outputs are unrelated to subsequent /// outputs and do not reveal information about the RNG's internal state. - fn random_mod(rng: impl CryptoRng + RngCore, modulus: &Self) -> Self; + fn random_mod(rng: impl CryptoRng + RngCore, modulus: &NonZero) -> Self; } /// Compute `self + rhs mod p`. diff --git a/src/uint/bits.rs b/src/uint/bits.rs index b6174b27..9d6f480d 100644 --- a/src/uint/bits.rs +++ b/src/uint/bits.rs @@ -2,8 +2,8 @@ use crate::limb::{Inner, BIT_SIZE}; use crate::{Limb, UInt}; impl UInt { - /// Calculate the number of bits needed to represent this number - pub(crate) const fn bits(self) -> Inner { + /// Calculate the number of bits needed to represent this number. + pub const fn bits(self) -> usize { let mut i = LIMBS - 1; while i > 0 && self.limbs[i].0 == 0 { i -= 1; @@ -17,6 +17,6 @@ impl UInt { Limb::ZERO, !self.limbs[0].is_nonzero() & !Limb(i as Inner).is_nonzero(), ) - .0 + .0 as usize } } diff --git a/src/uint/rand.rs b/src/uint/rand.rs index 86813cca..5763a487 100644 --- a/src/uint/rand.rs +++ b/src/uint/rand.rs @@ -2,7 +2,7 @@ // TODO(tarcieri): use `Random` and `RandomMod` impls exclusively in next breaking release use super::UInt; -use crate::{Limb, Random, RandomMod}; +use crate::{Limb, NonZero, Random, RandomMod}; use rand_core::{CryptoRng, RngCore}; use subtle::ConstantTimeLess; @@ -11,7 +11,7 @@ use subtle::ConstantTimeLess; impl UInt { /// Generate a cryptographically secure random [`UInt`]. pub fn random(mut rng: impl CryptoRng + RngCore) -> Self { - let mut limbs = [Limb::default(); LIMBS]; + let mut limbs = [Limb::ZERO; LIMBS]; for limb in &mut limbs { *limb = Limb::random(&mut rng) @@ -31,9 +31,31 @@ impl UInt { /// issue so long as the underlying random number generator is truly a /// [`CryptoRng`], where previous outputs are unrelated to subsequent /// outputs and do not reveal information about the RNG's internal state. - pub fn random_mod(mut rng: impl CryptoRng + RngCore, modulus: &Self) -> Self { + pub fn random_mod(mut rng: impl CryptoRng + RngCore, modulus: &NonZero) -> Self { + let mut n = Self::ZERO; + + // TODO(tarcieri): use `div_ceil` when available + // See: https://github.com/rust-lang/rust/issues/88581 + let mut n_limbs = modulus.bits() / Limb::BIT_SIZE; + if n_limbs < LIMBS { + n_limbs += 1; + } + + // Compute the highest limb of `modulus` as a `NonZero`. + // Add one to ensure `Limb::random_mod` returns values inclusive of this limb. + let modulus_hi = + NonZero::new(modulus.limbs[n_limbs.saturating_sub(1)].saturating_add(Limb::ONE)) + .unwrap(); // Always at least one due to `saturating_add` + loop { - let n = Self::random(&mut rng); + for i in 0..n_limbs { + n.limbs[i] = if (i + 1 == n_limbs) && (*modulus_hi != Limb::MAX) { + // Highest limb + Limb::random_mod(&mut rng, &modulus_hi) + } else { + Limb::random(&mut rng) + } + } if n.ct_lt(modulus).into() { return n; @@ -51,7 +73,33 @@ impl Random for UInt { #[cfg_attr(docsrs, doc(cfg(feature = "rand")))] impl RandomMod for UInt { - fn random_mod(rng: impl CryptoRng + RngCore, modulus: &Self) -> Self { + fn random_mod(rng: impl CryptoRng + RngCore, modulus: &NonZero) -> Self { Self::random_mod(rng, modulus) } } + +#[cfg(test)] +mod tests { + use crate::{NonZero, U256}; + use rand_core::SeedableRng; + + #[test] + fn random_mod() { + let mut rng = rand_chacha::ChaCha8Rng::seed_from_u64(1); + + // Ensure `random_mod` runs in a reasonable amount of time + let modulus = NonZero::new(U256::from(42u8)).unwrap(); + let res = U256::random_mod(&mut rng, &modulus); + + // Sanity check that the return value isn't zero + assert_ne!(res, U256::ZERO); + + // Ensure `random_mod` runs in a reasonable amount of time + // when the modulus is larger than 1 limb + let modulus = NonZero::new(U256::from(0x10000000000000001u128)).unwrap(); + let res = U256::random_mod(&mut rng, &modulus); + + // Sanity check that the return value isn't zero + assert_ne!(res, U256::ZERO); + } +} diff --git a/src/uint/sub_mod.rs b/src/uint/sub_mod.rs index 2f111b85..ed3fd968 100644 --- a/src/uint/sub_mod.rs +++ b/src/uint/sub_mod.rs @@ -45,18 +45,18 @@ impl_sub_mod!(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12); #[cfg(all(test, feature = "rand"))] mod tests { - use crate::UInt; + use crate::{Limb, NonZero, Random, UInt}; + use rand_core::SeedableRng; macro_rules! test_sub_mod { ($size:expr, $test_name:ident) => { #[test] fn $test_name() { - use crate::Limb; - use rand_core::SeedableRng; - let mut rng = rand_chacha::ChaCha8Rng::seed_from_u64(1); - - let moduli = [UInt::<$size>::random(&mut rng), UInt::random(&mut rng)]; + let moduli = [ + NonZero::>::random(&mut rng), + NonZero::>::random(&mut rng), + ]; for p in &moduli { let base_cases = [ @@ -79,7 +79,7 @@ mod tests { let (a, b) = if a < b { (b, a) } else { (a, b) }; let c = a.sub_mod(&b, p); - assert!(c < *p, "not reduced"); + assert!(c < **p, "not reduced"); assert_eq!(c, a.wrapping_sub(&b), "result incorrect"); } } @@ -89,10 +89,10 @@ mod tests { let b = UInt::<$size>::random_mod(&mut rng, p); let c = a.sub_mod(&b, p); - assert!(c < *p, "not reduced: {} >= {} ", c, p); + assert!(c < **p, "not reduced: {} >= {} ", c, p); let x = a.wrapping_sub(&b); - if a >= b && x < *p { + if a >= b && x < **p { assert_eq!(c, x, "incorrect result"); } }