From 7784f3e11a4c25a374f1f2b21e9a6e3b96477eca Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 6 Aug 2024 21:49:25 +0200 Subject: [PATCH 01/12] crypto: extract function that checks if session is shared too much --- .../group_sessions/share_strategy.rs | 80 ++++++++++++------- 1 file changed, 53 insertions(+), 27 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs index c2bec06f962..7f6e36728f6 100644 --- a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs +++ b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs @@ -162,33 +162,7 @@ pub(crate) async fn collect_session_recipients( // of the devices in the session got deleted or blacklisted in the // meantime. If so, we should also rotate the session. if !should_rotate { - // Device IDs that should receive this session - let recipient_device_ids: BTreeSet<&DeviceId> = - recipients.iter().map(|d| d.device_id()).collect(); - - if let Some(shared) = outbound.shared_with_set.read().unwrap().get(user_id) { - // Devices that received this session - let shared: BTreeSet = shared.keys().cloned().collect(); - let shared: BTreeSet<&DeviceId> = shared.iter().map(|d| d.as_ref()).collect(); - - // The set difference between - // - // 1. Devices that had previously received the session, and - // 2. Devices that would now receive the session - // - // Represents newly deleted or blacklisted devices. If this - // set is non-empty, we must rotate. - let newly_deleted_or_blacklisted = - shared.difference(&recipient_device_ids).collect::>(); - - should_rotate = !newly_deleted_or_blacklisted.is_empty(); - if should_rotate { - debug!( - "Rotating a room key due to these devices being deleted/blacklisted {:?}", - newly_deleted_or_blacklisted, - ); - } - }; + should_rotate = is_session_overshared_for_user(outbound, user_id, &recipients) } devices.entry(user_id.to_owned()).or_default().extend(recipients); @@ -209,6 +183,58 @@ pub(crate) async fn collect_session_recipients( Ok(CollectRecipientsResult { should_rotate, devices, withheld_devices }) } +/// Check if the session has been shared with a device belonging to the given +/// user, that is no longer in the pool of devices that should participate in +/// the discussion. +/// +/// # Arguments +/// +/// * `outbound_session` - the outbound group session to check for oversharing. +/// * `user_id` - the ID of the user we are checking the devices for. +/// * `recipient_devices` - the list of devices belonging to `user_id` that we +/// expect to share the session with. +/// +/// # Returns +/// +/// `true` if the session has been shared with any devices belonging to +/// `user_id` that are not in `recipient_devices`. Otherwise, `false`. +fn is_session_overshared_for_user( + outbound_session: &OutboundGroupSession, + user_id: &UserId, + recipient_devices: &[DeviceData], +) -> bool { + // Device IDs that should receive this session + let recipient_device_ids: BTreeSet<&DeviceId> = + recipient_devices.iter().map(|d| d.device_id()).collect(); + + if let Some(shared) = outbound_session.shared_with_set.read().unwrap().get(user_id) { + // Devices that received this session + let shared: BTreeSet = shared.keys().cloned().collect(); + let shared: BTreeSet<&DeviceId> = shared.iter().map(|d| d.as_ref()).collect(); + + // The set difference between + // + // 1. Devices that had previously received the session, and + // 2. Devices that would now receive the session + // + // Represents newly deleted or blacklisted devices. If this + // set is non-empty, we must rotate. + let newly_deleted_or_blacklisted = + shared.difference(&recipient_device_ids).collect::>(); + + let should_rotate = !newly_deleted_or_blacklisted.is_empty(); + if should_rotate { + debug!( + "Rotating a room key due to these devices being deleted/blacklisted {:?}", + newly_deleted_or_blacklisted, + ); + } + should_rotate + } else { + false + } +} + struct RecipientDevices { allowed_devices: Vec, denied_devices_with_code: Vec<(DeviceData, WithheldCode)>, From cf046daabee1f1761acdedd17a5cb76938a91301 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 14 Aug 2024 12:49:21 +0100 Subject: [PATCH 02/12] crypto: minor cleanups in `is_session_overshared_for_user` --- .../group_sessions/share_strategy.rs | 51 ++++++++++--------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs index 7f6e36728f6..03f0e880eda 100644 --- a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs +++ b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs @@ -207,32 +207,33 @@ fn is_session_overshared_for_user( let recipient_device_ids: BTreeSet<&DeviceId> = recipient_devices.iter().map(|d| d.device_id()).collect(); - if let Some(shared) = outbound_session.shared_with_set.read().unwrap().get(user_id) { - // Devices that received this session - let shared: BTreeSet = shared.keys().cloned().collect(); - let shared: BTreeSet<&DeviceId> = shared.iter().map(|d| d.as_ref()).collect(); - - // The set difference between - // - // 1. Devices that had previously received the session, and - // 2. Devices that would now receive the session - // - // Represents newly deleted or blacklisted devices. If this - // set is non-empty, we must rotate. - let newly_deleted_or_blacklisted = - shared.difference(&recipient_device_ids).collect::>(); - - let should_rotate = !newly_deleted_or_blacklisted.is_empty(); - if should_rotate { - debug!( - "Rotating a room key due to these devices being deleted/blacklisted {:?}", - newly_deleted_or_blacklisted, - ); - } - should_rotate - } else { - false + let guard = outbound_session.shared_with_set.read().unwrap(); + + let Some(shared) = guard.get(user_id) else { + return false; + }; + + // Devices that received this session + let shared: BTreeSet<&DeviceId> = shared.keys().map(|d| d.as_ref()).collect(); + + // The set difference between + // + // 1. Devices that had previously received the session, and + // 2. Devices that would now receive the session + // + // Represents newly deleted or blacklisted devices. If this + // set is non-empty, we must rotate. + let newly_deleted_or_blacklisted = + shared.difference(&recipient_device_ids).collect::>(); + + let should_rotate = !newly_deleted_or_blacklisted.is_empty(); + if should_rotate { + debug!( + "Rotating a room key due to these devices being deleted/blacklisted {:?}", + newly_deleted_or_blacklisted, + ); } + should_rotate } struct RecipientDevices { From 326e87d4c7466651318e08b73e19e400f9db40a9 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 9 Aug 2024 16:51:49 +0100 Subject: [PATCH 03/12] crypto: rafactor split_recipients_withhelds_for_user Use a for loop rather than `partition_map`. We're about to add a third list, so partition_map won't work. (partition_map ends up using Vec::push under the hood, so this is pretty much equivalent.) --- .../group_sessions/share_strategy.rs | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs index 03f0e880eda..a120a679afd 100644 --- a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs +++ b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs @@ -14,6 +14,7 @@ use std::{ collections::{BTreeMap, BTreeSet, HashMap}, + default::Default, ops::Deref, }; @@ -236,6 +237,7 @@ fn is_session_overshared_for_user( should_rotate } +#[derive(Default)] struct RecipientDevices { allowed_devices: Vec, denied_devices_with_code: Vec<(DeviceData, WithheldCode)>, @@ -248,23 +250,21 @@ fn split_recipients_withhelds_for_user( only_allow_trusted_devices: bool, ) -> RecipientDevices { // From all the devices a user has, we're splitting them into two - // buckets, a bucket of devices that should receive the - // room key and a bucket of devices that should receive + // buckets: a bucket of devices that should receive the + // room key, and a bucket of devices that should receive // a withheld code. - let (recipients, withheld_recipients): (Vec, Vec<(DeviceData, WithheldCode)>) = - user_devices.into_values().partition_map(|d| { - if d.is_blacklisted() { - Either::Right((d, WithheldCode::Blacklisted)) - } else if only_allow_trusted_devices - && !d.is_verified(own_identity, device_owner_identity) - { - Either::Right((d, WithheldCode::Unverified)) - } else { - Either::Left(d) - } - }); - - RecipientDevices { allowed_devices: recipients, denied_devices_with_code: withheld_recipients } + let mut recipient_devices: RecipientDevices = Default::default(); + for d in user_devices.into_values() { + if d.is_blacklisted() { + recipient_devices.denied_devices_with_code.push((d, WithheldCode::Blacklisted)); + } else if only_allow_trusted_devices && !d.is_verified(own_identity, device_owner_identity) + { + recipient_devices.denied_devices_with_code.push((d, WithheldCode::Unverified)); + } else { + recipient_devices.allowed_devices.push(d); + } + } + recipient_devices } fn split_recipients_withhelds_for_user_based_on_identity( From f555e076d5b13bb66037b96c5188956a031b4f19 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 13 Aug 2024 14:55:24 +0100 Subject: [PATCH 04/12] crypto: add `OwnUserIdentityData::is_identity_verified` ... and use it to remove a bit of duplicated code. --- .../src/identities/device.rs | 30 ++++++++----------- .../matrix-sdk-crypto/src/identities/user.rs | 26 ++++++++++++---- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/identities/device.rs b/crates/matrix-sdk-crypto/src/identities/device.rs index a7d0161fe0e..a1ec2e1748e 100644 --- a/crates/matrix-sdk-crypto/src/identities/device.rs +++ b/crates/matrix-sdk-crypto/src/identities/device.rs @@ -297,9 +297,7 @@ impl Device { self.device_owner_identity.as_ref().is_some_and(|id| match id { UserIdentityData::Own(own_identity) => own_identity.is_verified(), UserIdentityData::Other(other_identity) => { - self.own_identity.as_ref().is_some_and(|oi| { - oi.is_verified() && oi.is_identity_signed(other_identity).is_ok() - }) + self.own_identity.as_ref().is_some_and(|oi| oi.is_identity_verified(other_identity)) } }) } @@ -744,21 +742,19 @@ impl DeviceData { ) -> bool { own_identity.as_ref().zip(device_owner.as_ref()).is_some_and( |(own_identity, device_identity)| { - // Our own identity needs to be marked as verified. - own_identity.is_verified() - && match device_identity { - // If it's one of our own devices, just check that - // we signed the device. - UserIdentityData::Own(_) => own_identity.is_device_signed(self).is_ok(), - - // If it's a device from someone else, first check - // that our user has signed the other user and then - // check if the other user has signed this device. - UserIdentityData::Other(device_identity) => { - own_identity.is_identity_signed(device_identity).is_ok() - && device_identity.is_device_signed(self).is_ok() - } + match device_identity { + UserIdentityData::Own(_) => { + own_identity.is_verified() && own_identity.is_device_signed(self).is_ok() } + + // If it's a device from someone else, first check + // that our user has verified the other user and then + // check if the other user has signed this device. + UserIdentityData::Other(device_identity) => { + own_identity.is_identity_verified(device_identity) + && device_identity.is_device_signed(self).is_ok() + } + } }, ) } diff --git a/crates/matrix-sdk-crypto/src/identities/user.rs b/crates/matrix-sdk-crypto/src/identities/user.rs index ca7cd387e86..7eba6ea3573 100644 --- a/crates/matrix-sdk-crypto/src/identities/user.rs +++ b/crates/matrix-sdk-crypto/src/identities/user.rs @@ -228,11 +228,9 @@ impl Deref for UserIdentity { impl UserIdentity { /// Is this user identity verified. pub fn is_verified(&self) -> bool { - self.own_identity.as_ref().is_some_and(|own_identity| { - // The identity of another user is verified iff our own identity is verified and - // if our own identity has signed the other user's identity. - own_identity.is_verified() && own_identity.is_identity_signed(&self.inner).is_ok() - }) + self.own_identity + .as_ref() + .is_some_and(|own_identity| own_identity.is_identity_verified(&self.inner)) } /// Manually verify this user. @@ -886,8 +884,26 @@ impl OwnUserIdentityData { &self.user_signing_key } + /// Check if the given user identity has been verified. + /// + /// The identity of another user is verified iff our own identity is + /// verified and if our own identity has signed the other user's + /// identity. + /// + /// # Arguments + /// + /// * `identity` - The identity of another user which we want to check has + /// been verified. + pub fn is_identity_verified(&self, identity: &OtherUserIdentityData) -> bool { + self.is_verified() && self.is_identity_signed(identity).is_ok() + } + /// Check if the given identity has been signed by this identity. /// + /// Note that, normally, you'll also want to check that the + /// `OwnUserIdentityData` has been verified; for that, + /// [`Self::is_identity_verified`] is more appropriate. + /// /// # Arguments /// /// * `identity` - The identity of another user that we want to check if it From ede5e5ad431b517da671d647c927f9935c47be5c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 14 Aug 2024 12:19:13 +0100 Subject: [PATCH 05/12] crypto: remove `CollectStrategy::new_device_based` The list of boolean arguments is confusing. We may as well just construct the `DeviceBasedStrategy` directly. --- bindings/matrix-sdk-crypto-ffi/src/lib.rs | 4 +++- .../matrix-sdk-crypto/src/machine/tests/mod.rs | 2 +- .../src/olm/group_sessions/outbound.rs | 2 +- .../src/session_manager/group_sessions/mod.rs | 8 ++++++-- .../group_sessions/share_strategy.rs | 17 +++++++---------- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/bindings/matrix-sdk-crypto-ffi/src/lib.rs b/bindings/matrix-sdk-crypto-ffi/src/lib.rs index 1d34424b990..4ebcc2273eb 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/lib.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/lib.rs @@ -677,7 +677,9 @@ impl From for RustEncryptionSettings { rotation_period: Duration::from_secs(v.rotation_period), rotation_period_msgs: v.rotation_period_msgs, history_visibility: v.history_visibility.into(), - sharing_strategy: CollectStrategy::new_device_based(v.only_allow_trusted_devices), + sharing_strategy: CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: v.only_allow_trusted_devices, + }, } } } diff --git a/crates/matrix-sdk-crypto/src/machine/tests/mod.rs b/crates/matrix-sdk-crypto/src/machine/tests/mod.rs index 4dc1750cdb6..8e85ca5493a 100644 --- a/crates/matrix-sdk-crypto/src/machine/tests/mod.rs +++ b/crates/matrix-sdk-crypto/src/machine/tests/mod.rs @@ -587,7 +587,7 @@ async fn test_withheld_unverified() { let encryption_settings = EncryptionSettings::default(); let encryption_settings = EncryptionSettings { - sharing_strategy: CollectStrategy::new_device_based(true), + sharing_strategy: CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: true }, ..encryption_settings }; diff --git a/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs b/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs index 8d32538f342..e91e1fc7aba 100644 --- a/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs +++ b/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs @@ -133,7 +133,7 @@ impl EncryptionSettings { rotation_period, rotation_period_msgs, history_visibility, - sharing_strategy: CollectStrategy::new_device_based(only_allow_trusted_devices), + sharing_strategy: CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices }, } } } diff --git a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/mod.rs b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/mod.rs index 662ea1da447..70c2c81b20a 100644 --- a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/mod.rs +++ b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/mod.rs @@ -1162,7 +1162,9 @@ mod tests { .any(|d| d.user_id() == user_id && d.device_id() == device_id)); let settings = EncryptionSettings { - sharing_strategy: CollectStrategy::new_device_based(true), + sharing_strategy: CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: true, + }, ..Default::default() }; let users = [user_id].into_iter(); @@ -1222,7 +1224,9 @@ mod tests { let users = keys_claim.one_time_keys.keys().map(Deref::deref); let settings = EncryptionSettings { - sharing_strategy: CollectStrategy::new_device_based(true), + sharing_strategy: CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: true, + }, ..Default::default() }; diff --git a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs index a120a679afd..0888e880321 100644 --- a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs +++ b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs @@ -51,11 +51,6 @@ pub enum CollectStrategy { } impl CollectStrategy { - /// Creates a new legacy strategy, based on per device trust. - pub const fn new_device_based(only_allow_trusted_devices: bool) -> Self { - CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices } - } - /// Creates an identity based strategy pub const fn new_identity_based() -> Self { CollectStrategy::IdentityBasedStrategy @@ -64,7 +59,7 @@ impl CollectStrategy { impl Default for CollectStrategy { fn default() -> Self { - CollectStrategy::new_device_based(false) + CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: false } } } @@ -363,7 +358,8 @@ mod tests { async fn test_share_with_per_device_strategy_to_all() { let machine = set_up_test_machine().await; - let legacy_strategy = CollectStrategy::new_device_based(false); + let legacy_strategy = + CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: false }; let encryption_settings = EncryptionSettings { sharing_strategy: legacy_strategy.clone(), ..Default::default() }; @@ -414,7 +410,8 @@ mod tests { let fake_room_id = room_id!("!roomid:localhost"); - let legacy_strategy = CollectStrategy::new_device_based(true); + let legacy_strategy = + CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: true }; let encryption_settings = EncryptionSettings { sharing_strategy: legacy_strategy.clone(), ..Default::default() }; @@ -561,7 +558,7 @@ mod tests { let fake_room_id = room_id!("!roomid:localhost"); - let strategy = CollectStrategy::new_device_based(false); + let strategy = CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: false }; let encryption_settings = EncryptionSettings { sharing_strategy: strategy.clone(), @@ -615,7 +612,7 @@ mod tests { let fake_room_id = room_id!("!roomid:localhost"); - let strategy = CollectStrategy::new_device_based(false); + let strategy = CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: false }; let encryption_settings = EncryptionSettings { sharing_strategy: strategy.clone(), ..Default::default() }; From cfe16c455ac1483c8ba75dd9a6abacf7a6561332 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 14 Aug 2024 14:33:57 +0100 Subject: [PATCH 06/12] crypto: change `EncryptionSettings::new` to take a `CollectStrategy` Again, the list of boolean arguments is confusing. --- crates/matrix-sdk-base/src/client.rs | 12 ++++++++--- .../src/olm/group_sessions/outbound.rs | 20 +++++++++++++------ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index d04f5e1cab3..a9f11926374 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -28,8 +28,8 @@ use futures_util::Stream; use matrix_sdk_common::instant::Instant; #[cfg(feature = "e2e-encryption")] use matrix_sdk_crypto::{ - store::DynCryptoStore, EncryptionSettings, EncryptionSyncChanges, OlmError, OlmMachine, - ToDeviceRequest, + store::DynCryptoStore, CollectStrategy, EncryptionSettings, EncryptionSyncChanges, OlmError, + OlmMachine, ToDeviceRequest, }; #[cfg(feature = "e2e-encryption")] use ruma::events::{ @@ -1391,7 +1391,13 @@ impl BaseClient { let members = self.store.get_user_ids(room_id, filter).await?; let settings = settings.ok_or(Error::EncryptionNotEnabled)?; - let settings = EncryptionSettings::new(settings, history_visibility, false); + let settings = EncryptionSettings::new( + settings, + history_visibility, + CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: false, + }, + ); Ok(o.share_room_key(room_id, members.iter().map(Deref::deref), settings).await?) } diff --git a/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs b/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs index e91e1fc7aba..e83a5bf3ccc 100644 --- a/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs +++ b/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs @@ -116,12 +116,11 @@ impl Default for EncryptionSettings { impl EncryptionSettings { /// Create new encryption settings using an `RoomEncryptionEventContent`, - /// a history visibility, and setting if only trusted devices should receive - /// a room key. + /// a history visibility, and key sharing strategy. pub fn new( content: RoomEncryptionEventContent, history_visibility: HistoryVisibility, - only_allow_trusted_devices: bool, + sharing_strategy: CollectStrategy, ) -> Self { let rotation_period: Duration = content.rotation_period_ms.map_or(ROTATION_PERIOD, |r| Duration::from_millis(r.into())); @@ -133,7 +132,7 @@ impl EncryptionSettings { rotation_period, rotation_period_msgs, history_visibility, - sharing_strategy: CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices }, + sharing_strategy, } } } @@ -782,12 +781,17 @@ mod tests { }; use super::{EncryptionSettings, ROTATION_MESSAGES, ROTATION_PERIOD}; + use crate::CollectStrategy; #[test] fn test_encryption_settings_conversion() { let mut content = RoomEncryptionEventContent::new(EventEncryptionAlgorithm::MegolmV1AesSha2); - let settings = EncryptionSettings::new(content.clone(), HistoryVisibility::Joined, false); + let settings = EncryptionSettings::new( + content.clone(), + HistoryVisibility::Joined, + CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: false }, + ); assert_eq!(settings.rotation_period, ROTATION_PERIOD); assert_eq!(settings.rotation_period_msgs, ROTATION_MESSAGES); @@ -795,7 +799,11 @@ mod tests { content.rotation_period_ms = Some(uint!(3600)); content.rotation_period_msgs = Some(uint!(500)); - let settings = EncryptionSettings::new(content, HistoryVisibility::Shared, false); + let settings = EncryptionSettings::new( + content, + HistoryVisibility::Shared, + CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: false }, + ); assert_eq!(settings.rotation_period, Duration::from_millis(3600)); assert_eq!(settings.rotation_period_msgs, 500); From 2e4e11c6003f525ed079d4eb63a961dbc9c131ad Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 12 Aug 2024 15:43:23 +0100 Subject: [PATCH 07/12] crypto: extend `CollectionStrategy::DeviceBasedStrategy` Add (as yet unimplemented) `error_on_verified_user_problem` option --- bindings/matrix-sdk-crypto-ffi/src/lib.rs | 3 ++ crates/matrix-sdk-base/src/client.rs | 1 + .../src/machine/tests/mod.rs | 5 ++- .../src/olm/group_sessions/outbound.rs | 10 ++++- .../src/session_manager/group_sessions/mod.rs | 2 + .../group_sessions/share_strategy.rs | 42 +++++++++++++++---- 6 files changed, 52 insertions(+), 11 deletions(-) diff --git a/bindings/matrix-sdk-crypto-ffi/src/lib.rs b/bindings/matrix-sdk-crypto-ffi/src/lib.rs index 4ebcc2273eb..a2e904071e1 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/lib.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/lib.rs @@ -668,6 +668,8 @@ pub struct EncryptionSettings { /// Should untrusted devices receive the room key, or should they be /// excluded from the conversation. pub only_allow_trusted_devices: bool, + /// Should fail to send when a verified user has unverified devices. + pub error_on_verified_user_problem: bool, } impl From for RustEncryptionSettings { @@ -679,6 +681,7 @@ impl From for RustEncryptionSettings { history_visibility: v.history_visibility.into(), sharing_strategy: CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: v.only_allow_trusted_devices, + error_on_verified_user_problem: v.error_on_verified_user_problem, }, } } diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index a9f11926374..f437bab28e3 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -1396,6 +1396,7 @@ impl BaseClient { history_visibility, CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: false, + error_on_verified_user_problem: false, }, ); diff --git a/crates/matrix-sdk-crypto/src/machine/tests/mod.rs b/crates/matrix-sdk-crypto/src/machine/tests/mod.rs index 8e85ca5493a..6f8e210845b 100644 --- a/crates/matrix-sdk-crypto/src/machine/tests/mod.rs +++ b/crates/matrix-sdk-crypto/src/machine/tests/mod.rs @@ -587,7 +587,10 @@ async fn test_withheld_unverified() { let encryption_settings = EncryptionSettings::default(); let encryption_settings = EncryptionSettings { - sharing_strategy: CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: true }, + sharing_strategy: CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: true, + error_on_verified_user_problem: false, + }, ..encryption_settings }; diff --git a/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs b/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs index e83a5bf3ccc..79a1bab2865 100644 --- a/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs +++ b/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs @@ -790,7 +790,10 @@ mod tests { let settings = EncryptionSettings::new( content.clone(), HistoryVisibility::Joined, - CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: false }, + CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: false, + error_on_verified_user_problem: false, + }, ); assert_eq!(settings.rotation_period, ROTATION_PERIOD); @@ -802,7 +805,10 @@ mod tests { let settings = EncryptionSettings::new( content, HistoryVisibility::Shared, - CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: false }, + CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: false, + error_on_verified_user_problem: false, + }, ); assert_eq!(settings.rotation_period, Duration::from_millis(3600)); diff --git a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/mod.rs b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/mod.rs index 70c2c81b20a..65322840ecb 100644 --- a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/mod.rs +++ b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/mod.rs @@ -1164,6 +1164,7 @@ mod tests { let settings = EncryptionSettings { sharing_strategy: CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: true, + error_on_verified_user_problem: false, }, ..Default::default() }; @@ -1226,6 +1227,7 @@ mod tests { let settings = EncryptionSettings { sharing_strategy: CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: true, + error_on_verified_user_problem: false, }, ..Default::default() }; diff --git a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs index 0888e880321..d6d646cb939 100644 --- a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs +++ b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs @@ -28,6 +28,8 @@ use crate::{ error::OlmResult, store::Store, types::events::room_key_withheld::WithheldCode, DeviceData, EncryptionSettings, OwnUserIdentityData, UserIdentityData, }; +#[cfg(doc)] +use crate::{Device, LocalTrust}; /// Strategy to collect the devices that should receive room keys for the /// current discussion. @@ -43,7 +45,18 @@ pub enum CollectStrategy { /// trusted via interactive verification. /// - It is the current own device of the user. only_allow_trusted_devices: bool, + + /// If `true`, and a verified user has an unsigned device, key sharing + /// will fail with a + /// [`SessionRecipientCollectionError::VerifiedUserHasUnsignedDevice`]. + /// Otherwise, keys are shared with unsigned devices as normal. + /// + /// Once the problematic devices are blacklisted or whitelisted the + /// caller can retry to share a second time. + #[serde(default)] + error_on_verified_user_problem: bool, }, + /// Share based on identity. Only distribute to devices signed by their /// owner. If a user has no published identity he will not receive /// any room keys. @@ -59,7 +72,10 @@ impl CollectStrategy { impl Default for CollectStrategy { fn default() -> Self { - CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: false } + CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: false, + error_on_verified_user_problem: false, + } } } @@ -127,7 +143,7 @@ pub(crate) async fn collect_session_recipients( let user_devices = store.get_device_data_for_user_filtered(user_id).await?; let recipient_devices = match settings.sharing_strategy { - CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices } => { + CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices, .. } => { // We only need the user identity if only_allow_trusted_devices is set. let device_owner_identity = if only_allow_trusted_devices { store.get_user_identity(user_id).await? @@ -358,8 +374,10 @@ mod tests { async fn test_share_with_per_device_strategy_to_all() { let machine = set_up_test_machine().await; - let legacy_strategy = - CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: false }; + let legacy_strategy = CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: false, + error_on_verified_user_problem: false, + }; let encryption_settings = EncryptionSettings { sharing_strategy: legacy_strategy.clone(), ..Default::default() }; @@ -410,8 +428,10 @@ mod tests { let fake_room_id = room_id!("!roomid:localhost"); - let legacy_strategy = - CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: true }; + let legacy_strategy = CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: true, + error_on_verified_user_problem: false, + }; let encryption_settings = EncryptionSettings { sharing_strategy: legacy_strategy.clone(), ..Default::default() }; @@ -558,7 +578,10 @@ mod tests { let fake_room_id = room_id!("!roomid:localhost"); - let strategy = CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: false }; + let strategy = CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: false, + error_on_verified_user_problem: false, + }; let encryption_settings = EncryptionSettings { sharing_strategy: strategy.clone(), @@ -612,7 +635,10 @@ mod tests { let fake_room_id = room_id!("!roomid:localhost"); - let strategy = CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: false }; + let strategy = CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: false, + error_on_verified_user_problem: false, + }; let encryption_settings = EncryptionSettings { sharing_strategy: strategy.clone(), ..Default::default() }; From afe40151eecbb342b1677aa0cbac616794280839 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 6 Aug 2024 23:06:37 +0200 Subject: [PATCH 08/12] crypto: key sharing error for verified user with unverified devices --- crates/matrix-sdk-crypto/src/error.rs | 26 ++++ crates/matrix-sdk-crypto/src/lib.rs | 3 +- .../group_sessions/share_strategy.rs | 127 +++++++++++++++--- 3 files changed, 138 insertions(+), 18 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/error.rs b/crates/matrix-sdk-crypto/src/error.rs index 7e0adc2aca2..cdf7ae5c0f7 100644 --- a/crates/matrix-sdk-crypto/src/error.rs +++ b/crates/matrix-sdk-crypto/src/error.rs @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::collections::BTreeMap; + use ruma::{CanonicalJsonError, IdParseError, OwnedDeviceId, OwnedRoomId, OwnedUserId}; use serde::{ser::SerializeMap, Serializer}; use serde_json::Error as SerdeError; @@ -23,6 +25,8 @@ use crate::{ olm::SessionExportError, types::{events::room_key_withheld::WithheldCode, SignedKey}, }; +#[cfg(doc)] +use crate::{CollectStrategy, Device, LocalTrust}; pub type OlmResult = Result; pub type MegolmResult = Result; @@ -70,6 +74,10 @@ pub enum OlmError { have a valid Olm session with us" )] MissingSession, + + /// Encryption failed due to an error collecting the recipient devices. + #[error("encryption failed due to an error collecting the recipient devices: {0}")] + SessionRecipientCollectionError(SessionRecipientCollectionError), } /// Error representing a failure during a group encryption operation. @@ -360,3 +368,21 @@ pub enum SetRoomSettingsError { #[error(transparent)] Store(#[from] CryptoStoreError), } + +/// Error representing a problem when collecting the recipient devices for the +/// room key, during an encryption operation. +#[derive(Error, Debug)] +pub enum SessionRecipientCollectionError { + /// One or more verified users has one or more unsigned devices. + /// + /// Happens only with [`CollectStrategy::DeviceBasedStrategy`] when + /// [`error_on_verified_user_problem`](`CollectStrategy::DeviceBasedStrategy::error_on_verified_user_problem`) + /// is true. + /// + /// In order to resolve this, the caller can set the trust level of the + /// affected devices to [`LocalTrust::Ignored`] or + /// [`LocalTrust::BlackListed`] (see [`Device::set_local_trust`]), and + /// then retry the encryption operation. + #[error("one or more verified users have unsigned devices")] + VerifiedUserHasUnsignedDevice(BTreeMap>), +} diff --git a/crates/matrix-sdk-crypto/src/lib.rs b/crates/matrix-sdk-crypto/src/lib.rs index ba0733a7ecf..c712adb6aba 100644 --- a/crates/matrix-sdk-crypto/src/lib.rs +++ b/crates/matrix-sdk-crypto/src/lib.rs @@ -71,7 +71,8 @@ impl RoomKeyImportResult { } pub use error::{ - EventError, MegolmError, OlmError, SessionCreationError, SetRoomSettingsError, SignatureError, + EventError, MegolmError, OlmError, SessionCreationError, SessionRecipientCollectionError, + SetRoomSettingsError, SignatureError, }; pub use file_encryption::{ decrypt_room_key_export, encrypt_room_key_export, AttachmentDecryptor, AttachmentEncryptor, diff --git a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs index d6d646cb939..d03a72333d5 100644 --- a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs +++ b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs @@ -24,12 +24,14 @@ use serde::{Deserialize, Serialize}; use tracing::{debug, instrument, trace}; use super::OutboundGroupSession; +#[cfg(doc)] +use crate::Device; use crate::{ - error::OlmResult, store::Store, types::events::room_key_withheld::WithheldCode, DeviceData, - EncryptionSettings, OwnUserIdentityData, UserIdentityData, + error::{OlmResult, SessionRecipientCollectionError}, + store::Store, + types::events::room_key_withheld::WithheldCode, + DeviceData, EncryptionSettings, LocalTrust, OlmError, OwnUserIdentityData, UserIdentityData, }; -#[cfg(doc)] -use crate::{Device, LocalTrust}; /// Strategy to collect the devices that should receive room keys for the /// current discussion. @@ -112,6 +114,8 @@ pub(crate) async fn collect_session_recipients( let users: BTreeSet<&UserId> = users.collect(); let mut devices: BTreeMap> = Default::default(); let mut withheld_devices: Vec<(DeviceData, WithheldCode)> = Default::default(); + let mut unsigned_devices_of_verified_users: BTreeMap> = + Default::default(); trace!(?users, ?settings, "Calculating group session recipients"); @@ -140,21 +144,28 @@ pub(crate) async fn collect_session_recipients( let own_identity = store.get_user_identity(store.user_id()).await?.and_then(|i| i.into_own()); for user_id in users { + trace!("Considering recipient devices for user {}", user_id); let user_devices = store.get_device_data_for_user_filtered(user_id).await?; let recipient_devices = match settings.sharing_strategy { - CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices, .. } => { - // We only need the user identity if only_allow_trusted_devices is set. - let device_owner_identity = if only_allow_trusted_devices { - store.get_user_identity(user_id).await? - } else { - None - }; - split_recipients_withhelds_for_user( + CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices, + error_on_verified_user_problem, + } => { + // We only need the user identity if `only_allow_trusted_devices` or + // `error_on_verified_user_problem` is set. + let device_owner_identity = + if only_allow_trusted_devices || error_on_verified_user_problem { + store.get_user_identity(user_id).await? + } else { + None + }; + split_devices_for_user( user_devices, &own_identity, &device_owner_identity, only_allow_trusted_devices, + error_on_verified_user_problem, ) } CollectStrategy::IdentityBasedStrategy => { @@ -166,6 +177,21 @@ pub(crate) async fn collect_session_recipients( } }; + // If we're using a `DeviceBasedStrategy` with + // `error_on_verified_user_problem` set, then + // `unsigned_of_verified_user` may be populated. If so, add an entry to the + // list of users with unsigned devices. + if !recipient_devices.unsigned_of_verified_user.is_empty() { + unsigned_devices_of_verified_users.insert( + user_id.to_owned(), + recipient_devices + .unsigned_of_verified_user + .into_iter() + .map(|d| d.device_id().to_owned()) + .collect(), + ); + } + let recipients = recipient_devices.allowed_devices; let withheld_recipients = recipient_devices.denied_devices_with_code; @@ -181,6 +207,18 @@ pub(crate) async fn collect_session_recipients( withheld_devices.extend(withheld_recipients); } + // If we're using a `DeviceBasedStrategy` with + // `error_on_verified_user_problem` set, then + // `unsigned_devices_of_verified_users` may be populated. If so, we need to bail + // out with an error. + if !unsigned_devices_of_verified_users.is_empty() { + return Err(OlmError::SessionRecipientCollectionError( + SessionRecipientCollectionError::VerifiedUserHasUnsignedDevice( + unsigned_devices_of_verified_users, + ), + )); + } + if should_rotate { debug!( should_rotate, @@ -248,29 +286,59 @@ fn is_session_overshared_for_user( should_rotate } +/// Result type for [`split_devices_for_user`] and +/// [`split_recipients_withhelds_for_user_based_on_identity`]. #[derive(Default)] struct RecipientDevices { + /// Devices that should receive the room key. allowed_devices: Vec, + /// Devices that should receive a withheld code. denied_devices_with_code: Vec<(DeviceData, WithheldCode)>, + /// Devices that should cause the transmission to fail, due to being an + /// unsigned device belonging to a verified user. Only populated by + /// [`split_devices_for_user`], when + /// `error_on_verified_user_problem` is set. + unsigned_of_verified_user: Vec, } -fn split_recipients_withhelds_for_user( +/// Partition the list of a user's devices according to whether they should +/// receive the key, for [`CollectStrategy::DeviceBasedStrategy`]. +/// +/// We split the list into three buckets: +/// +/// * the devices that should receive the room key. +/// +/// * the devices that should receive a withheld code. +/// +/// * If `error_on_verified_user_problem` is set, the devices that should cause +/// the transmission to fail due to being unsigned. (If +/// `error_on_verified_user_problem` is unset, these devices are otherwise +/// partitioned into `allowed_devices`.) +fn split_devices_for_user( user_devices: HashMap, own_identity: &Option, device_owner_identity: &Option, only_allow_trusted_devices: bool, + error_on_verified_user_problem: bool, ) -> RecipientDevices { - // From all the devices a user has, we're splitting them into two - // buckets: a bucket of devices that should receive the - // room key, and a bucket of devices that should receive - // a withheld code. let mut recipient_devices: RecipientDevices = Default::default(); for d in user_devices.into_values() { if d.is_blacklisted() { recipient_devices.denied_devices_with_code.push((d, WithheldCode::Blacklisted)); + } else if d.local_trust_state() == LocalTrust::Ignored { + // Ignore the trust state of that device and share + recipient_devices.allowed_devices.push(d); } else if only_allow_trusted_devices && !d.is_verified(own_identity, device_owner_identity) { recipient_devices.denied_devices_with_code.push((d, WithheldCode::Unverified)); + } else if error_on_verified_user_problem + && is_unsigned_device_of_verified_user( + own_identity.as_ref(), + device_owner_identity.as_ref(), + &d, + ) + { + recipient_devices.unsigned_of_verified_user.push(d) } else { recipient_devices.allowed_devices.push(d); } @@ -292,6 +360,7 @@ fn split_recipients_withhelds_for_user_based_on_identity( .into_values() .map(|d| (d, WithheldCode::Unauthorised)) .collect(), + unsigned_of_verified_user: Vec::default(), } } Some(device_owner_identity) => { @@ -309,11 +378,35 @@ fn split_recipients_withhelds_for_user_based_on_identity( RecipientDevices { allowed_devices: recipients, denied_devices_with_code: withheld_recipients, + unsigned_of_verified_user: Vec::default(), } } } } +fn is_unsigned_device_of_verified_user( + own_identity: Option<&OwnUserIdentityData>, + device_owner_identity: Option<&UserIdentityData>, + device_data: &DeviceData, +) -> bool { + device_owner_identity.is_some_and(|device_owner_identity| { + is_user_verified(own_identity, device_owner_identity) + && !device_data.is_cross_signed_by_owner(device_owner_identity) + }) +} + +fn is_user_verified( + own_identity: Option<&OwnUserIdentityData>, + user_identity: &UserIdentityData, +) -> bool { + match user_identity { + UserIdentityData::Own(own_identity) => own_identity.is_verified(), + UserIdentityData::Other(other_identity) => { + own_identity.is_some_and(|oi| oi.is_identity_verified(other_identity)) + } + } +} + #[cfg(test)] mod tests { From cdb90e5abb60255ee426dc2a38ea7ce4f1925ad2 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 12 Aug 2024 19:43:54 +0100 Subject: [PATCH 09/12] crypto: test: factor out `create_test_outbound_group_session` helper --- .../group_sessions/share_strategy.rs | 59 ++++++------------- 1 file changed, 19 insertions(+), 40 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs index d03a72333d5..f6fce89339e 100644 --- a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs +++ b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs @@ -475,16 +475,7 @@ mod tests { let encryption_settings = EncryptionSettings { sharing_strategy: legacy_strategy.clone(), ..Default::default() }; - let fake_room_id = room_id!("!roomid:localhost"); - - let id_keys = machine.identity_keys(); - let group_session = OutboundGroupSession::new( - machine.device_id().into(), - Arc::new(id_keys), - fake_room_id, - encryption_settings.clone(), - ) - .unwrap(); + let group_session = create_test_outbound_group_session(&machine, &encryption_settings); let share_result = collect_session_recipients( machine.store(), @@ -519,8 +510,6 @@ mod tests { async fn test_share_with_per_device_strategy_only_trusted() { let machine = set_up_test_machine().await; - let fake_room_id = room_id!("!roomid:localhost"); - let legacy_strategy = CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: true, error_on_verified_user_problem: false, @@ -529,14 +518,7 @@ mod tests { let encryption_settings = EncryptionSettings { sharing_strategy: legacy_strategy.clone(), ..Default::default() }; - let id_keys = machine.identity_keys(); - let group_session = OutboundGroupSession::new( - machine.device_id().into(), - Arc::new(id_keys), - fake_room_id, - encryption_settings.clone(), - ) - .unwrap(); + let group_session = create_test_outbound_group_session(&machine, &encryption_settings); let share_result = collect_session_recipients( machine.store(), @@ -594,21 +576,12 @@ mod tests { async fn test_share_with_identity_strategy() { let machine = set_up_test_machine().await; - let fake_room_id = room_id!("!roomid:localhost"); - let strategy = CollectStrategy::new_identity_based(); let encryption_settings = EncryptionSettings { sharing_strategy: strategy.clone(), ..Default::default() }; - let id_keys = machine.identity_keys(); - let group_session = OutboundGroupSession::new( - machine.device_id().into(), - Arc::new(id_keys), - fake_room_id, - encryption_settings.clone(), - ) - .unwrap(); + let group_session = create_test_outbound_group_session(&machine, &encryption_settings); let share_result = collect_session_recipients( machine.store(), @@ -669,8 +642,6 @@ mod tests { async fn test_should_rotate_based_on_visibility() { let machine = set_up_test_machine().await; - let fake_room_id = room_id!("!roomid:localhost"); - let strategy = CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: false, error_on_verified_user_problem: false, @@ -682,14 +653,7 @@ mod tests { ..Default::default() }; - let id_keys = machine.identity_keys(); - let group_session = OutboundGroupSession::new( - machine.device_id().into(), - Arc::new(id_keys), - fake_room_id, - encryption_settings.clone(), - ) - .unwrap(); + let group_session = create_test_outbound_group_session(&machine, &encryption_settings); let _ = collect_session_recipients( machine.store(), @@ -772,4 +736,19 @@ mod tests { assert!(share_result.should_rotate); } + + /// Create an [`OutboundGroupSession`], backed by the given olm machine, + /// without sharing it. + fn create_test_outbound_group_session( + machine: &OlmMachine, + encryption_settings: &EncryptionSettings, + ) -> OutboundGroupSession { + OutboundGroupSession::new( + machine.device_id().into(), + Arc::new(machine.identity_keys()), + room_id!("!roomid:localhost"), + encryption_settings.clone(), + ) + .expect("creating an outbound group session should not fail") + } } From 2ed73b8a1755693fa6f90c88e6012a810698027c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 14 Aug 2024 14:17:45 +0100 Subject: [PATCH 10/12] crypto: test: factor out redundant variable This thing was confusing. What is "legacy" about it? --- .../group_sessions/share_strategy.rs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs index f6fce89339e..6647d06c6b7 100644 --- a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs +++ b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs @@ -467,14 +467,14 @@ mod tests { async fn test_share_with_per_device_strategy_to_all() { let machine = set_up_test_machine().await; - let legacy_strategy = CollectStrategy::DeviceBasedStrategy { - only_allow_trusted_devices: false, - error_on_verified_user_problem: false, + let encryption_settings = EncryptionSettings { + sharing_strategy: CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: false, + error_on_verified_user_problem: false, + }, + ..Default::default() }; - let encryption_settings = - EncryptionSettings { sharing_strategy: legacy_strategy.clone(), ..Default::default() }; - let group_session = create_test_outbound_group_session(&machine, &encryption_settings); let share_result = collect_session_recipients( @@ -510,14 +510,14 @@ mod tests { async fn test_share_with_per_device_strategy_only_trusted() { let machine = set_up_test_machine().await; - let legacy_strategy = CollectStrategy::DeviceBasedStrategy { - only_allow_trusted_devices: true, - error_on_verified_user_problem: false, + let encryption_settings = EncryptionSettings { + sharing_strategy: CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: true, + error_on_verified_user_problem: false, + }, + ..Default::default() }; - let encryption_settings = - EncryptionSettings { sharing_strategy: legacy_strategy.clone(), ..Default::default() }; - let group_session = create_test_outbound_group_session(&machine, &encryption_settings); let share_result = collect_session_recipients( From 56df4b3e5b93cc537d485dd92281be320104c6c7 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 13 Aug 2024 16:31:34 +0100 Subject: [PATCH 11/12] crypto: test: add tests for `error_on_verified_user_problem` --- .../group_sessions/share_strategy.rs | 414 +++++++++++++++++- .../src/test_json/keys_query_sets.rs | 75 +++- 2 files changed, 481 insertions(+), 8 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs index 6647d06c6b7..142a2a02ddc 100644 --- a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs +++ b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs @@ -409,19 +409,25 @@ fn is_user_verified( #[cfg(test)] mod tests { + use std::{collections::BTreeMap, iter, sync::Arc}; - use std::sync::Arc; - - use matrix_sdk_test::{async_test, test_json::keys_query_sets::KeyDistributionTestData}; - use ruma::{events::room::history_visibility::HistoryVisibility, room_id, TransactionId}; + use assert_matches2::assert_let; + use matrix_sdk_test::{ + async_test, test_json, + test_json::keys_query_sets::{KeyDistributionTestData, PreviouslyVerifiedTestData}, + }; + use ruma::{ + device_id, events::room::history_visibility::HistoryVisibility, room_id, TransactionId, + }; use crate::{ + error::SessionRecipientCollectionError, olm::OutboundGroupSession, session_manager::{ group_sessions::share_strategy::collect_session_recipients, CollectStrategy, }, types::events::room_key_withheld::WithheldCode, - CrossSigningKeyExport, EncryptionSettings, OlmMachine, + CrossSigningKeyExport, EncryptionSettings, LocalTrust, OlmError, OlmMachine, }; async fn set_up_test_machine() -> OlmMachine { @@ -508,12 +514,31 @@ mod tests { #[async_test] async fn test_share_with_per_device_strategy_only_trusted() { + test_share_only_trusted_helper(false).await; + } + + /// Variation of [`test_share_with_per_device_strategy_only_trusted`] to + /// test the interaction between + /// [`only_allow_trusted_devices`](`CollectStrategy::DeviceBasedStrategy::only_allow_trusted_devices`) and + /// [`error_on_verified_user_problem`](`CollectStrategy::DeviceBasedStrategy::error_on_verified_user_problem`). + /// + /// (Given that untrusted devices are ignored, we do not expect + /// [`collect_session_recipients`] to return an error, despite the presence + /// of unsigned devices.) + #[async_test] + async fn test_share_with_per_device_strategy_only_trusted_error_on_unsigned_of_verified() { + test_share_only_trusted_helper(true).await; + } + + /// Common helper for [`test_share_with_per_device_strategy_only_trusted`] + /// and [`test_share_with_per_device_strategy_only_trusted_error_on_unsigned_of_verified`]. + async fn test_share_only_trusted_helper(error_on_verified_user_problem: bool) { let machine = set_up_test_machine().await; let encryption_settings = EncryptionSettings { sharing_strategy: CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices: true, - error_on_verified_user_problem: false, + error_on_verified_user_problem, }, ..Default::default() }; @@ -572,6 +597,383 @@ mod tests { assert_eq!(code, &WithheldCode::Unverified); } + /// Test that [`collect_session_recipients`] returns an error if there are + /// unsigned devices belonging to verified users, when + /// `error_on_verified_user_problem` is set. + #[async_test] + async fn test_error_on_unsigned_of_verified_users() { + use PreviouslyVerifiedTestData as DataSet; + + // We start with Bob, who is verified and has one unsigned device. + let machine = unsigned_of_verified_setup().await; + + // Add Carol, also verified with one unsigned device. + let carol_keys = DataSet::carol_keys_query_response_signed(); + machine.mark_request_as_sent(&TransactionId::new(), &carol_keys).await.unwrap(); + + // Double-check the state of Carol. + let carol_identity = + machine.get_identity(DataSet::carol_id(), None).await.unwrap().unwrap(); + assert!(carol_identity.other().unwrap().is_verified()); + + let carol_unsigned_device = machine + .get_device(DataSet::carol_id(), DataSet::carol_unsigned_device_id(), None) + .await + .unwrap() + .unwrap(); + assert!(!carol_unsigned_device.is_verified()); + + // Sharing an OutboundGroupSession should fail. + let encryption_settings = EncryptionSettings { + sharing_strategy: CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: false, + error_on_verified_user_problem: true, + }, + ..Default::default() + }; + let group_session = create_test_outbound_group_session(&machine, &encryption_settings); + let share_result = collect_session_recipients( + machine.store(), + vec![DataSet::bob_id(), DataSet::carol_id()].into_iter(), + &encryption_settings, + &group_session, + ) + .await; + + assert_let!( + Err(OlmError::SessionRecipientCollectionError( + SessionRecipientCollectionError::VerifiedUserHasUnsignedDevice(unverified_devices) + )) = share_result + ); + + // Check the list of devices in the error. + assert_eq!( + unverified_devices, + BTreeMap::from([ + (DataSet::bob_id().to_owned(), vec![DataSet::bob_device_2_id().to_owned()]), + ( + DataSet::carol_id().to_owned(), + vec![DataSet::carol_unsigned_device_id().to_owned()] + ), + ]) + ); + } + + /// Test that we can resolve errors from + /// `error_on_verified_user_problem` by whitelisting the + /// device. + #[async_test] + async fn test_error_on_unsigned_of_verified_resolve_by_whitelisting() { + use PreviouslyVerifiedTestData as DataSet; + + let machine = unsigned_of_verified_setup().await; + + // Whitelist the unsigned device + machine + .get_device(DataSet::bob_id(), DataSet::bob_device_2_id(), None) + .await + .unwrap() + .unwrap() + .set_local_trust(LocalTrust::Ignored) + .await + .unwrap(); + + let encryption_settings = EncryptionSettings { + sharing_strategy: CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: false, + error_on_verified_user_problem: true, + }, + ..Default::default() + }; + + let group_session = create_test_outbound_group_session(&machine, &encryption_settings); + + // We should be able to share a key, and it should include the unsigned device. + let share_result = collect_session_recipients( + machine.store(), + iter::once(DataSet::bob_id()), + &encryption_settings, + &group_session, + ) + .await + .unwrap(); + + assert_eq!(2, share_result.devices.get(DataSet::bob_id()).unwrap().len()); + assert_eq!(0, share_result.withheld_devices.len()); + } + + /// Test that we can resolve errors from + /// `error_on_verified_user_problem` by blacklisting the + /// device. + #[async_test] + async fn test_error_on_unsigned_of_verified_resolve_by_blacklisting() { + use PreviouslyVerifiedTestData as DataSet; + + let machine = unsigned_of_verified_setup().await; + + // Blacklist the unsigned device + machine + .get_device(DataSet::bob_id(), DataSet::bob_device_2_id(), None) + .await + .unwrap() + .unwrap() + .set_local_trust(LocalTrust::BlackListed) + .await + .unwrap(); + + let encryption_settings = EncryptionSettings { + sharing_strategy: CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: false, + error_on_verified_user_problem: true, + }, + ..Default::default() + }; + + let group_session = create_test_outbound_group_session(&machine, &encryption_settings); + + // We should be able to share a key, and it should exclude the unsigned device. + let share_result = collect_session_recipients( + machine.store(), + iter::once(DataSet::bob_id()), + &encryption_settings, + &group_session, + ) + .await + .unwrap(); + + assert_eq!(1, share_result.devices.get(DataSet::bob_id()).unwrap().len()); + let withheld_list: Vec<_> = share_result + .withheld_devices + .iter() + .map(|(d, code)| (d.device_id().to_owned(), code.clone())) + .collect(); + assert_eq!( + withheld_list, + vec![(DataSet::bob_device_2_id().to_owned(), WithheldCode::Blacklisted)] + ); + } + + /// Test that [`collect_session_recipients`] returns an error when + /// `error_on_verified_user_problem` is set, if our own identity + /// is verified and we have unsigned devices. + #[async_test] + async fn test_error_on_unsigned_of_verified_owner_is_us() { + use PreviouslyVerifiedTestData as DataSet; + + let machine = unsigned_of_verified_setup().await; + + // Add a couple of devices to Alice's account + let mut own_keys = DataSet::own_keys_query_response_1().clone(); + own_keys.device_keys.insert( + DataSet::own_id().to_owned(), + BTreeMap::from([ + DataSet::own_signed_device_keys(), + DataSet::own_unsigned_device_keys(), + ]), + ); + machine.mark_request_as_sent(&TransactionId::new(), &own_keys).await.unwrap(); + + let encryption_settings = EncryptionSettings { + sharing_strategy: CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: false, + error_on_verified_user_problem: true, + }, + ..Default::default() + }; + + let group_session = create_test_outbound_group_session(&machine, &encryption_settings); + + let share_result = collect_session_recipients( + machine.store(), + iter::once(DataSet::own_id()), + &encryption_settings, + &group_session, + ) + .await; + + assert_let!( + Err(OlmError::SessionRecipientCollectionError( + SessionRecipientCollectionError::VerifiedUserHasUnsignedDevice(unverified_devices) + )) = share_result + ); + + // Check the list of devices in the error. + assert_eq!( + unverified_devices, + BTreeMap::from([( + DataSet::own_id().to_owned(), + vec![DataSet::own_unsigned_device_id()] + ),]) + ); + } + + /// Common setup for tests which require a verified user to have unsigned + /// devices. + /// + /// Returns an `OlmMachine` which is properly configured with trusted + /// cross-signing keys. Also imports a set of keys for + /// Bob ([`PreviouslyVerifiedTestData::bob_id`]), where Bob is verified and + /// has 2 devices, one signed and the other not. + async fn unsigned_of_verified_setup() -> OlmMachine { + use test_json::keys_query_sets::PreviouslyVerifiedTestData as DataSet; + + let machine = OlmMachine::new(DataSet::own_id(), device_id!("LOCAL")).await; + + // Tell the OlmMachine about our own public keys. + let own_keys = DataSet::own_keys_query_response_1(); + machine.mark_request_as_sent(&TransactionId::new(), &own_keys).await.unwrap(); + + // Import the secret parts of our own cross-signing keys. + machine + .import_cross_signing_keys(CrossSigningKeyExport { + master_key: DataSet::MASTER_KEY_PRIVATE_EXPORT.to_owned().into(), + self_signing_key: DataSet::SELF_SIGNING_KEY_PRIVATE_EXPORT.to_owned().into(), + user_signing_key: DataSet::USER_SIGNING_KEY_PRIVATE_EXPORT.to_owned().into(), + }) + .await + .unwrap(); + + // Tell the OlmMachine about Bob's keys. + let bob_keys = DataSet::bob_keys_query_response_signed(); + machine.mark_request_as_sent(&TransactionId::new(), &bob_keys).await.unwrap(); + + // Double-check the state of Bob: he should be verified, and should have one + // signed and one unsigned device. + let bob_identity = machine.get_identity(DataSet::bob_id(), None).await.unwrap().unwrap(); + assert!(bob_identity.other().unwrap().is_verified()); + + let bob_signed_device = machine + .get_device(DataSet::bob_id(), DataSet::bob_device_1_id(), None) + .await + .unwrap() + .unwrap(); + assert!(bob_signed_device.is_verified()); + assert!(bob_signed_device.device_owner_identity.is_some()); + + let bob_unsigned_device = machine + .get_device(DataSet::bob_id(), DataSet::bob_device_2_id(), None) + .await + .unwrap() + .unwrap(); + assert!(!bob_unsigned_device.is_verified()); + + machine + } + + /// Test that an unsigned device of an unverified user doesn't cause an + /// error. + #[async_test] + async fn test_should_not_error_on_unsigned_of_unverified() { + use PreviouslyVerifiedTestData as DataSet; + + let machine = OlmMachine::new(DataSet::own_id(), device_id!("LOCAL")).await; + + // Tell the OlmMachine about our own public keys. + let own_keys = DataSet::own_keys_query_response_1(); + machine.mark_request_as_sent(&TransactionId::new(), &own_keys).await.unwrap(); + + // Import the secret parts of our own cross-signing keys. + machine + .import_cross_signing_keys(CrossSigningKeyExport { + master_key: DataSet::MASTER_KEY_PRIVATE_EXPORT.to_owned().into(), + self_signing_key: DataSet::SELF_SIGNING_KEY_PRIVATE_EXPORT.to_owned().into(), + user_signing_key: DataSet::USER_SIGNING_KEY_PRIVATE_EXPORT.to_owned().into(), + }) + .await + .unwrap(); + + // This time our own identity is trusted but is not signing bob. + let bob_keys = DataSet::bob_keys_query_response_rotated(); + machine.mark_request_as_sent(&TransactionId::new(), &bob_keys).await.unwrap(); + + // Double-check the state of Bob: he should be unverified, and should have an + // unsigned device. + let bob_identity = machine.get_identity(DataSet::bob_id(), None).await.unwrap().unwrap(); + assert!(!bob_identity.other().unwrap().is_verified()); + + let bob_unsigned_device = machine + .get_device(DataSet::bob_id(), DataSet::bob_device_1_id(), None) + .await + .unwrap() + .unwrap(); + assert!(!bob_unsigned_device.is_cross_signed_by_owner()); + + let encryption_settings = EncryptionSettings { + sharing_strategy: CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: false, + error_on_verified_user_problem: true, + }, + ..Default::default() + }; + + let group_session = create_test_outbound_group_session(&machine, &encryption_settings); + + collect_session_recipients( + machine.store(), + iter::once(DataSet::bob_id()), + &encryption_settings, + &group_session, + ) + .await + .unwrap(); + } + + /// Test that an unsigned device of a signed user doesn't cause an + /// error, when we have not verified our own identity. + #[async_test] + async fn test_should_not_error_on_unsigned_of_signed_but_unverified() { + use PreviouslyVerifiedTestData as DataSet; + + let machine = OlmMachine::new(DataSet::own_id(), device_id!("LOCAL")).await; + + // Tell the OlmMachine about our own public keys. + let keys_query = DataSet::own_keys_query_response_1(); + machine.mark_request_as_sent(&TransactionId::new(), &keys_query).await.unwrap(); + + // ... and those of Bob. + let keys_query = DataSet::bob_keys_query_response_signed(); + machine.mark_request_as_sent(&TransactionId::new(), &keys_query).await.unwrap(); + + // Double-check the state of Bob: his identity should be signed but unverified, + // and he should have an unsigned device. + let bob_identity = + machine.get_identity(DataSet::bob_id(), None).await.unwrap().unwrap().other().unwrap(); + assert!(bob_identity + .own_identity + .as_ref() + .unwrap() + .is_identity_signed(&bob_identity.inner) + .is_ok()); + assert!(!bob_identity.is_verified()); + + let bob_unsigned_device = machine + .get_device(DataSet::bob_id(), DataSet::bob_device_2_id(), None) + .await + .unwrap() + .unwrap(); + assert!(!bob_unsigned_device.is_cross_signed_by_owner()); + + // Share a session, and ensure that it doesn't error. + let encryption_settings = EncryptionSettings { + sharing_strategy: CollectStrategy::DeviceBasedStrategy { + only_allow_trusted_devices: false, + error_on_verified_user_problem: true, + }, + ..Default::default() + }; + + let group_session = create_test_outbound_group_session(&machine, &encryption_settings); + + collect_session_recipients( + machine.store(), + iter::once(DataSet::bob_id()), + &encryption_settings, + &group_session, + ) + .await + .unwrap(); + } + #[async_test] async fn test_share_with_identity_strategy() { let machine = set_up_test_machine().await; diff --git a/testing/matrix-sdk-test/src/test_json/keys_query_sets.rs b/testing/matrix-sdk-test/src/test_json/keys_query_sets.rs index 4719ea650e0..cc17f1929ba 100644 --- a/testing/matrix-sdk-test/src/test_json/keys_query_sets.rs +++ b/testing/matrix-sdk-test/src/test_json/keys_query_sets.rs @@ -1,6 +1,6 @@ use ruma::{ - api::client::keys::get_keys::v3::Response as KeyQueryResponse, device_id, user_id, DeviceId, - UserId, + api::client::keys::get_keys::v3::Response as KeyQueryResponse, device_id, + encryption::DeviceKeys, serde::Raw, user_id, DeviceId, OwnedDeviceId, UserId, }; use serde_json::{json, Value}; @@ -774,6 +774,77 @@ impl PreviouslyVerifiedTestData { ruma_response_from_json(&data) } + /// Device ID of the device returned by [`Self::own_unsigned_device_keys`]. + pub fn own_unsigned_device_id() -> OwnedDeviceId { + Self::own_unsigned_device_keys().0 + } + + /// Device-keys response for a device belonging to Alice, which has *not* + /// been signed by her identity. This can be used as part of a + /// `/keys/query` response. + /// + /// For convenience, returns a tuple `(, )`. The + /// device id is also returned by [`Self::own_unsigned_device_id`]. + pub fn own_unsigned_device_keys() -> (OwnedDeviceId, Raw) { + let json = json!({ + "algorithms": [ + "m.olm.v1.curve25519-aes-sha2", + "m.megolm.v1.aes-sha2" + ], + "device_id": "AHIVRZICJK", + "keys": { + "curve25519:AHIVRZICJK": "3U73fbymtt6sn/H+5UYHiQxN2HfDmxzOMYZ+3JyPT2E", + "ed25519:AHIVRZICJK": "I0NV5nJYmnH+f5py4Fz2tdCeSKUChaaXV7m4UOq9bis" + }, + "signatures": { + "@alice:localhost": { + "ed25519:AHIVRZICJK": "HIs13b2GizN8gdZrYLWs9KZbcmKubXE+O4716Uow513e84JO8REy53OX4TDdoBfmVhPiZg5CIRrUDH7JxY4wAQ" + } + }, + "user_id": "@alice:localhost", + "unsigned": { + "device_display_name": "Element - dbg Android" + } + }); + (device_id!("AHIVRZICJK").to_owned(), serde_json::from_value(json).unwrap()) + } + + /// Device ID of the device returned by [`Self::own_signed_device_keys`]. + pub fn own_signed_device_id() -> OwnedDeviceId { + Self::own_signed_device_keys().0 + } + + /// Device-keys response for a device belonging to Alice, which has been + /// signed by her identity. This can be used as part of a `/keys/query` + /// response. + /// + /// For convenience, returns a tuple `(, )`. The + /// device id is also returned by [`Self::own_signed_device_id`]. + pub fn own_signed_device_keys() -> (OwnedDeviceId, Raw) { + let json = json!({ + "algorithms": [ + "m.olm.v1.curve25519-aes-sha2", + "m.megolm.v1.aes-sha2" + ], + "device_id": "LCNRWQAVWK", + "keys": { + "curve25519:LCNRWQAVWK": "fULFq9I6uYmsdDwRFU76wc43RqF7TVGvlWvKXhSrsS4", + "ed25519:LCNRWQAVWK": "F7E0EF0lzVJN31cnetLdeBuNvZ8jQqkUzt8/nGD9M/E" + }, + "signatures": { + "@alice:localhost": { + "ed25519:LCNRWQAVWK": "8kLsN76ytGRuHKMgIARaOds29QrPRzQ6Px+FOLsYK/ATmx5IVd65MpSh2pGjLAaPsSGWR1WLbBTq/LZtcpjTDQ", + "ed25519:WXLer0esHUanp8DCeu2Be0xB5ms9aKFFBrCFl50COjw": "lo4Vuuu+WvPt1hnOCv30iS1y/cF7DljfFZYF3ib5JH/6iPZTW4jYdlmWo4a7hDf0fb2pu3EFnghYMr7vVx41Aw" + } + }, + "user_id": "@alice:localhost", + "unsigned": { + "device_display_name": "develop.element.io: Chrome on macOS" + } + }); + (device_id!("LCNRWQAVWK").to_owned(), serde_json::from_value(json).unwrap()) + } + /// `/keys/query` response for Bob, signed by Alice's identity. /// /// Contains Bob's cross-signing identity, and two devices: From 5f24f8eb83cf0fe0a5a94e2630ded0bc17cac2cb Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 13 Aug 2024 16:45:32 +0100 Subject: [PATCH 12/12] crypto: Update changelog --- crates/matrix-sdk-crypto/CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/matrix-sdk-crypto/CHANGELOG.md b/crates/matrix-sdk-crypto/CHANGELOG.md index 393d1c09dc6..6e36dc038b7 100644 --- a/crates/matrix-sdk-crypto/CHANGELOG.md +++ b/crates/matrix-sdk-crypto/CHANGELOG.md @@ -32,6 +32,18 @@ Changes: Breaking changes: +- Add a new `error_on_verified_user_problem` property to + `CollectStrategy::DeviceBasedStrategy`, which, when set, causes + `OlmMachine::share_room_key` to fail with an error if any verified users on + the recipient list have unsigned devices. + + Also remove `CollectStrategy::new_device_based`: callers should construct a + `CollectStrategy::DeviceBasedStrategy` directly. + + `EncryptionSettings::new` now takes a `CollectStrategy` argument, instead of + a list of booleans. + ([#3810](https://github.com/matrix-org/matrix-rust-sdk/pull/3810)) + - Remove the method `OlmMachine::clear_crypto_cache()`, crypto stores are not supposed to have any caches anymore.