Repository navigation
Somewhere to publish the MLS person key binding (GRYT-1515) - #245
Merged
Merged
Conversation
Apps couldn't pin anybody's person key, because the server had no event to
publish the binding and the member list didn't carry one. The MLS driver
refuses every leaf it can't match to a pinned person key, so no DM could
open on MLS.
mls:person:publish takes { accessToken, binding } with an ack, like the
other mls:* events. The binding has to verify with @gryt/crypto's
verifyPersonKeyBinding, and it has to be signed by the same identity key
and for the same scope as the member's stored DM key binding. That's the
key peers have pinned. null withdraws it.
It's stored in a new users.person_key_binding column and goes out as
personKeyBinding in the member list, in buildMemberList and in the dedupe
hash, so both members:fetch and the broadcast carry it.
Adds @gryt/crypto 0.6.0 as a dependency, loaded by subpath so the MLS code
in its index stays out.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
The other repos are moving to 0.7.0. verifyPersonKeyBinding and verifyDmKeyBinding haven't changed since 0.6.0. Co-Authored-By: Claude Opus 5.5 <[email protected]>
sivert-io
marked this pull request as ready for review
September 28, 2026 08:05
sivert-io
added a commit
that referenced
this pull request
Sep 28, 2026
On upload the server now does what @gryt/crypto's readMlsKeyPackage does, minus the scope. After the lifetime check from #242, the KeyPackage has to be signed by its own leaf key. Its credential has to hold a device certificate the person key signed, for that same leaf key and for the deviceId the upload is under (device_mismatch otherwise). The lifetime goes first. It's a few integer compares, where the rest is two Ed25519 verifications, so junk that's also expired costs nothing. And a device whose clock is off hears about its clock, with serverTime to retry against, and not about a signature that was never the problem. The certificate is read by @gryt/crypto 0.7.0, already on main from #245, through its ./* export so neither index loads. crypto's reader wants the scope to expect, and the server can't know which one a client used, so it passes back the scope the certificate names. Clients check it against their pin. The bytes are copied first, because crypto reads a Node Buffer's lengths off the shared pool (GRYT-1520). The tests sign certificates with crypto, and still read one a crypto build signed at 272bb78. Co-Authored-By: Claude Opus 5.5 <[email protected]>
sivert-io
added a commit
that referenced
this pull request
Sep 28, 2026
) On upload the server now does what @gryt/crypto's readMlsKeyPackage does, minus the scope. After the lifetime check from #242, the KeyPackage has to be signed by its own leaf key. Its credential has to hold a device certificate the person key signed, for that same leaf key and for the deviceId the upload is under (device_mismatch otherwise). The lifetime goes first. It's a few integer compares, where the rest is two Ed25519 verifications, so junk that's also expired costs nothing. And a device whose clock is off hears about its clock, with serverTime to retry against, and not about a signature that was never the problem. The certificate is read by @gryt/crypto 0.7.0, already on main from #245, through its ./* export so neither index loads. crypto's reader wants the scope to expect, and the server can't know which one a client used, so it passes back the scope the certificate names. Clients check it against their pin. The bytes are copied first, because crypto reads a Node Buffer's lengths off the shared pool (GRYT-1520). The tests sign certificates with crypto, and still read one a crypto build signed at 272bb78. Co-authored-by: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GRYT-1515. Apps had nowhere to publish an MLS person key binding, and the member list didn't carry one. So nobody could pin a person key, the core driver (core#22) refused every leaf, and no DM could open on MLS.
What changes
mls:person:publishtakes{ accessToken, binding }and answers through an ack, like the othermls:*events.bindingis the compact JWT fromsignPersonKeyBinding, ornullto withdraw it. The reply is{ ok: true, changed }or{ ok: false, error, message }, witherrorone ofinvalid_payload,unauthenticated,rate_limited,unknown_member,invalid_binding,no_dm_key,wrong_identityorfailed. It's limited to 5 a minute, the same asdm:key:publish.src/services/personKeyBinding.ts). The binding has to passverifyPersonKeyBindingfrom @gryt/crypto. It also has to be signed by the same identity key, and for the same scope, as the member's stored DM key binding. If there's no DM key binding, or it doesn't verify, the reply isno_dm_key.users.person_key_bindingcolumn, added the same way asdm_key_binding, plussetUserPersonKeyBinding.personKeyBindingsits next todmKeyBindinginbuildMemberListand inmemberStateHash, so bothmembers:fetchand the broadcast carry it. It'snullfor somebody who hasn't published one.@gryt/[email protected], pinned exactly, loaded by subpath (@gryt/crypto/mls-person-key,@gryt/crypto/dm-key-binding) the waymlsWire.tsloads ts-mls, so the MLS code in the package index stays out. That needs Node'srequire(esm), which 22.12+ has. The esbuild bundle picks it up too.Why the DM key binding is the thing to check against
The task said to check the binding against the user's identity key. The server doesn't keep the key a member joined with. And for an account it wouldn't be the right key anyway: the client signs bindings with the key derived from the seed for that scope, whichever identity joined (GRYT-759). Peers pin the signer of the DM key binding, and crypto#22's
pinPersonKeyrefuses a person key signed by anybody else. So the server checks the same thing a peer will.What to look at
src/db/**(review-required): oneALTER TABLE users ADD COLUMN person_key_binding TEXT, the field inrowToUserandupsertUser, and the setter. Nothing else in the db changes.dmKeys.tsandinterfaces.tssay so).dm:key:publishstill stores it without reading it.dm:key:publishhas no ack, so there's a small window where the person key publish can land first and getno_dm_key. Retrying once is enough. The docs PR says so.srv:<origin key id>, or the bare host) means guessing atgetOriginKeyIdForHostfrom the server side, and a wrong guess would refuse honest clients.connection.tsand touchesinterfaces.ts, in different hunks. 1509 checks device certificates on KeyPackage upload. It could check the certificate's person key against this column, but that's its call.Tests
src/socket/handlers/mlsPersonKey.test.tssigns real bindings with @gryt/crypto. It covers a good binding stored byte for byte, the broadcast andmembers:fetchboth carrying it,changed: falseon a repeat, andnullwithdrawing it. It also covers refusals for no DM key binding, a stored DM key binding that doesn't verify, a different signer, a different scope, a DM key binding sent as a person key binding, a flipped signature, junk input, a bad token, and one member's binding sent with another's token.memberStateHash.test.tsgains cases for both bindings. I tookpersonKeyBindingout of the hash and the broadcast test failed, and I took out the signer check and two tests failed.Docs: Gryt-chat/docs#142
🤖 Generated with Claude Code