From 6c8e75618dabdd6b0464cf6206d3b18461b8418d Mon Sep 17 00:00:00 2001 From: ComplexSpaces Date: Thu, 9 Mar 2023 10:22:11 -0600 Subject: [PATCH] Fix nonce incrementing in stateful transport to match the specification --- src/cipherstate.rs | 27 +++++++++++++++++-- src/types.rs | 3 +-- tests/general.rs | 67 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 4 deletions(-) diff --git a/src/cipherstate.rs b/src/cipherstate.rs index 6ef741a..1b8c179 100644 --- a/src/cipherstate.rs +++ b/src/cipherstate.rs @@ -35,8 +35,11 @@ impl CipherState { return Err(StateProblem::MissingKeyMaterial.into()); } + validate_nonce(self.n)?; let len = self.cipher.encrypt(self.n, authtext, plaintext, out); - self.n = self.n.checked_add(1).ok_or(StateProblem::Exhausted)?; + + // We have validated this will not wrap around. + self.n += 1; Ok(len) } @@ -55,8 +58,11 @@ impl CipherState { return Err(StateProblem::MissingKeyMaterial.into()); } + validate_nonce(self.n)?; let len = self.cipher.decrypt(self.n, authtext, ciphertext, out); - self.n = self.n.checked_add(1).ok_or(StateProblem::Exhausted)?; + + // We have validated this will not wrap around. + self.n += 1; len } @@ -131,6 +137,8 @@ impl StatelessCipherState { return Err(StateProblem::MissingKeyMaterial.into()); } + validate_nonce(nonce)?; + Ok(self.cipher.encrypt(nonce, authtext, plaintext, out)) } @@ -149,6 +157,8 @@ impl StatelessCipherState { return Err(StateProblem::MissingKeyMaterial.into()); } + validate_nonce(nonce)?; + self.cipher.decrypt(nonce, authtext, ciphertext, out) } @@ -169,6 +179,19 @@ impl StatelessCipherState { } } +/// Validates that a nonce value has not exceeded the maximum +/// defined by the Noise spec. +fn validate_nonce(current: u64) -> Result<(), Error> { + // 2^64-1 is reserved and may not be used in the state machine (5.1). + // + // It is used by the default cipher rekey function (4.2). + if current == u64::MAX { + Err(Error::State(StateProblem::Exhausted)) + } else { + Ok(()) + } +} + impl From for StatelessCipherState { fn from(other: CipherState) -> Self { Self { cipher: other.cipher, has_key: other.has_key } diff --git a/src/types.rs b/src/types.rs index 3aa3bfb..73da21a 100644 --- a/src/types.rs +++ b/src/types.rs @@ -60,8 +60,7 @@ pub trait Cipher: Send + Sync { /// implementation guaranteed to be secure for all ciphers. fn rekey(&mut self) { let mut ciphertext = [0; CIPHERKEYLEN + TAGLEN]; - let ciphertext_len = - self.encrypt(u64::max_value(), &[], &[0; CIPHERKEYLEN], &mut ciphertext); + let ciphertext_len = self.encrypt(u64::MAX, &[], &[0; CIPHERKEYLEN], &mut ciphertext); assert_eq!(ciphertext_len, ciphertext.len()); self.set(&ciphertext[..CIPHERKEYLEN]); } diff --git a/tests/general.rs b/tests/general.rs index a919135..d95c1fe 100644 --- a/tests/general.rs +++ b/tests/general.rs @@ -803,3 +803,70 @@ fn test_handshake_read_oob_error() { // This shouldn't panic, but it *should* return an error. let _ = h_i.read_message(&buffer_msg[..len], &mut buffer_out); } + +#[test] +fn test_stateful_nonce_maxiumum_behavior() { + let params: NoiseParams = "Noise_NN_25519_ChaChaPoly_SHA256".parse().unwrap(); + let mut h_i = Builder::new(params.clone()).build_initiator().unwrap(); + let mut h_r = Builder::new(params).build_responder().unwrap(); + + let mut buffer_msg = [0u8; 200]; + let mut buffer_out = [0u8; 200]; + let len = h_i.write_message(b"abc", &mut buffer_msg).unwrap(); + h_r.read_message(&buffer_msg[..len], &mut buffer_out).unwrap(); + + let len = h_r.write_message(b"defg", &mut buffer_msg).unwrap(); + h_i.read_message(&buffer_msg[..len], &mut buffer_out).unwrap(); + + let h_i = h_i.into_stateless_transport_mode().unwrap(); + let mut h_r = h_r.into_transport_mode().unwrap(); + + let mut sender_nonce = u64::MAX - 2; + let len = h_i.write_message(sender_nonce, b"xyz", &mut buffer_msg).unwrap(); + + h_r.set_receiving_nonce(sender_nonce); + h_r.read_message(&buffer_msg[..len], &mut buffer_out).unwrap(); + + // Simulate exhausting the nonce space for the stateful transport. + sender_nonce += 1; + let len = h_i.write_message(sender_nonce, b"abc", &mut buffer_msg).unwrap(); + + h_r.set_receiving_nonce(sender_nonce + 1); // u64::MAX + + // This should fail because we've simulated exhausting the nonce space, as the spec says 2^64-1 is reserved + // and may not be used in the `CipherState` object. + assert!(matches!( + dbg!(h_r.read_message(&buffer_msg[..len], &mut buffer_out)), + Err(snow::Error::State(snow::error::StateProblem::Exhausted)) + )); +} + +#[test] +fn test_stateless_nonce_maximum_behavior() { + let params: NoiseParams = "Noise_NN_25519_ChaChaPoly_SHA256".parse().unwrap(); + let mut h_i = Builder::new(params.clone()).build_initiator().unwrap(); + let mut h_r = Builder::new(params).build_responder().unwrap(); + + let mut buffer_msg = [0u8; 200]; + let mut buffer_out = [0u8; 200]; + let len = h_i.write_message(b"abc", &mut buffer_msg).unwrap(); + h_r.read_message(&buffer_msg[..len], &mut buffer_out).unwrap(); + + let len = h_r.write_message(b"defg", &mut buffer_msg).unwrap(); + h_i.read_message(&buffer_msg[..len], &mut buffer_out).unwrap(); + + let h_i = h_i.into_stateless_transport_mode().unwrap(); + let h_r = h_r.into_stateless_transport_mode().unwrap(); + + let max_nonce = u64::MAX; + + assert!(matches!( + h_i.write_message(max_nonce, b"xyz", &mut buffer_msg), + Err(snow::Error::State(snow::error::StateProblem::Exhausted)) + )); + + assert!(matches!( + h_r.read_message(max_nonce, &buffer_msg, &mut buffer_out), + Err(snow::Error::State(snow::error::StateProblem::Exhausted)) + )); +}