Skip to content

Commit 9f095c4

Browse files
authored
Remove Clone impls on ciphers/RNGs (#462)
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
1 parent f0bb120 commit 9f095c4

File tree

4 files changed

+15
-29
lines changed

4 files changed

+15
-29
lines changed

chacha20/src/lib.rs

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ cfg_if! {
209209
/// The ChaCha core function.
210210
pub struct ChaChaCore<R: Rounds, V: Variant> {
211211
/// Internal state of the core function
212+
#[cfg(any(feature = "cipher", feature = "rng"))]
212213
state: [u32; STATE_WORDS],
213214
/// CPU target feature tokens
214215
#[allow(dead_code)]
@@ -217,16 +218,6 @@ pub struct ChaChaCore<R: Rounds, V: Variant> {
217218
_pd: PhantomData<(R, V)>,
218219
}
219220

220-
impl<R: Rounds, V: Variant> Clone for ChaChaCore<R, V> {
221-
fn clone(&self) -> Self {
222-
Self {
223-
state: self.state,
224-
tokens: self.tokens,
225-
_pd: PhantomData,
226-
}
227-
}
228-
}
229-
230221
impl<R: Rounds, V: Variant> ChaChaCore<R, V> {
231222
/// Constructs a ChaChaCore with the specified key, iv, and amount of rounds.
232223
/// You must ensure that the iv is of the correct size when using this method

chacha20/src/rng.rs

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -300,14 +300,12 @@ macro_rules! impl_chacha_rng {
300300
///
301301
/// [^2]: [eSTREAM: the ECRYPT Stream Cipher Project](
302302
/// http://www.ecrypt.eu.org/stream/)
303-
#[derive(Clone)]
304303
pub struct $ChaChaXRng {
305304
/// The ChaChaCore struct
306305
pub core: BlockRng<$ChaChaXCore>,
307306
}
308307

309308
/// The ChaCha core random number generator
310-
#[derive(Clone)]
311309
pub struct $ChaChaXCore(ChaChaCore<$rounds, Legacy>);
312310

313311
impl SeedableRng for $ChaChaXCore {
@@ -951,26 +949,26 @@ pub(crate) mod tests {
951949
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,
952950
0, 0, 0,
953951
];
954-
let mut rng = ChaChaRng::from_seed(seed);
955-
let mut clone = rng.clone();
952+
let mut rng1 = ChaChaRng::from_seed(seed);
953+
let mut rng2 = ChaChaRng::from_seed(seed);
956954
for _ in 0..16 {
957-
assert_eq!(rng.next_u64(), clone.next_u64());
955+
assert_eq!(rng1.next_u64(), rng2.next_u64());
958956
}
959957

960-
rng.set_stream(51);
961-
assert_eq!(rng.get_stream(), 51);
962-
assert_eq!(clone.get_stream(), 0);
958+
rng1.set_stream(51);
959+
assert_eq!(rng1.get_stream(), 51);
960+
assert_eq!(rng2.get_stream(), 0);
963961
let mut fill_1 = [0u8; 7];
964-
rng.fill_bytes(&mut fill_1);
962+
rng1.fill_bytes(&mut fill_1);
965963
let mut fill_2 = [0u8; 7];
966-
clone.fill_bytes(&mut fill_2);
964+
rng2.fill_bytes(&mut fill_2);
967965
assert_ne!(fill_1, fill_2);
968966
for _ in 0..7 {
969-
assert!(rng.next_u64() != clone.next_u64());
967+
assert!(rng1.next_u64() != rng2.next_u64());
970968
}
971-
clone.set_stream(51); // switch part way through block
969+
rng2.set_stream(51); // switch part way through block
972970
for _ in 7..16 {
973-
assert_eq!(rng.next_u64(), clone.next_u64());
971+
assert_eq!(rng1.next_u64(), rng2.next_u64());
974972
}
975973
}
976974

@@ -1110,8 +1108,9 @@ pub(crate) mod tests {
11101108
fn test_trait_objects() {
11111109
use rand_core::CryptoRng;
11121110

1113-
let mut rng1 = ChaChaRng::from_seed(Default::default());
1114-
let rng2 = &mut rng1.clone() as &mut dyn CryptoRng;
1111+
let seed = Default::default();
1112+
let mut rng1 = ChaChaRng::from_seed(seed);
1113+
let rng2 = &mut ChaChaRng::from_seed(seed) as &mut dyn CryptoRng;
11151114
for _ in 0..1000 {
11161115
assert_eq!(rng1.next_u64(), rng2.next_u64());
11171116
}

rabbit/src/lib.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ pub type RabbitKeyOnly = StreamCipherCoreWrapper<RabbitKeyOnlyCore>;
9696
pub type Rabbit = StreamCipherCoreWrapper<RabbitCore>;
9797

9898
/// RFC 4503. 2.2. Inner State (page 2).
99-
#[derive(Clone)]
10099
struct State {
101100
/// State variables
102101
x: [u32; 8],
@@ -262,7 +261,6 @@ impl core::ops::Drop for State {
262261
}
263262

264263
/// Core state of the Rabbit stream cipher initialized only with key.
265-
#[derive(Clone)]
266264
pub struct RabbitKeyOnlyCore {
267265
state: State,
268266
}
@@ -301,7 +299,6 @@ impl StreamCipherCore for RabbitKeyOnlyCore {
301299
impl ZeroizeOnDrop for RabbitKeyOnlyCore {}
302300

303301
/// Core state of the Rabbit stream cipher initialized with key and IV.
304-
#[derive(Clone)]
305302
pub struct RabbitCore {
306303
state: State,
307304
}

rc4/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ impl StreamCipherBackend for Backend<'_> {
119119
}
120120
}
121121

122-
#[derive(Clone)]
123122
struct Rc4State {
124123
state: [u8; 256],
125124
i: u8,

0 commit comments

Comments
 (0)