From 0cb5fc132cc50bd264e286de0fc0e68a40eda534 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Thu, 6 Aug 2026 09:44:32 +0530 Subject: [PATCH 01/21] noise_sv2: zero Initiator chaining key and hash in place `for mut b in self.ck` iterates the `[u8; 32]` by value, so each `b` is a stack-local copy and `write_volatile(&mut b, 0)` zeroes that copy rather than the field. Iterate by mutable reference instead, matching the `self.k` loop just above. --- sv2/noise-sv2/src/initiator.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sv2/noise-sv2/src/initiator.rs b/sv2/noise-sv2/src/initiator.rs index 2361571cd1..d7605a7f5b 100644 --- a/sv2/noise-sv2/src/initiator.rs +++ b/sv2/noise-sv2/src/initiator.rs @@ -401,11 +401,11 @@ impl Initiator { unsafe { ptr::write_volatile(b, 0) }; } } - for mut b in self.ck { - unsafe { ptr::write_volatile(&mut b, 0) }; + for b in &mut self.ck { + unsafe { ptr::write_volatile(b, 0) }; } - for mut b in self.h { - unsafe { ptr::write_volatile(&mut b, 0) }; + for b in &mut self.h { + unsafe { ptr::write_volatile(b, 0) }; } self.e.non_secure_erase(); } From 3ebb8c11b2bbd23a303de8f2eb085115ff8adbff Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Thu, 6 Aug 2026 09:44:39 +0530 Subject: [PATCH 02/21] noise_sv2: zero Responder chaining key and hash in place Same by-value iteration bug as in `Initiator::erase`: the volatile writes landed on stack-local copies, leaving `ck` and `h` untouched. --- sv2/noise-sv2/src/responder.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sv2/noise-sv2/src/responder.rs b/sv2/noise-sv2/src/responder.rs index e0f5623b8a..811e303446 100644 --- a/sv2/noise-sv2/src/responder.rs +++ b/sv2/noise-sv2/src/responder.rs @@ -422,11 +422,11 @@ impl Responder { unsafe { ptr::write_volatile(b, 0) }; } } - for mut b in self.ck { - unsafe { ptr::write_volatile(&mut b, 0) }; + for b in &mut self.ck { + unsafe { ptr::write_volatile(b, 0) }; } - for mut b in self.h { - unsafe { ptr::write_volatile(&mut b, 0) }; + for b in &mut self.h { + unsafe { ptr::write_volatile(b, 0) }; } self.e.non_secure_erase(); self.s.non_secure_erase(); From b542cf320781456557af4ec939f7192fe6f26008 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Thu, 6 Aug 2026 09:44:51 +0530 Subject: [PATCH 03/21] noise_sv2: test that erase wipes the chaining key and hash Both tests fail against the previous by-value erase loops. --- sv2/noise-sv2/src/initiator.rs | 14 ++++++++++++++ sv2/noise-sv2/src/responder.rs | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/sv2/noise-sv2/src/initiator.rs b/sv2/noise-sv2/src/initiator.rs index d7605a7f5b..6dcb2b48d5 100644 --- a/sv2/noise-sv2/src/initiator.rs +++ b/sv2/noise-sv2/src/initiator.rs @@ -432,6 +432,20 @@ mod test { assert!(msg.iter().any(|b| *b != 0)); } + #[test] + #[cfg(feature = "std")] + #[cfg_attr(miri, ignore)] + fn initiator_erase_zeroes_ck_and_h() { + let mut initiator = Initiator::without_pk().unwrap(); + assert!(initiator.ck.iter().any(|b| *b != 0)); + assert!(initiator.h.iter().any(|b| *b != 0)); + + initiator.erase(); + + assert_eq!(initiator.ck, [0u8; 32]); + assert_eq!(initiator.h, [0u8; 32]); + } + #[test] #[cfg(feature = "std")] #[cfg_attr(miri, ignore)] diff --git a/sv2/noise-sv2/src/responder.rs b/sv2/noise-sv2/src/responder.rs index 811e303446..9aaebe1573 100644 --- a/sv2/noise-sv2/src/responder.rs +++ b/sv2/noise-sv2/src/responder.rs @@ -477,6 +477,20 @@ mod test { assert_eq!(msg.len(), INITIATOR_EXPECTED_HANDSHAKE_MESSAGE_SIZE); } + #[test] + #[cfg(feature = "std")] + #[cfg_attr(miri, ignore)] + fn responder_erase_zeroes_ck_and_h() { + let mut responder = make_responder(); + assert!(responder.ck.iter().any(|b| *b != 0)); + assert!(responder.h.iter().any(|b| *b != 0)); + + responder.erase(); + + assert_eq!(responder.ck, [0u8; 32]); + assert_eq!(responder.h, [0u8; 32]); + } + #[test] #[cfg(feature = "std")] #[cfg_attr(miri, ignore)] From 6ca8dfa2108a763bf7eedb862a94197a681e2d8e Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Thu, 6 Aug 2026 09:45:00 +0530 Subject: [PATCH 04/21] codec_sv2: stop deriving Clone for State and HandshakeRole Both types carry Noise handshake and transport state. Duplicating them hands out two independent nonce counters over the same key, so cloning is never safe here. Nothing in the workspace relied on it. --- sv2/codec-sv2/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sv2/codec-sv2/src/lib.rs b/sv2/codec-sv2/src/lib.rs index d001aca843..b869871894 100644 --- a/sv2/codec-sv2/src/lib.rs +++ b/sv2/codec-sv2/src/lib.rs @@ -70,7 +70,7 @@ pub use encoder::NoiseEncoder; /// process accordingly. #[allow(clippy::large_enum_variant)] #[cfg(feature = "noise_sv2")] -#[derive(Debug, Clone)] +#[derive(Debug)] pub enum HandshakeRole { /// The initiator role in the Noise handshake process. /// @@ -94,7 +94,7 @@ pub enum HandshakeRole { /// [`State::HandShake`] and finally to transport mode [`State::Transport`] as the encryption /// handshake is completed. #[cfg(feature = "noise_sv2")] -#[derive(Debug, Clone)] +#[derive(Debug)] #[allow(clippy::large_enum_variant)] pub enum State { /// The codec has not been initialized yet. From c92dee4f7ebe5686b6be6f806c1746d988cff900 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Thu, 6 Aug 2026 09:45:15 +0530 Subject: [PATCH 05/21] noise_sv2: stop deriving Clone for Initiator and Responder A cloned handshake role keeps the same chaining key, so both copies derive identical transport keys and then encrypt distinct plaintexts under the same key and nonce. --- sv2/noise-sv2/src/initiator.rs | 8 +++++--- sv2/noise-sv2/src/responder.rs | 1 - 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/sv2/noise-sv2/src/initiator.rs b/sv2/noise-sv2/src/initiator.rs index 6dcb2b48d5..eb8247a7c1 100644 --- a/sv2/noise-sv2/src/initiator.rs +++ b/sv2/noise-sv2/src/initiator.rs @@ -58,9 +58,11 @@ use secp256k1::{ /// Manages the initiator's role in the Noise NX handshake, handling key exchange, encryption, and /// handshake state. It securely generates and manages cryptographic keys, performs Diffie-Hellman /// exchanges, and maintains the handshake hash, chaining key, and nonce for message encryption. -/// After the handshake, it facilitates secure communication using [`ChaCha20Poly1305`]. Sensitive -/// data is securely erased when no longer needed. -#[derive(Clone)] +/// After the handshake, it facilitates secure communication using either [`ChaCha20Poly1305`] or +/// `AES-GCM` ciphers. Sensitive data is securely erased when no longer needed. +/// +/// Deliberately not [`Clone`]: two copies of the same handshake state would derive the same +/// transport keys and reuse the same nonces. pub struct Initiator { // Cipher used for encrypting and decrypting messages during the handshake. // diff --git a/sv2/noise-sv2/src/responder.rs b/sv2/noise-sv2/src/responder.rs index 9aaebe1573..9ff202ad30 100644 --- a/sv2/noise-sv2/src/responder.rs +++ b/sv2/noise-sv2/src/responder.rs @@ -59,7 +59,6 @@ const VERSION: u16 = 0; /// a connection with the initiator. The responder manages key generation, Diffie-Hellman exchanges, /// message decryption, and state transitions, ensuring secure communication. Sensitive /// cryptographic material is securely erased when no longer needed. -#[derive(Clone)] pub struct Responder { // Cipher used for encrypting and decrypting messages during the handshake. // From 3460e78beda60a0f38d4d19f61c1652e1329954d Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Thu, 6 Aug 2026 09:45:26 +0530 Subject: [PATCH 06/21] noise_sv2: stop deriving Clone for NoiseCodec and its ciphers Each clone of a transport cipher carries its own nonce counter while sharing the key, so the copies silently encrypt different plaintexts under the same key and nonce. --- sv2/noise-sv2/src/cipher_state.rs | 1 - sv2/noise-sv2/src/lib.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/sv2/noise-sv2/src/cipher_state.rs b/sv2/noise-sv2/src/cipher_state.rs index 5838cb81d6..3bb87171f2 100644 --- a/sv2/noise-sv2/src/cipher_state.rs +++ b/sv2/noise-sv2/src/cipher_state.rs @@ -139,7 +139,6 @@ where // It stores the optional encryption key, the nonce, and the optional cipher instance itself. The // [`CipherState`] trait is implemented to provide a consistent interface for managing cipher // state across different AEAD ciphers. -#[derive(Clone)] pub struct Cipher { // Optional 32-byte encryption key. k: Option<[u8; 32]>, diff --git a/sv2/noise-sv2/src/lib.rs b/sv2/noise-sv2/src/lib.rs index 2b456b804c..70ee20ef45 100644 --- a/sv2/noise-sv2/src/lib.rs +++ b/sv2/noise-sv2/src/lib.rs @@ -104,7 +104,6 @@ const PARITY: secp256k1::Parity = secp256k1::Parity::Even; /// Manages the encryption and decryption of messages between two parties, the [`Initiator`] and /// [`Responder`], using the Noise protocol. A symmetric cipher is used for both encrypting /// outgoing messages and decrypting incoming messages. -#[derive(Clone)] pub struct NoiseEngine { // Cipher to encrypt outgoing messages. encryptor: Cipher, From f31936ced31eac8a22227dd3bf94a5ef7bf459e7 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Thu, 6 Aug 2026 09:45:40 +0530 Subject: [PATCH 07/21] noise_sv2: require CryptoRng for ephemeral key generation Ephemeral keypairs were generated from any `Rng`, which accepted weak or deterministic generators. Bound `generate_key_with_rng` on `CryptoRng` and propagate it through the `Initiator` and `Responder` constructors. --- sv2/noise-sv2/src/handshake.rs | 4 +++- sv2/noise-sv2/src/initiator.rs | 8 +++++--- sv2/noise-sv2/src/responder.rs | 4 ++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/sv2/noise-sv2/src/handshake.rs b/sv2/noise-sv2/src/handshake.rs index f597750be8..b66c340c93 100644 --- a/sv2/noise-sv2/src/handshake.rs +++ b/sv2/noise-sv2/src/handshake.rs @@ -109,8 +109,10 @@ pub trait HandshakeOp: CipherState { Self::generate_key_with_rng(&mut rand::thread_rng()) } + // The `CryptoRng` bound rejects generators that are not suitable for cryptographic use, such + // as deterministic or weak ones. #[inline] - fn generate_key_with_rng(rng: &mut R) -> Keypair { + fn generate_key_with_rng(rng: &mut R) -> Keypair { let secp = Secp256k1::new(); let (mut secret_key, public_key) = secp.generate_keypair(rng); if public_key.x_only_public_key().1 == secp256k1::Parity::Odd { diff --git a/sv2/noise-sv2/src/initiator.rs b/sv2/noise-sv2/src/initiator.rs index eb8247a7c1..8e7761124f 100644 --- a/sv2/noise-sv2/src/initiator.rs +++ b/sv2/noise-sv2/src/initiator.rs @@ -175,7 +175,7 @@ impl Initiator { /// provided in order to not implicitely rely on `std` and allow `no_std` environments to /// provide a hardware random number generator for example. #[inline] - pub fn new_with_rng( + pub fn new_with_rng( pk: Option, rng: &mut R, ) -> Box { @@ -213,7 +213,7 @@ impl Initiator { /// `std` and allow `no_std` environments to provide a hardware random number generator for /// example. #[inline] - pub fn from_raw_k_with_rng( + pub fn from_raw_k_with_rng( key: [u8; 32], rng: &mut R, ) -> Result, Error> { @@ -241,7 +241,9 @@ impl Initiator { /// `std` and allow `no_std` environments to provide a hardware random number generator for /// example. #[inline] - pub fn without_pk_with_rng(rng: &mut R) -> Result, Error> { + pub fn without_pk_with_rng( + rng: &mut R, + ) -> Result, Error> { Ok(Self::new_with_rng(None, rng)) } diff --git a/sv2/noise-sv2/src/responder.rs b/sv2/noise-sv2/src/responder.rs index 9ff202ad30..c85f2b5683 100644 --- a/sv2/noise-sv2/src/responder.rs +++ b/sv2/noise-sv2/src/responder.rs @@ -180,7 +180,7 @@ impl Responder { /// `std` and allow `no_std` environments to provide a hardware random number generator for /// example. #[inline] - pub fn new_with_rng( + pub fn new_with_rng( a: Keypair, cert_validity: u32, rng: &mut R, @@ -224,7 +224,7 @@ impl Responder { /// `std` and allow `no_std` environments to provide a hardware random number generator for /// example. #[inline] - pub fn from_authority_kp_with_rng( + pub fn from_authority_kp_with_rng( public: &[u8; 32], private: &[u8; 32], cert_validity: Duration, From 897fb5365d3ee59d87d3d303eb462baacec6e7f8 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sun, 9 Aug 2026 14:40:44 +0530 Subject: [PATCH 08/21] noise_sv2: split NoiseEngine into directional cipher halves A completed handshake yields one cipher per direction, each with its own key and nonce counter. Keeping them together forces the read and write sides of a connection to share the engine, so neither side can be owned independently. `into_split` consumes the engine and hands out `NoiseEncryptor` and `NoiseDecryptor`, one cipher each. Consuming is what keeps this safe: a duplicated cipher would reuse a nonce under the same key, which is why none of these types are `Clone`. --- sv2/noise-sv2/src/lib.rs | 50 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/sv2/noise-sv2/src/lib.rs b/sv2/noise-sv2/src/lib.rs index 70ee20ef45..6e7b3ac615 100644 --- a/sv2/noise-sv2/src/lib.rs +++ b/sv2/noise-sv2/src/lib.rs @@ -104,6 +104,8 @@ const PARITY: secp256k1::Parity = secp256k1::Parity::Even; /// Manages the encryption and decryption of messages between two parties, the [`Initiator`] and /// [`Responder`], using the Noise protocol. A symmetric cipher is used for both encrypting /// outgoing messages and decrypting incoming messages. +/// +/// Call [`Self::into_split`] to divide the engine into its two directional halves. pub struct NoiseEngine { // Cipher to encrypt outgoing messages. encryptor: Cipher, @@ -128,6 +130,54 @@ impl NoiseEngine { pub fn decrypt(&mut self, msg: &mut T) -> Result<(), AeadError> { self.decryptor.decrypt(msg) } + + /// Splits the engine into its sending and receiving halves; consuming it prevents nonce reuse. + pub fn into_split(self) -> (NoiseEncryptor, NoiseDecryptor) { + ( + NoiseEncryptor { + cipher: self.encryptor, + }, + NoiseDecryptor { + cipher: self.decryptor, + }, + ) + } +} + +/// The sending half of a [`NoiseEngine`], owning only the outgoing cipher and its nonce counter. +pub struct NoiseEncryptor { + cipher: Cipher, +} + +impl core::fmt::Debug for NoiseEncryptor { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("NoiseEncryptor").finish() + } +} + +impl NoiseEncryptor { + /// Encrypts a message (`msg`) in place using the stored cipher. + pub fn encrypt(&mut self, msg: &mut T) -> Result<(), AeadError> { + self.cipher.encrypt(msg) + } +} + +/// The receiving half of a [`NoiseEngine`], owning only the incoming cipher and its nonce counter. +pub struct NoiseDecryptor { + cipher: Cipher, +} + +impl core::fmt::Debug for NoiseDecryptor { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("NoiseDecryptor").finish() + } +} + +impl NoiseDecryptor { + /// Decrypts a message (`msg`) in place using the stored cipher. + pub fn decrypt(&mut self, msg: &mut T) -> Result<(), AeadError> { + self.cipher.decrypt(msg) + } } pub use error::Error; From 21b6049c06c9f423d1f3fa13d69f011e3d53d3a3 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sun, 9 Aug 2026 14:41:07 +0530 Subject: [PATCH 09/21] codec_sv2: extract the noise encrypt path out of encode `encode` matched on the state and inlined the whole serialize-then-encrypt sequence in its transport arm. Move that sequence into `encrypt_frame`, which takes the encryption step as a closure, so the arm is a single delegation. No behaviour change: this only gives the transport path a caller-agnostic home before a second caller is added. --- sv2/codec-sv2/src/encoder.rs | 89 ++++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 39 deletions(-) diff --git a/sv2/codec-sv2/src/encoder.rs b/sv2/codec-sv2/src/encoder.rs index 08d2290858..9084c33fbd 100644 --- a/sv2/codec-sv2/src/encoder.rs +++ b/sv2/codec-sv2/src/encoder.rs @@ -130,45 +130,8 @@ impl WithNoise { #[inline] pub fn encode(&mut self, item: Item, state: &mut State) -> Result { match state { - State::Transport(noise_engine) => { - let len = item.encoded_length(); - let writable = self.sv2_buffer.get_writable(len); - - // ENCODE THE SV2 FRAME - let i: Sv2Frame = item.try_into().map_err(|e| { - #[cfg(feature = "tracing")] - error!("Error while encoding 1 frame: {:?}", e); - Error::FramingError(e) - })?; - i.serialize(writable)?; - - let sv2 = self.sv2_buffer.get_data_owned(); - let sv2: &[u8] = sv2.as_ref(); - - // ENCRYPT THE HEADER - let to_encrypt = self.noise_buffer.get_writable(SV2_FRAME_HEADER_SIZE); - to_encrypt.copy_from_slice(&sv2[..SV2_FRAME_HEADER_SIZE]); - noise_engine.encrypt(&mut self.noise_buffer)?; - - // ENCRYPT THE PAYLOAD IN CHUNKS - let mut start = SV2_FRAME_HEADER_SIZE; - let mut end = if sv2.len() - start < (SV2_FRAME_CHUNK_SIZE - AEAD_MAC_LEN) { - sv2.len() - } else { - SV2_FRAME_CHUNK_SIZE + start - AEAD_MAC_LEN - }; - let mut encrypted_len = ENCRYPTED_SV2_FRAME_HEADER_SIZE; - - while start < sv2.len() { - let to_encrypt = self.noise_buffer.get_writable(end - start); - to_encrypt.copy_from_slice(&sv2[start..end]); - self.noise_buffer.danger_set_start(encrypted_len); - noise_engine.encrypt(&mut self.noise_buffer)?; - encrypted_len += self.noise_buffer.as_ref().len(); - start = end; - end = (start + SV2_FRAME_CHUNK_SIZE - AEAD_MAC_LEN).min(sv2.len()); - } - self.noise_buffer.danger_set_start(0); + State::Transport(engine) => { + self.encrypt_frame(item, |buf| engine.encrypt(buf).map_err(Into::into))? } State::HandShake(_) => self.while_handshaking(item)?, State::NotInitialized(_) => self.while_handshaking(item)?, @@ -180,6 +143,54 @@ impl WithNoise { Ok(self.noise_buffer.get_data_owned()) } + // Serializes `item` into an Sv2 frame and encrypts it in place through `encrypt`. + #[inline] + fn encrypt_frame( + &mut self, + item: Item, + mut encrypt: impl FnMut(&mut B) -> Result<()>, + ) -> Result<()> { + let len = item.encoded_length(); + let writable = self.sv2_buffer.get_writable(len); + + // ENCODE THE SV2 FRAME + let i: Sv2Frame = item.try_into().map_err(|e| { + #[cfg(feature = "tracing")] + error!("Error while encoding 1 frame: {:?}", e); + Error::FramingError(e) + })?; + i.serialize(writable)?; + + let sv2 = self.sv2_buffer.get_data_owned(); + let sv2: &[u8] = sv2.as_ref(); + + // ENCRYPT THE HEADER + let to_encrypt = self.noise_buffer.get_writable(SV2_FRAME_HEADER_SIZE); + to_encrypt.copy_from_slice(&sv2[..SV2_FRAME_HEADER_SIZE]); + encrypt(&mut self.noise_buffer)?; + + // ENCRYPT THE PAYLOAD IN CHUNKS + let mut start = SV2_FRAME_HEADER_SIZE; + let mut end = if sv2.len() - start < (SV2_FRAME_CHUNK_SIZE - AEAD_MAC_LEN) { + sv2.len() + } else { + SV2_FRAME_CHUNK_SIZE + start - AEAD_MAC_LEN + }; + let mut encrypted_len = ENCRYPTED_SV2_FRAME_HEADER_SIZE; + + while start < sv2.len() { + let to_encrypt = self.noise_buffer.get_writable(end - start); + to_encrypt.copy_from_slice(&sv2[start..end]); + self.noise_buffer.danger_set_start(encrypted_len); + encrypt(&mut self.noise_buffer)?; + encrypted_len += self.noise_buffer.as_ref().len(); + start = end; + end = (start + SV2_FRAME_CHUNK_SIZE - AEAD_MAC_LEN).min(sv2.len()); + } + self.noise_buffer.danger_set_start(0); + Ok(()) + } + // Encodes Sv2 frames during the handshake phase of the Noise protocol. // // Used when the encoder is in the handshake phase, before secure communication is fully From 2cd8fb86f9720472dabaaa6d09290b7622e0c950 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sun, 9 Aug 2026 14:41:28 +0530 Subject: [PATCH 10/21] codec_sv2: extract the noise decrypt path out of next_frame Mirror of the encoder change: the transport arm of `next_frame` moves into `next_transport`, and `decode_noise_frame` takes the decryption step as a closure instead of the engine itself. The arm becomes a single delegation. No behaviour change: this only gives the transport path a caller-agnostic home before a second caller is added. --- sv2/codec-sv2/src/decoder.rs | 68 +++++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 29 deletions(-) diff --git a/sv2/codec-sv2/src/decoder.rs b/sv2/codec-sv2/src/decoder.rs index 6793c13b25..9112ebd155 100644 --- a/sv2/codec-sv2/src/decoder.rs +++ b/sv2/codec-sv2/src/decoder.rs @@ -39,8 +39,6 @@ use framing_sv2::{ #[cfg(feature = "noise_sv2")] use framing_sv2::{ENCRYPTED_SV2_FRAME_HEADER_SIZE, SV2_FRAME_CHUNK_SIZE, SV2_FRAME_HEADER_SIZE}; #[cfg(feature = "noise_sv2")] -use noise_sv2::NoiseEngine; -#[cfg(feature = "noise_sv2")] use noise_sv2::NOISE_FRAME_HEADER_SIZE; #[cfg(feature = "noise_sv2")] @@ -155,31 +153,40 @@ impl<'a, T: Serialize + GetSize + Deserialize<'a>, B: IsBuffer + AeadBuffer> Wit } } } - State::Transport(noise_engine) => { - let hint = if IsBuffer::len(&self.sv2_buffer) < SV2_FRAME_HEADER_SIZE { - let len = IsBuffer::len(&self.noise_buffer); - let src = self.noise_buffer.get_data_by_ref(len); - if src.len() < ENCRYPTED_SV2_FRAME_HEADER_SIZE { - ENCRYPTED_SV2_FRAME_HEADER_SIZE - src.len() - } else { - 0 - } - } else { - let src = self.sv2_buffer.get_data_by_ref(SV2_FRAME_HEADER_SIZE); - let header = Header::from_bytes(src)?; - header.encrypted_len() - IsBuffer::len(&self.noise_buffer) - }; + State::Transport(engine) => { + self.next_transport(|buf| engine.decrypt(buf).map_err(Into::into)) + } + } + } - match hint { - 0 => { - self.missing_noise_b = ENCRYPTED_SV2_FRAME_HEADER_SIZE; - self.decode_noise_frame(noise_engine) - } - _ => { - self.missing_noise_b = hint; - Err(Error::MissingBytes(hint)) - } - } + // Decodes a transport-mode frame, decrypting through `decrypt`. + #[inline] + fn next_transport( + &mut self, + decrypt: impl FnMut(&mut B) -> Result<()>, + ) -> Result> { + let hint = if IsBuffer::len(&self.sv2_buffer) < SV2_FRAME_HEADER_SIZE { + let len = IsBuffer::len(&self.noise_buffer); + let src = self.noise_buffer.get_data_by_ref(len); + if src.len() < ENCRYPTED_SV2_FRAME_HEADER_SIZE { + ENCRYPTED_SV2_FRAME_HEADER_SIZE - src.len() + } else { + 0 + } + } else { + let src = self.sv2_buffer.get_data_by_ref(SV2_FRAME_HEADER_SIZE); + let header = Header::from_bytes(src)?; + header.encrypted_len() - IsBuffer::len(&self.noise_buffer) + }; + + match hint { + 0 => { + self.missing_noise_b = ENCRYPTED_SV2_FRAME_HEADER_SIZE; + self.decode_noise_frame(decrypt) + } + _ => { + self.missing_noise_b = hint; + Err(Error::MissingBytes(hint)) } } } @@ -255,7 +262,10 @@ impl<'a, T: Serialize + GetSize + Deserialize<'a>, B: IsBuffer + AeadBuffer> Wit // additional bytes required to fully decrypt the frame. Once all bytes are available, the // decryption process completes and the frame can be successfully decoded. #[inline] - fn decode_noise_frame(&mut self, noise_engine: &mut NoiseEngine) -> Result> { + fn decode_noise_frame( + &mut self, + mut decrypt: impl FnMut(&mut B) -> Result<()>, + ) -> Result> { match ( IsBuffer::len(&self.noise_buffer), IsBuffer::len(&self.sv2_buffer), @@ -268,7 +278,7 @@ impl<'a, T: Serialize + GetSize + Deserialize<'a>, B: IsBuffer + AeadBuffer> Wit .get_writable(ENCRYPTED_SV2_FRAME_HEADER_SIZE); decrypted_header.copy_from_slice(src.as_ref()); self.sv2_buffer.as_ref(); - noise_engine.decrypt(&mut self.sv2_buffer)?; + decrypt(&mut self.sv2_buffer)?; let header = Header::from_bytes(self.sv2_buffer.get_data_by_ref(SV2_FRAME_HEADER_SIZE))?; self.missing_noise_b = header.encrypted_len(); @@ -292,7 +302,7 @@ impl<'a, T: Serialize + GetSize + Deserialize<'a>, B: IsBuffer + AeadBuffer> Wit let decrypted_payload = self.sv2_buffer.get_writable(end - start); decrypted_payload.copy_from_slice(&encrypted_payload.as_ref()[start..end]); self.sv2_buffer.danger_set_start(decrypted_len); - noise_engine.decrypt(&mut self.sv2_buffer)?; + decrypt(&mut self.sv2_buffer)?; start = end; end = (start + SV2_FRAME_CHUNK_SIZE).min(encrypted_payload_len); decrypted_len += self.sv2_buffer.as_ref().len(); From a44ce3a6d540bec33496c3229d0aba6f66979ce5 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sun, 9 Aug 2026 14:41:47 +0530 Subject: [PATCH 11/21] codec_sv2: add transport states for splitting a noise connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A connection's reader and writer each need only one direction of the transport ciphers, but both had to share a `State`. `State` is not `Clone` — duplicating it would hand out two nonce counters over the same key — so the two halves could not be owned by separate tasks. `State::split_transport` consumes a transport-mode state and returns `TransportEncryptState` and `TransportDecryptState`, holding one cipher each. `NoiseEncoder::encode_transport` and `StandardNoiseDecoder::next_transport_frame` take them, so passing the wrong half to the wrong side is a compile error. Purely additive: `State`, its variants and constructors, `encode` and `next_frame` are unchanged, and an unsplit state still drives both directions. --- sv2/codec-sv2/src/decoder.rs | 11 ++++++- sv2/codec-sv2/src/encoder.rs | 17 ++++++++++- sv2/codec-sv2/src/lib.rs | 56 +++++++++++++++++++++++++++++++++- sv2/noise-sv2/src/initiator.rs | 3 -- 4 files changed, 81 insertions(+), 6 deletions(-) diff --git a/sv2/codec-sv2/src/decoder.rs b/sv2/codec-sv2/src/decoder.rs index 9112ebd155..a78a7db378 100644 --- a/sv2/codec-sv2/src/decoder.rs +++ b/sv2/codec-sv2/src/decoder.rs @@ -47,7 +47,7 @@ use crate::error::Result; use crate::Error::MissingBytes; #[cfg(feature = "noise_sv2")] -use crate::State; +use crate::{State, TransportDecryptState}; #[cfg(not(feature = "with_buffer_pool"))] use buffer_sv2::{Buffer as IsBuffer, BufferFromSystemMemory as Buffer}; @@ -159,6 +159,15 @@ impl<'a, T: Serialize + GetSize + Deserialize<'a>, B: IsBuffer + AeadBuffer> Wit } } + /// Attempts to decode the next Noise frame with the decrypting half of a split [`State`]. + #[inline] + pub fn next_transport_frame( + &mut self, + state: &mut TransportDecryptState, + ) -> Result> { + self.next_transport(|buf| state.decrypt(buf)) + } + // Decodes a transport-mode frame, decrypting through `decrypt`. #[inline] fn next_transport( diff --git a/sv2/codec-sv2/src/encoder.rs b/sv2/codec-sv2/src/encoder.rs index 9084c33fbd..b873611588 100644 --- a/sv2/codec-sv2/src/encoder.rs +++ b/sv2/codec-sv2/src/encoder.rs @@ -40,7 +40,7 @@ use noise_sv2::AEAD_MAC_LEN; use tracing::error; #[cfg(feature = "noise_sv2")] -use crate::{Error, Result, State}; +use crate::{Error, Result, State, TransportEncryptState}; #[cfg(not(feature = "with_buffer_pool"))] use buffer_sv2::{Buffer as IsBuffer, BufferFromSystemMemory as Buffer}; @@ -143,6 +143,21 @@ impl WithNoise { Ok(self.noise_buffer.get_data_owned()) } + /// Encodes an Sv2 frame and encrypts it with the encrypting half of a split [`State`]. + #[inline] + pub fn encode_transport( + &mut self, + item: Item, + state: &mut TransportEncryptState, + ) -> Result { + self.encrypt_frame(item, |buf| state.encrypt(buf))?; + + // Clear sv2_buffer + self.sv2_buffer.get_data_owned(); + // Return noise_buffer + Ok(self.noise_buffer.get_data_owned()) + } + // Serializes `item` into an Sv2 frame and encrypts it in place through `encrypt`. #[inline] fn encrypt_frame( diff --git a/sv2/codec-sv2/src/lib.rs b/sv2/codec-sv2/src/lib.rs index b869871894..3adc0cb7a9 100644 --- a/sv2/codec-sv2/src/lib.rs +++ b/sv2/codec-sv2/src/lib.rs @@ -40,9 +40,11 @@ extern crate alloc; #[cfg(feature = "noise_sv2")] use alloc::boxed::Box; #[cfg(feature = "noise_sv2")] +use buffer_sv2::AeadBuffer; +#[cfg(feature = "noise_sv2")] use framing_sv2::framing::{handshake_message_to_frame as h2f, HandShakeFrame}; #[cfg(feature = "noise_sv2")] -use noise_sv2::NoiseEngine; +use noise_sv2::{NoiseDecryptor, NoiseEncryptor, NoiseEngine}; mod decoder; mod encoder; @@ -119,6 +121,42 @@ pub enum State { Transport(NoiseEngine), } +/// The encrypting half of a transport-mode [`State`], used by the encoder's transport path. +#[cfg(feature = "noise_sv2")] +#[derive(Debug)] +pub struct TransportEncryptState { + encryption: NoiseEncryptor, +} + +#[cfg(feature = "noise_sv2")] +impl TransportEncryptState { + // Encrypts `msg` in place with the outgoing cipher. + pub(crate) fn encrypt( + &mut self, + msg: &mut T, + ) -> core::result::Result<(), Error> { + self.encryption.encrypt(msg).map_err(Into::into) + } +} + +/// The decrypting half of a transport-mode [`State`], used by the decoder's transport path. +#[cfg(feature = "noise_sv2")] +#[derive(Debug)] +pub struct TransportDecryptState { + decryption: NoiseDecryptor, +} + +#[cfg(feature = "noise_sv2")] +impl TransportDecryptState { + // Decrypts `msg` in place with the incoming cipher. + pub(crate) fn decrypt( + &mut self, + msg: &mut T, + ) -> core::result::Result<(), Error> { + self.decryption.decrypt(msg).map_err(Into::into) + } +} + #[cfg(feature = "noise_sv2")] impl State { /// Initiates the first step of the handshake process for the initiator. @@ -277,6 +315,22 @@ impl State { pub fn with_transport_mode(tm: NoiseEngine) -> Self { Self::Transport(tm) } + + /// Splits a transport-mode state into its encrypting and decrypting halves, consuming it. + pub fn split_transport( + self, + ) -> core::result::Result<(TransportEncryptState, TransportDecryptState), Error> { + match self { + Self::Transport(engine) => { + let (encryption, decryption) = engine.into_split(); + Ok(( + TransportEncryptState { encryption }, + TransportDecryptState { decryption }, + )) + } + _ => Err(Error::UnexpectedNoiseState), + } + } } #[cfg(test)] diff --git a/sv2/noise-sv2/src/initiator.rs b/sv2/noise-sv2/src/initiator.rs index 8e7761124f..750f3d8ce7 100644 --- a/sv2/noise-sv2/src/initiator.rs +++ b/sv2/noise-sv2/src/initiator.rs @@ -60,9 +60,6 @@ use secp256k1::{ /// exchanges, and maintains the handshake hash, chaining key, and nonce for message encryption. /// After the handshake, it facilitates secure communication using either [`ChaCha20Poly1305`] or /// `AES-GCM` ciphers. Sensitive data is securely erased when no longer needed. -/// -/// Deliberately not [`Clone`]: two copies of the same handshake state would derive the same -/// transport keys and reuse the same nonces. pub struct Initiator { // Cipher used for encrypting and decrypting messages during the handshake. // From 0ed0e353e1cfa3e5b9fa7fa65ef24f06293f412f Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Tue, 11 Aug 2026 15:23:22 +0530 Subject: [PATCH 12/21] Add transport round trip test --- sv2/codec-sv2/src/lib.rs | 110 ++++++++++++++++++++++++++++++++- sv2/noise-sv2/src/handshake.rs | 5 +- sv2/noise-sv2/src/initiator.rs | 4 +- 3 files changed, 114 insertions(+), 5 deletions(-) diff --git a/sv2/codec-sv2/src/lib.rs b/sv2/codec-sv2/src/lib.rs index 3adc0cb7a9..a582cd5425 100644 --- a/sv2/codec-sv2/src/lib.rs +++ b/sv2/codec-sv2/src/lib.rs @@ -336,7 +336,115 @@ impl State { #[cfg(test)] #[cfg(feature = "noise_sv2")] mod tests { - use super::*; + use crate::{ + Error, HandshakeRole, NoiseEncoder, StandardEitherFrame, StandardNoiseDecoder, + StandardSv2Frame, State, TransportDecryptState, TransportEncryptState, + }; + use binary_sv2::{Deserialize, Serialize}; + use framing_sv2::framing::Sv2Frame; + use key_utils::{Secp256k1PublicKey, Secp256k1SecretKey}; + use noise_sv2::{ + Initiator, Responder, ELLSWIFT_ENCODING_SIZE, INITIATOR_EXPECTED_HANDSHAKE_MESSAGE_SIZE, + }; + + const AUTHORITY_PUBLIC_K: &str = "9auqWEzQDVyd2oe1JVGFLMLHZtCo2FFqZwtKA5gd9xbuEu7PH72"; + const AUTHORITY_PRIVATE_K: &str = "mkDLTBBRxdBv998612qipDYoTK3YUrqLe8uWw7gu3iXbSrn2n"; + const CERT_VALIDITY: core::time::Duration = core::time::Duration::from_secs(3600); + const MSG_TYPE: u8 = 0xff; + + #[derive(Debug, Clone, Serialize, Deserialize)] + struct TestMsg { + nonce: u16, + } + + // Encrypts `nonce` through `enc`, then decrypts it back through `dec` and returns the nonce + // carried by the decoded message. + fn round_trip( + encoder: &mut NoiseEncoder, + enc: &mut TransportEncryptState, + dec: &mut TransportDecryptState, + nonce: u16, + ) -> u16 { + let frame = StandardEitherFrame::::Sv2( + Sv2Frame::from_message(TestMsg { nonce }, MSG_TYPE, 0, false).unwrap(), + ); + let encrypted = encoder.encode_transport(frame, enc).unwrap(); + + let mut decoder = StandardNoiseDecoder::::new(); + let mut offset = 0; + loop { + let writable = decoder.writable(); + let len = writable.len(); + writable.copy_from_slice(&encrypted[offset..offset + len]); + offset += len; + + match decoder.next_transport_frame(dec) { + Ok(frame) => { + let mut frame: StandardSv2Frame = frame.try_into().unwrap(); + assert_eq!(frame.get_header().unwrap().msg_type(), MSG_TYPE); + let msg: TestMsg = binary_sv2::from_bytes(frame.payload()).unwrap(); + return msg.nonce; + } + Err(Error::MissingBytes(_)) => {} + Err(e) => panic!("failed to decode a transport frame: {e:?}"), + } + } + } + + #[test] + fn split_transport_round_trips_in_both_directions() { + let authority_public_k: Secp256k1PublicKey = + AUTHORITY_PUBLIC_K.to_string().try_into().unwrap(); + let authority_private_k: Secp256k1SecretKey = + AUTHORITY_PRIVATE_K.to_string().try_into().unwrap(); + + let mut initiator_state = State::initialized(HandshakeRole::Initiator( + Initiator::from_raw_k(authority_public_k.into_bytes()).unwrap(), + )); + let mut responder_state = State::initialized(HandshakeRole::Responder( + Responder::from_authority_kp( + &authority_public_k.into_bytes(), + &authority_private_k.into_bytes(), + CERT_VALIDITY, + ) + .unwrap(), + )); + + let first_message: [u8; ELLSWIFT_ENCODING_SIZE] = initiator_state + .step_0() + .unwrap() + .get_payload_when_handshaking() + .try_into() + .unwrap(); + let (second_message, responder_state) = responder_state.step_1(first_message).unwrap(); + let second_message: [u8; INITIATOR_EXPECTED_HANDSHAKE_MESSAGE_SIZE] = second_message + .get_payload_when_handshaking() + .try_into() + .unwrap(); + let initiator_state = initiator_state.step_2(second_message).unwrap(); + + let (mut initiator_enc, mut initiator_dec) = initiator_state.split_transport().unwrap(); + let (mut responder_enc, mut responder_dec) = responder_state.split_transport().unwrap(); + let mut encoder = NoiseEncoder::::new(); + + // Each half keeps its own cipher and nonce counter, so the two sides only stay in step + // across repeated frames if every frame is sealed and opened by the matching direction. + for nonce in 0..8u16 { + assert_eq!( + round_trip(&mut encoder, &mut initiator_enc, &mut responder_dec, nonce), + nonce + ); + assert_eq!( + round_trip( + &mut encoder, + &mut responder_enc, + &mut initiator_dec, + nonce + 100 + ), + nonce + 100 + ); + } + } #[test] fn handshake_step_fails_if_state_is_not_initialized() { diff --git a/sv2/noise-sv2/src/handshake.rs b/sv2/noise-sv2/src/handshake.rs index b66c340c93..cd9ffbf7f5 100644 --- a/sv2/noise-sv2/src/handshake.rs +++ b/sv2/noise-sv2/src/handshake.rs @@ -109,8 +109,9 @@ pub trait HandshakeOp: CipherState { Self::generate_key_with_rng(&mut rand::thread_rng()) } - // The `CryptoRng` bound rejects generators that are not suitable for cryptographic use, such - // as deterministic or weak ones. + // The `CryptoRng` bound requires generators that declare themselves suitable for cryptographic + // use. Seed quality and correct `CryptoRng` implementations remain the caller's + // responsibility. #[inline] fn generate_key_with_rng(rng: &mut R) -> Keypair { let secp = Secp256k1::new(); diff --git a/sv2/noise-sv2/src/initiator.rs b/sv2/noise-sv2/src/initiator.rs index 750f3d8ce7..eb3ea16b27 100644 --- a/sv2/noise-sv2/src/initiator.rs +++ b/sv2/noise-sv2/src/initiator.rs @@ -58,8 +58,8 @@ use secp256k1::{ /// Manages the initiator's role in the Noise NX handshake, handling key exchange, encryption, and /// handshake state. It securely generates and manages cryptographic keys, performs Diffie-Hellman /// exchanges, and maintains the handshake hash, chaining key, and nonce for message encryption. -/// After the handshake, it facilitates secure communication using either [`ChaCha20Poly1305`] or -/// `AES-GCM` ciphers. Sensitive data is securely erased when no longer needed. +/// After the handshake, it facilitates secure communication using [`ChaCha20Poly1305`]. Sensitive +/// data is securely erased when no longer needed. pub struct Initiator { // Cipher used for encrypting and decrypting messages during the handshake. // From 79c26de96dad2a0401cdd2f84049165ad6234af9 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Tue, 11 Aug 2026 20:00:09 +0530 Subject: [PATCH 13/21] noise_sv2: wipe key material derived during the handshake erase covered ck and h, but the session keys, the HKDF pseudorandom key and the ECDH secrets were left in their stack slots. --- sv2/noise-sv2/src/aed_cipher.rs | 8 ++++--- sv2/noise-sv2/src/cipher_state.rs | 8 ++++--- sv2/noise-sv2/src/handshake.rs | 36 ++++++++++++++++++++++++------- sv2/noise-sv2/src/initiator.rs | 18 ++++++++++------ sv2/noise-sv2/src/lib.rs | 14 ++++++++++++ sv2/noise-sv2/src/responder.rs | 18 ++++++++++------ 6 files changed, 76 insertions(+), 26 deletions(-) diff --git a/sv2/noise-sv2/src/aed_cipher.rs b/sv2/noise-sv2/src/aed_cipher.rs index 4908831be6..a42e118a92 100644 --- a/sv2/noise-sv2/src/aed_cipher.rs +++ b/sv2/noise-sv2/src/aed_cipher.rs @@ -21,7 +21,7 @@ use chacha20poly1305::{ aead::{Buffer, Error}, - AeadInPlace, ChaCha20Poly1305, ChaChaPoly1305, KeyInit, + AeadInPlace, ChaCha20Poly1305, ChaChaPoly1305, Key, KeyInit, }; // Defines the interface for AEAD ciphers. @@ -67,8 +67,10 @@ pub trait AeadCipher { } impl AeadCipher for ChaCha20Poly1305 { - fn from_key(k: [u8; 32]) -> Self { - ChaChaPoly1305::new(&k.into()) + fn from_key(mut k: [u8; 32]) -> Self { + let cipher = ChaChaPoly1305::new(Key::from_slice(&k[..])); + crate::zeroize_bytes(&mut k); + cipher } fn encrypt( diff --git a/sv2/noise-sv2/src/cipher_state.rs b/sv2/noise-sv2/src/cipher_state.rs index 3bb87171f2..904bc48419 100644 --- a/sv2/noise-sv2/src/cipher_state.rs +++ b/sv2/noise-sv2/src/cipher_state.rs @@ -161,12 +161,14 @@ pub struct Cipher { // and non-`Copy`, even though the key and nonce are simple types. impl Cipher { // Internal use only, we need k for handshake - pub fn from_key_and_cipher(k: [u8; 32], c: C) -> Self { - Self { + pub fn from_key_and_cipher(mut k: [u8; 32], c: C) -> Self { + let state = Self { k: Some(k), n: 0, cipher: Some(c), - } + }; + crate::zeroize_bytes(&mut k); + state } // Encrypts data in place using an empty additional associated data buffer. diff --git a/sv2/noise-sv2/src/handshake.rs b/sv2/noise-sv2/src/handshake.rs index cd9ffbf7f5..fbd38fe7f3 100644 --- a/sv2/noise-sv2/src/handshake.rs +++ b/sv2/noise-sv2/src/handshake.rs @@ -33,7 +33,8 @@ use alloc::{string::String, vec::Vec}; use crate::{ - aed_cipher::AeadCipher, cipher_state::CipherState, AeadError, NOISE_HASHED_PROTOCOL_NAME_CHACHA, + aed_cipher::AeadCipher, cipher_state::CipherState, zeroize_bytes, AeadError, + NOISE_HASHED_PROTOCOL_NAME_CHACHA, }; use chacha20poly1305::ChaCha20Poly1305; use secp256k1::{ @@ -145,16 +146,26 @@ pub trait HandshakeOp: CipherState { opad[i] = key[i] ^ 0x5c; } - let mut to_hash = Vec::with_capacity(64 + data.len()); + // Sized for the larger of the two rounds (`opad` + a 32-byte digest) so neither one + // reallocates and abandons a copy of the padded key on the heap. + let mut to_hash = Vec::with_capacity(96 + data.len()); to_hash.extend_from_slice(&ipad); to_hash.extend_from_slice(data); - let temp = Sha256Hash::hash(&to_hash).to_byte_array(); + let mut temp = Sha256Hash::hash(&to_hash).to_byte_array(); + // Wiped before `clear`, while the length still spans the padded key. + zeroize_bytes(&mut to_hash); to_hash.clear(); to_hash.extend_from_slice(&opad); to_hash.extend_from_slice(&temp); - Sha256Hash::hash(&to_hash).to_byte_array() + let out = Sha256Hash::hash(&to_hash).to_byte_array(); + + zeroize_bytes(&mut ipad); + zeroize_bytes(&mut opad); + zeroize_bytes(&mut to_hash); + zeroize_bytes(&mut temp); + out } // Derives two new keys using the HKDF (HMAC-based Key Derivation Function) process. @@ -172,9 +183,11 @@ pub trait HandshakeOp: CipherState { // specific byte sequence (`0x02`). // 4. Returns both outputs. fn hkdf_2(chaining_key: &[u8; 32], input_key_material: &[u8]) -> ([u8; 32], [u8; 32]) { - let temp_key = Self::hmac_hash(chaining_key, input_key_material); + let mut temp_key = Self::hmac_hash(chaining_key, input_key_material); let out_1 = Self::hmac_hash(&temp_key, &[0x1]); let out_2 = Self::hmac_hash(&temp_key, &[&out_1[..], &[0x2][..]].concat()); + // Both outputs are recoverable from the pseudorandom key, so it must not outlive them. + zeroize_bytes(&mut temp_key); (out_1, out_2) } @@ -183,10 +196,11 @@ pub trait HandshakeOp: CipherState { chaining_key: &[u8; 32], input_key_material: &[u8], ) -> ([u8; 32], [u8; 32], [u8; 32]) { - let temp_key = Self::hmac_hash(chaining_key, input_key_material); + let mut temp_key = Self::hmac_hash(chaining_key, input_key_material); let out_1 = Self::hmac_hash(&temp_key, &[0x1]); let out_2 = Self::hmac_hash(&temp_key, &[&out_1[..], &[0x2][..]].concat()); let out_3 = Self::hmac_hash(&temp_key, &[&out_2[..], &[0x3][..]].concat()); + zeroize_bytes(&mut temp_key); (out_1, out_2, out_3) } @@ -199,9 +213,13 @@ pub trait HandshakeOp: CipherState { // use in the next step of the handshake. fn mix_key(&mut self, input_key_material: &[u8]) { let ck = self.get_ck(); - let (ck, temp_k) = Self::hkdf_2(ck, input_key_material); + let (mut ck, mut temp_k) = Self::hkdf_2(ck, input_key_material); self.set_ck(ck); self.initialize_key(temp_k); + // `set_ck` and `initialize_key` copy the material they are handed, so the locals holding + // the originals are wiped here. + zeroize_bytes(&mut ck); + zeroize_bytes(&mut temp_k); } // Encrypts the provided plaintext and updates the hash `h` value. @@ -269,7 +287,7 @@ pub trait HandshakeOp: CipherState { // Resets the nonce (`n`) to 0 and initializes the handshake cipher using the given 32-byte // encryption key. It also updates the internal key storage (`k`) with the new key, preparing // the cipher for encrypting or decrypting subsequent messages in the handshake. - fn initialize_key(&mut self, key: [u8; 32]) { + fn initialize_key(&mut self, mut key: [u8; 32]) { self.set_n(0); let cipher = ChaCha20Poly1305::from_key(key); self.set_handshake_cipher(cipher); @@ -279,6 +297,8 @@ pub trait HandshakeOp: CipherState { let set_k = self.get_k(); *set_k = Some(key); } + // The cipher and `k` hold their own copies now, so this one is wiped. + zeroize_bytes(&mut key); } fn set_handshake_cipher(&mut self, cipher: ChaCha20Poly1305); diff --git a/sv2/noise-sv2/src/initiator.rs b/sv2/noise-sv2/src/initiator.rs index eb3ea16b27..8595716d5d 100644 --- a/sv2/noise-sv2/src/initiator.rs +++ b/sv2/noise-sv2/src/initiator.rs @@ -49,7 +49,7 @@ use crate::{ ENCRYPTED_SIGNATURE_NOISE_MESSAGE_SIZE, INITIATOR_EXPECTED_HANDSHAKE_MESSAGE_SIZE, SIGNATURE_NOISE_MESSAGE_SIZE, }; -use chacha20poly1305::{ChaCha20Poly1305, KeyInit}; +use chacha20poly1305::{ChaCha20Poly1305, Key, KeyInit}; use secp256k1::{ ellswift::{ElligatorSwift, ElligatorSwiftParty}, Keypair, PublicKey, XOnlyPublicKey, @@ -320,7 +320,7 @@ impl Initiator { let elligatorswift_ours_ephemeral = ElligatorSwift::from_pubkey(self.e.public_key()); let elligatorswift_theirs_ephemeral = ElligatorSwift::from_array(elliswift_theirs_ephemeral_serialized); - let ecdh_ephemeral: [u8; 32] = ElligatorSwift::shared_secret( + let mut ecdh_ephemeral: [u8; 32] = ElligatorSwift::shared_secret( elligatorswift_ours_ephemeral, elligatorswift_theirs_ephemeral, e_private_key, @@ -329,6 +329,7 @@ impl Initiator { ) .to_secret_bytes(); self.mix_key(&ecdh_ephemeral); + crate::zeroize_bytes(&mut ecdh_ephemeral); // 5. decrypts next 80 bytes with `DecryptAndHash()` and stores the results as // `rs.public_key` which is **server's static public key** (note that 64 bytes is the @@ -344,7 +345,7 @@ impl Initiator { .expect("slice with incorrect length"); let elligatorswift_theirs_static = ElligatorSwift::from_array(elligatorswift_theirs_static_serialized); - let ecdh_static: [u8; 32] = ElligatorSwift::shared_secret( + let mut ecdh_static: [u8; 32] = ElligatorSwift::shared_secret( elligatorswift_ours_ephemeral, elligatorswift_theirs_static, e_private_key, @@ -353,6 +354,7 @@ impl Initiator { ) .to_secret_bytes(); self.mix_key(&ecdh_static); + crate::zeroize_bytes(&mut ecdh_static); // Decrypt and verify the SignatureNoiseMessage let mut to_decrypt = message[ELLSWIFT_ENCODING_SIZE + ENCRYPTED_ELLSWIFT_ENCODING_SIZE @@ -371,15 +373,19 @@ impl Initiator { .serialize(); let rs_pk_xonly = XOnlyPublicKey::from_slice(&rs_pub_key).unwrap(); if signature_message.verify_with_now(&rs_pk_xonly, &self.responder_authority_pk, now) { - let (temp_k1, temp_k2) = Self::hkdf_2(self.get_ck(), &[]); - let c1 = ChaCha20Poly1305::new(&temp_k1.into()); - let c2 = ChaCha20Poly1305::new(&temp_k2.into()); + let (mut temp_k1, mut temp_k2) = Self::hkdf_2(self.get_ck(), &[]); + let c1 = ChaCha20Poly1305::new(Key::from_slice(&temp_k1[..])); + let c2 = ChaCha20Poly1305::new(Key::from_slice(&temp_k2[..])); let c1: Cipher = Cipher::from_key_and_cipher(temp_k1, c1); let c2: Cipher = Cipher::from_key_and_cipher(temp_k2, c2); let mut encryptor = c1; let mut decryptor = c2; encryptor.erase_k(); decryptor.erase_k(); + // The ciphers keep the only copies of the session keys from here on; `erase_k` above + // wiped the copies the `Cipher`s carried, and these are the ones the handshake used. + crate::zeroize_bytes(&mut temp_k1); + crate::zeroize_bytes(&mut temp_k2); let engine = crate::NoiseEngine { encryptor, decryptor, diff --git a/sv2/noise-sv2/src/lib.rs b/sv2/noise-sv2/src/lib.rs index 6e7b3ac615..490c51eacd 100644 --- a/sv2/noise-sv2/src/lib.rs +++ b/sv2/noise-sv2/src/lib.rs @@ -99,6 +99,20 @@ pub const NOISE_HASHED_PROTOCOL_NAME_CHACHA: [u8; 32] = [ // In this case, `Parity::Even` is used. const PARITY: secp256k1::Parity = secp256k1::Parity::Even; +// Overwrites `bytes` with zeros, using volatile writes so that the compiler cannot elide the wipe +// as a dead store. +// +// Wiping a stack slot is best effort: the optimizer is free to keep the same material in registers +// or in spilled copies that this cannot reach, and intermediate heap buffers (such as the ones +// `HandshakeOp::hmac_hash` builds) are outside of its reach as well. It still removes the +// longest-lived copy of the material, which is the one that survives in the frame after the +// handshake has finished. +pub(crate) fn zeroize_bytes(bytes: &mut [u8]) { + for b in bytes { + unsafe { core::ptr::write_volatile(b, 0) }; + } +} + /// An engine for managing encrypted communication in the Noise protocol. /// /// Manages the encryption and decryption of messages between two parties, the [`Initiator`] and diff --git a/sv2/noise-sv2/src/responder.rs b/sv2/noise-sv2/src/responder.rs index c85f2b5683..315ba2de6e 100644 --- a/sv2/noise-sv2/src/responder.rs +++ b/sv2/noise-sv2/src/responder.rs @@ -49,7 +49,7 @@ use alloc::{ string::{String, ToString}, vec::Vec, }; -use chacha20poly1305::{ChaCha20Poly1305, KeyInit}; +use chacha20poly1305::{ChaCha20Poly1305, Key, KeyInit}; use secp256k1::{ellswift::ElligatorSwift, Keypair, Secp256k1, SecretKey}; const VERSION: u16 = 0; @@ -310,7 +310,7 @@ impl Responder { let e_private_key = keypair.secret_key(); let elligatorswift_theirs_ephemeral = ElligatorSwift::from_array(elligatorswift_theirs_ephemeral_serialized); - let ecdh_ephemeral = ElligatorSwift::shared_secret( + let mut ecdh_ephemeral = ElligatorSwift::shared_secret( elligatorswift_theirs_ephemeral, elligatorswitf_ours_ephemeral, e_private_key, @@ -319,6 +319,7 @@ impl Responder { ) .to_secret_bytes(); Self::mix_key(self, &ecdh_ephemeral); + crate::zeroize_bytes(&mut ecdh_ephemeral); // 5. appends `EncryptAndHash(s.public_key)` (64 bytes encrypted elligatorswift public key, // 16 bytes MAC) @@ -335,7 +336,7 @@ impl Responder { // 6. calls `MixKey(ECDH(s.private_key, re.public_key))` let s_private_key = self.s.secret_key(); - let ecdh_static = ElligatorSwift::shared_secret( + let mut ecdh_static = ElligatorSwift::shared_secret( elligatorswift_theirs_ephemeral, elligatorswift_ours_static, s_private_key, @@ -344,6 +345,7 @@ impl Responder { ) .to_secret_bytes(); Self::mix_key(self, &ecdh_static[..]); + crate::zeroize_bytes(&mut ecdh_static); // 7. appends `EncryptAndHash(SIGNATURE_NOISE_MESSAGE)` to the buffer let valid_from = now; @@ -360,9 +362,9 @@ impl Responder { // 9. return pair of CipherState objects, the first for encrypting transport messages from // initiator to responder, and the second for messages in the other direction: let ck = Self::get_ck(self); - let (temp_k1, temp_k2) = Self::hkdf_2(ck, &[]); - let c1 = ChaCha20Poly1305::new(&temp_k1.into()); - let c2 = ChaCha20Poly1305::new(&temp_k2.into()); + let (mut temp_k1, mut temp_k2) = Self::hkdf_2(ck, &[]); + let c1 = ChaCha20Poly1305::new(Key::from_slice(&temp_k1[..])); + let c2 = ChaCha20Poly1305::new(Key::from_slice(&temp_k2[..])); let c1: Cipher = Cipher::from_key_and_cipher(temp_k1, c1); let c2: Cipher = Cipher::from_key_and_cipher(temp_k2, c2); let to_send = out; @@ -370,6 +372,10 @@ impl Responder { let mut decryptor = c1; encryptor.erase_k(); decryptor.erase_k(); + // The ciphers keep the only copies of the session keys from here on; `erase_k` above wiped + // the copies the `Cipher`s carried, and these are the ones the handshake used. + crate::zeroize_bytes(&mut temp_k1); + crate::zeroize_bytes(&mut temp_k2); let engine = crate::NoiseEngine { encryptor, decryptor, From e4cff37745736b7a3727f0e8ffa3182c6acefb00 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Tue, 11 Aug 2026 20:00:09 +0530 Subject: [PATCH 14/21] noise_sv2: correct the cipher thread-safety rationale Cipher and NoiseEngine are Send + Sync. Nonce uniqueness comes from &mut exclusivity and the missing Clone, not from the absence of Sync. --- sv2/noise-sv2/src/cipher_state.rs | 17 +++++++---------- sv2/noise-sv2/src/initiator.rs | 14 +++----------- sv2/noise-sv2/src/responder.rs | 15 +++------------ 3 files changed, 13 insertions(+), 33 deletions(-) diff --git a/sv2/noise-sv2/src/cipher_state.rs b/sv2/noise-sv2/src/cipher_state.rs index 904bc48419..31dad966dc 100644 --- a/sv2/noise-sv2/src/cipher_state.rs +++ b/sv2/noise-sv2/src/cipher_state.rs @@ -148,17 +148,14 @@ pub struct Cipher { cipher: Option, } -// Ensures that the `Cipher` type is not `Sync`, which prevents multiple threads from -// simultaneously accessing the same instance of `Cipher`. This eliminates the need to handle -// potential issues related to visibility of changes across threads. +// Nonce uniqueness rests on exclusive access, not on thread affinity: `Cipher` is both `Send` and +// `Sync`, so sharing one instance across threads is allowed and still requires the caller to +// synchronize. What prevents two encryptions from reusing a nonce is that `encrypt`/`decrypt` take +// `&mut self` and that the type is deliberately not `Clone`, so the key and its nonce counter can +// never be duplicated or advanced from two places at once. // -// After sending the `k` value, we immediately clear it to prevent the original thread from -// accessing the value again, thereby enhancing security by ensuring the sensitive data is no -// longer available in memory. -// -// The `Cipher` struct is neither `Sync` nor `Copy` due to its `cipher` field, which implements -// the `AeadCipher` trait. This trait requires mutable access, making the entire struct non-`Sync` -// and non-`Copy`, even though the key and nonce are simple types. +// The handshake key `k` is cleared as soon as the handshake no longer needs it (see `erase_k`), +// so it does not outlive its use even though the cipher itself lives for the whole session. impl Cipher { // Internal use only, we need k for handshake pub fn from_key_and_cipher(mut k: [u8; 32], c: C) -> Self { diff --git a/sv2/noise-sv2/src/initiator.rs b/sv2/noise-sv2/src/initiator.rs index 8595716d5d..eecdc9761c 100644 --- a/sv2/noise-sv2/src/initiator.rs +++ b/sv2/noise-sv2/src/initiator.rs @@ -93,17 +93,9 @@ impl core::fmt::Debug for Initiator { } } -// Ensures that the `Cipher` type is not `Sync`, which prevents multiple threads from -// simultaneously accessing the same instance of `Cipher`. This eliminates the need to handle -// potential issues related to visibility of changes across threads. -// -// After sending the `k` value, we immediately clear it to prevent the original thread from -// accessing the value again, thereby enhancing security by ensuring the sensitive data is no -// longer available in memory. -// -// The `Cipher` struct is neither `Sync` nor `Copy` due to its `cipher` field, which implements -// the `AeadCipher` trait. This trait requires mutable access, making the entire struct non-`Sync` -// and non-`Copy`, even though the key and nonce are simple types. +// Every handshake encryption goes through `encrypt_with_ad`, which takes `&mut self`, and +// `Initiator` is deliberately not `Clone`, so the handshake key and its nonce counter cannot be +// duplicated or advanced from two places at once. See `Cipher` for the transport-mode counterpart. impl CipherState for Initiator { fn get_k(&mut self) -> &mut Option<[u8; 32]> { &mut self.k diff --git a/sv2/noise-sv2/src/responder.rs b/sv2/noise-sv2/src/responder.rs index 315ba2de6e..0705b11836 100644 --- a/sv2/noise-sv2/src/responder.rs +++ b/sv2/noise-sv2/src/responder.rs @@ -98,18 +98,9 @@ impl core::fmt::Debug for Responder { } } -// Ensures that the `Cipher` type is not `Sync`, which prevents multiple threads from -// simultaneously accessing the same instance of `Cipher`. This eliminates the need to handle -// potential issues related to visibility of changes across threads. -// -// After sending the `k` value, we immediately clear it to prevent the original thread from -// accessing the value again, thereby enhancing security by ensuring the sensitive data is no -// longer available in memory. -// -// The `Cipher` struct is neither `Sync` nor `Copy` due to its `cipher` field, which implements -// the `AeadCipher` trait. This trait requires mutable access, making the entire struct non-`Sync` -// and non-`Copy`, even though the key and nonce are simple types. - +// Every handshake encryption goes through `encrypt_with_ad`, which takes `&mut self`, and +// `Responder` is deliberately not `Clone`, so the handshake key and its nonce counter cannot be +// duplicated or advanced from two places at once. See `Cipher` for the transport-mode counterpart. impl CipherState for Responder { fn get_k(&mut self) -> &mut Option<[u8; 32]> { &mut self.k From c0fe88e04886b50773f344bd6d4a49054b119826 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Tue, 11 Aug 2026 20:00:09 +0530 Subject: [PATCH 15/21] noise_sv2: describe what erase actually wipes It never touched handshake_cipher, which relies on ChaCha20Poly1305 zeroizing its own key on drop. --- sv2/noise-sv2/src/initiator.rs | 12 ++++++++---- sv2/noise-sv2/src/responder.rs | 13 +++++++++---- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/sv2/noise-sv2/src/initiator.rs b/sv2/noise-sv2/src/initiator.rs index eecdc9761c..771748a81e 100644 --- a/sv2/noise-sv2/src/initiator.rs +++ b/sv2/noise-sv2/src/initiator.rs @@ -390,15 +390,19 @@ impl Initiator { // Securely erases sensitive data from the [`Initiator`] memory. // - // Clears all sensitive cryptographic material within the [`Initiator`] to prevent any - // accidental leakage or misuse. It overwrites the stored keys, chaining key, handshake hash, - // and session ciphers with zeros. This method is typically - // called when the [`Initiator`] instance is no longer needed or before deallocation. + // Volatile-writes zeros over the handshake key (`k`), the chaining key (`ck`) and the handshake + // hash (`h`), and non-securely erases the ephemeral keypair. This method is typically called + // when the [`Initiator`] instance is no longer needed or before deallocation. + // + // It does not touch `handshake_cipher`: that key is wiped by [`ChaCha20Poly1305`]'s own + // zeroize-on-drop. Nor does it reach the transport session ciphers, which by then live in the + // [`NoiseEngine`] returned by [`Self::step_2`] and are wiped when that is dropped. fn erase(&mut self) { if let Some(k) = self.k.as_mut() { for b in k { unsafe { ptr::write_volatile(b, 0) }; } + self.k = None; } for b in &mut self.ck { unsafe { ptr::write_volatile(b, 0) }; diff --git a/sv2/noise-sv2/src/responder.rs b/sv2/noise-sv2/src/responder.rs index 0705b11836..f480684915 100644 --- a/sv2/noise-sv2/src/responder.rs +++ b/sv2/noise-sv2/src/responder.rs @@ -408,15 +408,20 @@ impl Responder { // Securely erases sensitive data in the responder's memory. // - // Clears all sensitive cryptographic material within the [`Responder`] to prevent any - // accidental leakage or misuse. It overwrites the stored keys, chaining key, handshake hash, - // and session ciphers with zeros. This function is typically - // called when the [`Responder`] instance is no longer needed or before deallocation. + // Volatile-writes zeros over the handshake key (`k`), the chaining key (`ck`) and the handshake + // hash (`h`), and non-securely erases the ephemeral, static and authority keypairs. This + // function is typically called when the [`Responder`] instance is no longer needed or before + // deallocation. + // + // It does not touch `handshake_cipher`: that key is wiped by [`ChaCha20Poly1305`]'s own + // zeroize-on-drop. Nor does it reach the transport session ciphers, which by then live in the + // [`NoiseEngine`] returned by [`Self::step_1`] and are wiped when that is dropped. fn erase(&mut self) { if let Some(k) = self.k.as_mut() { for b in k { unsafe { ptr::write_volatile(b, 0) }; } + self.k = None; } for b in &mut self.ck { unsafe { ptr::write_volatile(b, 0) }; From 4c8df862050624cd0890c2776484131eedb67e4a Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Tue, 11 Aug 2026 20:00:09 +0530 Subject: [PATCH 16/21] codec_sv2: leave the noise encoder usable after a failed encryption A failed chunk returned past the offset reset, so the next frame encoded at a stale start behind the remains of this one. --- sv2/codec-sv2/src/encoder.rs | 90 ++++++++++++++++++++++++++++++++++-- 1 file changed, 87 insertions(+), 3 deletions(-) diff --git a/sv2/codec-sv2/src/encoder.rs b/sv2/codec-sv2/src/encoder.rs index b873611588..a9b0482096 100644 --- a/sv2/codec-sv2/src/encoder.rs +++ b/sv2/codec-sv2/src/encoder.rs @@ -158,9 +158,31 @@ impl WithNoise { Ok(self.noise_buffer.get_data_owned()) } - // Serializes `item` into an Sv2 frame and encrypts it in place through `encrypt`. + // Serializes `item` into an Sv2 frame and encrypts it in place through `encrypt`, leaving the + // buffers clean if any step of that fails. #[inline] fn encrypt_frame( + &mut self, + item: Item, + encrypt: impl FnMut(&mut B) -> Result<()>, + ) -> Result<()> { + let result = self.try_encrypt_frame(item, encrypt); + + if result.is_err() { + // A failure part way through leaves the write offset at the chunk that failed, and the + // bytes written so far in the buffer. Since the encoder is reusable, both have to be + // undone here: otherwise the next frame is written at a stale offset with the remains + // of this one in front of it, and the peer gets a frame it can never open. + self.noise_buffer.danger_set_start(0); + self.noise_buffer.get_data_owned(); + self.sv2_buffer.get_data_owned(); + } + + result + } + + #[inline] + fn try_encrypt_frame( &mut self, item: Item, mut encrypt: impl FnMut(&mut B) -> Result<()>, @@ -322,7 +344,10 @@ mod prop_tests { use super::*; #[cfg(feature = "noise_sv2")] use crate::{HandshakeRole, State}; - use binary_sv2::Serialize; + use binary_sv2::{Deserialize, Serialize}; + // The glob import above brings in `crate::Result`, whose single type parameter the code + // generated by the `Deserialize` derive cannot use. + use core::result::Result; #[cfg(feature = "noise_sv2")] use framing_sv2::framing::Frame; use framing_sv2::framing::Sv2Frame; @@ -344,7 +369,7 @@ mod prop_tests { type Slice = ::Slice; - #[derive(Debug, Clone, Serialize, PartialEq, Eq)] + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] struct TestMessage { value: u16, } @@ -476,6 +501,65 @@ mod prop_tests { } } + /// Verifies that a chunk encryption failure does not leave the encoder writing the next frame + /// at the offset the failing chunk was using, nor with the bytes of the failed frame still in + /// the buffer. + #[cfg(feature = "noise_sv2")] + #[test] + fn noise_encoder_recovers_from_a_failed_chunk_encryption() { + let frame = Frame::Sv2( + Sv2Frame::::from_message(TestMessage { value: 1 }, 0, 0, false) + .unwrap(), + ); + + let mut encoder = NoiseEncoder::::new(); + + // Let the header through and fail on the first payload chunk: that is the point where the + // write offset has already been moved. + let mut calls = 0; + let result = encoder.encrypt_frame(frame, |buf| { + calls += 1; + if calls == 1 { + // Stand in for the header encryption, which grows the buffer by a MAC. + buf.get_writable(AEAD_MAC_LEN); + Ok(()) + } else { + Err(crate::Error::UnexpectedNoiseState) + } + }); + assert!(result.is_err()); + assert_eq!(IsBuffer::len(&encoder.noise_buffer), 0); + assert!(encoder.noise_buffer.as_ref().is_empty()); + + // The next frame must be one the peer can open, with nothing of the failed one in it. + let (mut sender_state, mut receiver_state) = make_transport_state_pair(); + let next = Frame::Sv2( + Sv2Frame::::from_message(TestMessage { value: 2 }, 0, 0, false) + .unwrap(), + ); + let encrypted = encoder.encode(next, &mut sender_state).unwrap(); + + let mut decoder = crate::StandardNoiseDecoder::::new(); + let mut offset = 0; + let mut decoded = loop { + let writable = decoder.writable(); + let len = writable.len(); + writable.copy_from_slice(&encrypted.as_ref()[offset..offset + len]); + offset += len; + + match decoder.next_frame(&mut receiver_state) { + Ok(Frame::Sv2(f)) => break f, + Ok(_) => panic!("expected an Sv2 frame"), + Err(crate::Error::MissingBytes(_)) => {} + Err(e) => panic!("failed to decode the frame after a failed encode: {e:?}"), + } + }; + assert_eq!( + binary_sv2::from_bytes::(decoded.payload()).unwrap(), + TestMessage { value: 2 } + ); + } + /// Verifies that `NoiseEncoder` can encrypt multiple frames /// sequentially with the same transport state without errors. #[cfg(feature = "noise_sv2")] From b1b965705d6e70ef04b7afe276d7fc094acdddec Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Tue, 11 Aug 2026 20:00:09 +0530 Subject: [PATCH 17/21] codec_sv2: leave the noise decoder usable after a failed decryption The same offset leak as the encoder. MissingBytes is normal flow and keeps its state. --- sv2/codec-sv2/src/decoder.rs | 69 ++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/sv2/codec-sv2/src/decoder.rs b/sv2/codec-sv2/src/decoder.rs index a78a7db378..ecf7da95ed 100644 --- a/sv2/codec-sv2/src/decoder.rs +++ b/sv2/codec-sv2/src/decoder.rs @@ -272,6 +272,31 @@ impl<'a, T: Serialize + GetSize + Deserialize<'a>, B: IsBuffer + AeadBuffer> Wit // decryption process completes and the frame can be successfully decoded. #[inline] fn decode_noise_frame( + &mut self, + decrypt: impl FnMut(&mut B) -> Result<()>, + ) -> Result> { + let result = self.try_decode_noise_frame(decrypt); + + match &result { + // `MissingBytes` is the normal way out of the header round: the header has been + // decrypted into `sv2_buffer` and the payload is still on its way, so the buffer has to + // be left exactly as it is. + Err(Error::MissingBytes(_)) | Ok(_) => {} + Err(_) => { + // Any other failure leaves the decrypt offset at the chunk that failed and the + // bytes decrypted so far in the buffer. A decoder is kept for the life of a + // connection, so the next frame would be decrypted at that stale offset and then + // read as a frame starting in the middle of this one's plaintext. + self.sv2_buffer.danger_set_start(0); + self.sv2_buffer.get_data_owned(); + } + } + + result + } + + #[inline] + fn try_decode_noise_frame( &mut self, mut decrypt: impl FnMut(&mut B) -> Result<()>, ) -> Result> { @@ -817,6 +842,50 @@ mod prop_tests { } } + /// Verifies that a failed decryption does not leave the decoder decrypting the next frame at + /// the offset the failing chunk was using, nor with the plaintext of the failed frame still in + /// the buffer. + #[cfg(feature = "noise_sv2")] + #[test] + fn noise_decoder_recovers_from_a_failed_decryption() { + let (mut sender_state, mut receiver_state) = make_transport_state_pair(); + let frame = Frame::Sv2( + Sv2Frame::::from_message(TestMessage { value: 7 }, 0, 0, false) + .unwrap(), + ); + let mut encoder = NoiseEncoder::::new(); + let encrypted = encoder.encode(frame, &mut sender_state).unwrap(); + let encrypted: &[u8] = encrypted.as_ref(); + + let mut decoder = StandardNoiseDecoder::::new(); + + // Fail on the encrypted header. The closure never touches `receiver_state`, so its nonce + // stays where it was and the same bytes can be replayed below. + let hint = decoder + .next_transport(|_| Err(crate::Error::UnexpectedNoiseState)) + .unwrap_err(); + assert!(matches!(hint, crate::Error::MissingBytes(_))); + let writable = decoder.writable(); + let len = writable.len(); + writable.copy_from_slice(&encrypted[..len]); + let failed = decoder + .next_transport(|_| Err(crate::Error::UnexpectedNoiseState)) + .unwrap_err(); + assert!(matches!(failed, crate::Error::UnexpectedNoiseState)); + assert_eq!(IsBuffer::len(&decoder.sv2_buffer), 0); + assert!(decoder.sv2_buffer.as_ref().is_empty()); + + // The same decoder must now decode the frame from the start. + let decoded = decode_noise_frame(&mut decoder, &mut receiver_state, encrypted); + match decoded { + Some(mut f) => assert_eq!( + binary_sv2::from_bytes::(f.payload()).unwrap(), + TestMessage { value: 7 } + ), + None => panic!("failed to decode the frame after a failed decryption"), + } + } + /// Verifies that a single `StandardNoiseDecoder` instance correctly /// decodes two consecutive noise-encrypted frames in sequence using /// the same shared transport state. From 91c1e8ed8a2e69da6a07f108ce4a62b93d0728e5 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Tue, 11 Aug 2026 20:00:09 +0530 Subject: [PATCH 18/21] codec_sv2: let callers check for transport mode before splitting split_transport consumes the state on its error path too. --- sv2/codec-sv2/src/lib.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/sv2/codec-sv2/src/lib.rs b/sv2/codec-sv2/src/lib.rs index a582cd5425..07b0ddbee6 100644 --- a/sv2/codec-sv2/src/lib.rs +++ b/sv2/codec-sv2/src/lib.rs @@ -316,7 +316,19 @@ impl State { Self::Transport(tm) } + /// Returns whether this state is in transport mode, and so can be split into its two halves. + /// + /// [`Self::split_transport`] consumes the state on its error path as well, so this is the way + /// to check before committing to the call. + pub fn is_transport(&self) -> bool { + matches!(self, Self::Transport(_)) + } + /// Splits a transport-mode state into its encrypting and decrypting halves, consuming it. + /// + /// The state is consumed whatever the outcome: a state that is not in transport mode is dropped + /// rather than handed back, so callers that cannot guarantee the mode should check + /// [`Self::is_transport`] first. pub fn split_transport( self, ) -> core::result::Result<(TransportEncryptState, TransportDecryptState), Error> { @@ -423,6 +435,9 @@ mod tests { .unwrap(); let initiator_state = initiator_state.step_2(second_message).unwrap(); + assert!(initiator_state.is_transport()); + assert!(responder_state.is_transport()); + let (mut initiator_enc, mut initiator_dec) = initiator_state.split_transport().unwrap(); let (mut responder_enc, mut responder_dec) = responder_state.split_transport().unwrap(); let mut encoder = NoiseEncoder::::new(); @@ -446,6 +461,16 @@ mod tests { } } + #[test] + fn is_transport_reports_whether_the_state_can_be_split() { + let state = State::NotInitialized(32); + assert!(!state.is_transport()); + assert_eq!( + state.split_transport().unwrap_err(), + Error::UnexpectedNoiseState + ); + } + #[test] fn handshake_step_fails_if_state_is_not_initialized() { let mut state = State::NotInitialized(32); From a6faf64871a7221c1e812d22569f3153d1485f2e Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Tue, 11 Aug 2026 20:00:09 +0530 Subject: [PATCH 19/21] codec_sv2: cover chunking and decoder reuse in the transport tests TestMsg is single-chunk, and the decoder was rebuilt for every frame. --- sv2/codec-sv2/src/lib.rs | 98 +++++++++++++++++++++++++++++++++++----- 1 file changed, 87 insertions(+), 11 deletions(-) diff --git a/sv2/codec-sv2/src/lib.rs b/sv2/codec-sv2/src/lib.rs index 07b0ddbee6..e5b37aa3c3 100644 --- a/sv2/codec-sv2/src/lib.rs +++ b/sv2/codec-sv2/src/lib.rs @@ -352,11 +352,12 @@ mod tests { Error, HandshakeRole, NoiseEncoder, StandardEitherFrame, StandardNoiseDecoder, StandardSv2Frame, State, TransportDecryptState, TransportEncryptState, }; - use binary_sv2::{Deserialize, Serialize}; - use framing_sv2::framing::Sv2Frame; + use binary_sv2::{Deserialize, Serialize, B064K}; + use framing_sv2::{framing::Sv2Frame, SV2_FRAME_CHUNK_SIZE}; use key_utils::{Secp256k1PublicKey, Secp256k1SecretKey}; use noise_sv2::{ - Initiator, Responder, ELLSWIFT_ENCODING_SIZE, INITIATOR_EXPECTED_HANDSHAKE_MESSAGE_SIZE, + Initiator, Responder, AEAD_MAC_LEN, ELLSWIFT_ENCODING_SIZE, + INITIATOR_EXPECTED_HANDSHAKE_MESSAGE_SIZE, }; const AUTHORITY_PUBLIC_K: &str = "9auqWEzQDVyd2oe1JVGFLMLHZtCo2FFqZwtKA5gd9xbuEu7PH72"; @@ -369,10 +370,20 @@ mod tests { nonce: u16, } - // Encrypts `nonce` through `enc`, then decrypts it back through `dec` and returns the nonce - // carried by the decoded message. + // A message whose payload can be made large enough to span more than one chunk. + #[derive(Debug, Serialize, Deserialize)] + struct ChunkedMsg<'decoder> { + data: B064K<'decoder>, + } + + // Encrypts `nonce` through `enc`, then decrypts it back through `dec` and `decoder`, and + // returns the nonce carried by the decoded message. + // + // `decoder` is passed in rather than created here so that callers can reuse one decoder per + // direction across frames, the way a connection does. fn round_trip( encoder: &mut NoiseEncoder, + decoder: &mut StandardNoiseDecoder, enc: &mut TransportEncryptState, dec: &mut TransportDecryptState, nonce: u16, @@ -382,7 +393,6 @@ mod tests { ); let encrypted = encoder.encode_transport(frame, enc).unwrap(); - let mut decoder = StandardNoiseDecoder::::new(); let mut offset = 0; loop { let writable = decoder.writable(); @@ -403,8 +413,14 @@ mod tests { } } - #[test] - fn split_transport_round_trips_in_both_directions() { + // Runs a full handshake and returns both sides split into their halves, as + // `(initiator encrypt, initiator decrypt, responder encrypt, responder decrypt)`. + fn transport_halves() -> ( + TransportEncryptState, + TransportDecryptState, + TransportEncryptState, + TransportDecryptState, + ) { let authority_public_k: Secp256k1PublicKey = AUTHORITY_PUBLIC_K.to_string().try_into().unwrap(); let authority_private_k: Secp256k1SecretKey = @@ -438,20 +454,39 @@ mod tests { assert!(initiator_state.is_transport()); assert!(responder_state.is_transport()); - let (mut initiator_enc, mut initiator_dec) = initiator_state.split_transport().unwrap(); - let (mut responder_enc, mut responder_dec) = responder_state.split_transport().unwrap(); + let (initiator_enc, initiator_dec) = initiator_state.split_transport().unwrap(); + let (responder_enc, responder_dec) = responder_state.split_transport().unwrap(); + (initiator_enc, initiator_dec, responder_enc, responder_dec) + } + + #[test] + fn split_transport_round_trips_in_both_directions() { + let (mut initiator_enc, mut initiator_dec, mut responder_enc, mut responder_dec) = + transport_halves(); let mut encoder = NoiseEncoder::::new(); + // One decoder per direction, kept for every frame, as a connection would: leftover buffer + // state carries from one frame to the next here. + let mut to_responder = StandardNoiseDecoder::::new(); + let mut to_initiator = StandardNoiseDecoder::::new(); + // Each half keeps its own cipher and nonce counter, so the two sides only stay in step // across repeated frames if every frame is sealed and opened by the matching direction. for nonce in 0..8u16 { assert_eq!( - round_trip(&mut encoder, &mut initiator_enc, &mut responder_dec, nonce), + round_trip( + &mut encoder, + &mut to_responder, + &mut initiator_enc, + &mut responder_dec, + nonce + ), nonce ); assert_eq!( round_trip( &mut encoder, + &mut to_initiator, &mut responder_enc, &mut initiator_dec, nonce + 100 @@ -461,6 +496,47 @@ mod tests { } } + #[test] + fn split_transport_round_trips_a_frame_that_spans_several_chunks() { + let (mut initiator_enc, _, _, mut responder_dec) = transport_halves(); + + // The encoder seals, and the decoder opens, at most `SV2_FRAME_CHUNK_SIZE - AEAD_MAC_LEN` + // bytes of frame at a time, so a payload past that boundary is what makes both of them + // loop. Anything smaller only ever exercises the single-chunk path. + let mut data = vec![0xab; u16::MAX as usize]; + assert!(data.len() > SV2_FRAME_CHUNK_SIZE - AEAD_MAC_LEN); + let msg = ChunkedMsg { + data: (&mut data[..]).try_into().unwrap(), + }; + + let frame = StandardEitherFrame::::Sv2( + Sv2Frame::from_message(msg, MSG_TYPE, 0, false).unwrap(), + ); + let mut encoder = NoiseEncoder::::new(); + let encrypted = encoder.encode_transport(frame, &mut initiator_enc).unwrap(); + + let mut decoder = StandardNoiseDecoder::::new(); + let mut offset = 0; + loop { + let writable = decoder.writable(); + let len = writable.len(); + writable.copy_from_slice(&encrypted[offset..offset + len]); + offset += len; + + match decoder.next_transport_frame(&mut responder_dec) { + Ok(frame) => { + let mut frame: StandardSv2Frame = frame.try_into().unwrap(); + assert_eq!(frame.get_header().unwrap().msg_type(), MSG_TYPE); + let decoded: ChunkedMsg = binary_sv2::from_bytes(frame.payload()).unwrap(); + assert_eq!(decoded.data.as_bytes(), &vec![0xab; u16::MAX as usize][..]); + break; + } + Err(Error::MissingBytes(_)) => {} + Err(e) => panic!("failed to decode a chunked transport frame: {e:?}"), + } + } + } + #[test] fn is_transport_reports_whether_the_state_can_be_split() { let state = State::NotInitialized(32); From 4c2d4df31d50599571afdd51af45170bb914899d Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Tue, 11 Aug 2026 20:00:09 +0530 Subject: [PATCH 20/21] codec_sv2: trim the comments added by the transport-state changes Drop the ones that only restate the code: the test doc-comments and the descriptions of the round-trip helpers. Keep the ones a reader cannot recover from the code. split_transport consumes the state on its error path as well, which is surprising enough that callers need it in the rustdoc, together with the pointer to is_transport. The encoder and decoder error-path resets read as no-ops unless you know the write offset and the bytes buffered so far persist across calls. And core::result::Result in the encoder tests reads as redundant until you know the glob import shadows it with the crate alias. --- sv2/codec-sv2/src/decoder.rs | 14 ++++---------- sv2/codec-sv2/src/encoder.rs | 14 +++++--------- sv2/codec-sv2/src/lib.rs | 7 ------- 3 files changed, 9 insertions(+), 26 deletions(-) diff --git a/sv2/codec-sv2/src/decoder.rs b/sv2/codec-sv2/src/decoder.rs index ecf7da95ed..f439a38292 100644 --- a/sv2/codec-sv2/src/decoder.rs +++ b/sv2/codec-sv2/src/decoder.rs @@ -278,15 +278,12 @@ impl<'a, T: Serialize + GetSize + Deserialize<'a>, B: IsBuffer + AeadBuffer> Wit let result = self.try_decode_noise_frame(decrypt); match &result { - // `MissingBytes` is the normal way out of the header round: the header has been - // decrypted into `sv2_buffer` and the payload is still on its way, so the buffer has to - // be left exactly as it is. + // `MissingBytes` is the normal way out of the header round, so the buffer has to be + // left exactly as it is. Err(Error::MissingBytes(_)) | Ok(_) => {} Err(_) => { - // Any other failure leaves the decrypt offset at the chunk that failed and the - // bytes decrypted so far in the buffer. A decoder is kept for the life of a - // connection, so the next frame would be decrypted at that stale offset and then - // read as a frame starting in the middle of this one's plaintext. + // Not a no-op: the decrypt offset and the plaintext decrypted so far persist across + // calls, so without this the next frame is decrypted at the failing chunk's offset. self.sv2_buffer.danger_set_start(0); self.sv2_buffer.get_data_owned(); } @@ -842,9 +839,6 @@ mod prop_tests { } } - /// Verifies that a failed decryption does not leave the decoder decrypting the next frame at - /// the offset the failing chunk was using, nor with the plaintext of the failed frame still in - /// the buffer. #[cfg(feature = "noise_sv2")] #[test] fn noise_decoder_recovers_from_a_failed_decryption() { diff --git a/sv2/codec-sv2/src/encoder.rs b/sv2/codec-sv2/src/encoder.rs index a9b0482096..786cdb9a14 100644 --- a/sv2/codec-sv2/src/encoder.rs +++ b/sv2/codec-sv2/src/encoder.rs @@ -169,10 +169,9 @@ impl WithNoise { let result = self.try_encrypt_frame(item, encrypt); if result.is_err() { - // A failure part way through leaves the write offset at the chunk that failed, and the - // bytes written so far in the buffer. Since the encoder is reusable, both have to be - // undone here: otherwise the next frame is written at a stale offset with the remains - // of this one in front of it, and the peer gets a frame it can never open. + // Not a no-op: the write offset and the bytes written so far persist across calls, so + // without this the next frame is written at the failing chunk's offset with the remains + // of this one in front of it. self.noise_buffer.danger_set_start(0); self.noise_buffer.get_data_owned(); self.sv2_buffer.get_data_owned(); @@ -345,8 +344,8 @@ mod prop_tests { #[cfg(feature = "noise_sv2")] use crate::{HandshakeRole, State}; use binary_sv2::{Deserialize, Serialize}; - // The glob import above brings in `crate::Result`, whose single type parameter the code - // generated by the `Deserialize` derive cannot use. + // Not redundant: the glob import above brings in `crate::Result`, whose single type parameter + // the code generated by the `Deserialize` derive cannot use. use core::result::Result; #[cfg(feature = "noise_sv2")] use framing_sv2::framing::Frame; @@ -501,9 +500,6 @@ mod prop_tests { } } - /// Verifies that a chunk encryption failure does not leave the encoder writing the next frame - /// at the offset the failing chunk was using, nor with the bytes of the failed frame still in - /// the buffer. #[cfg(feature = "noise_sv2")] #[test] fn noise_encoder_recovers_from_a_failed_chunk_encryption() { diff --git a/sv2/codec-sv2/src/lib.rs b/sv2/codec-sv2/src/lib.rs index e5b37aa3c3..4659b55b8a 100644 --- a/sv2/codec-sv2/src/lib.rs +++ b/sv2/codec-sv2/src/lib.rs @@ -376,11 +376,6 @@ mod tests { data: B064K<'decoder>, } - // Encrypts `nonce` through `enc`, then decrypts it back through `dec` and `decoder`, and - // returns the nonce carried by the decoded message. - // - // `decoder` is passed in rather than created here so that callers can reuse one decoder per - // direction across frames, the way a connection does. fn round_trip( encoder: &mut NoiseEncoder, decoder: &mut StandardNoiseDecoder, @@ -413,8 +408,6 @@ mod tests { } } - // Runs a full handshake and returns both sides split into their halves, as - // `(initiator encrypt, initiator decrypt, responder encrypt, responder decrypt)`. fn transport_halves() -> ( TransportEncryptState, TransportDecryptState, From ca5b6a9eb8db2d9224305fe3eefc0353b26eecf1 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Tue, 11 Aug 2026 20:19:51 +0530 Subject: [PATCH 21/21] fix encoder copy_from_slice when buffer_sv2 is enabled --- sv2/codec-sv2/src/encoder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sv2/codec-sv2/src/encoder.rs b/sv2/codec-sv2/src/encoder.rs index 786cdb9a14..957e2e75ca 100644 --- a/sv2/codec-sv2/src/encoder.rs +++ b/sv2/codec-sv2/src/encoder.rs @@ -540,7 +540,7 @@ mod prop_tests { let mut decoded = loop { let writable = decoder.writable(); let len = writable.len(); - writable.copy_from_slice(&encrypted.as_ref()[offset..offset + len]); + writable.copy_from_slice(&encrypted[offset..offset + len]); offset += len; match decoder.next_frame(&mut receiver_state) {