PCH-02-002-noise-xx-msg1-hash.patch.md

5.9 KB · raw

rendered · source

1# PCH-02-002 — Noise XX message 1 empty-payload mixHash23**Severity:** Informational4**Target file:** `src/shared/noise-xx.ts:93-97` (initiator), `184-191` (responder)5**Also touched:** `src/shared/noise-js.ts:11-14` (doc comment correction — landable NOW)6**Report:** peach-code-review.md, Finding PCH-02-00278## Decision: DEFER the wire change to the Android coordination window910Per the fix brief's own escape hatch ("if both sides already skip it11consistently and the practical risk is only interop … defer the wire change to12the Android coordination window. Do not implement a half-fix.") — that is13exactly the situation, verified by a complete read of `noise-xx.ts`:1415- Both the initiator (msg 1 send, lines 93–96) and the responder (msg 116  receive, lines 188–191) skip the canonical `MixHash(empty payload)` —17  **consistently**. Every Peach↔Peach and Peach↔Android XX handshake computes18  an identical transcript hash `h`.19- Transport keys derive from the chaining key `ck`, not from `h`; the20  handshake hash still binds every byte both sides actually exchanged. There21  is no confidentiality/authenticity weakening — only non-interop with stock22  Noise stacks (e.g. `snow`), which Peach explicitly does not rely on23  (`noise-js.ts:5-9`: "interop holds by porting discipline, not by a shared24  library").25- Adding the `mixHash(empty)` changes `h` after message 1, which changes the26  AAD of every subsequent handshake AEAD → **every ciphertext from msg 227  onward changes**. peach-android must apply the identical change in the same28  release or all XX sync sessions fail to handshake. This is the same class29  of coordinated breaking change as PCH-01-002 (BLAKE2s), which shipped in30  lockstep as browser v3.9.1 / android v2.2.9.3132Implementing the browser side alone would be the forbidden half-fix.3334## Ready-to-apply patch (apply ONLY in the coordinated window)3536Initiator — `noise-xx.ts`, message 1 block:3738```typescript39// BEFORE (lines 93-96)40  // --- Message 1: -> e ---41  const e = generateKeyPair();42  h = mixHash(h, e.publicKey);43  await send(e.publicKey);4445// AFTER46  // --- Message 1: -> e ---47  const e = generateKeyPair();48  h = mixHash(h, e.publicKey);49  // PCH-02-002: canonical Noise ends every message with EncryptAndHash(payload);50  // with no symmetric key yet, an empty msg-1 payload is MixHash(empty). The51  // wire bytes are unchanged (empty payload), only the transcript hash moves.52  // BREAKING transcript change — ships in lockstep with peach-android.53  h = mixHash(h, new Uint8Array(0));54  await send(e.publicKey);55```5657Responder — `noise-xx.ts`, message 1 block:5859```typescript60// BEFORE (lines 187-191)61  // --- Message 1: <- e ---62  const msg1 = await receive();63  if (msg1.length < 32) throw new Error("Noise XX: msg1 too short");64  const remoteEphemeral = msg1.slice(0, 32);65  h = mixHash(h, remoteEphemeral);6667// AFTER68  // --- Message 1: <- e ---69  const msg1 = await receive();70  if (msg1.length < 32) throw new Error("Noise XX: msg1 too short");71  const remoteEphemeral = msg1.slice(0, 32);72  h = mixHash(h, remoteEphemeral);73  // PCH-02-002: mirror the initiator's empty-payload MixHash (see above).74  h = mixHash(h, new Uint8Array(0));75```7677The NK pattern (`noise-js.ts`) is already canonical — its msg-1 payload is a78real AEAD seal because `es` establishes a key before the payload step79(`noise-js.ts:84-91`); no change there.8081### Test impact when the wire change lands8283- `src/shared/noise-xx.test.ts` and `src/shared/noise-blake2s-kat.test.ts`:84  any pinned XX handshake-hash / transcript vectors must be regenerated (the85  NK vectors are unaffected). Initiator↔responder round-trip tests pass86  unchanged (both sides move together).87- Cross-repo: regenerate the XX vectors in peach-android88  `CrossPlatformKATTest` to the same values, exactly as PCH-01-002 did.8990## Landable NOW (no wire impact): doc comment correction in noise-js.ts9192The header claim at `noise-js.ts:12` — "A stock `snow` stack would93interoperate" — is true for NK but over-reads for XX. Correct it so the claim94cannot mislead a future maintainer:9596```typescript97// BEFORE (noise-js.ts:11-14)98 * PCH-01-002: this implementation derives `h` and HKDF over BLAKE2s to match the99 * named pattern. A stock `snow` stack would interoperate; the Android port must100 * mirror the same hash choices. Any change to the hash, HKDF, or transcript here101 * is a BREAKING wire change and requires a coordinated Android release.102103// AFTER104 * PCH-01-002: this implementation derives `h` and HKDF over BLAKE2s to match the105 * named pattern. A stock `snow` stack would interoperate with THIS NK pattern;106 * the XX implementation (noise-xx.ts) omits the canonical msg-1 empty-payload107 * MixHash (PCH-02-002, deferred to an Android-coordinated release), so stock108 * stacks would NOT interoperate with Peach XX until that lands. The Android port109 * must mirror the same hash choices. Any change to the hash, HKDF, or transcript110 * here is a BREAKING wire change and requires a coordinated Android release.111```112113And pin the deferral in `noise-xx.ts` so the omission is visible at the site:114115```typescript116// Insert after noise-xx.ts:17 (end of the header comment block):117//118// PCH-02-002 (DEFERRED — Android-coordinated wire change): message 1 omits the119// canonical empty-payload MixHash. Both Peach sides skip it identically, so120// Peach↔Peach/Android transcripts agree; only interop with stock Noise stacks121// is affected. The ready-to-apply fix lives in122// src/drafts/PCH-02-FIXES/PCH-02-002-noise-xx-msg1-hash.patch.md and MUST land123// in the same release as the matching peach-android change (PCH-01-002 model).124```125126## Save message (integration, comment-only part)127128`docs: PCH-02-002 pin Noise XX msg1 empty-payload mixHash deferral — wire change requires Android coordination; correct noise-js snow-interop claim`129130## Save message (coordinated window, wire part)131132`fix: PCH-02-002 Noise XX message 1 empty-payload mixHash — NOTE: wire change, ships in lockstep with peach-android`