Repository navigation
Refuse and prune expired KeyPackages (GRYT-1510) - #242
Merged
Merged
Conversation
A KeyPackage lasts 30 days now, from crypto#21. On upload the server reads the lifetime out of the leaf node and refuses one that's already expired, isn't valid yet, or claims more than 30 days. Each refusal carries serverTime, so a device with a fast clock can retry against it. A claim only hands out a package inside its lifetime with an hour to spare, and the count a device sees leaves the rest out, so it tops up in time. The hourly sweep takes the bytes off expired packages and keeps the ref, the way a replaced last-resort package is kept, so a Welcome built on one still routes. ts-mls's default lifetime, 0 to 2^63-1, is what @gryt/crypto 0.6.0 and older write. Those are still taken, and kept 30 days from upload (GRYT-1518 turns that off). Rows from before this change read as expired. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This was referenced Sep 28, 2026
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
1 participant
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.
What to look at
checkKeyPackageLifetimeinsrc/services/mlsWire.ts. It follows crypto#21: 30 days, both ends valid, checked against the server's own clock. The three refusals (key_package_lifetime,key_package_expired,key_package_not_yet_valid) carryserverTimein seconds, so a device with a fast clock can retry against it.claimMlsKeyPackagewon't hand out a package with less than an hour left (MLS_KEY_PACKAGE_CLAIM_MARGIN). crypto#21 only asks for no expired ones. The hour is there so the adder doesn't get a package that runs out before its commit lands, or on a clock that's a bit ahead. The number is a guess.not_beforeandnot_aftertomls_key_packages, both defaulting to 0. A row from before this reads as expired, so it's never handed out and the next sweep retires it. Server 1.10.35 already has the table, but no released app publishes KeyPackages, so there shouldn't be any rows.What's in it
The lifetime is read out of the leaf node on upload and stored next to the package. The count in
mls:syncand in the publish reply leaves out anything that can't be handed out, so a device tops up before it runs dry. The hourly sweep takes the bytes off expired packages and keeps the ref, the same way a replaced last-resort package is kept, so a Welcome built on one still finds its device.Tests
mls.test.tsindb/sqlite: claims skip expired, nearly expired and not-yet-valid packages (last-resort included), the count leaves them out, and the sweep retires them while the ref still routes.mls.test.tsinsocket/handlers: the four refusals each carryserverTime, both ends of the window are valid, and the legacy lifetime is stored as 30 days from upload. The existing tests now make their packages the way crypto#21 does. Full suite: 1759 pass.I checked the migration by hand too. A DB with the old table and one row came back with
not_after = 0, the row wasn't handed out, and the sweep retired it.Task: GRYT-1510. Crypto half: Gryt-chat/crypto#21. Docs: Gryt-chat/docs#141.
🤖 Generated with Claude Code