Skip to content

Commit f590543

Browse files
committed
crypto: Avoid deep copying the OlmMachine when creating a NotificationClient
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.
1 parent b3da70c commit f590543

File tree

2 files changed

+28
-24
lines changed

2 files changed

+28
-24
lines changed

crates/matrix-sdk-base/src/client.rs

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -162,21 +162,42 @@ impl BaseClient {
162162
/// Clones the current base client to use the same crypto store but a
163163
/// different, in-memory store config, and resets transient state.
164164
#[cfg(feature = "e2e-encryption")]
165-
pub fn clone_with_in_memory_state_store(&self) -> Self {
165+
pub async fn clone_with_in_memory_state_store(&self) -> Result<Self> {
166166
let config = StoreConfig::new().state_store(MemoryStore::new());
167167
let config = config.crypto_store(self.crypto_store.clone());
168168

169-
let mut result = Self::with_store_config(config);
170-
result.room_key_recipient_strategy = self.room_key_recipient_strategy.clone();
171-
result
169+
let copy = Self {
170+
store: Store::new(config.state_store),
171+
event_cache_store: config.event_cache_store,
172+
// We copy the crypto store as well as the `OlmMachine` for two reasons:
173+
// 1. The `self.crypto_store` is the same as the one used inside the `OlmMachine`.
174+
// 2. We need to ensure that the parent and child use the same data and caches inside
175+
// the `OlmMachine` so the various ratchets and places where new randomness gets
176+
// introduced don't diverge, i.e. one-time keys that get generated by the Olm Account
177+
// or Olm sessions when they encrypt or decrypt messages.
178+
crypto_store: self.crypto_store.clone(),
179+
olm_machine: self.olm_machine.clone(),
180+
ignore_user_list_changes: Default::default(),
181+
room_info_notable_update_sender: self.room_info_notable_update_sender.clone(),
182+
room_key_recipient_strategy: self.room_key_recipient_strategy.clone(),
183+
};
184+
185+
if let Some(session_meta) = self.session_meta().cloned() {
186+
copy.store
187+
.set_session_meta(session_meta, &copy.room_info_notable_update_sender)
188+
.await?;
189+
}
190+
191+
Ok(copy)
172192
}
173193

174194
/// Clones the current base client to use the same crypto store but a
175195
/// different, in-memory store config, and resets transient state.
176196
#[cfg(not(feature = "e2e-encryption"))]
177-
pub fn clone_with_in_memory_state_store(&self) -> Self {
197+
#[allow(clippy::unused_async)]
198+
pub async fn clone_with_in_memory_state_store(&self) -> Result<Self> {
178199
let config = StoreConfig::new().state_store(MemoryStore::new());
179-
Self::with_store_config(config)
200+
Ok(Self::with_store_config(config))
180201
}
181202

182203
/// Get the session meta information.

crates/matrix-sdk/src/client/mod.rs

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2204,7 +2204,7 @@ impl Client {
22042204
#[cfg(feature = "experimental-sliding-sync")]
22052205
self.sliding_sync_version(),
22062206
self.inner.http_client.clone(),
2207-
self.inner.base_client.clone_with_in_memory_state_store(),
2207+
self.inner.base_client.clone_with_in_memory_state_store().await?,
22082208
self.inner.server_capabilities.read().await.clone(),
22092209
self.inner.respect_login_well_known,
22102210
self.inner.event_cache.clone(),
@@ -2215,23 +2215,6 @@ impl Client {
22152215
.await,
22162216
};
22172217

2218-
// Copy the parent's session meta into the child. This initializes the in-memory
2219-
// state store of the child client with `SessionMeta`, and regenerates
2220-
// the `OlmMachine` if needs be.
2221-
//
2222-
// Note: we don't need to do a full `restore_session`, because this would
2223-
// overwrite the session information shared with the parent too, and it
2224-
// must be initialized at most once.
2225-
if let Some(session) = self.session() {
2226-
client
2227-
.set_session_meta(
2228-
session.into_meta(),
2229-
#[cfg(feature = "e2e-encryption")]
2230-
None,
2231-
)
2232-
.await?;
2233-
}
2234-
22352218
Ok(client)
22362219
}
22372220

0 commit comments

Comments
 (0)