From c2eeca3f333265230b0efbbb79c0943c0d1b7604 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 14 May 2025 18:24:35 +0100 Subject: [PATCH 1/4] crypto: add some instrumentation to `get_room_event_encryption_info` It helps to know which event we're getting the encryption info for. --- crates/matrix-sdk-crypto/src/machine/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/matrix-sdk-crypto/src/machine/mod.rs b/crates/matrix-sdk-crypto/src/machine/mod.rs index 15fe947e5..0f2fe1cc3 100644 --- a/crates/matrix-sdk-crypto/src/machine/mod.rs +++ b/crates/matrix-sdk-crypto/src/machine/mod.rs @@ -2270,6 +2270,7 @@ impl OlmMachine { /// /// * `event` - The event to get information for. /// * `room_id` - The ID of the room where the event was sent to. + #[instrument(skip(self, event), fields(event_id, sender, session_id))] pub async fn get_room_event_encryption_info( &self, event: &Raw, @@ -2286,6 +2287,11 @@ impl OlmMachine { } }; + Span::current() + .record("sender", debug(&event.sender)) + .record("event_id", debug(&event.event_id)) + .record("session_id", content.session_id()); + self.get_session_encryption_info(room_id, content.session_id(), &event.sender).await } From f0ab6cb1a40178e871497b4eb728532b0894d810 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 23 May 2025 13:53:17 +0100 Subject: [PATCH 2/4] crypto: use a dedicated VerificationLevel if we know the sender of an event is spoofed --- .../src/deserialized_responses.rs | 18 ++++++++++++++++++ crates/matrix-sdk-crypto/src/machine/mod.rs | 11 +++-------- .../tests/decryption_verification_state.rs | 6 ++---- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/crates/matrix-sdk-common/src/deserialized_responses.rs b/crates/matrix-sdk-common/src/deserialized_responses.rs index f03180aa6..af81ec64f 100644 --- a/crates/matrix-sdk-common/src/deserialized_responses.rs +++ b/crates/matrix-sdk-common/src/deserialized_responses.rs @@ -42,6 +42,8 @@ const VERIFICATION_VIOLATION: &str = "Encrypted by a previously-verified user who is no longer verified."; const UNSIGNED_DEVICE: &str = "Encrypted by a device not verified by its owner."; const UNKNOWN_DEVICE: &str = "Encrypted by an unknown or deleted device."; +const MISMATCHED_SENDER: &str = + "The sender of the event does not match the owner of the device that created the Megolm session."; pub const SENT_IN_CLEAR: &str = "Not encrypted."; /// Represents the state of verification for a decrypted message sent by a @@ -117,6 +119,10 @@ impl VerificationState { message: AUTHENTICITY_NOT_GUARANTEED, }, }, + VerificationLevel::MismatchedSender => ShieldState::Red { + code: ShieldStateCode::MismatchedSender, + message: MISMATCHED_SENDER, + }, }, } } @@ -171,6 +177,10 @@ impl VerificationState { } } }, + VerificationLevel::MismatchedSender => ShieldState::Red { + code: ShieldStateCode::MismatchedSender, + message: MISMATCHED_SENDER, + }, }, } } @@ -198,6 +208,10 @@ pub enum VerificationLevel { /// deleted) or because the key to decrypt the message was obtained from /// an insecure source. None(DeviceLinkProblem), + + /// The `sender` field on the event does not match the owner of the device + /// that established the Megolm session. + MismatchedSender, } impl fmt::Display for VerificationLevel { @@ -211,6 +225,7 @@ impl fmt::Display for VerificationLevel { "The sending device was not signed by the user's identity" } VerificationLevel::None(..) => "The sending device is not known", + VerificationLevel::MismatchedSender => MISMATCHED_SENDER, }; write!(f, "{display}") } @@ -271,6 +286,9 @@ pub enum ShieldStateCode { /// The sender was previously verified but changed their identity. #[serde(alias = "PreviouslyVerified")] VerificationViolation, + /// The `sender` field on the event does not match the owner of the device + /// that established the Megolm session. + MismatchedSender, } /// The algorithm specific information of a decrypted event. diff --git a/crates/matrix-sdk-crypto/src/machine/mod.rs b/crates/matrix-sdk-crypto/src/machine/mod.rs index 0f2fe1cc3..f78009912 100644 --- a/crates/matrix-sdk-crypto/src/machine/mod.rs +++ b/crates/matrix-sdk-crypto/src/machine/mod.rs @@ -1663,14 +1663,7 @@ impl OlmMachine { // `DeviceLinkProblem` for `VerificationLevel::None`. let (verification_state, device_id) = match sender_data.user_id() { Some(i) if i != sender => { - // For backwards compatibility, we treat this the same as "Unknown device". - // TODO: use a dedicated VerificationLevel here. - ( - VerificationState::Unverified(VerificationLevel::None( - DeviceLinkProblem::MissingDevice, - )), - None, - ) + (VerificationState::Unverified(VerificationLevel::MismatchedSender), None) } Some(_) | None => { @@ -1967,6 +1960,7 @@ impl OlmMachine { // Case 4 (VerificationLevel::VerificationViolation, _) + | (VerificationLevel::MismatchedSender, _) | (VerificationLevel::UnsignedDevice, false) | (VerificationLevel::None(_), false) => false, } @@ -1978,6 +1972,7 @@ impl OlmMachine { VerificationLevel::UnverifiedIdentity => true, VerificationLevel::VerificationViolation + | VerificationLevel::MismatchedSender | VerificationLevel::UnsignedDevice | VerificationLevel::None(_) => false, }, 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 cb704a245..30e184855 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 @@ -364,13 +364,11 @@ async fn test_verification_states_spoofed_sender() { .await .expect("Bob could not decrypt spoofed event"); - // The verification_state of the event should be `MissingDevice` (since it - // manifests as a message from Charlie which does not correspond to one of - // Charlie's devices). + // The verification_state of the event should be `MismatchedSender`. let event_encryption_info = bob.get_room_event_encryption_info(&event, room_id).await.unwrap(); assert_matches!( &event_encryption_info.verification_state, - VerificationState::Unverified(VerificationLevel::None(DeviceLinkProblem::MissingDevice)) + VerificationState::Unverified(VerificationLevel::MismatchedSender) ); } From b2210292bf81e919869112c69e4f9fc451a105aa Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 11 Jun 2025 17:06:31 +0100 Subject: [PATCH 3/4] crypto: Add a test for spoofed sender, with `TrustRequirement::CrossSigned` --- .../tests/decryption_verification_state.rs | 58 ++++++++++++++----- 1 file changed, 42 insertions(+), 16 deletions(-) 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 30e184855..9581034b8 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 @@ -46,8 +46,8 @@ use crate::{ CrossSigningKey, DeviceKeys, EventEncryptionAlgorithm, MasterPubkey, SelfSigningPubkey, }, utilities::json_convert, - CryptoStoreError, DecryptionSettings, DeviceData, EncryptionSettings, LocalTrust, OlmMachine, - OtherUserIdentityData, TrustRequirement, UserIdentity, + CryptoStoreError, DecryptionSettings, DeviceData, EncryptionSettings, LocalTrust, MegolmError, + OlmMachine, OtherUserIdentityData, TrustRequirement, UserIdentity, }; #[async_test] @@ -311,23 +311,37 @@ pub async fn mark_alice_identity_as_verified_test_helper(alice: &OlmMachine, bob .is_verified()); } +#[async_test] +async fn test_verification_states_spoofed_sender_untrusted() { + test_verification_states_spoofed_sender(TrustRequirement::Untrusted).await; +} + +#[async_test] +async fn test_verification_states_spoofed_sender_cross_signed() { + test_verification_states_spoofed_sender(TrustRequirement::CrossSigned).await; +} + /// Test that the verification state is set correctly when the sender of an /// event does not match the owner of the device that sent us the session. /// /// In this test, Bob receives an event from Alice, but the HS admin has /// rewritten the `sender` of the event to look like another user. -#[async_test] -async fn test_verification_states_spoofed_sender() { +/// +/// We run this test a couple of times, with different [`TrustRequirement`]s. +async fn test_verification_states_spoofed_sender( + sender_device_trust_requirement: TrustRequirement, +) { let (alice, bob) = get_machine_pair_with_setup_sessions_test_helper( tests::alice_id(), tests::user_id(), false, ) .await; + bob.bootstrap_cross_signing(false).await.unwrap(); + set_up_alice_cross_signing(&alice, &bob).await; let room_id = room_id!("!test:example.org"); - let decryption_settings = - DecryptionSettings { sender_device_trust_requirement: TrustRequirement::Untrusted }; + let decryption_settings = DecryptionSettings { sender_device_trust_requirement }; // Alice sends a message to Bob. let (event, _) = encrypt_message(&alice, room_id, &bob, "Secret message").await; @@ -337,7 +351,7 @@ async fn test_verification_states_spoofed_sender() { let event_encryption_info = bob.get_room_event_encryption_info(&event, room_id).await.unwrap(); assert_matches!( &event_encryption_info.verification_state, - VerificationState::Unverified(VerificationLevel::UnsignedDevice) + VerificationState::Unverified(VerificationLevel::UnverifiedIdentity) ); // Alice now sends a second message to Bob, using the same room key, but the HS @@ -360,16 +374,28 @@ async fn test_verification_states_spoofed_sender() { }); let event = json_convert(&event).unwrap(); - bob.decrypt_room_event(&event, room_id, &decryption_settings) - .await - .expect("Bob could not decrypt spoofed event"); + let decryption_result = bob.decrypt_room_event(&event, room_id, &decryption_settings).await; - // The verification_state of the event should be `MismatchedSender`. - let event_encryption_info = bob.get_room_event_encryption_info(&event, room_id).await.unwrap(); - assert_matches!( - &event_encryption_info.verification_state, - VerificationState::Unverified(VerificationLevel::MismatchedSender) - ); + if matches!(sender_device_trust_requirement, TrustRequirement::Untrusted) { + // In "Untrusted" mode, the event is decrypted correctly, but the + // verification_state should be `MismatchedSender`. + decryption_result.expect("Bob could not decrypt spoofed event"); + + let event_encryption_info = + bob.get_room_event_encryption_info(&event, room_id).await.unwrap(); + assert_matches!( + &event_encryption_info.verification_state, + VerificationState::Unverified(VerificationLevel::MismatchedSender) + ); + } else { + // In "CrossSigned" mode, we refuse to decrypt the event altogether. + let err = + decryption_result.expect_err("Bob was unexpectedly able to decrypt spoofed event"); + assert_matches!( + err, + MegolmError::SenderIdentityNotTrusted(VerificationLevel::MismatchedSender) + ); + } } #[async_test] From 0aece695dc444cd24ccbdee12fcc77e2dfa7af55 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 11 Jun 2025 17:15:44 +0100 Subject: [PATCH 4/4] crypto: update changelog --- crates/matrix-sdk-crypto/CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk-crypto/CHANGELOG.md b/crates/matrix-sdk-crypto/CHANGELOG.md index 1979c61e2..efcf7510b 100644 --- a/crates/matrix-sdk-crypto/CHANGELOG.md +++ b/crates/matrix-sdk-crypto/CHANGELOG.md @@ -6,17 +6,20 @@ All notable changes to this project will be documented in this file. ## [Unreleased] - ReleaseDate +- [**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)) + ## [0.12.0] - 2025-06-10 ### Features - [**breaking**] The `ProcessedToDeviceEvent::Decrypted` variant now also have an `EncryptionInfo` field. Format changed from `Decrypted(Raw)` to `Decrypted { raw: Raw, encryption_info: EncryptionInfo) }` - ([5074](https://github.com/matrix-org/matrix-rust-sdk/pull/5074)) + ([#5074](https://github.com/matrix-org/matrix-rust-sdk/pull/5074)) - [**breaking**] Move `session_id` from `EncryptionInfo` to `AlgorithmInfo` as it is megolm specific. Use `EncryptionInfo::session_id()` helper for quick access. - ([4981](https://github.com/matrix-org/matrix-rust-sdk/pull/4981)) + ([#4981](https://github.com/matrix-org/matrix-rust-sdk/pull/4981)) - Send stable identifier `sender_device_keys` for MSC4147 (Including device keys with Olm-encrypted events).