PCH-02-009-reject-zero-dh.patch.md

3.7 KB · raw

rendered · source

PCH-02-009 — Reject zero X25519 DH outputs in Noise XX and NK

Severity: Informational Target files: src/shared/noise-xx.ts (6 DH sites), src/shared/noise-js.ts (6 DH sites) Report: peach-code-review.md, Finding PCH-02-009

Implementation choice (simpler than the brief's sample)

@stablelib/x25519's sharedKey already has the exact guard built in: sharedKey(mySecretKey, theirPublicKey, rejectZero = false) — with rejectZero: true it throws "X25519: invalid shared key" on an all-zero output (verified in node_modules/@stablelib/x25519/lib/x25519.js:606-624). Passing true at every call site is equivalent to the brief's manual every(b => b === 0) check, applies to the raw 32-byte output before it reaches mixKey/HMAC, and adds no new code path to maintain.

Not a wire change. Honest peers never produce an all-zero shared secret (the probability for random keys is negligible; only adversarially chosen low-order/identity points do). Each side can adopt the check independently — no Android coordination required, though mirroring it there is recommended.

Exact edits — all 12 DH call sites

noise-xx.ts — add a rationale comment at the first site, , true at all six:

// Line 108 (initiator, ee):
// BEFORE
  const ee = sharedKey(e.secretKey, remoteEphemeral);
// AFTER
  // PCH-02-009: rejectZero — an all-zero DH output (low-order peer point)
  // aborts the handshake before the value can enter the key schedule. Applies
  // at every DH below; honest peers are unaffected.
  const ee = sharedKey(e.secretKey, remoteEphemeral, true);

// Line 119 (initiator, es):   sharedKey(e.secretKey, remoteStaticPlain, true)
// Line 140 (initiator, se):   sharedKey(ourStaticPrivateKey, remoteEphemeral, true)
// Line 196 (responder, ee):   sharedKey(e.secretKey, remoteEphemeral, true)
// Line 204 (responder, es):   sharedKey(ourStaticPrivateKey, remoteEphemeral, true)
// Line 231 (responder, se):   sharedKey(e.secretKey, remoteStaticPlain, true)

noise-js.ts — same pattern, comment at the first site:

// Line 84  (NK initiator, es):  sharedKey(ephemeral.secretKey, responderStaticPublicKey, true)
//   with the PCH-02-009 rationale comment above it (as in noise-xx.ts)
// Line 106 (NK initiator, ee):  sharedKey(ephemeral.secretKey, remoteEphemeralPublic, true)
// Line 175 (computeAuthProof):  sharedKey(initiatorPrivate, responderPublic, true)
// Line 248 (NK responder, es):  sharedKey(ourStaticPrivateKey, remoteEphemeralPublic, true)
// Line 263 (NK responder, ee):  sharedKey(ephemeral.secretKey, remoteEphemeralPublic, true)
// Line 362 (performResponderAuth): sharedKey(ourPrivateKey, initiatorPublicKey, true)

The two auth-proof sites (175, 362) key an HMAC rather than mixKey; the same contributory-behaviour argument applies — a zero MAC key derived from an attacker-chosen point should abort, not authenticate.

KAT / existing-test impact

None. The pinned vectors (noise-js.test.ts self-vectors, noise-blake2s-kat.test.ts cross-repo vectors) all use honest keys whose DH outputs are non-zero; rejectZero only changes behaviour on the degenerate inputs the new tests inject.

Tests

noise.reject-zero-dh.test.ts (in this folder) — 5 tests: NK initiator aborts against an all-zero (low-order) responder static; NK responder aborts on an all-zero msg-1 ephemeral; XX responder aborts on an all-zero msg-1 ephemeral; full NK and XX handshakes over an in-memory transport still succeed with honest keys (regression). Destination: src/shared/noise.reject-zero-dh.test.ts.

Save message (integration)

fix: PCH-02-009 reject zero X25519 DH output at all 12 Noise XX/NK call sites via stablelib rejectZero