From e22792607c175b700fb12f2fd0f339c24fb68087 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 9 Oct 2024 16:15:25 +0100 Subject: [PATCH 1/4] refactor(common): add `TimelineEventKind::UnableToDecrypt` --- .../src/deserialized_responses.rs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/crates/matrix-sdk-common/src/deserialized_responses.rs b/crates/matrix-sdk-common/src/deserialized_responses.rs index 672e057b6a1..4c59eb0c686 100644 --- a/crates/matrix-sdk-common/src/deserialized_responses.rs +++ b/crates/matrix-sdk-common/src/deserialized_responses.rs @@ -334,6 +334,12 @@ impl SyncTimelineEvent { Self { kind: TimelineEventKind::PlainText { event }, push_actions } } + /// Create a new `SyncTimelineEvent` to represent the given decryption + /// failure. + pub fn new_utd_event(event: Raw, utd_info: UnableToDecryptInfo) -> Self { + Self { kind: TimelineEventKind::UnableToDecrypt { event, utd_info }, push_actions: vec![] } + } + /// Get the event id of this `SyncTimelineEvent` if the event has any valid /// id. pub fn event_id(&self) -> Option { @@ -450,6 +456,11 @@ impl TimelineEvent { } } + /// Create a new `TimelineEvent` to represent the given decryption failure. + pub fn new_utd_event(event: Raw, utd_info: UnableToDecryptInfo) -> Self { + Self { kind: TimelineEventKind::UnableToDecrypt { event, utd_info }, push_actions: None } + } + /// Returns a reference to the (potentially decrypted) Matrix event inside /// this `TimelineEvent`. pub fn raw(&self) -> &Raw { @@ -482,6 +493,17 @@ pub enum TimelineEventKind { /// A successfully-decrypted encrypted event. Decrypted(DecryptedRoomEvent), + /// An encrypted event which could not be decrypted. + UnableToDecrypt { + /// The `m.room.encrypted` event. Depending on the source of the event, + /// it could actually be an [`AnyTimelineEvent`] (i.e., it may + /// have a `room_id` property). + event: Raw, + + /// Information on the reason we failed to decrypt + utd_info: UnableToDecryptInfo, + }, + /// An unencrypted event. PlainText { /// The actual event. Depending on the source of the event, it could @@ -502,6 +524,7 @@ impl TimelineEventKind { // expected to contain a `room_id`). It just means that the `room_id` will be ignored // in a future deserialization. TimelineEventKind::Decrypted(d) => d.event.cast_ref(), + TimelineEventKind::UnableToDecrypt { event, .. } => event.cast_ref(), TimelineEventKind::PlainText { event } => event, } } @@ -511,6 +534,7 @@ impl TimelineEventKind { pub fn encryption_info(&self) -> Option<&EncryptionInfo> { match self { TimelineEventKind::Decrypted(d) => Some(&d.encryption_info), + TimelineEventKind::UnableToDecrypt { .. } => None, TimelineEventKind::PlainText { .. } => None, } } @@ -525,6 +549,7 @@ impl TimelineEventKind { // expected to contain a `room_id`). It just means that the `room_id` will be ignored // in a future deserialization. TimelineEventKind::Decrypted(d) => d.event.cast(), + TimelineEventKind::UnableToDecrypt { event, .. } => event.cast(), TimelineEventKind::PlainText { event } => event, } } @@ -539,6 +564,12 @@ impl fmt::Debug for TimelineEventKind { .field("event", &DebugRawEvent(event)) .finish(), + Self::UnableToDecrypt { event, utd_info } => f + .debug_struct("TimelineEventDecryptionResult::UnableToDecrypt") + .field("event", &DebugRawEvent(event)) + .field("utd_info", &utd_info) + .finish(), + Self::Decrypted(decrypted) => { f.debug_tuple("TimelineEventDecryptionResult::Decrypted").field(decrypted).finish() } From 172b9fc8b5bf9de7655e4a20ebb2fa92450a451c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 10 Oct 2024 15:38:38 +0100 Subject: [PATCH 2/4] refactor(timeline): store UTDs in `decrypt_sync_room_event` When `decrypt_sync_room_event` fails to decrypt an event, return the UTD as a `SyncTimelineEvent` instead of an Error. --- crates/matrix-sdk-base/src/client.rs | 57 +++++++++++++++++++--------- 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 031beab66dc..fdb798fdc09 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -29,7 +29,8 @@ use futures_util::Stream; #[cfg(feature = "e2e-encryption")] use matrix_sdk_crypto::{ store::DynCryptoStore, CollectStrategy, DecryptionSettings, EncryptionSettings, - EncryptionSyncChanges, OlmError, OlmMachine, ToDeviceRequest, TrustRequirement, + EncryptionSyncChanges, OlmError, OlmMachine, RoomEventDecryptionResult, ToDeviceRequest, + TrustRequirement, }; #[cfg(feature = "e2e-encryption")] use ruma::events::{ @@ -343,6 +344,13 @@ impl BaseClient { Ok(()) } + /// Attempt to decrypt the given raw event into a `SyncTimelineEvent`. + /// + /// In the case of a decryption error, returns a `SyncTimelineEvent` + /// representing the decryption error; in the case of problems with our + /// application, returns `Err`. + /// + /// Returns `Ok(None)` if encryption is not configured. #[cfg(feature = "e2e-encryption")] async fn decrypt_sync_room_event( &self, @@ -355,24 +363,37 @@ impl BaseClient { let decryption_settings = DecryptionSettings { sender_device_trust_requirement: self.decryption_trust_requirement, }; - let event: SyncTimelineEvent = - olm.decrypt_room_event(event.cast_ref(), room_id, &decryption_settings).await?.into(); - - if let Ok(AnySyncTimelineEvent::MessageLike(e)) = event.raw().deserialize() { - match &e { - AnySyncMessageLikeEvent::RoomMessage(SyncMessageLikeEvent::Original( - original_event, - )) => { - if let MessageType::VerificationRequest(_) = &original_event.content.msgtype { - self.handle_verification_event(&e, room_id).await?; + + let event = match olm + .try_decrypt_room_event(event.cast_ref(), room_id, &decryption_settings) + .await? + { + RoomEventDecryptionResult::Decrypted(decrypted) => { + let event: SyncTimelineEvent = decrypted.into(); + + if let Ok(AnySyncTimelineEvent::MessageLike(e)) = event.raw().deserialize() { + match &e { + AnySyncMessageLikeEvent::RoomMessage(SyncMessageLikeEvent::Original( + original_event, + )) => { + if let MessageType::VerificationRequest(_) = + &original_event.content.msgtype + { + self.handle_verification_event(&e, room_id).await?; + } + } + _ if e.event_type().to_string().starts_with("m.key.verification") => { + self.handle_verification_event(&e, room_id).await?; + } + _ => (), } } - _ if e.event_type().to_string().starts_with("m.key.verification") => { - self.handle_verification_event(&e, room_id).await?; - } - _ => (), + event } - } + RoomEventDecryptionResult::UnableToDecrypt(utd_info) => { + SyncTimelineEvent::new_utd_event(event.clone(), utd_info) + } + }; Ok(Some(event)) } @@ -460,10 +481,10 @@ impl BaseClient { AnySyncMessageLikeEvent::RoomEncrypted( SyncMessageLikeEvent::Original(_), ) => { - if let Ok(Some(e)) = Box::pin( + if let Some(e) = Box::pin( self.decrypt_sync_room_event(event.raw(), room.room_id()), ) - .await + .await? { event = e; } From f4e8222dd0d90483dc63894127cbb5d9a3400de2 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 10 Oct 2024 17:32:00 +0100 Subject: [PATCH 3/4] refactor(timeline): store UTDs in `decrypt_room_event` When `decrypt_room_event` fails to decrypt an event, return the UTD as a `TimelineEvent` instead of an Error. --- .../src/deserialized_responses.rs | 8 +++ .../matrix-sdk-ui/src/notification_client.rs | 54 +++++++++++-------- crates/matrix-sdk/src/room/mod.rs | 20 ++++--- 3 files changed, 48 insertions(+), 34 deletions(-) diff --git a/crates/matrix-sdk-common/src/deserialized_responses.rs b/crates/matrix-sdk-common/src/deserialized_responses.rs index 4c59eb0c686..c63a0e42968 100644 --- a/crates/matrix-sdk-common/src/deserialized_responses.rs +++ b/crates/matrix-sdk-common/src/deserialized_responses.rs @@ -708,6 +708,14 @@ pub enum UnableToDecryptReason { SenderIdentityNotTrusted(VerificationLevel), } +impl UnableToDecryptReason { + /// Returns true if this UTD is due to a missing room key (and hence might + /// resolve itself if we wait a bit.) + pub fn is_missing_room_key(&self) -> bool { + matches!(self, Self::MissingMegolmSession | Self::UnknownMegolmMessageIndex) + } +} + /// Deserialization helper for [`SyncTimelineEvent`], for the modern format. /// /// This has the exact same fields as [`SyncTimelineEvent`] itself, but has a diff --git a/crates/matrix-sdk-ui/src/notification_client.rs b/crates/matrix-sdk-ui/src/notification_client.rs index cc306440f2d..0c80298f0c4 100644 --- a/crates/matrix-sdk-ui/src/notification_client.rs +++ b/crates/matrix-sdk-ui/src/notification_client.rs @@ -20,10 +20,7 @@ use std::{ use futures_util::{pin_mut, StreamExt as _}; use matrix_sdk::{room::Room, Client, ClientBuildError, SlidingSyncList, SlidingSyncMode}; use matrix_sdk_base::{ - crypto::{vodozemac, MegolmError}, - deserialized_responses::TimelineEvent, - sliding_sync::http, - RoomState, StoreError, + deserialized_responses::TimelineEvent, sliding_sync::http, RoomState, StoreError, }; use ruma::{ assign, @@ -216,24 +213,24 @@ impl NotificationClient { tokio::time::sleep(Duration::from_millis(wait)).await; - match room.decrypt_event(raw_event.cast_ref()).await { - Ok(new_event) => { + let new_event = room.decrypt_event(raw_event.cast_ref()).await?; + + match new_event.kind { + matrix_sdk::deserialized_responses::TimelineEventKind::UnableToDecrypt { + utd_info, ..} => { + if utd_info.reason.is_missing_room_key() { + // Decryption error that could be caused by a missing room + // key; retry in a few. + wait *= 2; + } else { + debug!("Event could not be decrypted, but waiting longer is unlikely to help: {:?}", utd_info.reason); + return Ok(None); + } + } + _ => { trace!("Waiting succeeded and event could be decrypted!"); return Ok(Some(new_event)); } - Err(matrix_sdk::Error::MegolmError( - MegolmError::MissingRoomKey(_) - | MegolmError::Decryption( - vodozemac::megolm::DecryptionError::UnknownMessageIndex(_, _), - ), - )) => { - // Decryption error that could be caused by a missing room key; - // retry in a few. - wait *= 2; - } - Err(err) => { - return Err(err.into()); - } } } @@ -259,10 +256,21 @@ impl NotificationClient { match encryption_sync { Ok(sync) => match sync.run_fixed_iterations(2, sync_permit_guard).await { Ok(()) => match room.decrypt_event(raw_event.cast_ref()).await { - Ok(new_event) => { - trace!("Encryption sync managed to decrypt the event."); - Ok(Some(new_event)) - } + Ok(new_event) => match new_event.kind { + matrix_sdk::deserialized_responses::TimelineEventKind::UnableToDecrypt { + utd_info, .. + } => { + trace!( + "Encryption sync failed to decrypt the event: {:?}", + utd_info.reason + ); + Ok(None) + } + _ => { + trace!("Encryption sync managed to decrypt the event."); + Ok(Some(new_event)) + } + }, Err(err) => { trace!("Encryption sync failed to decrypt the event: {err}"); Ok(None) diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index a0bf0ced59b..35d889d24f3 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -19,7 +19,7 @@ use futures_util::{ #[cfg(all(feature = "e2e-encryption", not(target_arch = "wasm32")))] pub use identity_status_changes::IdentityStatusChanges; #[cfg(feature = "e2e-encryption")] -use matrix_sdk_base::crypto::DecryptionSettings; +use matrix_sdk_base::crypto::{DecryptionSettings, RoomEventDecryptionResult}; #[cfg(all(feature = "e2e-encryption", not(target_arch = "wasm32")))] use matrix_sdk_base::crypto::{IdentityStatusChange, RoomIdentityProvider, UserIdentity}; use matrix_sdk_base::{ @@ -1215,7 +1215,8 @@ impl Room { /// # Arguments /// * `event` - The room event to be decrypted. /// - /// Returns the decrypted event. + /// Returns the decrypted event. In the case of a decryption error, returns + /// a `TimelineEvent` representing the decryption error. #[cfg(feature = "e2e-encryption")] pub async fn decrypt_event( &self, @@ -1227,24 +1228,21 @@ impl Room { let decryption_settings = DecryptionSettings { sender_device_trust_requirement: self.client.base_client().decryption_trust_requirement, }; - let decrypted = match machine - .decrypt_room_event(event.cast_ref(), self.inner.room_id(), &decryption_settings) - .await + let mut event: TimelineEvent = match machine + .try_decrypt_room_event(event.cast_ref(), self.inner.room_id(), &decryption_settings) + .await? { - Ok(event) => event, - Err(e) => { + RoomEventDecryptionResult::Decrypted(decrypted) => decrypted.into(), + RoomEventDecryptionResult::UnableToDecrypt(utd_info) => { self.client .encryption() .backups() .maybe_download_room_key(self.room_id().to_owned(), event.clone()); - - return Err(e.into()); + TimelineEvent::new_utd_event(event.clone().cast(), utd_info) } }; - let mut event: TimelineEvent = decrypted.into(); event.push_actions = self.event_push_actions(event.raw()).await?; - Ok(event) } From 1cc4629f60c24c9b8ce165a3fcd0000f7c9a05ae Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 11 Oct 2024 17:18:01 +0100 Subject: [PATCH 4/4] test: Update tests to use new UTD TimelineEventKind variant Make the tests behave the same way as the network code, by returning UTDs as `TimelineEventKind::UnableToDecrypt` instead of `TimelineEventKind::PlainText`. --- .../matrix-sdk-base/src/sliding_sync/mod.rs | 11 +++++- .../src/timeline/tests/encryption.rs | 39 ++++++++++++++----- .../src/timeline/tests/read_receipts.rs | 3 +- .../src/timeline/tests/shields.rs | 3 +- crates/matrix-sdk/src/test_utils/events.rs | 20 ++++++++++ 5 files changed, 62 insertions(+), 14 deletions(-) diff --git a/crates/matrix-sdk-base/src/sliding_sync/mod.rs b/crates/matrix-sdk-base/src/sliding_sync/mod.rs index 819d1662eec..17efc70323e 100644 --- a/crates/matrix-sdk-base/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk-base/src/sliding_sync/mod.rs @@ -876,7 +876,10 @@ mod tests { }; use assert_matches::assert_matches; - use matrix_sdk_common::{deserialized_responses::SyncTimelineEvent, ring_buffer::RingBuffer}; + use matrix_sdk_common::{ + deserialized_responses::{SyncTimelineEvent, UnableToDecryptInfo, UnableToDecryptReason}, + ring_buffer::RingBuffer, + }; use matrix_sdk_test::async_test; use ruma::{ api::client::sync::sync_events::UnreadNotificationsCount, @@ -2494,7 +2497,7 @@ mod tests { } fn make_encrypted_event(id: &str) -> SyncTimelineEvent { - SyncTimelineEvent::new( + SyncTimelineEvent::new_utd_event( Raw::from_json_string( json!({ "type": "m.room.encrypted", @@ -2512,6 +2515,10 @@ mod tests { .to_string(), ) .unwrap(), + UnableToDecryptInfo { + session_id: Some("".to_owned()), + reason: UnableToDecryptReason::MissingMegolmSession, + }, ) } diff --git a/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs b/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs index ce038e61574..4b0683529d8 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs @@ -27,7 +27,7 @@ use matrix_sdk::{ crypto::{decrypt_room_key_export, types::events::UtdCause, OlmMachine}, test_utils::test_client_builder, }; -use matrix_sdk_base::deserialized_responses::SyncTimelineEvent; +use matrix_sdk_base::deserialized_responses::{SyncTimelineEvent, UnableToDecryptReason}; use matrix_sdk_test::{async_test, BOB}; use ruma::{ assign, @@ -105,7 +105,8 @@ async fn test_retry_message_decryption() { ), None, )) - .sender(&BOB), + .sender(&BOB) + .into_utd_sync_timeline_event(), ) .await; @@ -215,7 +216,11 @@ async fn test_retry_edit_decryption() { .into(), ); timeline - .handle_live_event(f.event(RoomEncryptedEventContent::new(encrypted, None)).sender(&BOB)) + .handle_live_event( + f.event(RoomEncryptedEventContent::new(encrypted, None)) + .sender(&BOB) + .into_utd_sync_timeline_event(), + ) .await; let event_id = @@ -242,7 +247,8 @@ async fn test_retry_edit_decryption() { f.event(assign!(RoomEncryptedEventContent::new(encrypted, None), { relates_to: Some(Relation::Replacement(Replacement::new(event_id))), })) - .sender(&BOB), + .sender(&BOB) + .into_utd_sync_timeline_event(), ) .await; @@ -321,7 +327,8 @@ async fn test_retry_edit_and_more() { mBZdKIaqDTUBFvcvbn2gQaWtUipQdJQRKyv2h0AWveVkv75lp5hRb7jolCi08oMX8cM+V3Zzyi7\ mlPAzZjDz0PaRbQwfbMTTHkcL7TZybBi4vLX4f5ZR2Iiysc7gw", )) - .sender(&BOB), + .sender(&BOB) + .into_utd_sync_timeline_event(), ) .await; @@ -337,7 +344,9 @@ async fn test_retry_edit_and_more() { ); timeline .handle_live_event( - f.event(assign!(msg2, { relates_to: Some(Relation::Replacement(Replacement::new(event_id))) })).sender(&BOB), + f.event(assign!(msg2, { relates_to: Some(Relation::Replacement(Replacement::new(event_id))) })) + .sender(&BOB) + .into_utd_sync_timeline_event(), ) .await; @@ -349,7 +358,8 @@ async fn test_retry_edit_and_more() { 2r/fEvAW/9QB+N6fX4g9729bt5ftXRqa5QI7NA351RNUveRHxVvx+2x0WJArQjYGRk7tMS2rUto\ IYt2ZY17nE1UJjN7M87STnCF9c9qy4aGNqIpeVIht6XbtgD7gQ", )) - .sender(&BOB), + .sender(&BOB) + .into_utd_sync_timeline_event(), ) .await; @@ -422,7 +432,8 @@ async fn test_retry_message_decryption_highlighted() { ), None, )) - .sender(&BOB), + .sender(&BOB) + .into_utd_sync_timeline_event(), ) .await; @@ -547,7 +558,7 @@ async fn test_utd_cause_for_missing_membership_is_unknown() { } fn utd_event_with_unsigned(unsigned: serde_json::Value) -> SyncTimelineEvent { - SyncTimelineEvent::new(Raw::from_json( + let raw = Raw::from_json( to_raw_value(&json!({ "event_id": "$myevent", "sender": "@u:s", @@ -564,5 +575,13 @@ fn utd_event_with_unsigned(unsigned: serde_json::Value) -> SyncTimelineEvent { })) .unwrap(), - )) + ); + + SyncTimelineEvent::new_utd_event( + raw, + matrix_sdk::deserialized_responses::UnableToDecryptInfo { + session_id: Some("SESSION_ID".into()), + reason: UnableToDecryptReason::MissingMegolmSession, + }, + ) } diff --git a/crates/matrix-sdk-ui/src/timeline/tests/read_receipts.rs b/crates/matrix-sdk-ui/src/timeline/tests/read_receipts.rs index 574a8bec6a4..6f0c2e02a17 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/read_receipts.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/read_receipts.rs @@ -400,7 +400,8 @@ async fn test_read_receipts_updates_on_message_decryption() { ), None, )) - .sender(&BOB), + .sender(&BOB) + .into_utd_sync_timeline_event(), ) .await; diff --git a/crates/matrix-sdk-ui/src/timeline/tests/shields.rs b/crates/matrix-sdk-ui/src/timeline/tests/shields.rs index 5cc973c4016..33c90d0e25c 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/shields.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/shields.rs @@ -151,7 +151,8 @@ async fn test_utd_shield() { ), None, )) - .sender(&ALICE), + .sender(&ALICE) + .into_utd_sync_timeline_event(), ) .await; diff --git a/crates/matrix-sdk/src/test_utils/events.rs b/crates/matrix-sdk/src/test_utils/events.rs index 9ace4ccd49f..3ecaebb5b47 100644 --- a/crates/matrix-sdk/src/test_utils/events.rs +++ b/crates/matrix-sdk/src/test_utils/events.rs @@ -16,7 +16,9 @@ use std::sync::atomic::{AtomicU64, Ordering::SeqCst}; +use as_variant::as_variant; use matrix_sdk_base::deserialized_responses::{SyncTimelineEvent, TimelineEvent}; +use matrix_sdk_common::deserialized_responses::UnableToDecryptReason; use ruma::{ events::{ message::TextContentBlock, @@ -31,6 +33,7 @@ use ruma::{ reaction::ReactionEventContent, relation::{Annotation, InReplyTo, Replacement, Thread}, room::{ + encrypted::{EncryptedEventScheme, RoomEncryptedEventContent}, message::{Relation, RoomMessageEventContent, RoomMessageEventContentWithoutRelation}, redaction::RoomRedactionEventContent, }, @@ -185,6 +188,23 @@ where } } +impl EventBuilder { + /// Turn this event into a SyncTimelineEvent representing a decryption + /// failure + pub fn into_utd_sync_timeline_event(self) -> SyncTimelineEvent { + let session_id = as_variant!(&self.content.scheme, EncryptedEventScheme::MegolmV1AesSha2) + .map(|content| content.session_id.clone()); + + SyncTimelineEvent::new_utd_event( + self.into(), + crate::deserialized_responses::UnableToDecryptInfo { + session_id, + reason: UnableToDecryptReason::MissingMegolmSession, + }, + ) + } +} + impl EventBuilder { /// Adds a reply relation to the current event. pub fn reply_to(mut self, event_id: &EventId) -> Self {