From c45ede972ef12c2c45b37059a4d98f02d4988f85 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 8 Oct 2025 23:36:35 +0100 Subject: [PATCH 1/7] crypto: factor out `Account::get_event_sender_device` `Account::parse_decrypted_to_device_event` is getting a bit big and unwieldy, so factor out the bit that attempts to find the sending device. (Also, remove an outdated TODO.) --- crates/matrix-sdk-crypto/src/olm/account.rs | 100 +++++++++++--------- 1 file changed, 54 insertions(+), 46 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/olm/account.rs b/crates/matrix-sdk-crypto/src/olm/account.rs index 70e1a8ba98d..7969f0a4b08 100644 --- a/crates/matrix-sdk-crypto/src/olm/account.rs +++ b/crates/matrix-sdk-crypto/src/olm/account.rs @@ -1519,52 +1519,7 @@ impl Account { ) .into()) } else { - // If the event contained sender_device_keys, check them now. - // WARN: If you move or modify this check, ensure that the code below is still - // valid. The processing of the historic room key bundle depends on this being - // here. - Self::check_sender_device_keys(event.as_ref(), sender_key)?; - let mut sender_device: Option = None; - if let AnyDecryptedOlmEvent::RoomKey(_) = event.as_ref() { - // If this event is an `m.room_key` event, defer the check for - // the Ed25519 key of the sender until we decrypt room events. - // This ensures that we receive the room key even if we don't - // have access to the device. - } else if let AnyDecryptedOlmEvent::RoomKeyBundle(_) = event.as_ref() { - // If this is a room key bundle we're requiring the device keys to be part of - // the `AnyDecryptedOlmEvent`. This ensures that we can skip the check for the - // Ed25519 key below since `Self::check_sender_device_keys` already did so. - // - // If the event didn't contain any sender device keys we'll throw an error - // refusing to decrypt the room key bundle. - event.sender_device_keys().ok_or(EventError::MissingSigningKey).inspect_err( - |_| { - warn!("The room key bundle was missing the sender device keys in the event") - }, - )?; - } else { - let device = store - .get_device_from_curve_key(event.sender(), sender_key) - .await? - .ok_or(EventError::MissingSigningKey)?; - - let key = device.ed25519_key().ok_or(EventError::MissingSigningKey)?; - - if key != event.keys().ed25519 { - return Err(EventError::MismatchedKeys( - key.into(), - event.keys().ed25519.into(), - ) - .into()); - } - - // TODO: we should have access to some decryption settings here - // (TrustRequirement) and use it to manually reject the decryption. - // Similar to check_sender_trust_requirement for room events - - sender_device = Some(device); - } - + let sender_device = Self::get_event_sender_device(store, sender_key, &event).await?; let encryption_info = Self::get_olm_encryption_info(sender_key, sender, &sender_device); let result = DecryptionResult { @@ -1584,6 +1539,59 @@ impl Account { } } + /// Look up the [`Device`] that sent us a successfully-decrypted event. + /// + /// Also validates the `sender_device_keys` field, if present. + /// + /// `m.room_key` events are special-cased and return `None`: we look up + /// their devices later on. + /// + /// For other events, we look up the device in the store, and return the + /// details. + async fn get_event_sender_device( + store: &Store, + sender_key: Curve25519PublicKey, + event: &AnyDecryptedOlmEvent, + ) -> OlmResult> { + // If the event contained sender_device_keys, check them now. + // WARN: If you move or modify this check, ensure that the code below is still + // valid. The processing of the historic room key bundle depends on this being + // here. + Self::check_sender_device_keys(event, sender_key)?; + let mut sender_device: Option = None; + if let AnyDecryptedOlmEvent::RoomKey(_) = event { + // If this event is an `m.room_key` event, defer the check for + // the Ed25519 key of the sender until we decrypt room events. + // This ensures that we receive the room key even if we don't + // have access to the device. + } else if let AnyDecryptedOlmEvent::RoomKeyBundle(_) = event { + // If this is a room key bundle we're requiring the device keys to be part of + // the `AnyDecryptedOlmEvent`. This ensures that we can skip the check for the + // Ed25519 key below since `Self::check_sender_device_keys` already did so. + // + // If the event didn't contain any sender device keys we'll throw an error + // refusing to decrypt the room key bundle. + event.sender_device_keys().ok_or(EventError::MissingSigningKey).inspect_err(|_| { + warn!("The room key bundle was missing the sender device keys in the event") + })?; + } else { + let device = store + .get_device_from_curve_key(event.sender(), sender_key) + .await? + .ok_or(EventError::MissingSigningKey)?; + + let key = device.ed25519_key().ok_or(EventError::MissingSigningKey)?; + + if key != event.keys().ed25519 { + return Err( + EventError::MismatchedKeys(key.into(), event.keys().ed25519.into()).into() + ); + } + sender_device = Some(device); + } + Ok(sender_device) + } + /// Return true if: /// /// * the sending device is verified, or From 43e94bcfb4fa36f2c1792354526cccdc9bd38ecb Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 9 Oct 2025 00:52:05 +0100 Subject: [PATCH 2/7] crypto: look up sender device for key bundles Currently, when we receive a room key bundle to-device event, we don't look up the sender device at all, meaning that the message is then marked as "from missing device", which means that if you turn on "exclude insecure devices", the message is dropped. This patch changes the logic so that room key bundle to-device events are treated the same way as most other to-device events (except room keys, which continue to be special). Fixes: https://github.com/matrix-org/matrix-rust-sdk/issues/5613, although the integration test now fails because instead we hit https://github.com/matrix-org/matrix-rust-sdk/issues/5768. --- crates/matrix-sdk-crypto/CHANGELOG.md | 2 ++ crates/matrix-sdk-crypto/src/olm/account.rs | 38 +++++++++------------ 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/crates/matrix-sdk-crypto/CHANGELOG.md b/crates/matrix-sdk-crypto/CHANGELOG.md index a56e84a9e62..4b51a9cf05e 100644 --- a/crates/matrix-sdk-crypto/CHANGELOG.md +++ b/crates/matrix-sdk-crypto/CHANGELOG.md @@ -8,6 +8,8 @@ All notable changes to this project will be documented in this file. ### Bug Fixes +- Fix a bug which caused history shared on invite to be ignored when "exclude insecure devices" was enabled. + ([#5763](https://github.com/matrix-org/matrix-rust-sdk/pull/5763)) - Fix a bug introduced in 0.14.0 which meant that the serialization of the value returned by `OtherUserIdentity::verification_request_content` did not include a `msgtype` field. ([#5642](https://github.com/matrix-org/matrix-rust-sdk/pull/5642)) diff --git a/crates/matrix-sdk-crypto/src/olm/account.rs b/crates/matrix-sdk-crypto/src/olm/account.rs index 7969f0a4b08..67f939d5b70 100644 --- a/crates/matrix-sdk-crypto/src/olm/account.rs +++ b/crates/matrix-sdk-crypto/src/olm/account.rs @@ -1558,38 +1558,34 @@ impl Account { // valid. The processing of the historic room key bundle depends on this being // here. Self::check_sender_device_keys(event, sender_key)?; - let mut sender_device: Option = None; if let AnyDecryptedOlmEvent::RoomKey(_) = event { // If this event is an `m.room_key` event, defer the check for // the Ed25519 key of the sender until we decrypt room events. // This ensures that we receive the room key even if we don't // have access to the device. - } else if let AnyDecryptedOlmEvent::RoomKeyBundle(_) = event { - // If this is a room key bundle we're requiring the device keys to be part of - // the `AnyDecryptedOlmEvent`. This ensures that we can skip the check for the - // Ed25519 key below since `Self::check_sender_device_keys` already did so. - // - // If the event didn't contain any sender device keys we'll throw an error - // refusing to decrypt the room key bundle. + return Ok(None); + } + + // MSC4268 requires room key bundle events to have a `sender_device_keys` field. + // Enforce that now. + if let AnyDecryptedOlmEvent::RoomKeyBundle(_) = event { event.sender_device_keys().ok_or(EventError::MissingSigningKey).inspect_err(|_| { warn!("The room key bundle was missing the sender device keys in the event") })?; - } else { - let device = store - .get_device_from_curve_key(event.sender(), sender_key) - .await? - .ok_or(EventError::MissingSigningKey)?; + } - let key = device.ed25519_key().ok_or(EventError::MissingSigningKey)?; + let device = store + .get_device_from_curve_key(event.sender(), sender_key) + .await? + .ok_or(EventError::MissingSigningKey)?; - if key != event.keys().ed25519 { - return Err( - EventError::MismatchedKeys(key.into(), event.keys().ed25519.into()).into() - ); - } - sender_device = Some(device); + let key = device.ed25519_key().ok_or(EventError::MissingSigningKey)?; + + if key != event.keys().ed25519 { + return Err(EventError::MismatchedKeys(key.into(), event.keys().ed25519.into()).into()); } - Ok(sender_device) + + Ok(Some(device)) } /// Return true if: From 8b6572bb23c78ee339ad48dcf6679a7922acec54 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 8 Oct 2025 22:40:38 +0100 Subject: [PATCH 3/7] test(crypto): Factor out test helper for encrypting to-device content I'm going to need to suppress `sender_device_keys` for more tests, so pull out a test helper to help with this. --- .../src/machine/test_helpers.rs | 60 +++++++++++++++++-- .../machine/tests/send_encrypted_to_device.rs | 42 +++++-------- 2 files changed, 72 insertions(+), 30 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/machine/test_helpers.rs b/crates/matrix-sdk-crypto/src/machine/test_helpers.rs index 97c3ecf40f2..fe8458b47a1 100644 --- a/crates/matrix-sdk-crypto/src/machine/test_helpers.rs +++ b/crates/matrix-sdk-crypto/src/machine/test_helpers.rs @@ -33,23 +33,23 @@ use ruma::{ to_device::DeviceIdOrAllDevices, user_id, DeviceId, OwnedOneTimeKeyId, TransactionId, UserId, }; +use serde::Serialize; use serde_json::{json, Value}; use tokio::sync::Mutex; use crate::{ machine::tests, olm::PrivateCrossSigningIdentity, - session_manager::CollectStrategy, store::{types::Changes, CryptoStoreWrapper, MemoryStore}, types::{ - events::ToDeviceEvent, + events::{room::encrypted::ToDeviceEncryptedEventContent, ToDeviceEvent}, requests::{AnyOutgoingRequest, ToDeviceRequest}, DeviceKeys, }, utilities::json_convert, verification::VerificationMachine, - Account, CrossSigningBootstrapRequests, DecryptionSettings, Device, DeviceData, - EncryptionSyncChanges, OlmMachine, OtherUserIdentityData, TrustRequirement, + Account, CollectStrategy, CrossSigningBootstrapRequests, DecryptionSettings, Device, + DeviceData, EncryptionSyncChanges, OlmMachine, OtherUserIdentityData, TrustRequirement, }; /// These keys need to be periodically uploaded to the server. @@ -227,6 +227,58 @@ pub async fn send_and_receive_encrypted_to_device_test_helper( decrypted[0].clone() } +/// Encrypt the given event content into the content of an +/// olm-encrypted to-device event, suppressing the `sender_device_keys` field in +/// the encrypted content. +/// +/// This is much the same as calling [`Device::encrypt`] on the recipient +/// device, other than the suppression of `sender_device_keys`. +/// +/// # Arguments +/// +/// * `sender` - The OlmMachine to use to encrypt the event. +/// * `recipient` - The recipient of the encrypted event. +/// * `event_type` - The type of the event to encrypt. +/// * `content` - The content of the event to encrypt. +pub async fn build_encrypted_to_device_content_without_sender_data( + sender: &OlmMachine, + recipient_device: &DeviceKeys, + event_type: &str, + content: &impl Serialize, +) -> ToDeviceEncryptedEventContent { + let sender_store = &sender.inner.store; + + let sender_key = recipient_device.curve25519_key().unwrap(); + let sessions = sender_store + .get_sessions(&sender_key.to_base64()) + .await + .expect("Could not get most recent session") + .expect("No olm session found"); + let mut olm_session = sessions.lock().await.first().unwrap().clone(); + + let plaintext = serde_json::to_string(&json!({ + "sender": sender.user_id(), + "sender_device": sender.device_id(), + "keys": { "ed25519": sender.identity_keys().ed25519.to_base64() }, + "recipient": recipient_device.user_id, + "recipient_keys": { "ed25519": recipient_device.ed25519_key().unwrap().to_base64() }, + "type": event_type, + "content": content, + })) + .unwrap(); + + let ciphertext = olm_session.encrypt_helper(&plaintext).await; + let content = + olm_session.build_encrypted_event(ciphertext, None).await.expect("could not encrypt"); + + sender_store + .save_changes(Changes { sessions: vec![olm_session], ..Default::default() }) + .await + .expect("Could not save session"); + + content +} + /// Create a session for the two supplied Olm machines to communicate. pub async fn build_session_for_pair( alice: OlmMachine, 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 25ef2f13c11..416dff11fd2 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 @@ -28,8 +28,9 @@ use serde_json::{json, value::to_raw_value, Value}; use crate::{ machine::{ test_helpers::{ - 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, + build_encrypted_to_device_content_without_sender_data, 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::{self, decryption_verification_state::mark_alice_identity_as_verified_test_helper}, }, @@ -45,7 +46,7 @@ use crate::{ utilities::json_convert, verification::tests::bob_id, CrossSigningBootstrapRequests, DecryptionSettings, DeviceData, EncryptionSettings, - EncryptionSyncChanges, LocalTrust, OlmError, OlmMachine, Session, TrustRequirement, + EncryptionSyncChanges, LocalTrust, OlmError, OlmMachine, TrustRequirement, }; #[async_test] @@ -635,33 +636,22 @@ async fn create_and_share_session_without_sender_data( // 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()) + let bob_device = alice + .get_device(bob.user_id(), bob.device_id(), None) .await .unwrap() - .unwrap(); - let mut olm_session: Session = olm_sessions.lock().await[0].clone(); - + .expect("Attempt to send message to unknown device"); 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(), + + let content = build_encrypted_to_device_content_without_sender_data( + alice, + &bob_device.device_keys, + room_key_content.event_type(), + &room_key_content, ) + .await; + + ToDeviceEvent::new(alice.user_id().to_owned(), content) } /// Simulate uploading keys for alice that mean bob thinks alice's device From e7fa8a429a298e1f2510724ba49c1322347cd000 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 8 Oct 2025 22:54:54 +0100 Subject: [PATCH 4/7] test(crypto): simplify `send_and_receive_encrypted_to_device_test_helper` No need to convert the event content to a to-device request, and then convert back again. --- .../matrix-sdk-crypto/src/machine/test_helpers.rs | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/machine/test_helpers.rs b/crates/matrix-sdk-crypto/src/machine/test_helpers.rs index fe8458b47a1..2e18efc3c06 100644 --- a/crates/matrix-sdk-crypto/src/machine/test_helpers.rs +++ b/crates/matrix-sdk-crypto/src/machine/test_helpers.rs @@ -30,7 +30,6 @@ use ruma::{ encryption::OneTimeKey, events::dummy::ToDeviceDummyEventContent, serde::Raw, - to_device::DeviceIdOrAllDevices, user_id, DeviceId, OwnedOneTimeKeyId, TransactionId, UserId, }; use serde::Serialize; @@ -38,12 +37,11 @@ use serde_json::{json, Value}; use tokio::sync::Mutex; use crate::{ - machine::tests, olm::PrivateCrossSigningIdentity, store::{types::Changes, CryptoStoreWrapper, MemoryStore}, types::{ events::{room::encrypted::ToDeviceEncryptedEventContent, ToDeviceEvent}, - requests::{AnyOutgoingRequest, ToDeviceRequest}, + requests::AnyOutgoingRequest, DeviceKeys, }, utilities::json_convert, @@ -199,16 +197,7 @@ pub async fn send_and_receive_encrypted_to_device_test_helper( .await .expect("Should have encrypted the content"); - let request = ToDeviceRequest::new( - recipient.user_id(), - DeviceIdOrAllDevices::DeviceId(recipient.device_id().to_owned()), - "m.room.encrypted", - raw_encrypted.cast(), - ); - let event = ToDeviceEvent::new( - sender.user_id().to_owned(), - tests::to_device_requests_to_content(vec![request.clone().into()]), - ); + let event = ToDeviceEvent::new(sender.user_id().to_owned(), raw_encrypted); let event = json_convert(&event).unwrap(); From 35f354be41006e1b9891fa1289e11ea862cebd10 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 9 Oct 2025 18:20:52 +0100 Subject: [PATCH 5/7] crypto: fall back to `sender_device_keys` for encrypted to-device messages When receiving an encrypted to-device message, if the sender device is not in the store, but the event includes `sender_device_keys`, use `sender_device_keys` to do the verification checks etc. Fixes: https://github.com/matrix-org/matrix-rust-sdk/issues/5768 --- crates/matrix-sdk-crypto/CHANGELOG.md | 2 + .../src/machine/test_helpers.rs | 17 ++++- .../machine/tests/send_encrypted_to_device.rs | 30 ++++++--- crates/matrix-sdk-crypto/src/olm/account.rs | 63 +++++++++++++------ 4 files changed, 84 insertions(+), 28 deletions(-) diff --git a/crates/matrix-sdk-crypto/CHANGELOG.md b/crates/matrix-sdk-crypto/CHANGELOG.md index 4b51a9cf05e..10039c7d496 100644 --- a/crates/matrix-sdk-crypto/CHANGELOG.md +++ b/crates/matrix-sdk-crypto/CHANGELOG.md @@ -8,6 +8,8 @@ All notable changes to this project will be documented in this file. ### Bug Fixes +- Fix a bug which caused encrypted to-device messages from unknown devices to be ignored. + ([#5763](https://github.com/matrix-org/matrix-rust-sdk/pull/5763)) - Fix a bug which caused history shared on invite to be ignored when "exclude insecure devices" was enabled. ([#5763](https://github.com/matrix-org/matrix-rust-sdk/pull/5763)) - Fix a bug introduced in 0.14.0 which meant that the serialization of the value returned by `OtherUserIdentity::verification_request_content` did not include a `msgtype` field. diff --git a/crates/matrix-sdk-crypto/src/machine/test_helpers.rs b/crates/matrix-sdk-crypto/src/machine/test_helpers.rs index 2e18efc3c06..0fc0cabc2b0 100644 --- a/crates/matrix-sdk-crypto/src/machine/test_helpers.rs +++ b/crates/matrix-sdk-crypto/src/machine/test_helpers.rs @@ -197,7 +197,22 @@ pub async fn send_and_receive_encrypted_to_device_test_helper( .await .expect("Should have encrypted the content"); - let event = ToDeviceEvent::new(sender.user_id().to_owned(), raw_encrypted); + receive_encrypted_to_device_test_helper( + sender.user_id(), + recipient, + decryption_settings, + raw_encrypted, + ) + .await +} + +pub async fn receive_encrypted_to_device_test_helper( + sender: &UserId, + recipient: &OlmMachine, + decryption_settings: &DecryptionSettings, + raw_encrypted: Raw, +) -> ProcessedToDeviceEvent { + let event = ToDeviceEvent::new(sender.to_owned(), raw_encrypted); let event = json_convert(&event).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 416dff11fd2..66dce2c144d 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 @@ -30,6 +30,7 @@ use crate::{ test_helpers::{ build_encrypted_to_device_content_without_sender_data, build_session_for_pair, get_machine_pair, get_machine_pair_with_session, get_prepared_machine_test_helper, + receive_encrypted_to_device_test_helper, send_and_receive_encrypted_to_device_test_helper, }, tests::{self, decryption_verification_state::mark_alice_identity_as_verified_test_helper}, @@ -49,6 +50,8 @@ use crate::{ EncryptionSyncChanges, LocalTrust, OlmError, OlmMachine, TrustRequirement, }; +/// Happy path test: encrypt a to-device message, and check it is successfully +/// decrypted by the recipient, and that all the metadata is set as expected. #[async_test] async fn test_send_encrypted_to_device() { let (alice, bob) = @@ -114,14 +117,13 @@ async fn test_send_encrypted_to_device() { ); } -#[async_test] -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 where - // this check can be delayed. This is a reason why there is no test for - // verification_state `DeviceLinkProblem::MissingDevice` +/// If the sender device is genuinely unknown (it is not in the store, nor does +/// the to-device message contain `sender_device_keys`), decryption will fail, +/// with `EventError::MissingSigningKey`. +#[async_test] +async fn test_receive_custom_encrypted_to_device_with_no_sender_device_keys_fails_if_device_unknown( +) { let (bob, otk) = get_prepared_machine_test_helper(bob_id(), false).await; let alice = OlmMachine::new(tests::alice_id(), tests::alice_device_id()).await; @@ -141,12 +143,22 @@ async fn test_receive_custom_encrypted_to_device_fails_if_device_unknown() { let decryption_settings = DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; - let processed_event = send_and_receive_encrypted_to_device_test_helper( + // We need to suppress the sender_data field to correctly emulate an unknown + // device + let bob_device = alice.get_device(bob.user_id(), bob.device_id(), None).await.unwrap().unwrap(); + let raw_encrypted = build_encrypted_to_device_content_without_sender_data( &alice, - &bob, + &bob_device.device_keys, custom_event_type, &custom_content, + ) + .await; + + let processed_event = receive_encrypted_to_device_test_helper( + alice.user_id(), + &bob, &decryption_settings, + Raw::new(&raw_encrypted).unwrap(), ) .await; diff --git a/crates/matrix-sdk-crypto/src/olm/account.rs b/crates/matrix-sdk-crypto/src/olm/account.rs index 67f939d5b70..38b0aee25f0 100644 --- a/crates/matrix-sdk-crypto/src/olm/account.rs +++ b/crates/matrix-sdk-crypto/src/olm/account.rs @@ -1541,13 +1541,17 @@ impl Account { /// Look up the [`Device`] that sent us a successfully-decrypted event. /// - /// Also validates the `sender_device_keys` field, if present. + /// We first look for the sender device in our store; if it is found then we + /// return that (having checked that the keys match). If the device is + /// not found in the store, we return the details + /// from `sender_device_keys`, if present. If the device is not in the + /// store, and the event lacks `sender_device_keys`, an error is returned. + /// + /// Also validates the `sender_device_keys` field, if present, regardless of + /// whether it is used. /// /// `m.room_key` events are special-cased and return `None`: we look up /// their devices later on. - /// - /// For other events, we look up the device in the store, and return the - /// details. async fn get_event_sender_device( store: &Store, sender_key: Curve25519PublicKey, @@ -1557,7 +1561,7 @@ impl Account { // WARN: If you move or modify this check, ensure that the code below is still // valid. The processing of the historic room key bundle depends on this being // here. - Self::check_sender_device_keys(event, sender_key)?; + let sender_device_keys = Self::check_sender_device_keys(event, sender_key)?; if let AnyDecryptedOlmEvent::RoomKey(_) = event { // If this event is an `m.room_key` event, defer the check for // the Ed25519 key of the sender until we decrypt room events. @@ -1569,23 +1573,41 @@ impl Account { // MSC4268 requires room key bundle events to have a `sender_device_keys` field. // Enforce that now. if let AnyDecryptedOlmEvent::RoomKeyBundle(_) = event { - event.sender_device_keys().ok_or(EventError::MissingSigningKey).inspect_err(|_| { + sender_device_keys.ok_or(EventError::MissingSigningKey).inspect_err(|_| { warn!("The room key bundle was missing the sender device keys in the event") })?; } - let device = store - .get_device_from_curve_key(event.sender(), sender_key) - .await? - .ok_or(EventError::MissingSigningKey)?; + // For event types other than `m.room_key`, we need to look up the device in the + // database irrespective of whether the `sender_device_keys` field is + // present in the event, because it may have been marked as "locally + // trusted" in the database. + let store_device = store.get_device_from_curve_key(event.sender(), sender_key).await?; + + match (store_device, sender_device_keys) { + // If the device is in the database, it had better have an Ed25519 key which + // matches that in the event. + (Some(device), _) => { + let key = device.ed25519_key().ok_or(EventError::MissingSigningKey)?; + if key != event.keys().ed25519 { + return Err(EventError::MismatchedKeys( + key.into(), + event.keys().ed25519.into(), + ) + .into()); + } + Ok(Some(device)) + } - let key = device.ed25519_key().ok_or(EventError::MissingSigningKey)?; + (None, Some(sender_device_keys)) => { + // We have already validated the signature on `sender_device_keys`, so this + // try_into cannot fail. + let sender_device_data = sender_device_keys.try_into().unwrap(); + Ok(Some(store.wrap_device_data(sender_device_data).await?)) + } - if key != event.keys().ed25519 { - return Err(EventError::MismatchedKeys(key.into(), event.keys().ed25519.into()).into()); + (None, None) => Err(OlmError::EventError(EventError::MissingSigningKey)), } - - Ok(Some(device)) } /// Return true if: @@ -1733,13 +1755,18 @@ impl Account { /// * `sender_key` - The Curve25519 key that the sender used to establish /// the Olm session that was used to decrypt the event. /// + /// # Returns + /// + /// A reference to the `sender_device_keys` in the event, if it exists and + /// is valid. + /// /// [MSC4147]: https://github.com/matrix-org/matrix-spec-proposals/pull/4147 fn check_sender_device_keys( event: &AnyDecryptedOlmEvent, sender_key: Curve25519PublicKey, - ) -> OlmResult<()> { + ) -> OlmResult> { let Some(sender_device_keys) = event.sender_device_keys() else { - return Ok(()); + return Ok(None); }; // Check the signature within the device_keys structure @@ -1774,7 +1801,7 @@ impl Account { return Err(OlmError::EventError(EventError::InvalidSenderDeviceKeys)); } - Ok(()) + Ok(Some(sender_device_keys)) } /// Internal use only. From c52fee77066dff01423b623c5b7cb818ae768f8e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 9 Oct 2025 18:26:05 +0100 Subject: [PATCH 6/7] test(crypto): add regression test for #5768 --- .../machine/tests/send_encrypted_to_device.rs | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) 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 66dce2c144d..48a0df8bcfe 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 @@ -117,6 +117,44 @@ async fn test_send_encrypted_to_device() { ); } +/// Test what happens when the sending device is deleted before the to-device +/// event arrives. (It should still be successfully decrypted.) +/// +/// Regression test for https://github.com/matrix-org/matrix-rust-sdk/issues/5768. +#[async_test] +async fn test_encrypted_to_device_from_deleted_device() { + let (alice, bob) = + get_machine_pair_with_session(tests::alice_id(), tests::user_id(), false).await; + + // Tell Bob that Alice's device has been deleted + let mut keys_query_response = ruma::api::client::keys::get_keys::v3::Response::default(); + keys_query_response.device_keys.insert(alice.user_id().to_owned(), Default::default()); + bob.receive_keys_query_response(&TransactionId::new(), &keys_query_response).await.unwrap(); + + let custom_event_type = "m.new_device"; + let custom_content = json!({"a": "b"}); + + 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; + + assert_let!(ProcessedToDeviceEvent::Decrypted { raw, encryption_info } = processed_event); + + let decrypted_event = raw.deserialize().unwrap(); + assert_eq!(decrypted_event.event_type().to_string(), custom_event_type.to_owned()); + + assert_eq!(encryption_info.sender, alice.user_id().to_owned()); + assert_matches!(&encryption_info.sender_device, Some(sender_device)); + assert_eq!(sender_device.to_owned(), alice.device_id().to_owned()); +} /// If the sender device is genuinely unknown (it is not in the store, nor does /// the to-device message contain `sender_device_keys`), decryption will fail, From c6d1b608003e104cd25451072482491f009363b6 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 10 Oct 2025 15:35:04 +0100 Subject: [PATCH 7/7] test(crypto): Regresion test for #5613 Add a test to ensure that history-sharing still works when "exclude insecure devices" is enabled. --- .../src/room/shared_room_history.rs | 3 ++ .../src/helpers.rs | 26 +++++++++++++++ .../src/tests/e2ee/shared_history.rs | 32 ++++++++++++++++++- 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/room/shared_room_history.rs b/crates/matrix-sdk/src/room/shared_room_history.rs index ff8fcf6a511..3fc26b16010 100644 --- a/crates/matrix-sdk/src/room/shared_room_history.rs +++ b/crates/matrix-sdk/src/room/shared_room_history.rs @@ -132,6 +132,9 @@ pub(crate) async fn maybe_accept_key_bundle(room: &Room, inviter: &UserId) -> Re // Ensure that we get a fresh list of devices for the inviter, in case we need // to recalculate the `SenderData`. + // XXX: is this necessary, given (with exclude-insecure-devices), we should have + // checked that the inviter device was cross-signed when we received the + // to-device message? let (req_id, request) = olm_machine.query_keys_for_users(iter::once(bundle_info.sender_user.as_ref())); diff --git a/testing/matrix-sdk-integration-testing/src/helpers.rs b/testing/matrix-sdk-integration-testing/src/helpers.rs index 39aeac1b28d..6c38d586cce 100644 --- a/testing/matrix-sdk-integration-testing/src/helpers.rs +++ b/testing/matrix-sdk-integration-testing/src/helpers.rs @@ -12,6 +12,7 @@ use assign::assign; use matrix_sdk::{ Client, ClientBuilder, Room, config::{RequestConfig, SyncSettings}, + crypto::{CollectStrategy, DecryptionSettings}, encryption::EncryptionSettings, ruma::{ RoomId, @@ -22,6 +23,7 @@ use matrix_sdk::{ sync::SyncResponse, timeout::ElapsedError, }; +use matrix_sdk_base::crypto::TrustRequirement; use once_cell::sync::Lazy; use rand::Rng as _; use tempfile::{TempDir, tempdir}; @@ -39,6 +41,8 @@ enum SqlitePath { pub struct TestClientBuilder { username: String, use_sqlite_dir: Option, + decryption_settings: Option, + room_key_recipient_strategy: CollectStrategy, encryption_settings: EncryptionSettings, enable_share_history_on_invite: bool, http_proxy: Option, @@ -56,7 +60,9 @@ impl TestClientBuilder { Self { username, use_sqlite_dir: None, + decryption_settings: None, encryption_settings: Default::default(), + room_key_recipient_strategy: Default::default(), enable_share_history_on_invite: false, http_proxy: None, cross_process_store_locks_holder_name: None, @@ -87,6 +93,21 @@ impl TestClientBuilder { self } + /// Simulate the behaviour of the clients when the "exclude insecure + /// devices" (MSC4153) labs flag is enabled. + pub fn exclude_insecure_devices(mut self, exclude_insecure_devices: bool) -> Self { + let (sender_device_trust_requirement, room_key_recipient_strategy) = + if exclude_insecure_devices { + (TrustRequirement::CrossSignedOrLegacy, CollectStrategy::IdentityBasedStrategy) + } else { + (TrustRequirement::Untrusted, CollectStrategy::AllDevices) + }; + self.decryption_settings = Some(DecryptionSettings { sender_device_trust_requirement }); + self.room_key_recipient_strategy = room_key_recipient_strategy; + + self + } + pub fn http_proxy(mut self, url: String) -> Self { self.http_proxy = Some(url); self @@ -106,9 +127,14 @@ impl TestClientBuilder { .homeserver_url(homeserver_url) .sliding_sync_version_builder(VersionBuilder::Native) .with_encryption_settings(self.encryption_settings) + .with_room_key_recipient_strategy(self.room_key_recipient_strategy.clone()) .with_enable_share_history_on_invite(self.enable_share_history_on_invite) .request_config(RequestConfig::short_retry()); + if let Some(decryption_settings) = &self.decryption_settings { + client_builder = client_builder.with_decryption_settings(decryption_settings.clone()) + } + if let Some(holder_name) = &self.cross_process_store_locks_holder_name { client_builder = client_builder.cross_process_store_locks_holder_name(holder_name.clone()); diff --git a/testing/matrix-sdk-integration-testing/src/tests/e2ee/shared_history.rs b/testing/matrix-sdk-integration-testing/src/tests/e2ee/shared_history.rs index e4c0da78dae..18b985c6ee1 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/e2ee/shared_history.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/e2ee/shared_history.rs @@ -26,8 +26,26 @@ use crate::helpers::{SyncTokenAwareClient, TestClientBuilder, wait_for_room}; /// When we invite another user to a room with "joined" history visibility, we /// share the encryption history. +/// +/// Pre-"exclude insecure devices" test variant. #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn test_history_share_on_invite() -> Result<()> { + test_history_share_on_invite_helper(false).await +} + +/// When we invite another user to a room with "joined" history visibility, we +/// share the encryption history, even when "exclude insecure devices" is +/// enabled. +/// +/// Regression test for https://github.com/matrix-org/matrix-rust-sdk/issues/5613 +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn test_history_share_on_invite_exclude_insecure_devices() -> Result<()> { + test_history_share_on_invite_helper(true).await +} + +/// Common implementation for [`test_history_share_on_invite`] and +/// [`test_history_share_on_invite_exclude_insecure_devices]. +async fn test_history_share_on_invite_helper(exclude_insecure_devices: bool) -> Result<()> { let alice_span = tracing::info_span!("alice"); let bob_span = tracing::info_span!("bob"); @@ -38,6 +56,7 @@ async fn test_history_share_on_invite() -> Result<()> { .use_sqlite() .encryption_settings(encryption_settings) .enable_share_history_on_invite(true) + .exclude_insecure_devices(exclude_insecure_devices) .build() .await?; @@ -55,6 +74,7 @@ async fn test_history_share_on_invite() -> Result<()> { TestClientBuilder::new("bob") .encryption_settings(encryption_settings) .enable_share_history_on_invite(true) + .exclude_insecure_devices(exclude_insecure_devices) .build() .await?, ); @@ -86,8 +106,18 @@ async fn test_history_share_on_invite() -> Result<()> { alice_room.invite_user_by_id(bob.user_id().unwrap()).await?; // Alice is done. Bob has been invited and the room key bundle should have been - // sent out. Let's stop syncing so the logs contain less noise. + // sent out. Let's log her out, so we know that this feature works even when the + // sender device has been deleted (and to reduce the amount of noise in the + // logs). alice_sync_service.stop().await; + alice.logout().instrument(alice_span.clone()).await?; + + // Workaround for https://github.com/matrix-org/matrix-rust-sdk/issues/5770: Bob needs a copy of + // Alice's identity. + bob.encryption() + .request_user_identity(alice.user_id().unwrap()) + .instrument(bob_span.clone()) + .await?; let bob_response = bob.sync_once().instrument(bob_span.clone()).await?;