-
Notifications
You must be signed in to change notification settings - Fork 269
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
Changes from all commits
4243555
fbbdc51
e643830
046603d
f43de3f
a4cdd59
0f16ad6
a4c46b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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? | ||
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) | ||
} | ||
} | ||
} |
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> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (looking at the commit 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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"), | ||
¶ms, | ||
)?; | ||
|
||
let mut params = IdbIndexParameters::new(); | ||
params.unique(true); | ||
object_store.create_index_with_params( | ||
keys::GOSSIP_REQUESTS_BY_INFO_INDEX, | ||
&IdbKeyPath::str("info"), | ||
¶ms, | ||
)?; | ||
|
||
if db.object_store_names().any(|n| n == "outgoing_secret_requests") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(()) | ||
} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.