Make random_bits_core platform independent (#781)

In its current state, `random_bits_core` behaves differently on 32-bit
and 64-bit targets in two ways:

1. The platforms order the randomly sampled bytes in a different order,
generating different outputs.
2. For bit_length $\mod 64 \in [1, 32]$, the 64-bit platform samples
four random bytes more than the 32-bit platform.
a. this means the state of the RNG after calling this function is
platform-dependent. This makes it impossible to use this function as an
rng-seed-to-integer map.

This patch introduces:
- benchmarks to gauge the performance of the `random_bits_core`
function,
- a patch to the `random_bits_core` function, and
- a test ensuring the `random_bits_core` output and rng state after
calling it are platform-independent.

Author: erik-3milabs

benches/uint.rs

@@ -121,6 +121,27 @@ fn bench_random(c: &mut Criterion) {
     });
 }
 
+fn bench_random_bits(c: &mut Criterion) {
+    let mut group = c.benchmark_group("random_bits");
+
+    let mut rng = make_rng();
+    group.bench_function("random_bits, U256, full", |b| {
+        b.iter(|| black_box(U256::random_bits(&mut rng, U256::BITS)));
+    });
+
+    group.bench_function("random_bits, U256, bounded", |b| {
+        b.iter(|| black_box(U256::random_bits(&mut rng, 219)));
+    });
+
+    group.bench_function("random_bits, U2048, full", |b| {
+        b.iter(|| black_box(U2048::random_bits(&mut rng, U2048::BITS)));
+    });
+
+    group.bench_function("random_bits, U2048, bounded", |b| {
+        b.iter(|| black_box(U2048::random_bits(&mut rng, 1947)));
+    });
+}
+
 fn bench_mul(c: &mut Criterion) {
     let mut group = c.benchmark_group("wrapping ops");
 
@@ -493,6 +514,7 @@ fn bench_sqrt(c: &mut Criterion) {
 criterion_group!(
     benches,
     bench_random,
+    bench_random_bits,
     bench_mul,
     bench_division,
     bench_gcd,

src/uint/rand.rs

@@ -20,6 +20,12 @@ impl<const LIMBS: usize> Random for Uint<LIMBS> {
 /// Fill the given limbs slice with random bits.
 ///
 /// NOTE: Assumes that the limbs in the given slice are zeroed!
+///
+/// When combined with a platform-independent "4-byte sequential" `rng`, this function is
+/// platform-independent. We consider an RNG "`X`-byte sequential" whenever
+/// `rng.fill_bytes(&mut bytes[..i]); rng.fill_bytes(&mut bytes[i..])` constructs the same `bytes`,
+/// as long as `i` is a multiple of `X`.
+/// Note that the `TryRngCore` trait does _not_ require this behaviour from `rng`.
 pub(crate) fn random_bits_core<R: TryRngCore + ?Sized>(
     rng: &mut R,
     zeroed_limbs: &mut [Limb],
@@ -39,12 +45,26 @@ pub(crate) fn random_bits_core<R: TryRngCore + ?Sized>(
     for i in 0..nonzero_limbs - 1 {
         rng.try_fill_bytes(&mut buffer)
             .map_err(RandomBitsError::RandCore)?;
-        zeroed_limbs[i] = Limb(Word::from_be_bytes(buffer));
+        zeroed_limbs[i] = Limb(Word::from_le_bytes(buffer));
     }
 
-    rng.try_fill_bytes(&mut buffer)
+    // This algorithm should sample the same number of random bytes, regardless of the pointer width
+    // of the target platform. To this end, special attention has to be paid to the case where
+    // bit_length - 1 < 32 mod 64. Bit strings of that size can be represented using `2X+1` 32-bit
+    // words or `X+1` 64-bit words. Note that 64*(X+1) - 32*(2X+1) = 32. Hence, if we sample full
+    // words only, a 64-bit platform will sample 32 bits more than a 32-bit platform. We prevent
+    // this by forcing both platforms to only sample 4 bytes for the last word in this case.
+    let slice = if partial_limb > 0 && partial_limb <= 32 {
+        // Note: we do not have to zeroize the second half of the buffer, as the mask will take
+        // care of this in the end.
+        &mut buffer[0..4]
+    } else {
+        buffer.as_mut_slice()
+    };
+
+    rng.try_fill_bytes(slice)
         .map_err(RandomBitsError::RandCore)?;
-    zeroed_limbs[nonzero_limbs - 1] = Limb(Word::from_be_bytes(buffer) & mask);
+    zeroed_limbs[nonzero_limbs - 1] = Limb(Word::from_le_bytes(buffer) & mask);
 
     Ok(())
 }
@@ -144,9 +164,31 @@ where
 
 #[cfg(test)]
 mod tests {
-    use crate::{Limb, NonZero, RandomBits, RandomMod, U256};
+    use crate::uint::rand::random_bits_core;
+    use crate::{Limb, NonZero, Random, RandomBits, RandomMod, U256, U1024, Uint};
     use rand_chacha::ChaCha8Rng;
-    use rand_core::SeedableRng;
+    use rand_core::{RngCore, SeedableRng};
+
+    const RANDOM_OUTPUT: U1024 = Uint::from_be_hex(concat![
+        "A484C4C693EECC47C3B919AE0D16DF2259CD1A8A9B8EA8E0862878227D4B40A3",
+        "C54302F2EB1E2F69E17653A37F1BCC44277FA208E6B31E08CDC4A23A7E88E660",
+        "EF781C7DD2D368BAD438539D6A2E923C8CAE14CB947EB0BDE10D666732024679",
+        "0F6760A48F9B887CB2FB0D3281E2A6E67746A55FBAD8C037B585F767A79A3B6C"
+    ]);
+
+    /// Construct a 4-sequential `rng`, i.e., an `rng` such that
+    /// `rng.fill_bytes(&mut buffer[..x]); rng.fill_bytes(&mut buffer[x..])` will construct the
+    /// same `buffer`, for `x` any in `0..buffer.len()` that is `0 mod 4`.
+    fn get_four_sequential_rng() -> ChaCha8Rng {
+        ChaCha8Rng::seed_from_u64(0)
+    }
+
+    /// Make sure the random value constructed is consistent across platforms
+    #[test]
+    fn random_platform_independence() {
+        let mut rng = get_four_sequential_rng();
+        assert_eq!(U1024::random(&mut rng), RANDOM_OUTPUT);
+    }
 
     #[test]
     fn random_mod() {
@@ -219,4 +261,47 @@ mod tests {
             assert_eq!(res, U256::ZERO);
         }
     }
+
+    /// Make sure the random_bits output is consistent across platforms
+    #[test]
+    fn random_bits_platform_independence() {
+        let mut rng = get_four_sequential_rng();
+
+        let bit_length = 989;
+        let mut val = U1024::ZERO;
+        random_bits_core(&mut rng, val.as_limbs_mut(), bit_length).expect("safe");
+
+        assert_eq!(
+            val,
+            RANDOM_OUTPUT.bitand(&U1024::ONE.shl(bit_length).wrapping_sub(&Uint::ONE))
+        );
+
+        // Test that the RNG is in the same state
+        let mut state = [0u8; 16];
+        rng.fill_bytes(&mut state);
+
+        assert_eq!(
+            state,
+            [
+                198, 196, 132, 164, 240, 211, 223, 12, 36, 189, 139, 48, 94, 1, 123, 253
+            ]
+        );
+    }
+
+    /// Test that random bytes are sampled consecutively.
+    #[test]
+    fn random_bits_4_bytes_sequential() {
+        // Test for multiples of 4 bytes, i.e., multiples of 32 bits.
+        let bit_lengths = [0, 32, 64, 128, 192, 992];
+
+        for bit_length in bit_lengths {
+            let mut rng = get_four_sequential_rng();
+            let mut first = U1024::ZERO;
+            let mut second = U1024::ZERO;
+            random_bits_core(&mut rng, first.as_limbs_mut(), bit_length).expect("safe");
+            random_bits_core(&mut rng, second.as_limbs_mut(), U1024::BITS - bit_length)
+                .expect("safe");
+            assert_eq!(second.shl(bit_length).bitor(&first), RANDOM_OUTPUT);
+        }
+    }
 }