From cf9b425da505afcf92442af9257681f793be29d2 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 16 May 2024 12:34:48 +0100 Subject: [PATCH 1/6] IndexeddbCryptoStore: pull out some utility functions --- .../src/crypto_store/mod.rs | 107 ++++++++++++------ 1 file changed, 70 insertions(+), 37 deletions(-) diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs index b85c136a332..7d1b1b102d4 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs @@ -274,32 +274,8 @@ impl IndexeddbCryptoStore { /// Open a new `IndexeddbCryptoStore` with given name and passphrase pub async fn open_with_passphrase(prefix: &str, passphrase: &str) -> Result { - let name = format!("{prefix:0}::matrix-sdk-crypto-meta"); - - debug!("IndexedDbCryptoStore: Opening meta-store {name}"); - let mut db_req: OpenDbRequest = IdbDatabase::open_u32(&name, 1)?; - db_req.set_on_upgrade_needed(Some(|evt: &IdbVersionChangeEvent| -> Result<(), JsValue> { - let old_version = evt.old_version() as u32; - if old_version < 1 { - // migrating to version 1 - let db = evt.db(); - - db.create_object_store("matrix-sdk-crypto")?; - } - Ok(()) - })); - - let db: IdbDatabase = db_req.await?; - - let tx: IdbTransaction<'_> = - db.transaction_on_one_with_mode("matrix-sdk-crypto", IdbTransactionMode::Readonly)?; - let ob = tx.object_store("matrix-sdk-crypto")?; - - let store_cipher: Option> = ob - .get(&JsValue::from_str(keys::STORE_CIPHER))? - .await? - .map(|k| k.into_serde()) - .transpose()?; + let db = open_meta_db(prefix).await?; + let store_cipher = load_store_cipher(&db).await?; let store_cipher = match store_cipher { Some(cipher) => { @@ -315,17 +291,9 @@ impl IndexeddbCryptoStore { #[cfg(test)] let export = cipher._insecure_export_fast_for_testing(passphrase); - let tx: IdbTransaction<'_> = db.transaction_on_one_with_mode( - "matrix-sdk-crypto", - IdbTransactionMode::Readwrite, - )?; - let ob = tx.object_store("matrix-sdk-crypto")?; - - ob.put_key_val( - &JsValue::from_str(keys::STORE_CIPHER), - &JsValue::from_serde(&export.map_err(CryptoStoreError::backend)?)?, - )?; - tx.await.into_result()?; + let export = export.map_err(CryptoStoreError::backend)?; + + save_store_cipher(&db, &export).await?; cipher } }; @@ -1289,6 +1257,71 @@ impl Drop for IndexeddbCryptoStore { } } +/// Open the meta store. +/// +/// The meta store contains details about the encryption of the main store. +async fn open_meta_db(prefix: &str) -> Result { + let name = format!("{prefix:0}::matrix-sdk-crypto-meta"); + + debug!("IndexedDbCryptoStore: Opening meta-store {name}"); + let mut db_req: OpenDbRequest = IdbDatabase::open_u32(&name, 1)?; + db_req.set_on_upgrade_needed(Some(|evt: &IdbVersionChangeEvent| -> Result<(), JsValue> { + let old_version = evt.old_version() as u32; + if old_version < 1 { + // migrating to version 1 + let db = evt.db(); + + db.create_object_store("matrix-sdk-crypto")?; + } + Ok(()) + })); + + Ok(db_req.await?) +} + +/// Load the serialised store cipher from the meta store. +/// +/// # Arguments: +/// +/// * `meta_db`: Connection to the meta store, as returned by [`open_meta_db`]. +/// +/// # Returns: +/// +/// The serialised `StoreCipher` object. +async fn load_store_cipher( + meta_db: &IdbDatabase, +) -> Result>, IndexeddbCryptoStoreError> { + let tx: IdbTransaction<'_> = + meta_db.transaction_on_one_with_mode("matrix-sdk-crypto", IdbTransactionMode::Readonly)?; + let ob = tx.object_store("matrix-sdk-crypto")?; + + let store_cipher: Option> = ob + .get(&JsValue::from_str(keys::STORE_CIPHER))? + .await? + .map(|k| k.into_serde()) + .transpose()?; + Ok(store_cipher) +} + +/// Save the serialised store cipher to the meta store. +/// +/// # Arguments: +/// +/// * `meta_db`: Connection to the meta store, as returned by [`open_meta_db`]. +/// * `store_cipher`: The serialised `StoreCipher` object. +async fn save_store_cipher( + db: &IdbDatabase, + export: &Vec, +) -> Result<(), IndexeddbCryptoStoreError> { + let tx: IdbTransaction<'_> = + db.transaction_on_one_with_mode("matrix-sdk-crypto", IdbTransactionMode::Readwrite)?; + let ob = tx.object_store("matrix-sdk-crypto")?; + + ob.put_key_val(&JsValue::from_str(keys::STORE_CIPHER), &JsValue::from_serde(&export)?)?; + tx.await.into_result()?; + Ok(()) +} + /// Fetch items from an object store in batches, transform each item using /// the supplied function, and stuff the transformed items into a single /// vector to return. From 583ec70b0627deb9e19ab02c40b935242784faef Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 16 May 2024 12:43:11 +0100 Subject: [PATCH 2/6] IndexeddbCryptoStore: improve doc --- .../src/crypto_store/mod.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs index 7d1b1b102d4..e97127b8dce 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs @@ -272,7 +272,22 @@ impl IndexeddbCryptoStore { IndexeddbCryptoStore::open_with_store_cipher("crypto", None).await } - /// Open a new `IndexeddbCryptoStore` with given name and passphrase + /// Open an `IndexeddbCryptoStore` with given name and passphrase. + /// + /// If the store previously existed, the encryption cipher is initialised + /// using the given passphrase and the details from the meta store. If the + /// store did not previously exist, a new encryption cipher is derived + /// from the passphrase, and the details are stored to the metastore. + /// + /// The store is then opened, or a new one created, using the encryption + /// cipher. + /// + /// # Arguments + /// + /// * `prefix` - Common prefix for the names of the two IndexedDB stores. + /// * `passphrase` - Passphrase which is used to derive a key to encrypt the + /// key which is used to encrypt the store. Must be the same each time the + /// store is opened. pub async fn open_with_passphrase(prefix: &str, passphrase: &str) -> Result { let db = open_meta_db(prefix).await?; let store_cipher = load_store_cipher(&db).await?; From c097085d095409bc289ac03751e209a751929f82 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 16 May 2024 17:31:13 +0100 Subject: [PATCH 3/6] indexeddb: new method `IndexeddbCryptoStore::open_with_key`. Allow applications which already have a cryptographically-secure key to avoid PBKDF, by passing that key in directly. --- Cargo.lock | 1 + crates/matrix-sdk-indexeddb/Cargo.toml | 1 + .../src/crypto_store/mod.rs | 114 +++++++++++++++++- 3 files changed, 115 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 135078b9694..48ce17cb34c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3388,6 +3388,7 @@ dependencies = [ "matrix-sdk-crypto", "matrix-sdk-store-encryption", "matrix-sdk-test", + "rand", "ruma", "serde", "serde-wasm-bindgen", diff --git a/crates/matrix-sdk-indexeddb/Cargo.toml b/crates/matrix-sdk-indexeddb/Cargo.toml index 3cb85fc1545..718ddff415d 100644 --- a/crates/matrix-sdk-indexeddb/Cargo.toml +++ b/crates/matrix-sdk-indexeddb/Cargo.toml @@ -50,6 +50,7 @@ matrix-sdk-base = { workspace = true, features = ["testing"] } matrix-sdk-common = { workspace = true, features = ["js"] } matrix-sdk-crypto = { workspace = true, features = ["js", "testing"] } matrix-sdk-test = { workspace = true } +rand = { workspace = true } tracing-subscriber = { version = "0.3.18", default-features = false, features = ["registry", "tracing-log"] } uuid = "1.3.0" wasm-bindgen-test = "0.3.33" diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs index e97127b8dce..63e7ec03a14 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs @@ -30,6 +30,7 @@ use matrix_sdk_crypto::{ RoomKeyCounts, RoomSettings, }, types::events::room_key_withheld::RoomKeyWithheldEvent, + vodozemac::base64_encode, Account, GossipRequest, GossippedSecret, ReadOnlyDevice, ReadOnlyUserIdentities, SecretInfo, TrackedUser, }; @@ -320,6 +321,46 @@ impl IndexeddbCryptoStore { IndexeddbCryptoStore::open_with_store_cipher(prefix, Some(store_cipher.into())).await } + /// Open an `IndexeddbCryptoStore` with given name and key. + /// + /// If the store previously existed, the encryption cipher is initialised + /// using the given key and the details from the meta store. If the store + /// did not previously exist, a new encryption cipher is derived from + /// the passphrase, and the details are stored to the metastore. + /// + /// The store is then opened, or a new one created, using the encryption + /// cipher. + /// + /// # Arguments + /// + /// * `prefix` - Common prefix for the names of the two IndexedDB stores. + /// * `key` - key with which to encrypt the key which is used to encrypt the + /// store. Must be the same each time the store is opened. + pub async fn open_with_key(prefix: &str, key: &[u8; 32]) -> Result { + let db = open_meta_db(prefix).await?; + let store_cipher = load_store_cipher(&db).await?; + + let store_cipher = match store_cipher { + Some(cipher) => { + debug!("IndexedDbCryptoStore: decrypting store cipher"); + import_store_cipher_with_key(key, &cipher, &db).await? + } + None => { + debug!("IndexedDbCryptoStore: encrypting new store cipher"); + let cipher = StoreCipher::new().map_err(CryptoStoreError::backend)?; + let export = cipher.export_with_key(key).map_err(CryptoStoreError::backend)?; + save_store_cipher(&db, &export).await?; + cipher + } + }; + + // Must release the database access manually as it's not done when + // dropping it. + db.close(); + + IndexeddbCryptoStore::open_with_store_cipher(prefix, Some(store_cipher.into())).await + } + /// Open a new `IndexeddbCryptoStore` with given name and no passphrase pub async fn open_with_name(name: &str) -> Result { IndexeddbCryptoStore::open_with_store_cipher(name, None).await @@ -1337,6 +1378,38 @@ async fn save_store_cipher( Ok(()) } +/// Given a serialised store cipher, try importing with the given key. +/// +/// This is a helper for [`IndexeddbCryptoStore::open_with_key`]. +async fn import_store_cipher_with_key( + key: &[u8; 32], + serialised_cipher: &[u8], + db: &IdbDatabase, +) -> Result { + let cipher = match StoreCipher::import_with_key(key, serialised_cipher) { + Ok(cipher) => cipher, + Err(matrix_sdk_store_encryption::Error::KdfMismatch) => { + // Old versions of the matrix-js-sdk used to base64-encode their encryption + // key, and pass it into [`IndexeddbCryptoStore::open_with_passphrase`]. For + // backwards compatibility, we fall back to that if we discover we have a cipher + // encrypted with a KDF when we expected it to be encrypted directly with a key. + let cipher = StoreCipher::import(&base64_encode(key), serialised_cipher) + .map_err(|_| CryptoStoreError::UnpicklingError)?; + + // Loading the cipher with the passphrase was successful. Let's update the + // stored version of the cipher so that it is encrypted with a key, + // to save doing this again. + debug!("IndexedDbCryptoStore: Migrating passphrase-encrypted store cipher to key-encryption"); + + let export = cipher.export_with_key(key).map_err(CryptoStoreError::backend)?; + save_store_cipher(db, &export).await?; + cipher + } + Err(_) => Err(CryptoStoreError::UnpicklingError)?, + }; + Ok(cipher) +} + /// Fetch items from an object store in batches, transform each item using /// the supplied function, and stuff the transformed items into a single /// vector to return. @@ -1594,7 +1667,14 @@ mod tests { #[cfg(all(test, target_arch = "wasm32"))] mod encrypted_tests { - use matrix_sdk_crypto::cryptostore_integration_tests; + use matrix_sdk_crypto::{ + cryptostore_integration_tests, + olm::Account, + store::{CryptoStore, PendingChanges}, + vodozemac::base64_encode, + }; + use matrix_sdk_test::async_test; + use ruma::{device_id, user_id}; use super::IndexeddbCryptoStore; @@ -1609,4 +1689,36 @@ mod encrypted_tests { .expect("Can't create a passphrase protected store") } cryptostore_integration_tests!(); + + /// Test that we can migrate a store created with a passphrase, to being + /// encrypted with a key instead. + #[async_test] + async fn migrate_passphrase_to_key() { + let store_name = "test_migrate_passphrase_to_key"; + let passdata: [u8; 32] = rand::random(); + let b64_passdata = base64_encode(passdata); + + // Initialise the store with some account data + let store = IndexeddbCryptoStore::open_with_passphrase(&store_name, &b64_passdata) + .await + .expect("Can't create a passphrase-protected store"); + + store + .save_pending_changes(PendingChanges { + account: Some(Account::with_device_id( + user_id!("@alice:example.org"), + device_id!("ALICEDEVICE"), + )), + }) + .await + .expect("Can't save account"); + + // Now reopen the store, passing the key directly rather than as a b64 string. + let store = IndexeddbCryptoStore::open_with_key(&store_name, &passdata) + .await + .expect("Can't create a key-protected store"); + let loaded_account = + store.load_account().await.expect("Can't load account").expect("Account was not saved"); + assert_eq!(loaded_account.user_id, user_id!("@alice:example.org")); + } } From 1f037e1160ffecacf730468f0d1ec750112e2c1d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 16 May 2024 17:38:16 +0100 Subject: [PATCH 4/6] Update changelog --- crates/matrix-sdk-indexeddb/CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk-indexeddb/CHANGELOG.md b/crates/matrix-sdk-indexeddb/CHANGELOG.md index 17f84826909..276016b1cdd 100644 --- a/crates/matrix-sdk-indexeddb/CHANGELOG.md +++ b/crates/matrix-sdk-indexeddb/CHANGELOG.md @@ -1,4 +1,6 @@ -# unreleased +# UNRELEASED + +- Add new method `IndexeddbCryptoStore::open_with_key`. ([#3423](https://github.com/matrix-org/matrix-rust-sdk/pull/3423)) - `save_change` performance improvement, all encryption and serialization - is done now outside of the db transaction. \ No newline at end of file + is done now outside of the db transaction. From f5280d02e2238502c449aa017e1216ba67d868f6 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 20 May 2024 12:55:05 +0100 Subject: [PATCH 5/6] Use an HKDF for the key --- Cargo.lock | 3 ++ crates/matrix-sdk-indexeddb/Cargo.toml | 3 ++ .../src/crypto_store/mod.rs | 34 +++++++++++++++---- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 48ce17cb34c..34c4bed1f61 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3381,6 +3381,7 @@ dependencies = [ "base64 0.22.1", "getrandom", "gloo-utils", + "hkdf", "indexed_db_futures", "js-sys", "matrix-sdk-base", @@ -3393,6 +3394,7 @@ dependencies = [ "serde", "serde-wasm-bindgen", "serde_json", + "sha2", "thiserror", "tokio", "tracing", @@ -3401,6 +3403,7 @@ dependencies = [ "wasm-bindgen", "wasm-bindgen-test", "web-sys", + "zeroize", ] [[package]] diff --git a/crates/matrix-sdk-indexeddb/Cargo.toml b/crates/matrix-sdk-indexeddb/Cargo.toml index 718ddff415d..5c45fb27b06 100644 --- a/crates/matrix-sdk-indexeddb/Cargo.toml +++ b/crates/matrix-sdk-indexeddb/Cargo.toml @@ -38,6 +38,9 @@ tokio = { workspace = true } tracing = { workspace = true } wasm-bindgen = "0.2.83" web-sys = { version = "0.3.57", features = ["IdbKeyRange"] } +hkdf = "0.12.4" +zeroize.workspace = true +sha2.workspace = true [target.'cfg(target_arch = "wasm32")'.dependencies] # for wasm32 we need to activate this diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs index 63e7ec03a14..3169e73904a 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs @@ -19,6 +19,7 @@ use std::{ use async_trait::async_trait; use gloo_utils::format::JsValueSerdeExt; +use hkdf::Hkdf; use indexed_db_futures::prelude::*; use matrix_sdk_crypto::{ olm::{ @@ -39,6 +40,7 @@ use ruma::{ events::secret::request::SecretName, DeviceId, MilliSecondsSinceUnixEpoch, OwnedDeviceId, RoomId, TransactionId, UserId, }; +use sha2::Sha256; use tokio::sync::Mutex; use tracing::{debug, warn}; use wasm_bindgen::JsValue; @@ -337,18 +339,27 @@ impl IndexeddbCryptoStore { /// * `key` - key with which to encrypt the key which is used to encrypt the /// store. Must be the same each time the store is opened. pub async fn open_with_key(prefix: &str, key: &[u8; 32]) -> Result { + // The application might also use the provided key for something else, so to + // avoid key reuse, we pass the provided key through an HKDF + let mut chacha_key = zeroize::Zeroizing::new([0u8; 32]); + const HKDF_INFO: &[u8] = b"CRYPTOSTORE_CIPHER"; + let hkdf = Hkdf::::new(None, key); + hkdf.expand(HKDF_INFO, &mut *chacha_key) + .expect("We should be able to generate a 32-byte key"); + let db = open_meta_db(prefix).await?; let store_cipher = load_store_cipher(&db).await?; let store_cipher = match store_cipher { Some(cipher) => { debug!("IndexedDbCryptoStore: decrypting store cipher"); - import_store_cipher_with_key(key, &cipher, &db).await? + import_store_cipher_with_key(&chacha_key, key, &cipher, &db).await? } None => { debug!("IndexedDbCryptoStore: encrypting new store cipher"); let cipher = StoreCipher::new().map_err(CryptoStoreError::backend)?; - let export = cipher.export_with_key(key).map_err(CryptoStoreError::backend)?; + let export = + cipher.export_with_key(&chacha_key).map_err(CryptoStoreError::backend)?; save_store_cipher(&db, &export).await?; cipher } @@ -1381,19 +1392,30 @@ async fn save_store_cipher( /// Given a serialised store cipher, try importing with the given key. /// /// This is a helper for [`IndexeddbCryptoStore::open_with_key`]. +/// +/// # Arguments +/// +/// * `chacha_key`: The key to use with [`StoreCipher::import_with_key`]. +/// Derived from `original_key` via an HKDF. +/// * `original_key`: The key provided by the application. Used to provide a +/// migration path from an older key derivation system. +/// * `serialised_cipher`: The serialized `EncryptedStoreCipher`, retrieved from +/// the database. +/// * `db`: Connection to the database. async fn import_store_cipher_with_key( - key: &[u8; 32], + chacha_key: &[u8; 32], + original_key: &[u8], serialised_cipher: &[u8], db: &IdbDatabase, ) -> Result { - let cipher = match StoreCipher::import_with_key(key, serialised_cipher) { + let cipher = match StoreCipher::import_with_key(chacha_key, serialised_cipher) { Ok(cipher) => cipher, Err(matrix_sdk_store_encryption::Error::KdfMismatch) => { // Old versions of the matrix-js-sdk used to base64-encode their encryption // key, and pass it into [`IndexeddbCryptoStore::open_with_passphrase`]. For // backwards compatibility, we fall back to that if we discover we have a cipher // encrypted with a KDF when we expected it to be encrypted directly with a key. - let cipher = StoreCipher::import(&base64_encode(key), serialised_cipher) + let cipher = StoreCipher::import(&base64_encode(original_key), serialised_cipher) .map_err(|_| CryptoStoreError::UnpicklingError)?; // Loading the cipher with the passphrase was successful. Let's update the @@ -1401,7 +1423,7 @@ async fn import_store_cipher_with_key( // to save doing this again. debug!("IndexedDbCryptoStore: Migrating passphrase-encrypted store cipher to key-encryption"); - let export = cipher.export_with_key(key).map_err(CryptoStoreError::backend)?; + let export = cipher.export_with_key(chacha_key).map_err(CryptoStoreError::backend)?; save_store_cipher(db, &export).await?; cipher } From 09eca96bf13b3a507712e8278d6d4e24375dc030 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 21 May 2024 10:51:27 +0100 Subject: [PATCH 6/6] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Damir Jelić Signed-off-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- crates/matrix-sdk-indexeddb/Cargo.toml | 4 ++-- crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk-indexeddb/Cargo.toml b/crates/matrix-sdk-indexeddb/Cargo.toml index 5c45fb27b06..8063b55aca4 100644 --- a/crates/matrix-sdk-indexeddb/Cargo.toml +++ b/crates/matrix-sdk-indexeddb/Cargo.toml @@ -39,8 +39,8 @@ tracing = { workspace = true } wasm-bindgen = "0.2.83" web-sys = { version = "0.3.57", features = ["IdbKeyRange"] } hkdf = "0.12.4" -zeroize.workspace = true -sha2.workspace = true +zeroize = { workspace = true } +sha2 = { workspace = true } [target.'cfg(target_arch = "wasm32")'.dependencies] # for wasm32 we need to activate this diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs index 3169e73904a..d74192dce15 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs @@ -336,7 +336,7 @@ impl IndexeddbCryptoStore { /// # Arguments /// /// * `prefix` - Common prefix for the names of the two IndexedDB stores. - /// * `key` - key with which to encrypt the key which is used to encrypt the + /// * `key` - Key with which to encrypt the key which is used to encrypt the /// store. Must be the same each time the store is opened. pub async fn open_with_key(prefix: &str, key: &[u8; 32]) -> Result { // The application might also use the provided key for something else, so to