From 885b2b22bd85fbabc28cf2f5ceceb1c64821820f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 3 Jul 2024 17:59:37 +0100 Subject: [PATCH 1/6] indexeddb: Revert change from `into_serde` to `serde_wasm_bindgen::from_value` Turns out that `serde_wasm_bindgen` isn't happy with some input. Haven't figured out why, yet. --- .../src/crypto_store/indexeddb_serializer.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/indexeddb_serializer.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/indexeddb_serializer.rs index 6f20680c1db..454ea98106e 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/indexeddb_serializer.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/indexeddb_serializer.rs @@ -19,6 +19,7 @@ use base64::{ engine::{general_purpose, GeneralPurpose}, Engine, }; +use gloo_utils::format::JsValueSerdeExt; use matrix_sdk_crypto::CryptoStoreError; use matrix_sdk_store_encryption::{EncryptedValueBase64, StoreCipher}; use serde::{de::DeserializeOwned, Deserialize, Serialize}; @@ -193,7 +194,7 @@ impl IndexeddbSerializer { // Check for legacy unencrypted format if value.is_object() && self.store_cipher.is_none() { - return Ok(serde_wasm_bindgen::from_value(value)?); + return Ok(value.into_serde()?); } // Can't figure out what this is. From ab3ea8c467d45718d35a678392ea450b91e5dd40 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 3 Jul 2024 19:05:33 +0100 Subject: [PATCH 2/6] indexeddb: Improve handling of legacy values in `deserialize_value` Turns out legacy unencrypted objects can take many forms, and we need to be tolerant of them. Also expose `deserialize_legacy_value` as a separate function, because we're going to need to special-case it. --- .../src/crypto_store/indexeddb_serializer.rs | 65 ++++++++++++++----- 1 file changed, 49 insertions(+), 16 deletions(-) diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/indexeddb_serializer.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/indexeddb_serializer.rs index 454ea98106e..089df8940b0 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/indexeddb_serializer.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/indexeddb_serializer.rs @@ -166,7 +166,7 @@ impl IndexeddbSerializer { // - `MaybeEncrypted::Encrypted` becomes a JS object with properties {`version`, // `nonce`, `ciphertext`}. // - // - `MaybeEncrypted::Unencrypted` becomes a JS string. + // - `MaybeEncrypted::Unencrypted` becomes a JS string containing base64 text. // // Otherwise, it probably uses our old serialization format: // @@ -175,30 +175,63 @@ impl IndexeddbSerializer { // array of JSON bytes. Net result is a JS array. // // - Unencrypted values were serialized to JSON, then deserialized into a - // javascript object. (Hopefully not one that can be turned into a - // `MaybeEncrypted::Encrypted`.) + // javascript object/string/array/bool. + // + // Note that there are several potential ambiguities here: + // + // - A JS string could either be a legacy unencrypted value, or a + // `MaybeEncrypted::Unencrypted`. However, the only thing that actually got + // stored as a string under the legacy system was `backup_key_v1`, and that is + // special-cased not to use this path — so if we can convert it into a + // `MaybeEncrypted::Unencrypted`, then we assume it is one. + // + // - A JS array could be either a legacy encrypted value or a legacy unencrypted + // value. We can tell the difference by whether we have a `cipher`. + // + // - A JS object could be either a legacy unencrypted value or a + // `MaybeEncrypted::Encrypted`. We assume that no legacy JS objects have the + // properties to be successfully decoded into a `MaybeEncrypted::Encrypted`. // First check if it looks like a `MaybeEncrypted`, of either type. if let Ok(maybe_encrypted) = serde_wasm_bindgen::from_value(value.clone()) { return Ok(self.maybe_decrypt_value(maybe_encrypted)?); } - // Check for legacy encrypted format. - if let (true, Some(cipher)) = (value.is_array(), &self.store_cipher) { - // `value` is a JS-side array containing the byte values. Turn it into a - // rust-side Vec. - let value: Vec = serde_wasm_bindgen::from_value(value)?; + // Otherwise, fall back to the legacy deserializer. + self.deserialize_legacy_value(value) + } - return Ok(cipher.decrypt_value(&value).map_err(CryptoStoreError::backend)?); - } + /// Decode a value that was encoded with an old version of + /// `serialize_value`. + /// + /// This should only be used on values from an old database which + pub fn deserialize_legacy_value( + &self, + value: JsValue, + ) -> Result { + match &self.store_cipher { + Some(cipher) => { + if !value.is_array() { + return Err(IndexeddbCryptoStoreError::CryptoStoreError( + CryptoStoreError::UnpicklingError, + )); + } + + // Looks like legacy encrypted format. + // + // `value` is a JS-side array containing the byte values. Turn it into a + // rust-side Vec, then decrypt. + let value: Vec = serde_wasm_bindgen::from_value(value)?; + Ok(cipher.decrypt_value(&value).map_err(CryptoStoreError::backend)?) + } - // Check for legacy unencrypted format - if value.is_object() && self.store_cipher.is_none() { - return Ok(value.into_serde()?); + None => { + // Legacy unencrypted format could be just about anything; just try + // JSON-serializing the value, then deserializing it into the + // desired type. + Ok(value.into_serde()?) + } } - - // Can't figure out what this is. - Err(IndexeddbCryptoStoreError::CryptoStoreError(CryptoStoreError::UnpicklingError)) } /// Decode a value that was previously encoded with From bb0e50ce02efc34704d583ee396eaf501d248221 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 3 Jul 2024 19:41:49 +0100 Subject: [PATCH 3/6] indexeddb: migrate name and format of `backup_version` The name was stupid, and this was the only string that was stored in the legacy format; we can fix both problems with a cheeky migration. --- .../src/crypto_store/migrations/mod.rs | 12 ++++- .../src/crypto_store/migrations/old_keys.rs | 6 +++ .../src/crypto_store/migrations/v10_to_v11.rs | 54 +++++++++++++++++++ .../src/crypto_store/mod.rs | 15 ++++-- 4 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 crates/matrix-sdk-indexeddb/src/crypto_store/migrations/v10_to_v11.rs diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs index 77a64977f51..8ec4f5e3e24 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs @@ -25,6 +25,7 @@ use crate::{ mod old_keys; mod v0_to_v5; +mod v10_to_v11; mod v5_to_v7; mod v7; mod v7_to_v8; @@ -106,8 +107,12 @@ pub async fn open_and_upgrade_db( v8_to_v10::schema_delete(name).await?; } + if old_version < 11 { + v10_to_v11::data_migrate(name, serializer).await?; + } + // Open and return the DB (we know it's at the latest version) - Ok(IdbDatabase::open_u32(name, 10)?.await?) + Ok(IdbDatabase::open_u32(name, 11)?.await?) } async fn db_version(name: &str) -> Result { @@ -183,7 +188,10 @@ mod tests { use super::{v0_to_v5, v7::InboundGroupSessionIndexedDbObject2}; use crate::{ - crypto_store::{keys, migrations::*, InboundGroupSessionIndexedDbObject}, + crypto_store::{ + indexeddb_serializer::MaybeEncrypted, keys, migrations::*, + InboundGroupSessionIndexedDbObject, + }, IndexeddbCryptoStore, }; diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/old_keys.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/old_keys.rs index bd2383b1f39..1a82a14549e 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/old_keys.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/old_keys.rs @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +#[cfg(doc)] +use crate::crypto_store::keys::BACKUP_VERSION_V1; + /// Old format of the `inbound_group_sessions` store which lacked indexes or /// a sensible structure pub const INBOUND_GROUP_SESSIONS_V1: &str = "inbound_group_sessions"; @@ -20,3 +23,6 @@ pub const INBOUND_GROUP_SESSIONS_V1: &str = "inbound_group_sessions"; /// JSON-encoding and arrays of ints instead of base64. /// Also lacked the `backed_up_to` property+index. pub const INBOUND_GROUP_SESSIONS_V2: &str = "inbound_group_sessions2"; + +/// An old name for [`BACKUP_VERSION_V1`]. +pub const BACKUP_KEY_V1: &str = "backup_key_v1"; diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/v10_to_v11.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/v10_to_v11.rs new file mode 100644 index 00000000000..7bb94ffd349 --- /dev/null +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/v10_to_v11.rs @@ -0,0 +1,54 @@ +// Copyright 2024 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Migration code that moves from `backup_keys.backup_key_v1` to +//! `backup_keys.backup_version_v1`, switching to a new serialization format. + +use indexed_db_futures::IdbQuerySource; +use wasm_bindgen::JsValue; +use web_sys::IdbTransactionMode; + +use crate::crypto_store::{ + indexeddb_serializer::IndexeddbSerializer, + keys, + migrations::{old_keys, MigrationDb}, +}; + +/// Migrate data from `backup_keys.backup_key_v1` to +/// `backup_keys.backup_version_v1`. +pub(crate) async fn data_migrate( + name: &str, + serializer: &IndexeddbSerializer, +) -> crate::crypto_store::Result<()> { + let db = MigrationDb::new(name, 11).await?; + let txn = db.transaction_on_one_with_mode(keys::BACKUP_KEYS, IdbTransactionMode::Readwrite)?; + let store = txn.object_store(keys::BACKUP_KEYS)?; + + let bv = store.get(&JsValue::from_str(old_keys::BACKUP_KEY_V1))?.await?; + + let Some(bv) = bv else { + return Ok(()); + }; + + // backup_key_v1 was only ever serialized with the legacy format. Also, it's a + // string, so if we use `deserialize_value` on it, it will be incorrectly + // handled as a new-format object. + let bv: String = serializer.deserialize_legacy_value(bv)?; + + // Re-serialize as new format, then store in the new field. + let serialized = serializer.serialize_value(&bv)?; + store.put_key_val(&JsValue::from_str(keys::BACKUP_VERSION_V1), &serialized)?.await?; + store.delete(&JsValue::from_str(old_keys::BACKUP_KEY_V1))?.await?; + Ok(()) +} diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs index 8c2f371954d..3bcd3fce941 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs @@ -90,7 +90,11 @@ mod keys { // backup v1 pub const BACKUP_KEYS: &str = "backup_keys"; - pub const BACKUP_KEY_V1: &str = "backup_key_v1"; + + /// Indexeddb key for the key backup version that [`RECOVERY_KEY_V1`] + /// corresponds to. + pub const BACKUP_VERSION_V1: &str = "backup_version_v1"; + /// Indexeddb key for the backup decryption key. /// /// Known, for historical reasons, as the recovery key. Not to be confused @@ -479,9 +483,10 @@ impl IndexeddbCryptoStore { } if let Some(a) = &backup_version { - indexeddb_changes - .get(keys::BACKUP_KEYS) - .put(JsValue::from_str(keys::BACKUP_KEY_V1), self.serializer.serialize_value(&a)?); + indexeddb_changes.get(keys::BACKUP_KEYS).put( + JsValue::from_str(keys::BACKUP_VERSION_V1), + self.serializer.serialize_value(&a)?, + ); } if !changes.sessions.is_empty() { @@ -1208,7 +1213,7 @@ impl_crypto_store! { let store = tx.object_store(keys::BACKUP_KEYS)?; let backup_version = store - .get(&JsValue::from_str(keys::BACKUP_KEY_V1))? + .get(&JsValue::from_str(keys::BACKUP_VERSION_V1))? .await? .map(|i| self.serializer.deserialize_value(i)) .transpose()?; From 7b25a1c2f02ea6abd74e6772b0326a98fb4e184f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 3 Jul 2024 19:43:44 +0100 Subject: [PATCH 4/6] indexeddb: update changelog --- crates/matrix-sdk-indexeddb/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk-indexeddb/CHANGELOG.md b/crates/matrix-sdk-indexeddb/CHANGELOG.md index e36116c6035..7e5e0be9498 100644 --- a/crates/matrix-sdk-indexeddb/CHANGELOG.md +++ b/crates/matrix-sdk-indexeddb/CHANGELOG.md @@ -1,6 +1,7 @@ # UNRELEASED -- Improve the efficiency of objects stored in the crypto store. ([#3645](https://github.com/matrix-org/matrix-rust-sdk/pull/3645)) +- Improve the efficiency of objects stored in the crypto store. + ([#3645](https://github.com/matrix-org/matrix-rust-sdk/pull/3645), [#3651](https://github.com/matrix-org/matrix-rust-sdk/pull/3651)) - Add new method `IndexeddbCryptoStore::open_with_key`. ([#3423](https://github.com/matrix-org/matrix-rust-sdk/pull/3423)) From 2a8e8c1fffc8464bc62c2784944f9ecf7b321341 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 3 Jul 2024 22:25:28 +0100 Subject: [PATCH 5/6] indexeddb: improve docs and tests --- .../src/crypto_store/indexeddb_serializer.rs | 81 ++++++++++++------- 1 file changed, 53 insertions(+), 28 deletions(-) diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/indexeddb_serializer.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/indexeddb_serializer.rs index 089df8940b0..2ce0a32c533 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/indexeddb_serializer.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/indexeddb_serializer.rs @@ -204,7 +204,8 @@ impl IndexeddbSerializer { /// Decode a value that was encoded with an old version of /// `serialize_value`. /// - /// This should only be used on values from an old database which + /// This should only be used on values from an old database which are known + /// to be serialized with the old format. pub fn deserialize_legacy_value( &self, value: JsValue, @@ -229,6 +230,15 @@ impl IndexeddbSerializer { // Legacy unencrypted format could be just about anything; just try // JSON-serializing the value, then deserializing it into the // desired type. + // + // Note that the stored data was actually encoded by JSON-serializing it, and + // then deserializing the JSON into Javascript objects — so, for + // example, `HashMap`s are converted into Javascript Objects + // (whose keys are always strings) rather than Maps (whose keys + // can be other things). `serde_wasm_bindgen::from_value` will complain about + // such things. The correct thing to do is to go *back* to JSON + // and then deserialize into Rust again, which is what `JsValue::into_serde` + // does. Ok(value.into_serde()?) } } @@ -269,8 +279,9 @@ impl IndexeddbSerializer { #[cfg(all(test, target_arch = "wasm32"))] mod tests { - use std::sync::Arc; + use std::{collections::BTreeMap, sync::Arc}; + use gloo_utils::format::JsValueSerdeExt; use matrix_sdk_store_encryption::StoreCipher; use matrix_sdk_test::async_test; use serde::{Deserialize, Serialize}; @@ -312,33 +323,26 @@ mod tests { /// an old implementation of `serialize_value`, when a cipher is in use. #[async_test] async fn test_deserialize_old_serialized_value_with_cipher() { - // An example of an object which was serialized using the old-format - // `serialize_value`. - let serialized = [ - 123, 34, 118, 101, 114, 115, 105, 111, 110, 34, 58, 49, 44, 34, 99, 105, 112, 104, 101, - 114, 116, 101, 120, 116, 34, 58, 91, 52, 53, 44, 49, 49, 54, 44, 49, 54, 57, 44, 49, - 57, 49, 44, 49, 49, 54, 44, 57, 53, 44, 49, 50, 54, 44, 52, 51, 44, 54, 49, 44, 50, 52, - 55, 44, 50, 50, 52, 44, 57, 54, 44, 49, 49, 51, 44, 49, 50, 50, 44, 49, 57, 50, 44, 52, - 55, 44, 50, 51, 52, 44, 50, 52, 51, 44, 55, 44, 49, 55, 55, 44, 49, 57, 44, 50, 48, 57, - 44, 50, 52, 50, 44, 49, 48, 56, 44, 49, 52, 54, 44, 49, 52, 44, 49, 52, 52, 44, 50, 54, - 44, 50, 52, 56, 44, 49, 54, 55, 44, 49, 53, 44, 50, 51, 48, 44, 49, 52, 48, 44, 56, 51, - 44, 50, 48, 52, 44, 51, 55, 44, 49, 56, 51, 44, 55, 50, 93, 44, 34, 110, 111, 110, 99, - 101, 34, 58, 91, 49, 52, 48, 44, 49, 50, 56, 44, 50, 50, 52, 44, 50, 49, 52, 44, 56, - 54, 44, 49, 53, 51, 44, 50, 50, 52, 44, 49, 55, 56, 44, 49, 54, 54, 44, 49, 48, 49, 44, - 48, 44, 54, 54, 44, 49, 57, 55, 44, 53, 55, 44, 50, 57, 44, 50, 52, 51, 44, 57, 53, 44, - 50, 49, 53, 44, 56, 54, 44, 50, 48, 54, 44, 54, 53, 44, 49, 53, 54, 44, 49, 49, 57, 44, - 53, 57, 93, 125, - ] - .into_iter() - .map(JsValue::from) - .collect::() - .into(); - - let serializer = IndexeddbSerializer::new(Some(Arc::new(test_cipher()))); + let cipher = test_cipher(); + let obj = make_test_object(); + + // Follow the old format for encoding: + // 1. Encode as JSON, in a Vec of bytes + // 2. Encrypt + // 3. JSON-encode to another Vec + // 4. Turn the Vec into a Javascript array of numbers. + let data = serde_json::to_vec(&obj).unwrap(); + let data = cipher.encrypt_value_data(data).unwrap(); + let data = serde_json::to_vec(&data).unwrap(); + let serialized = JsValue::from_serde(&data).unwrap(); + + // Now, try deserializing with `deserialize_value`, and check we get the right + // thing. + let serializer = IndexeddbSerializer::new(Some(Arc::new(cipher))); let deserialized: TestStruct = serializer.deserialize_value(serialized).expect("could not deserialize"); - assert_eq!(make_test_object(), deserialized); + assert_eq!(obj, deserialized); } /// Test that `deserialize_value` can decode a value that was encoded with @@ -347,7 +351,8 @@ mod tests { async fn test_deserialize_old_serialized_value_no_cipher() { // An example of an object which was serialized using the old-format // `serialize_value`. - let serialized = js_sys::JSON::parse(&(json!({"id":0,"name":"test"}).to_string())).unwrap(); + let json = json!({ "id":0, "name": "test", "map": { "0": "test" }}); + let serialized = js_sys::JSON::parse(&json.to_string()).unwrap(); let serializer = IndexeddbSerializer::new(None); let deserialized: TestStruct = @@ -356,6 +361,21 @@ mod tests { assert_eq!(make_test_object(), deserialized); } + /// Test that `deserialize_value` can decode an array value that was encoded + /// with an old implementation of `serialize_value`, when no cipher is + /// in use. + #[async_test] + async fn test_deserialize_old_serialized_array_no_cipher() { + let json = json!([1, 2, 3, 4]); + let serialized = js_sys::JSON::parse(&json.to_string()).unwrap(); + + let serializer = IndexeddbSerializer::new(None); + let deserialized: Vec = + serializer.deserialize_value(serialized).expect("could not deserialize"); + + assert_eq!(vec![1, 2, 3, 4], deserialized); + } + /// Test that `deserialize_value` can decode a value encoded with /// `maybe_encrypt_value`, when a cipher is in use. #[async_test] @@ -418,10 +438,15 @@ mod tests { struct TestStruct { id: u32, name: String, + + // A map, whose keys are not strings. This is an edge-case we previously got wrong. Maps + // are represented differently in JSON from Javascript objects, and that particularly + // matters when their keys are not strings. + map: BTreeMap, } fn make_test_object() -> TestStruct { - TestStruct { id: 0, name: "test".to_owned() } + TestStruct { id: 0, name: "test".to_owned(), map: BTreeMap::from([(0, "test".to_owned())]) } } /// Build a [`StoreCipher`] using a hardcoded key. From 98e9abd6c9a0999769bffa7932ee93abc1825d13 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 3 Jul 2024 22:54:20 +0100 Subject: [PATCH 6/6] indexeddb: tests for new schema migration --- .../src/crypto_store/migrations/mod.rs | 94 ++++++++++++++++--- 1 file changed, 80 insertions(+), 14 deletions(-) diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs index 8ec4f5e3e24..db4aa99e3bb 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs @@ -183,6 +183,7 @@ mod tests { use matrix_sdk_store_encryption::StoreCipher; use matrix_sdk_test::async_test; use ruma::{room_id, OwnedRoomId, RoomId}; + use serde::Serialize; use tracing_subscriber::util::SubscriberInitExt; use web_sys::console; @@ -483,7 +484,7 @@ mod tests { fetched_backed_up_session: InboundGroupSession, ) { let db = IdbDatabase::open(&db_name).unwrap().await.unwrap(); - assert_eq!(db.version(), 10.0); + assert!(db.version() >= 10.0); let transaction = db.transaction_on_one("inbound_group_sessions3").unwrap(); let raw_store = transaction.object_store("inbound_group_sessions3").unwrap(); let key = store.serializer.encode_key( @@ -568,19 +569,9 @@ mod tests { serializer.encode_key(old_keys::INBOUND_GROUP_SESSIONS_V1, (room_id, session_id)); let pickle = session.pickle().await; - let serialized_session = if let Some(cipher) = &store_cipher { - // Old-style serialization/encryption. First JSON-serialize into a byte array... - let data = serde_json::to_vec(&pickle).unwrap(); - // ... then encrypt... - let encrypted = cipher.encrypt_value_data(data).unwrap(); - // ... then JSON-serialize into another byte array ... - let value = serde_json::to_vec(&encrypted).unwrap(); - // and finally, turn it into a javascript array. - JsValue::from_serde(&value).unwrap() - } else { - JsValue::from_serde(&pickle).unwrap() - }; - + // Serialize the session with the old style of serialization, since that's what + // we used at the time. + let serialized_session = serialize_value_as_legacy(&store_cipher, &pickle); sessions.put_key_val(&key, &serialized_session).unwrap(); } txn.await.into_result().unwrap(); @@ -590,8 +581,83 @@ mod tests { db.close(); } + /// Test migrating `backup_keys` data from store v10 to latest, + /// on a store with encryption disabled. + #[async_test] + async fn test_v10_v11_migration_unencrypted() { + test_v10_v11_migration_with_cipher("test_v10_migration_unencrypted", None).await + } + + /// Test migrating `backup_keys` data from store v10 to latest, + /// on a store with encryption enabled. + #[async_test] + async fn test_v10_v11_migration_encrypted() { + let cipher = StoreCipher::new().unwrap(); + test_v10_v11_migration_with_cipher("test_v10_migration_encrypted", Some(Arc::new(cipher))) + .await; + } + + /// Helper function for `test_v10_v11_migration_{un,}encrypted`: test + /// migrating `backup_keys` data from store v10 to store v11. + async fn test_v10_v11_migration_with_cipher( + db_prefix: &str, + store_cipher: Option>, + ) { + let _ = make_tracing_subscriber(None).try_init(); + let db_name = format!("{db_prefix:0}::matrix-sdk-crypto"); + + // delete the db in case it was used in a previous run + let _ = IdbDatabase::delete_by_name(&db_name); + + // Given a DB with data in it as it was at v5 + let db = create_v5_db(&db_name).await.unwrap(); + + let txn = db + .transaction_on_one_with_mode(keys::BACKUP_KEYS, IdbTransactionMode::Readwrite) + .unwrap(); + let store = txn.object_store(keys::BACKUP_KEYS).unwrap(); + store + .put_key_val( + &JsValue::from_str(old_keys::BACKUP_KEY_V1), + &serialize_value_as_legacy(&store_cipher, &"1".to_owned()), + ) + .unwrap(); + db.close(); + + // When I open a store based on that DB, triggering an upgrade + let store = + IndexeddbCryptoStore::open_with_store_cipher(&db_prefix, store_cipher).await.unwrap(); + + // Then I can read the backup settings + let backup_data = store.load_backup_keys().await.unwrap(); + assert_eq!(backup_data.backup_version, Some("1".to_owned())); + } + async fn create_v5_db(name: &str) -> std::result::Result { v0_to_v5::schema_add(name).await?; IdbDatabase::open_u32(name, 5)?.await } + + /// Emulate the old behaviour of [`IndexeddbSerializer::serialize_value`]. + /// + /// We used to use an inefficient format for serializing objects in the + /// indexeddb store. This replicates that old behaviour, for testing + /// purposes. + fn serialize_value_as_legacy( + store_cipher: &Option>, + value: &T, + ) -> JsValue { + if let Some(cipher) = &store_cipher { + // Old-style serialization/encryption. First JSON-serialize into a byte array... + let data = serde_json::to_vec(&value).unwrap(); + // ... then encrypt... + let encrypted = cipher.encrypt_value_data(data).unwrap(); + // ... then JSON-serialize into another byte array ... + let value = serde_json::to_vec(&encrypted).unwrap(); + // and finally, turn it into a javascript array. + JsValue::from_serde(&value).unwrap() + } else { + JsValue::from_serde(&value).unwrap() + } + } }