From 933ffc909f87932c1962dc1ec6124ea10e800f65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 22 Dec 2022 10:31:13 +0100 Subject: [PATCH 1/4] 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. --- .../src/gossiping/machine.rs | 97 +++++++++---------- crates/matrix-sdk-crypto/src/gossiping/mod.rs | 22 ++++- crates/matrix-sdk-crypto/src/lib.rs | 2 +- crates/matrix-sdk-crypto/src/machine.rs | 40 +++++--- .../src/store/integration_tests.rs | 81 +++++++++++++++- .../src/store/memorystore.rs | 47 ++++++--- crates/matrix-sdk-crypto/src/store/mod.rs | 25 +++-- crates/matrix-sdk-crypto/src/store/traits.rs | 30 +++++- .../src/types/events/secret_send.rs | 3 +- .../matrix-sdk-indexeddb/src/crypto_store.rs | 63 +++++++++++- .../crypto_store/008_secret_inbox.sql | 4 + crates/matrix-sdk-sqlite/src/crypto_store.rs | 67 ++++++++++++- 12 files changed, 377 insertions(+), 104 deletions(-) create mode 100644 crates/matrix-sdk-sqlite/migrations/crypto_store/008_secret_inbox.sql diff --git a/crates/matrix-sdk-crypto/src/gossiping/machine.rs b/crates/matrix-sdk-crypto/src/gossiping/machine.rs index 4d0bb8f3784..39f96d7a862 100644 --- a/crates/matrix-sdk-crypto/src/gossiping/machine.rs +++ b/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,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, 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)); + + 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( diff --git a/crates/matrix-sdk-crypto/src/gossiping/mod.rs b/crates/matrix-sdk-crypto/src/gossiping/mod.rs index 83d028bc786..f595688ab5d 100644 --- a/crates/matrix-sdk-crypto/src/gossiping/mod.rs +++ b/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)] diff --git a/crates/matrix-sdk-crypto/src/lib.rs b/crates/matrix-sdk-crypto/src/lib.rs index fb58a2dcd8c..b2121970e5c 100644 --- a/crates/matrix-sdk-crypto/src/lib.rs +++ b/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, diff --git a/crates/matrix-sdk-crypto/src/machine.rs b/crates/matrix-sdk-crypto/src/machine.rs index 26cd49af815..13cd6c3fa49 100644 --- a/crates/matrix-sdk-crypto/src/machine.rs +++ b/crates/matrix-sdk-crypto/src/machine.rs @@ -625,11 +625,12 @@ impl OlmMachine { async fn decrypt_to_device_event( &self, event: &EncryptedToDeviceEvent, + changes: &mut Changes, ) -> OlmResult { 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) } @@ -870,6 +871,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"); @@ -890,7 +892,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 @@ -1024,7 +1026,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 { @@ -1914,6 +1916,7 @@ pub(crate) mod tests { error::EventError, machine::OlmMachine, olm::{InboundGroupSession, OutboundGroupSession, VerifyJson}, + store::Changes, types::{ events::{ room::encrypted::{EncryptedToDeviceEvent, ToDeviceEncryptedEventContent}, @@ -2049,7 +2052,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) @@ -2370,9 +2373,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 @@ -2384,7 +2386,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", ); } @@ -2456,8 +2458,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 @@ -2600,8 +2606,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; @@ -3017,8 +3026,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(); @@ -3332,7 +3344,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)) ); diff --git a/crates/matrix-sdk-crypto/src/store/integration_tests.rs b/crates/matrix-sdk-crypto/src/store/integration_tests.rs index 4abe0d5ed33..46075667a76 100644 --- a/crates/matrix-sdk-crypto/src/store/integration_tests.rs +++ b/crates/matrix-sdk-crypto/src/store/integration_tests.rs @@ -10,6 +10,7 @@ macro_rules! cryptostore_integration_tests { use ruma::{ device_id, encryption::SignedKey, + events::secret::request::SecretName, room_id, serde::{Base64, Raw}, to_device::DeviceIdOrAllDevices, @@ -34,11 +35,13 @@ macro_rules! cryptostore_integration_tests { CommonWithheldCodeContent, MegolmV1AesSha2WithheldContent, RoomKeyWithheldContent, WithheldCode, }, + olm_v1::{DecryptedSecretSendEvent, OlmV1Keys}, + secret_send::SecretSendContent, ToDeviceEvent, }, EventEncryptionAlgorithm, }, - ReadOnlyDevice, SecretInfo, ToDeviceRequest, TrackedUser, + ReadOnlyDevice, SecretInfo, ToDeviceRequest, TrackedUser, GossippedSecret, }; use super::get_store; @@ -641,6 +644,82 @@ macro_rules! cryptostore_integration_tests { assert!(store.get_unsent_secret_requests().await.unwrap().is_empty()); } + #[async_test] + async fn gossipped_secret_saving() { + let (account, store) = get_loaded_store("key_request_saving").await; + + let secret = "It is a secret to everybody"; + + let id = TransactionId::new(); + let info: SecretInfo = MegolmV1AesSha2Content { + room_id: room_id!("!test:localhost").to_owned(), + sender_key: account.identity_keys().curve25519, + session_id: "test_session_id".to_owned(), + } + .into(); + + let gossip_request = GossipRequest { + request_recipient: account.user_id().to_owned(), + request_id: id.clone(), + info: info.clone(), + sent_out: true, + }; + + let mut event = DecryptedSecretSendEvent { + sender: account.user_id().to_owned(), + recipient: account.user_id().to_owned(), + keys: OlmV1Keys { + ed25519: account.identity_keys().ed25519, + }, + recipient_keys: OlmV1Keys { + ed25519: account.identity_keys().ed25519, + }, + content: SecretSendContent::new(id.to_owned(), secret.to_owned()), + }; + + let value = GossippedSecret { + secret_name: SecretName::RecoveryKey, + gossip_request: gossip_request.to_owned(), + event: event.to_owned(), + }; + + assert!( + store.get_secrets_from_inbox(&SecretName::RecoveryKey).await.unwrap().is_empty(), + "No secret should initially be found in the store" + ); + + let mut changes = Changes::default(); + changes.secrets.push(value); + store.save_changes(changes).await.unwrap(); + + let restored = store.get_secrets_from_inbox(&SecretName::RecoveryKey).await.unwrap(); + let first_secret = restored.first().expect("We should have restored a secret now"); + assert_eq!(first_secret.event.content.secret, secret); + assert_eq!(restored.len(), 1, "We should only have one secret stored for now"); + + event.content.request_id = TransactionId::new(); + let another_secret = GossippedSecret { + secret_name: SecretName::RecoveryKey, + gossip_request, + event, + }; + + let mut changes = Changes::default(); + changes.secrets.push(another_secret); + store.save_changes(changes).await.unwrap(); + + let restored = store.get_secrets_from_inbox(&SecretName::RecoveryKey).await.unwrap(); + assert_eq!(restored.len(), 2, "We should only have two secrets stored"); + + let restored = store.get_secrets_from_inbox(&SecretName::CrossSigningMasterKey).await.unwrap(); + assert!(restored.is_empty(), "We should not have secrets of a different type stored"); + + store.delete_secrets_from_inbox(&SecretName::RecoveryKey).await.unwrap(); + + let restored = store.get_secrets_from_inbox(&SecretName::RecoveryKey).await.unwrap(); + assert!(restored.is_empty(), "We should not have any secrets after we have deleted them"); + } + #[async_test] async fn withheld_info_storage() { let (account, store) = get_loaded_store("withheld_info_storage").await; diff --git a/crates/matrix-sdk-crypto/src/store/memorystore.rs b/crates/matrix-sdk-crypto/src/store/memorystore.rs index 1693bf38b69..e717f43e148 100644 --- a/crates/matrix-sdk-crypto/src/store/memorystore.rs +++ b/crates/matrix-sdk-crypto/src/store/memorystore.rs @@ -22,8 +22,8 @@ use std::{ use async_trait::async_trait; use dashmap::{DashMap, DashSet}; use ruma::{ - DeviceId, OwnedDeviceId, OwnedRoomId, OwnedTransactionId, OwnedUserId, RoomId, TransactionId, - UserId, + events::secret::request::SecretName, DeviceId, OwnedDeviceId, OwnedRoomId, OwnedTransactionId, + OwnedUserId, RoomId, TransactionId, UserId, }; use tokio::sync::Mutex; use tracing::warn; @@ -34,7 +34,7 @@ use super::{ RoomSettings, Session, }; use crate::{ - gossiping::{GossipRequest, SecretInfo}, + gossiping::{GossipRequest, GossippedSecret, SecretInfo}, identities::{ReadOnlyDevice, ReadOnlyUserIdentities}, olm::{OutboundGroupSession, PrivateCrossSigningIdentity}, types::events::room_key_withheld::RoomKeyWithheldEvent, @@ -63,6 +63,7 @@ pub struct MemoryStore { direct_withheld_info: Arc>>, custom_values: Arc>>, leases: Arc>, + secret_inbox: Arc>>, } impl Default for MemoryStore { @@ -78,6 +79,7 @@ impl Default for MemoryStore { direct_withheld_info: Default::default(), custom_values: Default::default(), leases: Default::default(), + secret_inbox: Default::default(), } } } @@ -159,6 +161,10 @@ impl CryptoStore for MemoryStore { self.key_requests_by_info.insert(info_string, id); } + for secret in changes.secrets { + self.secret_inbox.entry(secret.secret_name.to_string()).or_default().push(secret); + } + for (room_id, data) in changes.withheld_session_info { for (session_id, event) in data { self.direct_withheld_info @@ -183,6 +189,17 @@ impl CryptoStore for MemoryStore { Ok(self.inbound_group_sessions.get(room_id, session_id)) } + async fn get_withheld_info( + &self, + room_id: &RoomId, + session_id: &str, + ) -> Result> { + Ok(self + .direct_withheld_info + .get(room_id) + .and_then(|e| Some(e.value().get(session_id)?.value().to_owned()))) + } + async fn get_inbound_group_sessions(&self) -> Result> { Ok(self.inbound_group_sessions.get_all()) } @@ -215,6 +232,10 @@ impl CryptoStore for MemoryStore { Ok(()) } + async fn load_backup_keys(&self) -> Result { + Ok(BackupKeys::default()) + } + async fn get_outbound_group_session(&self, _: &RoomId) -> Result> { Ok(None) } @@ -291,19 +312,17 @@ impl CryptoStore for MemoryStore { Ok(()) } - async fn load_backup_keys(&self) -> Result { - Ok(BackupKeys::default()) + async fn get_secrets_from_inbox( + &self, + secret_name: &SecretName, + ) -> Result> { + Ok(self.secret_inbox.entry(secret_name.to_string()).or_default().to_owned()) } - async fn get_withheld_info( - &self, - room_id: &RoomId, - session_id: &str, - ) -> Result> { - Ok(self - .direct_withheld_info - .get(room_id) - .and_then(|e| Some(e.value().get(session_id)?.value().to_owned()))) + async fn delete_secrets_from_inbox(&self, secret_name: &SecretName) -> Result<()> { + self.secret_inbox.remove(secret_name.as_str()); + + Ok(()) } async fn get_room_settings(&self, _room_id: &RoomId) -> Result> { diff --git a/crates/matrix-sdk-crypto/src/store/mod.rs b/crates/matrix-sdk-crypto/src/store/mod.rs index 49c2bd76dc9..44be6a9e639 100644 --- a/crates/matrix-sdk-crypto/src/store/mod.rs +++ b/crates/matrix-sdk-crypto/src/store/mod.rs @@ -63,6 +63,7 @@ use vodozemac::{megolm::SessionOrdering, Curve25519PublicKey}; use zeroize::Zeroize; use crate::{ + gossiping::GossippedSecret, identities::{ user::{OwnUserIdentity, UserIdentities, UserIdentity}, Device, ReadOnlyDevice, ReadOnlyUserIdentities, UserDevices, @@ -151,6 +152,7 @@ pub struct Changes { /// Stores when a `m.room_key.withheld` is received pub withheld_session_info: BTreeMap>, pub room_settings: HashMap, + pub secrets: Vec, } /// A user for which we are tracking the list of devices. @@ -698,12 +700,8 @@ impl Store { } /// Import the given `secret` named `secret_name` into the keystore. - pub(crate) async fn import_secret( - &self, - secret_name: &SecretName, - secret: &str, - ) -> Result<(), SecretImportError> { - match secret_name { + pub async fn import_secret(&self, secret: &GossippedSecret) -> Result<(), SecretImportError> { + match &secret.secret_name { SecretName::CrossSigningMasterKey | SecretName::CrossSigningUserSigningKey | SecretName::CrossSigningSelfSigningKey => { @@ -712,9 +710,15 @@ impl Store { { let identity = self.inner.identity.lock().await; - identity.import_secret(public_identity, secret_name, secret).await?; + identity + .import_secret( + public_identity, + &secret.secret_name, + &secret.event.content.secret, + ) + .await?; info!( - secret_name = secret_name.as_ref(), + secret_name = %secret.secret_name, "Successfully imported a private cross signing key" ); @@ -727,8 +731,9 @@ impl Store { SecretName::RecoveryKey => { // We don't import the recovery key here since we'll want to // check if the public key matches to the latest version on the - // server. We instead leave the key in the event and let the - // user import it later. + // server. We instead put the secret into a secret inbox where + // it will stay until it either gets overwritten + // or the user accepts the secret. } name => { warn!(secret = ?name, "Tried to import an unknown secret"); diff --git a/crates/matrix-sdk-crypto/src/store/traits.rs b/crates/matrix-sdk-crypto/src/store/traits.rs index b0af3c9d726..98b156f7481 100644 --- a/crates/matrix-sdk-crypto/src/store/traits.rs +++ b/crates/matrix-sdk-crypto/src/store/traits.rs @@ -16,7 +16,9 @@ use std::{collections::HashMap, fmt, sync::Arc}; use async_trait::async_trait; use matrix_sdk_common::AsyncTraitDeps; -use ruma::{DeviceId, OwnedDeviceId, RoomId, TransactionId, UserId}; +use ruma::{ + events::secret::request::SecretName, DeviceId, OwnedDeviceId, RoomId, TransactionId, UserId, +}; use tokio::sync::Mutex; use super::{BackupKeys, Changes, CryptoStoreError, Result, RoomKeyCounts, RoomSettings}; @@ -26,8 +28,8 @@ use crate::{ Session, }, types::events::room_key_withheld::RoomKeyWithheldEvent, - GossipRequest, ReadOnlyAccount, ReadOnlyDevice, ReadOnlyUserIdentities, SecretInfo, - TrackedUser, + GossipRequest, GossippedSecret, ReadOnlyAccount, ReadOnlyDevice, ReadOnlyUserIdentities, + SecretInfo, TrackedUser, }; /// Represents a store that the `OlmMachine` uses to store E2EE data (such as @@ -200,6 +202,17 @@ pub trait CryptoStore: AsyncTraitDeps { request_id: &TransactionId, ) -> Result<(), Self::Error>; + /// Get all the secrets with the given [`SecretName`] we have currently + /// stored. + async fn get_secrets_from_inbox( + &self, + secret_name: &SecretName, + ) -> Result, Self::Error>; + + /// Delete all the secrets with the given [`SecretName`] we have currently + /// stored. + async fn delete_secrets_from_inbox(&self, secret_name: &SecretName) -> Result<(), Self::Error>; + /// Get the room settings, such as the encryption algorithm or whether to /// encrypt only for trusted devices. /// @@ -371,6 +384,17 @@ impl CryptoStore for EraseCryptoStoreError { self.0.delete_outgoing_secret_requests(request_id).await.map_err(Into::into) } + async fn get_secrets_from_inbox( + &self, + secret_name: &SecretName, + ) -> Result> { + self.0.get_secrets_from_inbox(secret_name).await.map_err(Into::into) + } + + async fn delete_secrets_from_inbox(&self, secret_name: &SecretName) -> Result<()> { + self.0.delete_secrets_from_inbox(secret_name).await.map_err(Into::into) + } + async fn get_withheld_info( &self, room_id: &RoomId, diff --git a/crates/matrix-sdk-crypto/src/types/events/secret_send.rs b/crates/matrix-sdk-crypto/src/types/events/secret_send.rs index 9ae69441025..c2b7701c9c8 100644 --- a/crates/matrix-sdk-crypto/src/types/events/secret_send.rs +++ b/crates/matrix-sdk-crypto/src/types/events/secret_send.rs @@ -31,7 +31,7 @@ pub type SecretSendEvent = ToDeviceEvent; /// Sent by a client to share a secret with another device, in response to an /// `m.secret.request` event. It must be encrypted as an `m.room.encrypted` /// event, then sent as a to-device event. -#[derive(Serialize, Deserialize)] +#[derive(Clone, Serialize, Deserialize)] pub struct SecretSendContent { /// The ID of the request that this a response to. pub request_id: OwnedTransactionId, @@ -70,7 +70,6 @@ impl std::fmt::Debug for SecretSendContent { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("SecretSendContent") .field("request_id", &self.request_id) - .field("secret_name", &self.secret_name) .finish_non_exhaustive() } } diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store.rs b/crates/matrix-sdk-indexeddb/src/crypto_store.rs index 6ea8a9a1eda..051ea3e60ea 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store.rs @@ -30,12 +30,13 @@ use matrix_sdk_crypto::{ RoomSettings, }, types::events::room_key_withheld::RoomKeyWithheldEvent, - GossipRequest, ReadOnlyAccount, ReadOnlyDevice, ReadOnlyUserIdentities, SecretInfo, - TrackedUser, + GossipRequest, GossippedSecret, ReadOnlyAccount, ReadOnlyDevice, ReadOnlyUserIdentities, + SecretInfo, TrackedUser, }; use matrix_sdk_store_encryption::StoreCipher; use ruma::{ - DeviceId, MilliSecondsSinceUnixEpoch, OwnedDeviceId, OwnedUserId, RoomId, TransactionId, UserId, + events::secret::request::SecretName, DeviceId, MilliSecondsSinceUnixEpoch, OwnedDeviceId, + OwnedUserId, RoomId, TransactionId, UserId, }; use serde::{de::DeserializeOwned, Serialize}; use tokio::sync::Mutex; @@ -65,6 +66,8 @@ mod keys { pub const KEY_REQUEST: &str = "key_request"; pub const ROOM_SETTINGS: &str = "room_settings"; + pub const SECRETS_INBOX: &str = "secrets_inbox"; + pub const DIRECT_WITHHELD_INFO: &str = "direct_withheld_info"; // keys @@ -149,7 +152,7 @@ impl IndexeddbCryptoStore { let name = format!("{prefix:0}::matrix-sdk-crypto"); // Open my_db v1 - let mut db_req: OpenDbRequest = IdbDatabase::open_u32(&name, 3)?; + let mut db_req: OpenDbRequest = IdbDatabase::open_u32(&name, 4)?; db_req.set_on_upgrade_needed(Some(|evt: &IdbVersionChangeEvent| -> Result<(), JsValue> { // Even if the web-sys bindings expose the version as a f64, the IndexedDB API // works with an unsigned integer. @@ -203,6 +206,12 @@ impl IndexeddbCryptoStore { db.create_object_store(keys::DIRECT_WITHHELD_INFO)?; } + if old_version < 4 { + let db = evt.db(); + + db.create_object_store(keys::SECRETS_INBOX)?; + } + Ok(()) })); @@ -395,6 +404,7 @@ impl_crypto_store! { (!changes.message_hashes.is_empty(), keys::OLM_HASHES), (!changes.withheld_session_info.is_empty(), keys::DIRECT_WITHHELD_INFO), (!changes.room_settings.is_empty(), keys::ROOM_SETTINGS), + (!changes.secrets.is_empty(), keys::SECRETS_INBOX), ] .iter() .filter_map(|(id, key)| if *id { Some(*key) } else { None }) @@ -591,6 +601,17 @@ impl_crypto_store! { } } + if !changes.secrets.is_empty() { + let secrets_store = tx.object_store(keys::SECRETS_INBOX)?; + + for secret in changes.secrets { + let key = self.encode_key(keys::SECRETS_INBOX, (secret.secret_name.as_str(), secret.event.content.request_id.as_str())); + let value = self.serialize_value(&secret)?; + + secrets_store.put_key_val(&key, &value)?; + } + } + tx.await.into_result()?; // all good, let's update our caches:indexeddb @@ -906,6 +927,40 @@ impl_crypto_store! { .is_some()) } + async fn get_secrets_from_inbox( + &self, + secret_name: &SecretName, + ) -> Result> { + let range = self.encode_to_range(keys::SECRETS_INBOX, secret_name.as_str())?; + + self + .inner + .transaction_on_one_with_mode(keys::SECRETS_INBOX, IdbTransactionMode::Readonly)? + .object_store(keys::SECRETS_INBOX)? + .get_all_with_key(&range)? + .await? + .iter() + .map(|d| { + let secret = self.deserialize_value(d)?; + Ok(secret) + }).collect() + } + + async fn delete_secrets_from_inbox( + &self, + secret_name: &SecretName, + ) -> Result<()> { + let range = self.encode_to_range(keys::SECRETS_INBOX, secret_name.as_str())?; + + self + .inner + .transaction_on_one_with_mode(keys::SECRETS_INBOX, IdbTransactionMode::Readonly)? + .object_store(keys::SECRETS_INBOX)? + .delete(&range)?; + + Ok(()) + } + async fn get_secret_request_by_info( &self, key_info: &SecretInfo, diff --git a/crates/matrix-sdk-sqlite/migrations/crypto_store/008_secret_inbox.sql b/crates/matrix-sdk-sqlite/migrations/crypto_store/008_secret_inbox.sql new file mode 100644 index 00000000000..20b2c2a4f66 --- /dev/null +++ b/crates/matrix-sdk-sqlite/migrations/crypto_store/008_secret_inbox.sql @@ -0,0 +1,4 @@ +CREATE TABLE "secrets" ( + "secret_name" BLOB NOT NULL, + "data" BLOB NOT NULL +); diff --git a/crates/matrix-sdk-sqlite/src/crypto_store.rs b/crates/matrix-sdk-sqlite/src/crypto_store.rs index 7811013f938..735423c823f 100644 --- a/crates/matrix-sdk-sqlite/src/crypto_store.rs +++ b/crates/matrix-sdk-sqlite/src/crypto_store.rs @@ -29,12 +29,13 @@ use matrix_sdk_crypto::{ }, store::{caches::SessionStore, BackupKeys, Changes, CryptoStore, RoomKeyCounts, RoomSettings}, types::events::room_key_withheld::RoomKeyWithheldEvent, - GossipRequest, ReadOnlyAccount, ReadOnlyDevice, ReadOnlyUserIdentities, SecretInfo, - TrackedUser, + GossipRequest, GossippedSecret, ReadOnlyAccount, ReadOnlyDevice, ReadOnlyUserIdentities, + SecretInfo, TrackedUser, }; use matrix_sdk_store_encryption::StoreCipher; use ruma::{ - DeviceId, MilliSecondsSinceUnixEpoch, OwnedDeviceId, OwnedUserId, RoomId, TransactionId, UserId, + events::secret::request::SecretName, DeviceId, MilliSecondsSinceUnixEpoch, OwnedDeviceId, + OwnedUserId, RoomId, TransactionId, UserId, }; use rusqlite::OptionalExtension; use serde::{de::DeserializeOwned, Serialize}; @@ -195,7 +196,7 @@ impl SqliteCryptoStore { } } -const DATABASE_VERSION: u8 = 7; +const DATABASE_VERSION: u8 = 8; /// Run migrations for the given version of the database. async fn run_migrations(conn: &SqliteConn, version: u8) -> Result<()> { @@ -263,6 +264,13 @@ async fn run_migrations(conn: &SqliteConn, version: u8) -> Result<()> { .await?; } + if version < 8 { + conn.with_transaction(|txn| { + txn.execute_batch(include_str!("../migrations/crypto_store/008_secret_inbox.sql")) + }) + .await?; + } + conn.set_kv("version", vec![DATABASE_VERSION]).await?; Ok(()) @@ -308,6 +316,8 @@ trait SqliteConnectionExt { ) -> rusqlite::Result<()>; fn set_room_settings(&self, room_id: &[u8], data: &[u8]) -> rusqlite::Result<()>; + + fn set_secret(&self, request_id: &[u8], data: &[u8]) -> rusqlite::Result<()>; } impl SqliteConnectionExt for rusqlite::Connection { @@ -424,6 +434,16 @@ impl SqliteConnectionExt for rusqlite::Connection { )?; Ok(()) } + + fn set_secret(&self, secret_name: &[u8], data: &[u8]) -> rusqlite::Result<()> { + self.execute( + "INSERT INTO secrets (secret_name, data) + VALUES (?1, ?2)", + (secret_name, data), + )?; + + Ok(()) + } } #[async_trait] @@ -592,6 +612,19 @@ trait SqliteObjectCryptoStoreExt: SqliteObjectExt { Ok(()) } + async fn get_secrets_from_inbox(&self, secret_name: Key) -> Result>> { + Ok(self + .prepare("SELECT data FROM secrets WHERE secret_name = ?", |mut stmt| { + stmt.query((secret_name,))?.mapped(|row| row.get(0)).collect() + }) + .await?) + } + + async fn delete_secrets_from_inbox(&self, secret_name: Key) -> Result<()> { + self.execute("DELETE FROM secrets WHERE secret_name = ?", (secret_name,)).await?; + Ok(()) + } + async fn get_direct_withheld_info( &self, session_id: Key, @@ -805,6 +838,12 @@ impl CryptoStore for SqliteCryptoStore { txn.set_room_settings(&room_id, &value)?; } + for secret in changes.secrets { + let secret_name = this.encode_key("secrets", secret.secret_name.to_string()); + let value = this.serialize_json(&secret)?; + txn.set_secret(&secret_name, &value)?; + } + Ok::<_, Error>(()) }) .await?; @@ -1062,6 +1101,26 @@ impl CryptoStore for SqliteCryptoStore { Ok(self.acquire().await?.delete_key_request(request_id).await?) } + async fn get_secrets_from_inbox( + &self, + secret_name: &SecretName, + ) -> Result> { + let secret_name = self.encode_key("secrets", secret_name.to_string()); + + self.acquire() + .await? + .get_secrets_from_inbox(secret_name) + .await? + .into_iter() + .map(|value| self.deserialize_json(value.as_ref())) + .collect() + } + + async fn delete_secrets_from_inbox(&self, secret_name: &SecretName) -> Result<()> { + let secret_name = self.encode_key("secrets", secret_name.to_string()); + self.acquire().await?.delete_secrets_from_inbox(secret_name).await + } + async fn get_withheld_info( &self, room_id: &RoomId, From da482bc0740adf7a6a02e2165ea0d8f41a02fb7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 14 Jun 2023 14:58:41 +0200 Subject: [PATCH 2/4] Broadcast valid secrets we receive over m.secret.send --- .../src/file_encryption/key_export.rs | 9 +- .../src/gossiping/machine.rs | 85 ++++++++++++++++++- crates/matrix-sdk-crypto/src/machine.rs | 69 +++++++++------ .../src/store/integration_tests.rs | 26 +++++- .../src/store/memorystore.rs | 14 ++- crates/matrix-sdk-crypto/src/store/mod.rs | 73 +++++++++++++++- 6 files changed, 241 insertions(+), 35 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/file_encryption/key_export.rs b/crates/matrix-sdk-crypto/src/file_encryption/key_export.rs index 637de86142b..8efec04d530 100644 --- a/crates/matrix-sdk-crypto/src/file_encryption/key_export.rs +++ b/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?; diff --git a/crates/matrix-sdk-crypto/src/gossiping/machine.rs b/crates/matrix-sdk-crypto/src/gossiping/machine.rs index 39f96d7a862..084c8233b52 100644 --- a/crates/matrix-sdk-crypto/src/gossiping/machine.rs +++ b/crates/matrix-sdk-crypto/src/gossiping/machine.rs @@ -819,6 +819,7 @@ impl GossipMachine { // // 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); } @@ -1271,7 +1272,7 @@ mod tests { let content = extract_content(recipient, request); let content: C = content .deserialize_as() - .expect("We can always deserialize the to-device event content"); + .expect(&format!("We can always deserialize the to-device event content {content:?}")); ToDeviceEvent::new(sender.to_owned(), content) } @@ -1751,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() { diff --git a/crates/matrix-sdk-crypto/src/machine.rs b/crates/matrix-sdk-crypto/src/machine.rs index 13cd6c3fa49..b88c93d2642 100644 --- a/crates/matrix-sdk-crypto/src/machine.rs +++ b/crates/matrix-sdk-crypto/src/machine.rs @@ -122,7 +122,7 @@ pub struct OlmMachineInner { verification_machine: VerificationMachine, /// The state machine that is responsible to handle outgoing and incoming /// key requests. - key_request_machine: GossipMachine, + pub(crate) key_request_machine: GossipMachine, /// State machine handling public user identities and devices, keeping track /// of when a key query needs to be done and handling one. identity_manager: IdentityManager, @@ -1979,8 +1979,11 @@ pub(crate) mod tests { .unwrap() } - pub(crate) async fn get_prepared_machine(use_fallback_key: bool) -> (OlmMachine, OneTimeKeys) { - let machine = OlmMachine::new(user_id(), bob_device_id()).await; + pub(crate) async fn get_prepared_machine( + user_id: &UserId, + use_fallback_key: bool, + ) -> (OlmMachine, OneTimeKeys) { + let machine = OlmMachine::new(user_id, bob_device_id()).await; machine.account().generate_fallback_key_helper().await; machine.account().update_uploaded_key_count(0); let request = machine.keys_for_upload().await.expect("Can't prepare initial key upload"); @@ -1993,7 +1996,7 @@ pub(crate) mod tests { } async fn get_machine_after_query() -> (OlmMachine, OneTimeKeys) { - let (machine, otk) = get_prepared_machine(false).await; + let (machine, otk) = get_prepared_machine(user_id(), false).await; let response = keys_query_response(); let req_id = TransactionId::new(); @@ -2002,12 +2005,15 @@ pub(crate) mod tests { (machine, otk) } - async fn get_machine_pair(use_fallback_key: bool) -> (OlmMachine, OlmMachine, OneTimeKeys) { - let (bob, otk) = get_prepared_machine(use_fallback_key).await; + async fn get_machine_pair( + alice: &UserId, + bob: &UserId, + use_fallback_key: bool, + ) -> (OlmMachine, OlmMachine, OneTimeKeys) { + let (bob, otk) = get_prepared_machine(bob, use_fallback_key).await; - let alice_id = alice_id(); let alice_device = alice_device_id(); - let alice = OlmMachine::new(alice_id, alice_device).await; + let alice = OlmMachine::new(alice, alice_device).await; let alice_device = ReadOnlyDevice::from_machine(&alice).await; let bob_device = ReadOnlyDevice::from_machine(&bob).await; @@ -2017,8 +2023,12 @@ pub(crate) mod tests { (alice, bob, otk) } - async fn get_machine_pair_with_session(use_fallback_key: bool) -> (OlmMachine, OlmMachine) { - let (alice, bob, one_time_keys) = get_machine_pair(use_fallback_key).await; + async fn get_machine_pair_with_session( + alice: &UserId, + bob: &UserId, + use_fallback_key: bool, + ) -> (OlmMachine, OlmMachine) { + let (alice, bob, one_time_keys) = get_machine_pair(alice, bob, use_fallback_key).await; let mut bob_keys = BTreeMap::new(); @@ -2037,8 +2047,12 @@ pub(crate) mod tests { (alice, bob) } - async fn get_machine_pair_with_setup_sessions() -> (OlmMachine, OlmMachine) { - let (alice, bob) = get_machine_pair_with_session(false).await; + pub(crate) async fn get_machine_pair_with_setup_sessions( + alice: &UserId, + bob: &UserId, + use_fallback_key: bool, + ) -> (OlmMachine, OlmMachine) { + let (alice, bob) = get_machine_pair_with_session(alice, bob, use_fallback_key).await; let bob_device = alice.get_device(bob.user_id(), bob.device_id(), None).await.unwrap().unwrap(); @@ -2210,7 +2224,7 @@ pub(crate) mod tests { #[async_test] async fn test_keys_query() { - let (machine, _) = get_prepared_machine(false).await; + let (machine, _) = get_prepared_machine(user_id(), false).await; let response = keys_query_response(); let alice_id = user_id!("@alice:example.org"); let alice_device_id: &DeviceId = device_id!("JLAFKJWSCS"); @@ -2258,7 +2272,8 @@ pub(crate) mod tests { #[async_test] async fn test_session_creation() { - let (alice_machine, bob_machine, mut one_time_keys) = get_machine_pair(false).await; + let (alice_machine, bob_machine, mut one_time_keys) = + get_machine_pair(alice_id(), user_id(), false).await; let (device_key_id, one_time_key) = one_time_keys.pop_first().unwrap(); create_session( @@ -2282,7 +2297,8 @@ pub(crate) mod tests { #[async_test] async fn getting_most_recent_session() { - let (alice_machine, bob_machine, mut one_time_keys) = get_machine_pair(false).await; + let (alice_machine, bob_machine, mut one_time_keys) = + get_machine_pair(alice_id(), user_id(), false).await; let (device_key_id, one_time_key) = one_time_keys.pop_first().unwrap(); let device = alice_machine @@ -2355,7 +2371,8 @@ pub(crate) mod tests { } async fn olm_encryption_test(use_fallback_key: bool) { - let (alice, bob) = get_machine_pair_with_session(use_fallback_key).await; + let (alice, bob) = + get_machine_pair_with_session(alice_id(), user_id(), use_fallback_key).await; let bob_device = alice.get_device(bob.user_id(), bob.device_id(), None).await.unwrap().unwrap(); @@ -2403,7 +2420,7 @@ pub(crate) mod tests { #[async_test] async fn test_room_key_sharing() { - let (alice, bob) = get_machine_pair_with_session(false).await; + let (alice, bob) = get_machine_pair_with_session(alice_id(), user_id(), false).await; let room_id = room_id!("!test:example.org"); @@ -2443,7 +2460,7 @@ pub(crate) mod tests { #[async_test] async fn test_megolm_encryption() { - let (alice, bob) = get_machine_pair_with_setup_sessions().await; + let (alice, bob) = get_machine_pair_with_setup_sessions(alice_id(), user_id(), false).await; let room_id = room_id!("!test:example.org"); let to_device_requests = alice @@ -2522,7 +2539,7 @@ pub(crate) mod tests { #[async_test] async fn test_withheld_unverified() { - let (alice, bob) = get_machine_pair_with_setup_sessions().await; + let (alice, bob) = get_machine_pair_with_setup_sessions(alice_id(), user_id(), false).await; let room_id = room_id!("!test:example.org"); let encryption_settings = EncryptionSettings::default(); @@ -2593,7 +2610,7 @@ pub(crate) mod tests { assert_matches!(strict, ShieldState::$strict { .. }); }; } - let (alice, bob) = get_machine_pair_with_setup_sessions().await; + let (alice, bob) = get_machine_pair_with_setup_sessions(alice_id(), user_id(), false).await; let room_id = room_id!("!test:example.org"); let to_device_requests = alice @@ -2911,7 +2928,7 @@ pub(crate) mod tests { #[async_test] async fn test_verification_states_multiple_device() { - let (bob, _) = get_prepared_machine(false).await; + let (bob, _) = get_prepared_machine(user_id(), false).await; let other_user_id = user_id!("@web2:localhost:8482"); @@ -2981,7 +2998,7 @@ pub(crate) mod tests { #[async_test] #[cfg(feature = "automatic-room-key-forwarding")] async fn test_query_ratcheted_key() { - let (alice, bob) = get_machine_pair_with_setup_sessions().await; + let (alice, bob) = get_machine_pair_with_setup_sessions(alice_id(), user_id(), false).await; let room_id = room_id!("!test:example.org"); // Need a second bob session to check gossiping @@ -3053,7 +3070,7 @@ pub(crate) mod tests { #[async_test] async fn interactive_verification() { - let (alice, bob) = get_machine_pair_with_setup_sessions().await; + let (alice, bob) = get_machine_pair_with_setup_sessions(alice_id(), user_id(), false).await; let bob_device = alice.get_device(bob.user_id(), bob.device_id(), None).await.unwrap().unwrap(); @@ -3130,7 +3147,7 @@ pub(crate) mod tests { #[async_test] async fn interactive_verification_started_from_request() { - let (alice, bob) = get_machine_pair_with_setup_sessions().await; + let (alice, bob) = get_machine_pair_with_setup_sessions(alice_id(), user_id(), false).await; // ---------------------------------------------------------------------------- // On Alice's device: @@ -3299,7 +3316,7 @@ pub(crate) mod tests { #[async_test] async fn room_key_over_megolm() { - let (alice, bob) = get_machine_pair_with_setup_sessions().await; + let (alice, bob) = get_machine_pair_with_setup_sessions(alice_id(), user_id(), false).await; let room_id = room_id!("!test:example.org"); let to_device_requests = alice @@ -3360,7 +3377,7 @@ pub(crate) mod tests { #[async_test] async fn room_key_with_fake_identity_keys() { let room_id = room_id!("!test:localhost"); - let (alice, _) = get_machine_pair_with_setup_sessions().await; + let (alice, _) = get_machine_pair_with_setup_sessions(alice_id(), user_id(), false).await; let device = ReadOnlyDevice::from_machine(&alice).await; alice.store().save_devices(&[device]).await.unwrap(); diff --git a/crates/matrix-sdk-crypto/src/store/integration_tests.rs b/crates/matrix-sdk-crypto/src/store/integration_tests.rs index 46075667a76..1dab8519c4f 100644 --- a/crates/matrix-sdk-crypto/src/store/integration_tests.rs +++ b/crates/matrix-sdk-crypto/src/store/integration_tests.rs @@ -23,7 +23,7 @@ macro_rules! cryptostore_integration_tests { PrivateCrossSigningIdentity, ReadOnlyAccount, Session, }, store::{ - Changes, CryptoStore, DeviceChanges, + BackupKeys, Changes, CryptoStore, DeviceChanges, GossipRequest, IdentityChanges, RecoveryKey, RoomSettings, }, testing::{get_device, get_other_identity, get_own_identity}, @@ -836,6 +836,30 @@ macro_rules! cryptostore_integration_tests { assert_eq!(None, loaded_settings_3); } + #[async_test] + async fn backup_keys_saving() { + let (account, store) = get_loaded_store("backup_keys_saving").await; + + let restored = store.load_backup_keys().await.unwrap(); + assert!(restored.recovery_key.is_none(), "Initially no backup recovery key should be present"); + + let recovery_key = Some(RecoveryKey::new().unwrap()); + + let changes = Changes { recovery_key, ..Default::default() }; + store.save_changes(changes).await.unwrap(); + + let restored = store.load_backup_keys().await.unwrap(); + assert!(restored.recovery_key.is_some(), "We should be able to restore a backup recovery key"); + assert!(restored.backup_version.is_none(), "The backup version should still be None"); + + let changes = Changes { backup_version: Some("some_version".to_owned()), ..Default::default() }; + store.save_changes(changes).await.unwrap(); + + let restored = store.load_backup_keys().await.unwrap(); + assert!(restored.recovery_key.is_some(), "The backup recovery key should still be known"); + assert!(restored.backup_version.is_some(), "The backup version should now be Some as well"); + } + #[async_test] async fn custom_value_saving() { let (account, store) = get_loaded_store("custom_value_saving").await; diff --git a/crates/matrix-sdk-crypto/src/store/memorystore.rs b/crates/matrix-sdk-crypto/src/store/memorystore.rs index e717f43e148..5bd068dcd44 100644 --- a/crates/matrix-sdk-crypto/src/store/memorystore.rs +++ b/crates/matrix-sdk-crypto/src/store/memorystore.rs @@ -25,7 +25,7 @@ use ruma::{ events::secret::request::SecretName, DeviceId, OwnedDeviceId, OwnedRoomId, OwnedTransactionId, OwnedUserId, RoomId, TransactionId, UserId, }; -use tokio::sync::Mutex; +use tokio::sync::{Mutex, RwLock}; use tracing::warn; use super::{ @@ -64,6 +64,7 @@ pub struct MemoryStore { custom_values: Arc>>, leases: Arc>, secret_inbox: Arc>>, + backup_keys: Arc>, } impl Default for MemoryStore { @@ -79,6 +80,7 @@ impl Default for MemoryStore { direct_withheld_info: Default::default(), custom_values: Default::default(), leases: Default::default(), + backup_keys: Default::default(), secret_inbox: Default::default(), } } @@ -161,6 +163,14 @@ impl CryptoStore for MemoryStore { self.key_requests_by_info.insert(info_string, id); } + if let Some(key) = changes.recovery_key { + self.backup_keys.write().await.recovery_key = Some(key); + } + + if let Some(version) = changes.backup_version { + self.backup_keys.write().await.backup_version = Some(version); + } + for secret in changes.secrets { self.secret_inbox.entry(secret.secret_name.to_string()).or_default().push(secret); } @@ -233,7 +243,7 @@ impl CryptoStore for MemoryStore { } async fn load_backup_keys(&self) -> Result { - Ok(BackupKeys::default()) + Ok(self.backup_keys.read().await.to_owned()) } async fn get_outbound_group_session(&self, _: &RoomId) -> Result> { diff --git a/crates/matrix-sdk-crypto/src/store/mod.rs b/crates/matrix-sdk-crypto/src/store/mod.rs index 44be6a9e639..1b4eb967dce 100644 --- a/crates/matrix-sdk-crypto/src/store/mod.rs +++ b/crates/matrix-sdk-crypto/src/store/mod.rs @@ -133,6 +133,9 @@ struct StoreInner { /// The sender side of a broadcast stream that is notified whenever we get /// an update to an inbound group session. room_keys_received_sender: broadcast::Sender>, + + /// TODO + secrets_broadcaster: broadcast::Sender, } #[derive(Default, Debug)] @@ -204,7 +207,7 @@ pub struct DeviceChanges { } /// The private part of a backup key. -#[derive(Zeroize, Deserialize, Serialize)] +#[derive(Clone, Zeroize, Deserialize, Serialize)] #[zeroize(drop)] #[serde(transparent)] pub struct RecoveryKey { @@ -261,7 +264,7 @@ pub struct RoomKeyCounts { } /// Stored versions of the backup keys. -#[derive(Default, Debug)] +#[derive(Default, Clone, Debug)] pub struct BackupKeys { /// The recovery key, the one used to decrypt backed up room keys. pub recovery_key: Option, @@ -381,6 +384,8 @@ impl Store { verification_machine: VerificationMachine, ) -> Self { let (room_keys_received_sender, _) = broadcast::channel(10); + let (secrets_broadcaster, _) = broadcast::channel(10); + let inner = Arc::new(StoreInner { user_id, identity, @@ -392,7 +397,9 @@ impl Store { tracked_users_loaded: AtomicBool::new(false), tracked_user_loading_lock: Mutex::new(()), room_keys_received_sender, + secrets_broadcaster, }); + Self { inner } } @@ -433,6 +440,8 @@ impl Store { let room_key_updates: Vec<_> = changes.inbound_group_sessions.iter().map(RoomKeyInfo::from).collect(); + let secrets = changes.secrets.to_owned(); + self.inner.store.save_changes(changes).await?; if !room_key_updates.is_empty() { @@ -440,6 +449,10 @@ impl Store { let _ = self.inner.room_keys_received_sender.send(room_key_updates); } + for secret in secrets { + let _ = self.inner.secrets_broadcaster.send(secret); + } + Ok(()) } @@ -999,6 +1012,62 @@ impl Store { pub fn create_store_lock(&self, lock_key: String, lock_value: String) -> CryptoStoreLock { CryptoStoreLock::new(self.inner.store.clone(), lock_key, lock_value) } + + /// Receive notifications of gossipped secrets being received and stored in + /// the secret inbox as a [`Stream`]. + /// + /// The gossipped secrets are received using the `m.secret.send` event type + /// and are guaranteed to have been received over a 1-to-1 Olm + /// [`Session`] from a verified [`Device`]. + /// + /// The [`GossippedSecret`] can also be later found in the secret inbox and + /// retrieved using the [`CryptoStore::get_secrets_from_inbox()`] method. + /// + /// After a suitable secret of a certain type has been found it can be + /// removed from the store + /// using the [`CryptoStore::delete_secrets_from_inbox()`] method. + /// + /// The only secret this will currently broadcast is the + /// `m.megolm_backup.v1`. + /// + /// If the reader of the stream lags too far behind, a warning will be + /// logged and items will be dropped. + /// + /// # Examples + /// + /// ```no_run + /// # use matrix_sdk_crypto::OlmMachine; + /// # use ruma::{device_id, user_id}; + /// # use futures_util::{pin_mut, StreamExt}; + /// # let alice = user_id!("@alice:example.org").to_owned(); + /// # futures_executor::block_on(async { + /// # let machine = OlmMachine::new(&alice, device_id!("DEVICEID")).await; + /// + /// let secret_stream = machine.store().secrets_stream(); + /// pin_mut!(secret_stream); + /// + /// for secret in secret_stream.next().await { + /// // Accept the secret if it's valid, then delete all the secrets of this type. + /// machine.store().delete_secrets_from_inbox(&secret.secret_name); + /// } + /// # }); + /// ``` + pub fn secrets_stream(&self) -> impl Stream { + let stream = BroadcastStream::new(self.inner.secrets_broadcaster.subscribe()); + + // the raw BroadcastStream gives us Results which can fail with + // BroadcastStreamRecvError if the reader falls behind. That's annoying to work + // with, so here we just drop the errors. + stream.filter_map(|result| async move { + match result { + Ok(r) => Some(r), + Err(BroadcastStreamRecvError::Lagged(lag)) => { + warn!("secrets_stream missed {} updates", lag); + None + } + } + }) + } } impl Deref for Store { From ce1fd33c03c67afc86690c275d2c5cceba25a7f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 6 Jul 2023 11:15:33 +0200 Subject: [PATCH 3/4] fixup! Broadcast valid secrets we receive over m.secret.send --- crates/matrix-sdk-crypto/src/gossiping/machine.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/gossiping/machine.rs b/crates/matrix-sdk-crypto/src/gossiping/machine.rs index 084c8233b52..25e495ebaf0 100644 --- a/crates/matrix-sdk-crypto/src/gossiping/machine.rs +++ b/crates/matrix-sdk-crypto/src/gossiping/machine.rs @@ -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(&format!("We can always deserialize the to-device event content {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) } From 32738f30d65a96aa4a2d7689d075800600b9d6a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 10 Jul 2023 13:34:49 +0200 Subject: [PATCH 4/4] Update crates/matrix-sdk-crypto/src/store/mod.rs Co-authored-by: Jonas Platte --- crates/matrix-sdk-crypto/src/store/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk-crypto/src/store/mod.rs b/crates/matrix-sdk-crypto/src/store/mod.rs index 1b4eb967dce..5bf28f82868 100644 --- a/crates/matrix-sdk-crypto/src/store/mod.rs +++ b/crates/matrix-sdk-crypto/src/store/mod.rs @@ -1062,7 +1062,7 @@ impl Store { match result { Ok(r) => Some(r), Err(BroadcastStreamRecvError::Lagged(lag)) => { - warn!("secrets_stream missed {} updates", lag); + warn!("secrets_stream missed {lag} updates"); None } }