From 30f303a2c4ede8419a027cc8bf6f39c9539f72bc Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Tue, 7 Nov 2023 12:53:18 -0800 Subject: [PATCH] RSA: Remove QQ from RsaKeyPair. QQ comprised almost 25% of the bulk of RsaKeyPair and is actually completely unnecessary since `elem_reduced` can do the whole reduction itself. This has the nice and important side effect of eliminating some conversion operations between `bigint` types. This is also a step towards eliminating some of the `unsafe trait` stuff that kinda-but-not-really modeled modulus relationships. --- src/arithmetic/bigint.rs | 59 +++++++++----------- src/arithmetic/bigint/boxed_limbs.rs | 10 +--- src/arithmetic/bigint/modulus.rs | 15 +---- src/arithmetic/bigint_elem_reduced_tests.txt | 15 +++++ src/rsa/keypair.rs | 50 +++-------------- 5 files changed, 50 insertions(+), 99 deletions(-) diff --git a/src/arithmetic/bigint.rs b/src/arithmetic/bigint.rs index f439df8a7b..198f8b1956 100644 --- a/src/arithmetic/bigint.rs +++ b/src/arithmetic/bigint.rs @@ -45,6 +45,7 @@ use super::n0::N0; pub(crate) use super::nonnegative::Nonnegative; use crate::{ arithmetic::montgomery::*, + bits::BitLength, c, cpu, error, limb::{self, Limb, LimbMask, LIMB_BITS}, polyfill::u64_from_usize, @@ -80,31 +81,6 @@ pub unsafe trait Prime {} /// trait preemptively.) pub unsafe trait SmallerModulus {} -/// A modulus *s* where s < l < 2*s for the given larger modulus *l*. This is -/// the precondition for reduction by conditional subtraction, -/// `elem_reduce_once()`. -/// -/// # Safety -/// -/// Some logic may assume that the invariant holds when accessing limbs within -/// a value, e.g. by assuming that the smaller modulus is at most one limb -/// smaller than the larger modulus. TODO: Any such logic should be -/// encapsulated here, or this trait should be made non-`unsafe`. (In retrospect, -/// this shouldn't have been made an `unsafe` trait preemptively.) -pub unsafe trait SlightlySmallerModulus: SmallerModulus {} - -/// A modulus *s* where √l <= s < l for the given larger modulus *l*. This is -/// the precondition for the more general Montgomery reduction from ℤ/lℤ to -/// ℤ/sℤ. -/// -/// # Safety -/// -/// Some logic may assume that the invariant holds when accessing limbs within -/// a value. TODO: Any such logic should be encapsulated here, or this trait -/// should be made non-`unsafe`. (In retrospect, this shouldn't have been made -/// an `unsafe` trait preemptively.) -pub unsafe trait NotMuchSmallerModulus: SmallerModulus {} - pub trait PublicModulus {} /// Elements of ℤ/mℤ for some modulus *m*. @@ -214,12 +190,20 @@ fn elem_mul_by_2(a: &mut Elem, m: &Modulus) { } } -pub fn elem_reduced_once>( +// TODO: This is currently unused, but we intend to eventually use this to +// reduce elements (x mod q) mod p in the RSA CRT. If/when we do so, we +// should update the testing so it is reflective of that usage, instead of +// the old usage. +#[cfg(test)] +pub fn elem_reduced_once( a: &Elem, m: &Modulus, ) -> Elem { + // `limbs_reduce_once_constant_time` requires `r` and `m` to have the same + // number of limbs. + assert_eq!(a.limbs.len(), m.limbs().len()); + let mut r = a.limbs.clone(); - assert!(r.len() <= m.limbs().len()); limb::limbs_reduce_once_constant_time(&mut r, m.limbs()); Elem { limbs: BoxedLimbs::new_unchecked(r.into_limbs()), @@ -228,10 +212,19 @@ pub fn elem_reduced_once>( } #[inline] -pub fn elem_reduced>( +pub fn elem_reduced( a: &Elem, m: &Modulus, + other_prime_len_bits: BitLength, ) -> Elem { + // This is stricter than required mathematically but this is what we + // guarantee and this is easier to check. The real requirement is that + // that `a < m*R` where `R` is the Montgomery `R` for `m`. + assert_eq!(other_prime_len_bits, m.len_bits()); + + // `limbs_from_mont_in_place` requires this. + assert_eq!(a.limbs.len(), m.limbs().len() * 2); + let mut tmp = [0; MODULUS_MAX_LIMBS]; let tmp = &mut tmp[..a.limbs.len()]; tmp.copy_from_slice(&a.limbs); @@ -919,17 +912,16 @@ mod tests { |section, test_case| { assert_eq!(section, ""); - struct MM {} - unsafe impl SmallerModulus for M {} - unsafe impl NotMuchSmallerModulus for M {} + struct M {} let m_ = consume_modulus::(test_case, "M", cpu_features); let m = m_.modulus(); let expected_result = consume_elem(test_case, "R", &m); let a = - consume_elem_unchecked::(test_case, "A", expected_result.limbs.len() * 2); + consume_elem_unchecked::(test_case, "A", expected_result.limbs.len() * 2); + let other_modulus_len_bits = m_.len_bits(); - let actual_result = elem_reduced(&a, &m); + let actual_result = elem_reduced(&a, &m, other_modulus_len_bits); let oneRR = m_.oneRR(); let actual_result = elem_mul(oneRR.as_ref(), actual_result, &m); assert_elem_eq(&actual_result, &expected_result); @@ -950,7 +942,6 @@ mod tests { struct N {} struct QQ {} unsafe impl SmallerModulus for QQ {} - unsafe impl SlightlySmallerModulus for QQ {} let qq = consume_modulus::(test_case, "QQ", cpu_features); let expected_result = consume_elem::(test_case, "R", &qq.modulus()); diff --git a/src/arithmetic/bigint/boxed_limbs.rs b/src/arithmetic/bigint/boxed_limbs.rs index fdb91a4410..5064d3e1f4 100644 --- a/src/arithmetic/bigint/boxed_limbs.rs +++ b/src/arithmetic/bigint/boxed_limbs.rs @@ -17,7 +17,7 @@ use crate::{ error, limb::{self, Limb, LimbMask, LIMB_BYTES}, }; -use alloc::{borrow::ToOwned, boxed::Box, vec}; +use alloc::{boxed::Box, vec}; use core::{ marker::PhantomData, ops::{Deref, DerefMut}, @@ -82,14 +82,6 @@ impl BoxedLimbs { Ok(r) } - pub(super) fn minimal_width_from_unpadded(limbs: &[Limb]) -> Self { - debug_assert_ne!(limbs.last(), Some(&0)); - Self { - limbs: limbs.to_owned().into_boxed_slice(), - m: PhantomData, - } - } - pub(super) fn from_be_bytes_padded_less_than( input: untrusted::Input, m: &Modulus, diff --git a/src/arithmetic/bigint/modulus.rs b/src/arithmetic/bigint/modulus.rs index 51d874dc2d..5db15f376f 100644 --- a/src/arithmetic/bigint/modulus.rs +++ b/src/arithmetic/bigint/modulus.rs @@ -17,7 +17,7 @@ use super::{ montgomery::{Unencoded, RR}, n0::N0, }, - BoxedLimbs, Elem, Nonnegative, One, PublicModulus, SlightlySmallerModulus, SmallerModulus, + BoxedLimbs, Elem, Nonnegative, One, PublicModulus, SmallerModulus, }; use crate::{ bits::BitLength, @@ -124,19 +124,6 @@ impl OwnedModulusWithOne { Self::from_boxed_limbs(limbs, cpu_features) } - pub(crate) fn from_elem( - elem: Elem, - cpu_features: cpu::Features, - ) -> Result - where - M: SlightlySmallerModulus, - { - Self::from_boxed_limbs( - BoxedLimbs::minimal_width_from_unpadded(&elem.limbs), - cpu_features, - ) - } - fn from_boxed_limbs( n: BoxedLimbs, cpu_features: cpu::Features, diff --git a/src/arithmetic/bigint_elem_reduced_tests.txt b/src/arithmetic/bigint_elem_reduced_tests.txt index 009692d70c..7afffc21bb 100644 --- a/src/arithmetic/bigint_elem_reduced_tests.txt +++ b/src/arithmetic/bigint_elem_reduced_tests.txt @@ -32,6 +32,21 @@ R = fffffffdfffffd01000009000002f6fffdf403000312000402f3fff5f602fe080a0005fdfaff A = fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe00000000000001fffffffe00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000fffffffffffffe00000002000000fffffffe00000200fffffffffffdfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe00000000000003fffffffdfffffe00000002000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000fffffffffffffe000000000000010000000000000000 M = ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff00000000000000ffffffff00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ffffffffffffff +# m = 2**1023 + 1 (the smallest 1024 bit odd value), a = (m * 2**1024) - 1 (m*R - 1) +R = 8000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 +A = 8000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff +M = 8000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001 + +# m = 2**1023 + 1 (the smallest 1024 bit odd value), a = 2**(2*1024) - 1 (the largest 2048-bit value). +R = 03 +A = ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff +M = 8000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001 + +# m = 2**1024 - 1, a = 2**(2*1024) - 1 +R = 00 +A = ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff +M = ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff + # These vectors were adapted from BoringSSL's "Quotient" BIGNUM tests in the # following ways: diff --git a/src/rsa/keypair.rs b/src/rsa/keypair.rs index 3803bec3ac..5b34979338 100644 --- a/src/rsa/keypair.rs +++ b/src/rsa/keypair.rs @@ -35,13 +35,6 @@ pub struct KeyPair { q: PrivatePrime, qInv: bigint::Elem, - // XXX: qq's `oneRR` isn't used and thus this is about twice as large as - // it needs to be, according to how it is used. Further, it appears to be - // completely unnecessary since `elem_reduced` seems to be able to reduce - // an `Elem` directly to an `Elem`. TODO: Verify that is true and - // eliminate this. - qq: bigint::OwnedModulusWithOne, - // TODO: Eliminate `q_mod_n` entirely since it is a bad space:time trade-off. // Also, this is the only non-temporary `Elem` so if we eliminate this, we // can make all `Elem`s temporary (borrowed) values. @@ -336,8 +329,6 @@ impl KeyPair { let p = PrivatePrime::new(p, dP, n_bits, cpu_features)?; let q = PrivatePrime::new(q, dQ, n_bits, cpu_features)?; - let q_mod_n_decoded = q.modulus.to_elem(n); - // TODO: Step 5.i // // 3.b is unneeded since `n_bits` is derived here from `n`. @@ -349,7 +340,8 @@ impl KeyPair { // 0 < q < p < n. We check that q and p are close to sqrt(n) and then // assume that these preconditions are enough to let us assume that // checking p * q == 0 (mod n) is equivalent to checking p * q == n. - let q_mod_n = bigint::elem_mul(n_one.as_ref(), q_mod_n_decoded.clone(), n); + let q_mod_n = q.modulus.to_elem(n); + let q_mod_n = bigint::elem_mul(n_one.as_ref(), q_mod_n, n); let p_mod_n = p.modulus.to_elem(n); let pq_mod_n = bigint::elem_mul(&q_mod_n, p_mod_n, n); if !pq_mod_n.is_zero() { @@ -405,11 +397,6 @@ impl KeyPair { bigint::verify_inverses_consttime(&qInv, q_mod_p, pm) .map_err(|error::Unspecified| KeyRejected::inconsistent_components())?; - let qq = bigint::OwnedModulusWithOne::from_elem( - bigint::elem_mul(&q_mod_n, q_mod_n_decoded, n), - cpu_features, - )?; - // This should never fail since `n` and `e` were validated above. Ok(Self { @@ -417,7 +404,6 @@ impl KeyPair { q, qInv, q_mod_n, - qq, public: public_key, }) } @@ -501,16 +487,16 @@ impl PrivatePrime { } } -fn elem_exp_consttime( - c: &bigint::Elem, +fn elem_exp_consttime( + c: &bigint::Elem, p: &PrivatePrime, + other_prime_len_bits: BitLength, ) -> Result, error::Unspecified> where - M: bigint::NotMuchSmallerModulus, M: Prime, { let m = &p.modulus.modulus(); - let c_mod_m = bigint::elem_reduced(c, m); + let c_mod_m = bigint::elem_reduced(c, m, other_prime_len_bits); // We could precompute `oneRRR = elem_squared(&p.oneRR`) as mentioned // in the Smooth CRT-RSA paper. let c_mod_m = bigint::elem_mul(p.modulus.oneRR().as_ref(), c_mod_m, m); @@ -525,19 +511,6 @@ where enum P {} unsafe impl Prime for P {} unsafe impl bigint::SmallerModulus for P {} -unsafe impl bigint::NotMuchSmallerModulus for P {} - -#[derive(Copy, Clone)] -enum QQ {} -unsafe impl bigint::SmallerModulus for QQ {} -unsafe impl bigint::NotMuchSmallerModulus for QQ {} - -// `q < p < 2*q` since `q` is slightly smaller than `p` (see below). Thus: -// -// q < p < 2*q -// q*q < p*q < 2*q*q. -// q**2 < n < 2*(q**2). -unsafe impl bigint::SlightlySmallerModulus for QQ {} #[derive(Copy, Clone)] enum Q {} @@ -545,12 +518,6 @@ unsafe impl Prime for Q {} unsafe impl bigint::SmallerModulus for Q {} unsafe impl bigint::SmallerModulus

for Q {} -// q < p && `p.bit_length() == q.bit_length()` implies `q < p < 2*q`. -unsafe impl bigint::SlightlySmallerModulus

for Q {} - -unsafe impl bigint::SmallerModulus for Q {} -unsafe impl bigint::NotMuchSmallerModulus for Q {} - impl KeyPair { /// Computes the signature of `msg` and writes it into `signature`. /// @@ -620,9 +587,8 @@ impl KeyPair { let c = base; // Step 2.b.i. - let m_1 = elem_exp_consttime(&c, &self.p)?; - let c_mod_qq = bigint::elem_reduced_once(&c, &self.qq.modulus()); - let m_2 = elem_exp_consttime(&c_mod_qq, &self.q)?; + let m_1 = elem_exp_consttime(&c, &self.p, self.q.modulus.len_bits())?; + let m_2 = elem_exp_consttime(&c, &self.q, self.p.modulus.len_bits())?; // Step 2.b.ii isn't needed since there are only two primes.