Skip to content

agent: initialize double ratchet from the invitation via contact address#1829

Open
epoberezkin wants to merge 80 commits into
masterfrom
ep/rpc
Open

agent: initialize double ratchet from the invitation via contact address#1829
epoberezkin wants to merge 80 commits into
masterfrom
ep/rpc

Conversation

@epoberezkin

Copy link
Copy Markdown
Member

No description provided.

@epoberezkin
epoberezkin requested a review from spaced4ndy as a code owner July 11, 2026 16:39

@simplex-chat-agent simplex-chat-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

RFC + implementation plan for one-off requests to service addresses (PRC pattern without a duplex connection). Two new markdown files; no code changes.

I read both files in full and cross-checked the design against the existing code:

  • Address type: plan's CCTService/'S' matches the ContactConnType pattern in Protocol.hs (ctTypeChar/ctTypeP, 1744-1759) and the contactType ABNF in agent-protocol.md:414.
  • UserContactData extension: the appended serviceKeys :: Maybe ServiceKeyBundle and the optional smpP + takeByteString parse are consistent with the current tail-ignoring forward-compat convention (Protocol.hs:1974-1982). Both "field absent" (older data) and "present as 0" collapse to Nothing correctly.
  • Hybrid PubHeader: additive '2' variant, preserves the existing 0/1 Maybe-key encoding (Protocol.hs:1096-1104).
  • Sizes: sntrup761 pk 1158 / ct 1039 and userDataPaddedLength = 13784 (Crypto/ShortLink.hs:45) match the RFC.
  • SSND encoding, agent envelope tags (Q/P), signed-reply hash chain, and hybrid secret derivation are internally consistent between the RFC ABNF and the plan's Haskell.

The security objectives (unlinkability, PQ encryption bounded by key rotation, reply authenticity via root-key signature + hash chain, replay protection via agent dedup) are coherent and the tradeoffs (tail-truncation detected only by deadline; dedup bounded by a fixed window) are stated honestly.

No substantive correctness, security, concurrency, or architecture defects found. Names are explicitly provisional, so I did not raise naming nits. Nothing blocks merge of the design docs.

Comment thread rfcs/2026-07-11-service-rpc.md Outdated
Comment thread rfcs/2026-07-11-service-rpc.md Outdated
Comment thread rfcs/2026-07-11-service-rpc.md Outdated
@epoberezkin epoberezkin changed the title rfc: agent support for PRC pattern without duplex connection agent: initialize double ratchet from the invitation via contact address Jul 14, 2026

@simplex-chat-agent simplex-chat-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed the whole branch (agent DR-from-invitation via contact address). The change is internally consistent and I found no blocking issues.

What I verified against the code, not the description:

  • Version gating. currentSMPAgentVersion bumped to 8; AgentConfirmation gains ratchetKeyId :: Maybe RatchetKeyId, encoded/decoded symmetrically behind addressDRSMPAgentVersion. Normal confirmations set ratchetKeyId = Nothing, so pre-v8 peers still parse and v8 peers round-trip the extra tag byte.
  • Dispatch. The new (PHEmpty, AgentConfirmation {e2eEncryption_ = Just, ratchetKeyId = Just}) case precedes the generic PHEmpty case and can't shadow it — only joinAddressDR ever sets ratchetKeyId = Just. smpAddressConfirmation additionally guards on ContactConnection.
  • Ratchet flow. Requester builds the snd ratchet in joinAddressDR (createSndRatchet) and later decrypts the owner's reply via getRatchet (the snd-initialized ratchet is a full RatchetX448, matching the existing joiner path). Owner builds the recv ratchet in initRcvRatchetDecrypt, persists it inside the stored DRRequest, and on accept re-creates it against the new connId and creates its reply queue via secureConfirmQueue rq_=Nothing → createReplyQueue. Duplex is fully established.
  • Composition operators. .:::/.::. (Data.Composition) match the new arities of createConnectionForLink'/joinConn.
  • Store & migrations. M20260712_address_dr appended in date order for both SQLite and Postgres; agent_schema.sql regenerated. ContactRequest field disambiguates CRInvitation (URI) vs CRConfirmation (JSON, leading {). JSON derivation of DRRequest is satisfied by existing instances plus the newly added StrEncoding/StrJSON SMPQueueInfo.
  • Encoding compat. The optional smpP <* A.takeByteString for ratchetKeys in UserContactData is deliberate: the standard tagged Maybe encoding would fail on pre-existing tail bytes, whereas optional backtracks to Nothing and ignores the tail.
  • Test expectation changes. The False→True flips in testMatrix2/testBasicMatrix2 etc. are correct: prevRange moves the prev agent version from 6 to 7, and agentSecureSndQueue returns connAgentVersion >= ratchetOnConfSMPAgentVersion (v7). acceptContactAsync' correctly rejects CRConfirmation (async accept unsupported for DR). New testContactDRMatrix covers the DR and classic-ignore paths across PQ settings.

Non-blocking observations (no action requested):

  • address_ratchet_keys.retired_at and key rotation/retirement are schema-only; nothing sets retired_at yet. Keys are looked up per request and reused for all requesters (documented forward-secrecy tradeoff), and cleaned up only via ON DELETE CASCADE. Fine for this first cut.
  • getAddressRatchetKeys reuses SEX3dhKeysNotFound; acceptable since these are X3DH keys.
  • DR address keys are wired through createConnectionForLink only; setConnShortLink link-data updates don't (re)generate them. Consistent with the plan docs; flagging only so it isn't assumed covered.

@simplex-chat-agent simplex-chat-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Summary

This branch lets a contact address publish the owner's X3DH ratchet keys in the mutable link data, so a requester establishes the double ratchet from its first message — putting the invitation profile under the ratchet with post-quantum protection instead of per-queue X25519 only. It is a new agent-layer version bump gated behind an opt-in UseRatchetKeys flag; chat wiring is deferred, so default behavior is unchanged.

I read every touched source file in full plus the imports/callers (Agent.hs, Agent/Protocol.hs, AgentStore.hs, Store.hs, Crypto/ShortLink.hs, Crypto/Ratchet.hs, Encoding.hs, Env/SQLite.hs, both migrations, agent_schema.sql, cabal, and the test files). The RFC/plan docs and the RPC RFCs are planning-only and add no code.

Assessment

The design is well-executed and the backward/forward-compatibility handling is careful and correct:

  • Fixed vs. mutable split. linkConnReq in FixedLinkData changes to BinaryConnectionRequestUri (no address keys), and binaryConnReq drops the keys — so the hash-committed fixed data is byte-identical to before, and the ratchet-keys bundle lives in root-signed UserContactData. getConnShortLink' correctly reconstructs the full CRContactUri crd ratchetKeys from fixed + mutable data.
  • Encoding compatibility. UserContactData appends ratchetKeys as a trailing Maybe with smpP <|> pure Nothing + tail-ignore; AgentConfirmation/AgentInvitationDR use a new envelope tag 'J' (cleaner than the plan's AgentConfirmation.ratchetKeyId); NEW gains a trailing useDR defaulting to False; JOIN reshapes to JoinRequest whose JRConnReq serialization is byte-identical to the pre-DR JOIN, with JRInvitationDR disambiguated by the leading decimal-length. ContactRequest's ToField/FromField keep the legacy URI for CRInvitation and JSON for CRInvitationDR, disambiguated by a leading {.
  • Handshake reuse. The requester takes the joiner role (joinConnSrv … CRContactUri DR branch), the owner the initiator role (smpInvitationDRacceptContact' continue-ratchet branch → startJoinInvitationDR), reusing initRcvRatchet_, createRatchet_, secureConfirmQueue, mkAgentConfirmation. Idempotent re-join advances the send ratchet and the owner absorbs the skip. Sync and async accept, and async join, are all handled and tested.
  • Tests. The DR matrix covers IKPQOff/IKPQOn/IKUsePQ × dh/pq join × sync/async accept × async join, plus rotation, retrofit-via-setConnShortLink, and update-preserves-keys. Encoding round-trip tests are updated to binaryConnReq.

I found no correctness, concurrency, or security defect that warrants a code change. Store writes on the create path are atomic and cascade-cleaned on failure; unauthenticated receive-time ratchet establishment is an RFC-acknowledged, bounded extension of the existing contact-request surface.

Notes for the author (non-blocking)

  1. IKPQOn now advertises the KEM. prepareConnectionLink'/newRcvConnSrv prohibit IKUsePQ for contact and generate the bundle with connPQEncryption pqInitKeys, so IKPQOn produces a bundle with a KEM (PQ from message 1). The RFC's three-way distinction (where IKPQOn = X448-only bundle, PQ from message 2, to avoid the ~1158-byte KEM in widely-fetched link data) is therefore not reachable. The tests assert the implemented behavior (isJust kem_ == supportPQ (connPQEncryption addrIK)), so this looks intentional — worth confirming the RFC is simply stale.

  2. Retention is count-based, not time-based. deleteOldAddressRatchetKeys keeps the keepAddressKeys = 3 most recent generations rather than the RFC's window tied to storedMsgDataTTL. Since automated rotateRatchetKeys is deferred and rotation only happens on explicit setConnShortLink … rotate=True, this is fine; note that a published-but-unused key becomes undecryptable after 3 rotations (this is exercised by testAddressKeyRotation).

  3. Unused export. RatchetKEMStateI (..) is newly exported from Crypto.Ratchet but has no consumer inside simplexmq yet (presumably for the deferred chat wiring).

  4. API change. LDATA gains a third field (ConnectionRequestUri 'CMContact) and several public signatures gain UseRatchetKeys; downstream (chat) must adapt when wired.

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.

2 participants