Reapply "BREAKING: Make random_mod platform-independent"

This reverts 5784b13, reapplying 62b90b8. See also:
RustCrypto#1018 (comment)

It turns out that the previous approach ran afoul of a likely LLVM (or
possibly rustc) bug.

This time, I tried removing `random_mod_core` entirely and instead
writing `random_mod` and `try_random_mod` as rejection loops over
`random_bits` and `try_random_bits`. Somehow, this does not (seem to)
run afoul of the bug.

Author: mrdomino

src/uint/rand.rs

@@ -1,9 +1,8 @@
 //! Random number generator support
 
 use super::{Uint, Word};
-use crate::{Encoding, Limb, NonZero, Random, RandomBits, RandomBitsError, RandomMod, Zero};
+use crate::{Limb, NonZero, Random, RandomBits, RandomBitsError, RandomMod};
 use rand_core::{RngCore, TryRngCore};
-use subtle::ConstantTimeLess;
 
 impl<const LIMBS: usize> Random for Uint<LIMBS> {
     fn try_random<R: TryRngCore + ?Sized>(rng: &mut R) -> Result<Self, R::Error> {
@@ -30,7 +29,7 @@ pub(crate) fn random_bits_core<R: TryRngCore + ?Sized>(
     rng: &mut R,
     zeroed_limbs: &mut [Limb],
     bit_length: u32,
-) -> Result<(), RandomBitsError<R::Error>> {
+) -> Result<(), R::Error> {
     if bit_length == 0 {
         return Ok(());
     }
@@ -43,8 +42,7 @@ pub(crate) fn random_bits_core<R: TryRngCore + ?Sized>(
     let mask = Word::MAX >> ((Word::BITS - partial_limb) % Word::BITS);
 
     for i in 0..nonzero_limbs - 1 {
-        rng.try_fill_bytes(&mut buffer)
-            .map_err(RandomBitsError::RandCore)?;
+        rng.try_fill_bytes(&mut buffer)?;
         zeroed_limbs[i] = Limb(Word::from_le_bytes(buffer));
     }
 
@@ -62,8 +60,7 @@ pub(crate) fn random_bits_core<R: TryRngCore + ?Sized>(
         buffer.as_mut_slice()
     };
 
-    rng.try_fill_bytes(slice)
-        .map_err(RandomBitsError::RandCore)?;
+    rng.try_fill_bytes(slice)?;
     zeroed_limbs[nonzero_limbs - 1] = Limb(Word::from_le_bytes(buffer) & mask);
 
     Ok(())
@@ -95,71 +92,38 @@ impl<const LIMBS: usize> RandomBits for Uint<LIMBS> {
             });
         }
         let mut limbs = [Limb::ZERO; LIMBS];
-        random_bits_core(rng, &mut limbs, bit_length)?;
+        random_bits_core(rng, &mut limbs, bit_length).map_err(RandomBitsError::RandCore)?;
         Ok(Self::from(limbs))
     }
 }
 
 impl<const LIMBS: usize> RandomMod for Uint<LIMBS> {
     fn random_mod<R: RngCore + ?Sized>(rng: &mut R, modulus: &NonZero<Self>) -> Self {
-        let mut n = Self::ZERO;
-        let Ok(()) = random_mod_core(rng, &mut n, modulus, modulus.bits_vartime());
-        n
+        let n_bits = modulus.bits_vartime();
+        loop {
+            let n = Self::random_bits(rng, n_bits);
+            if &n < modulus.as_ref() {
+                return n;
+            }
+        }
     }
 
     fn try_random_mod<R: TryRngCore + ?Sized>(
         rng: &mut R,
         modulus: &NonZero<Self>,
     ) -> Result<Self, R::Error> {
-        let mut n = Self::ZERO;
-        random_mod_core(rng, &mut n, modulus, modulus.bits_vartime())?;
-        Ok(n)
-    }
-}
-
-/// Generic implementation of `random_mod` which can be shared with `BoxedUint`.
-// TODO(tarcieri): obtain `n_bits` via a trait like `Integer`
-pub(super) fn random_mod_core<T, R: TryRngCore + ?Sized>(
-    rng: &mut R,
-    n: &mut T,
-    modulus: &NonZero<T>,
-    n_bits: u32,
-) -> Result<(), R::Error>
-where
-    T: AsMut<[Limb]> + AsRef<[Limb]> + ConstantTimeLess + Zero,
-{
-    #[cfg(target_pointer_width = "64")]
-    let mut next_word = || rng.try_next_u64();
-    #[cfg(target_pointer_width = "32")]
-    let mut next_word = || rng.try_next_u32();
-
-    let n_limbs = n_bits.div_ceil(Limb::BITS) as usize;
-
-    let hi_word_modulus = modulus.as_ref().as_ref()[n_limbs - 1].0;
-    let mask = !0 >> hi_word_modulus.leading_zeros();
-    let mut hi_word = next_word()? & mask;
-
-    loop {
-        while hi_word > hi_word_modulus {
-            hi_word = next_word()? & mask;
-        }
-        // Set high limb
-        n.as_mut()[n_limbs - 1] = Limb::from_le_bytes(hi_word.to_le_bytes());
-        // Set low limbs
-        for i in 0..n_limbs - 1 {
-            // Need to deserialize from little-endian to make sure that two 32-bit limbs
-            // deserialized sequentially are equal to one 64-bit limb produced from the same
-            // byte stream.
-            n.as_mut()[i] = Limb::from_le_bytes(next_word()?.to_le_bytes());
+        let n_bits = modulus.bits_vartime();
+        loop {
+            let n = match Self::try_random_bits(rng, n_bits) {
+                Ok(n) => n,
+                Err(RandomBitsError::RandCore(e)) => return Err(e),
+                _ => unreachable!(),
+            };
+            if &n < modulus.as_ref() {
+                return Ok(n);
+            }
         }
-        // If the high limb is equal to the modulus' high limb, it's still possible
-        // that the full uint is too big so we check and repeat if it is.
-        if n.ct_lt(modulus).into() {
-            break;
-        }
-        hi_word = next_word()? & mask;
     }
-    Ok(())
 }
 
 #[cfg(test)]
@@ -288,6 +252,32 @@ mod tests {
         );
     }
 
+    /// Make sure random_mod output is consistent across platforms
+    #[test]
+    fn random_mod_platform_independence() {
+        let mut rng = get_four_sequential_rng();
+
+        let modulus = NonZero::new(U256::from_u32(8192)).unwrap();
+        let mut vals = [U256::ZERO, U256::ZERO, U256::ZERO, U256::ZERO, U256::ZERO];
+        for val in &mut vals {
+            *val = U256::random_mod(&mut rng, &modulus);
+        }
+        let expected = [55, 3378, 2172, 1657, 5323];
+        for (want, got) in expected.into_iter().zip(vals.into_iter()) {
+            assert_eq!(got, U256::from_u32(want));
+        }
+
+        let mut state = [0u8; 16];
+        rng.fill_bytes(&mut state);
+
+        assert_eq!(
+            state,
+            [
+                60, 146, 46, 106, 157, 83, 56, 212, 186, 104, 211, 210, 125, 28, 120, 239
+            ],
+        );
+    }
+
     /// Test that random bytes are sampled consecutively.
     #[test]
     fn random_bits_4_bytes_sequential() {