Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(crypto): Add optional withheld reason to UnableToDecryptReason #4305

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/matrix-sdk-base/src/sliding_sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2656,7 +2656,7 @@ mod tests {
.unwrap(),
UnableToDecryptInfo {
session_id: Some("".to_owned()),
reason: UnableToDecryptReason::MissingMegolmSession,
reason: UnableToDecryptReason::MissingMegolmSession(None),
},
)
}
Expand Down
28 changes: 24 additions & 4 deletions crates/matrix-sdk-common/src/deserialized_responses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,23 @@ fn unknown_utd_reason() -> UnableToDecryptReason {
UnableToDecryptReason::Unknown
}

/// A room key might be missing because the sender refused to share it or was
/// not technically able to share it.
/// In the protocol this is reflected by clients sending `m.room_key.withheld`
/// to the participant that won't receive the key.
/// We only want here to consider the subset of codes that clients should
/// use to display specific UX.
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)]
pub enum WithheldReason {
/// When the sender refuses to send the key because his security settings in
/// that room are not met by your device. Could be that your device is
/// not cross-signed (insecure device) or that you are not verified by
/// the sender.
TrustRequirementMismatch,
/// Other reasons. Like fail to establish a secure channel to share the key,
/// or if for some reason our device is blocked.
Other,
}
/// Reason code for a decryption failure
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub enum UnableToDecryptReason {
Expand All @@ -684,9 +701,7 @@ pub enum UnableToDecryptReason {

/// Decryption failed because we're missing the megolm session that was used
/// to encrypt the event.
///
/// TODO: support withheld codes?
MissingMegolmSession,
MissingMegolmSession(Option<WithheldReason>),

/// Decryption failed because, while we have the megolm session that was
/// used to encrypt the message, it is ratcheted too far forward.
Expand Down Expand Up @@ -718,7 +733,12 @@ 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)
match self {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc says here that true means that ( [it] might resolve itself if we wait a bit.)
If we have a withheld code waiting won't help. It is used by the notification client to decide if it should work for an additional encryption sync.

// Return true only if the key is not withheld on purpose.
Self::MissingMegolmSession(None) => true,
Self::UnknownMegolmMessageIndex => true,
_ => false,
}
}
}

Expand Down
11 changes: 9 additions & 2 deletions crates/matrix-sdk-crypto/src/machine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use matrix_sdk_common::{
deserialized_responses::{
AlgorithmInfo, DecryptedRoomEvent, DeviceLinkProblem, EncryptionInfo, UnableToDecryptInfo,
UnableToDecryptReason, UnsignedDecryptionResult, UnsignedEventLocation, VerificationLevel,
VerificationState,
VerificationState, WithheldReason,
},
BoxFuture,
};
Expand Down Expand Up @@ -87,6 +87,7 @@ use crate::{
room_key::{MegolmV1AesSha2Content, RoomKeyContent},
room_key_withheld::{
MegolmV1AesSha2WithheldContent, RoomKeyWithheldContent, RoomKeyWithheldEvent,
WithheldCode,
},
ToDeviceEvents,
},
Expand Down Expand Up @@ -2579,7 +2580,13 @@ fn megolm_error_to_utd_info(
let reason = match error {
EventError(_) => UnableToDecryptReason::MalformedEncryptedEvent,
Decode(_) => UnableToDecryptReason::MalformedEncryptedEvent,
MissingRoomKey(_) => UnableToDecryptReason::MissingMegolmSession,
MissingRoomKey(maybe_withheld) => match maybe_withheld {
Some(WithheldCode::Unverified) => UnableToDecryptReason::MissingMegolmSession(Some(
WithheldReason::TrustRequirementMismatch,
)),
Some(_) => UnableToDecryptReason::MissingMegolmSession(Some(WithheldReason::Other)),
_ => UnableToDecryptReason::MissingMegolmSession(None),
},
Decryption(DecryptionError::UnknownMessageIndex(_, _)) => {
UnableToDecryptReason::UnknownMegolmMessageIndex
}
Expand Down
10 changes: 7 additions & 3 deletions crates/matrix-sdk-crypto/src/machine/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use futures_util::{pin_mut, FutureExt, StreamExt};
use itertools::Itertools;
use matrix_sdk_common::deserialized_responses::{
UnableToDecryptInfo, UnableToDecryptReason, UnsignedDecryptionResult, UnsignedEventLocation,
WithheldReason,
};
use matrix_sdk_test::{async_test, message_like_event_content, ruma_response_from_json, test_json};
use ruma::{
Expand Down Expand Up @@ -682,7 +683,10 @@ async fn test_withheld_unverified() {
bob.try_decrypt_room_event(&room_event, room_id, &decryption_settings).await.unwrap();
assert_let!(RoomEventDecryptionResult::UnableToDecrypt(utd_info) = decrypt_result);
assert!(utd_info.session_id.is_some());
assert_eq!(utd_info.reason, UnableToDecryptReason::MissingMegolmSession);
assert_eq!(
utd_info.reason,
UnableToDecryptReason::MissingMegolmSession(Some(WithheldReason::TrustRequirementMismatch))
);
}

/// Test what happens when we feed an unencrypted event into the decryption
Expand Down Expand Up @@ -1361,7 +1365,7 @@ async fn test_unsigned_decryption() {
replace_encryption_result,
UnsignedDecryptionResult::UnableToDecrypt(UnableToDecryptInfo {
session_id: Some(second_room_key_session_id),
reason: UnableToDecryptReason::MissingMegolmSession,
reason: UnableToDecryptReason::MissingMegolmSession(None),
})
);

Expand Down Expand Up @@ -1467,7 +1471,7 @@ async fn test_unsigned_decryption() {
thread_encryption_result,
UnsignedDecryptionResult::UnableToDecrypt(UnableToDecryptInfo {
session_id: Some(third_room_key_session_id),
reason: UnableToDecryptReason::MissingMegolmSession,
reason: UnableToDecryptReason::MissingMegolmSession(None),
})
);

Expand Down
36 changes: 27 additions & 9 deletions crates/matrix-sdk-crypto/src/types/events/utd_cause.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// limitations under the License.

use matrix_sdk_common::deserialized_responses::{
UnableToDecryptInfo, UnableToDecryptReason, VerificationLevel,
UnableToDecryptInfo, UnableToDecryptReason, VerificationLevel, WithheldReason,
};
use ruma::{events::AnySyncTimelineEvent, serde::Raw};
use serde::Deserialize;
Expand Down Expand Up @@ -47,6 +47,18 @@ pub enum UtdCause {
/// data was obtained from an insecure source (imported from a file,
/// obtained from a legacy (asymmetric) backup, unsafe key forward, etc.)
UnknownDevice = 4,

/// The keys for this event are intentionally withheld.
/// The sender has refused to share the key because our device does not meet
/// the sender's security requirements.
WithheldForUnverifiedOrInsecureDevice = 5,

/// The keys for this event are missing, likely because the sender was
/// unable to share them (e.g., failure to establish an Olm 1:1
/// channel). Alternatively, the sender may have deliberately excluded
/// this device by cherry-picking and blocking it, no action can be taken on
/// our side.
WithheldBySender = 6,
}

/// MSC4115 membership info in the unsigned area.
Expand All @@ -73,7 +85,13 @@ impl UtdCause {
) -> Self {
// TODO: in future, use more information to give a richer answer. E.g.
match unable_to_decrypt_info.reason {
UnableToDecryptReason::MissingMegolmSession
UnableToDecryptReason::MissingMegolmSession(Some(reason)) => match reason {
WithheldReason::TrustRequirementMismatch => {
UtdCause::WithheldForUnverifiedOrInsecureDevice
}
WithheldReason::Other => UtdCause::WithheldBySender,
},
UnableToDecryptReason::MissingMegolmSession(None)
| UnableToDecryptReason::UnknownMegolmMessageIndex => {
// Look in the unsigned area for a `membership` field.
if let Some(raw_event) = raw_event {
Expand Down Expand Up @@ -125,7 +143,7 @@ mod tests {
None,
&UnableToDecryptInfo {
session_id: None,
reason: UnableToDecryptReason::MissingMegolmSession,
reason: UnableToDecryptReason::MissingMegolmSession(None),
}
),
UtdCause::Unknown
Expand All @@ -140,7 +158,7 @@ mod tests {
Some(&raw_event(json!({}))),
&UnableToDecryptInfo {
session_id: None,
reason: UnableToDecryptReason::MissingMegolmSession
reason: UnableToDecryptReason::MissingMegolmSession(None)
}
),
UtdCause::Unknown
Expand All @@ -156,7 +174,7 @@ mod tests {
Some(&raw_event(json!({ "unsigned": { "membership": 3 } }))),
&UnableToDecryptInfo {
session_id: None,
reason: UnableToDecryptReason::MissingMegolmSession
reason: UnableToDecryptReason::MissingMegolmSession(None)
}
),
UtdCause::Unknown
Expand All @@ -172,7 +190,7 @@ mod tests {
Some(&raw_event(json!({ "unsigned": { "membership": "invite" } }),)),
&UnableToDecryptInfo {
session_id: None,
reason: UnableToDecryptReason::MissingMegolmSession
reason: UnableToDecryptReason::MissingMegolmSession(None)
}
),
UtdCause::Unknown
Expand All @@ -188,7 +206,7 @@ mod tests {
Some(&raw_event(json!({ "unsigned": { "membership": "join" } }))),
&UnableToDecryptInfo {
session_id: None,
reason: UnableToDecryptReason::MissingMegolmSession
reason: UnableToDecryptReason::MissingMegolmSession(None)
}
),
UtdCause::Unknown
Expand All @@ -204,7 +222,7 @@ mod tests {
Some(&raw_event(json!({ "unsigned": { "membership": "leave" } }))),
&UnableToDecryptInfo {
session_id: None,
reason: UnableToDecryptReason::MissingMegolmSession
reason: UnableToDecryptReason::MissingMegolmSession(None)
}
),
UtdCause::SentBeforeWeJoined
Expand Down Expand Up @@ -238,7 +256,7 @@ mod tests {
)),
&UnableToDecryptInfo {
session_id: None,
reason: UnableToDecryptReason::MissingMegolmSession
reason: UnableToDecryptReason::MissingMegolmSession(None)
}
),
UtdCause::SentBeforeWeJoined
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk-ui/src/timeline/tests/encryption.rs
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ fn utd_event_with_unsigned(unsigned: serde_json::Value) -> SyncTimelineEvent {
raw,
matrix_sdk::deserialized_responses::UnableToDecryptInfo {
session_id: Some("SESSION_ID".into()),
reason: UnableToDecryptReason::MissingMegolmSession,
reason: UnableToDecryptReason::MissingMegolmSession(None),
},
)
}
2 changes: 1 addition & 1 deletion crates/matrix-sdk/src/test_utils/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ impl EventBuilder<RoomEncryptedEventContent> {
self.into(),
crate::deserialized_responses::UnableToDecryptInfo {
session_id,
reason: UnableToDecryptReason::MissingMegolmSession,
reason: UnableToDecryptReason::MissingMegolmSession(None),
},
)
}
Expand Down