Skip to content

fix: bind OTKs into signed prekey transcripts (#48) - #57

Merged
commitchan merged 1 commit into
ni5arga:mainfrom
chtnnh:fix/48-signed-otk-binding
Jul 21, 2026
Merged

fix: bind OTKs into signed prekey transcripts (#48)#57
commitchan merged 1 commit into
ni5arga:mainfrom
chtnnh:fix/48-signed-otk-binding

Conversation

@chtnnh

@chtnnh chtnnh commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Test plan

  • npm test (229)
  • Regression: forged Eve OTKs under Bob SPK fail absorb; honest bundle seals to Bob only
  • Manual: two phones exchange in-band replenishment after update

Absorb rejected substituted one-time publics that rode under a valid
SPK signature. Sign and verify the exact OTK list with the SPK so Eve
cannot redirect seals meant for Bob.

Co-authored-by: Cursor <cursoragent@cursor.com>

@commitchan commitchan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the crypto closely since this is the FS path. The fix is right and tightly scoped.

The transcript is unambiguous: fixed-width SIG_CONTEXT_RECV || SPK(32) || createdAt(8) || u16be(count) || OTK×n(32), and verify rejects any OTK that isn't 32 bytes, so there's no room for a length-confusion split between the SPK and the OTK list. The security-critical detail is that the legacy SPK-only fallback is reachable ONLY when oneTimePublics is empty — so an attacker can't downgrade a bundle that carries OTKs, and a stripped-OTK bundle fails closed rather than silently verifying SPK-only. Order is preserved on both reconstruction paths (decodeBundle and bundleFromWire), and all four call sites are bound (bundleForQr with the empty list, updateForPeer signing after issuing, absorb, and decodeContactCode). Backward compat holds: fresh QR signs count=0 and old plaques verify via the legacy path.

The regression test is a real PoC — Eve's OTK spliced under Bob's valid SPK signature fails verify + absorb, with a control proving the honest bundle seals to Bob and not Eve. 229 tests green, typecheck and lint clean.

Merging. Heads up that #58#60 sit on top of this and will each need a rebase now.

@commitchan
commitchan merged commit a0e6dc9 into ni5arga:main Jul 21, 2026
2 checks passed
@chtnnh
chtnnh deleted the fix/48-signed-otk-binding branch July 23, 2026 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[High][FS] Unsigned OTKs can redirect decryption to a third party

2 participants