From 833ff3233a089c0b28a6dbfa3033828b666946a8 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 2 Aug 2024 13:34:54 +0100 Subject: [PATCH] crypto: Add curve_key and sender_data_type columns to inboundgroupsessions table/store --- .../src/crypto_store/migrations/mod.rs | 66 ++++++++++--- .../src/crypto_store/migrations/v11_to_v12.rs | 37 ++++++++ .../src/crypto_store/migrations/v8_to_v10.rs | 2 + .../src/crypto_store/mod.rs | 95 ++++++++++++++++++- ...oup_session_curve_key_sender_data_type.sql | 8 ++ crates/matrix-sdk-sqlite/src/crypto_store.rs | 23 ++++- 6 files changed, 210 insertions(+), 21 deletions(-) create mode 100644 crates/matrix-sdk-indexeddb/src/crypto_store/migrations/v11_to_v12.rs create mode 100644 crates/matrix-sdk-sqlite/migrations/crypto_store/009_inbound_group_session_curve_key_sender_data_type.sql 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 79be30b5152..e8ef42c26f8 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs @@ -26,6 +26,7 @@ use crate::{ mod old_keys; mod v0_to_v5; mod v10_to_v11; +mod v11_to_v12; mod v5_to_v7; mod v7; mod v7_to_v8; @@ -156,6 +157,10 @@ pub async fn open_and_upgrade_db( v10_to_v11::schema_bump(name).await?; } + if old_version < 12 { + v11_to_v12::schema_add(name).await?; + } + // If you add more migrations here, you'll need to update // `tests::EXPECTED_SCHEMA_VERSION`. @@ -249,7 +254,7 @@ mod tests { wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser); /// The schema version we expect after we open the store. - const EXPECTED_SCHEMA_VERSION: u32 = 11; + const EXPECTED_SCHEMA_VERSION: u32 = 12; /// Adjust this to test do a more comprehensive perf test const NUM_RECORDS_FOR_PERF: usize = 2_000; @@ -401,6 +406,8 @@ mod tests { pickled_session: serializer.maybe_encrypt_value(pickled_session).unwrap(), needs_backup: false, backed_up_to: -1, + curve_key: None, + sender_data_type: None, }; let session_js: JsValue = serde_wasm_bindgen::to_value(&session_dbo).unwrap(); @@ -466,22 +473,25 @@ mod tests { /// Test migrating `inbound_group_sessions` data from store v5 to latest, /// on a store with encryption disabled. #[async_test] - async fn test_v8_v10_migration_unencrypted() { - test_v8_v10_migration_with_cipher("test_v8_migration_unencrypted", None).await + async fn test_v8_v10_v12_migration_unencrypted() { + test_v8_v10_v12_migration_with_cipher("test_v8_migration_unencrypted", None).await } /// Test migrating `inbound_group_sessions` data from store v5 to store v8, /// on a store with encryption enabled. #[async_test] - async fn test_v8_v10_migration_encrypted() { + async fn test_v8_v10_v12_migration_encrypted() { let cipher = StoreCipher::new().unwrap(); - test_v8_v10_migration_with_cipher("test_v8_migration_encrypted", Some(Arc::new(cipher))) - .await; + test_v8_v10_v12_migration_with_cipher( + "test_v8_migration_encrypted", + Some(Arc::new(cipher)), + ) + .await; } - /// Helper function for `test_v8_v10_migration_{un,}encrypted`: test - /// migrating `inbound_group_sessions` data from store v5 to store v10. - async fn test_v8_v10_migration_with_cipher( + /// Helper function for `test_v8_v10_v12_migration_{un,}encrypted`: test + /// migrating `inbound_group_sessions` data from store v5 to store v12. + async fn test_v8_v10_v12_migration_with_cipher( db_prefix: &str, store_cipher: Option>, ) { @@ -525,13 +535,17 @@ mod tests { assert!(!fetched_not_backed_up_session.backed_up()); // For v10: they have the backed_up_to property and it is indexed - assert_matches_v10_schema(db_name, store, fetched_backed_up_session).await; + assert_matches_v10_schema(&db_name, &store, &fetched_backed_up_session).await; + + // For v12: they have the curve_key and sender_data_type properties and they are + // indexed + assert_matches_v12_schema(&db_name, &store, &fetched_backed_up_session).await; } async fn assert_matches_v10_schema( - db_name: String, - store: IndexeddbCryptoStore, - fetched_backed_up_session: InboundGroupSession, + db_name: &str, + store: &IndexeddbCryptoStore, + fetched_backed_up_session: &InboundGroupSession, ) { let db = IdbDatabase::open(&db_name).unwrap().await.unwrap(); assert!(db.version() >= 10.0); @@ -551,6 +565,32 @@ mod tests { db.close(); } + async fn assert_matches_v12_schema( + db_name: &str, + store: &IndexeddbCryptoStore, + session: &InboundGroupSession, + ) { + let db = IdbDatabase::open(&db_name).unwrap().await.unwrap(); + 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(keys::INBOUND_GROUP_SESSIONS_V3, (session.room_id(), session.session_id())); + let idb_object: InboundGroupSessionIndexedDbObject = + serde_wasm_bindgen::from_value(raw_store.get(&key).unwrap().await.unwrap().unwrap()) + .unwrap(); + + assert_eq!(idb_object.curve_key, None); + assert_eq!(idb_object.sender_data_type, None); + assert!(raw_store + .index_names() + .find(|idx| idx == "inbound_group_session_curve_key_sender_data_type_idx") + .is_some()); + + db.close(); + } + fn create_sessions(room_id: &RoomId) -> (InboundGroupSession, InboundGroupSession) { let curve_key = Curve25519PublicKey::from(&Curve25519SecretKey::new()); let ed_key = Ed25519SecretKey::new().public_key(); diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/v11_to_v12.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/v11_to_v12.rs new file mode 100644 index 00000000000..f3f4e0a3ee5 --- /dev/null +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/v11_to_v12.rs @@ -0,0 +1,37 @@ +// 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 inbound_group_sessions2 to +//! inbound_group_sessions3, shrinking the values stored in each record. + +use indexed_db_futures::IdbKeyPath; +use web_sys::DomException; + +use crate::crypto_store::{keys, migrations::do_schema_upgrade, Result}; + +/// Perform the schema upgrade v11 to v12, adding an index on +/// `(curve_key, sender_data_type)` to `inbound_group_sessions3`. +pub(crate) async fn schema_add(name: &str) -> Result<(), DomException> { + do_schema_upgrade(name, 12, |_, transaction, _| { + let object_store = transaction.object_store(keys::INBOUND_GROUP_SESSIONS_V3)?; + + object_store.create_index( + keys::INBOUND_GROUP_SESSIONS_CURVE_KEY_INDEX, + &IdbKeyPath::str_sequence(&["curve_key", "sender_data_type"]), + )?; + + Ok(()) + }) + .await +} diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/v8_to_v10.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/v8_to_v10.rs index c6e805c9b78..04ffb0b0762 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/v8_to_v10.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/migrations/v8_to_v10.rs @@ -103,6 +103,8 @@ pub(crate) async fn data_migrate(name: &str, serializer: &IndexeddbSerializer) - let new_value = InboundGroupSessionIndexedDbObject::new( serializer.maybe_encrypt_value(session.pickle().await)?, !session.backed_up(), + None, + None, ); // Write it to the new store diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs index 11f1ec6fbac..155289659bb 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs @@ -24,7 +24,7 @@ use indexed_db_futures::prelude::*; use matrix_sdk_crypto::{ olm::{ InboundGroupSession, OlmMessageHash, OutboundGroupSession, PickledInboundGroupSession, - PrivateCrossSigningIdentity, Session, StaticAccountData, + PrivateCrossSigningIdentity, SenderDataType, Session, StaticAccountData, }, store::{ BackupKeys, Changes, CryptoStore, CryptoStoreError, PendingChanges, RoomKeyCounts, @@ -62,6 +62,8 @@ mod keys { pub const INBOUND_GROUP_SESSIONS_V3: &str = "inbound_group_sessions3"; pub const INBOUND_GROUP_SESSIONS_BACKUP_INDEX: &str = "backup"; pub const INBOUND_GROUP_SESSIONS_BACKED_UP_TO_INDEX: &str = "backed_up_to"; + pub const INBOUND_GROUP_SESSIONS_CURVE_KEY_INDEX: &str = + "inbound_group_session_curve_key_sender_data_type_idx"; pub const OUTBOUND_GROUP_SESSIONS: &str = "outbound_group_sessions"; @@ -400,6 +402,8 @@ impl IndexeddbCryptoStore { let obj = InboundGroupSessionIndexedDbObject::new( self.serializer.maybe_encrypt_value(session.pickle().await)?, !session.backed_up(), + Some(session.sender_key().to_base64()), + Some(session.sender_data_type()), ); Ok(serde_wasm_bindgen::to_value(&obj)?) } @@ -1584,7 +1588,7 @@ struct GossipRequestIndexedDbObject { unsent: bool, } -/// The objects we store in the inbound_group_sessions2 indexeddb object store +/// The objects we store in the inbound_group_sessions3 indexeddb object store #[derive(serde::Serialize, serde::Deserialize)] struct InboundGroupSessionIndexedDbObject { /// Possibly encrypted @@ -1617,16 +1621,43 @@ struct InboundGroupSessionIndexedDbObject { /// "refer to the `needs_backup` property". See: /// https://github.com/element-hq/element-web/issues/26892#issuecomment-1906336076 backed_up_to: i32, + + /// The curve key of the device that sent us this room key, base64-encoded. + /// + /// Added in database schema v12, and lazily populated, so it is only + /// present for sessions received or modified since DB schema v12. + #[serde(default, skip_serializing_if = "Option::is_none")] + curve_key: Option, + + /// The type of the [`SenderData`] within this session, converted to a u8 + /// from [`SenderDataType`]. + /// + /// Added in database schema v12, and lazily populated, so it is only + /// present for sessions received or modified since DB schema v12. + #[serde(default, skip_serializing_if = "Option::is_none")] + sender_data_type: Option, } impl InboundGroupSessionIndexedDbObject { - pub fn new(pickled_session: MaybeEncrypted, needs_backup: bool) -> Self { - Self { pickled_session, needs_backup, backed_up_to: -1 } + pub fn new( + pickled_session: MaybeEncrypted, + needs_backup: bool, + curve_key: Option, + sender_data_type: Option, + ) -> Self { + Self { + pickled_session, + needs_backup, + backed_up_to: -1, + curve_key, + sender_data_type: sender_data_type.map(|t| t as u8), + } } } #[cfg(test)] mod unit_tests { + use matrix_sdk_crypto::olm::SenderDataType; use matrix_sdk_store_encryption::EncryptedValueBase64; use super::InboundGroupSessionIndexedDbObject; @@ -1637,6 +1668,8 @@ mod unit_tests { let session_needs_backup = InboundGroupSessionIndexedDbObject::new( MaybeEncrypted::Encrypted(EncryptedValueBase64::new(1, "", "")), true, + None, + None, ); // Testing the exact JSON here is theoretically flaky in the face of @@ -1652,6 +1685,8 @@ mod unit_tests { let session_backed_up = InboundGroupSessionIndexedDbObject::new( MaybeEncrypted::Encrypted(EncryptedValueBase64::new(1, "", "")), false, + None, + None, ); assert!( @@ -1659,10 +1694,24 @@ mod unit_tests { "The needs_backup field should be missing!" ); } + + #[test] + fn curve_key_and_sender_data_type_are_serialized_in_json() { + let db_object = InboundGroupSessionIndexedDbObject::new( + MaybeEncrypted::Encrypted(EncryptedValueBase64::new(1, "", "")), + true, + Some("KEY".to_owned()), + Some(SenderDataType::SenderKnown), + ); + + assert!(serde_json::to_string(&db_object).unwrap().contains(r#""curve_key":"KEY""#),); + assert!(serde_json::to_string(&db_object).unwrap().contains(r#""sender_data_type":3"#),); + } } #[cfg(all(test, target_arch = "wasm32"))] mod wasm_unit_tests { + use matrix_sdk_crypto::olm::SenderDataType; use matrix_sdk_store_encryption::EncryptedValueBase64; use matrix_sdk_test::async_test; use wasm_bindgen::JsValue; @@ -1681,6 +1730,8 @@ mod wasm_unit_tests { let session_needs_backup = InboundGroupSessionIndexedDbObject::new( MaybeEncrypted::Encrypted(EncryptedValueBase64::new(3, "", "")), true, + None, + None, ); let js_value = serde_wasm_bindgen::to_value(&session_needs_backup).unwrap(); @@ -1694,12 +1745,48 @@ mod wasm_unit_tests { let session_backed_up = InboundGroupSessionIndexedDbObject::new( MaybeEncrypted::Encrypted(EncryptedValueBase64::new(3, "", "")), false, + None, + None, ); let js_value = serde_wasm_bindgen::to_value(&session_backed_up).unwrap(); assert!(!js_sys::Reflect::has(&js_value, &"needs_backup".into()).unwrap()); } + + #[async_test] + fn curve_key_and_device_type_are_serialized_in_js() { + let db_object = InboundGroupSessionIndexedDbObject::new( + MaybeEncrypted::Encrypted(EncryptedValueBase64::new(1, "", "")), + true, + Some("KEY".to_owned()), + Some(SenderDataType::DeviceInfo), + ); + + let js_value = serde_wasm_bindgen::to_value(&db_object).unwrap(); + + assert!(js_value.is_object()); + assert_field_equals(&js_value, "sender_data_type", 2); + assert_eq!( + js_sys::Reflect::get(&js_value, &"curve_key".into()).unwrap(), + JsValue::from_str("KEY"), + ); + } + + #[async_test] + fn none_values_are_serialized_with_missing_fields_in_js() { + let db_object = InboundGroupSessionIndexedDbObject::new( + MaybeEncrypted::Encrypted(EncryptedValueBase64::new(1, "", "")), + false, + None, + None, + ); + + let js_value = serde_wasm_bindgen::to_value(&db_object).unwrap(); + + assert!(!js_sys::Reflect::has(&js_value, &"curve_key".into()).unwrap()); + assert!(!js_sys::Reflect::has(&js_value, &"sender_data_type".into()).unwrap()); + } } #[cfg(all(test, target_arch = "wasm32"))] diff --git a/crates/matrix-sdk-sqlite/migrations/crypto_store/009_inbound_group_session_curve_key_sender_data_type.sql b/crates/matrix-sdk-sqlite/migrations/crypto_store/009_inbound_group_session_curve_key_sender_data_type.sql new file mode 100644 index 00000000000..a54042b307d --- /dev/null +++ b/crates/matrix-sdk-sqlite/migrations/crypto_store/009_inbound_group_session_curve_key_sender_data_type.sql @@ -0,0 +1,8 @@ +ALTER TABLE "inbound_group_session" + ADD COLUMN "curve_key" BLOB; + +ALTER TABLE "inbound_group_session" + ADD COLUMN "sender_data_type" INTEGER; + +CREATE INDEX "inbound_group_session_curve_key_sender_data_type_idx" + ON "inbound_group_session" ("curve_key", "sender_data_type"); diff --git a/crates/matrix-sdk-sqlite/src/crypto_store.rs b/crates/matrix-sdk-sqlite/src/crypto_store.rs index 3e687ae4571..aeb0ee07422 100644 --- a/crates/matrix-sdk-sqlite/src/crypto_store.rs +++ b/crates/matrix-sdk-sqlite/src/crypto_store.rs @@ -189,7 +189,7 @@ impl SqliteCryptoStore { } } -const DATABASE_VERSION: u8 = 8; +const DATABASE_VERSION: u8 = 9; /// Run migrations for the given version of the database. async fn run_migrations(conn: &SqliteConn, version: u8) -> Result<()> { @@ -264,6 +264,15 @@ async fn run_migrations(conn: &SqliteConn, version: u8) -> Result<()> { .await?; } + if version < 9 { + conn.with_transaction(|txn| { + txn.execute_batch(include_str!( + "../migrations/crypto_store/009_inbound_group_session_curve_key_sender_data_type.sql" + )) + }) + .await?; + } + conn.set_kv("version", vec![DATABASE_VERSION]).await?; Ok(()) @@ -283,6 +292,8 @@ trait SqliteConnectionExt { session_id: &[u8], data: &[u8], backed_up: bool, + curve_key: Option<&str>, + sender_data_type: Option, ) -> rusqlite::Result<()>; fn set_outbound_group_session(&self, room_id: &[u8], data: &[u8]) -> rusqlite::Result<()>; @@ -335,12 +346,14 @@ impl SqliteConnectionExt for rusqlite::Connection { session_id: &[u8], data: &[u8], backed_up: bool, + curve_key: Option<&str>, + sender_data_type: Option, ) -> rusqlite::Result<()> { self.execute( - "INSERT INTO inbound_group_session (session_id, room_id, data, backed_up) \ - VALUES (?1, ?2, ?3, ?4) + "INSERT INTO inbound_group_session (session_id, room_id, data, backed_up, curve_key, sender_data_type) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6) ON CONFLICT (session_id) DO UPDATE SET data = ?3, backed_up = ?4", - (session_id, room_id, data, backed_up), + (session_id, room_id, data, backed_up, curve_key, sender_data_type), )?; Ok(()) } @@ -821,6 +834,8 @@ impl CryptoStore for SqliteCryptoStore { session_id, &serialized_session, pickle.backed_up, + Some(&pickle.sender_key.to_base64()), + Some(pickle.sender_data.to_type() as u8), )?; }