From 98b064afc855ee14a0239495a5c05e3f9ea6bac4 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 24 Jun 2025 13:37:11 +0100 Subject: [PATCH 1/6] refactor(crypto): Pass DecryptionSettings in to OlmMachine::decrypt_to_device_event This will be used in the next commit, but it was very noisy, so I separated it out into this commit to make the next one easier to read. --- .../src/dehydrated_devices.rs | 9 +- bindings/matrix-sdk-crypto-ffi/src/machine.rs | 21 ++- crates/matrix-sdk-base/src/client.rs | 7 +- .../src/response_processors/e2ee/to_device.rs | 11 +- crates/matrix-sdk-base/src/sliding_sync.rs | 9 +- crates/matrix-sdk-crypto/README.md | 31 ++-- .../src/dehydrated_devices.rs | 27 ++- .../src/gossiping/machine.rs | 21 ++- crates/matrix-sdk-crypto/src/lib.rs | 20 ++- crates/matrix-sdk-crypto/src/machine/mod.rs | 33 +++- .../src/machine/test_helpers.rs | 22 ++- .../tests/decryption_verification_state.rs | 25 ++- .../src/machine/tests/megolm_sender_data.rs | 42 +++-- .../src/machine/tests/mod.rs | 167 +++++++++++++----- .../src/machine/tests/olm_encryption.rs | 31 +++- .../machine/tests/send_encrypted_to_device.rs | 24 ++- .../src/session_manager/group_sessions/mod.rs | 23 ++- 17 files changed, 395 insertions(+), 128 deletions(-) diff --git a/bindings/matrix-sdk-crypto-ffi/src/dehydrated_devices.rs b/bindings/matrix-sdk-crypto-ffi/src/dehydrated_devices.rs index b3477ee1739..7597d0ffb21 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/dehydrated_devices.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/dehydrated_devices.rs @@ -7,6 +7,7 @@ use matrix_sdk_crypto::{ RehydratedDevice as InnerRehydratedDevice, }, store::types::DehydratedDeviceKey as InnerDehydratedDeviceKey, + DecryptionSettings, }; use ruma::{api::client::dehydrated_device, events::AnyToDeviceEvent, serde::Raw, OwnedDeviceId}; use serde_json::json; @@ -154,9 +155,13 @@ impl Drop for RehydratedDevice { #[matrix_sdk_ffi_macros::export] impl RehydratedDevice { - pub fn receive_events(&self, events: String) -> Result<(), crate::CryptoStoreError> { + pub fn receive_events( + &self, + events: String, + decryption_settings: &DecryptionSettings, + ) -> Result<(), crate::CryptoStoreError> { let events: Vec> = serde_json::from_str(&events)?; - self.runtime.block_on(self.inner.receive_events(events))?; + self.runtime.block_on(self.inner.receive_events(events, decryption_settings))?; Ok(()) } diff --git a/bindings/matrix-sdk-crypto-ffi/src/machine.rs b/bindings/matrix-sdk-crypto-ffi/src/machine.rs index cd5784d80e9..3c58139ce9e 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/machine.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/machine.rs @@ -526,6 +526,7 @@ impl OlmMachine { key_counts: HashMap, unused_fallback_keys: Option>, next_batch_token: String, + decryption_settings: &DecryptionSettings, ) -> Result { let to_device: ToDevice = serde_json::from_str(&events)?; let device_changes: RumaDeviceLists = device_changes.into(); @@ -544,15 +545,17 @@ impl OlmMachine { let unused_fallback_keys: Option> = unused_fallback_keys.map(|u| u.into_iter().map(OneTimeKeyAlgorithm::from).collect()); - let (to_device_events, room_key_infos) = self.runtime.block_on( - self.inner.receive_sync_changes(matrix_sdk_crypto::EncryptionSyncChanges { - to_device_events: to_device.events, - changed_devices: &device_changes, - one_time_keys_counts: &key_counts, - unused_fallback_keys: unused_fallback_keys.as_deref(), - next_batch_token: Some(next_batch_token), - }), - )?; + let (to_device_events, room_key_infos) = + self.runtime.block_on(self.inner.receive_sync_changes( + matrix_sdk_crypto::EncryptionSyncChanges { + to_device_events: to_device.events, + changed_devices: &device_changes, + one_time_keys_counts: &key_counts, + unused_fallback_keys: unused_fallback_keys.as_deref(), + next_batch_token: Some(next_batch_token), + }, + decryption_settings, + ))?; let to_device_events = to_device_events .into_iter() diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 474ae4aaaa4..4b54c922b96 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -593,7 +593,12 @@ impl BaseClient { let processors::e2ee::to_device::Output { processed_to_device_events: to_device, room_key_updates, - } = processors::e2ee::to_device::from_sync_v2(&response, olm_machine.as_ref()).await?; + } = processors::e2ee::to_device::from_sync_v2( + &response, + olm_machine.as_ref(), + &self.decryption_settings, + ) + .await?; processors::latest_event::decrypt_from_rooms( &mut context, diff --git a/crates/matrix-sdk-base/src/response_processors/e2ee/to_device.rs b/crates/matrix-sdk-base/src/response_processors/e2ee/to_device.rs index 94c4ef29fa7..31b2d55a708 100644 --- a/crates/matrix-sdk-base/src/response_processors/e2ee/to_device.rs +++ b/crates/matrix-sdk-base/src/response_processors/e2ee/to_device.rs @@ -15,7 +15,9 @@ use std::collections::BTreeMap; use matrix_sdk_common::deserialized_responses::ProcessedToDeviceEvent; -use matrix_sdk_crypto::{EncryptionSyncChanges, OlmMachine, store::types::RoomKeyInfo}; +use matrix_sdk_crypto::{ + DecryptionSettings, EncryptionSyncChanges, OlmMachine, store::types::RoomKeyInfo, +}; use ruma::{ OneTimeKeyAlgorithm, UInt, api::client::sync::sync_events::{DeviceLists, v3, v5}, @@ -34,6 +36,7 @@ pub async fn from_msc4186( to_device: Option<&v5::response::ToDevice>, e2ee: &v5::response::E2EE, olm_machine: Option<&OlmMachine>, + decryption_settings: &DecryptionSettings, ) -> Result { process( olm_machine, @@ -42,6 +45,7 @@ pub async fn from_msc4186( &e2ee.device_one_time_keys_count, e2ee.device_unused_fallback_key_types.as_deref(), to_device.as_ref().map(|to_device| to_device.next_batch.clone()), + decryption_settings, ) .await } @@ -54,6 +58,7 @@ pub async fn from_msc4186( pub async fn from_sync_v2( response: &v3::Response, olm_machine: Option<&OlmMachine>, + decryption_settings: &DecryptionSettings, ) -> Result { process( olm_machine, @@ -62,6 +67,7 @@ pub async fn from_sync_v2( &response.device_one_time_keys_count, response.device_unused_fallback_key_types.as_deref(), Some(response.next_batch.clone()), + decryption_settings, ) .await } @@ -77,6 +83,7 @@ async fn process( one_time_keys_counts: &BTreeMap, unused_fallback_keys: Option<&[OneTimeKeyAlgorithm]>, next_batch_token: Option, + decryption_settings: &DecryptionSettings, ) -> Result { let encryption_sync_changes = EncryptionSyncChanges { to_device_events, @@ -92,7 +99,7 @@ async fn process( // This makes sure that we have the decryption keys for the room // events at hand. let (events, room_key_updates) = - olm_machine.receive_sync_changes(encryption_sync_changes).await?; + olm_machine.receive_sync_changes(encryption_sync_changes, decryption_settings).await?; Output { processed_to_device_events: events, room_key_updates: Some(room_key_updates) } } else { diff --git a/crates/matrix-sdk-base/src/sliding_sync.rs b/crates/matrix-sdk-base/src/sliding_sync.rs index b6941e9c068..7ee7bd6303a 100644 --- a/crates/matrix-sdk-base/src/sliding_sync.rs +++ b/crates/matrix-sdk-base/src/sliding_sync.rs @@ -63,8 +63,13 @@ impl BaseClient { let mut context = processors::Context::default(); let processors::e2ee::to_device::Output { processed_to_device_events, room_key_updates } = - processors::e2ee::to_device::from_msc4186(to_device, e2ee, olm_machine.as_ref()) - .await?; + processors::e2ee::to_device::from_msc4186( + to_device, + e2ee, + olm_machine.as_ref(), + &self.decryption_settings, + ) + .await?; processors::latest_event::decrypt_from_rooms( &mut context, diff --git a/crates/matrix-sdk-crypto/README.md b/crates/matrix-sdk-crypto/README.md index 154c9f0fed7..a0e1c83c52e 100644 --- a/crates/matrix-sdk-crypto/README.md +++ b/crates/matrix-sdk-crypto/README.md @@ -17,11 +17,10 @@ The state machine works in a push/pull manner: ```rust,no_run use std::collections::BTreeMap; -use matrix_sdk_crypto::{EncryptionSyncChanges, OlmMachine, OlmError}; -use ruma::{ - api::client::sync::sync_events::{v3::ToDevice, DeviceLists}, - device_id, user_id, +use matrix_sdk_crypto::{ + DecryptionSettings, EncryptionSyncChanges, OlmError, OlmMachine, TrustRequirement, }; +use ruma::{api::client::sync::sync_events::DeviceLists, device_id, user_id}; #[tokio::main] async fn main() -> Result<(), OlmError> { @@ -33,19 +32,27 @@ async fn main() -> Result<(), OlmError> { let unused_fallback_keys = Some(Vec::new()); let next_batch_token = "T0K3N".to_owned(); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + // Push changes that the server sent to us in a sync response. - let decrypted_to_device = machine.receive_sync_changes(EncryptionSyncChanges { - to_device_events: vec![], - changed_devices: &changed_devices, - one_time_keys_counts: &one_time_key_counts, - unused_fallback_keys: unused_fallback_keys.as_deref(), - next_batch_token: Some(next_batch_token), - }).await?; + let decrypted_to_device = machine + .receive_sync_changes( + EncryptionSyncChanges { + to_device_events: vec![], + changed_devices: &changed_devices, + one_time_keys_counts: &one_time_key_counts, + unused_fallback_keys: unused_fallback_keys.as_deref(), + next_batch_token: Some(next_batch_token), + }, + &decryption_settings, + ) + .await?; // Pull requests that we need to send out. let outgoing_requests = machine.outgoing_requests().await?; - // Send the requests here out and call machine.mark_request_as_sent(). + // Send the requests out here and call machine.mark_request_as_sent(). Ok(()) } diff --git a/crates/matrix-sdk-crypto/src/dehydrated_devices.rs b/crates/matrix-sdk-crypto/src/dehydrated_devices.rs index 903fdde3f83..90fee8ae9ec 100644 --- a/crates/matrix-sdk-crypto/src/dehydrated_devices.rs +++ b/crates/matrix-sdk-crypto/src/dehydrated_devices.rs @@ -60,7 +60,8 @@ use crate::{ CryptoStoreWrapper, MemoryStore, Store, }, verification::VerificationMachine, - Account, CryptoStoreError, EncryptionSyncChanges, OlmError, OlmMachine, SignatureError, + Account, CryptoStoreError, DecryptionSettings, EncryptionSyncChanges, OlmError, OlmMachine, + SignatureError, }; /// Error type for device dehydration issues. @@ -215,7 +216,9 @@ impl RehydratedDevice { /// /// ```no_run /// # use anyhow::Result; - /// # use matrix_sdk_crypto::{ OlmMachine, store::types::DehydratedDeviceKey }; + /// # use matrix_sdk_crypto::{ + /// DecryptionSettings, OlmMachine, TrustRequirement, store::types::DehydratedDeviceKey + /// }; /// # use ruma::{api::client::dehydrated_device, DeviceId}; /// # async fn example() -> Result<()> { /// # let machine: OlmMachine = unimplemented!(); @@ -245,6 +248,9 @@ impl RehydratedDevice { /// /// let mut since_token = None; /// let mut imported_room_keys = 0; + /// let decryption_settings = DecryptionSettings { + /// sender_device_trust_requirement: TrustRequirement::Untrusted + /// }; /// /// loop { /// let response = @@ -255,7 +261,7 @@ impl RehydratedDevice { /// } /// /// since_token = response.next_batch.as_deref(); - /// imported_room_keys += rehydrated.receive_events(response.events).await?.len(); + /// imported_room_keys += rehydrated.receive_events(response.events, &decryption_settings).await?.len(); /// } /// /// println!("Successfully imported {imported_room_keys} from the dehydrated device."); @@ -273,6 +279,7 @@ impl RehydratedDevice { pub async fn receive_events( &self, events: Vec>, + decryption_settings: &DecryptionSettings, ) -> Result, OlmError> { trace!("Receiving events for a rehydrated Device"); @@ -290,7 +297,7 @@ impl RehydratedDevice { let (_, changes) = self .rehydrated - .preprocess_sync_changes(&mut rehydrated_transaction, sync_changes) + .preprocess_sync_changes(&mut rehydrated_transaction, sync_changes, decryption_settings) .await?; // Now take the room keys and persist them in our original `OlmMachine`. @@ -423,7 +430,7 @@ mod tests { store::types::DehydratedDeviceKey, types::{events::ToDeviceEvent, DeviceKeys as DeviceKeysType}, utilities::json_convert, - EncryptionSettings, OlmMachine, + DecryptionSettings, EncryptionSettings, OlmMachine, TrustRequirement, }; fn pickle_key() -> DehydratedDeviceKey { @@ -568,9 +575,12 @@ mod tests { assert_eq!(rehydrated.rehydrated.device_id(), request.device_id); assert_eq!(rehydrated.original.device_id(), alice.device_id()); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + // Push the to-device event containing the room key into the rehydrated device. let ret = rehydrated - .receive_events(vec![event]) + .receive_events(vec![event], &decryption_settings) .await .expect("We should be able to push to-device events into the rehydrated device"); @@ -680,9 +690,12 @@ mod tests { assert_eq!(rehydrated.rehydrated.device_id(), &device_id); assert_eq!(rehydrated.original.device_id(), alice.device_id()); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + // Push the to-device event containing the room key into the rehydrated device. let ret = rehydrated - .receive_events(vec![event]) + .receive_events(vec![event], &decryption_settings) .await .expect("We should be able to push to-device events into the rehydrated device"); diff --git a/crates/matrix-sdk-crypto/src/gossiping/machine.rs b/crates/matrix-sdk-crypto/src/gossiping/machine.rs index 2c9efbf6063..9b70ec9e473 100644 --- a/crates/matrix-sdk-crypto/src/gossiping/machine.rs +++ b/crates/matrix-sdk-crypto/src/gossiping/machine.rs @@ -1142,6 +1142,7 @@ mod tests { EncryptedEvent, EncryptedToDeviceEvent, RoomEncryptedEventContent, }, verification::VerificationMachine, + DecryptionSettings, TrustRequirement, }; fn alice_id() -> &'static UserId { @@ -2051,14 +2052,20 @@ mod tests { let stream = bob_machine.store().secrets_stream(); pin_mut!(stream); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + bob_machine - .receive_sync_changes(EncryptionSyncChanges { - to_device_events: vec![event], - changed_devices: &Default::default(), - one_time_keys_counts: &Default::default(), - unused_fallback_keys: None, - next_batch_token: None, - }) + .receive_sync_changes( + EncryptionSyncChanges { + to_device_events: vec![event], + changed_devices: &Default::default(), + one_time_keys_counts: &Default::default(), + unused_fallback_keys: None, + next_batch_token: None, + }, + &decryption_settings, + ) .await .unwrap(); diff --git a/crates/matrix-sdk-crypto/src/lib.rs b/crates/matrix-sdk-crypto/src/lib.rs index 8402ef679c3..048e9d2f3d2 100644 --- a/crates/matrix-sdk-crypto/src/lib.rs +++ b/crates/matrix-sdk-crypto/src/lib.rs @@ -590,7 +590,9 @@ pub enum RoomEventDecryptionResult { /// /// ```no_run /// # use anyhow::Result; -/// # use matrix_sdk_crypto::{EncryptionSyncChanges, OlmMachine}; +/// # use matrix_sdk_crypto::{ +/// DecryptionSettings, EncryptionSyncChanges, OlmMachine, TrustRequirement +/// }; /// # use ruma::api::client::sync::sync_events::v3::Response; /// # #[tokio::main] /// # async fn main() -> Result<()> { @@ -617,11 +619,15 @@ pub enum RoomEventDecryptionResult { /// next_batch_token: Some(response.next_batch), /// }; /// +/// let decryption_settings = DecryptionSettings { +/// sender_device_trust_requirement: TrustRequirement::Untrusted +/// }; +/// /// // Push the sync changes into the OlmMachine, make sure that this is /// // happening before the `next_batch` token of the sync is persisted. /// let to_device_events = client /// .olm_machine -/// .receive_sync_changes(sync_changes) +/// .receive_sync_changes(sync_changes, &decryption_settings) /// .await?; /// /// // Send the outgoing requests out that the sync changes produced. @@ -768,7 +774,9 @@ pub enum RoomEventDecryptionResult { /// ```no_run /// # use anyhow::Result; /// # use std::ops::Deref; -/// # use matrix_sdk_crypto::{EncryptionSyncChanges, OlmMachine}; +/// # use matrix_sdk_crypto::{ +/// # DecryptionSettings, EncryptionSyncChanges, OlmMachine, TrustRequirement +/// # }; /// # use ruma::api::client::sync::sync_events::v3::{Response, JoinedRoom}; /// # use ruma::{OwnedUserId, serde::Raw, events::AnySyncStateEvent}; /// # #[tokio::main] @@ -805,11 +813,15 @@ pub enum RoomEventDecryptionResult { /// next_batch_token: Some(response.next_batch), /// }; /// +/// let decryption_settings = DecryptionSettings { +/// sender_device_trust_requirement: TrustRequirement::Untrusted +/// }; +/// /// // Push the sync changes into the OlmMachine, make sure that this is /// // happening before the `next_batch` token of the sync is persisted. /// let to_device_events = client /// .olm_machine -/// .receive_sync_changes(sync_changes) +/// .receive_sync_changes(sync_changes, &decryption_settings) /// .await?; /// /// // Send the outgoing requests out that the sync changes produced. diff --git a/crates/matrix-sdk-crypto/src/machine/mod.rs b/crates/matrix-sdk-crypto/src/machine/mod.rs index bfcf6eb6a44..89e7204203a 100644 --- a/crates/matrix-sdk-crypto/src/machine/mod.rs +++ b/crates/matrix-sdk-crypto/src/machine/mod.rs @@ -859,6 +859,7 @@ impl OlmMachine { transaction: &mut StoreTransaction, event: &EncryptedToDeviceEvent, changes: &mut Changes, + _decryption_settings: &DecryptionSettings, ) -> Result { // Decrypt the event let mut decrypted = @@ -1398,6 +1399,7 @@ impl OlmMachine { transaction: &mut StoreTransaction, changes: &mut Changes, raw_event: Raw, + decryption_settings: &DecryptionSettings, ) -> Option { Self::record_message_id(&raw_event); @@ -1414,7 +1416,14 @@ impl OlmMachine { match event { ToDeviceEvents::RoomEncrypted(e) => { - self.receive_encrypted_to_device_event(transaction, changes, raw_event, e).await + self.receive_encrypted_to_device_event( + transaction, + changes, + raw_event, + e, + decryption_settings, + ) + .await } e => { self.handle_to_device_event(changes, &e).await; @@ -1436,8 +1445,12 @@ impl OlmMachine { changes: &mut Changes, mut raw_event: Raw, e: ToDeviceEvent, + decryption_settings: &DecryptionSettings, ) -> Option { - let decrypted = match self.decrypt_to_device_event(transaction, &e, changes).await { + let decrypted = match self + .decrypt_to_device_event(transaction, &e, changes, decryption_settings) + .await + { Ok(decrypted) => decrypted, Err(DecryptToDeviceError::OlmError(err)) => { if let OlmError::SessionWedged(sender, curve_key) = err { @@ -1546,11 +1559,13 @@ impl OlmMachine { pub async fn receive_sync_changes( &self, sync_changes: EncryptionSyncChanges<'_>, + decryption_settings: &DecryptionSettings, ) -> OlmResult<(Vec, Vec)> { let mut store_transaction = self.inner.store.transaction().await; - let (events, changes) = - self.preprocess_sync_changes(&mut store_transaction, sync_changes).await?; + let (events, changes) = self + .preprocess_sync_changes(&mut store_transaction, sync_changes, decryption_settings) + .await?; // Technically save_changes also does the same work, so if it's slow we could // refactor this to do it only once. @@ -1575,6 +1590,7 @@ impl OlmMachine { &self, transaction: &mut StoreTransaction, sync_changes: EncryptionSyncChanges<'_>, + decryption_settings: &DecryptionSettings, ) -> OlmResult<(Vec, Changes)> { // Remove verification objects that have expired or are done. let mut events: Vec = self @@ -1612,8 +1628,13 @@ impl OlmMachine { } for raw_event in sync_changes.to_device_events { - let processed_event = - Box::pin(self.receive_to_device_event(transaction, &mut changes, raw_event)).await; + let processed_event = Box::pin(self.receive_to_device_event( + transaction, + &mut changes, + raw_event, + decryption_settings, + )) + .await; if let Some(processed_event) = processed_event { events.push(processed_event); diff --git a/crates/matrix-sdk-crypto/src/machine/test_helpers.rs b/crates/matrix-sdk-crypto/src/machine/test_helpers.rs index cc757ff663f..818aac43b90 100644 --- a/crates/matrix-sdk-crypto/src/machine/test_helpers.rs +++ b/crates/matrix-sdk-crypto/src/machine/test_helpers.rs @@ -47,8 +47,8 @@ use crate::{ }, utilities::json_convert, verification::VerificationMachine, - Account, CrossSigningBootstrapRequests, Device, DeviceData, EncryptionSyncChanges, OlmMachine, - OtherUserIdentityData, + Account, CrossSigningBootstrapRequests, DecryptionSettings, Device, DeviceData, + EncryptionSyncChanges, OlmMachine, OtherUserIdentityData, TrustRequirement, }; /// These keys need to be periodically uploaded to the server. @@ -218,7 +218,11 @@ pub async fn send_and_receive_encrypted_to_device_test_helper( next_batch_token: None, }; - let (decrypted, _) = recipient.receive_sync_changes(sync_changes).await.unwrap(); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + + let (decrypted, _) = + recipient.receive_sync_changes(sync_changes, &decryption_settings).await.unwrap(); assert_eq!(1, decrypted.len()); decrypted[0].clone() } @@ -266,10 +270,20 @@ pub async fn get_machine_pair_with_setup_sessions_test_helper( let event = ToDeviceEvent::new(alice.user_id().to_owned(), content.deserialize_as_unchecked().unwrap()); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let decrypted = bob .store() .with_transaction(|mut tr| async { - let res = bob.decrypt_to_device_event(&mut tr, &event, &mut Changes::default()).await?; + let res = bob + .decrypt_to_device_event( + &mut tr, + &event, + &mut Changes::default(), + &decryption_settings, + ) + .await?; Ok((tr, res)) }) .await 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 9581034b8cd..c2f49af52b0 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 @@ -79,10 +79,20 @@ async fn test_decryption_verification_state() { tests::to_device_requests_to_content(to_device_requests), ); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let group_session = bob .store() .with_transaction(|mut tr| async { - let res = bob.decrypt_to_device_event(&mut tr, &event, &mut Changes::default()).await?; + let res = bob + .decrypt_to_device_event( + &mut tr, + &event, + &mut Changes::default(), + &decryption_settings, + ) + .await?; Ok((tr, res)) }) .await @@ -633,11 +643,20 @@ async fn encrypt_message( tests::to_device_requests_to_content(to_device_requests), ); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let group_session = recipient .store() .with_transaction(|mut tr| async { - let res = - recipient.decrypt_to_device_event(&mut tr, &event, &mut Changes::default()).await?; + let res = recipient + .decrypt_to_device_event( + &mut tr, + &event, + &mut Changes::default(), + &decryption_settings, + ) + .await?; Ok((tr, res)) }) .await diff --git a/crates/matrix-sdk-crypto/src/machine/tests/megolm_sender_data.rs b/crates/matrix-sdk-crypto/src/machine/tests/megolm_sender_data.rs index 48ad9230912..9427af3e6a9 100644 --- a/crates/matrix-sdk-crypto/src/machine/tests/megolm_sender_data.rs +++ b/crates/matrix-sdk-crypto/src/machine/tests/megolm_sender_data.rs @@ -35,7 +35,8 @@ use crate::{ olm::{InboundGroupSession, SenderData}, store::types::RoomKeyInfo, types::events::{room::encrypted::ToDeviceEncryptedEventContent, EventType, ToDeviceEvent}, - DeviceData, EncryptionSettings, EncryptionSyncChanges, OlmMachine, Session, + DecryptionSettings, DeviceData, EncryptionSettings, EncryptionSyncChanges, OlmMachine, Session, + TrustRequirement, }; /// Test the behaviour when a megolm session is received from an unknown device, @@ -55,8 +56,11 @@ async fn test_receive_megolm_session_from_unknown_device() { let room_id = room_id!("!test:example.org"); let event = create_and_share_session_without_sender_data(&alice, &bob, room_id).await; + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + // Bob receives the to-device message - receive_to_device_event(&bob, &event).await; + receive_to_device_event(&bob, &event, &decryption_settings).await; // Then Bob should know about the session, and it should have // `SenderData::UnknownDevice`. @@ -92,8 +96,11 @@ async fn test_receive_megolm_session_from_known_device() { ), ); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + // Bob receives the to-device message - receive_to_device_event(&bob, &event).await; + receive_to_device_event(&bob, &event, &decryption_settings).await; // Then Bob should know about the session, and it should have // `SenderData::DeviceInfo` @@ -126,8 +133,11 @@ async fn test_update_unknown_device_senderdata_on_keys_query() { let room_id = room_id!("!test:example.org"); let event = create_and_share_session_without_sender_data(&alice, &bob, room_id).await; + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + // Bob receives the to-device message - receive_to_device_event(&bob, &event).await; + receive_to_device_event(&bob, &event, &decryption_settings).await; // and now Bob should know about the session. let room_key_info = get_room_key_received_update(&mut bob_room_keys_received_stream); @@ -183,8 +193,12 @@ async fn test_update_device_info_senderdata_on_keys_query() { alice.user_id().to_owned(), to_device_requests_to_content(to_device_requests), ); + + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + // Bob receives the to-device message - receive_to_device_event(&bob, &event).await; + receive_to_device_event(&bob, &event, &decryption_settings).await; // and now Bob should know about the session. let room_key_info = get_room_key_received_update(&mut bob_room_keys_received_stream); @@ -287,6 +301,7 @@ async fn create_and_share_session_without_sender_data( pub async fn receive_to_device_event( machine: &OlmMachine, event: &ToDeviceEvent, + decryption_settings: &DecryptionSettings, ) -> (Vec, Vec) where C: EventType + Serialize + Debug, @@ -294,13 +309,16 @@ where let event_json = serde_json::to_string(event).expect("Unable to serialize to-device message"); machine - .receive_sync_changes(EncryptionSyncChanges { - to_device_events: vec![serde_json::from_str(&event_json).unwrap()], - changed_devices: &Default::default(), - one_time_keys_counts: &Default::default(), - unused_fallback_keys: None, - next_batch_token: None, - }) + .receive_sync_changes( + EncryptionSyncChanges { + to_device_events: vec![serde_json::from_str(&event_json).unwrap()], + changed_devices: &Default::default(), + one_time_keys_counts: &Default::default(), + unused_fallback_keys: None, + next_batch_token: None, + }, + decryption_settings, + ) .await .expect("Error receiving to-device event") } diff --git a/crates/matrix-sdk-crypto/src/machine/tests/mod.rs b/crates/matrix-sdk-crypto/src/machine/tests/mod.rs index 662358eddd4..a60f9cc3178 100644 --- a/crates/matrix-sdk-crypto/src/machine/tests/mod.rs +++ b/crates/matrix-sdk-crypto/src/machine/tests/mod.rs @@ -284,15 +284,22 @@ fn test_one_time_key_signing() { async fn test_keys_for_upload() { let machine = OlmMachine::new(user_id(), alice_device_id()).await; + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let key_counts = BTreeMap::from([(OneTimeKeyAlgorithm::SignedCurve25519, 49u8.into())]); + machine - .receive_sync_changes(EncryptionSyncChanges { - to_device_events: Vec::new(), - changed_devices: &Default::default(), - one_time_keys_counts: &key_counts, - unused_fallback_keys: None, - next_batch_token: None, - }) + .receive_sync_changes( + EncryptionSyncChanges { + to_device_events: Vec::new(), + changed_devices: &Default::default(), + one_time_keys_counts: &key_counts, + unused_fallback_keys: None, + next_batch_token: None, + }, + &decryption_settings, + ) .await .expect("We should be able to update our one-time key counts"); @@ -506,14 +513,20 @@ async fn send_room_key_to_device( ); let event = json_convert(&event).unwrap(); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + receiver - .receive_sync_changes(EncryptionSyncChanges { - to_device_events: vec![event], - changed_devices: &Default::default(), - one_time_keys_counts: &Default::default(), - unused_fallback_keys: None, - next_batch_token: None, - }) + .receive_sync_changes( + EncryptionSyncChanges { + to_device_events: vec![event], + changed_devices: &Default::default(), + one_time_keys_counts: &Default::default(), + unused_fallback_keys: None, + next_batch_token: None, + }, + &decryption_settings, + ) .await } @@ -631,10 +644,20 @@ async fn test_megolm_encryption() { let mut room_keys_received_stream = Box::pin(bob.store().room_keys_received_stream()); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let group_session = bob .store() .with_transaction(|mut tr| async { - let res = bob.decrypt_to_device_event(&mut tr, &event, &mut Changes::default()).await?; + let res = bob + .decrypt_to_device_event( + &mut tr, + &event, + &mut Changes::default(), + &decryption_settings, + ) + .await?; Ok((tr, res)) }) .await @@ -742,13 +765,19 @@ async fn test_withheld_unverified() { let event = json_convert(&event).unwrap(); - bob.receive_sync_changes(EncryptionSyncChanges { - to_device_events: vec![event], - changed_devices: &Default::default(), - one_time_keys_counts: &Default::default(), - unused_fallback_keys: None, - next_batch_token: None, - }) + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + + bob.receive_sync_changes( + EncryptionSyncChanges { + to_device_events: vec![event], + changed_devices: &Default::default(), + one_time_keys_counts: &Default::default(), + unused_fallback_keys: None, + next_batch_token: None, + }, + &decryption_settings, + ) .await .unwrap(); @@ -1002,10 +1031,20 @@ async fn test_query_ratcheted_key() { to_device_requests_to_content(to_device_requests), ); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let group_session = bob .store() .with_transaction(|mut tr| async { - let res = bob.decrypt_to_device_event(&mut tr, &event, &mut Changes::default()).await?; + let res = bob + .decrypt_to_device_event( + &mut tr, + &event, + &mut Changes::default(), + &decryption_settings, + ) + .await?; Ok((tr, res)) }) .await @@ -1054,14 +1093,20 @@ async fn test_room_key_over_megolm() { let changed_devices = DeviceLists::new(); let key_counts: BTreeMap<_, _> = Default::default(); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let _ = bob - .receive_sync_changes(EncryptionSyncChanges { - to_device_events: vec![event], - changed_devices: &changed_devices, - one_time_keys_counts: &key_counts, - unused_fallback_keys: None, - next_batch_token: None, - }) + .receive_sync_changes( + EncryptionSyncChanges { + to_device_events: vec![event], + changed_devices: &changed_devices, + one_time_keys_counts: &key_counts, + unused_fallback_keys: None, + next_batch_token: None, + }, + &decryption_settings, + ) .await .unwrap(); @@ -1086,10 +1131,20 @@ async fn test_room_key_over_megolm() { let event: EncryptedToDeviceEvent = serde_json::from_value(event).unwrap(); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let decrypt_result = bob .store() .with_transaction(|mut tr| async { - let res = bob.decrypt_to_device_event(&mut tr, &event, &mut Changes::default()).await?; + let res = bob + .decrypt_to_device_event( + &mut tr, + &event, + &mut Changes::default(), + &decryption_settings, + ) + .await?; Ok((tr, res)) }) .await; @@ -1098,13 +1153,19 @@ async fn test_room_key_over_megolm() { let event: Raw = json_convert(&event).unwrap(); - bob.receive_sync_changes(EncryptionSyncChanges { - to_device_events: vec![event], - changed_devices: &changed_devices, - one_time_keys_counts: &key_counts, - unused_fallback_keys: None, - next_batch_token: None, - }) + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + + bob.receive_sync_changes( + EncryptionSyncChanges { + to_device_events: vec![event], + changed_devices: &changed_devices, + one_time_keys_counts: &key_counts, + unused_fallback_keys: None, + next_batch_token: None, + }, + &decryption_settings, + ) .await .unwrap(); @@ -1385,12 +1446,20 @@ async fn test_unsigned_decryption() { to_device_requests_to_content(to_device_requests), ); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + // Save the first room key. let group_session = bob .store() .with_transaction(|mut tr| async { let res = bob - .decrypt_to_device_event(&mut tr, &first_room_key_event, &mut Changes::default()) + .decrypt_to_device_event( + &mut tr, + &first_room_key_event, + &mut Changes::default(), + &decryption_settings, + ) .await?; Ok((tr, res)) }) @@ -1490,12 +1559,20 @@ async fn test_unsigned_decryption() { }) ); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + // Give Bob the second room key. let group_session = bob .store() .with_transaction(|mut tr| async { let res = bob - .decrypt_to_device_event(&mut tr, &second_room_key_event, &mut Changes::default()) + .decrypt_to_device_event( + &mut tr, + &second_room_key_event, + &mut Changes::default(), + &decryption_settings, + ) .await?; Ok((tr, res)) }) @@ -1599,12 +1676,20 @@ async fn test_unsigned_decryption() { }) ); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + // Give Bob the third room key. let group_session = bob .store() .with_transaction(|mut tr| async { let res = bob - .decrypt_to_device_event(&mut tr, &third_room_key_event, &mut Changes::default()) + .decrypt_to_device_event( + &mut tr, + &third_room_key_event, + &mut Changes::default(), + &decryption_settings, + ) .await?; Ok((tr, res)) }) diff --git a/crates/matrix-sdk-crypto/src/machine/tests/olm_encryption.rs b/crates/matrix-sdk-crypto/src/machine/tests/olm_encryption.rs index 8a2c32e0a75..a24c70c5001 100644 --- a/crates/matrix-sdk-crypto/src/machine/tests/olm_encryption.rs +++ b/crates/matrix-sdk-crypto/src/machine/tests/olm_encryption.rs @@ -34,13 +34,12 @@ use crate::{ create_session, get_machine_pair, get_machine_pair_with_session, get_machine_pair_with_setup_sessions_test_helper, }, - tests, - tests::megolm_sender_data::receive_to_device_event, + tests::{self, megolm_sender_data::receive_to_device_event}, }, olm::utility::SignJson, store::types::Changes, types::{events::ToDeviceEvent, DeviceKeys}, - DeviceData, OlmMachine, + DecryptionSettings, DeviceData, OlmMachine, TrustRequirement, }; #[async_test] @@ -212,11 +211,21 @@ async fn olm_encryption_test_helper(use_fallback_key: bool) { .expect("We should be able to deserialize the encrypted content"), ); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + // Decrypting the first time should succeed. let decrypted = bob .store() .with_transaction(|mut tr| async { - let res = bob.decrypt_to_device_event(&mut tr, &event, &mut Changes::default()).await?; + let res = bob + .decrypt_to_device_event( + &mut tr, + &event, + &mut Changes::default(), + &decryption_settings, + ) + .await?; Ok((tr, res)) }) .await @@ -232,7 +241,14 @@ async fn olm_encryption_test_helper(use_fallback_key: bool) { // Replaying the event should now result in a decryption failure. bob.store() .with_transaction(|mut tr| async { - let res = bob.decrypt_to_device_event(&mut tr, &event, &mut Changes::default()).await?; + let res = bob + .decrypt_to_device_event( + &mut tr, + &event, + &mut Changes::default(), + &decryption_settings, + ) + .await?; Ok((tr, res)) }) .await @@ -292,8 +308,11 @@ async fn test_decrypt_to_device_message_with_unsigned_sender_keys() { alice_session.build_encrypted_event(ciphertext, None).await.unwrap(), ); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + // Bob receives the to-device message - let (to_device_events, _) = receive_to_device_event(&bob, &event).await; + let (to_device_events, _) = receive_to_device_event(&bob, &event, &decryption_settings).await; let event = to_device_events.first().expect("Bob did not get a to-device event").clone(); diff --git a/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs b/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs index 74c925e6837..962e4aba782 100644 --- a/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs +++ b/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs @@ -27,8 +27,7 @@ use crate::{ build_session_for_pair, get_machine_pair, get_machine_pair_with_session, get_prepared_machine_test_helper, send_and_receive_encrypted_to_device_test_helper, }, - tests, - tests::decryption_verification_state::mark_alice_identity_as_verified_test_helper, + tests::{self, decryption_verification_state::mark_alice_identity_as_verified_test_helper}, }, types::{ events::{ToDeviceCustomEvent, ToDeviceEvent}, @@ -36,7 +35,8 @@ use crate::{ }, utilities::json_convert, verification::tests::bob_id, - DeviceData, EncryptionSyncChanges, LocalTrust, OlmError, OlmMachine, + DecryptionSettings, DeviceData, EncryptionSyncChanges, LocalTrust, OlmError, OlmMachine, + TrustRequirement, }; #[async_test] @@ -90,7 +90,11 @@ async fn test_send_encrypted_to_device() { next_batch_token: None, }; - let (decrypted, _) = bob.receive_sync_changes(sync_changes).await.unwrap(); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + + let (decrypted, _) = + bob.receive_sync_changes(sync_changes, &decryption_settings).await.unwrap(); assert_eq!(1, decrypted.len()); let processed_event = &decrypted[0]; @@ -188,7 +192,11 @@ async fn test_receive_custom_encrypted_to_device_fails_if_device_unknown() { next_batch_token: None, }; - let (decrypted, _) = bob.receive_sync_changes(sync_changes).await.unwrap(); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + + let (decrypted, _) = + bob.receive_sync_changes(sync_changes, &decryption_settings).await.unwrap(); assert_eq!(1, decrypted.len()); let processed_event = &decrypted[0]; @@ -453,7 +461,11 @@ async fn test_processed_to_device_variants() { next_batch_token: None, }; - let (processed, _) = bob.receive_sync_changes(sync_changes).await.unwrap(); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + + let (processed, _) = + bob.receive_sync_changes(sync_changes, &decryption_settings).await.unwrap(); assert_eq!(4, processed.len()); 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 f42ea7723b0..484b8594052 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 @@ -1068,7 +1068,7 @@ mod tests { requests::ToDeviceRequest, DeviceKeys, EventEncryptionAlgorithm, }, - EncryptionSettings, LocalTrust, OlmMachine, + DecryptionSettings, EncryptionSettings, LocalTrust, OlmMachine, TrustRequirement, }; fn alice_id() -> &'static UserId { @@ -1649,7 +1649,12 @@ mod tests { unused_fallback_keys: None, next_batch_token: None, }; - let (decrypted, _) = machine.receive_sync_changes(sync_changes).await.unwrap(); + + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + + let (decrypted, _) = + machine.receive_sync_changes(sync_changes, &decryption_settings).await.unwrap(); assert_eq!(1, decrypted.len()); } @@ -1714,7 +1719,12 @@ mod tests { unused_fallback_keys: None, next_batch_token: None, }; - let (decrypted, _) = machine.receive_sync_changes(sync_changes).await.unwrap(); + + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + + let (decrypted, _) = + machine.receive_sync_changes(sync_changes, &decryption_settings).await.unwrap(); assert_eq!(1, decrypted.len()); } @@ -1822,7 +1832,12 @@ mod tests { unused_fallback_keys: None, next_batch_token: None, }; - let (decrypted, _) = bob.receive_sync_changes(sync_changes).await.unwrap(); + + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + + let (decrypted, _) = + bob.receive_sync_changes(sync_changes, &decryption_settings).await.unwrap(); assert_eq!(1, decrypted.len()); use crate::types::events::EventType; assert_let!( From aa87c142304a7a50df1daa5457ddeb44ec92c0f8 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Wed, 25 Jun 2025 14:08:14 +0100 Subject: [PATCH 2/6] refactor(crypto): Extract a method to check for to-device events from dehydrated devices --- crates/matrix-sdk-crypto/src/machine/mod.rs | 45 ++++++++++++--------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/machine/mod.rs b/crates/matrix-sdk-crypto/src/machine/mod.rs index 89e7204203a..a13df5d743f 100644 --- a/crates/matrix-sdk-crypto/src/machine/mod.rs +++ b/crates/matrix-sdk-crypto/src/machine/mod.rs @@ -865,27 +865,13 @@ impl OlmMachine { let mut decrypted = transaction.account().await?.decrypt_to_device_event(&self.inner.store, event).await?; - let from_dehydrated_device = - self.to_device_event_is_from_dehydrated_device(&decrypted, &event.sender).await?; - - // Check whether this event is from a dehydrated device - if so, return Ok(None) - // to skip it because we don't expect ever to receive an event from a - // dehydrated device. - if from_dehydrated_device { - // Device is dehydrated: ignore this event - warn!( - sender = ?event.sender, - session = ?decrypted.session, - "Received a to-device event from a dehydrated device. This is unexpected: ignoring event" - ); - Err(DecryptToDeviceError::FromDehydratedDevice) - } else { - // Device is not dehydrated: handle it as normal e.g. create a Megolm session - self.handle_decrypted_to_device_event(transaction.cache(), &mut decrypted, changes) - .await?; + // Return early if the sending device is decrypted + self.check_to_device_event_is_not_from_dehydrated_device(&decrypted, &event.sender).await?; - Ok(decrypted) - } + // Device is not dehydrated: handle it as normal e.g. create a Megolm session + self.handle_decrypted_to_device_event(transaction.cache(), &mut decrypted, changes).await?; + + Ok(decrypted) } #[instrument( @@ -1504,6 +1490,25 @@ impl OlmMachine { }) } + /// Return an error if the supplied to-device event was sent from a + /// dehydrated device. + async fn check_to_device_event_is_not_from_dehydrated_device( + &self, + decrypted: &OlmDecryptionInfo, + sender_user_id: &UserId, + ) -> Result<(), DecryptToDeviceError> { + if self.to_device_event_is_from_dehydrated_device(decrypted, sender_user_id).await? { + warn!( + sender = ?sender_user_id, + session = ?decrypted.session, + "Received a to-device event from a dehydrated device. This is unexpected: ignoring event" + ); + Err(DecryptToDeviceError::FromDehydratedDevice) + } else { + Ok(()) + } + } + /// Decide whether a decrypted to-device event was sent from a dehydrated /// device. /// From 2fe09fbe6875623ab1d2288fce95cef18b4c0b97 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 1 Jul 2025 11:26:25 +0100 Subject: [PATCH 3/6] refactor(tests): Take a reference to content in send_and_receive_encrypted_to_device_test_helper This will allow us to re-use it in more tests. --- crates/matrix-sdk-crypto/src/machine/test_helpers.rs | 2 +- .../src/machine/tests/send_encrypted_to_device.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/machine/test_helpers.rs b/crates/matrix-sdk-crypto/src/machine/test_helpers.rs index 818aac43b90..14f2701a823 100644 --- a/crates/matrix-sdk-crypto/src/machine/test_helpers.rs +++ b/crates/matrix-sdk-crypto/src/machine/test_helpers.rs @@ -187,7 +187,7 @@ pub async fn send_and_receive_encrypted_to_device_test_helper( sender: &OlmMachine, recipient: &OlmMachine, event_type: &str, - content: Value, + content: &Value, ) -> ProcessedToDeviceEvent { let device = sender.get_device(recipient.user_id(), recipient.device_id(), None).await.unwrap().unwrap(); diff --git a/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs b/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs index 962e4aba782..0e7d6262013 100644 --- a/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs +++ b/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs @@ -226,7 +226,7 @@ async fn test_send_olm_encryption_info_unverified_identity() { &alice, &bob, custom_event_type, - custom_content, + &custom_content, ) .await; @@ -266,7 +266,7 @@ async fn test_send_olm_encryption_info_verified_identity() { &alice, &bob, custom_event_type, - custom_content, + &custom_content, ) .await; @@ -302,7 +302,7 @@ async fn test_send_olm_encryption_info_verified_locally() { &alice, &bob, custom_event_type, - custom_content, + &custom_content, ) .await; @@ -344,7 +344,7 @@ async fn test_send_olm_encryption_info_verification_violation() { &alice, &bob, custom_event_type, - custom_content, + &custom_content, ) .await; From 31a16a177baaa8514c214eb28ff65bb77775449b Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 1 Jul 2025 11:34:40 +0100 Subject: [PATCH 4/6] refactor(tests): Re-use send_and_receive_encrypted_to_device_test_helper in 2 more tests --- .../machine/tests/send_encrypted_to_device.rs | 99 +++---------------- 1 file changed, 15 insertions(+), 84 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs b/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs index 0e7d6262013..f9e7427ef3e 100644 --- a/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs +++ b/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs @@ -51,53 +51,13 @@ async fn test_send_encrypted_to_device() { "rooms": ["!726s6s6q:example.com"] }); - let device = alice.get_device(bob.user_id(), bob.device_id(), None).await.unwrap().unwrap(); - let raw_encrypted = device - .encrypt_event_raw(custom_event_type, &custom_content) - .await - .expect("Should have encrypted the content"); - - let request = ToDeviceRequest::new( - bob.user_id(), - DeviceIdOrAllDevices::DeviceId(tests::bob_device_id().to_owned()), - "m.room.encrypted", - raw_encrypted.cast(), - ); - - assert_eq!("m.room.encrypted", request.event_type.to_string()); - - let messages = &request.messages; - assert_eq!(1, messages.len()); - assert!(messages.get(bob.user_id()).is_some()); - let target_devices = messages.get(bob.user_id()).unwrap(); - assert_eq!(1, target_devices.len()); - assert!(target_devices - .get(&DeviceIdOrAllDevices::DeviceId(tests::bob_device_id().to_owned())) - .is_some()); - - let event = ToDeviceEvent::new( - alice.user_id().to_owned(), - tests::to_device_requests_to_content(vec![request.clone().into()]), - ); - - let event = json_convert(&event).unwrap(); - - let sync_changes = EncryptionSyncChanges { - to_device_events: vec![event], - changed_devices: &Default::default(), - one_time_keys_counts: &Default::default(), - unused_fallback_keys: None, - next_batch_token: None, - }; - - let decryption_settings = - DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; - - let (decrypted, _) = - bob.receive_sync_changes(sync_changes, &decryption_settings).await.unwrap(); - - assert_eq!(1, decrypted.len()); - let processed_event = &decrypted[0]; + let processed_event = send_and_receive_encrypted_to_device_test_helper( + &alice, + &bob, + custom_event_type, + &custom_content, + ) + .await; assert_let!(ProcessedToDeviceEvent::Decrypted { raw, encryption_info } = processed_event); @@ -105,7 +65,7 @@ async fn test_send_encrypted_to_device() { assert_eq!(decrypted_event.event_type().to_string(), custom_event_type.to_owned()); - let decrypted_value = to_raw_value(&decrypted[0].to_raw()).unwrap(); + let decrypted_value = to_raw_value(&raw).unwrap(); let decrypted_value = serde_json::to_value(decrypted_value).unwrap(); assert_eq!( @@ -164,42 +124,13 @@ async fn test_receive_custom_encrypted_to_device_fails_if_device_unknown() { "rooms": ["!726s6s6q:example.com"] }); - let device = alice.get_device(bob.user_id(), bob.device_id(), None).await.unwrap().unwrap(); - let raw_encrypted = device - .encrypt_event_raw(custom_event_type, &custom_content) - .await - .expect("Should have encrypted the content"); - - let request = ToDeviceRequest::new( - bob.user_id(), - DeviceIdOrAllDevices::DeviceId(tests::bob_device_id().to_owned()), - "m.room.encrypted", - raw_encrypted.cast(), - ); - - let event = ToDeviceEvent::new( - alice.user_id().to_owned(), - tests::to_device_requests_to_content(vec![request.clone().into()]), - ); - - let event = json_convert(&event).unwrap(); - - let sync_changes = EncryptionSyncChanges { - to_device_events: vec![event], - changed_devices: &Default::default(), - one_time_keys_counts: &Default::default(), - unused_fallback_keys: None, - next_batch_token: None, - }; - - let decryption_settings = - DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; - - let (decrypted, _) = - bob.receive_sync_changes(sync_changes, &decryption_settings).await.unwrap(); - - assert_eq!(1, decrypted.len()); - let processed_event = &decrypted[0]; + let processed_event = send_and_receive_encrypted_to_device_test_helper( + &alice, + &bob, + custom_event_type, + &custom_content, + ) + .await; assert_let!(ProcessedToDeviceEvent::UnableToDecrypt(_) = processed_event); } From 1684a832edd6f04bb248c9fd9b060881a83e1b13 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 1 Jul 2025 11:38:26 +0100 Subject: [PATCH 5/6] refactor(tests): Pass decryption_settings in to send_and_receive_encrypted_to_device_test_helper To allow passing in different values in future tests. --- .../src/machine/test_helpers.rs | 5 ++-- .../machine/tests/send_encrypted_to_device.rs | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/machine/test_helpers.rs b/crates/matrix-sdk-crypto/src/machine/test_helpers.rs index 14f2701a823..8b940f23c3c 100644 --- a/crates/matrix-sdk-crypto/src/machine/test_helpers.rs +++ b/crates/matrix-sdk-crypto/src/machine/test_helpers.rs @@ -188,6 +188,7 @@ pub async fn send_and_receive_encrypted_to_device_test_helper( recipient: &OlmMachine, event_type: &str, content: &Value, + decryption_settings: &DecryptionSettings, ) -> ProcessedToDeviceEvent { let device = sender.get_device(recipient.user_id(), recipient.device_id(), None).await.unwrap().unwrap(); @@ -218,11 +219,9 @@ pub async fn send_and_receive_encrypted_to_device_test_helper( next_batch_token: None, }; - let decryption_settings = - DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; - let (decrypted, _) = recipient.receive_sync_changes(sync_changes, &decryption_settings).await.unwrap(); + assert_eq!(1, decrypted.len()); decrypted[0].clone() } diff --git a/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs b/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs index f9e7427ef3e..53c036ab105 100644 --- a/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs +++ b/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs @@ -51,11 +51,15 @@ async fn test_send_encrypted_to_device() { "rooms": ["!726s6s6q:example.com"] }); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let processed_event = send_and_receive_encrypted_to_device_test_helper( &alice, &bob, custom_event_type, &custom_content, + &decryption_settings, ) .await; @@ -124,11 +128,15 @@ async fn test_receive_custom_encrypted_to_device_fails_if_device_unknown() { "rooms": ["!726s6s6q:example.com"] }); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let processed_event = send_and_receive_encrypted_to_device_test_helper( &alice, &bob, custom_event_type, &custom_content, + &decryption_settings, ) .await; @@ -153,11 +161,15 @@ async fn test_send_olm_encryption_info_unverified_identity() { "rooms": ["!726s6s6q:example.com"] }); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let processed_event = send_and_receive_encrypted_to_device_test_helper( &alice, &bob, custom_event_type, &custom_content, + &decryption_settings, ) .await; @@ -193,11 +205,15 @@ async fn test_send_olm_encryption_info_verified_identity() { "rooms": ["!726s6s6q:example.com"] }); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let processed_event = send_and_receive_encrypted_to_device_test_helper( &alice, &bob, custom_event_type, &custom_content, + &decryption_settings, ) .await; @@ -229,11 +245,15 @@ async fn test_send_olm_encryption_info_verified_locally() { .await .unwrap(); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let processed_event = send_and_receive_encrypted_to_device_test_helper( &alice, &bob, custom_event_type, &custom_content, + &decryption_settings, ) .await; @@ -271,11 +291,15 @@ async fn test_send_olm_encryption_info_verification_violation() { "rooms": ["!726s6s6q:example.com"] }); + let decryption_settings = + DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let processed_event = send_and_receive_encrypted_to_device_test_helper( &alice, &bob, custom_event_type, &custom_content, + &decryption_settings, ) .await; From a597752641ba09d4d5e49e5712352d4e324b3fe4 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 1 Jul 2025 13:04:47 +0100 Subject: [PATCH 6/6] feat(crypto): Refuse to decrypt to-device messages from unverified devices (when in exclude insecure mode) --- crates/matrix-sdk-base/src/client.rs | 17 +- .../src/response_processors/e2ee/to_device.rs | 11 +- .../src/deserialized_responses.rs | 38 ++- crates/matrix-sdk-crypto/CHANGELOG.md | 13 + crates/matrix-sdk-crypto/src/error.rs | 8 + .../src/gossiping/machine.rs | 24 +- crates/matrix-sdk-crypto/src/machine/mod.rs | 73 ++++-- .../src/machine/test_helpers.rs | 4 +- .../src/machine/tests/olm_encryption.rs | 10 +- .../machine/tests/send_encrypted_to_device.rs | 223 +++++++++++++++++- crates/matrix-sdk-crypto/src/olm/account.rs | 151 +++++++++++- 11 files changed, 525 insertions(+), 47 deletions(-) diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 4b54c922b96..332b5070128 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -624,14 +624,25 @@ impl BaseClient { .events .into_iter() .map(|raw| { + use matrix_sdk_common::deserialized_responses::{ + ProcessedToDeviceEvent, ToDeviceUnableToDecryptInfo, + ToDeviceUnableToDecryptReason, + }; + if let Ok(Some(event_type)) = raw.get_field::("type") { if event_type == "m.room.encrypted" { - matrix_sdk_common::deserialized_responses::ProcessedToDeviceEvent::UnableToDecrypt(raw) + ProcessedToDeviceEvent::UnableToDecrypt { + encrypted_event: raw, + utd_info: ToDeviceUnableToDecryptInfo { + reason: ToDeviceUnableToDecryptReason::EncryptionIsDisabled, + }, + } } else { - matrix_sdk_common::deserialized_responses::ProcessedToDeviceEvent::PlainText(raw) + ProcessedToDeviceEvent::PlainText(raw) } } else { - matrix_sdk_common::deserialized_responses::ProcessedToDeviceEvent::Invalid(raw) // Exclude events with no type + // Exclude events with no type + ProcessedToDeviceEvent::Invalid(raw) } }) .collect(); diff --git a/crates/matrix-sdk-base/src/response_processors/e2ee/to_device.rs b/crates/matrix-sdk-base/src/response_processors/e2ee/to_device.rs index 31b2d55a708..f30a3085254 100644 --- a/crates/matrix-sdk-base/src/response_processors/e2ee/to_device.rs +++ b/crates/matrix-sdk-base/src/response_processors/e2ee/to_device.rs @@ -14,7 +14,9 @@ use std::collections::BTreeMap; -use matrix_sdk_common::deserialized_responses::ProcessedToDeviceEvent; +use matrix_sdk_common::deserialized_responses::{ + ProcessedToDeviceEvent, ToDeviceUnableToDecryptInfo, ToDeviceUnableToDecryptReason, +}; use matrix_sdk_crypto::{ DecryptionSettings, EncryptionSyncChanges, OlmMachine, store::types::RoomKeyInfo, }; @@ -114,7 +116,12 @@ async fn process( .map(|raw| { if let Ok(Some(event_type)) = raw.get_field::("type") { if event_type == "m.room.encrypted" { - ProcessedToDeviceEvent::UnableToDecrypt(raw) + ProcessedToDeviceEvent::UnableToDecrypt { + encrypted_event: raw, + utd_info: ToDeviceUnableToDecryptInfo { + reason: ToDeviceUnableToDecryptReason::NoOlmMachine, + }, + } } else { ProcessedToDeviceEvent::PlainText(raw) } diff --git a/crates/matrix-sdk-common/src/deserialized_responses.rs b/crates/matrix-sdk-common/src/deserialized_responses.rs index 6c6ebfdb408..9982b40d8ca 100644 --- a/crates/matrix-sdk-common/src/deserialized_responses.rs +++ b/crates/matrix-sdk-common/src/deserialized_responses.rs @@ -1179,6 +1179,33 @@ impl From for TimelineEvent { } } +/// Reason code for a to-device decryption failure +#[derive(Debug, Clone, PartialEq)] +pub enum ToDeviceUnableToDecryptReason { + /// An error occurred while encrypting the event. This covers all + /// `OlmError` types. + DecryptionFailure, + + /// We refused to decrypt the message because the sender's device is not + /// verified, or more generally, the sender's identity did not match the + /// trust requirement we were asked to provide. + UnverifiedSenderDevice, + + /// We have no `OlmMachine`. This should not happen unless we forget to set + /// things up by calling `OlmMachine::activate()`. + NoOlmMachine, + + /// The Matrix SDK was compiled without encryption support. + EncryptionIsDisabled, +} + +/// Metadata about a to-device event that could not be decrypted. +#[derive(Clone, Debug)] +pub struct ToDeviceUnableToDecryptInfo { + /// Reason code for the decryption failure + pub reason: ToDeviceUnableToDecryptReason, +} + /// Represents a to-device event after it has been processed by the Olm machine. #[derive(Clone, Debug)] pub enum ProcessedToDeviceEvent { @@ -1192,7 +1219,10 @@ pub enum ProcessedToDeviceEvent { }, /// An encrypted event which could not be decrypted. - UnableToDecrypt(Raw), + UnableToDecrypt { + encrypted_event: Raw, + utd_info: ToDeviceUnableToDecryptInfo, + }, /// An unencrypted event. PlainText(Raw), @@ -1209,7 +1239,9 @@ impl ProcessedToDeviceEvent { pub fn to_raw(&self) -> Raw { match self { ProcessedToDeviceEvent::Decrypted { raw, .. } => raw.clone(), - ProcessedToDeviceEvent::UnableToDecrypt(event) => event.clone(), + ProcessedToDeviceEvent::UnableToDecrypt { encrypted_event, .. } => { + encrypted_event.clone() + } ProcessedToDeviceEvent::PlainText(event) => event.clone(), ProcessedToDeviceEvent::Invalid(event) => event.clone(), } @@ -1219,7 +1251,7 @@ impl ProcessedToDeviceEvent { pub fn as_raw(&self) -> &Raw { match self { ProcessedToDeviceEvent::Decrypted { raw, .. } => raw, - ProcessedToDeviceEvent::UnableToDecrypt(event) => event, + ProcessedToDeviceEvent::UnableToDecrypt { encrypted_event, .. } => encrypted_event, ProcessedToDeviceEvent::PlainText(event) => event, ProcessedToDeviceEvent::Invalid(event) => event, } diff --git a/crates/matrix-sdk-crypto/CHANGELOG.md b/crates/matrix-sdk-crypto/CHANGELOG.md index dc4e24822a0..ff03de305e1 100644 --- a/crates/matrix-sdk-crypto/CHANGELOG.md +++ b/crates/matrix-sdk-crypto/CHANGELOG.md @@ -13,6 +13,19 @@ All notable changes to this project will be documented in this file. - [**breaking**] Add a new `VerificationLevel::MismatchedSender` to indicate that the sender of an event appears to have been tampered with. ([#5219](https://github.com/matrix-org/matrix-rust-sdk/pull/5219)) +- [**breaking**]: When in "exclude insecure devices" mode, refuse to decrypt + incoming to-device messages from unverified devices, except for some + exceptions for certain event types. To support this, a new variant has been + added to `ProcessedToDeviceEvent`: `UnverifiedSender`, which is returned from + `OlmMachine::receive_sync_changes` when we are excluding insecure devices and + the sender's device is not verified. Also, several methods now take a + `DecryptionSettings` argument to allow controlling the processing of to-device + events based on those settings. To recreate the previous behaviour pass in: + `DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }`. + Affected methods are `OlmMachine::receive_sync_changes`, + `RehydratedDevice::receive_events`, and several internal methods. + ([#5319](https://github.com/matrix-org/matrix-rust-sdk/pull/5319) + ### Refactor - [**breaking**] The `PendingChanges`, `Changes`, `StoredRoomKeyBundleData`, diff --git a/crates/matrix-sdk-crypto/src/error.rs b/crates/matrix-sdk-crypto/src/error.rs index 7d77144f75d..10e8ddf03ee 100644 --- a/crates/matrix-sdk-crypto/src/error.rs +++ b/crates/matrix-sdk-crypto/src/error.rs @@ -76,6 +76,14 @@ pub enum OlmError { /// Encryption failed due to an error collecting the recipient devices. #[error("encryption failed due to an error collecting the recipient devices: {0}")] SessionRecipientCollectionError(SessionRecipientCollectionError), + + /// Refused to decrypt because the sender was not verified or did not meet + /// the required VerificationLevel. + #[error( + "refusing to decrypt the event because the sender device was not \ + verified and 'exclude insecure devices' is enabled." + )] + UnverifiedSenderDevice, } /// Error representing a failure during a group encryption operation. diff --git a/crates/matrix-sdk-crypto/src/gossiping/machine.rs b/crates/matrix-sdk-crypto/src/gossiping/machine.rs index 9b70ec9e473..7f55b90ecea 100644 --- a/crates/matrix-sdk-crypto/src/gossiping/machine.rs +++ b/crates/matrix-sdk-crypto/src/gossiping/machine.rs @@ -1757,7 +1757,13 @@ mod tests { let res = tr .account() .await? - .decrypt_to_device_event(&alice_machine.inner.store, &event) + .decrypt_to_device_event( + &alice_machine.inner.store, + &event, + &DecryptionSettings { + sender_device_trust_requirement: TrustRequirement::Untrusted, + }, + ) .await?; Ok((tr, res)) }) @@ -1838,7 +1844,13 @@ mod tests { let res = tr .account() .await? - .decrypt_to_device_event(&alice_machine.inner.store, &event) + .decrypt_to_device_event( + &alice_machine.inner.store, + &event, + &DecryptionSettings { + sender_device_trust_requirement: TrustRequirement::Untrusted, + }, + ) .await?; Ok((tr, res)) }) @@ -2166,7 +2178,13 @@ mod tests { let res = tr .account() .await? - .decrypt_to_device_event(&alice_machine.inner.store, &event) + .decrypt_to_device_event( + &alice_machine.inner.store, + &event, + &DecryptionSettings { + sender_device_trust_requirement: TrustRequirement::Untrusted, + }, + ) .await?; Ok((tr, res)) }) diff --git a/crates/matrix-sdk-crypto/src/machine/mod.rs b/crates/matrix-sdk-crypto/src/machine/mod.rs index a13df5d743f..c18c26b9e05 100644 --- a/crates/matrix-sdk-crypto/src/machine/mod.rs +++ b/crates/matrix-sdk-crypto/src/machine/mod.rs @@ -24,8 +24,9 @@ use matrix_sdk_common::deserialized_responses::WithheldCode; use matrix_sdk_common::{ deserialized_responses::{ AlgorithmInfo, DecryptedRoomEvent, DeviceLinkProblem, EncryptionInfo, - ProcessedToDeviceEvent, UnableToDecryptInfo, UnableToDecryptReason, - UnsignedDecryptionResult, UnsignedEventLocation, VerificationLevel, VerificationState, + ProcessedToDeviceEvent, ToDeviceUnableToDecryptInfo, ToDeviceUnableToDecryptReason, + UnableToDecryptInfo, UnableToDecryptReason, UnsignedDecryptionResult, + UnsignedEventLocation, VerificationLevel, VerificationState, }, locks::RwLock as StdRwLock, BoxFuture, @@ -842,9 +843,17 @@ impl OlmMachine { /// Decrypt and handle a to-device event. /// - /// If decryption (or checking the sender device) fails, returns + /// If decryption (or checking the sender device) fails, returns an /// `Err(DecryptToDeviceError::OlmError)`. /// + /// If we are in strict "exclude insecure devices" mode and the sender + /// device is not verified, and the decrypted event type is not on the + /// allow list, returns `Err(DecryptToDeviceError::UnverifiedSender)` + /// + /// (The allow list of types that are processed even if the sender is + /// unverified is: `m.room_key`, `m.room_key.withheld`, + /// `m.room_key_request`, `m.secret.request` and `m.key.verification.*`.) + /// /// If the sender device is dehydrated, does no handling and immediately /// returns `Err(DecryptToDeviceError::FromDehydratedDevice)`. /// @@ -859,13 +868,16 @@ impl OlmMachine { transaction: &mut StoreTransaction, event: &EncryptedToDeviceEvent, changes: &mut Changes, - _decryption_settings: &DecryptionSettings, + decryption_settings: &DecryptionSettings, ) -> Result { // Decrypt the event - let mut decrypted = - transaction.account().await?.decrypt_to_device_event(&self.inner.store, event).await?; + let mut decrypted = transaction + .account() + .await? + .decrypt_to_device_event(&self.inner.store, event, decryption_settings) + .await?; - // Return early if the sending device is decrypted + // Return early if the sending device is a dehydrated device self.check_to_device_event_is_not_from_dehydrated_device(&decrypted, &event.sender).await?; // Device is not dehydrated: handle it as normal e.g. create a Megolm session @@ -1321,7 +1333,10 @@ impl OlmMachine { match event { // These are handled here because we accept them either plaintext or - // encrypted + // encrypted. + // + // Note: this list should match the allowed types in + // check_to_device_is_from_verified_device_or_allowed_type RoomKeyRequest(e) => self.inner.key_request_machine.receive_incoming_key_request(e), SecretRequest(e) => self.inner.key_request_machine.receive_incoming_secret_request(e), RoomKeyWithheld(e) => self.add_withheld_info(changes, e), @@ -1423,8 +1438,14 @@ impl OlmMachine { /// /// Return the same event, decrypted if possible. /// - /// If we can identify that this to-device event came from a dehydrated - /// device, this method does not process it, and returns `None`. + /// If we are in strict "exclude insecure devices" mode and the sender + /// device is not verified, and the decrypted event type is not on the + /// allow list, or if this event comes from a dehydrated device, this method + /// does not process it, and returns `None`. + /// + /// (The allow list of types that are processed even if the sender is + /// unverified is: `m.room_key`, `m.room_key.withheld`, + /// `m.room_key_request`, `m.secret.request` and `m.key.verification.*`.) async fn receive_encrypted_to_device_event( &self, transaction: &mut StoreTransaction, @@ -1439,6 +1460,12 @@ impl OlmMachine { { Ok(decrypted) => decrypted, Err(DecryptToDeviceError::OlmError(err)) => { + let reason = if let OlmError::UnverifiedSenderDevice = &err { + ToDeviceUnableToDecryptReason::UnverifiedSenderDevice + } else { + ToDeviceUnableToDecryptReason::DecryptionFailure + }; + if let OlmError::SessionWedged(sender, curve_key) = err { if let Err(e) = self.inner.session_manager.mark_device_as_wedged(&sender, curve_key).await @@ -1450,7 +1477,10 @@ impl OlmMachine { } } - return Some(ProcessedToDeviceEvent::UnableToDecrypt(raw_event)); + return Some(ProcessedToDeviceEvent::UnableToDecrypt { + encrypted_event: raw_event, + utd_info: ToDeviceUnableToDecryptInfo { reason }, + }); } Err(DecryptToDeviceError::FromDehydratedDevice) => return None, }; @@ -1591,6 +1621,15 @@ impl OlmMachine { /// If any of the to-device events in the supplied changes were sent from /// dehydrated devices, these are not processed, and are omitted from /// the returned list, as per MSC3814. + /// + /// If we are in strict "exclude insecure devices" mode and the sender + /// device of any event is not verified, and the decrypted event type is not + /// on the allow list, these events are not processed and are omitted from + /// the returned list. + /// + /// (The allow list of types that are processed even if the sender is + /// unverified is: `m.room_key`, `m.room_key.withheld`, + /// `m.room_key_request`, `m.secret.request` and `m.key.verification.*`.) pub(crate) async fn preprocess_sync_changes( &self, transaction: &mut StoreTransaction, @@ -2957,9 +2996,15 @@ fn megolm_error_to_utd_info( Ok(UnableToDecryptInfo { session_id, reason }) } -/// An error that can occur during [`OlmMachine::decrypt_to_device_event`] - -/// either because decryption failed, or because the sender device was a -/// dehydrated device, which should never send any to-device messages. +/// An error that can occur during [`OlmMachine::decrypt_to_device_event`]: +/// +/// * because decryption failed, or +/// +/// * because the sender device was not verified when we are in strict "exclude +/// insecure devices" mode, or +/// +/// * because the sender device was a dehydrated device, which should never send +/// any to-device messages. #[derive(Debug, thiserror::Error)] pub(crate) enum DecryptToDeviceError { #[error("An Olm error occurred meaning we failed to decrypt the event")] diff --git a/crates/matrix-sdk-crypto/src/machine/test_helpers.rs b/crates/matrix-sdk-crypto/src/machine/test_helpers.rs index 8b940f23c3c..4c5f542a2c3 100644 --- a/crates/matrix-sdk-crypto/src/machine/test_helpers.rs +++ b/crates/matrix-sdk-crypto/src/machine/test_helpers.rs @@ -194,7 +194,7 @@ pub async fn send_and_receive_encrypted_to_device_test_helper( sender.get_device(recipient.user_id(), recipient.device_id(), None).await.unwrap().unwrap(); let raw_encrypted = device - .encrypt_event_raw(event_type, &content) + .encrypt_event_raw(event_type, content) .await .expect("Should have encrypted the content"); @@ -220,7 +220,7 @@ pub async fn send_and_receive_encrypted_to_device_test_helper( }; let (decrypted, _) = - recipient.receive_sync_changes(sync_changes, &decryption_settings).await.unwrap(); + recipient.receive_sync_changes(sync_changes, decryption_settings).await.unwrap(); assert_eq!(1, decrypted.len()); decrypted[0].clone() diff --git a/crates/matrix-sdk-crypto/src/machine/tests/olm_encryption.rs b/crates/matrix-sdk-crypto/src/machine/tests/olm_encryption.rs index a24c70c5001..87e69161efe 100644 --- a/crates/matrix-sdk-crypto/src/machine/tests/olm_encryption.rs +++ b/crates/matrix-sdk-crypto/src/machine/tests/olm_encryption.rs @@ -18,7 +18,9 @@ use std::{ }; use assert_matches2::assert_let; -use matrix_sdk_common::deserialized_responses::ProcessedToDeviceEvent; +use matrix_sdk_common::deserialized_responses::{ + ProcessedToDeviceEvent, ToDeviceUnableToDecryptReason, +}; use matrix_sdk_test::async_test; use ruma::{ device_id, @@ -317,6 +319,8 @@ async fn test_decrypt_to_device_message_with_unsigned_sender_keys() { let event = to_device_events.first().expect("Bob did not get a to-device event").clone(); // The to-device event should remain encrypted. - assert_let!(ProcessedToDeviceEvent::UnableToDecrypt(event) = event); - assert_eq!(event.get_field("type").unwrap(), Some("m.room.encrypted")); + assert_let!(ProcessedToDeviceEvent::UnableToDecrypt { encrypted_event, utd_info } = event); + assert_eq!(encrypted_event.get_field("type").unwrap(), Some("m.room.encrypted")); + + assert_eq!(utd_info.reason, ToDeviceUnableToDecryptReason::DecryptionFailure); } diff --git a/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs b/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs index 53c036ab105..1a2d1ab7b3c 100644 --- a/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs +++ b/crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs @@ -15,10 +15,14 @@ use assert_matches2::{assert_let, assert_matches}; use insta::assert_json_snapshot; use matrix_sdk_common::deserialized_responses::{ - AlgorithmInfo, ProcessedToDeviceEvent, VerificationLevel, VerificationState, + AlgorithmInfo, ProcessedToDeviceEvent, ToDeviceUnableToDecryptReason, VerificationLevel, + VerificationState, +}; +use matrix_sdk_test::{async_test, ruma_response_from_json}; +use ruma::{ + events::AnyToDeviceEvent, room_id, serde::Raw, to_device::DeviceIdOrAllDevices, RoomId, + TransactionId, }; -use matrix_sdk_test::async_test; -use ruma::{events::AnyToDeviceEvent, serde::Raw, to_device::DeviceIdOrAllDevices}; use serde_json::{json, value::to_raw_value, Value}; use crate::{ @@ -29,14 +33,18 @@ use crate::{ }, tests::{self, decryption_verification_state::mark_alice_identity_as_verified_test_helper}, }, + olm::SenderData, types::{ - events::{ToDeviceCustomEvent, ToDeviceEvent}, + events::{ + room::encrypted::ToDeviceEncryptedEventContent, EventType as _, ToDeviceCustomEvent, + ToDeviceEvent, + }, requests::ToDeviceRequest, }, utilities::json_convert, verification::tests::bob_id, - DecryptionSettings, DeviceData, EncryptionSyncChanges, LocalTrust, OlmError, OlmMachine, - TrustRequirement, + CrossSigningBootstrapRequests, DecryptionSettings, DeviceData, EncryptionSettings, + EncryptionSyncChanges, LocalTrust, OlmError, OlmMachine, Session, TrustRequirement, }; #[async_test] @@ -108,7 +116,7 @@ async fn test_send_encrypted_to_device() { async fn test_receive_custom_encrypted_to_device_fails_if_device_unknown() { // When decrypting a custom to device, we expect the recipient to know the // sending device. If the device is not known decryption will fail (see - // `EventError(MissingSigningKey)`). The only exception is room keys were + // `EventError(MissingSigningKey)`). The only exception is room keys where // this check can be delayed. This is a reason why there is no test for // verification_state `DeviceLinkProblem::MissingDevice` @@ -140,7 +148,108 @@ async fn test_receive_custom_encrypted_to_device_fails_if_device_unknown() { ) .await; - assert_let!(ProcessedToDeviceEvent::UnableToDecrypt(_) = processed_event); + assert_let!(ProcessedToDeviceEvent::UnableToDecrypt { utd_info, .. } = processed_event); + assert_eq!(utd_info.reason, ToDeviceUnableToDecryptReason::DecryptionFailure); +} + +#[async_test] +async fn test_excluding_insecure_means_custom_to_device_events_from_unverified_devices_are_utd() { + // Given we are in "exclude insecure devices" mode + let decryption_settings = DecryptionSettings { + sender_device_trust_requirement: TrustRequirement::CrossSignedOrLegacy, + }; + + // Bob is the receiver + let (bob, otk) = get_prepared_machine_test_helper(bob_id(), false).await; + + // Alice is the sender + let alice = OlmMachine::new(tests::alice_id(), tests::alice_device_id()).await; + + let bob_device = DeviceData::from_machine_test_helper(&bob).await.unwrap(); + alice.store().save_device_data(&[bob_device]).await.unwrap(); + + let (alice, bob) = build_session_for_pair(alice, bob, otk).await; + + // And the receiving device does not consider the sending device verified + make_alice_unverified(&alice, &bob).await; + + assert!(!bob + .get_device(alice.user_id(), alice.device_id(), None) + .await + .unwrap() + .unwrap() + .is_verified()); + + // When we send a custom event + let custom_event_type = "m.new_device"; + + let custom_content = json!({ + "device_id": "XYZABCDE", + "rooms": ["!726s6s6q:example.com"] + }); + + let processed_event = send_and_receive_encrypted_to_device_test_helper( + &alice, + &bob, + custom_event_type, + &custom_content, + &decryption_settings, + ) + .await; + + // Then it was not processed because the sending device was not verified + assert_let!(ProcessedToDeviceEvent::UnableToDecrypt { utd_info, .. } = processed_event); + + // And the info provided in the UnableToDecrypt matches what we supplied + assert_eq!(utd_info.reason, ToDeviceUnableToDecryptReason::UnverifiedSenderDevice); +} + +#[async_test] +async fn test_excluding_insecure_does_not_prevent_key_events_being_processed() { + // Given we are in "exclude insecure devices" mode + let decryption_settings = DecryptionSettings { + sender_device_trust_requirement: TrustRequirement::CrossSignedOrLegacy, + }; + + // Bob is the receiver + let (bob, otk) = get_prepared_machine_test_helper(bob_id(), false).await; + + // Alice is the sender + let alice = OlmMachine::new(tests::alice_id(), tests::alice_device_id()).await; + + let bob_device = DeviceData::from_machine_test_helper(&bob).await.unwrap(); + alice.store().save_device_data(&[bob_device]).await.unwrap(); + + let (alice, bob) = build_session_for_pair(alice, bob, otk).await; + + // And the receiving device does not consider the sending device verified + make_alice_unverified(&alice, &bob).await; + + assert!(!bob + .get_device(alice.user_id(), alice.device_id(), None) + .await + .unwrap() + .unwrap() + .is_verified()); + + // When we send a room key event + let key_event = + create_and_share_session_without_sender_data(&alice, &bob, room_id!("!23:s.co")).await; + + let key_event_content = serde_json::to_value(&key_event.content).unwrap(); + + let processed_event = send_and_receive_encrypted_to_device_test_helper( + &alice, + &bob, + "m.room_key", + &key_event_content, + &decryption_settings, + ) + .await; + + // Then it was processed because even though the sending device was not + // verified, room key events are allowed through. + assert_matches!(processed_event, ProcessedToDeviceEvent::Decrypted { .. }); } #[async_test] @@ -461,7 +570,8 @@ async fn test_processed_to_device_variants() { }); let processed_event = &processed[3]; - assert_matches!(processed_event, ProcessedToDeviceEvent::UnableToDecrypt(_)); + assert_matches!(processed_event, ProcessedToDeviceEvent::UnableToDecrypt { utd_info, .. }); + assert_eq!(utd_info.reason, ToDeviceUnableToDecryptReason::DecryptionFailure); insta::with_settings!({ prepend_module_to_snapshot => false }, { assert_json_snapshot!( @@ -495,3 +605,98 @@ async fn test_send_encrypted_to_device_no_session() { assert_matches!(encryption_result, Err(OlmError::MissingSession)); } + +/// Create a new [`OutboundGroupSession`], and build a to-device event to share +/// it with another [`OlmMachine`], *without* sending the MSC4147 sender data. +/// +/// # Arguments +/// +/// * `alice` - sending device. +/// * `bob` - receiving device. +/// * `room_id` - room to create a session for. +async fn create_and_share_session_without_sender_data( + alice: &OlmMachine, + bob: &OlmMachine, + room_id: &RoomId, +) -> ToDeviceEvent { + let (outbound_session, _) = alice + .inner + .group_session_manager + .get_or_create_outbound_session( + room_id, + EncryptionSettings::default(), + SenderData::unknown(), + ) + .await + .unwrap(); + + // In future, we might want to save the session to the store, to better match + // the behaviour of the real implementation. See + // `GroupSessionManager::share_room_key` for inspiration on how to do that. + + let olm_sessions = alice + .store() + .get_sessions(&bob.identity_keys().curve25519.to_base64()) + .await + .unwrap() + .unwrap(); + let mut olm_session: Session = olm_sessions.lock().await[0].clone(); + + let room_key_content = outbound_session.as_content().await; + let plaintext = serde_json::to_string(&json!({ + "sender": alice.user_id(), + "sender_device": alice.device_id(), + "keys": { "ed25519": alice.identity_keys().ed25519.to_base64() }, + // We deliberately do *not* include: + // "org.matrix.msc4147.device_keys": alice_device_keys, + "recipient": bob.user_id(), + "recipient_keys": { "ed25519": bob.identity_keys().ed25519.to_base64() }, + "type": room_key_content.event_type(), + "content": room_key_content, + })) + .unwrap(); + + let ciphertext = olm_session.encrypt_helper(&plaintext).await; + ToDeviceEvent::new( + alice.user_id().to_owned(), + olm_session.build_encrypted_event(ciphertext, None).await.unwrap(), + ) +} + +/// Simulate uploading keys for alice that mean bob thinks alice's device +/// exists, but is unverified. +async fn make_alice_unverified(alice: &OlmMachine, bob: &OlmMachine) { + let CrossSigningBootstrapRequests { upload_signing_keys_req: upload_signing, .. } = + alice.bootstrap_cross_signing(false).await.expect("Expect Alice x-signing key request"); + + let device_keys = alice + .get_device(alice.user_id(), alice.device_id(), None) + .await + .unwrap() + .unwrap() + .as_device_keys() + .to_owned(); + + let updated_keys_with_x_signing = json!({ device_keys.device_id.to_string(): device_keys }); + + let json = json!({ + "device_keys": { + alice.user_id() : updated_keys_with_x_signing + }, + "failures": {}, + "master_keys": { + alice.user_id() : upload_signing.master_key.unwrap(), + }, + "user_signing_keys": { + alice.user_id() : upload_signing.user_signing_key.unwrap(), + }, + "self_signing_keys": { + alice.user_id() : upload_signing.self_signing_key.unwrap(), + }, + } + ); + + let kq_response = ruma_response_from_json(&json); + alice.receive_keys_query_response(&TransactionId::new(), &kq_response).await.unwrap(); + bob.receive_keys_query_response(&TransactionId::new(), &kq_response).await.unwrap(); +} diff --git a/crates/matrix-sdk-crypto/src/olm/account.rs b/crates/matrix-sdk-crypto/src/olm/account.rs index e4a50fee291..68387af31a4 100644 --- a/crates/matrix-sdk-crypto/src/olm/account.rs +++ b/crates/matrix-sdk-crypto/src/olm/account.rs @@ -84,7 +84,7 @@ use crate::{ requests::UploadSigningKeysRequest, CrossSigningKey, DeviceKeys, EventEncryptionAlgorithm, MasterPubkey, OneTimeKey, SignedKey, }, - Device, OlmError, SignatureError, + DecryptionSettings, Device, OlmError, SignatureError, TrustRequirement, }; #[derive(Debug)] @@ -1164,10 +1164,20 @@ impl Account { sender: &UserId, sender_key: Curve25519PublicKey, ciphertext: &OlmMessage, + decryption_settings: &DecryptionSettings, ) -> OlmResult { let message_hash = OlmMessageHash::new(sender_key, ciphertext); - match self.decrypt_and_parse_olm_message(store, sender, sender_key, ciphertext).await { + match self + .decrypt_and_parse_olm_message( + store, + sender, + sender_key, + ciphertext, + decryption_settings, + ) + .await + { Ok((session, result)) => { Ok(OlmDecryptionInfo { session, message_hash, result, inbound_group_session: None }) } @@ -1189,8 +1199,16 @@ impl Account { store: &Store, sender: &UserId, content: &OlmV2Curve25519AesSha2Content, + decryption_settings: &DecryptionSettings, ) -> OlmResult { - self.decrypt_olm_helper(store, sender, content.sender_key, &content.ciphertext).await + self.decrypt_olm_helper( + store, + sender, + content.sender_key, + &content.ciphertext, + decryption_settings, + ) + .await } #[instrument(skip_all, fields(sender, sender_key = ?content.sender_key))] @@ -1199,6 +1217,7 @@ impl Account { store: &Store, sender: &UserId, content: &OlmV1Curve25519AesSha2Content, + decryption_settings: &DecryptionSettings, ) -> OlmResult { if content.recipient_key != self.static_data.identity_keys.curve25519 { warn!("Olm event doesn't contain a ciphertext for our key"); @@ -1210,6 +1229,7 @@ impl Account { sender, content.sender_key, &content.ciphertext, + decryption_settings, )) .await } @@ -1220,16 +1240,17 @@ impl Account { &mut self, store: &Store, event: &EncryptedToDeviceEvent, + decryption_settings: &DecryptionSettings, ) -> OlmResult { trace!("Decrypting a to-device event"); match &event.content { ToDeviceEncryptedEventContent::OlmV1Curve25519AesSha2(c) => { - self.decrypt_olm_v1(store, &event.sender, c).await + self.decrypt_olm_v1(store, &event.sender, c, decryption_settings).await } #[cfg(feature = "experimental-algorithms")] ToDeviceEncryptedEventContent::OlmV2Curve25519AesSha2(c) => { - self.decrypt_olm_v2(store, &event.sender, c).await + self.decrypt_olm_v2(store, &event.sender, c, decryption_settings).await } ToDeviceEncryptedEventContent::Unknown(_) => { warn!( @@ -1391,13 +1412,23 @@ impl Account { sender: &UserId, sender_key: Curve25519PublicKey, message: &OlmMessage, + decryption_settings: &DecryptionSettings, ) -> OlmResult<(SessionType, DecryptionResult)> { let (session, plaintext) = self.decrypt_olm_message(store, sender, sender_key, message).await?; trace!("Successfully decrypted an Olm message"); - match self.parse_decrypted_to_device_event(store, sender, sender_key, plaintext).await { + match self + .parse_decrypted_to_device_event( + store, + sender, + sender_key, + plaintext, + decryption_settings, + ) + .await + { Ok(result) => Ok((session, result)), Err(e) => { // We might have created a new session but decryption might still @@ -1446,6 +1477,7 @@ impl Account { sender: &UserId, sender_key: Curve25519PublicKey, plaintext: String, + decryption_settings: &DecryptionSettings, ) -> OlmResult { let event: Box = serde_json::from_str(&plaintext)?; let identity_keys = &self.static_data.identity_keys; @@ -1516,12 +1548,78 @@ impl Account { let encryption_info = Self::get_olm_encryption_info(sender_key, sender, &sender_device); - Ok(DecryptionResult { + let result = DecryptionResult { event, raw_event: Raw::from_json(RawJsonValue::from_string(plaintext)?), sender_key, encryption_info, - }) + }; + + // Return an error if the sender is unverified (and we care) + if !self.is_from_verified_device_or_allowed_type(decryption_settings, &result) { + Err(OlmError::UnverifiedSenderDevice) + } else { + // Sender is ok - return the decrypted event + Ok(result) + } + } + } + + /// Return true if: + /// + /// * the sending device is verified, or + /// * the event type is one of those we allow to be sent from unverified + /// devices, or + /// * we are not in "exclude_insecure_devices" mode, so everything is + /// allowed. + /// + /// Return false if: + /// + /// * we are in "exclude_insecure_devices" mode AND the sending device is + /// unverified. + fn is_from_verified_device_or_allowed_type( + &self, + decryption_settings: &DecryptionSettings, + result: &DecryptionResult, + ) -> bool { + let event_type = result.event.event_type(); + + // If we're in "exclude insecure devices" mode, we prevent most + // to-device events with unverified senders from being allowed + // through here, but there are some exceptions: + // + // * m.room_key - we hold on to these until later, so if the sender becomes + // verified later we can still use the key. + // + // * m.room_key_request, m.room_key.withheld, m.key.verification.*, + // m.secret.request - these are allowed as plaintext events, so we also allow + // them encrypted from insecure devices. Note: the list of allowed types here + // should match with what is allowed in handle_to_device_event. + match event_type { + "m.room_key" + | "m.room_key.withheld" + | "m.room_key_request" + | "m.secret.request" + | "m.key.verification.key" + | "m.key.verification.mac" + | "m.key.verification.done" + | "m.key.verification.ready" + | "m.key.verification.start" + | "m.key.verification.accept" + | "m.key.verification.cancel" + | "m.key.verification.request" => { + // This is one of the exception types - we allow it even if the sender device is + // not verified. + true + } + _ => { + // This is not an exception type - check for "exclude insecure devices" mode, + // and whether the sender is verified. + satisfies_sender_trust_requirement( + &result.encryption_info, + &decryption_settings.sender_device_trust_requirement, + ) + } } } @@ -1699,6 +1797,43 @@ fn expand_legacy_pickle_key(key: &[u8; 32], device_id: &DeviceId) -> Box<[u8; 32 key } +/// Does the to-device event satisfy the sender trust requirement from the +/// decryption settings? +fn satisfies_sender_trust_requirement( + encryption_info: &EncryptionInfo, + trust_requirement: &TrustRequirement, +) -> bool { + trace!( + verification_state = ?encryption_info.verification_state, + ?trust_requirement, "check_to_device_sender_trust_requirement", + ); + + match (&encryption_info.verification_state, trust_requirement) { + // If we don't care, everything is OK. + (_, TrustRequirement::Untrusted) => true, + + // Verified is OK whatever our requirements are. + (VerificationState::Verified, _) => true, + + // We do care, and we are not fully verified: check more deeply. + // (Note that for to-device messages the legacy trust requirement is not relevant.) + ( + VerificationState::Unverified(verification_level), + TrustRequirement::CrossSignedOrLegacy | TrustRequirement::CrossSigned, + ) => match verification_level { + // The device is signed but the identity is only pinned - this is fine. + VerificationLevel::UnverifiedIdentity => true, + + // The device is unsigned or missing, or the user is in verification violation, + // or the sender is mismatched: this is not fine. + VerificationLevel::UnsignedDevice + | VerificationLevel::None(_) + | VerificationLevel::VerificationViolation + | VerificationLevel::MismatchedSender => false, + }, + } +} + #[cfg(test)] mod tests { use std::{