crypto: support for building key bundles #4775

richvdh · 2025-03-10T13:07:53Z

Add a method to CryptoStore which will construct a key bundle, ready for encrypting and sharing with invited users.

Fixes #4504.

crates/matrix-sdk-crypto/src/store/mod.rs

codecov · 2025-03-10T16:02:43Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.12%. Comparing base (3d653d3) to head (a870c02).
Report is 113 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4775   +/-   ##
=======================================
  Coverage   86.11%   86.12%           
=======================================
  Files         292      293    +1     
  Lines       34369    34397   +28     
=======================================
+ Hits        29596    29623   +27     
- Misses       4773     4774    +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

poljar

Looks good minus some nits about type names and documentation.

Mainly requesting changes to see what you'll decide to do with the tests.

poljar · 2025-03-12T12:03:17Z

crates/matrix-sdk-crypto/src/store/mod.rs

@@ -1987,6 +1992,36 @@ impl Store {
        Ok(futures_util::stream::iter(sessions.into_iter().filter(predicate))
            .then(|session| async move { session.export().await }))
    }
+
+    /// Assemble a room key bundle for sharing encrypted history, as per
+    /// MSC4268.


Can we please link to the MSC if we mention it. I have some scripts that find the PR just from MSCXXXX but other people might not have such advanced technology in their IDE.

yup. Done in 89dee25.

poljar · 2025-03-12T12:10:05Z

crates/matrix-sdk-crypto/src/store/mod.rs

+        // We sort the sessions in the bundle, so that the snapshot is stable.
+        bundle.room_keys.sort_by_key(|session| session.session_id.clone());
+        insta::with_settings!({ sort_maps => true }, {
+            assert_json_snapshot!(bundle);


An alternative to this might be to "redact" or rather replace the random bytes in the snapshot with something stable like I did in c1e28aa:

matrix-rust-sdk/crates/matrix-sdk-crypto/src/olm/group_sessions/inbound.rs

Lines 770 to 775 in 53853c2

assert_json_snapshot!(pickle, {

".pickle.initial_ratchet.inner" => "[ratchet]",

".pickle.signing_key" => "[signing_key]",

".sender_key" => "[sender_key]",

".signing_key.ed25519" => "[ed25519_key]",

});

That would avoid having all the test data here as well as in the snapshot.

Unfortunately, this doesn't work very well here. Because there are several sessions, and we want to check which end up in which section of the output, the redactions get very fiddly. If we wanted to go down that route, it would be simpler to write out the test code by hand.

What does work quite well is a halfway house: we can generate the OlmMachines dynamically whilst still hardcoding the megolm sessions.

Done in 6515b6f.

poljar · 2025-03-12T12:10:35Z

crates/matrix-sdk-crypto/src/types/room_history.rs

+limitations under the License.
+*/
+
+//! Types for sharing encrypted room history, per MSC4268


Link to the MSC pretty please.

poljar · 2025-03-12T12:11:28Z

crates/matrix-sdk-crypto/src/types/room_history.rs

+    pub withheld: Vec<RoomKeyWithheldContent>,
+}
+
+/// An [`InboundGroupSession`] for sharing as part of the key bundle.


We should probably describe how this differs from other 4 different room key types we have.

Would be neat to document the dangers of accepting such a room key and that they need to be treated somewhat specially, i.e. they will never be fully trusted like the room keys we receive directly from the creator.

I've added a bunch of documentation which I hope will help, in 27bfeb8

poljar · 2025-03-12T12:13:53Z

crates/matrix-sdk-crypto/src/types/room_history.rs

+
+/// An [`InboundGroupSession`] for sharing as part of the key bundle.
+#[derive(Deserialize, Serialize)]
+pub struct SharedRoomKey {


I find that this name doesn't tell me much, every room key has been shared with you or you're going to share it.

Perhaps we could rename the type and the whole concept as something like a HistoricRoomKey, which would fit to the planned UI description of "History shared as presented by".

Renamed to HistoricRoomKey

poljar · 2025-03-12T12:17:34Z

crates/matrix-sdk-crypto/src/types/room_history.rs

+impl Debug for SharedRoomKey {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("SharedRoomKey")
+            .field("algorithm", &self.algorithm)
+            .field("room_id", &self.room_id)
+            .field("sender_key", &self.sender_key)
+            .field("session_id", &self.session_id)
+            .field("sender_claimed_keys", &self.sender_claimed_keys)
+            .finish_non_exhaustive()
+    }
+}


Might make sense to add a snapshot test for this using assert_debug_snapshot!().

done in 36f74ed

poljar

Thanks for adding so many docs for the different room key types, this is a much needed overview.

poljar · 2025-03-16T18:56:17Z

crates/matrix-sdk-crypto/src/olm/group_sessions/inbound.rs

+/// 3. [`ExportedRoomKey`] is very similar to `ForwardedRoomKeyContent`. The
+///    only difference is that the original sender's Ed25519 key is embedded in
+///    a `sender_claimed_keys` map rather than a top-level
+///    `sender_claimed_ed25519_key` field.


I almost forgot about this, now I'm sad again.

Add a method to CryptoStore which will construct a key bundle, ready for encrypting and sharing with invited users. Part of #4504

richvdh force-pushed the rav/history_sharing/room_key_bundle branch 3 times, most recently from 2098247 to 1faf406 Compare March 10, 2025 15:22

richvdh commented Mar 10, 2025

View reviewed changes

crates/matrix-sdk-crypto/src/store/mod.rs Show resolved Hide resolved

richvdh marked this pull request as ready for review March 11, 2025 10:09

richvdh requested review from a team as code owners March 11, 2025 10:09

richvdh requested review from stefanceriu and poljar and removed request for a team March 11, 2025 10:09

poljar requested changes Mar 12, 2025

View reviewed changes

stefanceriu removed their request for review March 14, 2025 07:24

richvdh requested a review from poljar March 14, 2025 16:11

poljar approved these changes Mar 16, 2025

View reviewed changes

richvdh added 2 commits March 17, 2025 11:24

crypto: support for building key bundles

002e776

Add a method to CryptoStore which will construct a key bundle, ready for encrypting and sharing with invited users. Part of #4504

test: snapshot test for HistoricRoomKey::debug

a870c02

richvdh force-pushed the rav/history_sharing/room_key_bundle branch from 36f74ed to a870c02 Compare March 17, 2025 11:24

richvdh enabled auto-merge March 17, 2025 11:25

richvdh merged commit ad0223c into main Mar 17, 2025
43 checks passed

richvdh deleted the rav/history_sharing/room_key_bundle branch March 17, 2025 11:45

	assert_json_snapshot!(pickle, {
	".pickle.initial_ratchet.inner" => "[ratchet]",
	".pickle.signing_key" => "[signing_key]",
	".sender_key" => "[sender_key]",
	".signing_key.ed25519" => "[ed25519_key]",
	});

crypto: support for building key bundles #4775

crypto: support for building key bundles #4775

Uh oh!

Conversation

richvdh commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

poljar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poljar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

richvdh commented Mar 10, 2025 •

edited

Loading

codecov bot commented Mar 10, 2025 •

edited

Loading