diff --git a/Cargo.toml b/Cargo.toml index 1eb2676..101bb27 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ anyhow = "1.0" bytes = "1.6" chacha20poly1305 = "0.10" rand = "0.8" -snow = "0.9" +snow = { version = "0.9", features = ["risky-raw-split"] } thiserror = "1.0" tokio = { version = "1.37", features = ["rt-multi-thread", "macros", "net", "time", "io-util", "sync", "signal"] } tracing = "0.1" diff --git a/ostp-core/src/crypto/noise.rs b/ostp-core/src/crypto/noise.rs index 4fc96ba..d1f2e25 100644 --- a/ostp-core/src/crypto/noise.rs +++ b/ostp-core/src/crypto/noise.rs @@ -1,4 +1,4 @@ -use snow::{Builder, HandshakeState, TransportState}; +use snow::{Builder, HandshakeState}; use crate::protocol::ProtocolError; @@ -10,9 +10,15 @@ pub enum NoiseRole { Responder, } -pub enum NoiseSession { - Handshake(Box), - Transport(TransportState), +/// A Noise handshake in progress. OSTP does not use snow's transport mode: once +/// the handshake finishes we extract the raw Split() keys (see [`raw_split`]) +/// and drive our own out-of-order AEAD (see `crypto::aead`), because the wire +/// protocol needs explicit per-frame nonces for reordering that snow's internal +/// nonce counter can't express. +/// +/// [`raw_split`]: NoiseSession::raw_split +pub struct NoiseSession { + handshake: Box, } impl NoiseSession { @@ -36,50 +42,92 @@ impl NoiseSession { .map_err(|_| ProtocolError::Crypto("noise-responder".to_string()))?, }; - Ok(Self::Handshake(Box::new(handshake))) + Ok(Self { handshake: Box::new(handshake) }) } pub fn write_handshake(&mut self, payload: &[u8], out: &mut [u8]) -> Result { - match self { - NoiseSession::Handshake(hs) => hs - .write_message(payload, out) - .map_err(|_| ProtocolError::Crypto("noise-write".to_string())), - NoiseSession::Transport(_) => Err(ProtocolError::State("noise already in transport".to_string())), - } + self.handshake + .write_message(payload, out) + .map_err(|_| ProtocolError::Crypto("noise-write".to_string())) } pub fn read_handshake(&mut self, input: &[u8], out: &mut [u8]) -> Result { - match self { - NoiseSession::Handshake(hs) => hs - .read_message(input, out) - .map_err(|e| ProtocolError::Crypto(format!("noise-read: {:?}", e))), - NoiseSession::Transport(_) => Err(ProtocolError::State("noise already in transport".to_string())), - } + self.handshake + .read_message(input, out) + .map_err(|e| ProtocolError::Crypto(format!("noise-read: {:?}", e))) } - pub fn handshake_hash(&self, out: &mut [u8]) -> Result<(), ProtocolError> { - match self { - NoiseSession::Handshake(hs) => { - let hash = hs.get_handshake_hash(); - if out.len() != hash.len() { - return Err(ProtocolError::Crypto("handshake hash length mismatch".to_string())); - } - out.copy_from_slice(hash); - Ok(()) - } - NoiseSession::Transport(_) => Err(ProtocolError::State("noise already in transport".to_string())), - } - } - - pub fn into_transport(self) -> Result { - match self { - NoiseSession::Handshake(hs) => { - let transport = hs - .into_transport_mode() - .map_err(|_| ProtocolError::Crypto("noise-transport".to_string()))?; - Ok(NoiseSession::Transport(transport)) - } - NoiseSession::Transport(_) => Ok(self), + /// Derive the two directional transport keys via Noise's Split(). + /// + /// SECURITY: keys are taken from the final chaining key `ck` (which absorbs + /// the ephemeral `ee` DH result via MixKey), NOT from the handshake hash `h` + /// (which only absorbs public transcript data — ephemeral pubkeys and + /// ciphertexts — and never the DH secret). Deriving from `ck` is what gives + /// the session forward secrecy: an adversary who later learns the PSK still + /// cannot recompute these keys without the ephemeral private keys, which are + /// discarded after the handshake. + /// + /// Must only be called once the handshake is finished (both messages of the + /// NNpsk0 exchange processed); at that point `ck` is final. Returns + /// `(send_key, recv_key)` for the given role, matching snow's TransportState + /// direction mapping: split output `.0` is initiator→responder, `.1` is + /// responder→initiator. + pub fn raw_split(&mut self, role: NoiseRole) -> Result<([u8; 32], [u8; 32]), ProtocolError> { + if !self.handshake.is_handshake_finished() { + return Err(ProtocolError::State("handshake not finished at key split".to_string())); } + let (k0, k1) = self.handshake.dangerously_get_raw_split(); + Ok(match role { + // Initiator sends on .0 (i→r), receives on .1 (r→i). + NoiseRole::Initiator => (k0, k1), + // Responder is the mirror image. + NoiseRole::Responder => (k1, k0), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Drive a full NNpsk0 handshake and confirm both sides derive matching + /// directional keys. This guards the .0/.1 → send/recv role mapping in + /// `raw_split`: if it were wrong, the two sides' send/recv keys wouldn't + /// cross-match and the transport channel would silently fail to decrypt. + #[test] + fn raw_split_keys_agree_across_roles() { + let psk = [7u8; 32]; + let mut initiator = NoiseSession::new(NoiseRole::Initiator, &psk).unwrap(); + let mut responder = NoiseSession::new(NoiseRole::Responder, &psk).unwrap(); + + // msg1: initiator -> responder + let mut buf1 = [0u8; 1024]; + let n1 = initiator.write_handshake(&[], &mut buf1).unwrap(); + let mut tmp = [0u8; 1024]; + responder.read_handshake(&buf1[..n1], &mut tmp).unwrap(); + + // msg2: responder -> initiator + let mut buf2 = [0u8; 1024]; + let n2 = responder.write_handshake(&[], &mut buf2).unwrap(); + initiator.read_handshake(&buf2[..n2], &mut tmp).unwrap(); + + let (i_send, i_recv) = initiator.raw_split(NoiseRole::Initiator).unwrap(); + let (r_send, r_recv) = responder.raw_split(NoiseRole::Responder).unwrap(); + + // What the initiator sends with, the responder must receive with. + assert_eq!(i_send, r_recv, "initiator send key must equal responder recv key"); + assert_eq!(r_send, i_recv, "responder send key must equal initiator recv key"); + // The two directions use distinct keys. + assert_ne!(i_send, i_recv, "the two directions must not share a key"); + } + + /// raw_split must refuse to hand out keys before the handshake is complete — + /// keys taken from a half-mixed chaining key would be wrong and insecure. + #[test] + fn raw_split_rejected_before_handshake_finishes() { + let psk = [9u8; 32]; + let mut initiator = NoiseSession::new(NoiseRole::Initiator, &psk).unwrap(); + // No messages exchanged yet: handshake not finished. + assert!(initiator.raw_split(NoiseRole::Initiator).is_err()); } } diff --git a/ostp-core/src/crypto/obfuscation.rs b/ostp-core/src/crypto/obfuscation.rs index 63b1427..5d58756 100644 --- a/ostp-core/src/crypto/obfuscation.rs +++ b/ostp-core/src/crypto/obfuscation.rs @@ -74,8 +74,11 @@ pub struct DerivedSecrets { /// without a version) produces a different obfuscation key, so a 0.4.0 server /// cannot recover its handshake header and rejects it as an unauthorized probe. /// -/// Bump this on any wire-breaking protocol change. 0.4.0 = version 4. -pub const PROTOCOL_VERSION: u8 = 4; +/// Bump this on any wire-breaking protocol change. 0.4.0 = version 4; +/// version 5 (0.4.x hardening) moved transport keys from the handshake hash to +/// Noise's Split() output — a wire-breaking crypto change, so old peers must not +/// interop (they would derive different session keys and fail decryption). +pub const PROTOCOL_VERSION: u8 = 5; pub fn derive_all_secrets(access_key: &[u8]) -> DerivedSecrets { derive_all_secrets_versioned(access_key, PROTOCOL_VERSION) diff --git a/ostp-core/src/protocol.rs b/ostp-core/src/protocol.rs index 8ec23cf..92bee13 100644 --- a/ostp-core/src/protocol.rs +++ b/ostp-core/src/protocol.rs @@ -1,6 +1,5 @@ use bytes::Bytes; use rand::Rng; -use sha2::{Digest, Sha256}; use thiserror::Error; use std::collections::{BTreeMap, VecDeque}; use std::time::{Duration, Instant}; @@ -281,9 +280,12 @@ impl ProtocolMachine { NoiseRole::Initiator => None, }; - let mut key = [0_u8; 32]; - self.noise.handshake_hash(&mut key)?; - let (send_key, recv_key) = derive_split_keys(&key, self.role); + // Transport keys come from Noise's Split() over the final chaining key, + // so they depend on the ephemeral `ee` DH secret and give the session + // forward secrecy. (Previously these were derived from the handshake + // hash, which never absorbs the DH result — see raw_split's SECURITY + // note. That is the wire-breaking change gated by PROTOCOL_VERSION.) + let (send_key, recv_key) = self.noise.raw_split(self.role)?; self.send_cipher = Some(SessionCipher::new(&send_key)); self.recv_cipher = Some(SessionCipher::new(&recv_key)); self.state = OstpState::Established; @@ -732,26 +734,6 @@ fn nonce_in_ranges(nonce: u64, ranges: &[(u64, u64)]) -> bool { ranges.iter().any(|(start, end)| nonce >= *start && nonce <= *end) } -fn derive_split_keys(base_key: &[u8; 32], role: NoiseRole) -> ([u8; 32], [u8; 32]) { - let mut initiator_key = [0u8; 32]; - let mut responder_key = [0u8; 32]; - - let mut h1 = Sha256::new(); - h1.update(base_key); - h1.update(b"ostp-initiator"); - initiator_key.copy_from_slice(&h1.finalize()); - - let mut h2 = Sha256::new(); - h2.update(base_key); - h2.update(b"ostp-responder"); - responder_key.copy_from_slice(&h2.finalize()); - - match role { - NoiseRole::Initiator => (initiator_key, responder_key), - NoiseRole::Responder => (responder_key, initiator_key), - } -} - #[cfg(test)] mod tests { use super::*;