diff --git a/bindings/matrix-sdk-crypto-ffi/src/lib.rs b/bindings/matrix-sdk-crypto-ffi/src/lib.rs index 255f69d78..a8223e53e 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/lib.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/lib.rs @@ -506,6 +506,7 @@ fn collect_sessions( }) .collect::>()?, sender_data: SenderData::legacy(), + forwarder_data: None, room_id: RoomId::parse(session.room_id)?, imported: session.imported, backed_up: session.backed_up, diff --git a/crates/matrix-sdk-crypto/src/machine/tests/decryption_verification_state.rs b/crates/matrix-sdk-crypto/src/machine/tests/decryption_verification_state.rs index 0574d395d..bd678419e 100644 --- a/crates/matrix-sdk-crypto/src/machine/tests/decryption_verification_state.rs +++ b/crates/matrix-sdk-crypto/src/machine/tests/decryption_verification_state.rs @@ -450,6 +450,7 @@ async fn test_verification_states_multiple_device() { fake_room_id, &olm, SenderData::unknown(), + None, EventEncryptionAlgorithm::MegolmV1AesSha2, None, false, @@ -468,6 +469,7 @@ async fn test_verification_states_multiple_device() { fake_room_id, &olm, SenderData::unknown(), + None, EventEncryptionAlgorithm::MegolmV1AesSha2, None, false, diff --git a/crates/matrix-sdk-crypto/src/olm/account.rs b/crates/matrix-sdk-crypto/src/olm/account.rs index 290539785..30edc4fea 100644 --- a/crates/matrix-sdk-crypto/src/olm/account.rs +++ b/crates/matrix-sdk-crypto/src/olm/account.rs @@ -234,6 +234,7 @@ impl StaticAccountData { room_id, &outbound.session_key().await, own_sender_data, + None, algorithm, Some(visibility), shared_history, diff --git a/crates/matrix-sdk-crypto/src/olm/group_sessions/inbound.rs b/crates/matrix-sdk-crypto/src/olm/group_sessions/inbound.rs index 9fba02ab2..82a7bd8d9 100644 --- a/crates/matrix-sdk-crypto/src/olm/group_sessions/inbound.rs +++ b/crates/matrix-sdk-crypto/src/olm/group_sessions/inbound.rs @@ -187,6 +187,13 @@ pub struct InboundGroupSession { /// key. pub sender_data: SenderData, + /// If this session was shared-on-invite as part of an [MSC4268] key bundle, + /// information about the user who forwarded us the session information. + /// This is distinct from [`InboundGroupSession::sender_data`]. + /// + /// [MSC4268]: https://github.com/matrix-org/matrix-spec-proposals/pull/4268 + pub forwarder_data: Option, + /// The Room this GroupSession belongs to pub room_id: OwnedRoomId, @@ -240,6 +247,10 @@ impl InboundGroupSession { /// * `sender_data` - Information about the sender of the to-device message /// that established this session. /// + /// * `forwarder_data` - If present, indicates this session was received via + /// an [MSC4268] room key bundle, and provides information about the + /// forwarder of this bundle. + /// /// * `encryption_algorithm` - The [`EventEncryptionAlgorithm`] that should /// be used when messages are being decrypted. The method will return an /// [`SessionCreationError::Algorithm`] error if an algorithm we do not @@ -256,6 +267,7 @@ impl InboundGroupSession { /// history visibility of the room. /// /// [MSC3061]: https://github.com/matrix-org/matrix-spec-proposals/pull/3061 + /// [MSC4268]: https://github.com/matrix-org/matrix-spec-proposals/pull/4268 #[allow(clippy::too_many_arguments)] pub fn new( sender_key: Curve25519PublicKey, @@ -263,6 +275,7 @@ impl InboundGroupSession { room_id: &RoomId, session_key: &SessionKey, sender_data: SenderData, + forwarder_data: Option, encryption_algorithm: EventEncryptionAlgorithm, history_visibility: Option, shared_history: bool, @@ -286,6 +299,7 @@ impl InboundGroupSession { signing_keys: keys.into(), }, sender_data, + forwarder_data, room_id: room_id.into(), imported: false, algorithm: encryption_algorithm.into(), @@ -325,6 +339,7 @@ impl InboundGroupSession { room_id, session_key, SenderData::unknown(), + None, EventEncryptionAlgorithm::MegolmV1AesSha2, None, *shared_history, @@ -380,6 +395,7 @@ impl InboundGroupSession { sender_key: self.creator_info.curve25519_key, signing_key: (*self.creator_info.signing_keys).clone(), sender_data: self.sender_data.clone(), + forwarder_data: self.forwarder_data.clone(), room_id: self.room_id().to_owned(), imported: self.imported, backed_up: self.backed_up(), @@ -459,6 +475,7 @@ impl InboundGroupSession { sender_key, signing_key, sender_data, + forwarder_data, room_id, imported, backed_up, @@ -479,6 +496,7 @@ impl InboundGroupSession { signing_keys: signing_key.into(), }, sender_data, + forwarder_data, history_visibility: history_visibility.into(), first_known_index, room_id, @@ -691,6 +709,9 @@ pub struct PickledInboundGroupSession { /// Information on the device/sender who sent us this session #[serde(default)] pub sender_data: SenderData, + /// Information on the device/sender who forwarded us this session + #[serde(default)] + pub forwarder_data: Option, /// The id of the room that the session is used in. pub room_id: OwnedRoomId, /// Flag remembering if the session was directly sent to us by the sender @@ -717,10 +738,33 @@ fn default_algorithm() -> EventEncryptionAlgorithm { EventEncryptionAlgorithm::MegolmV1AesSha2 } -impl TryFrom<&HistoricRoomKey> for InboundGroupSession { - type Error = SessionCreationError; - - fn try_from(key: &HistoricRoomKey) -> Result { +impl HistoricRoomKey { + /// Converts a `HistoricRoomKey` into an `InboundGroupSession`. + /// + /// This method takes the current `HistoricRoomKey` instance and attempts to + /// create an `InboundGroupSession` from it. The `forwarder_data` parameter + /// provides information about the user or device that forwarded the session + /// information. This is normally distinct from the original sender of the + /// session. + /// + /// # Arguments + /// + /// * `forwarder_data` - A reference to a `SenderData` object containing + /// information about the forwarder of the session. + /// + /// # Returns + /// + /// Returns a `Result` containing the newly created `InboundGroupSession` on + /// success, or a `SessionCreationError` if the conversion fails. + /// + /// # Errors + /// + /// This method will return a `SessionCreationError` if the session + /// configuration for the given algorithm cannot be determined. + pub fn try_into_inbound_group_session( + &self, + forwarder_data: &SenderData, + ) -> Result { let HistoricRoomKey { algorithm, room_id, @@ -728,7 +772,7 @@ impl TryFrom<&HistoricRoomKey> for InboundGroupSession { session_id, session_key, sender_claimed_keys, - } = key; + } = self; let config = OutboundGroupSession::session_config(algorithm)?; let session = InnerSession::import(session_key, config); @@ -744,6 +788,7 @@ impl TryFrom<&HistoricRoomKey> for InboundGroupSession { // TODO: How do we remember that this is a historic room key and events decrypted using // this room key should always show some form of warning. sender_data: SenderData::default(), + forwarder_data: Some(forwarder_data.clone()), history_visibility: None.into(), first_known_index, room_id: room_id.to_owned(), @@ -784,6 +829,7 @@ impl TryFrom<&ExportedRoomKey> for InboundGroupSession { // TODO: In future, exported keys should contain sender data that we can use here. // See https://github.com/matrix-org/matrix-rust-sdk/issues/3548 sender_data: SenderData::default(), + forwarder_data: None, history_visibility: None.into(), first_known_index, room_id: room_id.to_owned(), @@ -815,6 +861,7 @@ impl From<&ForwardedMegolmV1AesSha2Content> for InboundGroupSession { // In future, exported keys should contain sender data that we can use here. // See https://github.com/matrix-org/matrix-rust-sdk/issues/3548 sender_data: SenderData::default(), + forwarder_data: None, history_visibility: None.into(), first_known_index, room_id: value.room_id.to_owned(), @@ -842,6 +889,7 @@ impl From<&ForwardedMegolmV2AesSha2Content> for InboundGroupSession { // In future, exported keys should contain sender data that we can use here. // See https://github.com/matrix-org/matrix-rust-sdk/issues/3548 sender_data: SenderData::default(), + forwarder_data: None, history_visibility: None.into(), first_known_index, room_id: value.room_id.to_owned(), @@ -871,7 +919,7 @@ impl TryFrom<&DecryptedForwardedRoomKeyEvent> for InboundGroupSession { #[cfg(test)] mod tests { use assert_matches2::assert_let; - use insta::assert_json_snapshot; + use insta::{assert_json_snapshot, with_settings}; use matrix_sdk_test::async_test; use ruma::{ DeviceId, UserId, device_id, events::room::history_visibility::HistoryVisibility, @@ -906,11 +954,17 @@ mod tests { let pickle = session.pickle().await; - assert_json_snapshot!(pickle, { - ".pickle.initial_ratchet.inner" => "[ratchet]", - ".pickle.signing_key" => "[signing_key]", - ".sender_key" => "[sender_key]", - ".signing_key.ed25519" => "[ed25519_key]", + with_settings!({prepend_module_to_snapshot => false}, { + assert_json_snapshot!( + "InboundGroupSession__test_pickle_snapshot__regression", + pickle, + { + ".pickle.initial_ratchet.inner" => "[ratchet]", + ".pickle.signing_key" => "[signing_key]", + ".sender_key" => "[sender_key]", + ".signing_key.ed25519" => "[ed25519_key]", + } + ); }); } @@ -982,6 +1036,7 @@ mod tests { room_id!("!test:localhost"), &create_session_key(), SenderData::unknown(), + None, EventEncryptionAlgorithm::MegolmV1AesSha2, Some(HistoryVisibility::Shared), false, @@ -1028,6 +1083,7 @@ mod tests { "legacy_session":false } }, + "forwarder_data":null, "room_id":"!test:localhost", "imported":false, "backed_up":false, diff --git a/crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs b/crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs index e00f458c0..c60b76671 100644 --- a/crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs +++ b/crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data_finder.rs @@ -826,6 +826,7 @@ mod tests { room_id, &session_key, SenderData::unknown(), + None, EventEncryptionAlgorithm::MegolmV1AesSha2, None, false, diff --git a/crates/matrix-sdk-crypto/src/olm/group_sessions/snapshots/matrix_sdk_crypto__olm__group_sessions__inbound__tests__pickle_snapshot.snap b/crates/matrix-sdk-crypto/src/olm/group_sessions/snapshots/InboundGroupSession__test_pickle_snapshot__regression.snap similarity index 96% rename from crates/matrix-sdk-crypto/src/olm/group_sessions/snapshots/matrix_sdk_crypto__olm__group_sessions__inbound__tests__pickle_snapshot.snap rename to crates/matrix-sdk-crypto/src/olm/group_sessions/snapshots/InboundGroupSession__test_pickle_snapshot__regression.snap index 8b69cf401..3ca8da7da 100644 --- a/crates/matrix-sdk-crypto/src/olm/group_sessions/snapshots/matrix_sdk_crypto__olm__group_sessions__inbound__tests__pickle_snapshot.snap +++ b/crates/matrix-sdk-crypto/src/olm/group_sessions/snapshots/InboundGroupSession__test_pickle_snapshot__regression.snap @@ -23,6 +23,7 @@ expression: pickle "legacy_session": false } }, + "forwarder_data": null, "room_id": "!test:localhost", "imported": false, "backed_up": false, diff --git a/crates/matrix-sdk-crypto/src/store/integration_tests.rs b/crates/matrix-sdk-crypto/src/store/integration_tests.rs index 3dcafb523..3f9b032f5 100644 --- a/crates/matrix-sdk-crypto/src/store/integration_tests.rs +++ b/crates/matrix-sdk-crypto/src/store/integration_tests.rs @@ -1430,6 +1430,7 @@ macro_rules! cryptostore_integration_tests { room_id!("!r:s.co"), &session_key, sender_data, + None, EventEncryptionAlgorithm::MegolmV1AesSha2, None, false, diff --git a/crates/matrix-sdk-crypto/src/store/memorystore.rs b/crates/matrix-sdk-crypto/src/store/memorystore.rs index fcd6c2294..7230f96ae 100644 --- a/crates/matrix-sdk-crypto/src/store/memorystore.rs +++ b/crates/matrix-sdk-crypto/src/store/memorystore.rs @@ -847,6 +847,7 @@ mod tests { room_id, &outbound.session_key().await, SenderData::unknown(), + None, outbound.settings().algorithm.to_owned(), None, false, @@ -1245,6 +1246,7 @@ mod tests { room_id, &outbound.session_key().await, SenderData::unknown(), + None, outbound.settings().algorithm.to_owned(), None, false, diff --git a/crates/matrix-sdk-crypto/src/store/mod.rs b/crates/matrix-sdk-crypto/src/store/mod.rs index add3c682f..96d2c47ec 100644 --- a/crates/matrix-sdk-crypto/src/store/mod.rs +++ b/crates/matrix-sdk-crypto/src/store/mod.rs @@ -1455,7 +1455,19 @@ impl Store { from_backup_version: Option<&str>, progress_listener: impl Fn(usize, usize), ) -> Result { - let exported_keys: Vec<&ExportedRoomKey> = exported_keys.iter().collect(); + let exported_keys = exported_keys.iter().filter_map(|key| { + key.try_into() + .map_err(|e| { + warn!( + sender_key = key.sender_key().to_base64(), + room_id = ?key.room_id(), + session_id = key.session_id(), + error = ?e, + "Couldn't import a room key from a file export." + ); + }) + .ok() + }); self.import_sessions_impl(exported_keys, from_backup_version, progress_listener).await } @@ -1493,57 +1505,44 @@ impl Store { self.import_room_keys(exported_keys, None, progress_listener).await } - async fn import_sessions_impl( + async fn import_sessions_impl( &self, - room_keys: Vec, + sessions: impl Iterator, from_backup_version: Option<&str>, progress_listener: impl Fn(usize, usize), - ) -> Result - where - T: TryInto + RoomKeyExport + Copy, - T::Error: Debug, - { - let mut sessions = Vec::new(); + ) -> Result { + let sessions: Vec<_> = sessions.collect(); + let mut imported_sessions = Vec::new(); - let total_count = room_keys.len(); + let total_count = sessions.len(); let mut keys = BTreeMap::new(); - for (i, key) in room_keys.into_iter().enumerate() { - match key.try_into() { - Ok(session) => { - // Only import the session if we didn't have this session or - // if it's a better version of the same session. - if let Some(merged) = self.merge_received_group_session(session).await? { - if from_backup_version.is_some() { - merged.mark_as_backed_up(); - } - - keys.entry(merged.room_id().to_owned()) - .or_insert_with(BTreeMap::new) - .entry(merged.sender_key().to_base64()) - .or_insert_with(BTreeSet::new) - .insert(merged.session_id().to_owned()); - - sessions.push(merged); - } - } - Err(e) => { - warn!( - sender_key = key.sender_key().to_base64(), - room_id = ?key.room_id(), - session_id = key.session_id(), - error = ?e, - "Couldn't import a room key from a file export." - ); + for (i, session) in sessions.into_iter().enumerate() { + // Only import the session if we didn't have this session or + // if it's a better version of the same session. + if let Some(merged) = self.merge_received_group_session(session).await? { + if from_backup_version.is_some() { + merged.mark_as_backed_up(); } + + keys.entry(merged.room_id().to_owned()) + .or_insert_with(BTreeMap::new) + .entry(merged.sender_key().to_base64()) + .or_insert_with(BTreeSet::new) + .insert(merged.session_id().to_owned()); + + imported_sessions.push(merged); } progress_listener(i, total_count); } - let imported_count = sessions.len(); + let imported_count = imported_sessions.len(); - self.inner.store.save_inbound_group_sessions(sessions, from_backup_version).await?; + self.inner + .store + .save_inbound_group_sessions(imported_sessions, from_backup_version) + .await?; info!(total_count, imported_count, room_keys = ?keys, "Successfully imported room keys"); @@ -1706,6 +1705,9 @@ impl Store { tracing::Span::current().record("sender_data", tracing::field::debug(&sender_data)); + // The sender's device must be either `SenderData::SenderUnverified` (i.e., + // TOFU-trusted) or `SenderData::SenderVerified` (i.e., fully verified + // via user verification and cross-signing). if matches!( &sender_data, SenderData::UnknownDevice { .. } @@ -1718,7 +1720,8 @@ impl Store { return Ok(()); } - self.import_room_key_bundle_sessions(bundle_info, &bundle, progress_listener).await?; + self.import_room_key_bundle_sessions(bundle_info, &bundle, &sender_data, progress_listener) + .await?; self.import_room_key_bundle_withheld_info(bundle_info, &bundle).await?; Ok(()) @@ -1728,6 +1731,7 @@ impl Store { &self, bundle_info: &StoredRoomKeyBundleData, bundle: &RoomKeyBundle, + forwarder_data: &SenderData, progress_listener: impl Fn(usize, usize), ) -> Result<(), CryptoStoreError> { let (good, bad): (Vec<_>, Vec<_>) = bundle.room_keys.iter().partition_map(|key| { @@ -1768,7 +1772,21 @@ impl Store { ); } - self.import_sessions_impl(good, None, progress_listener).await?; + let keys = good.iter().filter_map(|key| { + key.try_into_inbound_group_session(forwarder_data) + .map_err(|e| { + warn!( + sender_key = ?key.sender_key().to_base64(), + room_id = ?key.room_id(), + session_id = key.session_id(), + error = ?e, + "Couldn't import a room key from a key bundle." + ); + }) + .ok() + }); + + self.import_sessions_impl(keys, None, progress_listener).await?; } } @@ -1985,6 +2003,7 @@ mod tests { BTreeMap::new(), crate::types::Signatures::new(), )), + None, EventEncryptionAlgorithm::MegolmV1AesSha2, Some(ruma::events::room::history_visibility::HistoryVisibility::Shared), true, @@ -2343,6 +2362,17 @@ mod tests { assert_eq!(imported_sessions.len(), 1); assert_eq!(imported_sessions[0].room_id(), room_id); + // The session forwarder data should be set correctly. + assert_eq!( + imported_sessions[0] + .forwarder_data + .as_ref() + .expect("Session should contain forwarder data.") + .user_id() + .expect("Forwarder data should contain user ID."), + alice.user_id() + ); + assert_matches!( bob.store() .get_withheld_info(room_id, sessions[1].session_id()) @@ -2430,6 +2460,7 @@ mod tests { room_id, session_key, SenderData::unknown(), + None, #[cfg(not(feature = "experimental-algorithms"))] EventEncryptionAlgorithm::MegolmV1AesSha2, #[cfg(feature = "experimental-algorithms")] diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs index ae8624353..6ed4f1783 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs @@ -524,6 +524,7 @@ mod tests { &room_id, session_key, SenderData::unknown(), + None, encryption_algorithm, history_visibility, false, @@ -683,6 +684,7 @@ mod tests { ) .unwrap(), SenderData::legacy(), + None, EventEncryptionAlgorithm::MegolmV1AesSha2, None, false, @@ -703,6 +705,7 @@ mod tests { ) .unwrap(), SenderData::legacy(), + None, EventEncryptionAlgorithm::MegolmV1AesSha2, None, false, diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs index efe745cce..2fbe6f33f 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs @@ -2067,6 +2067,7 @@ mod unit_tests { ) .unwrap(), sender_data, + None, EventEncryptionAlgorithm::MegolmV1AesSha2, None, false, diff --git a/crates/matrix-sdk/tests/integration/encryption/backups.rs b/crates/matrix-sdk/tests/integration/encryption/backups.rs index ca56b5862..245cfb71d 100644 --- a/crates/matrix-sdk/tests/integration/encryption/backups.rs +++ b/crates/matrix-sdk/tests/integration/encryption/backups.rs @@ -1540,6 +1540,7 @@ async fn inbound_session_from_outbound_session( room_id, &outbound_group_session.session_key().await, SenderData::unknown(), + None, EventEncryptionAlgorithm::MegolmV1AesSha2, None, false,