Merge pull request #383 from pitdicker/impl_default_doc

Improve doc on implementing `Default` and `Clone`

Author: dhardy

rand_core/src/lib.rs

@@ -91,15 +91,16 @@ pub mod le;
 /// It is recommended that implementations also implement:
 /// 
 /// - `Debug` with a custom implementation which *does not* print any internal
-///   state (at least, [`CryptoRng`]s should not risk leaking state through Debug)
+///   state (at least, [`CryptoRng`]s should not risk leaking state through
+///   `Debug`).
 /// - `Serialize` and `Deserialize` (from Serde), preferably making Serde
-///   support optional at the crate level in PRNG libs
-/// - `Clone` if, and only if, the clone will have identical output to the
-///   original (i.e. all deterministic PRNGs but not external generators)
-/// - *never* implement `Copy` (accidental copies may cause repeated values)
-/// - also *do not* implement `Default`, but instead implement `SeedableRng`
-///   thus allowing use of `rand::FromEntropy` (which is automatically implemented)
-/// - `Eq` and `PartialEq` could be implemented, but are probably not useful
+///   support optional at the crate level in PRNG libs.
+/// - `Clone`, if possible.
+/// - *never* implement `Copy` (accidental copies may cause repeated values).
+/// - *do not* implement `Default` for pseudorandom generators, but instead
+///   implement [`SeedableRng`], to guide users towards proper seeding.
+///   External / hardware RNGs can choose to implement `Default`.
+/// - `Eq` and `PartialEq` could be implemented, but are probably not useful.
 /// 
 /// # Example
 /// 

src/jitter.rs

@@ -63,6 +63,22 @@ pub struct JitterRng {
     data_half_used: bool,
 }
 
+// Note: `JitterRng` maintains a small 64-bit entropy pool. With every
+// `generate` 64 new bits should be integrated in the pool. If a round of
+// `generate` were to collect less than the expected 64 bit, then the returned
+// value, and the new state of the entropy pool, would be in some way related to
+// the initial state. It is therefore better if the initial state of the entropy
+// pool is different on each call to `generate`. This has a few implications:
+// - `generate` should be called once before using `JitterRng` to produce the
+//   first usable value (this is done by default in `new`);
+// - We do not zero the entropy pool after generating a result. The reference
+//   implementation also does not support zeroing, but recommends generating a
+//   new value without using it if you want to protect a previously generated
+//   'secret' value from someone inspecting the memory;
+// - Implementing `Clone` seems acceptable, as it would not cause the systematic
+//   bias a constant might cause. Only instead of one value that could be
+//   potentially related to the same initial state, there are now two.
+
 // Entropy collector state.
 // These values are not necessary to preserve across runs.
 struct EcState {
@@ -103,6 +119,20 @@ impl fmt::Debug for JitterRng {
     }
 }
 
+impl Clone for JitterRng {
+    fn clone(&self) -> JitterRng {
+        JitterRng {
+            data: self.data,
+            rounds: self.rounds,
+            timer: self.timer,
+            mem_prev_index: self.mem_prev_index,
+            // The 32 bits that may still be unused from the previous round are
+            // for the original to use, not for the clone.
+            data_half_used: false,
+        }
+    }
+}
+
 /// An error that can occur when [`JitterRng::test_timer`] fails.
 ///
 /// [`JitterRng::test_timer`]: struct.JitterRng.html#method.test_timer

src/os.rs

@@ -29,8 +29,8 @@ use rand_core::{RngCore, Error, impls};
 /// machines, it may block very early in the init process, when the OS CSPRNG
 /// has not yet been seeded.
 ///
-/// `OsRng::new()` is guaranteed to be very cheap (after first call), and will
-/// never consume more than one file handle per process.
+/// `OsRng::new()` is guaranteed to be very cheap (after the first successful
+/// call), and will never consume more than one file handle per process.
 ///
 /// ## Platform sources:
 ///

src/reseeding.rs

@@ -19,6 +19,8 @@ use rand_core::impls::BlockRng;
 /// A wrapper around any PRNG which reseeds the underlying PRNG after it has
 /// generated a certain number of random bytes.
 ///
+/// When the RNG gets cloned, the clone is reseeded on first use.
+///
 /// Reseeding is never strictly *necessary*. Cryptographic PRNGs don't have a
 /// limited number of bytes they can output, or at least not a limit reachable
 /// in any practical way. There is no such thing as 'running out of entropy'.
@@ -103,6 +105,17 @@ where R: BlockRngCore<Item = u32> + SeedableRng,
     }
 }
 
+impl<R, Rsdr> Clone for ReseedingRng<R, Rsdr>
+where R: BlockRngCore + SeedableRng + Clone,
+      Rsdr: RngCore + Clone
+{
+    fn clone(&self) -> ReseedingRng<R, Rsdr> {
+        // Recreating `BlockRng` seems easier than cloning it and resetting
+        // the index.
+        ReseedingRng(BlockRng::new(self.0.inner().clone()))
+    }
+}
+
 impl<R, Rsdr> CryptoRng for ReseedingRng<R, Rsdr>
 where R: BlockRngCore + SeedableRng + CryptoRng,
       Rsdr: RngCore + CryptoRng {}
@@ -189,6 +202,20 @@ where R: BlockRngCore + SeedableRng,
     }
 }
 
+impl<R, Rsdr> Clone for ReseedingCore<R, Rsdr>
+where R: BlockRngCore + SeedableRng + Clone,
+      Rsdr: RngCore + Clone
+{
+    fn clone(&self) -> ReseedingCore<R, Rsdr> {
+        ReseedingCore {
+            inner: self.inner.clone(),
+            reseeder: self.reseeder.clone(),
+            threshold: self.threshold,
+            bytes_until_reseed: 0, // reseed clone on first use
+        }
+    }
+}
+
 impl<R, Rsdr> CryptoRng for ReseedingCore<R, Rsdr>
 where R: BlockRngCore + SeedableRng + CryptoRng,
       Rsdr: RngCore + CryptoRng {}
@@ -217,4 +244,17 @@ mod test {
             assert_eq!(buf, seq);
         }
     }
+
+    #[test]
+    fn test_clone_reseeding() {
+        let mut zero = StepRng::new(0, 0);
+        let rng = ChaChaCore::from_rng(&mut zero).unwrap();
+        let mut rng1 = ReseedingRng::new(rng, 32*4, zero);
+
+        let first: u32 = rng1.gen();
+        for _ in 0..10 { let _ = rng1.gen::<u32>(); }
+
+        let mut rng2 = rng1.clone();
+        assert_eq!(first, rng2.gen::<u32>());
+    }
 }