From fb63c8dfbcd2301fa1aa2dc037338d92c791452b Mon Sep 17 00:00:00 2001
From: Andy Balaam <andy.balaam@matrix.org>
Date: Thu, 22 Aug 2024 11:22:25 +0100
Subject: [PATCH] crypto: Move should_recalculate_sender_data to OlmMachine

---
 crates/matrix-sdk-crypto/src/machine/mod.rs   | 26 +++++++++++---
 .../src/olm/group_sessions/sender_data.rs     | 35 -------------------
 2 files changed, 22 insertions(+), 39 deletions(-)

diff --git a/crates/matrix-sdk-crypto/src/machine/mod.rs b/crates/matrix-sdk-crypto/src/machine/mod.rs
index e0891d3b0..b2c782dbe 100644
--- a/crates/matrix-sdk-crypto/src/machine/mod.rs
+++ b/crates/matrix-sdk-crypto/src/machine/mod.rs
@@ -1439,10 +1439,26 @@ impl OlmMachine {
         session: &InboundGroupSession,
         sender: &UserId,
     ) -> MegolmResult<(VerificationState, Option<OwnedDeviceId>)> {
-        let sender_data = if session.sender_data.is_known() {
-            // The existing sender_data is known, so there is no need to recalculate it.
-            session.sender_data.clone()
-        } else {
+        /// Whether we should recalculate the Megolm sender's data, given the
+        /// current sender data. We only want to recalculate if it might
+        /// increase trust and allow us to decrypt messages that we
+        /// otherwise might refuse to decrypt.
+        ///
+        /// We recalculate for all states except:
+        ///
+        /// - SenderUnverified: the sender is trusted enough that we will
+        ///   decrypt their messages in all cases, or
+        /// - SenderVerified: the sender is the most trusted they can be.
+        fn should_recalculate_sender_data(sender_data: &SenderData) -> bool {
+            matches!(
+                sender_data,
+                SenderData::UnknownDevice { .. }
+                    | SenderData::DeviceInfo { .. }
+                    | SenderData::SenderUnverifiedButPreviouslyVerified { .. }
+            )
+        }
+
+        let sender_data = if should_recalculate_sender_data(&session.sender_data) {
             // The session is not sure of the sender yet. Calculate it.
             let calculated_sender_data = SenderDataFinder::find_using_curve_key(
                 self.store(),
@@ -1465,6 +1481,8 @@ impl OlmMachine {
                 // No - use the existing data.
                 session.sender_data.clone()
             }
+        } else {
+            session.sender_data.clone()
         };
 
         Ok(sender_data_to_verification_state(sender_data, session.has_been_imported()))
diff --git a/crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data.rs b/crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data.rs
index 5db29afd3..989eb0fbb 100644
--- a/crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data.rs
+++ b/crates/matrix-sdk-crypto/src/olm/group_sessions/sender_data.rs
@@ -166,13 +166,6 @@ impl SenderData {
         Self::UnknownDevice { legacy_session: true, owner_check_failed: false }
     }
 
-    /// Returns true if this is SenderKnown and `master_key_verified` is true.
-    pub(crate) fn is_known(&self) -> bool {
-        matches!(self, SenderData::SenderUnverifiedButPreviouslyVerified { .. })
-            || matches!(self, SenderData::SenderUnverified { .. })
-            || matches!(self, SenderData::SenderVerified { .. })
-    }
-
     /// Returns `Greater` if this `SenderData` represents a greater level of
     /// trust than the supplied one, `Equal` if they have the same level, and
     /// `Less` if the supplied one has a greater level of trust.
@@ -356,34 +349,6 @@ mod tests {
         assert_let!(SenderData::SenderVerified { .. } = end);
     }
 
-    #[test]
-    fn known_session_is_known() {
-        let user_id = user_id!("@u:s.co");
-        let device_id = device_id!("DEV");
-        let master_key =
-            Ed25519PublicKey::from_base64("2/5LWJMow5zhJqakV88SIc7q/1pa8fmkfgAzx72w9G4").unwrap();
-
-        assert!(!SenderData::unknown().is_known());
-        assert!(SenderData::sender_previously_verified(user_id, device_id, master_key).is_known());
-        assert!(SenderData::sender_unverified(user_id, device_id, master_key).is_known());
-        assert!(SenderData::sender_verified(user_id, device_id, master_key).is_known());
-    }
-
-    #[test]
-    fn unknown_and_device_sessions_are_not_known() {
-        // Anything that is not SenderKnown is not know and verified.
-        assert!(!SenderData::unknown().is_known());
-
-        let device_keys = DeviceKeys::new(
-            owned_user_id!("@u:s.co"),
-            owned_device_id!("DEV"),
-            Vec::new(),
-            BTreeMap::new(),
-            Signatures::new(),
-        );
-        assert!(!SenderData::device_info(device_keys).is_known());
-    }
-
     #[test]
     fn equal_sessions_have_same_trust_level() {
         let unknown = SenderData::unknown();