Introduce a secret inbox

Up until now, users had to listen for to-device events to check for
secrets that were received as an `m.secret.send` event.

This has a bunch of shortcomings:
    1. Once the has been given to the consumer, it's gone and can't be
       retrieved anymore. Secrets may get lost if an app restart happens
       before the consumer decides what to do with it.
    2. The consumer can't be sure if the event was received in a
       secure manner.

This commit ads a inbox for our received secrets where we will long-term
store all secrets we receive until the user decides to delete them.

It's deemed fine to store all secrets, since we only accept secrets we
have requested and if they have been received from a verified device of
ours.

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::debug, 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,32 @@ 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.
+            changes.secrets.push(secret);
         }
 
         Ok(())
@@ -826,74 +828,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", debug(secret_name));
+
+                    let secret_name = secret_name.to_owned();
 
-                    Some(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(

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,

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

@@ -649,11 +649,12 @@ impl OlmMachine {
     async fn decrypt_to_device_event(
         &self,
         event: &EncryptedToDeviceEvent,
+        changes: &mut Changes,
     ) -> OlmResult<OlmDecryptionInfo> {
         let mut decrypted = self.inner.account.decrypt_to_device_event(event).await?;
         // Handle the decrypted event, e.g. fetch out Megolm sessions out of
         // the event.
-        self.handle_decrypted_to_device_event(&mut decrypted).await?;
+        self.handle_decrypted_to_device_event(&mut decrypted, changes).await?;
 
         Ok(decrypted)
     }
@@ -894,6 +895,7 @@ impl OlmMachine {
     async fn handle_decrypted_to_device_event(
         &self,
         decrypted: &mut OlmDecryptionInfo,
+        changes: &mut Changes,
     ) -> OlmResult<()> {
         debug!("Received a decrypted to-device event");
 
@@ -914,7 +916,7 @@ impl OlmMachine {
                 let name = self
                     .inner
                     .key_request_machine
-                    .receive_secret_event(decrypted.result.sender_key, e)
+                    .receive_secret_event(decrypted.result.sender_key, e, changes)
                     .await?;
 
                 // Set the secret name so other consumers of the event know
@@ -1048,7 +1050,7 @@ impl OlmMachine {
 
         match event {
             ToDeviceEvents::RoomEncrypted(e) => {
-                let decrypted = match self.decrypt_to_device_event(&e).await {
+                let decrypted = match self.decrypt_to_device_event(&e, changes).await {
                     Ok(e) => e,
                     Err(err) => {
                         if let OlmError::SessionWedged(sender, curve_key) = err {
@@ -1947,6 +1949,7 @@ pub(crate) mod tests {
         error::EventError,
         machine::OlmMachine,
         olm::{InboundGroupSession, OutboundGroupSession, VerifyJson},
+        store::Changes,
         types::{
             events::{
                 room::encrypted::{EncryptedToDeviceEvent, ToDeviceEncryptedEventContent},
@@ -2083,7 +2086,7 @@ pub(crate) mod tests {
         let event =
             ToDeviceEvent::new(alice.user_id().to_owned(), content.deserialize_as().unwrap());
 
-        let decrypted = bob.decrypt_to_device_event(&event).await.unwrap();
+        let decrypted = bob.decrypt_to_device_event(&event, &mut Changes::default()).await.unwrap();
         bob.store().save_sessions(&[decrypted.session.session()]).await.unwrap();
 
         (alice, bob)
@@ -2417,9 +2420,8 @@ pub(crate) mod tests {
         );
 
         // Decrypting the first time should succeed.
-
         let decrypted = bob
-            .decrypt_to_device_event(&event)
+            .decrypt_to_device_event(&event, &mut Changes::default())
             .await
             .expect("We should be able to decrypt the event.")
             .result
@@ -2431,7 +2433,7 @@ pub(crate) mod tests {
         assert_eq!(&decrypted.sender, alice.user_id());
 
         // Replaying the event should now result in a decryption failure.
-        bob.decrypt_to_device_event(&event).await.expect_err(
+        bob.decrypt_to_device_event(&event, &mut Changes::default()).await.expect_err(
             "Decrypting a replayed event should not succeed, even if it's a pre-key message",
         );
     }
@@ -2507,8 +2509,12 @@ pub(crate) mod tests {
 
         let mut room_keys_received_stream = Box::pin(bob.store().room_keys_received_stream());
 
-        let group_session =
-            bob.decrypt_to_device_event(&event).await.unwrap().inbound_group_session.unwrap();
+        let group_session = bob
+            .decrypt_to_device_event(&event, &mut Changes::default())
+            .await
+            .unwrap()
+            .inbound_group_session
+            .unwrap();
         bob.store().save_inbound_group_sessions(&[group_session.clone()]).await.unwrap();
 
         // when we decrypt the room key, the
@@ -2651,8 +2657,11 @@ pub(crate) mod tests {
             to_device_requests_to_content(to_device_requests),
         );
 
-        let group_session =
-            bob.decrypt_to_device_event(&event).await.unwrap().inbound_group_session;
+        let group_session = bob
+            .decrypt_to_device_event(&event, &mut Changes::default())
+            .await
+            .unwrap()
+            .inbound_group_session;
 
         let export = group_session.as_ref().unwrap().clone().export().await;
 
@@ -3068,8 +3077,11 @@ pub(crate) mod tests {
             to_device_requests_to_content(to_device_requests),
         );
 
-        let group_session =
-            bob.decrypt_to_device_event(&event).await.unwrap().inbound_group_session;
+        let group_session = bob
+            .decrypt_to_device_event(&event, &mut Changes::default())
+            .await
+            .unwrap()
+            .inbound_group_session;
         bob.store().save_inbound_group_sessions(&[group_session.unwrap()]).await.unwrap();
 
         let room_event = json_convert(&room_event).unwrap();
@@ -3383,7 +3395,7 @@ pub(crate) mod tests {
         let event: EncryptedToDeviceEvent = serde_json::from_value(event).unwrap();
 
         assert_matches!(
-            bob.decrypt_to_device_event(&event).await,
+            bob.decrypt_to_device_event(&event, &mut Changes::default()).await,
             Err(OlmError::EventError(EventError::UnsupportedAlgorithm))
         );