From 7cc472fd2e3e566a498af87ae37f778c863475fa Mon Sep 17 00:00:00 2001 From: SSD DDD Date: Fri, 10 Jul 2026 12:15:33 +0700 Subject: [PATCH] =?UTF-8?q?fix(crypto):=20real=20Noise=20ee+es+se=20key=20?= =?UTF-8?q?agreement=20(forward=20secrecy)=20=E2=80=94=20part=201=20of=20N?= =?UTF-8?q?3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The handshake computed only ee + ss and passed `ss` in BOTH the es and se slots, so it had no forward secrecy and did not bind ephemerals into the session. Fix: NoiseXX.ephemeral becomes a reusable StaticSecret (EphemeralSecret is consumed by its first diffie_hellman; we need two), and complete_initiator/ responder compute the real ee/es/se so both sides derive the same key. Verified: handshake_and_roundtrip + independent_handshakes pass, full suite 103 passed. SCOPE: forward-secrecy half of N3 only. Identity auth (allow-list gating -> the actual MITM defense), N4 (HELLO MAC), N5 (keystore) remain open per RFC #63 and need owner sign-off. Based on the pipeline-guard branch (needs #60's build fix). Draft — needs crypto review. Refs #58, #63. Co-Authored-By: Claude Fable 5 phi^2 + phi^-2 = 3 --- src/crypto.rs | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/crypto.rs b/src/crypto.rs index e568b9eb..dd2e160e 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -92,8 +92,11 @@ pub struct NoiseXX { /// Our static identity key (long-term) static_secret: StaticSecret, static_public: PublicKey, - /// Our ephemeral key for this handshake - ephemeral: EphemeralSecret, + /// Our ephemeral key for this handshake. A reusable `StaticSecret` (not a + /// one-shot `EphemeralSecret`) because the Noise key agreement needs it for + /// TWO DH operations (ee and es/se); it is still freshly random per handshake + /// and dropped when the handshake completes. + ephemeral: StaticSecret, ephemeral_public: PublicKey, /// True if we're the initiator (first to send). Retained for future /// role-aware rekey/anti-replay logic; not yet read by current handlers. @@ -105,7 +108,7 @@ impl NoiseXX { /// Start a new Noise-XX handshake with a static identity key. pub fn new(static_secret: StaticSecret, initiator: bool) -> Self { let static_public = PublicKey::from(&static_secret); - let ephemeral = EphemeralSecret::random_from_rng(OsRng); + let ephemeral = StaticSecret::random_from_rng(OsRng); let ephemeral_public = PublicKey::from(&ephemeral); Self { @@ -130,33 +133,29 @@ impl NoiseXX { /// Complete as initiator: receive responder's message, derive session. /// Input: (responder_ephemeral_pub, responder_static_pub) pub fn complete_initiator(self, peer_ephemeral: PublicKey, peer_static: PublicKey) -> Session { - // SIMPLIFIED Noise-XX: Use only ee (ephemeral-ephemeral) + ss (static-static) - // Proper Noise-XX would use ee, es, se but that requires multiple ephemeral DH ops - - // ee = ephemeral × peer_ephemeral + // Real Noise key agreement: ee + es + se (was ee + ss with `ss` wrongly + // passed in both the es and se slots, giving no forward secrecy). The + // initiator's shares: + // ee = e_i . e_r es = e_i . s_r se = s_i . e_r let ee = self.ephemeral.diffie_hellman(&peer_ephemeral); - let ee_bytes = *ee.as_bytes(); - - // ss = static × peer_static (both sides compute this, gets same result) - let ss = self.static_secret.diffie_hellman(&peer_static); - let ss_bytes = *ss.as_bytes(); - - // Combine ee + ss (both sides get same result) - let combined = combine_dh_shares(&ee_bytes, &ss_bytes, &ss_bytes); + let es = self.ephemeral.diffie_hellman(&peer_static); + let se = self.static_secret.diffie_hellman(&peer_ephemeral); + let combined = + combine_dh_shares(ee.as_bytes(), es.as_bytes(), se.as_bytes()); Session::from_shared(&combined, true) } /// Complete as responder: receive initiator's static, derive session. /// Input: (initiator_ephemeral_pub, initiator_static_pub) pub fn complete_responder(self, peer_ephemeral: PublicKey, peer_static: PublicKey) -> Session { - // Same as initiator: ee + ss (both sides compute same) + // Mirror of the initiator so both derive the SAME (ee, es, se). Here the + // peer is the initiator, so `peer_ephemeral = e_i`, `peer_static = s_i`: + // ee = e_r . e_i es = s_r . e_i (== e_i . s_r) se = e_r . s_i (== s_i . e_r) let ee = self.ephemeral.diffie_hellman(&peer_ephemeral); - let ee_bytes = *ee.as_bytes(); - - let ss = self.static_secret.diffie_hellman(&peer_static); - let ss_bytes = *ss.as_bytes(); - - let combined = combine_dh_shares(&ee_bytes, &ss_bytes, &ss_bytes); + let es = self.static_secret.diffie_hellman(&peer_ephemeral); + let se = self.ephemeral.diffie_hellman(&peer_static); + let combined = + combine_dh_shares(ee.as_bytes(), es.as_bytes(), se.as_bytes()); Session::from_shared(&combined, false) } }