fix(crypto)!: derive transport keys from DH-inclusive Noise Split, not the handshake hash

CRITICAL forward-secrecy fix. Session transport keys were derived as
SHA256(get_handshake_hash() || label). The Noise handshake hash `h` only
ever absorbs PUBLIC transcript data (ephemeral pubkeys + on-wire
ciphertexts, via MixHash); the ephemeral ee DH result is mixed via MixKey
into the chaining key `ck` ONLY, never into `h` (confirmed in snow 0.9.6
symmetricstate.rs). So the data-transport keys depended on the PSK and the
public transcript but NOT on the DH secret, meaning:

  - zero forward secrecy: anyone who later learns the access-key PSK can
    decrypt all recorded past sessions from the observed handshake alone;
  - any PSK holder can passively decrypt any other session on that key;
  - the ephemeral Diffie-Hellman was cryptographically wasted.

Fix: take the two directional keys from Noise's Split() over the final `ck`
via snow's dangerously_get_raw_split (risky-raw-split feature). These keys
depend on ee, restoring forward secrecy. The custom out-of-order AEAD,
explicit nonces, session_id AAD, framing and reordering are all unchanged
- only the key SOURCE moved. The dead into_transport()/handshake_hash()
paths and the unreachable NoiseSession::Transport variant are removed.

Wire-breaking: PROTOCOL_VERSION 4 -> 5 so pre-fix peers derive different
keys and cannot interop (version gate is invisible on the wire).

Added noise unit tests for the .0/.1 -> send/recv role mapping and the
not-finished guard.

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"

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<HandshakeState>),
-    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<HandshakeState>,
 }
 
 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<usize, ProtocolError> {
-        match self {
-            NoiseSession::Handshake(hs) => hs
+        self.handshake
             .write_message(payload, out)
-                .map_err(|_| ProtocolError::Crypto("noise-write".to_string())),
-            NoiseSession::Transport(_) => Err(ProtocolError::State("noise already in transport".to_string())),
-        }
+            .map_err(|_| ProtocolError::Crypto("noise-write".to_string()))
     }
 
     pub fn read_handshake(&mut self, input: &[u8], out: &mut [u8]) -> Result<usize, ProtocolError> {
-        match self {
-            NoiseSession::Handshake(hs) => hs
+        self.handshake
             .read_message(input, out)
-                .map_err(|e| ProtocolError::Crypto(format!("noise-read: {:?}", e))),
-            NoiseSession::Transport(_) => Err(ProtocolError::State("noise already in transport".to_string())),
-        }
+            .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<Self, ProtocolError> {
-        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());
     }
 }

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)

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::*;