Merge pull request #2227 from poljar/secret-send-inbox

crates/matrix-sdk-crypto/src/file_encryption/key_export.rs

@@ -281,7 +281,7 @@ mod tests {
 
     use indoc::indoc;
     use matrix_sdk_test::async_test;
-    use ruma::room_id;
+    use ruma::{room_id, user_id};
 
     use super::{
         decode, decrypt_helper, decrypt_room_key_export, encrypt_helper, encrypt_room_key_export,
@@ -330,7 +330,8 @@ mod tests {
 
     #[async_test]
     async fn test_session_encrypt() {
-        let (machine, _) = get_prepared_machine(false).await;
+        let user_id = user_id!("@alice:localhost");
+        let (machine, _) = get_prepared_machine(user_id, false).await;
         let room_id = room_id!("!test:localhost");
 
         machine.create_outbound_group_session_with_defaults(room_id).await.unwrap();
@@ -353,7 +354,9 @@ mod tests {
 
     #[async_test]
     async fn test_importing_better_session() -> OlmResult<()> {
-        let (machine, _) = get_prepared_machine(false).await;
+        let user_id = user_id!("@alice:localhost");
+
+        let (machine, _) = get_prepared_machine(user_id, false).await;
         let room_id = room_id!("!test:localhost");
         let session = machine.create_inbound_session(room_id).await?;
 

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

@@ -35,10 +35,10 @@ use ruma::{
     DeviceId, DeviceKeyAlgorithm, OwnedDeviceId, OwnedTransactionId, OwnedUserId, RoomId,
     TransactionId, UserId,
 };
-use tracing::{debug, info, trace, warn};
+use tracing::{debug, field::display, info, instrument, trace, warn, Span};
 use vodozemac::{megolm::SessionOrdering, Curve25519PublicKey};
 
-use super::{GossipRequest, RequestEvent, RequestInfo, SecretInfo, WaitQueue};
+use super::{GossipRequest, GossippedSecret, RequestEvent, RequestInfo, SecretInfo, WaitQueue};
 use crate::{
     error::{EventError, OlmError, OlmResult},
     olm::{InboundGroupSession, Session},
@@ -794,30 +794,33 @@ impl GossipMachine {
 
     async fn accept_secret(
         &self,
-        event: &DecryptedSecretSendEvent,
-        request: &GossipRequest,
-        secret_name: &SecretName,
+        secret: GossippedSecret,
+        changes: &mut Changes,
     ) -> Result<(), CryptoStoreError> {
-        if secret_name != &SecretName::RecoveryKey {
-            match self.inner.store.import_secret(secret_name, &event.content.secret).await {
-                Ok(_) => self.mark_as_done(request).await?,
+        if secret.secret_name != SecretName::RecoveryKey {
+            match self.inner.store.import_secret(&secret).await {
+                Ok(_) => self.mark_as_done(&secret.gossip_request).await?,
                 // If this is a store error propagate it up the call stack.
                 Err(SecretImportError::Store(e)) => return Err(e),
                 // Otherwise warn that there was something wrong with the
                 // secret.
                 Err(e) => {
                     warn!(
-                        secret_name = secret_name.as_ref(),
+                        secret_name = %secret.secret_name,
                         error = ?e,
                         "Error while importing a secret"
                     );
                 }
             }
         } else {
-            // Skip importing the recovery key here since we'll want to check
-            // if the public key matches to the latest version on the server.
-            // The key will not be zeroized and  instead leave the key in the
-            // event and let the user import it later.
+            // We would need to fire out a request to figure out if this backup recovery key
+            // is the one that is used for the current backup and if the backup
+            // is trusted.
+            //
+            // So we put the secret into our inbox. Later users can inspect the contents of
+            // the inbox and decide if they want to activate the backup.
+            info!("Received a backup recovery key, storing it into the secret inbox.");
+            changes.secrets.push(secret);
         }
 
         Ok(())
@@ -826,74 +829,71 @@ impl GossipMachine {
     async fn receive_secret(
         &self,
         sender_key: Curve25519PublicKey,
-        event: &DecryptedSecretSendEvent,
-        request: &GossipRequest,
-        secret_name: &SecretName,
+        secret: GossippedSecret,
+        changes: &mut Changes,
     ) -> Result<(), CryptoStoreError> {
-        debug!(
-            request_id = event.content.request_id.as_str(),
-            secret_name = secret_name.as_ref(),
-            "Received a m.secret.send event with a matching request"
-        );
+        debug!("Received a m.secret.send event with a matching request");
 
         if let Some(device) =
-            self.inner.store.get_device_from_curve_key(&event.sender, sender_key).await?
+            self.inner.store.get_device_from_curve_key(&secret.event.sender, sender_key).await?
         {
             // Only accept secrets from one of our own trusted devices.
             if device.user_id() == self.user_id() && device.is_verified() {
-                self.accept_secret(event, request, secret_name).await?;
+                self.accept_secret(secret, changes).await?;
             } else {
-                warn!(
-                    request_id = event.content.request_id.as_str(),
-                    secret_name = secret_name.as_ref(),
-                    "Received a m.secret.send event from another user or from \
-                    unverified device"
-                );
+                warn!("Received a m.secret.send event from another user or from unverified device");
             }
         } else {
-            warn!(
-                request_id = event.content.request_id.as_str(),
-                secret_name = secret_name.as_ref(),
-                "Received a m.secret.send event from an unknown device"
-            );
-            self.inner.store.mark_user_as_changed(&event.sender).await?;
+            warn!("Received a m.secret.send event from an unknown device");
+
+            self.inner.store.mark_user_as_changed(&secret.event.sender).await?;
         }
 
         Ok(())
     }
 
+    #[instrument(skip_all, fields(sender_key, sender = %event.sender, request_id = %event.content.request_id, secret_name))]
     pub async fn receive_secret_event(
         &self,
         sender_key: Curve25519PublicKey,
         event: &DecryptedSecretSendEvent,
+        changes: &mut Changes,
     ) -> Result<Option<SecretName>, CryptoStoreError> {
-        debug!(request_id = event.content.request_id.as_str(), "Received a m.secret.send event");
+        debug!("Received a m.secret.send event");
 
-        let maybe_request = self
-            .inner
-            .store
-            .get_outgoing_secret_requests(event.content.request_id.as_str().into())
-            .await?;
+        let request_id = &event.content.request_id;
 
-        Ok(if let Some(request) = maybe_request {
+        let name = if let Some(request) =
+            self.inner.store.get_outgoing_secret_requests(request_id).await?
+        {
             match &request.info {
                 SecretInfo::KeyRequest(_) => {
-                    warn!(
-                        request_id = event.content.request_id.as_str(),
-                        "Received a m.secret.send event but the request was for a room key"
-                    );
+                    warn!("Received a m.secret.send event but the request was for a room key");
 
                     None
                 }
                 SecretInfo::SecretRequest(secret_name) => {
-                    self.receive_secret(sender_key, event, &request, secret_name).await?;
+                    Span::current().record("secret_name", display(secret_name));
 
-                    Some(secret_name.to_owned())
+                    let secret_name = secret_name.to_owned();
+
+                    let secret = GossippedSecret {
+                        secret_name: secret_name.to_owned(),
+                        event: event.to_owned(),
+                        gossip_request: request,
+                    };
+
+                    self.receive_secret(sender_key, secret, changes).await?;
+
+                    Some(secret_name)
                 }
             }
         } else {
+            warn!("Received a m.secret.send event, but no matching request was found");
             None
-        })
+        };
+
+        Ok(name)
     }
 
     async fn accept_forwarded_room_key(
@@ -1270,9 +1270,9 @@ mod tests {
         C: EventType + DeserializeOwned + Serialize + std::fmt::Debug,
     {
         let content = extract_content(recipient, request);
-        let content: C = content
-            .deserialize_as()
-            .expect("We can always deserialize the to-device event content");
+        let content: C = content.deserialize_as().unwrap_or_else(|_| {
+            panic!("We can always deserialize the to-device event content {content:?}")
+        });
 
         ToDeviceEvent::new(sender.to_owned(), content)
     }
@@ -1752,6 +1752,88 @@ mod tests {
         assert!(!alice_machine.inner.outgoing_requests.is_empty());
     }
 
+    #[async_test]
+    #[cfg(feature = "backups_v1")]
+    async fn secret_broadcasting() {
+        use futures_util::{pin_mut, FutureExt};
+        use ruma::api::client::to_device::send_event_to_device::v3::Response as ToDeviceResponse;
+        use serde_json::value::to_raw_value;
+        use tokio_stream::StreamExt;
+
+        use crate::machine::tests::get_machine_pair_with_setup_sessions;
+
+        let alice_id = user_id!("@alice:localhost");
+
+        let (alice_machine, bob_machine) =
+            get_machine_pair_with_setup_sessions(alice_id, alice_id, false).await;
+
+        let key_requests = GossipMachine::request_missing_secrets(
+            bob_machine.user_id(),
+            vec![SecretName::RecoveryKey],
+        );
+        let mut changes = Changes::default();
+        let request_id = key_requests.first().unwrap().request_id.to_owned();
+        changes.key_requests = key_requests;
+        bob_machine.store().save_changes(changes).await.unwrap();
+        for request in bob_machine.outgoing_requests().await.unwrap() {
+            bob_machine
+                .mark_request_as_sent(request.request_id(), &ToDeviceResponse::new())
+                .await
+                .unwrap();
+        }
+
+        let event = RumaToDeviceEvent {
+            sender: alice_machine.user_id().to_owned(),
+            content: ToDeviceSecretRequestEventContent::new(
+                RequestAction::Request(SecretName::RecoveryKey),
+                bob_machine.device_id().to_owned(),
+                request_id,
+            ),
+        };
+
+        let bob_device = alice_machine
+            .get_device(alice_id, bob_machine.device_id(), None)
+            .await
+            .unwrap()
+            .unwrap();
+        let alice_device = bob_machine
+            .get_device(alice_id, alice_machine.device_id(), None)
+            .await
+            .unwrap()
+            .unwrap();
+
+        // We need a trusted device, otherwise we won't serve nor accept secrets.
+        bob_device.set_trust_state(LocalTrust::Verified);
+        alice_device.set_trust_state(LocalTrust::Verified);
+        alice_machine.store().save_devices(&[bob_device.inner]).await.unwrap();
+        bob_machine.store().save_devices(&[alice_device.inner]).await.unwrap();
+
+        let recovery_key = crate::store::RecoveryKey::new().unwrap();
+        alice_machine.backup_machine().save_recovery_key(Some(recovery_key), None).await.unwrap();
+        alice_machine.inner.key_request_machine.receive_incoming_secret_request(&event);
+        alice_machine.inner.key_request_machine.collect_incoming_key_requests().await.unwrap();
+
+        let requests =
+            alice_machine.inner.key_request_machine.outgoing_to_device_requests().await.unwrap();
+
+        assert_eq!(requests.len(), 1);
+        let request = requests.first().expect("We should have an outgoing to-device request");
+
+        let event: EncryptedToDeviceEvent =
+            request_to_event(bob_machine.user_id(), alice_machine.user_id(), request);
+        let event = Raw::from_json(to_raw_value(&event).unwrap());
+
+        let stream = bob_machine.store().secrets_stream();
+        pin_mut!(stream);
+
+        bob_machine
+            .receive_sync_changes(vec![event], &Default::default(), &Default::default(), None)
+            .await
+            .unwrap();
+
+        stream.next().now_or_never().expect("The broadcaster should have sent out the secret");
+    }
+
     #[async_test]
     #[cfg(feature = "automatic-room-key-forwarding")]
     async fn key_share_cycle_without_session() {

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

@@ -35,12 +35,30 @@ use serde::{Deserialize, Serialize};
 
 use crate::{
     requests::{OutgoingRequest, ToDeviceRequest},
-    types::events::room_key_request::{
-        RoomKeyRequestContent, RoomKeyRequestEvent, SupportedKeyInfo,
+    types::events::{
+        olm_v1::DecryptedSecretSendEvent,
+        room_key_request::{RoomKeyRequestContent, RoomKeyRequestEvent, SupportedKeyInfo},
     },
     Device,
 };
 
+/// Struct containing a `m.secret.send` event and its acompanying info.
+///
+/// This struct is created only iff the following three things are true:
+///
+/// 1. We requested the secret.
+/// 2. The secret was received over an encrypted channel.
+/// 3. The secret it was received from one ouf our own verified devices.
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct GossippedSecret {
+    /// The name of the secret.
+    pub secret_name: SecretName,
+    /// The [`GossipRequest`] that has requested the secret.
+    pub gossip_request: GossipRequest,
+    /// The `m.secret.send` event containing the actual secret.
+    pub event: DecryptedSecretSendEvent,
+}
+
 /// An error describing why a key share request won't be honored.
 #[cfg(feature = "automatic-room-key-forwarding")]
 #[derive(Debug, Clone, thiserror::Error, PartialEq, Eq)]

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

@@ -73,7 +73,7 @@ pub use file_encryption::{
     decrypt_room_key_export, encrypt_room_key_export, AttachmentDecryptor, AttachmentEncryptor,
     DecryptorError, KeyExportError, MediaEncryptionInfo,
 };
-pub use gossiping::GossipRequest;
+pub use gossiping::{GossipRequest, GossippedSecret};
 pub use identities::{
     Device, LocalTrust, OwnUserIdentity, ReadOnlyDevice, ReadOnlyOwnUserIdentity,
     ReadOnlyUserIdentities, ReadOnlyUserIdentity, UserDevices, UserIdentities, UserIdentity,