diff --git a/crates/matrix-sdk-crypto/src/error.rs b/crates/matrix-sdk-crypto/src/error.rs index 89fe5c5edaf..4400da9d696 100644 --- a/crates/matrix-sdk-crypto/src/error.rs +++ b/crates/matrix-sdk-crypto/src/error.rs @@ -120,6 +120,8 @@ pub enum MegolmError { /// An encrypted message wasn't decrypted, because the sender's /// cross-signing identity did not satisfy the requested /// [`crate::TrustRequirement`]. + /// + /// The nested value is the sender's current verification level. #[error("decryption failed because trust requirement not satisfied: {0}")] SenderIdentityNotTrusted(VerificationLevel), } diff --git a/crates/matrix-sdk-crypto/src/types/events/utd_cause.rs b/crates/matrix-sdk-crypto/src/types/events/utd_cause.rs index 1bd99fde29e..5a260ad50d9 100644 --- a/crates/matrix-sdk-crypto/src/types/events/utd_cause.rs +++ b/crates/matrix-sdk-crypto/src/types/events/utd_cause.rs @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +use matrix_sdk_common::deserialized_responses::{ + UnableToDecryptInfo, UnableToDecryptReason, VerificationLevel, +}; use ruma::{events::AnySyncTimelineEvent, serde::Raw}; use serde::Deserialize; @@ -24,16 +27,26 @@ pub enum UtdCause { #[default] Unknown = 0, - /// This event was sent when we were not a member of the room (or invited), - /// so it is impossible to decrypt (without MSC3061). - Membership = 1, - // - // TODO: Other causes for UTDs. For example, this message is device-historical, information - // extracted from the WithheldCode in the MissingRoomKey object, or various types of Olm - // session problems. - // - // Note: This needs to be a simple enum so we can export it via FFI, so if more information - // needs to be provided, it should be through a separate type. + /// We are missing the keys for this event, and the event was sent when we + /// were not a member of the room (or invited). + SentBeforeWeJoined = 1, + + /// The message was sent by a user identity we have not verified, but the + /// user was previously verified. + VerificationViolation = 2, + + /// The [`crate::TrustRequirement`] requires that the sending device be + /// signed by its owner, and it was not. + UnsignedDevice = 3, + + /// The [`crate::TrustRequirement`] requires that the sending device be + /// signed by its owner, and we were unable to securely find the device. + /// + /// This could be because the device has since been deleted, because we + /// haven't yet downloaded it from the server, or because the session + /// data was obtained from an insecure source (imported from a file, + /// obtained from a legacy (asymmetric) backup, unsafe key forward, etc.) + UnknownDevice = 4, } /// MSC4115 membership info in the unsigned area. @@ -54,96 +67,229 @@ enum Membership { impl UtdCause { /// Decide the cause of this UTD, based on the evidence we have. - pub fn determine(raw_event: Option<&Raw>) -> Self { + pub fn determine( + raw_event: Option<&Raw>, + unable_to_decrypt_info: &UnableToDecryptInfo, + ) -> Self { // TODO: in future, use more information to give a richer answer. E.g. - // is this event device-historical? Was the Olm communication disrupted? - // Did the sender refuse to send the key because we're not verified? - - // Look in the unsigned area for a `membership` field. - if let Some(raw_event) = raw_event { - if let Ok(Some(unsigned)) = raw_event.get_field::("unsigned") { - if let Membership::Leave = unsigned.membership { - // We were not a member - this is the cause of the UTD - return UtdCause::Membership; + match unable_to_decrypt_info.reason { + UnableToDecryptReason::MissingMegolmSession + | UnableToDecryptReason::UnknownMegolmMessageIndex => { + // Look in the unsigned area for a `membership` field. + if let Some(raw_event) = raw_event { + if let Ok(Some(unsigned)) = + raw_event.get_field::("unsigned") + { + if let Membership::Leave = unsigned.membership { + // We were not a member - this is the cause of the UTD + return UtdCause::SentBeforeWeJoined; + } + } } + UtdCause::Unknown + } + + UnableToDecryptReason::SenderIdentityNotTrusted( + VerificationLevel::VerificationViolation, + ) => UtdCause::VerificationViolation, + + UnableToDecryptReason::SenderIdentityNotTrusted(VerificationLevel::UnsignedDevice) => { + UtdCause::UnsignedDevice + } + + UnableToDecryptReason::SenderIdentityNotTrusted(VerificationLevel::None(_)) => { + UtdCause::UnknownDevice } - } - // We can't find an explanation for this UTD - UtdCause::Unknown + _ => UtdCause::Unknown, + } } } #[cfg(test)] mod tests { + use matrix_sdk_common::deserialized_responses::{ + DeviceLinkProblem, UnableToDecryptInfo, UnableToDecryptReason, VerificationLevel, + }; use ruma::{events::AnySyncTimelineEvent, serde::Raw}; use serde_json::{json, value::to_raw_value}; use crate::types::events::UtdCause; #[test] - fn a_missing_raw_event_means_we_guess_unknown() { + fn test_a_missing_raw_event_means_we_guess_unknown() { // When we don't provide any JSON to check for membership, then we guess the UTD // is unknown. - assert_eq!(UtdCause::determine(None), UtdCause::Unknown); + assert_eq!( + UtdCause::determine( + None, + &UnableToDecryptInfo { + session_id: None, + reason: UnableToDecryptReason::MissingMegolmSession, + } + ), + UtdCause::Unknown + ); } #[test] - fn if_there_is_no_membership_info_we_guess_unknown() { + fn test_if_there_is_no_membership_info_we_guess_unknown() { // If our JSON contains no membership info, then we guess the UTD is unknown. - assert_eq!(UtdCause::determine(Some(&raw_event(json!({})))), UtdCause::Unknown); + assert_eq!( + UtdCause::determine( + Some(&raw_event(json!({}))), + &UnableToDecryptInfo { + session_id: None, + reason: UnableToDecryptReason::MissingMegolmSession + } + ), + UtdCause::Unknown + ); } #[test] - fn if_membership_info_cant_be_parsed_we_guess_unknown() { + fn test_if_membership_info_cant_be_parsed_we_guess_unknown() { // If our JSON contains a membership property but not the JSON we expected, then // we guess the UTD is unknown. assert_eq!( - UtdCause::determine(Some(&raw_event(json!({ "unsigned": { "membership": 3 } })))), + UtdCause::determine( + Some(&raw_event(json!({ "unsigned": { "membership": 3 } }))), + &UnableToDecryptInfo { + session_id: None, + reason: UnableToDecryptReason::MissingMegolmSession + } + ), UtdCause::Unknown ); } #[test] - fn if_membership_is_invite_we_guess_unknown() { + fn test_if_membership_is_invite_we_guess_unknown() { // If membership=invite then we expected to be sent the keys so the cause of the // UTD is unknown. assert_eq!( - UtdCause::determine(Some(&raw_event( - json!({ "unsigned": { "membership": "invite" } }), - ))), + UtdCause::determine( + Some(&raw_event(json!({ "unsigned": { "membership": "invite" } }),)), + &UnableToDecryptInfo { + session_id: None, + reason: UnableToDecryptReason::MissingMegolmSession + } + ), UtdCause::Unknown ); } #[test] - fn if_membership_is_join_we_guess_unknown() { + fn test_if_membership_is_join_we_guess_unknown() { // If membership=join then we expected to be sent the keys so the cause of the // UTD is unknown. assert_eq!( - UtdCause::determine(Some(&raw_event(json!({ "unsigned": { "membership": "join" } })))), + UtdCause::determine( + Some(&raw_event(json!({ "unsigned": { "membership": "join" } }))), + &UnableToDecryptInfo { + session_id: None, + reason: UnableToDecryptReason::MissingMegolmSession + } + ), UtdCause::Unknown ); } #[test] - fn if_membership_is_leave_we_guess_membership() { + fn test_if_membership_is_leave_we_guess_membership() { // If membership=leave then we have an explanation for why we can't decrypt, // until we have MSC3061. assert_eq!( - UtdCause::determine(Some(&raw_event(json!({ "unsigned": { "membership": "leave" } })))), - UtdCause::Membership + UtdCause::determine( + Some(&raw_event(json!({ "unsigned": { "membership": "leave" } }))), + &UnableToDecryptInfo { + session_id: None, + reason: UnableToDecryptReason::MissingMegolmSession + } + ), + UtdCause::SentBeforeWeJoined + ); + } + + #[test] + fn test_if_reason_is_not_missing_key_we_guess_unknown_even_if_membership_is_leave() { + // If the UnableToDecryptReason is other than MissingMegolmSession or + // UnknownMegolmMessageIndex, we do not know the reason for the failure + // even if membership=leave. + assert_eq!( + UtdCause::determine( + Some(&raw_event(json!({ "unsigned": { "membership": "leave" } }))), + &UnableToDecryptInfo { + session_id: None, + reason: UnableToDecryptReason::MalformedEncryptedEvent + } + ), + UtdCause::Unknown ); } #[test] - fn if_unstable_prefix_membership_is_leave_we_guess_membership() { + fn test_if_unstable_prefix_membership_is_leave_we_guess_membership() { // Before MSC4115 is merged, we support the unstable prefix too. assert_eq!( - UtdCause::determine(Some(&raw_event( - json!({ "unsigned": { "io.element.msc4115.membership": "leave" } }) - ))), - UtdCause::Membership + UtdCause::determine( + Some(&raw_event( + json!({ "unsigned": { "io.element.msc4115.membership": "leave" } }) + )), + &UnableToDecryptInfo { + session_id: None, + reason: UnableToDecryptReason::MissingMegolmSession + } + ), + UtdCause::SentBeforeWeJoined + ); + } + + #[test] + fn test_verification_violation_is_passed_through() { + assert_eq!( + UtdCause::determine( + Some(&raw_event(json!({}))), + &UnableToDecryptInfo { + session_id: None, + reason: UnableToDecryptReason::SenderIdentityNotTrusted( + VerificationLevel::VerificationViolation, + ) + } + ), + UtdCause::VerificationViolation + ); + } + + #[test] + fn test_unsigned_device_is_passed_through() { + assert_eq!( + UtdCause::determine( + Some(&raw_event(json!({}))), + &UnableToDecryptInfo { + session_id: None, + reason: UnableToDecryptReason::SenderIdentityNotTrusted( + VerificationLevel::UnsignedDevice, + ) + } + ), + UtdCause::UnsignedDevice + ); + } + + #[test] + fn test_unknown_device_is_passed_through() { + assert_eq!( + UtdCause::determine( + Some(&raw_event(json!({}))), + &UnableToDecryptInfo { + session_id: None, + reason: UnableToDecryptReason::SenderIdentityNotTrusted( + VerificationLevel::None(DeviceLinkProblem::MissingDevice) + ) + } + ), + UtdCause::UnknownDevice ); } diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index 6dbab04a5db..3a37135059e 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -989,8 +989,6 @@ impl TimelineController

{ decryptor: impl Decryptor, session_ids: Option>, ) { - use matrix_sdk::crypto::types::events::UtdCause; - use super::EncryptedMessage; let mut state = self.state.clone().write_owned().await; @@ -1038,16 +1036,17 @@ impl TimelineController

{ async move { let event_item = item.as_event()?; - let session_id = match event_item.content().as_unable_to_decrypt()? { - EncryptedMessage::MegolmV1AesSha2 { session_id, .. } - if should_retry(session_id) => - { - session_id - } - EncryptedMessage::MegolmV1AesSha2 { .. } - | EncryptedMessage::OlmV1Curve25519AesSha2 { .. } - | EncryptedMessage::Unknown => return None, - }; + let (session_id, utd_cause) = + match event_item.content().as_unable_to_decrypt()? { + EncryptedMessage::MegolmV1AesSha2 { session_id, cause, .. } + if should_retry(session_id) => + { + (session_id, cause) + } + EncryptedMessage::MegolmV1AesSha2 { .. } + | EncryptedMessage::OlmV1Curve25519AesSha2 { .. } + | EncryptedMessage::Unknown => return None, + }; tracing::Span::current().record("session_id", session_id); @@ -1069,11 +1068,9 @@ impl TimelineController

{ "Successfully decrypted event that previously failed to decrypt" ); - let cause = UtdCause::determine(Some(original_json)); - // Notify observers that we managed to eventually decrypt an event. if let Some(hook) = unable_to_decrypt_hook { - hook.on_late_decrypt(&remote_event.event_id, cause).await; + hook.on_late_decrypt(&remote_event.event_id, *utd_cause).await; } Some(event) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index 4f23f58a426..b30102525fb 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -422,7 +422,17 @@ impl TimelineStateTransaction<'_> { settings: &TimelineSettings, day_divider_adjuster: &mut DayDividerAdjuster, ) -> HandleEventResult { - let raw = event.raw(); + let SyncTimelineEvent { push_actions, kind } = event; + let encryption_info = kind.encryption_info().cloned(); + + let (raw, utd_info) = match kind { + matrix_sdk::deserialized_responses::TimelineEventKind::UnableToDecrypt { + utd_info, + event, + } => (event, Some(utd_info)), + _ => (kind.into_raw(), None), + }; + let (event_id, sender, timestamp, txn_id, event_kind, should_add) = match raw.deserialize() { Ok(event) => { @@ -479,7 +489,7 @@ impl TimelineStateTransaction<'_> { event.sender().to_owned(), event.origin_server_ts(), event.transaction_id().map(ToOwned::to_owned), - TimelineEventKind::from_event(event, &room_version), + TimelineEventKind::from_event(event, &room_version, utd_info), should_add, ) } @@ -578,11 +588,11 @@ impl TimelineStateTransaction<'_> { } else { Default::default() }, - is_highlighted: event.push_actions.iter().any(Action::is_highlight), + is_highlighted: push_actions.iter().any(Action::is_highlight), flow: Flow::Remote { event_id: event_id.clone(), - raw_event: raw.clone(), - encryption_info: event.encryption_info().cloned(), + raw_event: raw, + encryption_info, txn_id, position, }, diff --git a/crates/matrix-sdk-ui/src/timeline/event_handler.rs b/crates/matrix-sdk-ui/src/timeline/event_handler.rs index 1161030522f..228eb96e606 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_handler.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_handler.rs @@ -18,8 +18,10 @@ use as_variant::as_variant; use eyeball_im::{ObservableVectorTransaction, ObservableVectorTransactionEntry}; use indexmap::IndexMap; use matrix_sdk::{ - crypto::types::events::UtdCause, deserialized_responses::EncryptionInfo, - ring_buffer::RingBuffer, send_queue::SendHandle, + crypto::types::events::UtdCause, + deserialized_responses::{EncryptionInfo, UnableToDecryptInfo}, + ring_buffer::RingBuffer, + send_queue::SendHandle, }; use ruma::{ events::{ @@ -35,6 +37,7 @@ use ruma::{ receipt::Receipt, relation::Replacement, room::{ + encrypted::RoomEncryptedEventContent, member::RoomMemberEventContent, message::{self, RoomMessageEventContent, RoomMessageEventContentWithoutRelation}, }, @@ -137,6 +140,12 @@ pub(super) enum TimelineEventKind { relations: BundledMessageLikeRelations, }, + /// An encrypted event that could not be decrypted + UnableToDecrypt { + content: RoomEncryptedEventContent, + unable_to_decrypt_info: UnableToDecryptInfo, + }, + /// Some remote event that was redacted a priori, i.e. we never had the /// original content, so we'll just display a dummy redacted timeline /// item. @@ -172,7 +181,11 @@ pub(super) enum TimelineEventKind { impl TimelineEventKind { /// Creates a new `TimelineEventKind` with the given event and room version. - pub fn from_event(event: AnySyncTimelineEvent, room_version: &RoomVersionId) -> Self { + pub fn from_event( + event: AnySyncTimelineEvent, + room_version: &RoomVersionId, + unable_to_decrypt_info: Option, + ) -> Self { match event { AnySyncTimelineEvent::MessageLike(AnySyncMessageLikeEvent::RoomRedaction(ev)) => { if let Some(redacts) = ev.redacts(room_version).map(ToOwned::to_owned) { @@ -182,6 +195,22 @@ impl TimelineEventKind { } } AnySyncTimelineEvent::MessageLike(ev) => match ev.original_content() { + Some(AnyMessageLikeEventContent::RoomEncrypted(content)) => { + // An event which is still encrypted. + if let Some(unable_to_decrypt_info) = unable_to_decrypt_info { + Self::UnableToDecrypt { content, unable_to_decrypt_info } + } else { + // If we get here, it means that some part of the code has created a + // `SyncTimelineEvent` containing an `m.room.encrypted` event + // without decrypting it. Possibly this means that encryption has not been + // configured. + // We treat it the same as any other message-like event. + Self::Message { + content: AnyMessageLikeEventContent::RoomEncrypted(content), + relations: ev.relations(), + } + } + } Some(content) => Self::Message { content, relations: ev.relations() }, None => Self::RedactedMessage { event_type: ev.event_type() }, }, @@ -344,21 +373,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } } - AnyMessageLikeEventContent::RoomEncrypted(c) => { - // TODO: Handle replacements if the replaced event is also UTD - let raw_event = self.ctx.flow.raw_event(); - let cause = UtdCause::determine(raw_event); - self.add_item(TimelineItemContent::unable_to_decrypt(c, cause), None); - - // Let the hook know that we ran into an unable-to-decrypt that is added to the - // timeline. - if let Some(hook) = self.meta.unable_to_decrypt_hook.as_ref() { - if let Some(event_id) = &self.ctx.flow.event_id() { - hook.on_utd(event_id, cause).await; - } - } - } - AnyMessageLikeEventContent::Sticker(content) => { if should_add { self.add_item(TimelineItemContent::Sticker(Sticker { content }), None); @@ -402,6 +416,21 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } }, + TimelineEventKind::UnableToDecrypt { content, unable_to_decrypt_info } => { + // TODO: Handle replacements if the replaced event is also UTD + let raw_event = self.ctx.flow.raw_event(); + let cause = UtdCause::determine(raw_event, &unable_to_decrypt_info); + self.add_item(TimelineItemContent::unable_to_decrypt(content, cause), None); + + // Let the hook know that we ran into an unable-to-decrypt that is added to the + // timeline. + if let Some(hook) = self.meta.unable_to_decrypt_hook.as_ref() { + if let Some(event_id) = &self.ctx.flow.event_id() { + hook.on_utd(event_id, cause).await; + } + } + } + TimelineEventKind::RedactedMessage { event_type } => { if event_type != MessageLikeEventType::Reaction && should_add { self.add_item(TimelineItemContent::RedactedMessage, None); diff --git a/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs b/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs index 4b0683529d8..0bf9d80d4ce 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs @@ -493,7 +493,7 @@ async fn test_utd_cause_for_nonmember_event_is_found() { TimelineItemContent::UnableToDecrypt(EncryptedMessage::MegolmV1AesSha2 { cause, .. }) = event.content() ); - assert_eq!(*cause, UtdCause::Membership); + assert_eq!(*cause, UtdCause::SentBeforeWeJoined); } #[async_test] @@ -516,7 +516,7 @@ async fn test_utd_cause_for_nonmember_event_is_found_unstable_prefix() { TimelineItemContent::UnableToDecrypt(EncryptedMessage::MegolmV1AesSha2 { cause, .. }) = event.content() ); - assert_eq!(*cause, UtdCause::Membership); + assert_eq!(*cause, UtdCause::SentBeforeWeJoined); } #[async_test]