From f59054386530a9aa3384f1de45da7bc6b2542da7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 13 Sep 2024 17:32:15 +0200 Subject: [PATCH] crypto: Avoid deep copying the OlmMachine when creating a NotificationClient MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NotificationClient, responsible for handling, fetching, and potentially decrypting events received via push notifications, creates a copy of the main Client object. During this process, the Client object is adjusted to use an in-memory state store to prevent concurrency issues from multiple sync loops attempting to write to the same database. This copying unintentionally recreated the OlmMachine with fresh data loaded from the database. If both Client instances were used for syncing without proper cross-process locking, forks of the vodozemac Account and Olm Sessions could be created and later persisted to the database. This behavior can lead to the duplication of one-time keys, cause sessions to lose their ability to decrypt messages, and result in the generation of undecryptable messages on the recipient’s side. --- crates/matrix-sdk-base/src/client.rs | 33 +++++++++++++++++++++++----- crates/matrix-sdk/src/client/mod.rs | 19 +--------------- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 211499d767e..586192b397c 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -162,21 +162,42 @@ impl BaseClient { /// Clones the current base client to use the same crypto store but a /// different, in-memory store config, and resets transient state. #[cfg(feature = "e2e-encryption")] - pub fn clone_with_in_memory_state_store(&self) -> Self { + pub async fn clone_with_in_memory_state_store(&self) -> Result { let config = StoreConfig::new().state_store(MemoryStore::new()); let config = config.crypto_store(self.crypto_store.clone()); - let mut result = Self::with_store_config(config); - result.room_key_recipient_strategy = self.room_key_recipient_strategy.clone(); - result + let copy = Self { + store: Store::new(config.state_store), + event_cache_store: config.event_cache_store, + // We copy the crypto store as well as the `OlmMachine` for two reasons: + // 1. The `self.crypto_store` is the same as the one used inside the `OlmMachine`. + // 2. We need to ensure that the parent and child use the same data and caches inside + // the `OlmMachine` so the various ratchets and places where new randomness gets + // introduced don't diverge, i.e. one-time keys that get generated by the Olm Account + // or Olm sessions when they encrypt or decrypt messages. + crypto_store: self.crypto_store.clone(), + olm_machine: self.olm_machine.clone(), + ignore_user_list_changes: Default::default(), + room_info_notable_update_sender: self.room_info_notable_update_sender.clone(), + room_key_recipient_strategy: self.room_key_recipient_strategy.clone(), + }; + + if let Some(session_meta) = self.session_meta().cloned() { + copy.store + .set_session_meta(session_meta, ©.room_info_notable_update_sender) + .await?; + } + + Ok(copy) } /// Clones the current base client to use the same crypto store but a /// different, in-memory store config, and resets transient state. #[cfg(not(feature = "e2e-encryption"))] - pub fn clone_with_in_memory_state_store(&self) -> Self { + #[allow(clippy::unused_async)] + pub async fn clone_with_in_memory_state_store(&self) -> Result { let config = StoreConfig::new().state_store(MemoryStore::new()); - Self::with_store_config(config) + Ok(Self::with_store_config(config)) } /// Get the session meta information. diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index ae2d4ad608b..18c190907d4 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -2204,7 +2204,7 @@ impl Client { #[cfg(feature = "experimental-sliding-sync")] self.sliding_sync_version(), self.inner.http_client.clone(), - self.inner.base_client.clone_with_in_memory_state_store(), + self.inner.base_client.clone_with_in_memory_state_store().await?, self.inner.server_capabilities.read().await.clone(), self.inner.respect_login_well_known, self.inner.event_cache.clone(), @@ -2215,23 +2215,6 @@ impl Client { .await, }; - // Copy the parent's session meta into the child. This initializes the in-memory - // state store of the child client with `SessionMeta`, and regenerates - // the `OlmMachine` if needs be. - // - // Note: we don't need to do a full `restore_session`, because this would - // overwrite the session information shared with the parent too, and it - // must be initialized at most once. - if let Some(session) = self.session() { - client - .set_session_meta( - session.into_meta(), - #[cfg(feature = "e2e-encryption")] - None, - ) - .await?; - } - Ok(client) }