Skip to content

Commit

Permalink
crypto: Avoid deep copying the OlmMachine when creating a Notificatio…
Browse files Browse the repository at this point in the history
…nClient

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.
  • Loading branch information
poljar committed Sep 16, 2024
1 parent b3da70c commit f590543
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 24 deletions.
33 changes: 27 additions & 6 deletions crates/matrix-sdk-base/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self> {
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, &copy.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<Self> {
let config = StoreConfig::new().state_store(MemoryStore::new());
Self::with_store_config(config)
Ok(Self::with_store_config(config))
}

/// Get the session meta information.
Expand Down
19 changes: 1 addition & 18 deletions crates/matrix-sdk/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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)
}

Expand Down

0 comments on commit f590543

Please sign in to comment.