diff --git a/Cargo.toml b/Cargo.toml index bde9e31..dec20ed 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,7 +35,7 @@ hex = "0.4" rand = "0.10" serde_json = "1" -# Criteron benches +# Criterion benches [[bench]] name = "benches" harness = false diff --git a/README.md b/README.md index d715ece..c51e858 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ Default features flags: _none_ Feature flag list: -* `serialize_secret_state` — Implements `serde`'s `Serialize` and `Deserialize` traits for the `Strobe` struct. **SECURITY NOTE**: Serializing Strobe state outputs security sensitive data that MUST be kept private. Treat the data as you would a private encryption/decryption key. +* `serialize_secret_state` — Implements `serde`'s `Serialize` and `Deserialize` traits for the `Strobe` struct. ⚠️Security warning⚠️: Do NOT use this if you don't know what you're doing. Serializing Strobe state outputs security-sensitive data that MUST be kept private. Treat the data as you would a private encryption/decryption key. * `kat` — Required for running known-answer tests. Use only when testing. For info on how to omit or include feature flags, see the [cargo docs on features](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#choosing-features). @@ -100,7 +100,7 @@ To benchmark, run cargo bench ``` -This will produce a summary with plots in `target/crieteron/report/index.html`. These won't be very interesting, since almost every function in STROBE has the same runtime. +This will produce a summary with plots in `target/criterion/report/index.html`. These won't be very interesting, since almost every function in STROBE has the same runtime. License ------- diff --git a/benches/benches.rs b/benches/benches.rs index a4909ee..aff0d5e 100644 --- a/benches/benches.rs +++ b/benches/benches.rs @@ -10,6 +10,10 @@ fn bench_nonmeta(c: &mut Criterion) { let mut s = Strobe::new(b"simplebench", SecParam::B256); let mut v = [0u8; 256]; + let mut big_v = [0u8; 8192]; + g.bench_function("8KiB send_enc", |b| { + b.iter(|| s.send_enc(&mut big_v, false)) + }); g.bench_function("send_enc", |b| b.iter(|| s.send_enc(&mut v, false))); g.bench_function("recv_enc", |b| b.iter(|| s.recv_enc(&mut v, false))); g.bench_function("send_clr", |b| b.iter(|| s.send_clr(&v, false))); diff --git a/src/basic_tests.rs b/src/basic_tests.rs index 11dce8f..aadd482 100644 --- a/src/basic_tests.rs +++ b/src/basic_tests.rs @@ -20,7 +20,7 @@ s = Strobe("", security=128) print("[{}]".format(', '.join(map("0x{:02x}".format, s.st)))) */ #[test] -fn test_init_128() { +fn init_128() { let s = Strobe::new(b"", SecParam::B128); let initial_st = s.st.0; let expected_st: &[u8; 8 * KECCAK_BLOCK_SIZE] = &[ @@ -52,7 +52,7 @@ s = Strobe("", security=256) print("[{}]".format(', '.join(map("0x{:02x}".format, s.st)))) */ #[test] -fn test_init_256() { +fn init_256() { let s = Strobe::new(b"", SecParam::B256); let initial_st = s.st.0; let expected_st: &[u8; 8 * KECCAK_BLOCK_SIZE] = &[ @@ -98,7 +98,7 @@ print("[{}]".format(', '.join(map("0x{:02x}".format, s.st)))) */ #[cfg(feature = "kat")] #[test] -fn test_seq() { +fn seq() { let mut s = Strobe::new(b"seqtest", SecParam::B256); let mut buf = [0u8; 10]; @@ -165,7 +165,7 @@ print("state == [{}]".format(', '.join(map("0x{:02x}".format, s.st)))) */ #[cfg(feature = "kat")] #[test] -fn test_metadata() { +fn metadata() { // We will accumulate output over 3 operations and 3 meta-operations let mut s = Strobe::new(b"metadatatest", SecParam::B256); let mut output = std::vec::Vec::new(); @@ -256,7 +256,7 @@ s.send_mac(small_n, meta_flags=C|T|M, metadata=small_n) print("[{}]".format(', '.join(map("0x{:02x}".format, s.st)))) */ #[test] -fn test_long_inputs() { +fn long_inputs() { let mut s = Strobe::new(b"bigtest", SecParam::B256); const BIG_N: usize = 9823; const SMALL_N: usize = 65; @@ -312,7 +312,7 @@ fn test_long_inputs() { // Test that streaming in data using the `more` flag works as expected #[cfg(feature = "kat")] #[test] -fn test_streaming_correctness() { +fn streaming_correctness() { // Compute a few things without breaking up their inputs let one_shot_st: std::vec::Vec = { let mut s = Strobe::new(b"streamingtest", SecParam::B256); @@ -359,7 +359,7 @@ fn test_streaming_correctness() { // after the same op. In this instance, the violating operation is a nonmutating one (it's AD) #[test] #[should_panic] -fn test_streaming_soundness_nomutate() { +fn streaming_soundness_nomutate() { let mut s = Strobe::new(b"mactest", SecParam::B256); // Key with valid steps @@ -373,7 +373,7 @@ fn test_streaming_soundness_nomutate() { // Same as above, but whose violating operation is a mutating one (it's send_enc) #[test] #[should_panic] -fn test_streaming_soundness_mutate() { +fn streaming_soundness_mutate() { let mut s = Strobe::new(b"mactest", SecParam::B256); // Key with valid steps @@ -388,7 +388,7 @@ fn test_streaming_soundness_mutate() { // Same as above but with ratchet #[test] #[should_panic] -fn test_streaming_soundness_ratchet() { +fn streaming_soundness_ratchet() { let mut s = Strobe::new(b"mactest", SecParam::B256); // Key with valid steps @@ -401,7 +401,7 @@ fn test_streaming_soundness_ratchet() { // Test that decrypt(encrypt(msg)) == msg #[test] -fn test_enc_correctness() { +fn enc_correctness() { let orig_msg = b"Hello there"; let mut tx = Strobe::new(b"enccorrectnesstest", SecParam::B256); let mut rx = Strobe::new(b"enccorrectnesstest", SecParam::B256); @@ -419,7 +419,7 @@ fn test_enc_correctness() { // Test that recv_mac(send_mac()) doesn't error, and recv_mac(otherstuff) does error #[test] -fn test_mac_correctness_and_soundness() { +fn mac_correctness_and_soundness() { let mut tx = Strobe::new(b"mactest", SecParam::B256); let mut rx = Strobe::new(b"mactest", SecParam::B256); @@ -446,3 +446,39 @@ fn test_mac_correctness_and_soundness() { let bad_res = rx.recv_mac(&bad_mac.try_into().unwrap()); assert!(bad_res.is_err()); } + +// Regression test: `prf` and `send_mac` must overwrite the caller's buffer, not XOR into it. +// A previous commit incorrectly XORed the PRF buffer into the state, and it was barely caught by +// tests. This test explicitly checks that the prior value of the PRF buffer does not matter. +#[test] +fn output_independent_of_input_buffer() { + // Build up some nontrivial state to extract from + let mut s = Strobe::new(b"output-overwrite-regression", SecParam::B256); + s.key(b"secretsauce", false); + s.ad(b"some associated data", false); + + // Clone the state twice and have it output the PRF into two buffers. One filled with zeroes + // and one filled with 0xAA. They should be identical after being filled. + { + let mut from_zeros = [0x00u8; 64]; + let mut from_dirty = [0xAAu8; 64]; + s.clone().prf(&mut from_zeros, false); + s.clone().prf(&mut from_dirty, false); + assert_eq!( + from_zeros, from_dirty, + "PRF cannot depend on the initial contents of the output buffer" + ); + } + + // Do the same for send_mac + { + let mut from_zeros = [0x00u8; 32]; + let mut from_dirty = [0xAAu8; 32]; + s.clone().send_mac(&mut from_zeros, false); + s.clone().send_mac(&mut from_dirty, false); + assert_eq!( + from_zeros, from_dirty, + "send-MAC cannot depend on the initial contents of the output buffer" + ); + } +} diff --git a/src/keccak.rs b/src/keccak.rs index 6be654d..d135b1b 100644 --- a/src/keccak.rs +++ b/src/keccak.rs @@ -18,7 +18,6 @@ use serde_big_array::BigArray; /// safely convertible to a pointer to [u64; 25] (since u64 words must be 8-byte aligned) #[derive(Clone, Zeroize)] #[cfg_attr(feature = "serialize_secret_state", derive(Serialize, Deserialize))] -#[repr(align(8))] pub(crate) struct AlignedKeccakState( #[cfg_attr(feature = "serialize_secret_state", serde(with = "BigArray"))] pub(crate) [u8; 8 * KECCAK_BLOCK_SIZE], @@ -26,13 +25,14 @@ pub(crate) struct AlignedKeccakState( /// Performs the keccakf\[1600\] permutation on a byte buffer // Make a little-endian copy, do the operation, then copy the bytes back. Hopefully the compiler -// will optimize out the copy if we' re on a little endian machine. I don't feel comfortable doing +// will optimize out the copy if we're on a little endian machine. I don't feel comfortable doing // a mem transmute. pub(crate) fn keccakf_u8(st: &mut AlignedKeccakState) { let mut keccak_block = [0u64; KECCAK_BLOCK_SIZE]; LittleEndian::read_u64_into(&st.0, &mut keccak_block); Keccak::new().with_f1600(|f| f(&mut keccak_block)); LittleEndian::write_u64_into(&keccak_block, &mut st.0); + keccak_block.zeroize(); } /* diff --git a/src/strobe.rs b/src/strobe.rs index 1b65e6f..1d8165b 100644 --- a/src/strobe.rs +++ b/src/strobe.rs @@ -108,6 +108,15 @@ impl core::fmt::Display for AuthError { /// /// Finally, `ratchet` and `meta_ratchet` take a `usize` argument instead of bytes. These functions /// are individually commented below. +#[cfg_attr( + feature = "serialize_secret_state", + doc = "\n\n\ + ⚠️Security warning⚠️ \ + When the `serialize_secret_state` feature is enabled, `Strobe` implements \ + `serde::Serialize`/`serde::Deserialize`. Serializing Strobe state outputs \ + security-sensitive data that MUST be kept private. Treat the data as you would a private \ + encryption/decryption key." +)] #[derive(Clone, Zeroize, ZeroizeOnDrop)] #[cfg_attr(feature = "serialize_secret_state", derive(Serialize, Deserialize))] pub struct Strobe { @@ -243,109 +252,115 @@ impl Strobe { self.pos_begin = 0; } - /// XORs the given data into the state. This is a special case of the `duplex` code in the - /// STROBE paper. - fn absorb(&mut self, data: &[u8]) { - for b in data { - self.st.0[self.pos] ^= *b; + /// Runs the duplex loop over `data`, applying `f` to each `(state_byte, data_byte)` pair and + /// running the permutation each time the rate boundary is reached. For simplicity's sake, + /// rather than implementing the entire `duplex` function from the paper, we implement this for + /// generic `f` and let the caller pick `f`. + fn duplex_mut(&mut self, data: &mut [u8], mut f: impl FnMut(&mut u8, &mut u8)) { + let mut data_idx = 0; + while data_idx < data.len() { + // Pick out two equal-sized slices from state and chunk. We will zip them and run `f` + let chunk_size = core::cmp::min(self.rate - self.pos, data.len() - data_idx); + let state_chunk = &mut self.st.0[self.pos..self.pos + chunk_size]; + let data_chunk = &mut data[data_idx..data_idx + chunk_size]; + + for (s, d) in state_chunk.iter_mut().zip(data_chunk.iter_mut()) { + f(s, d); + } + + // Update the data cursor and self cursor + self.pos += chunk_size; + data_idx += chunk_size; - self.pos += 1; + // If we XORed enough to exhaust the rate, then permute if self.pos == self.rate { self.run_f(); } } } - /// XORs the given data into the state, then sets the data equal the state. This is a special - /// case of the `duplex` code in the STROBE paper. - fn absorb_and_set(&mut self, data: &mut [u8]) { - for b in data { - let state_byte = self.st.0.get_mut(self.pos).unwrap(); - *state_byte ^= *b; - *b = *state_byte; + /// Identical as [`Strobe::duplex_mut`], but where `data` is read-only + fn duplex_const(&mut self, data: &[u8], mut f: impl FnMut(&mut u8, u8)) { + let mut data_idx = 0; + while data_idx < data.len() { + // Pick out two equal-sized slices from state and chunk. We will zip them and run `f` + let chunk_size = core::cmp::min(self.rate - self.pos, data.len() - data_idx); + let state_chunk = &mut self.st.0[self.pos..self.pos + chunk_size]; + let data_chunk = &data[data_idx..data_idx + chunk_size]; + + for (s, &d) in state_chunk.iter_mut().zip(data_chunk.iter()) { + f(s, d); + } - self.pos += 1; + // Update the data cursor and self cursor + self.pos += chunk_size; + data_idx += chunk_size; + + // If we XORed enough to exhaust the rate, then permute if self.pos == self.rate { self.run_f(); } } } + /// XORs the given data into the state. This is a special case of the `duplex` code in the + /// STROBE paper. + fn absorb(&mut self, data: &[u8]) { + self.duplex_const(data, |s, d| *s ^= d); + } + + /// XORs the given data into the state, then sets the data equal the state. This is a special + /// case of the `duplex` code in the STROBE paper. + fn absorb_and_set(&mut self, data: &mut [u8]) { + self.duplex_mut(data, |s, d| { + *s ^= *d; + *d = *s; + }); + } + /// Copies the internal state into the given buffer. This is a special case of `absorb_and_set` /// where `data` is all zeros. fn copy_state(&mut self, data: &mut [u8]) { - for b in data { - *b = self.st.0[self.pos]; - - self.pos += 1; - if self.pos == self.rate { - self.run_f(); - } - } + self.duplex_mut(data, |s, d| *d = *s); } /// Overwrites the state with the given data while XORing the given data with the old state. /// This is a special case of the `duplex` code in the STROBE paper. fn exchange(&mut self, data: &mut [u8]) { - for b in data { - let state_byte = self.st.0.get_mut(self.pos).unwrap(); - *b ^= *state_byte; - *state_byte ^= *b; - - self.pos += 1; - if self.pos == self.rate { - self.run_f(); - } - } + self.duplex_mut(data, |s, d| { + *d ^= *s; + *s ^= *d; + }); } /// Overwrites the state with the given data. This is a special case of `Strobe::exchange`, /// where we do not want to mutate the input data. fn overwrite(&mut self, data: &[u8]) { - for b in data { - self.st.0[self.pos] = *b; - - self.pos += 1; - if self.pos == self.rate { - self.run_f(); - } - } + self.duplex_const(data, |s, d| *s = d); } /// Copies the state into the given buffer and sets the state to 0. This is a special case of /// `Strobe::exchange`, where `data` is assumed to be the all-zeros string. This is precisely /// the case when the current operation is PRF. fn squeeze(&mut self, data: &mut [u8]) { - for b in data { - let state_byte = self.st.0.get_mut(self.pos).unwrap(); - *b = *state_byte; - *state_byte = 0; - - self.pos += 1; - if self.pos == self.rate { - self.run_f(); - } - } + self.duplex_mut(data, |s, d| { + *d = *s; + *s = 0; + }); } - /// Overwrites the state with a specified number of zeros. This is a special case of - /// `Strobe::exchange`. More specifically, it's a special case of `Strobe::overwrite` and - /// `Strobe::squeeze`. It's like `squeeze` in that we assume we've been given all zeros as - /// input, and like `overwrite` in that we do not mutate (or take) any input. + /// Overwrites the state with a specified number of zeros fn zero_state(&mut self, mut bytes_to_zero: usize) { static ZEROS: [u8; 8 * KECCAK_BLOCK_SIZE] = [0u8; 8 * KECCAK_BLOCK_SIZE]; - // Do the zero-writing in chunks + // Repeatedly `overwrite` a chunk of zeros into the state until we've written the desired + // number of zeros while bytes_to_zero > 0 { - let slice_len = core::cmp::min(self.rate - self.pos, bytes_to_zero); - self.st.0[self.pos..(self.pos + slice_len)].copy_from_slice(&ZEROS[..slice_len]); - - self.pos += slice_len; - bytes_to_zero -= slice_len; + let chunk_size = core::cmp::min(bytes_to_zero, 8 * KECCAK_BLOCK_SIZE); + let chunk = &ZEROS[..chunk_size]; + self.overwrite(chunk); - if self.pos == self.rate { - self.run_f(); - } + bytes_to_zero -= chunk_size; } }