fix(crypto): Fixed a bug where room keys would be rotated unecessarily

Previously, `is_session_overshared_for_user` did not take into account
that `shared_with_set` also contains withheld device IDs who explicitly
have never received the session keys. This would lead to it mistakenly
determining oversharing for those devices for every event being sent in
the presence of blacklisted/withheld devices in the room, and rotating
the group session accordingly.

The fix is to correctly exclude devices with `ShareInfo::Withheld` from
the enumeration.

Signed-off-by: Niklas Baumstark [email protected]

Author: procr1337

crates/matrix-sdk-crypto/CHANGELOG.md

@@ -20,6 +20,10 @@ All notable changes to this project will be documented in this file.
   (as per [MSC4147](https://github.com/matrix-org/matrix-spec-proposals/pull/4147)).
   ([#4922](https://github.com/matrix-org/matrix-rust-sdk/pull/4922))
 
+- Fix bug which caused room keys to be unnecessarily rotated on every send in the
+  presence of blacklisted/withheld devices in the room.
+  ([#4954](https://github.com/matrix-org/matrix-rust-sdk/pull/4954))
+
 ## [0.11.0] - 2025-04-11
 
 ### Features

crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs

@@ -27,6 +27,7 @@ use tracing::{debug, instrument, trace};
 use super::OutboundGroupSession;
 use crate::{
     error::{OlmResult, SessionRecipientCollectionError},
+    olm::ShareInfo,
     store::Store,
     DeviceData, EncryptionSettings, LocalTrust, OlmError, OwnUserIdentityData, UserIdentityData,
 };
@@ -433,7 +434,11 @@ fn is_session_overshared_for_user(
     };
 
     // Devices that received this session
-    let shared: BTreeSet<&DeviceId> = shared.keys().map(|d| d.as_ref()).collect();
+    let shared: BTreeSet<&DeviceId> = shared
+        .iter()
+        .filter(|(_, info)| matches!(info, ShareInfo::Shared(_)))
+        .map(|(d, _)| d.as_ref())
+        .collect();
 
     // The set difference between
     //
@@ -2095,9 +2100,55 @@ mod tests {
         let machine = test_machine().await;
         import_known_users_to_test_machine(&machine).await;
 
-        let fake_room_id = room_id!("!roomid:localhost");
         let encryption_settings = all_devices_strategy_settings();
+        let group_session = create_test_outbound_group_session(&machine, &encryption_settings);
+        let sender_key = machine.identity_keys().curve25519;
+
+        group_session
+            .mark_shared_with(
+                KeyDistributionTestData::dan_id(),
+                KeyDistributionTestData::dan_signed_device_id(),
+                sender_key,
+            )
+            .await;
+        group_session
+            .mark_shared_with(
+                KeyDistributionTestData::dan_id(),
+                KeyDistributionTestData::dan_unsigned_device_id(),
+                sender_key,
+            )
+            .await;
+
+        // Try to share again after dan has removed one of his devices
+        let keys_query = KeyDistributionTestData::dan_keys_query_response_device_loggedout();
+        machine.mark_request_as_sent(&TransactionId::new(), &keys_query).await.unwrap();
 
+        // share again
+        let share_result = collect_session_recipients(
+            machine.store(),
+            vec![KeyDistributionTestData::dan_id()].into_iter(),
+            &encryption_settings,
+            &group_session,
+        )
+        .await
+        .unwrap();
+
+        assert!(share_result.should_rotate);
+    }
+
+    /// Test that the session is not rotated if a devices is removed
+    /// but was already withheld from receiving the session.
+    #[async_test]
+    async fn test_should_not_rotate_if_keys_were_withheld() {
+        let machine = test_machine().await;
+        import_known_users_to_test_machine(&machine).await;
+
+        let encryption_settings = all_devices_strategy_settings();
+        let group_session = create_test_outbound_group_session(&machine, &encryption_settings);
+        let fake_room_id = group_session.room_id();
+
+        // Because we don't have Olm sessions initialized, this will contain
+        // withheld requests for both of Dan's devices
         let requests = machine
             .share_room_key(
                 fake_room_id,
@@ -2115,13 +2166,11 @@ mod tests {
                 .await
                 .unwrap();
         }
+
         // Try to share again after dan has removed one of his devices
         let keys_query = KeyDistributionTestData::dan_keys_query_response_device_loggedout();
-        let txn_id = TransactionId::new();
-        machine.mark_request_as_sent(&txn_id, &keys_query).await.unwrap();
+        machine.mark_request_as_sent(&TransactionId::new(), &keys_query).await.unwrap();
 
-        let group_session =
-            machine.store().get_outbound_group_session(fake_room_id).await.unwrap().unwrap();
         // share again
         let share_result = collect_session_recipients(
             machine.store(),
@@ -2132,7 +2181,7 @@ mod tests {
         .await
         .unwrap();
 
-        assert!(share_result.should_rotate);
+        assert!(!share_result.should_rotate);
     }
 
     /// Common setup for tests which require a verified user to have unsigned