crypto: Fix bugs in processing incoming encrypted to-device messages

crates/matrix-sdk-crypto/CHANGELOG.md

@@ -8,6 +8,10 @@ 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.
   ([#5642](https://github.com/matrix-org/matrix-rust-sdk/pull/5642))
 

crates/matrix-sdk-crypto/src/machine/test_helpers.rs

@@ -30,26 +30,24 @@ use ruma::{
     encryption::OneTimeKey,
     events::dummy::ToDeviceDummyEventContent,
     serde::Raw,
-    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,
-        requests::{AnyOutgoingRequest, ToDeviceRequest},
+        events::{room::encrypted::ToDeviceEncryptedEventContent, ToDeviceEvent},
+        requests::AnyOutgoingRequest,
         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.
@@ -199,16 +197,22 @@ 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()]),
-    );
+    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();
 
@@ -227,6 +231,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,

crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs

@@ -28,8 +28,10 @@ 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,
+            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},
     },
@@ -45,9 +47,11 @@ use crate::{
     utilities::json_convert,
     verification::tests::bob_id,
     CrossSigningBootstrapRequests, DecryptionSettings, DeviceData, EncryptionSettings,
-    EncryptionSyncChanges, LocalTrust, OlmError, OlmMachine, Session, TrustRequirement,
+    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) =
@@ -113,14 +117,51 @@ 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_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`
+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,
+/// 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;
@@ -140,12 +181,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;
 
@@ -635,33 +686,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