agent: initialize double ratchet from the invitation via contact address#1829
agent: initialize double ratchet from the invitation via contact address#1829epoberezkin wants to merge 80 commits into
Conversation
There was a problem hiding this comment.
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 theContactConnTypepattern inProtocol.hs(ctTypeChar/ctTypeP, 1744-1759) and thecontactTypeABNF inagent-protocol.md:414. UserContactDataextension: the appendedserviceKeys :: Maybe ServiceKeyBundleand theoptional smpP+takeByteStringparse are consistent with the current tail-ignoring forward-compat convention (Protocol.hs:1974-1982). Both "field absent" (older data) and "present as0" collapse toNothingcorrectly.- Hybrid
PubHeader: additive'2'variant, preserves the existing0/1Maybe-key encoding (Protocol.hs:1096-1104). - Sizes: sntrup761 pk 1158 / ct 1039 and
userDataPaddedLength = 13784(Crypto/ShortLink.hs:45) match the RFC. SSNDencoding, 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.
Co-authored-by: Evgeny <evgeny@poberezkin.com>
There was a problem hiding this comment.
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.
currentSMPAgentVersionbumped to 8;AgentConfirmationgainsratchetKeyId :: Maybe RatchetKeyId, encoded/decoded symmetrically behindaddressDRSMPAgentVersion. Normal confirmations setratchetKeyId = 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 genericPHEmptycase and can't shadow it — onlyjoinAddressDRever setsratchetKeyId = Just.smpAddressConfirmationadditionally guards onContactConnection. - Ratchet flow. Requester builds the snd ratchet in
joinAddressDR(createSndRatchet) and later decrypts the owner's reply viagetRatchet(the snd-initialized ratchet is a fullRatchetX448, matching the existing joiner path). Owner builds the recv ratchet ininitRcvRatchetDecrypt, persists it inside the storedDRRequest, and on accept re-creates it against the new connId and creates its reply queue viasecureConfirmQueue rq_=Nothing → createReplyQueue. Duplex is fully established. - Composition operators.
.:::/.::.(Data.Composition) match the new arities ofcreateConnectionForLink'/joinConn. - Store & migrations.
M20260712_address_drappended in date order for both SQLite and Postgres;agent_schema.sqlregenerated.ContactRequestfield disambiguatesCRInvitation(URI) vsCRConfirmation(JSON, leading{). JSON derivation ofDRRequestis satisfied by existing instances plus the newly addedStrEncoding/StrJSON SMPQueueInfo. - Encoding compat. The
optional smpP <* A.takeByteStringforratchetKeysinUserContactDatais deliberate: the standard taggedMaybeencoding would fail on pre-existing tail bytes, whereasoptionalbacktracks toNothingand ignores the tail. - Test expectation changes. The
False→Trueflips intestMatrix2/testBasicMatrix2etc. are correct:prevRangemoves the prev agent version from 6 to 7, andagentSecureSndQueuereturnsconnAgentVersion >= ratchetOnConfSMPAgentVersion(v7).acceptContactAsync'correctly rejectsCRConfirmation(async accept unsupported for DR). NewtestContactDRMatrixcovers the DR and classic-ignore paths across PQ settings.
Non-blocking observations (no action requested):
address_ratchet_keys.retired_atand key rotation/retirement are schema-only; nothing setsretired_atyet. Keys are looked up per request and reused for all requesters (documented forward-secrecy tradeoff), and cleaned up only viaON DELETE CASCADE. Fine for this first cut.getAddressRatchetKeysreusesSEX3dhKeysNotFound; acceptable since these are X3DH keys.- DR address keys are wired through
createConnectionForLinkonly;setConnShortLinklink-data updates don't (re)generate them. Consistent with the plan docs; flagging only so it isn't assumed covered.
There was a problem hiding this comment.
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.
linkConnReqinFixedLinkDatachanges toBinaryConnectionRequestUri(no address keys), andbinaryConnReqdrops the keys — so the hash-committed fixed data is byte-identical to before, and the ratchet-keys bundle lives in root-signedUserContactData.getConnShortLink'correctly reconstructs the fullCRContactUri crd ratchetKeysfrom fixed + mutable data. - Encoding compatibility.
UserContactDataappendsratchetKeysas a trailingMaybewithsmpP <|> pure Nothing+ tail-ignore;AgentConfirmation/AgentInvitationDRuse a new envelope tag'J'(cleaner than the plan'sAgentConfirmation.ratchetKeyId);NEWgains a trailinguseDRdefaulting toFalse;JOINreshapes toJoinRequestwhoseJRConnReqserialization is byte-identical to the pre-DRJOIN, withJRInvitationDRdisambiguated by the leading decimal-length.ContactRequest'sToField/FromFieldkeep the legacy URI forCRInvitationand JSON forCRInvitationDR, disambiguated by a leading{. - Handshake reuse. The requester takes the joiner role (
joinConnSrv … CRContactUriDR branch), the owner the initiator role (smpInvitationDR→acceptContact'continue-ratchet branch →startJoinInvitationDR), reusinginitRcvRatchet_,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 tobinaryConnReq.
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)
-
IKPQOn now advertises the KEM.
prepareConnectionLink'/newRcvConnSrvprohibitIKUsePQfor contact and generate the bundle withconnPQEncryption pqInitKeys, soIKPQOnproduces a bundle with a KEM (PQ from message 1). The RFC's three-way distinction (whereIKPQOn= 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. -
Retention is count-based, not time-based.
deleteOldAddressRatchetKeyskeeps thekeepAddressKeys = 3most recent generations rather than the RFC's window tied tostoredMsgDataTTL. Since automatedrotateRatchetKeysis deferred and rotation only happens on explicitsetConnShortLink … rotate=True, this is fine; note that a published-but-unused key becomes undecryptable after 3 rotations (this is exercised bytestAddressKeyRotation). -
Unused export.
RatchetKEMStateI (..)is newly exported fromCrypto.Ratchetbut has no consumer inside simplexmq yet (presumably for the deferred chat wiring). -
API change.
LDATAgains a third field (ConnectionRequestUri 'CMContact) and several public signatures gainUseRatchetKeys; downstream (chat) must adapt when wired.
No description provided.