From 09447f5a1c97ec9e475f0be5dcc70eb2707aa541 Mon Sep 17 00:00:00 2001 From: durch Date: Thu, 27 Nov 2025 13:56:55 +0100 Subject: [PATCH] Add response validation, zeroize, and replay protection - Validate Ack response in NestedLpSession and LP client final handshake - Replace manual Drop with derive(Zeroize, ZeroizeOnDrop) for OuterAeadKey - Add replay counter check before AEAD decryption to prevent DoS [nym-qm2q, nym-z82d, nym-9ik3, nym-62fs] --- Cargo.lock | 1 + common/nym-lp/Cargo.toml | 1 + common/nym-lp/src/codec.rs | 10 ++------- gateway/src/node/lp_listener/handler.rs | 22 ++++++++++++++++--- .../src/lp_client/client.rs | 18 +++++++++++++-- .../src/lp_client/nested_session.rs | 22 +++++++++++++++++-- 6 files changed, 59 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5b68782a59..171155a834 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6613,6 +6613,7 @@ dependencies = [ "tls_codec", "tracing", "utoipa", + "zeroize", ] [[package]] diff --git a/common/nym-lp/Cargo.toml b/common/nym-lp/Cargo.toml index a8b4c44707..50fec3ac36 100644 --- a/common/nym-lp/Cargo.toml +++ b/common/nym-lp/Cargo.toml @@ -35,6 +35,7 @@ libcrux-traits = { git = "https://github.com/cryspen/libcrux" } tls_codec = { workspace = true } num_enum = { workspace = true } chacha20poly1305 = { workspace = true } +zeroize = { workspace = true, features = ["zeroize_derive"] } [dev-dependencies] criterion = { version = "0.5", features = ["html_reports"] } diff --git a/common/nym-lp/src/codec.rs b/common/nym-lp/src/codec.rs index de7684d376..aa98659694 100644 --- a/common/nym-lp/src/codec.rs +++ b/common/nym-lp/src/codec.rs @@ -12,6 +12,7 @@ use chacha20poly1305::{ aead::{AeadInPlace, KeyInit}, ChaCha20Poly1305, Key, Nonce, Tag, }; +use zeroize::{Zeroize, ZeroizeOnDrop}; /// Outer AEAD key for LP packet encryption. /// @@ -34,7 +35,7 @@ use chacha20poly1305::{ /// 3. **No PSK persistence**: PSK handles are not stored/reused across sessions. /// Each connection performs fresh KKT+PSQ handshake. /// -#[derive(Clone)] +#[derive(Clone, Zeroize, ZeroizeOnDrop)] pub struct OuterAeadKey { key: [u8; 32], } @@ -58,13 +59,6 @@ impl OuterAeadKey { } } -impl Drop for OuterAeadKey { - fn drop(&mut self) { - // Zeroize key material on drop - self.key.iter_mut().for_each(|b| *b = 0); - } -} - impl std::fmt::Debug for OuterAeadKey { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("OuterAeadKey") diff --git a/gateway/src/node/lp_listener/handler.rs b/gateway/src/node/lp_listener/handler.rs index 76a50ea9c8..c6eabff671 100644 --- a/gateway/src/node/lp_listener/handler.rs +++ b/gateway/src/node/lp_listener/handler.rs @@ -375,6 +375,8 @@ impl LpConnectionHandler { self.remote_addr, receiver_idx ); + let counter = packet.header().counter(); + // Get session and decrypt payload let decrypted_bytes = { let session_entry = self.state.session_states.get(&receiver_idx).ok_or_else(|| { @@ -386,10 +388,24 @@ impl LpConnectionHandler { let session = &session_entry.value().state; - // Decrypt packet - session.decrypt_data(packet.message()).map_err(|e| { + // AIDEV-NOTE: Validate counter BEFORE decryption to prevent replay DoS attacks. + // Counter is from cleartext header but authenticated by AEAD AAD, so this is safe. + session.receiving_counter_quick_check(counter).map_err(|e| { + inc!("lp_errors_replay_check"); + GatewayError::LpProtocolError(format!("Replay check failed: {}", e)) + })?; + + // Decrypt packet (Noise inner layer) + let decrypted = session.decrypt_data(packet.message()).map_err(|e| { GatewayError::LpProtocolError(format!("Failed to decrypt packet: {}", e)) - })? + })?; + + // Mark counter as received after successful decryption + session.receiving_counter_mark(counter).map_err(|e| { + GatewayError::LpProtocolError(format!("Failed to mark counter: {}", e)) + })?; + + decrypted }; // Try to deserialize as LpRegistrationRequest first (most common case after handshake) diff --git a/nym-registration-client/src/lp_client/client.rs b/nym-registration-client/src/lp_client/client.rs index 0d111fc4ac..210b7323da 100644 --- a/nym-registration-client/src/lp_client/client.rs +++ b/nym-registration-client/src/lp_client/client.rs @@ -292,14 +292,28 @@ impl LpRegistrationClient { .ok() .and_then(|s| s.outer_aead_key()); tracing::trace!("Sending final handshake packet"); - // Final packet - we don't need the response - let _ = Self::connect_send_receive( + let ack_response = Self::connect_send_receive( self.gateway_lp_address, &final_packet, outer_key.as_ref(), &self.config, ) .await?; + + // Validate Ack response + match ack_response.message() { + nym_lp::LpMessage::Ack => { + tracing::debug!( + "Received Ack for final handshake packet" + ); + } + other => { + return Err(LpClientError::Transport(format!( + "Expected Ack for final handshake packet, got: {:?}", + other + ))); + } + } } tracing::info!("LP handshake completed after sending final packet"); break; diff --git a/nym-registration-client/src/lp_client/nested_session.rs b/nym-registration-client/src/lp_client/nested_session.rs index 68f318852c..5f462bd0ae 100644 --- a/nym-registration-client/src/lp_client/nested_session.rs +++ b/nym-registration-client/src/lp_client/nested_session.rs @@ -153,7 +153,7 @@ impl NestedLpSession { // Serialize and forward ClientHello (no state machine yet, no outer key) let client_hello_bytes = Self::serialize_packet(&client_hello_packet, None)?; - let _response_bytes = outer_client + let response_bytes = outer_client .send_forward_packet( self.exit_identity, self.exit_address.clone(), @@ -161,7 +161,25 @@ impl NestedLpSession { ) .await?; - tracing::debug!("Sent ClientHello to exit gateway via entry"); + // Parse and validate Ack response (cleartext, no outer key before PSK derivation) + let ack_response = Self::parse_packet(&response_bytes, None)?; + match ack_response.message() { + LpMessage::Ack => { + tracing::debug!("Received Ack for ClientHello from exit gateway"); + } + LpMessage::Collision => { + return Err(LpClientError::Transport(format!( + "Exit gateway returned Collision - receiver_index {} already in use", + receiver_index + ))); + } + other => { + return Err(LpClientError::Transport(format!( + "Expected Ack for ClientHello from exit gateway, got: {:?}", + other + ))); + } + } // Step 4: Create state machine for exit gateway handshake let mut state_machine = LpStateMachine::new(