crypto: Implemented sharing room keys for past messages (MSC3061)

Cargo.toml

@@ -36,7 +36,7 @@ futures-executor = "0.3.21"
 futures-util = { version = "0.3.26", default-features = false, features = ["alloc"] }
 http = "0.2.6"
 itertools = "0.11.0"
-ruma = { version = "0.9.0", features = ["client-api-c", "compat-upload-signatures", "compat-user-id", "compat-arbitrary-length-ids"] }
+ruma = { version = "0.9.1", features = ["client-api-c", "compat-upload-signatures", "compat-user-id", "compat-arbitrary-length-ids", "unstable-msc3061"] }
 ruma-common = "0.12.0"
 once_cell = "1.16.0"
 serde = "1.0.151"

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

@@ -749,6 +749,38 @@ impl OlmMachine {
         Ok(requests.into_iter().map(|r| r.as_ref().into()).collect())
     }
 
+    /// Shares historical room keys used in previous sessions with the list of
+    /// users for the given room.
+    ///
+    /// After the request was sent out and a successful response was received
+    /// the response body should be passed back to the state machine using the
+    /// [mark_request_as_sent()](Self::mark_request_as_sent) method.
+    ///
+    /// This method should be called after users have been invited to the room.
+    ///
+    /// # Arguments
+    ///
+    /// * `room_id` - The unique id of the room of which previous room keys
+    /// will be sent out.
+    ///
+    /// * `users` - The list of users which are considered to be members of the
+    /// room and should receive previous room keys.
+    pub fn share_room_history_keys(
+        &self,
+        room_id: String,
+        users: Vec<String>,
+    ) -> Result<Vec<Request>, CryptoStoreError> {
+        let users: Vec<OwnedUserId> =
+            users.into_iter().filter_map(|u| UserId::parse(u).ok()).collect();
+
+        let room_id = RoomId::parse(room_id)?;
+        let requests = self.runtime.block_on(
+            self.inner.share_room_history_keys(&room_id, users.iter().map(Deref::deref)),
+        )?;
+
+        Ok(requests.into_iter().map(|r| r.as_ref().into()).collect())
+    }
+
     /// Encrypt the given event with the given type and content for the given
     /// room.
     ///

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

@@ -31,8 +31,11 @@ use std::{
 
 use ruma::{
     api::client::keys::claim_keys::v3::Request as KeysClaimRequest,
-    events::secret::request::{
-        RequestAction, SecretName, ToDeviceSecretRequestEvent as SecretRequestEvent,
+    events::{
+        room::history_visibility::HistoryVisibility,
+        secret::request::{
+            RequestAction, SecretName, ToDeviceSecretRequestEvent as SecretRequestEvent,
+        },
     },
     DeviceId, DeviceKeyAlgorithm, OwnedDeviceId, OwnedTransactionId, OwnedUserId, RoomId,
     TransactionId, UserId,
@@ -73,6 +76,12 @@ pub(crate) struct GossipMachineInner {
     wait_queue: WaitQueue,
     users_for_key_claim: Arc<StdRwLock<BTreeMap<OwnedUserId, BTreeSet<OwnedDeviceId>>>>,
     room_key_forwarding_enabled: AtomicBool,
+    /// A map from the device id to a set of session_id strings for previous
+    /// `InboundGroupSession`s. Used when an olm session is not established
+    /// with a user's device on invite, in which specific keys are flagged to
+    /// allow for sharing with the device when the olm session is established.
+    #[cfg(feature = "automatic-room-key-forwarding")]
+    pending_room_key_forwarding_devices: Arc<StdRwLock<BTreeMap<OwnedDeviceId, BTreeSet<String>>>>,
 }
 
 impl GossipMachine {
@@ -94,6 +103,8 @@ impl GossipMachine {
                 wait_queue: WaitQueue::new(),
                 users_for_key_claim,
                 room_key_forwarding_enabled,
+                #[cfg(feature = "automatic-room-key-forwarding")]
+                pending_room_key_forwarding_devices: Default::default(),
             }),
         }
     }
@@ -175,8 +186,14 @@ impl GossipMachine {
         // out using a wildcard instead of a specific device as a recipient.
         //
         // Check if we're the sender of this request event and ignore it if
-        // so.
-        if event.sender() == self.user_id() && event.requesting_device_id() == self.device_id() {
+        // so. However if the request is a room key request, we should allow it
+        // since one of our other devices could be forwarding a room key to us.
+        // This could happen when one device is unable to decrypt an event and
+        // requests the key from another device.
+        if event.sender() == self.user_id()
+            && event.requesting_device_id() == self.device_id()
+            && matches!(event, RequestEvent::Secret(_))
+        {
             trace!("Received a secret request event from ourselves, ignoring")
         } else {
             let request_info = event.to_request_info();
@@ -345,13 +362,15 @@ impl GossipMachine {
     /// `/keys/claim` request to be sent out and retry once the 1-to-1 Olm
     /// session has been established.
     #[cfg(feature = "automatic-room-key-forwarding")]
-    async fn try_to_forward_room_key(
+    pub async fn try_to_forward_room_key(
         &self,
         event: &RoomKeyRequestEvent,
         device: Device,
         session: &InboundGroupSession,
         message_index: Option<u32>,
+        update_outbound_devices: bool,
     ) -> OlmResult<Option<Session>> {
+        use crate::olm::{ShareInfo, ShareState};
         info!(?message_index, "Serving a room key request",);
 
         match self.forward_room_key(session, &device, message_index).await {
@@ -360,6 +379,49 @@ impl GossipMachine {
                 info!(
                     "Key request is missing an Olm session, putting the request in the wait queue",
                 );
+
+                // In the case when we are inviting a user that has a device
+                // with a missing session, we need to ensure that we allow the
+                // forwarded key to be in the outbound session's
+                // `shared_with_set` when the olm session is established
+                if update_outbound_devices {
+                    self.inner
+                        .pending_room_key_forwarding_devices
+                        .write()
+                        .unwrap()
+                        .entry(device.device_id().to_owned())
+                        .or_default()
+                        .insert(session.session_id().to_string());
+
+                    let outbound_session =
+                        self.inner.outbound_group_sessions.get_or_load(session.room_id()).await;
+
+                    if let Some(outbound) = outbound_session {
+                        match outbound.is_shared_with(&device) {
+                            ShareState::NotShared => {
+                                let share_info = ShareInfo::new_shared(
+                                    device.curve25519_key().unwrap(),
+                                    outbound.message_index().await,
+                                );
+                                outbound
+                                    .shared_with_set
+                                    .write()
+                                    .unwrap()
+                                    .entry(device.user_id().to_owned())
+                                    .or_default()
+                                    .insert(device.device_id().to_owned(), share_info);
+
+                                self.inner.outbound_group_sessions.insert(outbound.clone());
+
+                                let mut changes = Changes::default();
+                                changes.outbound_group_sessions.push(outbound);
+                                self.inner.store.save_changes(changes).await?;
+                            }
+                            ShareState::SharedButChangedSenderKey | ShareState::Shared(_) => {}
+                        }
+                    }
+                }
+
                 self.handle_key_share_without_session(device, event.to_owned().into());
 
                 Ok(None)
@@ -397,7 +459,7 @@ impl GossipMachine {
 
         match self.should_share_key(&device, session).await {
             Ok(message_index) => {
-                self.try_to_forward_room_key(event, device, session, message_index).await
+                self.try_to_forward_room_key(event, device, session, message_index, false).await
             }
             Err(e) => {
                 if let KeyForwardDecision::ChangedSenderKey = e {
@@ -573,11 +635,40 @@ impl GossipMachine {
         use super::KeyForwardDecision;
         use crate::olm::ShareState;
 
-        let outbound_session = self
-            .inner
-            .outbound_group_sessions
-            .get_with_id(session.room_id(), session.session_id())
-            .await;
+        // We don't store prior `OutboundGroupSession` objects in the store.
+        // Because of this, we check if the key that is about to be shared has
+        // been flagged as a forwarded key from a result of a user invite.
+        // If so, we must use the current outbound session since it has the
+        // latest `shared_with_set` device map.
+        let mut use_current_session = false;
+        {
+            let mut pending_device_keys =
+                self.inner.pending_room_key_forwarding_devices.write().unwrap();
+
+            if pending_device_keys
+                .get(device.device_id())
+                .is_some_and(|s| s.contains(session.session_id()))
+            {
+                pending_device_keys.entry(device.device_id().to_owned()).and_modify(|s| {
+                    s.remove(session.session_id());
+                });
+
+                if pending_device_keys.get(device.device_id()).is_some_and(|s| s.is_empty()) {
+                    pending_device_keys.remove(device.device_id());
+                }
+
+                use_current_session = true;
+            }
+        }
+
+        let outbound_session = if use_current_session {
+            self.inner.outbound_group_sessions.get_or_load(session.room_id()).await
+        } else {
+            self.inner
+                .outbound_group_sessions
+                .get_with_id(session.room_id(), session.session_id())
+                .await
+        };
 
         // If this is our own, verified device, we share the entire session from the
         // earliest known index.
@@ -589,7 +680,22 @@ impl GossipMachine {
         // information is recorded there.
         } else if let Some(outbound) = outbound_session {
             match outbound.is_shared_with(device) {
-                ShareState::Shared(message_index) => Ok(Some(message_index)),
+                ShareState::Shared(message_index) => {
+                    if let Some(history) = session.history_visibility.as_ref() {
+                        match history {
+                            HistoryVisibility::Shared | HistoryVisibility::WorldReadable => {
+                                Ok(None)
+                            }
+
+                            HistoryVisibility::Invited
+                            | HistoryVisibility::Joined
+                            | HistoryVisibility::_Custom(_) => Ok(Some(message_index)),
+                            _ => Ok(Some(message_index)),
+                        }
+                    } else {
+                        Ok(Some(message_index))
+                    }
+                }
                 ShareState::SharedButChangedSenderKey => Err(KeyForwardDecision::ChangedSenderKey),
                 ShareState::NotShared => Err(KeyForwardDecision::OutboundSessionNotShared),
             }
@@ -977,6 +1083,39 @@ impl GossipMachine {
         let Some(request) =
             self.inner.store.get_secret_request_by_info(&info.clone().into()).await?
         else {
+            // We did not request this key, so determine if this key was
+            // forwarded as a result from a room invite
+            let session = match &event.content {
+                ForwardedRoomKeyContent::MegolmV1AesSha2(c) => {
+                    InboundGroupSession::from(c.as_ref())
+                }
+                #[cfg(feature = "experimental-algorithms")]
+                ForwardedRoomKeyContent::MegolmV2AesSha2(c) => {
+                    InboundGroupSession::from(c.as_ref())
+                }
+                ForwardedRoomKeyContent::Unknown(_) => {
+                    warn!(
+                        sender_key = ?sender_key,
+                        room_id = ?info.room_id(),
+                        session_id = info.session_id(),
+                        sender_key = ?sender_key,
+                        algorithm = ?info.algorithm(),
+                        "Received an unknown forwarded room key that we didn't request",
+                    );
+
+                    return Ok(None);
+                }
+            };
+
+            if let Some(visibility) = session.history_visibility.as_ref() {
+                if matches!(
+                    visibility,
+                    HistoryVisibility::Shared | HistoryVisibility::WorldReadable
+                ) {
+                    return Ok(Some(session));
+                }
+            }
+
             warn!(
                 sender_key = ?sender_key,
                 room_id = ?info.room_id(),
@@ -985,6 +1124,7 @@ impl GossipMachine {
                 algorithm = ?info.algorithm(),
                 "Received a forwarded room key that we didn't request",
             );
+
             return Ok(None);
         };
 
@@ -1464,6 +1604,8 @@ mod tests {
     #[async_test]
     #[cfg(feature = "automatic-room-key-forwarding")]
     async fn should_share_key_test() {
+        use ruma::events::room::history_visibility::HistoryVisibility;
+
         let machine = get_machine().await;
         let account = account();
 
@@ -1527,8 +1669,14 @@ mod tests {
             .await;
         machine.should_share_key(&bob_device, &inbound).await.unwrap();
 
-        let (other_outbound, other_inbound) =
-            account.create_group_session_pair_with_defaults(room_id()).await;
+        let encryption_settings = EncryptionSettings {
+            history_visibility: HistoryVisibility::Invited,
+            ..Default::default()
+        };
+        let (other_outbound, other_inbound) = account
+            .create_group_session_pair(room_id(), encryption_settings)
+            .await
+            .expect("Can't create group session pair");
 
         // But we don't share some other session that doesn't match our outbound
         // session.