Skip to content

Retain keys-message bytes so an expired GroupKeys can be put back - #123

Open
mpretty-cyro wants to merge 3 commits into
session-foundation:devfrom
mpretty-cyro:feature/config-recovery
Open

Retain keys-message bytes so an expired GroupKeys can be put back#123
mpretty-cyro wants to merge 3 commits into
session-foundation:devfrom
mpretty-cyro:feature/config-recovery

Conversation

@mpretty-cyro

Copy link
Copy Markdown
Collaborator

Why this exists

Config messages have a 30-day TTL, refreshed by the expire bump clients already piggyback on every
poll. A device offline past the TTL loses its config from the swarm silently. Clients are gaining
detection and re-store for the configs they can rebuild; this PR is the piece libSession has to
provide for the one they cannot.

A GroupKeys message is not reproducible by the device that needs it. It carries an admin
signature, and its junk padding derives from the group secret key with the count structurally enforced
at verification. A member cannot regenerate one, so today an expired keys message means the group is
unrecoverable until an admin happens to rekey.

The way out is to never regenerate it: keep the bytes we received and push those back unchanged.
They land on the same message hash — the storage server hashes the ciphertext — so the re-store is
idempotent and needs no signature the pusher does not have. That is what lets a member repair the
group
, which is the point of the change.

What it adds

  • Keys::active_key_messages()map<hash, span<const unsigned char>>
  • groups_keys_active_message(conf, msg_hash, &data, &len) — borrowed bytes, following
    groups_keys_pending_config's convention; returns false when we hold nothing for that hash,
    which means cannot recover this one, not an error
  • a "C" key in the dump

Three decisions worth reviewing

Keyed by hash, not generation. One generation carries the full rekey plus every supplemental
issued against it
, and a member that receives only some of them does not get the key. So recovery
must re-store every held message for a generation, not one — which a generation-keyed cache could not
express.

Pruned from active_msgs_ as the single authority, in one place, rather than beside each site that
drops hashes. remove_expired() already drops hashes in two branches; duplicating the erase is exactly
how the two get out of step, and it would leave a third branch — added later — equally exposed. This
also bounds the cache to the same KEY_EXPIRY window as the keys themselves, on disk as well as in
memory
, which is the whole size argument.

Dump compatibility is by construction and tested both ways. "C" sorts between the existing "A"
and "L", and load_dump reads with skip_until — so an old dump loads under new code and a new dump
loads under old code. Both directions have tests rather than a comment; the old-code case is built by
stripping "C" from a real dump and re-emitting the raw sub-encodings, so it is byte-for-byte what old
code would have written.

What it deliberately does not do

Retention happens on load, not on create. The device that authors a keys message holds no bytes
for it until it loads its own message back from the swarm — so an admin immediately after a rekey is
the device least able to repair. It converges within a poll cycle, and it is stated at the accessor
because the natural reading is the opposite.

Only messages loaded after this ships are retained. Existing groups are unchanged; a hash returned
by active_hashes() may legitimately have no bytes behind it. In short: this does not fix currently
broken groups, it prevents future groups from being broken.


A pre-existing needs_dump_ bug, fixed separately

insert_key's early-return path — we already hold this key, but this is a different message carrying
it
— recorded the new hash in active_msgs_ without setting needs_dump_, unlike both of the
other recording paths.

So the hash was lost on the next restart, and that message stopped having its TTL renewed — it
could expire from the swarm while still being a live copy of a current key. Reachable whenever an admin
issues a supplemental carrying a key the recipient already holds.

Independent of this feature and separately revertible, but it would also have holed it: the retained
bytes would be dropped with the hash, making that message unrecoverable.


Determinism tests

test_config_determinism.cpp is the evidence for a property the clients depend on and that had only
ever been established by reading: re-encrypting an unchanged config reproduces the received message
byte for byte, and a clean push is not a new revision.

Covers the full round trip — push, receive, dump, reload, push — rather than encrypting the same bytes
twice, because recovery happens after a restart and it is the reload path that has to be reproducible.
User configs (protobuf-wrapped), group configs (raw), multipart, and a read-only member reproducing an
admin's bytes from a dump that retained a signature it cannot re-derive.

Two behaviours it pins, both of which caught client bugs: a clean push consumes the obsolete-hash
list
, so a re-store must plumb it through to the delete call or leak those messages; and the
hand-back is gated on !is_readonly() while the clear is not
, so on the member path that list is
always empty — which is expected, not a failure.

The protobuf wrapper additionally carries a golden digest, because every other assertion compares
bytes made within one process: a wall-clock or random field added to the wrapper would break re-store
idempotency while leaving all of them green.


Testing

Full suite 138 cases / 25,756,113 assertions, [recovery] 99 assertions / 2 cases,
format.sh -c clean.

The pruning is mutation-verified rather than argued: disabling the single prune_key_msgs() call fails
3 assertions across both branches of remove_expired() — in memory, after a dump/reload, and in the
keys_-empty path.

Downstream

The NodeJS and Android wrappers have branches waiting on this; both expose the accessor and neither
compiles until this merges and their pin moves
. Nothing else depends on it.

Config recovery re-stores an unchanged config to refresh its TTL rather
than pushing a new revision, which requires that re-encrypting reproduces
the received message byte for byte (the storage server hashes the
ciphertext) and that a clean push is not a new revision. Both were
established by reading only.

Covers the full round trip -- push, receive, dump, reload, push -- rather
than encrypting the same bytes twice, since recovery happens after a
restart and it is the reload path that has to be reproducible: user
configs (protobuf-wrapped), group configs (raw), multipart configs, and a
read-only member reproducing an admin's bytes from a dump that retained
the signature it cannot re-derive.

Two behaviours worth knowing that the tests pin: a clean push consumes the
obsolete-hash list, so a re-store must plumb it through to the delete call
or leak those messages; and the hand-back is gated on !is_readonly() while
the clear is not, so on the member path that list is always empty.

The protobuf wrapper is additionally pinned by a golden digest: every
other assertion compares bytes made within one process, so a wall-clock or
random field added to the wrapper would break re-store idempotency while
leaving them all green.
insert_key's early-return path -- we already hold this key, but this is a
different message carrying it -- recorded the new hash in active_msgs_
without setting needs_dump_, unlike both of the other recording paths. The
hash was therefore lost on the next restart, and that message stopped
having its TTL renewed, so it could expire from the swarm while still
being a live copy of a current key.

Reachable whenever an admin issues a supplemental carrying a key the
recipient already holds.
A keys message that expires from the swarm is currently unrecoverable: it
is signed by an admin and padded from the group secret key, so a member
cannot regenerate one, and only an admin rekey repairs the group. Retain
the raw bytes of each message named in active_msgs_ so recovery can push
them back verbatim, landing on the same message hash.

Stored by hash rather than generation because one generation carries the
full rekey plus every supplemental issued against it; a member that
receives only some of them does not get the key. Recovery must therefore
re-store every held message for a generation, not one of them.

The cache is pruned from active_msgs_ as the authority in a single place
rather than alongside each site that drops hashes, so a future way of
dropping a hash cannot leak bytes by forgetting to prune too. That
pruning is what bounds this to the same KEY_EXPIRY window as the keys
themselves, on disk as well as in memory.

Adds Keys::active_key_messages() and a groups_keys_active_message() C
shim returning borrowed bytes, following pending_config(). The dump gains
a "C" key, which sorts between the existing "A" and "L"; old dumps load
without it and old code skips it.

Only messages loaded after this ships are retained, so existing groups are
unchanged and a hash may legitimately have no bytes behind it.
@mpretty-cyro
mpretty-cyro requested a review from jagerman August 6, 2026 21:19
@mpretty-cyro mpretty-cyro self-assigned this Aug 6, 2026
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.

1 participant