From 764ed69a8cbec783650acedb258056aaeb57bdeb Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Wed, 10 Sep 2025 18:27:07 -0600 Subject: [PATCH] Remove `Clone` impls on ciphers/RNGs Allow cloning on a stream cipher or RNG is problematic because it duplicates internal states, which can lead to keystream reuse / RNG output duplication, which in cryptographic contexts can be catastrophic. Instead, for things like tests ciphers can be initialized from the same seed repeatedly, which is what this PR changes the e.g. `chacha20` tests to do. This is a much more explicit way of deliberately duplicating stream ciphers/RNGs for the purposes of testing. See also: - #220 - #461 - RustCrypto/block-modes/pull/91 - rust-random/rand#1101 --- chacha20/src/lib.rs | 11 +---------- chacha20/src/rng.rs | 29 ++++++++++++++--------------- rabbit/src/lib.rs | 3 --- rc4/src/lib.rs | 1 - 4 files changed, 15 insertions(+), 29 deletions(-) diff --git a/chacha20/src/lib.rs b/chacha20/src/lib.rs index 075d604b..d9846f71 100644 --- a/chacha20/src/lib.rs +++ b/chacha20/src/lib.rs @@ -209,6 +209,7 @@ cfg_if! { /// The ChaCha core function. pub struct ChaChaCore { /// Internal state of the core function + #[cfg(any(feature = "cipher", feature = "rng"))] state: [u32; STATE_WORDS], /// CPU target feature tokens #[allow(dead_code)] @@ -217,16 +218,6 @@ pub struct ChaChaCore { _pd: PhantomData<(R, V)>, } -impl Clone for ChaChaCore { - fn clone(&self) -> Self { - Self { - state: self.state, - tokens: self.tokens, - _pd: PhantomData, - } - } -} - impl ChaChaCore { /// Constructs a ChaChaCore with the specified key, iv, and amount of rounds. /// You must ensure that the iv is of the correct size when using this method diff --git a/chacha20/src/rng.rs b/chacha20/src/rng.rs index 63962bc2..d9f540dd 100644 --- a/chacha20/src/rng.rs +++ b/chacha20/src/rng.rs @@ -300,14 +300,12 @@ macro_rules! impl_chacha_rng { /// /// [^2]: [eSTREAM: the ECRYPT Stream Cipher Project]( /// http://www.ecrypt.eu.org/stream/) - #[derive(Clone)] pub struct $ChaChaXRng { /// The ChaChaCore struct pub core: BlockRng<$ChaChaXCore>, } /// The ChaCha core random number generator - #[derive(Clone)] pub struct $ChaChaXCore(ChaChaCore<$rounds, Legacy>); impl SeedableRng for $ChaChaXCore { @@ -951,26 +949,26 @@ pub(crate) mod tests { 0, 0, 0, 0, 1, 0, 0, 0, 2, 0, 0, 0, 3, 0, 0, 0, 4, 0, 0, 0, 5, 0, 0, 0, 6, 0, 0, 0, 7, 0, 0, 0, ]; - let mut rng = ChaChaRng::from_seed(seed); - let mut clone = rng.clone(); + let mut rng1 = ChaChaRng::from_seed(seed); + let mut rng2 = ChaChaRng::from_seed(seed); for _ in 0..16 { - assert_eq!(rng.next_u64(), clone.next_u64()); + assert_eq!(rng1.next_u64(), rng2.next_u64()); } - rng.set_stream(51); - assert_eq!(rng.get_stream(), 51); - assert_eq!(clone.get_stream(), 0); + rng1.set_stream(51); + assert_eq!(rng1.get_stream(), 51); + assert_eq!(rng2.get_stream(), 0); let mut fill_1 = [0u8; 7]; - rng.fill_bytes(&mut fill_1); + rng1.fill_bytes(&mut fill_1); let mut fill_2 = [0u8; 7]; - clone.fill_bytes(&mut fill_2); + rng2.fill_bytes(&mut fill_2); assert_ne!(fill_1, fill_2); for _ in 0..7 { - assert!(rng.next_u64() != clone.next_u64()); + assert!(rng1.next_u64() != rng2.next_u64()); } - clone.set_stream(51); // switch part way through block + rng2.set_stream(51); // switch part way through block for _ in 7..16 { - assert_eq!(rng.next_u64(), clone.next_u64()); + assert_eq!(rng1.next_u64(), rng2.next_u64()); } } @@ -1110,8 +1108,9 @@ pub(crate) mod tests { fn test_trait_objects() { use rand_core::CryptoRng; - let mut rng1 = ChaChaRng::from_seed(Default::default()); - let rng2 = &mut rng1.clone() as &mut dyn CryptoRng; + let seed = Default::default(); + let mut rng1 = ChaChaRng::from_seed(seed); + let rng2 = &mut ChaChaRng::from_seed(seed) as &mut dyn CryptoRng; for _ in 0..1000 { assert_eq!(rng1.next_u64(), rng2.next_u64()); } diff --git a/rabbit/src/lib.rs b/rabbit/src/lib.rs index a7d0461a..46e9f9b7 100644 --- a/rabbit/src/lib.rs +++ b/rabbit/src/lib.rs @@ -96,7 +96,6 @@ pub type RabbitKeyOnly = StreamCipherCoreWrapper; pub type Rabbit = StreamCipherCoreWrapper; /// RFC 4503. 2.2. Inner State (page 2). -#[derive(Clone)] struct State { /// State variables x: [u32; 8], @@ -262,7 +261,6 @@ impl core::ops::Drop for State { } /// Core state of the Rabbit stream cipher initialized only with key. -#[derive(Clone)] pub struct RabbitKeyOnlyCore { state: State, } @@ -301,7 +299,6 @@ impl StreamCipherCore for RabbitKeyOnlyCore { impl ZeroizeOnDrop for RabbitKeyOnlyCore {} /// Core state of the Rabbit stream cipher initialized with key and IV. -#[derive(Clone)] pub struct RabbitCore { state: State, } diff --git a/rc4/src/lib.rs b/rc4/src/lib.rs index e31e10e8..077abb93 100644 --- a/rc4/src/lib.rs +++ b/rc4/src/lib.rs @@ -119,7 +119,6 @@ impl StreamCipherBackend for Backend<'_> { } } -#[derive(Clone)] struct Rc4State { state: [u8; 256], i: u8,