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: #5768

Author: richvdh

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.

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<ToDeviceEncryptedEventContent>,
+) -> ProcessedToDeviceEvent {
+    let event = ToDeviceEvent::new(sender.to_owned(), raw_encrypted);
 
     let event = json_convert(&event).unwrap();
 

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;
 

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<Option<&DeviceKeys>> {
         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.