Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Indexeddb: Groundwork for fixing inbound_group_session lookups #2884

Merged
merged 8 commits into from
Nov 27, 2023
164 changes: 164 additions & 0 deletions crates/matrix-sdk-indexeddb/src/crypto_store/indexeddb_serializer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
// Copyright 2023 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.

use std::sync::Arc;

use gloo_utils::format::JsValueSerdeExt;
use matrix_sdk_crypto::CryptoStoreError;
use matrix_sdk_store_encryption::StoreCipher;
use serde::{de::DeserializeOwned, Serialize};
use wasm_bindgen::JsValue;
use web_sys::IdbKeyRange;

use crate::{safe_encode::SafeEncode, IndexeddbCryptoStoreError};

type Result<A, E = IndexeddbCryptoStoreError> = std::result::Result<A, E>;

/// Handles the functionality of serializing and encrypting data for the
/// indexeddb store.
pub struct IndexeddbSerializer {
store_cipher: Option<Arc<StoreCipher>>,
}

impl IndexeddbSerializer {
pub fn new(store_cipher: Option<Arc<StoreCipher>>) -> Self {
Self { store_cipher }
}

/// Hash the given key securely for the given tablename, using the store
/// cipher.
///
/// First calls [`SafeEncode::as_encoded_string`]
/// on the `key` to encode it into a formatted string.
///
/// Then, if a cipher is configured, hashes the formatted key and returns
/// the hash encoded as unpadded base64.
///
/// If no cipher is configured, just returns the formatted key.
///
/// This is faster than [`serialize_value`] and reliably gives the same
/// output for the same input, making it suitable for index keys.
pub fn encode_key<T>(&self, table_name: &str, key: T) -> JsValue
where
T: SafeEncode,
{
self.encode_key_as_string(table_name, key).into()
}

/// Hash the given key securely for the given tablename, using the store
/// cipher.
///
/// The same as [`encode_key`], but stops short of converting the resulting
/// base64 string into a JsValue
pub fn encode_key_as_string<T>(&self, table_name: &str, key: T) -> String
where
T: SafeEncode,
{
match &self.store_cipher {
Some(cipher) => key.as_secure_string(table_name, cipher),
None => key.as_encoded_string(),
}
}

pub fn encode_to_range<T>(
&self,
table_name: &str,
key: T,
) -> Result<IdbKeyRange, IndexeddbCryptoStoreError>
where
T: SafeEncode,
{
match &self.store_cipher {
Some(cipher) => key.encode_to_range_secure(table_name, cipher),
None => key.encode_to_range(),
}
.map_err(|e| IndexeddbCryptoStoreError::DomException {
code: 0,
name: "IdbKeyRangeMakeError".to_owned(),
message: e,
})
}

/// Encode the value for storage as a value in indexeddb.
///
/// First, serialise the given value as JSON.
///
/// Then, if a store cipher is enabled, encrypt the JSON string using the
/// configured store cipher, giving a byte array. Then, wrap the byte
/// array as a `JsValue`.
///
/// If no cipher is enabled, deserialises the JSON string again giving a JS
/// object.
pub fn serialize_value(&self, value: &impl Serialize) -> Result<JsValue, CryptoStoreError> {
if let Some(cipher) = &self.store_cipher {
let value = cipher.encrypt_value(value).map_err(CryptoStoreError::backend)?;

// Turn the Vec<u8> into a Javascript-side `Array<number>`.
// XXX Isn't there a way to do this that *doesn't* involve going via a JSON
// string?
Comment on lines +108 to +109
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, there's at least comment changes and code motion in this commit, and since it's a much bigger chunk than the previous, I'll request to split it into a code motion one first, and then the changes to the code as a second commit please.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, sorry about this. The comments aren't particularly significant imho which is why I rolled them all together, but that was bad of me.

I've now split this into three separate commits.

Ok(JsValue::from_serde(&value)?)
} else {
// Turn the rust-side struct into a JS-side `Object`.
Ok(JsValue::from_serde(&value)?)
}
}

/// Encode the value for storage as a value in indexeddb.
///
/// This is the same algorithm as [`serialize_value`], but stops short of
/// encoding the resultant byte vector in a JsValue.
///
/// Returns a byte vector which is either the JSON serialisation of the
/// value, or an encrypted version thereof.
pub fn serialize_value_as_bytes(
&self,
value: &impl Serialize,
) -> Result<Vec<u8>, CryptoStoreError> {
match &self.store_cipher {
Some(cipher) => cipher.encrypt_value(value).map_err(CryptoStoreError::backend),
None => serde_json::to_vec(value).map_err(CryptoStoreError::backend),
}
}

/// Decode a value that was previously encoded with [`serialize_value`]
pub fn deserialize_value<T: DeserializeOwned>(
&self,
value: JsValue,
) -> Result<T, CryptoStoreError> {
if let Some(cipher) = &self.store_cipher {
// `value` is a JS-side array containing the byte values. Turn it into a
// rust-side Vec<u8>.
// XXX: Isn't there a way to do this that *doesn't* involve going via a JSON
// string?
let value: Vec<u8> = value.into_serde()?;

cipher.decrypt_value(&value).map_err(CryptoStoreError::backend)
} else {
Ok(value.into_serde()?)
}
}

/// Decode a value that was previously encoded with
/// [`serialize_value_as_bytes`]
pub fn deserialize_value_from_bytes<T: DeserializeOwned>(
&self,
value: &[u8],
) -> Result<T, CryptoStoreError> {
if let Some(cipher) = &self.store_cipher {
cipher.decrypt_value(value).map_err(CryptoStoreError::backend)
} else {
serde_json::from_slice(value).map_err(CryptoStoreError::backend)
}
}
}
138 changes: 138 additions & 0 deletions crates/matrix-sdk-indexeddb/src/crypto_store/migrations.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
// Copyright 2023 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.

use indexed_db_futures::{prelude::*, web_sys::DomException};
use tracing::info;
use wasm_bindgen::JsValue;

use crate::crypto_store::{keys, Result};

/// Open the indexeddb with the given name, upgrading it to the latest version
/// of the schema if necessary.
pub async fn open_and_upgrade_db(name: &str) -> Result<IdbDatabase, DomException> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(looking at the commit Move migration functions to a separate module and split it up into smaller functions)

In the future, can you avoid changes that both 1. move the code, 2. change the code, please? They make the review really hard. In this case the amount of code is small enough that it's not a big deal, but it takes quadratically more time than if the commit had been split into two smaller logical ones.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted, and agreed. Sorry about this!

let mut db_req: OpenDbRequest = IdbDatabase::open_u32(name, 5)?;

db_req.set_on_upgrade_needed(Some(|evt: &IdbVersionChangeEvent| -> Result<(), JsValue> {
// Even if the web-sys bindings expose the version as a f64, the IndexedDB API
// works with an unsigned integer.
// See <https://github.com/rustwasm/wasm-bindgen/issues/1149>
let old_version = evt.old_version() as u32;
let new_version = evt.new_version() as u32;

info!(old_version, new_version, "Upgrading IndexeddbCryptoStore");

if old_version < 1 {
create_stores_for_v1(evt.db())?;
}

if old_version < 2 {
create_stores_for_v2(evt.db())?;
}

if old_version < 3 {
create_stores_for_v3(evt.db())?;
}

if old_version < 4 {
create_stores_for_v4(evt.db())?;
}

if old_version < 5 {
create_stores_for_v5(evt.db())?;
}

info!(old_version, new_version, "IndexeddbCryptoStore upgrade complete");
Ok(())
}));

db_req.await
}

fn create_stores_for_v1(db: &IdbDatabase) -> Result<(), DomException> {
db.create_object_store(keys::CORE)?;
db.create_object_store(keys::SESSION)?;

db.create_object_store(keys::INBOUND_GROUP_SESSIONS)?;
db.create_object_store(keys::OUTBOUND_GROUP_SESSIONS)?;
db.create_object_store(keys::TRACKED_USERS)?;
db.create_object_store(keys::OLM_HASHES)?;
db.create_object_store(keys::DEVICES)?;

db.create_object_store(keys::IDENTITIES)?;
db.create_object_store(keys::BACKUP_KEYS)?;

Ok(())
}

fn create_stores_for_v2(db: &IdbDatabase) -> Result<(), DomException> {
// We changed how we store inbound group sessions, the key used to
// be a tuple of `(room_id, sender_key, session_id)` now it's a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I noticed this comment has changed 👀, good job 👍)

// tuple of `(room_id, session_id)`
//
// Let's just drop the whole object store.
db.delete_object_store(keys::INBOUND_GROUP_SESSIONS)?;
db.create_object_store(keys::INBOUND_GROUP_SESSIONS)?;

db.create_object_store(keys::ROOM_SETTINGS)?;

Ok(())
}

fn create_stores_for_v3(db: &IdbDatabase) -> Result<(), DomException> {
// We changed the way we store outbound session.
// ShareInfo changed from a struct to an enum with struct variant.
// Let's just discard the existing outbounds
db.delete_object_store(keys::OUTBOUND_GROUP_SESSIONS)?;
db.create_object_store(keys::OUTBOUND_GROUP_SESSIONS)?;

// Support for MSC2399 withheld codes
db.create_object_store(keys::DIRECT_WITHHELD_INFO)?;

Ok(())
}

fn create_stores_for_v4(db: &IdbDatabase) -> Result<(), DomException> {
db.create_object_store(keys::SECRETS_INBOX)?;
Ok(())
}

fn create_stores_for_v5(db: &IdbDatabase) -> Result<(), DomException> {
// Create a new store for outgoing secret requests
let object_store = db.create_object_store(keys::GOSSIP_REQUESTS)?;

let mut params = IdbIndexParameters::new();
params.unique(false);
object_store.create_index_with_params(
keys::GOSSIP_REQUESTS_UNSENT_INDEX,
&IdbKeyPath::str("unsent"),
&params,
)?;

let mut params = IdbIndexParameters::new();
params.unique(true);
object_store.create_index_with_params(
keys::GOSSIP_REQUESTS_BY_INFO_INDEX,
&IdbKeyPath::str("info"),
&params,
)?;

if db.object_store_names().any(|n| n == "outgoing_secret_requests") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found the previous check with respect to the previous version number more obvious, but I think it should be equivalent, assuming atomic operations.

// Delete the old store names. We just delete any existing requests.
db.delete_object_store("outgoing_secret_requests")?;
db.delete_object_store("unsent_secret_requests")?;
db.delete_object_store("secret_requests_by_info")?;
}

Ok(())
}
Loading