From fcecddb365492374b027ff0bc1da8f951115bc25 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 6 Aug 2026 14:49:37 +1000 Subject: [PATCH 1/3] tests: prove config re-store is byte-identical and seqno-neutral 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. --- tests/CMakeLists.txt | 1 + tests/test_config_determinism.cpp | 511 ++++++++++++++++++++++++++++++ 2 files changed, 512 insertions(+) create mode 100644 tests/test_config_determinism.cpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2ed0d1af5..8995fca36 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -15,6 +15,7 @@ set(LIB_SESSION_UTESTS_SOURCES test_configdata.cpp test_config_contacts.cpp test_config_convo_info_volatile.cpp + test_config_determinism.cpp test_config_local.cpp test_config_pro.cpp test_curve25519.cpp diff --git a/tests/test_config_determinism.cpp b/tests/test_config_determinism.cpp new file mode 100644 index 000000000..e77ddbbaf --- /dev/null +++ b/tests/test_config_determinism.cpp @@ -0,0 +1,511 @@ +// Round-trip determinism of config storage. +// +// Config recovery (re-storing an unchanged config to refresh its TTL, rather than pushing a new +// revision) depends on two properties that were previously only established by reading the code: +// +// 1. Loading a config from its stored dump and re-pushing it reproduces the message that was +// originally received from the swarm *byte for byte*. The storage server hashes the +// ciphertext, so byte-equality is what makes a re-store an idempotent TTL refresh instead of a +// second message. +// 2. `push()` on a clean config does not bump the seqno, so a re-store is not a new revision and +// provokes no merge. +// +// The tests below exercise the full round trip (push -> receive -> dump -> reload -> push), not +// just "encrypt the same bytes twice": recovery happens after a restart, so the reload path is the +// part that has to be reproducible — including a signature the reloading device cannot re-derive. + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "utils.hpp" + +using namespace session; +using namespace session::config; + +namespace { + +// The user's ed25519 seed; also the config encryption key for user configs. +const auto user_seed = "0123456789abcdef0123456789abcdef00000000000000000000000000000000"_hexbytes; + +// Group identity keypair (the admin holds the secret key; members hold only the pubkey). +const auto group_seed = "0123456789abcdef0123456789abcdeffedcba9876543210fedcba9876543210"_hexbytes; + +// The group's symmetric config encryption key (normally handed out by groups::Keys). +const auto group_enc_key = + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"_hexbytes; + +struct GroupKeys { + std::array pk; + std::array sk; +}; + +GroupKeys group_keys() { + GroupKeys k{}; + crypto_sign_ed25519_seed_keypair(k.pk.data(), k.sk.data(), group_seed.data()); + return k; +} + +// A single incoming message as `merge()` wants it. +std::vector>> incoming( + std::string hash, std::vector data) { + std::vector>> configs; + configs.emplace_back(std::move(hash), std::move(data)); + return configs; +} + +// Strip the null prefix padding that `pad_message` adds, so the result is the bencoded config. +std::vector unpad(std::vector plain) { + auto it = std::find_if(plain.begin(), plain.end(), [](unsigned char c) { return c != 0; }); + plain.erase(plain.begin(), it); + return plain; +} + +} // namespace + +TEST_CASE("config re-store is byte-identical: user config", "[config][determinism][user]") { + + // === The device that creates the config pushes it to the swarm === + + UserProfile a{to_span(user_seed), std::nullopt}; + a.set_name("Determinism"); + a.set_profile_pic( + "http://example.com/12345", + "abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcd"_hexbytes); + + auto [seqno, pushes, obs] = a.push(); + REQUIRE(pushes.size() == 1); + CHECK(seqno == 1); + // `received` stands in for the message as it lands on (and is fetched back from) the swarm. + const auto received = pushes[0]; + a.confirm_pushed(seqno, {"fakehash1"}); + + // User configs really are protobuf-wrapped: the stored bytes are not the config ciphertext, + // they unwrap *to* it. This is the layer most likely to smuggle in a timestamp. + CHECK_THROWS(decrypt(received, user_seed, "UserProfile")); + std::vector unwrapped; + REQUIRE_NOTHROW(unwrapped = protos::unwrap_config(user_seed, received, Namespace::UserProfile)); + CHECK_NOTHROW(decrypt(unwrapped, user_seed, "UserProfile")); + + SECTION("the pushing device reproduces it after a dump/reload") { + auto dump = a.dump(); + UserProfile reloaded{to_span(user_seed), to_span(dump)}; + + CHECK_FALSE(reloaded.needs_push()); + auto [s, p, o] = reloaded.push(); + CHECK(s == seqno); + REQUIRE(p.size() == 1); + CHECK(to_hex(p[0]) == to_hex(received)); + CHECK(o.empty()); + } + + SECTION("a receiving device reproduces it after a dump/reload") { + UserProfile b{to_span(user_seed), std::nullopt}; + CHECK(b.merge(incoming("fakehash1", received)) == std::unordered_set{{"fakehash1"s}}); + CHECK_FALSE(b.needs_push()); + CHECK(b.get_name() == "Determinism"); + + auto dump = b.dump(); + UserProfile reloaded{to_span(user_seed), to_span(dump)}; + + CHECK_FALSE(reloaded.needs_push()); + auto [s, p, o] = reloaded.push(); + CHECK(s == seqno); + REQUIRE(p.size() == 1); + CHECK(to_hex(p[0]) == to_hex(received)); + CHECK(o.empty()); + } +} + +TEST_CASE("config re-store is byte-identical: group config", "[config][determinism][groups]") { + + auto gk = group_keys(); + + groups::Info admin{gk.pk, to_span(gk.sk), std::nullopt}; + admin.add_key(group_enc_key, false); + admin.set_name("Determinism Group"); + admin.set_expiry_timer(1h); + admin.set_created(1682529839); + + auto [seqno, pushes, obs] = admin.push(); + REQUIRE(pushes.size() == 1); + CHECK(seqno == 1); + const auto received = pushes[0]; + admin.confirm_pushed(seqno, {"fakehash1"}); + + // Group configs go up raw — no protobuf wrapper — so the stored bytes decrypt straight to a + // bencoded config dict with the group's config key. + std::vector plain; + REQUIRE_NOTHROW(plain = unpad(decrypt(received, group_enc_key, "groups::Info"))); + REQUIRE_FALSE(plain.empty()); + CHECK(plain.front() == 'd'); + + SECTION("the pushing admin reproduces it after a dump/reload") { + auto dump = admin.dump(); + groups::Info reloaded{gk.pk, to_span(gk.sk), to_span(dump)}; + reloaded.add_key(group_enc_key, false); + + CHECK_FALSE(reloaded.needs_push()); + auto [s, p, o] = reloaded.push(); + CHECK(s == seqno); + REQUIRE(p.size() == 1); + CHECK(to_hex(p[0]) == to_hex(received)); + CHECK(o.empty()); + } + + SECTION("a receiving admin reproduces it after a dump/reload") { + groups::Info admin2{gk.pk, to_span(gk.sk), std::nullopt}; + admin2.add_key(group_enc_key, false); + CHECK(admin2.merge(incoming("fakehash1", received)) == std::unordered_set{{"fakehash1"s}}); + CHECK_FALSE(admin2.needs_push()); + + auto dump = admin2.dump(); + groups::Info reloaded{gk.pk, to_span(gk.sk), to_span(dump)}; + reloaded.add_key(group_enc_key, false); + + auto [s, p, o] = reloaded.push(); + CHECK(s == seqno); + REQUIRE(p.size() == 1); + CHECK(to_hex(p[0]) == to_hex(received)); + CHECK(o.empty()); + } +} + +TEST_CASE( + "config re-store is byte-identical: multipart config", "[config][determinism][multipart]") { + + // A config too big for one message is split into parts (and, note, skips protobuf wrapping + // entirely — see the `else` branch in ConfigBase::push()). Recovery has to re-store every + // part, so each one has to come back byte-identical after a dump/reload, including the + // reassembly state the dump carries in its "*" key. + Contacts contacts{to_span(user_seed), std::nullopt}; + + for (size_t i = 0; i < 3000; i++) { + // Random (i.e. poorly compressible) session ids, so the config gets big enough to split. + std::mt19937_64 rng{i}; + std::array random_sessionid; + random_sessionid[0] = 0x05; + for (int j = 1; j < 33; j += 8) + oxenc::write_host_as_little(rng(), random_sessionid.data() + j); + + auto c = contacts.get_or_construct(oxenc::to_hex(random_sessionid)); + c.nickname = "My friend {}"_format(i); + c.approved = true; + contacts.set(c); + } + + auto [seqno, pushes, obs] = contacts.push(); + REQUIRE(pushes.size() > 1); + const auto received = pushes; + + std::unordered_set hashes; + std::vector>> configs; + for (size_t i = 0; i < received.size(); i++) { + auto hash = "fakehash_part{}"_format(i); + hashes.insert(hash); + configs.emplace_back(std::move(hash), received[i]); + } + contacts.confirm_pushed(seqno, hashes); + + SECTION("the pushing device reproduces every part after a dump/reload") { + auto dump = contacts.dump(); + Contacts reloaded{to_span(user_seed), to_span(dump)}; + + CHECK_FALSE(reloaded.needs_push()); + auto [s, p, o] = reloaded.push(); + CHECK(s == seqno); + REQUIRE(p.size() == received.size()); + for (size_t i = 0; i < p.size(); i++) + CHECK(to_hex(p[i]) == to_hex(received[i])); + CHECK(o.empty()); + } + + SECTION("a receiving device reproduces every part after a dump/reload") { + Contacts b{to_span(user_seed), std::nullopt}; + CHECK(b.merge(configs) == hashes); + CHECK_FALSE(b.needs_push()); + + auto dump = b.dump(); + Contacts reloaded{to_span(user_seed), to_span(dump)}; + + CHECK_FALSE(reloaded.needs_push()); + auto [s, p, o] = reloaded.push(); + CHECK(s == seqno); + REQUIRE(p.size() == received.size()); + for (size_t i = 0; i < p.size(); i++) + CHECK(to_hex(p[i]) == to_hex(received[i])); + CHECK(o.empty()); + } +} + +TEST_CASE( + "config re-store is byte-identical: dump with a retained signature", + "[config][determinism][groups][signature]") { + + // The load-bearing case for group recovery: a non-admin member has no signing key, so it can + // only reproduce the admin's bytes if the signature survives dump -> reload (make_dump() + // serialises with signing disabled, but writes the stored signature; the reload trusts it). + auto gk = group_keys(); + + groups::Info admin{gk.pk, to_span(gk.sk), std::nullopt}; + admin.add_key(group_enc_key, false); + admin.set_name("Signed Group"); + admin.set_created(1682529839); + auto [seqno, pushes, obs] = admin.push(); + REQUIRE(pushes.size() == 1); + const auto received = pushes[0]; + admin.confirm_pushed(seqno, {"fakehash1"}); + + // The message is signed: the bencoded config ends with a "~" key holding the 64-byte + // signature, i.e. `1:~64:<64 bytes>` followed by the dict's closing `e`. + auto plain = unpad(decrypt(received, group_enc_key, "groups::Info")); + REQUIRE(plain.size() > 71); + CHECK(printable(std::span{plain}.subspan(plain.size() - 71, 6)) == "1:~64:"); + CHECK(plain.back() == 'e'); + + groups::Info member{gk.pk, std::nullopt, std::nullopt}; + member.add_key(group_enc_key, false); + CHECK(member.merge(incoming("fakehash1", received)) == std::unordered_set{{"fakehash1"s}}); + REQUIRE(member.is_readonly()); + CHECK(member.get_name() == "Signed Group"); + + auto dump = member.dump(); + groups::Info reloaded{gk.pk, std::nullopt, to_span(dump)}; + reloaded.add_key(group_enc_key, false); + REQUIRE(reloaded.is_readonly()); + + CHECK_FALSE(reloaded.needs_push()); + auto [s, p, o] = reloaded.push(); + CHECK(s == seqno); + REQUIRE(p.size() == 1); + CHECK(to_hex(p[0]) == to_hex(received)); + CHECK(o.empty()); + + // ...and the re-store is still a validly signed config, not merely the right length: another + // member verifies the signature against the group pubkey when it merges, and would throw + // (dropping the message) if it no longer matched. + groups::Info member2{gk.pk, std::nullopt, std::nullopt}; + member2.add_key(group_enc_key, false); + CHECK(member2.merge(incoming("fakehash1", p[0])) == std::unordered_set{{"fakehash1"s}}); + CHECK(member2.get_name() == "Signed Group"); +} + +TEST_CASE("push on a clean config does not bump the seqno", "[config][determinism][seqno]") { + + // Recovery must never dirty the config: dirtying bumps the seqno and turns an idempotent + // re-store into a new revision plus a merge. + + SECTION("user config") { + UserProfile a{to_span(user_seed), std::nullopt}; + a.set_name("Determinism"); + auto [seqno, pushes, obs] = a.push(); + a.confirm_pushed(seqno, {"fakehash1"}); + REQUIRE(a.is_clean()); + REQUIRE_FALSE(a.needs_push()); + + for (int i = 0; i < 3; i++) { + auto [s, p, o] = a.push(); + CHECK(s == seqno); + REQUIRE(p.size() == 1); + CHECK(to_hex(p[0]) == to_hex(pushes[0])); + CHECK(o.empty()); + CHECK(a.is_clean()); + CHECK_FALSE(a.is_dirty()); + CHECK_FALSE(a.needs_push()); + } + } + + SECTION("group config, admin") { + auto gk = group_keys(); + groups::Info admin{gk.pk, to_span(gk.sk), std::nullopt}; + admin.add_key(group_enc_key, false); + admin.set_name("Determinism Group"); + auto [seqno, pushes, obs] = admin.push(); + admin.confirm_pushed(seqno, {"fakehash1"}); + REQUIRE(admin.is_clean()); + REQUIRE_FALSE(admin.needs_push()); + + for (int i = 0; i < 3; i++) { + auto [s, p, o] = admin.push(); + CHECK(s == seqno); + REQUIRE(p.size() == 1); + CHECK(to_hex(p[0]) == to_hex(pushes[0])); + CHECK(o.empty()); + CHECK(admin.is_clean()); + CHECK_FALSE(admin.is_dirty()); + CHECK_FALSE(admin.needs_push()); + } + } + + SECTION("group config, read-only member") { + auto gk = group_keys(); + groups::Info admin{gk.pk, to_span(gk.sk), std::nullopt}; + admin.add_key(group_enc_key, false); + admin.set_name("Determinism Group"); + auto [seqno, pushes, obs] = admin.push(); + + groups::Info member{gk.pk, std::nullopt, std::nullopt}; + member.add_key(group_enc_key, false); + REQUIRE(member.merge(incoming("fakehash1", pushes[0])) == + std::unordered_set{{"fakehash1"s}}); + REQUIRE(member.is_readonly()); + REQUIRE_FALSE(member.needs_push()); + + for (int i = 0; i < 3; i++) { + auto [s, p, o] = member.push(); + CHECK(s == seqno); + REQUIRE(p.size() == 1); + CHECK(to_hex(p[0]) == to_hex(pushes[0])); + CHECK(o.empty()); + CHECK_FALSE(member.is_dirty()); + CHECK_FALSE(member.needs_push()); + } + } +} + +TEST_CASE( + "push on a clean config still consumes the obsolete-hash list", + "[config][determinism][seqno][obsolete]") { + + // A re-store leaves the seqno and the bytes alone, but it is NOT side-effect free: push() + // clears `_old_hashes` unconditionally (src/config/base.cpp:810-813), outside the + // `if (is_dirty())` guard above it. So a recovery push hands back the superseded hashes and + // forgets them. A caller that treats a recovery push as a no-op and discards its + // obsolete-hash return will leak those messages on the swarm: nothing reports them again, and + // the next dump no longer records them. + UserProfile a{to_span(user_seed), std::nullopt}; + a.set_name("First"); + auto [s1, p1, o1] = a.push(); + a.confirm_pushed(s1, {"fakehash1"}); + a.set_name("Second"); + auto [s2, p2, o2] = a.push(); + a.confirm_pushed(s2, {"fakehash2"}); + + // A device that fetches both messages at once: the seqno-2 message carries the seqno-1 diff, + // so it supersedes it and fakehash1 becomes obsolete. + UserProfile b{to_span(user_seed), std::nullopt}; + std::vector>> both; + both.emplace_back("fakehash1", p1[0]); + both.emplace_back("fakehash2", p2[0]); + CHECK(b.merge(both) == std::unordered_set{{"fakehash1"s, "fakehash2"s}}); + REQUIRE(b.is_clean()); + REQUIRE_FALSE(b.needs_push()); + + // The re-store itself is byte-identical and does not move the seqno, as everywhere above... + auto [s3, p3, o3] = b.push(); + CHECK(s3 == s2); + REQUIRE(p3.size() == 1); + CHECK(to_hex(p3[0]) == to_hex(p2[0])); + + // ...but it also hands back the obsolete hash and clears it. + CHECK(o3 == std::vector{"fakehash1"s}); + + auto [s4, p4, o4] = b.push(); + CHECK(s4 == s2); + REQUIRE(p4.size() == 1); + CHECK(to_hex(p4[0]) == to_hex(p2[0])); + CHECK(o4.empty()); // consumed by the previous push, not re-reported +} + +TEST_CASE( + "a read-only member re-stores but is never handed the obsolete hashes", + "[config][determinism][groups][obsolete]") { + + // The hand-back and the clear are gated differently (src/config/base.cpp:809-813): + // + // if (!is_readonly()) + // for (auto& old : _old_hashes) + // obs.push_back(std::move(old)); // <-- read-only: skipped + // _old_hashes.clear(); // <-- read-only: still happens + // + // So a read-only member — the actor that *can* re-store (see the retained-signature case) — + // never receives the superseded hashes at all. That is coherent rather than broken: a + // member's subaccount carries Write but not Delete, so it could not prune them anyway. But + // it means an EMPTY obsolete list is the *expected* result on the member path, not evidence + // that recovery failed, and superseded messages persist until an admin next pushes. + auto gk = group_keys(); + + groups::Info admin{gk.pk, to_span(gk.sk), std::nullopt}; + admin.add_key(group_enc_key, false); + admin.set_name("First"); + auto [s1, p1, o1] = admin.push(); + admin.confirm_pushed(s1, {"fakehash1"}); + admin.set_name("Second"); + auto [s2, p2, o2] = admin.push(); + admin.confirm_pushed(s2, {"fakehash2"}); + + std::vector>> both; + both.emplace_back("fakehash1", p1[0]); + both.emplace_back("fakehash2", p2[0]); + + SECTION("an admin that merged both is handed fakehash1") { + groups::Info admin2{gk.pk, to_span(gk.sk), std::nullopt}; + admin2.add_key(group_enc_key, false); + REQUIRE(admin2.merge(both) == std::unordered_set{{"fakehash1"s, "fakehash2"s}}); + REQUIRE_FALSE(admin2.is_readonly()); + + auto [s, p, o] = admin2.push(); + CHECK(s == s2); + REQUIRE(p.size() == 1); + CHECK(to_hex(p[0]) == to_hex(p2[0])); + CHECK(o == std::vector{"fakehash1"s}); + } + + SECTION("a read-only member that merged both is handed nothing") { + groups::Info member{gk.pk, std::nullopt, std::nullopt}; + member.add_key(group_enc_key, false); + REQUIRE(member.merge(both) == std::unordered_set{{"fakehash1"s, "fakehash2"s}}); + REQUIRE(member.is_readonly()); + + // The re-store still works — byte-identical, same seqno... + auto [s, p, o] = member.push(); + CHECK(s == s2); + REQUIRE(p.size() == 1); + CHECK(to_hex(p[0]) == to_hex(p2[0])); + + // ...but the obsolete hash is dropped rather than returned. + CHECK(o.empty()); + } +} + +TEST_CASE( + "the protobuf config wrapper contains no wall-clock or random data", + "[config][determinism][proto]") { + + // The wrapper is deterministic only because every non-deterministic element was deliberately + // pinned: Envelope.timestamp is hardcoded to 1, padding is a bare 0x80 with no random fill, + // and the inner encryption is encrypt_for_recipient_deterministic (src/config/protos.cpp). + // + // Adding a field carrying a wall-clock value or fresh randomness would silently break config + // re-store idempotency, and every test above would keep passing, because they only compare + // bytes produced within a single run. This golden digest is the tripwire for that: if it + // fails, check what changed in wrap_config before updating the constant. + const auto payload = "Hello from the other side"_bytes; + + const std::pair expected[] = { + {Namespace::UserProfile, + "e5bcbe0595e82d55e80079dc9cf8c8b6952c099ee8306c413833e0eecd45ce03"}, + {Namespace::Contacts, + "1573a2e233008a47f445a8f1b328fe37bcd49450c94b743202b68902a1a94a89"}, + {Namespace::ConvoInfoVolatile, + "121dabefb251fa586c14f7e9c82590361c2db4efcb9817771bb4d9b0aaba9362"}, + {Namespace::UserGroups, + "b78bd9b62719db76ff7a9ec59eef3de0b4aaee4e4d55b515f70b04d674f8b8a9"}, + }; + + for (const auto& [ns, digest] : expected) { + auto wrapped = protos::wrap_config(user_seed, payload, 1, ns); + CHECK(to_hex(session::hash::hash(32, wrapped)) == digest); + } +} From 7008cf61320dfd4032dd5e0d16b6c0653fd673f4 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 6 Aug 2026 14:52:45 +1000 Subject: [PATCH 2/3] groups::Keys: flag a dump when a known key arrives under a new hash 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. --- src/config/groups/keys.cpp | 6 ++++- tests/test_group_keys.cpp | 51 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/config/groups/keys.cpp b/src/config/groups/keys.cpp index cc42ab8d3..29685a4d9 100644 --- a/src/config/groups/keys.cpp +++ b/src/config/groups/keys.cpp @@ -843,7 +843,11 @@ void Keys::insert_key(std::string_view msg_hash, key_info&& new_key) { }); for (auto it = gen_begin; it != gen_end; ++it) if (it->key == new_key.key) { - active_msgs_[new_key.generation].emplace(msg_hash); + // We already have this key, but this may be a *different* message carrying it, in + // which case we want to keep renewing this copy too. Flag a dump only when the hash + // is actually new, so re-loading a message we already know stays free. + if (active_msgs_[new_key.generation].emplace(msg_hash).second) + needs_dump_ = true; return; } diff --git a/tests/test_group_keys.cpp b/tests/test_group_keys.cpp index 0af36f311..82c51e795 100644 --- a/tests/test_group_keys.cpp +++ b/tests/test_group_keys.cpp @@ -1037,3 +1037,54 @@ TEST_CASE("Group Keys promotion", "[config][groups][keys][promotion]") { CHECK(admin.info.get_name() == "new name"); } + +TEST_CASE( + "Group Keys - the same key under a second message hash", + "[config][groups][keys][recovery]") { + + // insert_key's early-return path: we already hold this key, but this is a *different* message + // carrying it. Both copies want renewing, so the new hash has to be recorded -- and persisted, + // or it is silently lost on the next restart and that message stops being renewed. + const std::vector group_seed = + "0123456789abcdeffedcba98765432100123456789abcdeffedcba9876543210"_hexbytes; + const std::vector admin_seed = + "0123456789abcdef0123456789abcdeffedcba9876543210fedcba9876543210"_hexbytes; + const std::vector member_seed = + "000111222333444555666777888999aaabbbcccdddeeefff0123456789abcdef"_hexbytes; + + std::array group_pk; + std::array group_sk; + crypto_sign_ed25519_seed_keypair(group_pk.data(), group_sk.data(), group_seed.data()); + + pseudo_client admin{admin_seed, true, group_pk.data(), group_sk.data()}; + pseudo_client member{member_seed, false, group_pk.data(), std::nullopt}; + + for (const auto* c : {&admin, &member}) { + auto m = admin.members.get_or_construct(c->session_id); + m.admin = (c == &admin); + admin.members.set(m); + } + auto rekey1 = session::to_vector(admin.keys.rekey(admin.info, admin.members)); + + constexpr int64_t t0 = 1'700'000'000'000; + REQUIRE(admin.keys.load_key_message("keyhash1", rekey1, t0, admin.info, admin.members)); + REQUIRE(member.keys.load_key_message("keyhash1", rekey1, t0, member.info, member.members)); + + member.keys.dump(); // clear needs_dump + REQUIRE_FALSE(member.keys.needs_dump()); + + CHECK(member.keys.load_key_message("keyhash1b", rekey1, t0, member.info, member.members)); + CHECK(member.keys.active_hashes().count("keyhash1b")); + CHECK(member.keys.needs_dump()); + + // Re-loading a message we already know changes nothing, so it must not ask for a dump. + member.keys.dump(); + CHECK(member.keys.load_key_message("keyhash1b", rekey1, t0, member.info, member.members)); + CHECK_FALSE(member.keys.needs_dump()); + + // The second copy survives the round trip, so it is still renewable after a restart. + auto dump = member.keys.dump(); + pseudo_client reloaded{ + member_seed, false, group_pk.data(), std::nullopt, std::nullopt, std::nullopt, dump}; + CHECK(reloaded.keys.active_hashes().count("keyhash1b")); +} From f7c67cf77c7ebea62fa6b3811012e3d7f4fc0da5 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 6 Aug 2026 14:54:17 +1000 Subject: [PATCH 3/3] groups::Keys: retain active keys message bytes so they can be re-stored 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. --- include/session/config/groups/keys.h | 28 +++ include/session/config/groups/keys.hpp | 48 ++++- src/config/groups/keys.cpp | 79 ++++++- tests/test_group_keys.cpp | 278 +++++++++++++++++++++++++ 4 files changed, 425 insertions(+), 8 deletions(-) diff --git a/include/session/config/groups/keys.h b/include/session/config/groups/keys.h index 33c42368b..0a029d564 100644 --- a/include/session/config/groups/keys.h +++ b/include/session/config/groups/keys.h @@ -293,6 +293,34 @@ LIBSESSION_EXPORT bool groups_keys_load_message( /// to the caller and must be free()d when done. LIBSESSION_EXPORT config_string_list* groups_keys_active_hashes(const config_group_keys* conf); +/// API: groups/groups_keys_active_message +/// +/// Retrieves the raw bytes of an active keys message previously loaded by this device, by its +/// message hash. Use this together with `groups_keys_active_hashes` to re-store a keys message +/// that has expired from the swarm: the bytes must be pushed back *unchanged*, which is what lets +/// a non-admin do it at all (a keys message is admin-signed and cannot be regenerated by a member). +/// +/// Only messages loaded since this device began retaining them are available, so a hash returned by +/// `groups_keys_active_hashes` may legitimately have no bytes here. That means "cannot recover +/// this one", not an error. +/// +/// Inputs: +/// - `conf` -- [in] Pointer to the keys config object +/// - `msg_hash` -- [in] Null-terminated C string containing the message hash +/// - `data` -- [out] Set to a pointer to the message bytes, if found +/// - `datalen` -- [out] Set to the length of `data`, if found +/// +/// Outputs: +/// - `true` if the message was found, with `data`/`datalen` set. The pointer belongs to `conf` and +/// must NOT be free()d; it is invalidated by anything that modifies `conf` (loading a message, +/// rekeying, etc.), so copy the bytes if you need to keep them. +/// - `false` if no bytes are retained for that hash, leaving `data`/`datalen` untouched. +LIBSESSION_EXPORT bool groups_keys_active_message( + const config_group_keys* conf, + const char* msg_hash, + const unsigned char** data, + size_t* datalen); + /// API: groups/groups_keys_needs_rekey /// /// Checks whether a rekey is required (for instance, because of key generation conflict). Note diff --git a/include/session/config/groups/keys.hpp b/include/session/config/groups/keys.hpp index 5eaa27bb4..115ac1719 100644 --- a/include/session/config/groups/keys.hpp +++ b/include/session/config/groups/keys.hpp @@ -108,6 +108,18 @@ class Keys : public ConfigSig { /// Hashes of messages we have successfully parsed; used for deciding what needs to be renewed. std::map> active_msgs_; + /// The raw bytes of the messages named in `active_msgs_`, keyed by hash so that one generation + /// can hold both the full rekey message and every supplemental issued against it. + /// + /// We keep these so that an expired keys message can be re-stored *verbatim*: a keys message + /// carries an admin signature that a member cannot produce, and its junk padding derives from + /// `_sign_sk`, so re-pushing bytes we already hold is the only way a non-admin can put one + /// back. It is not secret — the same bytes sit on the swarm — so this is a plain vector + /// rather than a `sodium_vector`. + /// + /// Kept in lockstep with `active_msgs_` by `remove_expired()`; nothing else may add to it. + std::map> key_msgs_; + sodium_cleared> pending_key_; sodium_vector pending_key_config_; int64_t pending_gen_ = -1; @@ -125,11 +137,20 @@ class Keys : public ConfigSig { // Checks for and drops expired keys. void remove_expired(); + // Drops any retained message bytes whose hash is no longer in `active_msgs_`. Derived from + // `active_msgs_` rather than repeated at each place that drops hashes, so that a new way of + // dropping a hash cannot leak bytes by forgetting to prune here as well. + void prune_key_msgs(); + // Loads existing state from a previous dump of keys data void load_dump(std::span dump); - // Inserts a key into the correct place in `keys_`. - void insert_key(std::string_view message_hash, key_info&& key); + // Inserts a key into the correct place in `keys_`. `message_data` is the raw message the key + // came from, retained alongside the hash so it can be re-stored verbatim later. + void insert_key( + std::string_view message_hash, + std::span message_data, + key_info&& key); // Returned the blinding factor for a given session X25519 pubkey. This depends on the group's // seed and thus is only obtainable by an admin account. @@ -596,6 +617,29 @@ class Keys : public ConfigSig { /// - vector of message hashes std::unordered_set active_hashes() const; + /// API: groups/Keys::active_key_messages + /// + /// Returns the raw bytes of the currently active keys messages, keyed by message hash. These + /// are the same messages `active_hashes()` names; this gives you the contents as well, so that + /// a message which has expired from the swarm can be re-stored *verbatim*. + /// + /// Re-storing the bytes unchanged is what makes this usable by a non-admin: a keys message is + /// signed by an admin and padded from the group secret key, so it cannot be regenerated by a + /// member, but bytes already held can be pushed back as-is and land on the same message hash. + /// + /// Only messages loaded by this device since it began retaining them appear here, so a group + /// whose keys messages all predate that support will return fewer entries than + /// `active_hashes()` — or none at all. Callers must treat a missing entry as "cannot recover + /// this one" rather than as an error. + /// + /// Inputs: none + /// + /// Outputs: + /// - map of message hash to the message bytes. The spans point at data owned by this object + /// and are invalidated by anything that modifies it (e.g. `load_key_message`, `rekey`), + /// exactly as for `pending_config()`. + std::map> active_key_messages() const; + /// API: groups/Keys::needs_rekey /// /// Returns true if the key list requires a new key to be generated and pushed to the server (by diff --git a/src/config/groups/keys.cpp b/src/config/groups/keys.cpp index 29685a4d9..81e0fe532 100644 --- a/src/config/groups/keys.cpp +++ b/src/config/groups/keys.cpp @@ -86,6 +86,15 @@ std::vector Keys::make_dump() const { } } + { + // Raw bytes of the messages named in "A", so they can be re-stored verbatim if they expire + // from the swarm. `key_msgs_` is a std::map, so this comes out in the sorted order bt_dict + // requires. Old dumps simply won't have this key, and old code skips it. + auto msgs = d.append_dict("C"); + for (const auto& [hash, data] : key_msgs_) + msgs.append(hash, to_string_view(data)); + } + { auto keys = d.append_list("L"); for (auto& k : keys_) { @@ -123,6 +132,16 @@ void Keys::load_dump(std::span dump) { throw config_value_error{"Invalid Keys dump: `active` not found"}; } + // Optional: absent in dumps written before we retained message bytes, in which case we simply + // have hashes we cannot re-store. Not an error. + if (d.skip_until("C")) { + auto msgs = d.consume_dict_consumer(); + while (!msgs.is_finished()) { + auto [hash, data] = msgs.next_string(); + key_msgs_.emplace(hash, to_vector(data)); + } + } + if (d.skip_until("L")) { auto keys = d.consume_list_consumer(); while (!keys.is_finished()) { @@ -176,6 +195,17 @@ void Keys::load_dump(std::span dump) { std::to_string(pk.size()) + ")"}; std::memcpy(pending_key_.data(), pk.data(), pending_key_.size()); } + + // `key_msgs_` must never outlive the hashes in `active_msgs_`; enforce that on the way in too, + // so a hand-written or corrupted dump can't seed an entry that nothing will ever prune. + prune_key_msgs(); +} + +void Keys::prune_key_msgs() { + if (key_msgs_.empty()) + return; + auto keep = active_hashes(); + std::erase_if(key_msgs_, [&](const auto& item) { return !keep.count(item.first); }); } size_t Keys::size() const { @@ -833,7 +863,8 @@ std::optional> Keys::pending_config() const { return std::span{pending_key_config_.data(), pending_key_config_.size()}; } -void Keys::insert_key(std::string_view msg_hash, key_info&& new_key) { +void Keys::insert_key( + std::string_view msg_hash, std::span msg_data, key_info&& new_key) { // Find all keys with the same generation and see if our key is in there (that is: we are // deliberately ignoring timestamp so that we don't add the same key with slight timestamp // variations). @@ -843,10 +874,14 @@ void Keys::insert_key(std::string_view msg_hash, key_info&& new_key) { }); for (auto it = gen_begin; it != gen_end; ++it) if (it->key == new_key.key) { - // We already have this key, but this may be a *different* message carrying it, in - // which case we want to keep renewing this copy too. Flag a dump only when the hash - // is actually new, so re-loading a message we already know stays free. - if (active_msgs_[new_key.generation].emplace(msg_hash).second) + // We already have this key, but this may be a *different* message carrying it (the + // same key can arrive again under another hash), in which case we want to renew and be + // able to re-store this copy too. Flag a dump only when something actually changed, + // so re-loading a message we already know stays free. + bool new_hash = active_msgs_[new_key.generation].emplace(msg_hash).second; + bool new_bytes = + key_msgs_.try_emplace(std::string{msg_hash}, to_vector(msg_data)).second; + if (new_hash || new_bytes) needs_dump_ = true; return; } @@ -860,6 +895,7 @@ void Keys::insert_key(std::string_view msg_hash, key_info&& new_key) { return; active_msgs_[new_key.generation].emplace(msg_hash); + key_msgs_.insert_or_assign(std::string{msg_hash}, to_vector(msg_data)); keys_.insert(it, std::move(new_key)); remove_expired(); needs_dump_ = true; @@ -1106,14 +1142,18 @@ bool Keys::load_key_message( if (!new_keys.empty()) { for (auto& k : new_keys) - insert_key(hash, std::move(k)); + insert_key(hash, data, std::move(k)); auto new_key_list = group_keys(); members.replace_keys(new_key_list, /*dirty=*/false); info.replace_keys(new_key_list, /*dirty=*/false); return true; } else if (max_gen) { + // A valid keys message that held no key for us — a supplemental aimed at other members, + // typically. Still worth retaining: it is part of the generation, and a member who gets + // only some of a generation's messages doesn't get the key. active_msgs_[*max_gen].emplace(hash); + key_msgs_.insert_or_assign(std::string{hash}, to_vector(data)); remove_expired(); needs_dump_ = true; } @@ -1128,6 +1168,13 @@ std::unordered_set Keys::active_hashes() const { return hashes; } +std::map> Keys::active_key_messages() const { + std::map> msgs; + for (const auto& [hash, data] : key_msgs_) + msgs.emplace(hash, std::span{data.data(), data.size()}); + return msgs; +} + void Keys::remove_expired() { if (keys_.size() >= 2) { // When we're done, this will point at the first element we want to keep (i.e. we want to @@ -1174,6 +1221,11 @@ void Keys::remove_expired() { // something) and so it isn't really up to us to keep them alive, since that's a history of // the group we apparently don't have access to. active_msgs_.clear(); + + // Retained message bytes follow the hashes exactly, for both of the above branches. This is + // what bounds the size of `key_msgs_` (and hence of the dump) to the same KEY_EXPIRY window as + // the keys themselves; without it an expired generation's bytes would live forever on disk. + prune_key_msgs(); } bool Keys::needs_rekey() const { @@ -1470,6 +1522,21 @@ LIBSESSION_C_API config_string_list* groups_keys_active_hashes(const config_grou return make_string_list(unbox(conf).active_hashes()); } +LIBSESSION_C_API bool groups_keys_active_message( + const config_group_keys* conf, + const char* msg_hash, + const unsigned char** data, + size_t* datalen) { + assert(msg_hash && data && datalen); + auto msgs = unbox(conf).active_key_messages(); + if (auto it = msgs.find(msg_hash); it != msgs.end()) { + *data = it->second.data(); + *datalen = it->second.size(); + return true; + } + return false; +} + LIBSESSION_C_API bool groups_keys_needs_rekey(const config_group_keys* conf) { return unbox(conf).needs_rekey(); } diff --git a/tests/test_group_keys.cpp b/tests/test_group_keys.cpp index 82c51e795..72456c668 100644 --- a/tests/test_group_keys.cpp +++ b/tests/test_group_keys.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -1088,3 +1089,280 @@ TEST_CASE( member_seed, false, group_pk.data(), std::nullopt, std::nullopt, std::nullopt, dump}; CHECK(reloaded.keys.active_hashes().count("keyhash1b")); } + +// Recovery support: `Keys` retains the raw bytes of each active keys message so that a message +// which has expired from the swarm can be re-stored *verbatim*. Verbatim is the whole point — a +// keys message carries an admin signature and junk padding derived from the group secret key, so a +// member cannot regenerate one, but it can push back bytes it already holds. +namespace { + +// Rebuilds a Keys dump with the "C" (retained message bytes) key stripped, i.e. the format written +// before this feature existed. Used to check an old dump still loads. +std::vector strip_retained_messages(std::span dump) { + // Re-emit the raw sub-encodings rather than re-serialising values, so what comes out is + // byte-for-byte what the old code would have written. + oxenc::bt_dict_consumer d{dump}; + std::string out = "d"; + REQUIRE(d.skip_until("A")); + out += "1:A"; + out += d.consume_list_data(); + if (d.skip_until("C")) + d.consume_dict_data(); // drop it + REQUIRE(d.skip_until("L")); + out += "1:L"; + out += d.consume_list_data(); + if (d.skip_until("P")) { + out += "1:P"; + out += d.consume_dict_data(); + } + out += "e"; + return session::to_vector(out); +} + +} // namespace + +TEST_CASE("Group Keys - retained message bytes", "[config][groups][keys][recovery]") { + + const std::vector group_seed = + "0123456789abcdeffedcba98765432100123456789abcdeffedcba9876543210"_hexbytes; + const std::vector admin_seed = + "0123456789abcdef0123456789abcdeffedcba9876543210fedcba9876543210"_hexbytes; + const std::vector member_seed = + "000111222333444555666777888999aaabbbcccdddeeefff0123456789abcdef"_hexbytes; + const std::vector member2_seed = + "00011122435111155566677788811263446552465222efff0123456789abcdef"_hexbytes; + + std::array group_pk; + std::array group_sk; + crypto_sign_ed25519_seed_keypair(group_pk.data(), group_sk.data(), group_seed.data()); + + pseudo_client admin{admin_seed, true, group_pk.data(), group_sk.data()}; + pseudo_client member{member_seed, false, group_pk.data(), std::nullopt}; + pseudo_client member2{member2_seed, false, group_pk.data(), std::nullopt}; + + // Put the admin and member1 in the group, then rekey so member1 can actually read the message. + for (const auto* c : {&admin, &member}) { + auto m = admin.members.get_or_construct(c->session_id); + m.admin = (c == &admin); + admin.members.set(m); + } + auto rekey1 = session::to_vector(admin.keys.rekey(admin.info, admin.members)); + + constexpr int64_t t0 = 1'700'000'000'000; + + // The admin has to load its own rekey back to confirm it; until it does, the key is only + // pending and the generation doesn't advance. + REQUIRE(admin.keys.load_key_message("keyhash1", rekey1, t0, admin.info, admin.members)); + REQUIRE(member.keys.load_key_message("keyhash1", rekey1, t0, member.info, member.members)); + + SECTION("bytes are retained verbatim, and survive a dump/reload") { + auto held = member.keys.active_key_messages(); + REQUIRE(held.size() == 1); + REQUIRE(held.count("keyhash1")); + CHECK(to_hex(session::to_vector(held.at("keyhash1"))) == to_hex(rekey1)); + + // Every hash we advertise for renewal has bytes behind it. + CHECK(as_set(member.keys.active_hashes()) == std::set{{"keyhash1"s}}); + + auto dump = member.keys.dump(); + pseudo_client reloaded{ + member_seed, + false, + group_pk.data(), + std::nullopt, + std::nullopt, + std::nullopt, + dump}; + + auto held2 = reloaded.keys.active_key_messages(); + REQUIRE(held2.size() == 1); + REQUIRE(held2.count("keyhash1")); + CHECK(to_hex(session::to_vector(held2.at("keyhash1"))) == to_hex(rekey1)); + } + + SECTION("a supplemental is retained alongside the full message of the same generation") { + // This is why storage is keyed by hash rather than generation: one generation carries the + // full rekey plus every supplemental issued against it, and a member that receives only + // the supplemental does not get the key. + auto supp = admin.keys.key_supplement(member2.session_id); + // False here just means "no key in it for us" -- it is addressed to member2. We still + // retain it, which is the point: this is the path where a message is recorded against the + // generation without contributing a key, and a member who receives only the supplemental + // needs the full message too. + CHECK_FALSE(member.keys.load_key_message( + "keyhash2", supp, t0 + 1000, member.info, member.members)); + + auto held = member.keys.active_key_messages(); + REQUIRE(held.size() == 2); + REQUIRE(held.count("keyhash1")); + REQUIRE(held.count("keyhash2")); + CHECK(to_hex(session::to_vector(held.at("keyhash1"))) == to_hex(rekey1)); + CHECK(to_hex(session::to_vector(held.at("keyhash2"))) == to_hex(supp)); + + // Both belong to the same generation, and both are advertised for renewal. + CHECK(as_set(member.keys.active_hashes()) == + std::set{{"keyhash1"s, "keyhash2"s}}); + + // ...and both survive a dump/reload, which is what recovery after a restart depends on. + auto dump = member.keys.dump(); + pseudo_client reloaded{ + member_seed, + false, + group_pk.data(), + std::nullopt, + std::nullopt, + std::nullopt, + dump}; + auto held2 = reloaded.keys.active_key_messages(); + REQUIRE(held2.size() == 2); + CHECK(to_hex(session::to_vector(held2.at("keyhash1"))) == to_hex(rekey1)); + CHECK(to_hex(session::to_vector(held2.at("keyhash2"))) == to_hex(supp)); + } + + SECTION("pruning: bytes are dropped when their generation expires") { + // The generation-erase branch of remove_expired(). Expiry is driven by the swarm-provided + // message timestamps, not the system clock, so we just hand it messages far enough apart. + auto rekey2 = session::to_vector(admin.keys.rekey(admin.info, admin.members)); + REQUIRE(admin.keys.load_key_message( + "keyhash2", rekey2, t0 + 1000, admin.info, admin.members)); + REQUIRE(member.keys.load_key_message( + "keyhash2", rekey2, t0 + 1000, member.info, member.members)); + CHECK(member.keys.active_key_messages().size() == 2); + + // A third generation, more than KEY_EXPIRY (60d) after the second, retires the first. + constexpr int64_t past_expiry = t0 + 1000 + 61 * 24 * 60 * 60 * 1000LL; + auto rekey3 = session::to_vector(admin.keys.rekey(admin.info, admin.members)); + REQUIRE(admin.keys.load_key_message( + "keyhash3", rekey3, past_expiry, admin.info, admin.members)); + REQUIRE(member.keys.load_key_message( + "keyhash3", rekey3, past_expiry, member.info, member.members)); + + auto held = member.keys.active_key_messages(); + CHECK_FALSE(held.count("keyhash1")); + + // The invariant that bounds this cache: it holds exactly the hashes we still advertise. + std::set held_hashes; + for (const auto& [h, _] : held) + held_hashes.insert(h); + CHECK(held_hashes == as_set(member.keys.active_hashes())); + + // And the shrink is real on disk, not just in memory. + auto dump = member.keys.dump(); + pseudo_client reloaded{ + member_seed, + false, + group_pk.data(), + std::nullopt, + std::nullopt, + std::nullopt, + dump}; + CHECK_FALSE(reloaded.keys.active_key_messages().count("keyhash1")); + } + + SECTION("pruning: bytes are dropped when we hold no keys at all") { + // The other branch of remove_expired(): `keys_` empty means we aren't keeping any of the + // group's history, so active_msgs_ is cleared outright. member2 was never given the keys, + // so it reaches this by loading a supplemental addressed to somebody else. + auto supp = admin.keys.key_supplement(member.session_id); + + REQUIRE(member2.keys.size() == 0); + CHECK_FALSE(member2.keys.load_key_message( + "keyhash9", supp, t0 + 1000, member2.info, member2.members)); + + CHECK(member2.keys.active_hashes().empty()); + CHECK(member2.keys.active_key_messages().empty()); + } + + SECTION("dump compatibility, both directions") { + auto supp = admin.keys.key_supplement(member2.session_id); + // False here just means "no key in it for us" -- it is addressed to member2. We still + // retain it, which is the point: this is the path where a message is recorded against the + // generation without contributing a key, and a member who receives only the supplemental + // needs the full message too. + CHECK_FALSE(member.keys.load_key_message( + "keyhash2", supp, t0 + 1000, member.info, member.members)); + auto dump = member.keys.dump(); + + SECTION("an old dump loads under new code") { + auto old_dump = strip_retained_messages(dump); + pseudo_client reloaded{ + member_seed, + false, + group_pk.data(), + std::nullopt, + std::nullopt, + std::nullopt, + old_dump}; + + // The hashes still load; we simply have no bytes for them, which is exactly the + // position of every group that predates this feature. + CHECK(as_set(reloaded.keys.active_hashes()) == + std::set{{"keyhash1"s, "keyhash2"s}}); + CHECK(reloaded.keys.active_key_messages().empty()); + + // Still fully functional: it can decrypt with the keys it loaded. + CHECK(reloaded.keys.size() == member.keys.size()); + } + + SECTION("a new dump loads under old code") { + // Old code reads with skip_until over "A", "L", "P" and never asks for "C", so the + // unknown key is skipped. Parsing the new dump the way the old code does must still + // yield the same "A" and "L". + oxenc::bt_dict_consumer d{dump}; + REQUIRE(d.skip_until("A")); + auto active = d.consume_list_consumer(); + std::set hashes; + while (!active.is_finished()) { + auto lst = active.consume_list_consumer(); + lst.consume_integer(); // generation + while (!lst.is_finished()) + hashes.insert(lst.consume_string()); + } + CHECK(hashes == std::set{{"keyhash1"s, "keyhash2"s}}); + + // "C" sorts between "A" and "L", so a consumer that skips straight to "L" must still + // find it -- this is the assertion that would fail if the new key were misplaced. + REQUIRE(d.skip_until("L")); + auto keys = d.consume_list_consumer(); + CHECK_FALSE(keys.is_finished()); + } + } + + SECTION("C API") { + auto dump = member.keys.dump(); + config_group_keys* conf; + config_object* info_conf; + config_object* mem_conf; + char err[256]; + REQUIRE(groups_info_init(&info_conf, group_pk.data(), nullptr, nullptr, 0, err) == 0); + REQUIRE(groups_members_init(&mem_conf, group_pk.data(), nullptr, nullptr, 0, err) == 0); + + auto member_sk = sk_from_seed(member_seed); + REQUIRE(groups_keys_init( + &conf, + member_sk.data(), + group_pk.data(), + nullptr, + info_conf, + mem_conf, + dump.data(), + dump.size(), + err) == 0); + + const unsigned char* data = nullptr; + size_t datalen = 0; + CHECK(groups_keys_active_message(conf, "keyhash1", &data, &datalen)); + REQUIRE(data); + CHECK(to_hex(std::vector{data, data + datalen}) == to_hex(rekey1)); + + // A hash we don't hold bytes for is a plain "no", not an error. + const unsigned char* missing = nullptr; + size_t missinglen = 0; + CHECK_FALSE(groups_keys_active_message(conf, "nosuchhash", &missing, &missinglen)); + CHECK(missing == nullptr); + + groups_keys_free(conf); + config_free(info_conf); + config_free(mem_conf); + } +}