crypto: rewrite check_sender_trust_requirement to use VerificationState (#5044)

There are two reasons for this.

Firstly. we've already done a bunch of work to map `SenderData` into a
`VerificationState`, and the decision tree from `VerificationState` to
allow/reject is simpler than going from `SenderData`, even if we have to
fudge it a bit to get the "legacy" flag. (Note that it allows us to get
rid of an `unreachable!` panic.)

Secondly, `VerficationState` represents the state of an *event*, whereas
`SenderData` is about the session as a whole. A session can be fine,
whilst events (claiming to be) encrypted with it can be suspect. What we
want here is to check a specific message. Currently, this doesn't make
any functional difference, but conceptually it's cleaner to check the
`VerificationState`.

Note that there are a bunch of tests for this method in
`matrix-sdk-crypto/src/machine/tests/decryption_verification_state.rs`,
called `test_decryption_trust_requirement`.

Author: richvdh

crates/matrix-sdk-crypto/src/machine/mod.rs

@@ -57,7 +57,7 @@ use tracing::trace;
 use tracing::{
     debug, error,
     field::{debug, display},
-    info, instrument, warn, Span,
+    info, instrument, trace, warn, Span,
 };
 use vodozemac::{
     megolm::{DecryptionError, SessionOrdering},
@@ -1834,52 +1834,85 @@ impl OlmMachine {
         }
     }
 
-    /// Check that the sender of a Megolm session satisfies the trust
+    /// Check that a Megolm event satisfies the sender trust
     /// requirement from the decryption settings.
+    ///
+    /// If the requirement is not satisfied, returns
+    /// [`MegolmError::SenderIdentityNotTrusted`].
     fn check_sender_trust_requirement(
         &self,
         session: &InboundGroupSession,
         encryption_info: &EncryptionInfo,
         trust_requirement: &TrustRequirement,
     ) -> MegolmResult<()> {
-        /// Get the error from the encryption information.
-        fn encryption_info_to_error(encryption_info: &EncryptionInfo) -> MegolmResult<()> {
-            // When this is called, the verification state *must* be unverified,
-            // otherwise the sender_data would have been SenderVerified
-            let VerificationState::Unverified(verification_level) =
-                &encryption_info.verification_state
-            else {
-                unreachable!("inconsistent verification state");
-            };
-            Err(MegolmError::SenderIdentityNotTrusted(verification_level.clone()))
-        }
+        trace!(
+            verification_state = ?encryption_info.verification_state,
+            ?trust_requirement, "check_sender_trust_requirement",
+        );
 
-        match trust_requirement {
-            TrustRequirement::Untrusted => Ok(()),
-
-            TrustRequirement::CrossSignedOrLegacy => match &session.sender_data {
-                // Reject if the sender was previously verified, but changed
-                // their identity and is not verified any more.
-                SenderData::VerificationViolation(..) => Err(
-                    MegolmError::SenderIdentityNotTrusted(VerificationLevel::VerificationViolation),
-                ),
-                SenderData::SenderUnverified(..) => Ok(()),
-                SenderData::SenderVerified(..) => Ok(()),
-                SenderData::DeviceInfo { legacy_session: true, .. } => Ok(()),
-                SenderData::UnknownDevice { legacy_session: true, .. } => Ok(()),
-                _ => encryption_info_to_error(encryption_info),
-            },
+        // VerificationState::Verified is acceptable for all TrustRequirement levels, so
+        // let's get that out of the way
+        let verification_level = match &encryption_info.verification_state {
+            VerificationState::Verified => return Ok(()),
+            VerificationState::Unverified(verification_level) => verification_level,
+        };
+
+        let ok = match trust_requirement {
+            TrustRequirement::Untrusted => true,
+
+            TrustRequirement::CrossSignedOrLegacy => {
+                // `VerificationLevel::UnsignedDevice` and `VerificationLevel::None` correspond
+                // to `SenderData::DeviceInfo` and `SenderData::UnknownDevice`
+                // respectively, and those cases may be acceptable if the reason
+                // for the lack of data is that the sessions were established
+                // before we started collecting SenderData.
+                let legacy_session = match session.sender_data {
+                    SenderData::DeviceInfo { legacy_session, .. } => legacy_session,
+                    SenderData::UnknownDevice { legacy_session, .. } => legacy_session,
+                    _ => false,
+                };
 
-            TrustRequirement::CrossSigned => match &session.sender_data {
-                // Reject if the sender was previously verified, but changed
-                // their identity and is not verified any more.
-                SenderData::VerificationViolation(..) => Err(
-                    MegolmError::SenderIdentityNotTrusted(VerificationLevel::VerificationViolation),
-                ),
-                SenderData::SenderUnverified(..) => Ok(()),
-                SenderData::SenderVerified(..) => Ok(()),
-                _ => encryption_info_to_error(encryption_info),
+                // In the CrossSignedOrLegacy case the following rules apply:
+                //
+                // 1. Identities we have not yet verified can be decrypted regardless of the
+                //    legacy state of the session.
+                // 2. Devices that aren't signed by the owning identity of the device can only
+                //    be decrypted if it's a legacy session.
+                // 3. If we have no information about the device, we should only decrypt if it's
+                //    a legacy session.
+                // 4. Anything else, should throw an error.
+                match (verification_level, legacy_session) {
+                    // Case 1
+                    (VerificationLevel::UnverifiedIdentity, _) => true,
+
+                    // Case 2
+                    (VerificationLevel::UnsignedDevice, true) => true,
+
+                    // Case 3
+                    (VerificationLevel::None(_), true) => true,
+
+                    // Case 4
+                    (VerificationLevel::VerificationViolation, _)
+                    | (VerificationLevel::UnsignedDevice, false)
+                    | (VerificationLevel::None(_), false) => false,
+                }
+            }
+
+            // If cross-signing of identities is required, the only acceptable unverified case
+            // is when the identity is signed but not yet verified by us.
+            TrustRequirement::CrossSigned => match verification_level {
+                VerificationLevel::UnverifiedIdentity => true,
+
+                VerificationLevel::VerificationViolation
+                | VerificationLevel::UnsignedDevice
+                | VerificationLevel::None(_) => false,
             },
+        };
+
+        if ok {
+            Ok(())
+        } else {
+            Err(MegolmError::SenderIdentityNotTrusted(verification_level.clone()))
         }
     }