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

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

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<int64_t, std::unordered_set<std::string>> 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<std::string, std::vector<unsigned char>> key_msgs_;
+
     sodium_cleared<std::array<unsigned char, 32>> pending_key_;
     sodium_vector<unsigned char> 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<const unsigned char> 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<const unsigned char> 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<std::string> 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<std::string, std::span<const unsigned char>> 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

src/config/groups/keys.cpp

@@ -86,6 +86,15 @@ std::vector<unsigned char> 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<const unsigned char> 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<const unsigned char> 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<std::span<const unsigned char>> Keys::pending_config() const {
     return std::span<const unsigned char>{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<const unsigned char> 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,7 +874,15 @@ 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 (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;
         }
 
@@ -856,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;
@@ -1102,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;
     }
@@ -1124,6 +1168,13 @@ std::unordered_set<std::string> Keys::active_hashes() const {
     return hashes;
 }
 
+std::map<std::string, std::span<const unsigned char>> Keys::active_key_messages() const {
+    std::map<std::string, std::span<const unsigned char>> msgs;
+    for (const auto& [hash, data] : key_msgs_)
+        msgs.emplace(hash, std::span<const unsigned char>{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
@@ -1170,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 {
@@ -1466,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();
 }

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