Merge pull request #4305 from matrix-org/valere/support_for_withheld_reason

feat(crypto): Add optional withheld reason to `UnableToDecryptReason`

Author: BillCarsonFr

Cargo.lock

@@ -112,7 +112,7 @@ mod tests {
 
     #[test]
     fn test_withheld_error_mapping() {
-        use matrix_sdk_crypto::types::events::room_key_withheld::WithheldCode;
+        use matrix_sdk_common::deserialized_responses::WithheldCode;
 
         let inner_error = MegolmError::MissingRoomKey(Some(WithheldCode::Unverified));
 

bindings/matrix-sdk-crypto-ffi/src/error.rs

@@ -2656,7 +2656,7 @@ mod tests {
             .unwrap(),
             UnableToDecryptInfo {
                 session_id: Some("".to_owned()),
-                reason: UnableToDecryptReason::MissingMegolmSession,
+                reason: UnableToDecryptReason::MissingMegolmSession { withheld_code: None },
             },
         )
     }

crates/matrix-sdk-base/src/sliding_sync/mod.rs

@@ -17,7 +17,10 @@ use std::{collections::BTreeMap, fmt};
 use ruma::{
     events::{AnyMessageLikeEvent, AnySyncTimelineEvent, AnyTimelineEvent},
     push::Action,
-    serde::{JsonObject, Raw},
+    serde::{
+        AsRefStr, AsStrAsRefStr, DebugAsRefStr, DeserializeFromCowStr, FromString, JsonObject, Raw,
+        SerializeAsRefStr,
+    },
     DeviceKeyAlgorithm, OwnedDeviceId, OwnedEventId, OwnedUserId,
 };
 use serde::{Deserialize, Serialize};
@@ -666,14 +669,32 @@ pub struct UnableToDecryptInfo {
     pub session_id: Option<String>,
 
     /// Reason code for the decryption failure
-    #[serde(default = "unknown_utd_reason")]
+    #[serde(default = "unknown_utd_reason", deserialize_with = "deserialize_utd_reason")]
     pub reason: UnableToDecryptReason,
 }
 
 fn unknown_utd_reason() -> UnableToDecryptReason {
     UnableToDecryptReason::Unknown
 }
 
+/// Provides basic backward compatibility for deserializing older serialized
+/// `UnableToDecryptReason` values.
+pub fn deserialize_utd_reason<'de, D>(d: D) -> Result<UnableToDecryptReason, D::Error>
+where
+    D: serde::Deserializer<'de>,
+{
+    // Start by deserializing as to an untyped JSON value.
+    let v: serde_json::Value = Deserialize::deserialize(d)?;
+    // Backwards compatibility: `MissingMegolmSession` used to be stored without the
+    // withheld code.
+    if v.as_str().is_some_and(|s| s == "MissingMegolmSession") {
+        return Ok(UnableToDecryptReason::MissingMegolmSession { withheld_code: None });
+    }
+    // Otherwise, use the derived deserialize impl to turn the JSON into a
+    // UnableToDecryptReason
+    serde_json::from_value::<UnableToDecryptReason>(v).map_err(serde::de::Error::custom)
+}
+
 /// Reason code for a decryption failure
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
 pub enum UnableToDecryptReason {
@@ -689,9 +710,11 @@ 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 {
+        /// If the key was withheld on purpose, the associated code. `None`
+        /// means no withheld code was received.
+        withheld_code: Option<WithheldCode>,
+    },
 
     /// Decryption failed because, while we have the megolm session that was
     /// used to encrypt the message, it is ratcheted too far forward.
@@ -723,7 +746,86 @@ 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)
+        // In case of MissingMegolmSession with a withheld code we return false here
+        // given that this API is used to decide if waiting a bit will help.
+        matches!(
+            self,
+            Self::MissingMegolmSession { withheld_code: None } | Self::UnknownMegolmMessageIndex
+        )
+    }
+}
+
+/// A machine-readable code for why a Megolm key was not sent.
+///
+/// Normally sent as the payload of an [`m.room_key.withheld`](https://spec.matrix.org/v1.12/client-server-api/#mroom_keywithheld) to-device message.
+#[derive(
+    Clone,
+    PartialEq,
+    Eq,
+    Hash,
+    AsStrAsRefStr,
+    AsRefStr,
+    FromString,
+    DebugAsRefStr,
+    SerializeAsRefStr,
+    DeserializeFromCowStr,
+)]
+pub enum WithheldCode {
+    /// the user/device was blacklisted.
+    #[ruma_enum(rename = "m.blacklisted")]
+    Blacklisted,
+
+    /// the user/devices is unverified.
+    #[ruma_enum(rename = "m.unverified")]
+    Unverified,
+
+    /// The user/device is not allowed have the key. For example, this would
+    /// usually be sent in response to a key request if the user was not in
+    /// the room when the message was sent.
+    #[ruma_enum(rename = "m.unauthorised")]
+    Unauthorised,
+
+    /// Sent in reply to a key request if the device that the key is requested
+    /// from does not have the requested key.
+    #[ruma_enum(rename = "m.unavailable")]
+    Unavailable,
+
+    /// An olm session could not be established.
+    /// This may happen, for example, if the sender was unable to obtain a
+    /// one-time key from the recipient.
+    #[ruma_enum(rename = "m.no_olm")]
+    NoOlm,
+
+    #[doc(hidden)]
+    _Custom(PrivOwnedStr),
+}
+
+impl fmt::Display for WithheldCode {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
+        let string = match self {
+            WithheldCode::Blacklisted => "The sender has blocked you.",
+            WithheldCode::Unverified => "The sender has disabled encrypting to unverified devices.",
+            WithheldCode::Unauthorised => "You are not authorised to read the message.",
+            WithheldCode::Unavailable => "The requested key was not found.",
+            WithheldCode::NoOlm => "Unable to establish a secure channel.",
+            _ => self.as_str(),
+        };
+
+        f.write_str(string)
+    }
+}
+
+// The Ruma macro expects the type to have this name.
+// The payload is counter intuitively made public in order to avoid having
+// multiple copies of this struct.
+#[doc(hidden)]
+#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct PrivOwnedStr(pub Box<str>);
+
+#[cfg(not(tarpaulin_include))]
+impl fmt::Debug for PrivOwnedStr {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.0.fmt(f)
     }
 }
 
@@ -817,7 +919,7 @@ mod tests {
     use super::{
         AlgorithmInfo, DecryptedRoomEvent, EncryptionInfo, SyncTimelineEvent, TimelineEvent,
         TimelineEventKind, UnableToDecryptInfo, UnableToDecryptReason, UnsignedDecryptionResult,
-        UnsignedEventLocation, VerificationState,
+        UnsignedEventLocation, VerificationState, WithheldCode,
     };
     use crate::deserialized_responses::{DeviceLinkProblem, VerificationLevel};
 
@@ -1038,4 +1140,111 @@ mod tests {
             });
         });
     }
+
+    #[test]
+    fn sync_timeline_event_deserialisation_migration_for_withheld() {
+        // Old serialized version was
+        //    "utd_info": {
+        //         "reason": "MissingMegolmSession",
+        //         "session_id": "session000"
+        //       }
+
+        // The new version would be
+        //      "utd_info": {
+        //         "reason": {
+        //           "MissingMegolmSession": {
+        //              "withheld_code": null
+        //           }
+        //         },
+        //         "session_id": "session000"
+        //       }
+
+        let serialized = json!({
+             "kind": {
+                "UnableToDecrypt": {
+                  "event": {
+                    "content": {
+                      "algorithm": "m.megolm.v1.aes-sha2",
+                      "ciphertext": "AwgAEoABzL1JYhqhjW9jXrlT3M6H8mJ4qffYtOQOnPuAPNxsuG20oiD/Fnpv6jnQGhU6YbV9pNM+1mRnTvxW3CbWOPjLKqCWTJTc7Q0vDEVtYePg38ncXNcwMmfhgnNAoW9S7vNs8C003x3yUl6NeZ8bH+ci870BZL+kWM/lMl10tn6U7snNmSjnE3ckvRdO+11/R4//5VzFQpZdf4j036lNSls/WIiI67Fk9iFpinz9xdRVWJFVdrAiPFwb8L5xRZ8aX+e2JDMlc1eW8gk",
+                      "device_id": "SKCGPNUWAU",
+                      "sender_key": "Gim/c7uQdSXyrrUbmUOrBT6sMC0gO7QSLmOK6B7NOm0",
+                      "session_id": "hgLyeSqXfb8vc5AjQLsg6TSHVu0HJ7HZ4B6jgMvxkrs"
+                    },
+                    "event_id": "$xxxxx:example.org",
+                    "origin_server_ts": 2189,
+                    "room_id": "!someroom:example.com",
+                    "sender": "@carl:example.com",
+                    "type": "m.room.message"
+                  },
+                  "utd_info": {
+                    "reason": "MissingMegolmSession",
+                    "session_id": "session000"
+                  }
+                }
+              }
+        });
+
+        let result = serde_json::from_value(serialized);
+        assert!(result.is_ok());
+
+        // should have migrated to the new format
+        let event: SyncTimelineEvent = result.unwrap();
+        assert_matches!(
+            event.kind,
+            TimelineEventKind::UnableToDecrypt { utd_info, .. }=> {
+                assert_matches!(
+                    utd_info.reason,
+                    UnableToDecryptReason::MissingMegolmSession { withheld_code: None }
+                );
+            }
+        )
+    }
+
+    #[test]
+    fn unable_to_decrypt_info_migration_for_withheld() {
+        let old_format = json!({
+            "reason": "MissingMegolmSession",
+            "session_id": "session000"
+        });
+
+        let deserialized = serde_json::from_value::<UnableToDecryptInfo>(old_format).unwrap();
+        let session_id = Some("session000".to_owned());
+
+        assert_eq!(deserialized.session_id, session_id);
+        assert_eq!(
+            deserialized.reason,
+            UnableToDecryptReason::MissingMegolmSession { withheld_code: None },
+        );
+
+        let new_format = json!({
+             "session_id": "session000",
+              "reason": {
+                "MissingMegolmSession": {
+                  "withheld_code": null
+                }
+              }
+        });
+
+        let deserialized = serde_json::from_value::<UnableToDecryptInfo>(new_format).unwrap();
+
+        assert_eq!(
+            deserialized.reason,
+            UnableToDecryptReason::MissingMegolmSession { withheld_code: None },
+        );
+        assert_eq!(deserialized.session_id, session_id);
+    }
+
+    #[test]
+    fn unable_to_decrypt_reason_is_missing_room_key() {
+        let reason = UnableToDecryptReason::MissingMegolmSession { withheld_code: None };
+        assert!(reason.is_missing_room_key());
+
+        let reason = UnableToDecryptReason::MissingMegolmSession {
+            withheld_code: Some(WithheldCode::Blacklisted),
+        };
+        assert!(!reason.is_missing_room_key());
+
+        let reason = UnableToDecryptReason::UnknownMegolmMessageIndex;
+        assert!(reason.is_missing_room_key());
+    }
 }

crates/matrix-sdk-common/src/deserialized_responses.rs

@@ -6,6 +6,11 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased] - ReleaseDate
 
+- Added new `UtdCause` variants `WithheldForUnverifiedOrInsecureDevice` and `WithheldBySender`.
+  These variants provide clearer categorization for expected Unable-To-Decrypt (UTD) errors 
+  when the sender either did not wish to share or was unable to share the room_key.
+  ([#4305](https://github.com/matrix-org/matrix-rust-sdk/pull/4305))
+
 ## [0.8.0] - 2024-11-19
 
 ### Features

crates/matrix-sdk-crypto/CHANGELOG.md

@@ -14,18 +14,15 @@
 
 use std::collections::BTreeMap;
 
-use matrix_sdk_common::deserialized_responses::VerificationLevel;
+use matrix_sdk_common::deserialized_responses::{VerificationLevel, WithheldCode};
 use ruma::{CanonicalJsonError, IdParseError, OwnedDeviceId, OwnedRoomId, OwnedUserId};
 use serde::{ser::SerializeMap, Serializer};
 use serde_json::Error as SerdeError;
 use thiserror::Error;
 use vodozemac::{Curve25519PublicKey, Ed25519PublicKey};
 
 use super::store::CryptoStoreError;
-use crate::{
-    olm::SessionExportError,
-    types::{events::room_key_withheld::WithheldCode, SignedKey},
-};
+use crate::{olm::SessionExportError, types::SignedKey};
 #[cfg(doc)]
 use crate::{CollectStrategy, Device, LocalTrust, OtherUserIdentity};
 

crates/matrix-sdk-crypto/src/error.rs

@@ -21,6 +21,7 @@ use std::{
     },
 };
 
+use matrix_sdk_common::deserialized_responses::WithheldCode;
 use ruma::{
     api::client::keys::upload_signatures::v3::Request as SignatureUploadRequest,
     events::{key::verification::VerificationMethod, AnyToDeviceEventContent},
@@ -48,8 +49,7 @@ use crate::{
     types::{
         events::{
             forwarded_room_key::ForwardedRoomKeyContent,
-            room::encrypted::ToDeviceEncryptedEventContent, room_key_withheld::WithheldCode,
-            EventType,
+            room::encrypted::ToDeviceEncryptedEventContent, EventType,
         },
         requests::{OutgoingVerificationRequest, ToDeviceRequest},
         DeviceKey, DeviceKeys, EventEncryptionAlgorithm, Signatures, SignedKey,

crates/matrix-sdk-crypto/src/identities/device.rs

@@ -2582,7 +2582,9 @@ fn megolm_error_to_utd_info(
     let reason = match error {
         EventError(_) => UnableToDecryptReason::MalformedEncryptedEvent,
         Decode(_) => UnableToDecryptReason::MalformedEncryptedEvent,
-        MissingRoomKey(_) => UnableToDecryptReason::MissingMegolmSession,
+        MissingRoomKey(maybe_withheld) => {
+            UnableToDecryptReason::MissingMegolmSession { withheld_code: maybe_withheld }
+        }
         Decryption(DecryptionError::UnknownMessageIndex(_, _)) => {
             UnableToDecryptReason::UnknownMegolmMessageIndex
         }