RSA: Rearrange private prime validity checks.

Move all the checks that are done for each private prime into
the `PrivatePrime` constructor, to eliminate duplication.

This causes the 512-bit-ness check to be done earlier than before,
which affects some of the tests..

src/arithmetic/bigint.rs

@@ -740,13 +740,6 @@ pub fn elem_verify_equal_consttime<M, E>(
 
 // TODO: Move these methods from `Nonnegative` to `Modulus`.
 impl Nonnegative {
-    pub fn to_elem<M>(&self, m: &Modulus<M>) -> Result<Elem<M, Unencoded>, error::Unspecified> {
-        self.verify_less_than_modulus(m)?;
-        let mut r = m.zero();
-        r.limbs[0..self.limbs().len()].copy_from_slice(self.limbs());
-        Ok(r)
-    }
-
     pub fn verify_less_than_modulus<M>(&self, m: &Modulus<M>) -> Result<(), error::Unspecified> {
         if self.limbs().len() > m.limbs().len() {
             return Err(error::Unspecified);

src/arithmetic/bigint/modulus.rs

@@ -203,10 +203,10 @@ impl<M> OwnedModulusWithOne<M> {
     where
         M: SmallerModulus<L>,
     {
-        // TODO: Encode this assertion into the `where` above.
-        assert_eq!(self.limbs.len(), l.limbs.len());
+        let mut limbs = BoxedLimbs::zero(l.limbs.len());
+        limbs[..self.limbs.len()].copy_from_slice(&self.limbs);
         Elem {
-            limbs: BoxedLimbs::new_unchecked(self.limbs.clone().into_limbs()),
+            limbs,
             encoding: PhantomData,
         }
     }

src/rsa/keypair.rs

@@ -22,7 +22,8 @@ use crate::{
         bigint::{self, Prime},
         montgomery::R,
     },
-    bits, cpu, digest,
+    bits::BitLength,
+    cpu, digest,
     error::{self, KeyRejected},
     io::der,
     pkcs8, rand, signature,
@@ -267,20 +268,20 @@ impl KeyPair {
         let dQ = untrusted::Input::from(dQ);
         let qInv = untrusted::Input::from(qInv);
 
-        let (p, p_bits) = bigint::Nonnegative::from_be_bytes_with_bit_length(p)
+        let (p, _p_bits) = bigint::Nonnegative::from_be_bytes_with_bit_length(p)
             .map_err(|error::Unspecified| KeyRejected::invalid_encoding())?;
-        let (q, q_bits) = bigint::Nonnegative::from_be_bytes_with_bit_length(q)
+        let (q, _q_bits) = bigint::Nonnegative::from_be_bytes_with_bit_length(q)
             .map_err(|error::Unspecified| KeyRejected::invalid_encoding())?;
 
         // Our implementation of CRT-based modular exponentiation used requires
         // that `p > q` so swap them if `p < q`. If swapped, `qInv` is
         // recalculated below. `p != q` is verified implicitly below, e.g. when
         // `q_mod_p` is constructed.
-        let ((p, p_bits, dP), (q, q_bits, dQ, qInv)) = match q.verify_less_than(&p) {
-            Ok(_) => ((p, p_bits, dP), (q, q_bits, dQ, Some(qInv))),
+        let ((p, dP), (q, dQ, qInv)) = match q.verify_less_than(&p) {
+            Ok(_) => ((p, dP), (q, dQ, Some(qInv))),
             Err(error::Unspecified) => {
                 // TODO: verify `q` and `qInv` are inverses (mod p).
-                ((q, q_bits, dQ), (p, p_bits, dP, None))
+                ((q, dQ), (p, dP, None))
             }
         };
 
@@ -307,7 +308,7 @@ impl KeyPair {
         let public_key = PublicKey::from_modulus_and_exponent(
             n,
             e,
-            bits::BitLength::from_usize_bits(2048),
+            BitLength::from_usize_bits(2048),
             super::PRIVATE_KEY_PUBLIC_MODULUS_MAX_BITS,
             PublicExponent::_65537,
             cpu_features,
@@ -330,34 +331,12 @@ impl KeyPair {
 
         // Steps 5.a and 5.b are omitted, as explained above.
 
-        // Step 5.c.
-        //
-        // TODO: First, stop if `p < (√2) * 2**((nBits/2) - 1)`.
-        //
-        // Second, stop if `p > 2**(nBits/2) - 1`.
-        let half_n_bits = public_key.inner().n().len_bits().half_rounded_up();
-        if p_bits != half_n_bits {
-            return Err(KeyRejected::inconsistent_components());
-        }
-
-        // TODO: Step 5.d: Verify GCD(p - 1, e) == 1.
-
-        // Steps 5.e and 5.f are omitted as explained above.
+        let n_bits = public_key.inner().n().len_bits();
 
-        // Step 5.g.
-        //
-        // TODO: First, stop if `q < (√2) * 2**((nBits/2) - 1)`.
-        //
-        // Second, stop if `q > 2**(nBits/2) - 1`.
-        if p_bits != q_bits {
-            return Err(KeyRejected::inconsistent_components());
-        }
-
-        // TODO: Step 5.h: Verify GCD(p - 1, e) == 1.
+        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
-            .to_elem(n)
-            .map_err(|error::Unspecified| KeyRejected::inconsistent_components())?;
+        let q_mod_n_decoded = q.modulus.to_elem(n);
 
         // TODO: Step 5.i
         //
@@ -371,9 +350,7 @@ impl KeyPair {
         // 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 p_mod_n = p
-            .to_elem(n)
-            .map_err(|error::Unspecified| KeyRejected::inconsistent_components())?;
+        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() {
             return Err(KeyRejected::inconsistent_components());
@@ -388,7 +365,7 @@ impl KeyPair {
         // and knowing d is odd makes the inequality strict.
         let (d, d_bits) = bigint::Nonnegative::from_be_bytes_with_bit_length(d)
             .map_err(|_| error::KeyRejected::invalid_encoding())?;
-        if !(half_n_bits < d_bits) {
+        if !(n_bits.half_rounded_up() < d_bits) {
             return Err(KeyRejected::inconsistent_components());
         }
         // XXX: This check should be `d < LCM(p - 1, q - 1)`, but we don't have
@@ -401,14 +378,9 @@ impl KeyPair {
 
         // Step 6.b is omitted as explained above.
 
-        // 6.4.1.4.3 - Step 7.
-
-        // Step 7.a.
-        let p = PrivatePrime::new(p, dP, cpu_features)?;
         let pm = &p.modulus.modulus();
 
-        // Step 7.b.
-        let q = PrivatePrime::new(q, dQ, cpu_features)?;
+        // 6.4.1.4.3 - Step 7.
 
         let q_mod_p = q.modulus.to_elem(pm);
 
@@ -484,13 +456,27 @@ impl<M: Prime> PrivatePrime<M> {
     fn new(
         p: bigint::Nonnegative,
         dP: untrusted::Input,
+        n_bits: BitLength,
         cpu_features: cpu::Features,
     ) -> Result<Self, KeyRejected> {
         let p = bigint::OwnedModulusWithOne::from_nonnegative(p, cpu_features)?;
-        if p.len_bits().as_usize_bits() % 512 != 0 {
-            return Err(error::KeyRejected::private_modulus_len_not_multiple_of_512_bits());
+
+        // 5.c / 5.g:
+        //
+        // TODO: First, stop if `p < (√2) * 2**((nBits/2) - 1)`.
+        // TODO: First, stop if `q < (√2) * 2**((nBits/2) - 1)`.
+        //
+        // Second, stop if `p > 2**(nBits/2) - 1`.
+        // Second, stop if `q > 2**(nBits/2) - 1`.
+        if p.len_bits() != n_bits.half_rounded_up() {
+            return Err(KeyRejected::inconsistent_components());
         }
 
+        // TODO: Step 5.d: Verify GCD(p - 1, e) == 1.
+        // TODO: Step 5.h: Verify GCD(q - 1, e) == 1.
+
+        // Steps 5.e and 5.f are omitted as explained above.
+
         // [NIST SP-800-56B rev. 1] 6.4.1.4.3 - Steps 7.a & 7.b.
         let dP = bigint::PrivateExponent::from_be_bytes_padded(dP, &p.modulus())
             .map_err(|error::Unspecified| KeyRejected::inconsistent_components())?;
@@ -504,6 +490,10 @@ impl<M: Prime> PrivatePrime<M> {
         // and `e`. TODO: Either prove that what we do is sufficient, or make
         // it so.
 
+        if p.len_bits().as_usize_bits() % 512 != 0 {
+            return Err(error::KeyRejected::private_modulus_len_not_multiple_of_512_bits());
+        }
+
         Ok(Self {
             modulus: p,
             exponent: dP,

tests/rsa_from_pkcs8_tests.txt

@@ -38,7 +38,7 @@ Error = InconsistentComponents
 # private primes aren't equal and because the private modulus length isn't a
 # multiple of 512 bits.
 Input = 308204be020100300d06092a864886f70d0101010500048204a8308204a402010002820101019129387ed5eafe167c7c575ad391104cdb7ac95d25c2b0e3699a98f477de9729755574b112f552b28204480a061cf6a6360e0959000cb1b175301ee7d7accc82735c2388fb7bbf2673a309e25e2e4b26f046c9dfd858e78de52bd36991e0fc1a28f04b9ebdd6b0ab8da95b6467a7869b4e5a19dc1bc189b666729f7edb24a64965f53382ca45e20bb0a315439f7bead07a0bea88cf25a179e2aba7ee8584c1fa58e731246fcb4778e6f9ef455d0e50fc60c044f7745a4380b9f4c82dd4a64dae636461fd351f2e871477d801833bf44e7e4a7e08691776d263433c19d477290f1b844f0555c2995b22525b57681962332271a3cb522464c27989ab7c943f48e1020301000102820101011c7c6b00279bfa8cdfc31dffdf416499dc9bfae8ffba4dcf3838d677a7fa46a0b400e23c2101b09fbeec625a1973b8c6ae56cdf23bc2c4d0e0163c14963288e58a01b18197366e9c4a2d38f93b69010aa022d34ed71255439f5db11390cc487c14c4341c62ec3965af9486a7718ab03ebb15d278f18612af337fddf40c5cb53e46b99d7da4b7d8d9d586be8add8b0da6756a301155f2472b484c0ed8a23c052dcfe7ce40e99b6cae94edd41cc6a24fb5fda50b5ba3958cd3ccaa6aa2674cb9f492e208c8eb966f1d4d9b1c2a39efc324a41b562ebce4cbea0af06a6bdf19f4c45aed56efd9fd892ceeaa9abd3458c151a83a2b80075e1bab92b388a747baa64102818101aa9f633cbbe8135713302e9b0d22bfb7589c710a6516d25521550d98ee4b827183fe8351285387686d15abb3226813f5ec6367fd9138e2f5e5f248bacb3247219490092620f3660c4d7e091c5e61cbcd4992fc10441261aef9e80d4baa74f1fe9e7eb358c5098693954701a62c0309aecb7a32f0a763988d6b5fb26b5be8a07d02818100f0b8b228a6ef97211d25f0962132ec78ef48f99877d49d3b02e969a34b72eef8e289d0c99b8576d0e704ab27cb908b8c791ae9cba56340ceb9ab13436f6d19b07f1f485d8577b72f6bbc42b6edef6f9fe6dff28bbf790f26dae0c85f76689a440663372cf21f660cd860967b69d6ba491b58b7ad817eecb7f088df4053027b35028180265fd5e655a4a770b2aa27d70c946e984861320dd44ffc356a1d236ef92853a07056096bd86bc30752a09b642e991c0a87ebdd23c2d7521afa4713e1b17b614894fb6bc74139839961b30f90bcb0a14b62edd4bc85d2fd7466c847c1e1a0495034e382b05e70dfe91089658d93f1e602120d78dd8ac0fbace4d6a3cee2628765028181008155fb03348e4e59105c2e23bbbaacab5d858bf58b8cc4ddcbf6b5377376514790101409fe717b214abe8b675a4c536e2a3377a25f3e30b7e1b2352b6a56e812987aaa5af5371949754d355b2c0415a9885692eedaf5a45a70078e211c719f51254d717bc8ab6e1d40b4c4a5927c38a2c6faa7d5a55a18bdcb92fec084d934710281803e411557b7c0e6e6e296a1d0473b7bcf5f7a71d03c70bd6cadc3a9fb3a11cd0294dd02893ad1c28f62c20294d3893d6ac77baabf7772dd47be587893c79c8d66701c3037ceaec38088b500e2197994e048b1b357f8fffbbdbbdc94c0c82a7cac7ef0136cd6f7795355aeb3e4179eb2b363315f6ccb0a26434682adbbe7b6ce6b
-Error = InconsistentComponents
+Error = PrivateModulusLenNotMultipleOf512Bits
 
 # RSA 2050-bit key that is valid, but we reject it because the private modulus
 # length isn't a multiple of 512 bits.
@@ -59,7 +59,7 @@ Error = PrivateModulusLenNotMultipleOf512Bits
 # length isn't a multiple of 512 bits and because the private primes aren't
 # the same length.
 Input = 3082054b020100300d06092a864886f70d010101050004820535308205310201000282012061ca8d93087a5eb0f9a4e5dd47f62651673943ba5f3cfd6aaecbe355e6f37b422d7cbb13fd0675bdb22cf0e5906a224a83701f3256a78f61ba294160cb212dd5a6b42bda1a8bbfb7b21863dcc8a7aaf2a473dc71a9371e8f16e5dc77b2c4025a69fd27c60c7123e33d8c8bd03398566fe99e7601378e4acd97235bdcba08e388107cfab48c0ac6cc713aa09be6eaa4173bebac4141fc8857e09cb136d9e994efdc5a42a2dd2cf0a15a8f67ab8bf2ca5bfe728bbdd06b1d1eaddfa238d28a857e1b376613cd2260bfa7905300af58f8a14e7ecf41fc31cabd45f7f4445bb00d86f15912bcb730216dfdc656b57b94ee5f5c8b3000ca80628713332028b6d48249174fc8a0bd0ec2917b76218764070ae24355c31eaf389d3403d802bf3dc42b7d0203010001028201205268fd0804a4033a871744b3471a93ce13fc392c4d683b73751cd357280ad63bbdd398604ee1ebc551eaf793ae3112f6d26f618aed65c5ba28c4ed98a1763387123651a23aaebc8114afe3304354b1064c3a6b72c9946ec74d17a6c4a4bcf3f2b7d1247c8d2da77daff7ca3749d3be3c0f977f80a50ddfd203e5435beed6e0d0607cc255f6927a57fdafd77baa8ce07caa191b77c5b2b725636fd252f9246c4d26d4734e472e3884a0c205ef9e947b8882f9b27d8b3e305231757826b900aed455252552af86a1cc0f9f405126272d4e86a5b3a4539fa4ec46d1737921ad5a47046a9b2e1fbf9e745e802155b2e8205286b78317d5855494aae4ce94874c527d21027d11979bcef23917eef81bbd3e1fcf7cb04f8b0f61083acb08d94ed990cd02819100e6c1fc3765e9c9eb83316e06bd2e910f04cf23aa7aa5d303001db15f93bfba4fe28f19205499035be6d2c224396c9d669897dedd6a8c4f399dcffdb8181078baddc34d159a10741384db2a1850b4242a8f63b16c5478b72fbc17efef4866a5ae156e4b6222c86c2f4e370a55c9690f20122f6cffb37a1aa71d848bb9b7b84116a511ba76dedf8743751b842fb140ca4f0281906c7d0b982df2ac5d289aa1925f65be3c562373fc8f19171b53d85ffc4989cd30442175e423d4c106cd00e64f3108fe3f8cb3dd8eff6fd380334baa0e4372d98755a82c6d02e63443eb5020dcc2ce6e452bb6c7434a9758f1cb33b9ce6148a4bed5f042da4a14cbe83d161c26450b8c2f0fe4267d7ef0499b4c7b6407aa7e66f2d1ff5724a01ace5d30a3bc0bb2319673028191009c9e73122290b024b2119d8ce36034c24dd04d73caf4bba860aec69189556b1e07367aa64fe4debbc489ad8d2a9086b078c73353729ab2b6f75e90e51f2826d925e5056fb0f693e21f9d251005ce8e71788b0083f73d4b901188e9a7adab45ae470b6a0cbc70edc499c08fba340ea35e70195250a6bf2c51da9df8bbdf6f267639293436b40460e92f15b2fd2fc03acf028190157424f6d31f4a36a1f0f2429fdadee3cfa4f32adea177904fe45a16e9f3f63fb53968b6d4df1dccda7d730df6047142dff031c0358347aff274e40c2e0a6839fb1666b2a8001e15d052e82cbfd952b51b0fea17c488696e6760b33dbcbf40d4fc39a6fec3e798eb34294b27c63fa2592c2b51f13f49054550ba95ed6e29d0f1f3d52ac67362d9ac54120cfdc31f4b410281901ee70df154d97ae00935532cfdb43c4464bfdd0d6f76712fec6ae76f2a08f37f48eb2d54a341c023a1f79b143414f79479f5ace419ebca650a133fd6fa599dc3754851e04a5128bc007cccb1116339844861beacb1ab660ab629fcf0ed70ec41368324848f9fedc296c7104811b85141dfda4e7a51af43483e133d557c906b75590dea5480c2a3b7ad881ae6de3047f3
-Error = InconsistentComponents
+Error = PrivateModulusLenNotMultipleOf512Bits
 
 # RSA 2304-bit key that is valid, but we reject it because  the private modulus
 # length isn't a multiple of 512 bits.