From 6d2e9cfc02411130192eed7e5993f95a7ca9ba65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 9 Oct 2020 11:36:31 +0200 Subject: [PATCH 01/34] crypto: Share the users_for_key_claim map between modules. --- matrix_sdk_crypto/src/key_request.rs | 10 ++++------ matrix_sdk_crypto/src/machine.rs | 11 +++++++++-- matrix_sdk_crypto/src/session_manager.rs | 20 ++++++++++++++++---- 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/matrix_sdk_crypto/src/key_request.rs b/matrix_sdk_crypto/src/key_request.rs index 84f4eab2..060a85cb 100644 --- a/matrix_sdk_crypto/src/key_request.rs +++ b/matrix_sdk_crypto/src/key_request.rs @@ -196,6 +196,7 @@ impl KeyRequestMachine { device_id: Arc, store: Store, outbound_group_sessions: Arc>, + users_for_key_claim: Arc>>, ) -> Self { Self { user_id, @@ -205,7 +206,7 @@ impl KeyRequestMachine { outgoing_to_device_requests: Arc::new(DashMap::new()), incoming_key_requests: Arc::new(DashMap::new()), wait_queue: WaitQueue::new(), - users_for_key_claim: Arc::new(DashMap::new()), + users_for_key_claim, } } @@ -214,11 +215,6 @@ impl KeyRequestMachine { &self.user_id } - /// Get the map of user/devices which we need to claim one-time for. - pub fn users_for_key_claim(&self) -> &DashMap> { - &self.users_for_key_claim - } - pub fn outgoing_to_device_requests(&self) -> Vec { #[allow(clippy::map_clone)] self.outgoing_to_device_requests @@ -719,6 +715,7 @@ mod test { Arc::new(bob_device_id()), store, Arc::new(DashMap::new()), + Arc::new(DashMap::new()), ) } @@ -736,6 +733,7 @@ mod test { Arc::new(alice_device_id()), store, Arc::new(DashMap::new()), + Arc::new(DashMap::new()), ) } diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index cbe0acbf..10a1fd4c 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -131,11 +131,14 @@ impl OlmMachine { let store = Store::new(user_id.clone(), store, verification_machine.clone()); let device_id: Arc = Arc::new(device_id); let outbound_group_sessions = Arc::new(DashMap::new()); + let users_for_key_claim = Arc::new(DashMap::new()); + let key_request_machine = KeyRequestMachine::new( user_id.clone(), device_id.clone(), store.clone(), outbound_group_sessions, + users_for_key_claim.clone(), ); let account = Account { @@ -143,8 +146,12 @@ impl OlmMachine { store: store.clone(), }; - let session_manager = - SessionManager::new(account.clone(), key_request_machine.clone(), store.clone()); + let session_manager = SessionManager::new( + account.clone(), + users_for_key_claim, + key_request_machine.clone(), + store.clone(), + ); let group_session_manager = GroupSessionManager::new(account.clone(), store.clone()); let identity_manager = IdentityManager::new( user_id.clone(), diff --git a/matrix_sdk_crypto/src/session_manager.rs b/matrix_sdk_crypto/src/session_manager.rs index e0dc57bf..e10a2e5e 100644 --- a/matrix_sdk_crypto/src/session_manager.rs +++ b/matrix_sdk_crypto/src/session_manager.rs @@ -12,12 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{collections::BTreeMap, time::Duration}; +use std::{collections::BTreeMap, sync::Arc, time::Duration}; +use dashmap::{DashMap, DashSet}; use matrix_sdk_common::{ api::r0::keys::claim_keys::{Request as KeysClaimRequest, Response as KeysClaimResponse}, assign, - identifiers::{DeviceKeyAlgorithm, UserId}, + identifiers::{DeviceIdBox, DeviceKeyAlgorithm, UserId}, uuid::Uuid, }; use tracing::{error, info, warn}; @@ -28,17 +29,28 @@ use crate::{error::OlmResult, key_request::KeyRequestMachine, olm::Account, stor pub(crate) struct SessionManager { account: Account, store: Store, + /// A map of user/devices that we need to automatically claim keys for. + /// Submodules can insert user/device pairs into this map and the + /// user/device paris will be added to the list of users when + /// [`get_missing_sessions`](#method.get_missing_sessions) is called. + users_for_key_claim: Arc>>, key_request_machine: KeyRequestMachine, } impl SessionManager { const KEY_CLAIM_TIMEOUT: Duration = Duration::from_secs(10); - pub fn new(account: Account, key_request_machine: KeyRequestMachine, store: Store) -> Self { + pub fn new( + account: Account, + users_for_key_claim: Arc>>, + key_request_machine: KeyRequestMachine, + store: Store, + ) -> Self { Self { account, store, key_request_machine, + users_for_key_claim, } } @@ -109,7 +121,7 @@ impl SessionManager { // Add the list of sessions that for some reason automatically need to // create an Olm session. - for item in self.key_request_machine.users_for_key_claim().iter() { + for item in self.users_for_key_claim.iter() { let user = item.key(); for device_id in item.value().iter() { From bd0ac703a0008ad810f50d4d182150eaba1d2f61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 9 Oct 2020 15:39:35 +0200 Subject: [PATCH 02/34] crypto: Initial logic for session unwedging. --- matrix_sdk_crypto/src/error.rs | 4 +- matrix_sdk_crypto/src/identities/device.rs | 12 ++- matrix_sdk_crypto/src/machine.rs | 19 +++- matrix_sdk_crypto/src/olm/account.rs | 15 ++- matrix_sdk_crypto/src/session_manager.rs | 116 ++++++++++++++++++++- matrix_sdk_crypto/src/store/mod.rs | 17 ++- 6 files changed, 168 insertions(+), 15 deletions(-) diff --git a/matrix_sdk_crypto/src/error.rs b/matrix_sdk_crypto/src/error.rs index ee4e5f1f..39a554bb 100644 --- a/matrix_sdk_crypto/src/error.rs +++ b/matrix_sdk_crypto/src/error.rs @@ -48,8 +48,8 @@ pub enum OlmError { Store(#[from] CryptoStoreError), /// The session with a device has become corrupted. - #[error("decryption failed likely because a Olm session was wedged")] - SessionWedged, + #[error("decryption failed likely because an Olm from {0} with sender key {1} was wedged")] + SessionWedged(UserId, String), /// Encryption failed because the device does not have a valid Olm session /// with us. diff --git a/matrix_sdk_crypto/src/identities/device.rs b/matrix_sdk_crypto/src/identities/device.rs index 73aeb22d..578e7718 100644 --- a/matrix_sdk_crypto/src/identities/device.rs +++ b/matrix_sdk_crypto/src/identities/device.rs @@ -31,12 +31,13 @@ use matrix_sdk_common::{ EventType, }, identifiers::{DeviceId, DeviceKeyAlgorithm, DeviceKeyId, EventEncryptionAlgorithm, UserId}, + locks::Mutex, }; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; use tracing::warn; -use crate::olm::InboundGroupSession; +use crate::olm::{InboundGroupSession, Session}; #[cfg(test)] use crate::{OlmMachine, ReadOnlyAccount}; @@ -89,6 +90,15 @@ impl Device { .await } + /// Get the Olm sessions that belong to this device. + pub(crate) async fn get_sessions(&self) -> StoreResult>>>> { + if let Some(k) = self.get_key(DeviceKeyAlgorithm::Curve25519) { + self.verification_machine.store.get_sessions(k).await + } else { + Ok(None) + } + } + /// Get the trust state of the device. pub fn trust_state(&self) -> bool { self.inner diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 10a1fd4c..3e64e48a 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -45,7 +45,7 @@ use matrix_sdk_common::{ #[cfg(feature = "sqlite_cryptostore")] use crate::store::sqlite::SqliteStore; use crate::{ - error::{EventError, MegolmError, MegolmResult, OlmResult}, + error::{EventError, MegolmError, MegolmResult, OlmError, OlmResult}, group_manager::GroupSessionManager, identities::{Device, IdentityManager, ReadOnlyDevice, UserDevices, UserIdentities}, key_request::KeyRequestMachine, @@ -645,6 +645,8 @@ impl OlmMachine { .mark_outgoing_request_as_sent(request_id) .await?; self.group_session_manager.mark_request_as_sent(request_id); + self.session_manager + .mark_outgoing_request_as_sent(request_id); Ok(()) } @@ -710,8 +712,19 @@ impl OlmMachine { "Failed to decrypt to-device event from {} {}", e.sender, err ); - // TODO if the session is wedged mark it for - // unwedging. + + if let OlmError::SessionWedged(sender, curve_key) = err { + if let Err(e) = self + .session_manager + .mark_device_as_wedged(&sender, &curve_key) + .await + { + error!( + "Couldn't mark device from {} to be unwedged {:?}", + sender, e + ); + } + } continue; } }; diff --git a/matrix_sdk_crypto/src/olm/account.rs b/matrix_sdk_crypto/src/olm/account.rs index 0cf5e569..a6522ba1 100644 --- a/matrix_sdk_crypto/src/olm/account.rs +++ b/matrix_sdk_crypto/src/olm/account.rs @@ -205,7 +205,10 @@ impl Account { for sender {} and sender_key {} {:?}", sender, sender_key, e ); - return Err(OlmError::SessionWedged); + return Err(OlmError::SessionWedged( + sender.to_owned(), + sender_key.to_owned(), + )); } } } @@ -248,7 +251,10 @@ impl Account { available sessions {} {}", sender, sender_key ); - return Err(OlmError::SessionWedged); + return Err(OlmError::SessionWedged( + sender.to_owned(), + sender_key.to_owned(), + )); } OlmMessage::PreKey(m) => { @@ -265,7 +271,10 @@ impl Account { from a prekey message: {}", sender, sender_key, e ); - return Err(OlmError::SessionWedged); + return Err(OlmError::SessionWedged( + sender.to_owned(), + sender_key.to_owned(), + )); } }; diff --git a/matrix_sdk_crypto/src/session_manager.rs b/matrix_sdk_crypto/src/session_manager.rs index e10a2e5e..792caa81 100644 --- a/matrix_sdk_crypto/src/session_manager.rs +++ b/matrix_sdk_crypto/src/session_manager.rs @@ -16,14 +16,26 @@ use std::{collections::BTreeMap, sync::Arc, time::Duration}; use dashmap::{DashMap, DashSet}; use matrix_sdk_common::{ - api::r0::keys::claim_keys::{Request as KeysClaimRequest, Response as KeysClaimResponse}, + api::r0::{ + keys::claim_keys::{Request as KeysClaimRequest, Response as KeysClaimResponse}, + to_device::DeviceIdOrAllDevices, + }, assign, - identifiers::{DeviceIdBox, DeviceKeyAlgorithm, UserId}, + events::EventType, + identifiers::{DeviceId, DeviceIdBox, DeviceKeyAlgorithm, UserId}, uuid::Uuid, }; +use serde_json::{json, value::to_raw_value}; use tracing::{error, info, warn}; -use crate::{error::OlmResult, key_request::KeyRequestMachine, olm::Account, store::Store}; +use crate::{ + error::OlmResult, + key_request::KeyRequestMachine, + olm::Account, + requests::{OutgoingRequest, ToDeviceRequest}, + store::{Result as StoreResult, Store}, + Device, +}; #[derive(Debug, Clone)] pub(crate) struct SessionManager { @@ -34,11 +46,14 @@ pub(crate) struct SessionManager { /// user/device paris will be added to the list of users when /// [`get_missing_sessions`](#method.get_missing_sessions) is called. users_for_key_claim: Arc>>, + wedged_devices: Arc>>, key_request_machine: KeyRequestMachine, + outgoing_to_device_requests: Arc>, } impl SessionManager { const KEY_CLAIM_TIMEOUT: Duration = Duration::from_secs(10); + const UNWEDGING_INTERVAL: Duration = Duration::from_secs(60 * 60); pub fn new( account: Account, @@ -51,9 +66,95 @@ impl SessionManager { store, key_request_machine, users_for_key_claim, + wedged_devices: Arc::new(DashMap::new()), + outgoing_to_device_requests: Arc::new(DashMap::new()), } } + /// Mark the outgoing request as sent. + pub fn mark_outgoing_request_as_sent(&self, id: &Uuid) { + self.outgoing_to_device_requests.remove(id); + } + + pub async fn mark_device_as_wedged(&self, sender: &UserId, curve_key: &str) -> StoreResult<()> { + if let Some(device) = self + .store + .get_device_from_curve_key(sender, curve_key) + .await? + { + let sessions = device.get_sessions().await?; + + if let Some(sessions) = sessions { + let mut sessions = sessions.lock().await; + sessions.sort_by_key(|s| s.creation_time.clone()); + + let session = sessions.get(0); + + if let Some(session) = session { + if session.creation_time.elapsed() > Self::UNWEDGING_INTERVAL { + self.wedged_devices + .entry(device.user_id().to_owned()) + .or_insert_with(DashSet::new) + .insert(device.device_id().into()); + } + } + } + } + + Ok(()) + } + + #[allow(dead_code)] + pub fn is_device_wedged(&self, device: &Device) -> bool { + self.wedged_devices + .get(device.user_id()) + .map(|d| d.contains(device.device_id())) + .unwrap_or(false) + } + + /// Check if the session was created to unwedge a Device. + /// + /// If the device was wedged this will queue up a dummy to-device message. + async fn check_if_unwedged(&self, user_id: &UserId, device_id: &DeviceId) -> OlmResult<()> { + if self + .wedged_devices + .get(user_id) + .map(|d| d.remove(device_id)) + .flatten() + .is_some() + { + if let Some(device) = self.store.get_device(user_id, device_id).await? { + let content = device.encrypt(EventType::Dummy, json!({})).await?; + let id = Uuid::new_v4(); + let mut messages = BTreeMap::new(); + + messages + .entry(device.user_id().to_owned()) + .or_insert_with(BTreeMap::new) + .insert( + DeviceIdOrAllDevices::DeviceId(device.device_id().into()), + to_raw_value(&content)?, + ); + + let request = OutgoingRequest { + request_id: id, + request: Arc::new( + ToDeviceRequest { + event_type: EventType::RoomEncrypted, + txn_id: id, + messages, + } + .into(), + ), + }; + + self.outgoing_to_device_requests.insert(id, request); + } + } + + Ok(()) + } + /// Get the a key claiming request for the user/device pairs that we are /// missing Olm sessions for. /// @@ -189,9 +290,14 @@ impl SessionManager { continue; } - // TODO if this session was created because a previous one was - // wedged queue up a dummy event to be sent out. self.key_request_machine.retry_keyshare(&user_id, device_id); + + if let Err(e) = self.check_if_unwedged(&user_id, device_id).await { + error!( + "Error while treating an unwedged device {} {} {:?}", + user_id, device_id, e + ); + } } } Ok(()) diff --git a/matrix_sdk_crypto/src/store/mod.rs b/matrix_sdk_crypto/src/store/mod.rs index 83c2a0f9..767bbae4 100644 --- a/matrix_sdk_crypto/src/store/mod.rs +++ b/matrix_sdk_crypto/src/store/mod.rs @@ -63,7 +63,9 @@ use url::ParseError; use sqlx::Error as SqlxError; use matrix_sdk_common::{ - identifiers::{DeviceId, Error as IdentifierValidationError, RoomId, UserId}, + identifiers::{ + DeviceId, DeviceKeyAlgorithm, Error as IdentifierValidationError, RoomId, UserId, + }, locks::Mutex, }; use matrix_sdk_common_macros::async_trait; @@ -118,6 +120,19 @@ impl Store { self.inner.get_user_devices(user_id).await } + pub async fn get_device_from_curve_key( + &self, + user_id: &UserId, + curve_key: &str, + ) -> Result> { + self.get_user_devices(user_id).await.map(|d| { + d.devices().find(|d| { + d.get_key(DeviceKeyAlgorithm::Curve25519) + .map_or(false, |k| k == curve_key) + }) + }) + } + pub async fn get_user_devices(&self, user_id: &UserId) -> Result { let devices = self.inner.get_user_devices(user_id).await?; From 1c008f549c44e4d941e621c1a1c4b84042c43f99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Sat, 10 Oct 2020 12:08:58 +0200 Subject: [PATCH 03/34] common: Expose the lock guards publicly. --- matrix_sdk_common/src/locks.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/matrix_sdk_common/src/locks.rs b/matrix_sdk_common/src/locks.rs index 6f648a39..aebf244b 100644 --- a/matrix_sdk_common/src/locks.rs +++ b/matrix_sdk_common/src/locks.rs @@ -3,11 +3,7 @@ // https://www.reddit.com/r/rust/comments/f4zldz/i_audited_3_different_implementation_of_async/ #[cfg(target_arch = "wasm32")] -pub use futures_locks::Mutex; -#[cfg(target_arch = "wasm32")] -pub use futures_locks::RwLock; +pub use futures_locks::{Mutex, MutexGuard, RwLock, RwLockReadGuard, RwLockWriteGuard}; #[cfg(not(target_arch = "wasm32"))] -pub use tokio::sync::Mutex; -#[cfg(not(target_arch = "wasm32"))] -pub use tokio::sync::RwLock; +pub use tokio::sync::{Mutex, MutexGuard, RwLock, RwLockReadGuard, RwLockWriteGuard}; From c85fe6bc2118bfcb8dce50e2f75e4245d1fcf435 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 14 Oct 2020 15:35:06 +0200 Subject: [PATCH 04/34] crypto: Initial support for private cross signing identities. --- matrix_sdk/Cargo.toml | 4 +- matrix_sdk_base/Cargo.toml | 2 +- matrix_sdk_crypto/Cargo.toml | 5 +- matrix_sdk_crypto/src/identities/user.rs | 24 +- matrix_sdk_crypto/src/olm/mod.rs | 1 + matrix_sdk_crypto/src/olm/signing.rs | 606 +++++++++++++++++++++++ matrix_sdk_test/Cargo.toml | 2 +- 7 files changed, 635 insertions(+), 9 deletions(-) create mode 100644 matrix_sdk_crypto/src/olm/signing.rs diff --git a/matrix_sdk/Cargo.toml b/matrix_sdk/Cargo.toml index cc1ba580..608f6c57 100644 --- a/matrix_sdk/Cargo.toml +++ b/matrix_sdk/Cargo.toml @@ -31,7 +31,7 @@ docs = ["encryption", "sqlite_cryptostore", "messages"] async-trait = "0.1.41" dashmap = { version = "3.11.10", optional = true } http = "0.2.1" -serde_json = "1.0.58" +serde_json = "1.0.59" thiserror = "1.0.21" tracing = "0.1.21" url = "2.1.1" @@ -73,7 +73,7 @@ async-std = { version = "1.6.5", features = ["unstable"] } dirs = "3.0.1" matrix-sdk-test = { version = "0.1.0", path = "../matrix_sdk_test" } tokio = { version = "0.2.22", features = ["rt-threaded", "macros"] } -serde_json = "1.0.58" +serde_json = "1.0.59" tracing-subscriber = "0.2.13" tempfile = "3.1.0" mockito = "0.27.0" diff --git a/matrix_sdk_base/Cargo.toml b/matrix_sdk_base/Cargo.toml index 7d8de457..5eb04c64 100644 --- a/matrix_sdk_base/Cargo.toml +++ b/matrix_sdk_base/Cargo.toml @@ -26,7 +26,7 @@ docs = ["encryption", "sqlite_cryptostore", "messages"] [dependencies] async-trait = "0.1.41" serde = "1.0.116" -serde_json = "1.0.58" +serde_json = "1.0.59" zeroize = "1.1.1" tracing = "0.1.21" diff --git a/matrix_sdk_crypto/Cargo.toml b/matrix_sdk_crypto/Cargo.toml index 023c9cb3..c437da65 100644 --- a/matrix_sdk_crypto/Cargo.toml +++ b/matrix_sdk_crypto/Cargo.toml @@ -28,7 +28,7 @@ matrix-sdk-common = { version = "0.1.0", path = "../matrix_sdk_common" } olm-rs = { version = "1.0.0", features = ["serde"] } getrandom = "0.2.0" serde = { version = "1.0.116", features = ["derive", "rc"] } -serde_json = "1.0.58" +serde_json = "1.0.59" cjson = "0.1.1" zeroize = { version = "1.1.1", features = ["zeroize_derive"] } url = "2.1.1" @@ -39,6 +39,7 @@ tracing = "0.1.21" atomic = "0.5.0" dashmap = "3.11.10" sha2 = "0.9.1" +aes-gcm = "0.7.0" aes-ctr = "0.5.0" pbkdf2 = { version = "0.5.0", default-features = false } hmac = "0.9.0" @@ -60,7 +61,7 @@ features = ["runtime-tokio", "sqlite", "macros"] tokio = { version = "0.2.22", features = ["rt-threaded", "macros"] } futures = "0.3.6" proptest = "0.10.1" -serde_json = "1.0.58" +serde_json = "1.0.59" tempfile = "3.1.0" http = "0.2.1" matrix-sdk-test = { version = "0.1.0", path = "../matrix_sdk_test" } diff --git a/matrix_sdk_crypto/src/identities/user.rs b/matrix_sdk_crypto/src/identities/user.rs index a23dbc96..83055ab2 100644 --- a/matrix_sdk_crypto/src/identities/user.rs +++ b/matrix_sdk_crypto/src/identities/user.rs @@ -86,6 +86,24 @@ impl From for UserSigningPubkey { } } +impl Into for MasterPubkey { + fn into(self) -> CrossSigningKey { + self.0.as_ref().clone() + } +} + +impl Into for UserSigningPubkey { + fn into(self) -> CrossSigningKey { + self.0.as_ref().clone() + } +} + +impl Into for SelfSigningPubkey { + fn into(self) -> CrossSigningKey { + self.0.as_ref().clone() + } +} + impl AsRef for MasterPubkey { fn as_ref(&self) -> &CrossSigningKey { &self.0 @@ -135,7 +153,7 @@ impl<'a> From<&'a UserSigningPubkey> for CrossSigningSubKeys<'a> { } /// Enum over the cross signing sub-keys. -enum CrossSigningSubKeys<'a> { +pub(crate) enum CrossSigningSubKeys<'a> { /// The self signing subkey. SelfSigning(&'a SelfSigningPubkey), /// The user signing subkey. @@ -152,7 +170,7 @@ impl<'a> CrossSigningSubKeys<'a> { } /// Get the `CrossSigningKey` from an sub-keys enum - fn cross_signing_key(&self) -> &CrossSigningKey { + pub(crate) fn cross_signing_key(&self) -> &CrossSigningKey { match self { CrossSigningSubKeys::SelfSigning(key) => &key.0, CrossSigningSubKeys::UserSigning(key) => &key.0, @@ -198,7 +216,7 @@ impl MasterPubkey { /// /// Returns an empty result if the signature check succeeded, otherwise a /// SignatureError indicating why the check failed. - fn verify_subkey<'a>( + pub(crate) fn verify_subkey<'a>( &self, subkey: impl Into>, ) -> Result<(), SignatureError> { diff --git a/matrix_sdk_crypto/src/olm/mod.rs b/matrix_sdk_crypto/src/olm/mod.rs index eef28ff1..f2e39094 100644 --- a/matrix_sdk_crypto/src/olm/mod.rs +++ b/matrix_sdk_crypto/src/olm/mod.rs @@ -20,6 +20,7 @@ mod account; mod group_sessions; mod session; +mod signing; mod utility; pub(crate) use account::Account; diff --git a/matrix_sdk_crypto/src/olm/signing.rs b/matrix_sdk_crypto/src/olm/signing.rs new file mode 100644 index 00000000..db15bd95 --- /dev/null +++ b/matrix_sdk_crypto/src/olm/signing.rs @@ -0,0 +1,606 @@ +// Copyright 2020 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. + +#![allow(dead_code)] + +use aes_gcm::{ + aead::{generic_array::GenericArray, Aead, NewAead}, + Aes256Gcm, +}; +use base64::{decode_config, encode_config, DecodeError, URL_SAFE_NO_PAD}; +use serde::{Deserialize, Serialize}; +use std::{ + collections::BTreeMap, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }, +}; +use zeroize::Zeroizing; + +use olm_rs::{errors::OlmUtilityError, pk::OlmPkSigning, utility::OlmUtility, PicklingMode}; + +use matrix_sdk_common::{ + api::r0::keys::{upload_signing_keys::Request as UploadRequest, CrossSigningKey, KeyUsage}, + identifiers::UserId, + locks::Mutex, +}; + +use crate::identities::{MasterPubkey, SelfSigningPubkey, UserSigningPubkey}; + +fn encode>(input: T) -> String { + encode_config(input, URL_SAFE_NO_PAD) +} + +fn decode>(input: T) -> Result, DecodeError> { + decode_config(input, URL_SAFE_NO_PAD) +} + +#[derive(Clone)] +pub struct Signing { + inner: Arc>, + seed: Arc>>, + public_key: PublicSigningKey, +} + +impl std::fmt::Debug for Signing { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Signing") + .field("public_key", &self.public_key.as_str()) + .finish() + } +} + +impl PartialEq for Signing { + fn eq(&self, other: &Signing) -> bool { + self.seed == other.seed + } +} + +#[derive(Clone, PartialEq, Debug)] +struct MasterSigning { + inner: Signing, + public_key: MasterPubkey, +} + +#[derive(Debug, Clone, Deserialize, Serialize)] +struct PickledMasterSigning { + pickle: PickledSigning, + public_key: CrossSigningKey, +} + +#[derive(Debug, Clone, Deserialize, Serialize)] +struct PickledUserSigning { + pickle: PickledSigning, + public_key: CrossSigningKey, +} + +#[derive(Debug, Clone, Deserialize, Serialize)] +struct PickledSelfSigning { + pickle: PickledSigning, + public_key: CrossSigningKey, +} + +impl MasterSigning { + async fn pickle(&self, pickle_mode: PicklingMode) -> PickledMasterSigning { + let pickle = self.inner.pickle(pickle_mode).await; + let public_key = self.public_key.clone().into(); + PickledMasterSigning { pickle, public_key } + } + + fn from_pickle(pickle: PickledMasterSigning, pickle_mode: PicklingMode) -> Self { + let inner = Signing::from_pickle(pickle.pickle, pickle_mode); + + Self { + inner, + public_key: pickle.public_key.into(), + } + } + + async fn sign_subkey<'a>(&self, subkey: &mut CrossSigningKey) { + // TODO create a borrowed version of a cross singing key. + let subkey_wihtout_signatures = CrossSigningKey { + user_id: subkey.user_id.clone(), + keys: subkey.keys.clone(), + usage: subkey.usage.clone(), + signatures: BTreeMap::new(), + }; + + let message = cjson::to_string(&subkey_wihtout_signatures) + .expect("Can't serialize cross signing subkey"); + let signature = self.inner.sign(&message).await; + + subkey + .signatures + .entry(self.public_key.user_id().to_owned()) + .or_insert_with(BTreeMap::new) + .insert( + format!("ed25519:{}", self.inner.public_key().as_str()), + signature.0, + ); + } +} + +impl UserSigning { + async fn pickle(&self, pickle_mode: PicklingMode) -> PickledUserSigning { + let pickle = self.inner.pickle(pickle_mode).await; + let public_key = self.public_key.clone().into(); + PickledUserSigning { pickle, public_key } + } + + fn from_pickle(pickle: PickledUserSigning, pickle_mode: PicklingMode) -> Self { + let inner = Signing::from_pickle(pickle.pickle, pickle_mode); + + Self { + inner, + public_key: pickle.public_key.into(), + } + } +} + +impl SelfSigning { + async fn pickle(&self, pickle_mode: PicklingMode) -> PickledSelfSigning { + let pickle = self.inner.pickle(pickle_mode).await; + let public_key = self.public_key.clone().into(); + PickledSelfSigning { pickle, public_key } + } + + fn from_pickle(pickle: PickledSelfSigning, pickle_mode: PicklingMode) -> Self { + let inner = Signing::from_pickle(pickle.pickle, pickle_mode); + + Self { + inner, + public_key: pickle.public_key.into(), + } + } +} + +#[derive(Clone, PartialEq, Debug)] +struct SelfSigning { + inner: Signing, + public_key: SelfSigningPubkey, +} + +#[derive(Clone, PartialEq, Debug)] +struct UserSigning { + inner: Signing, + public_key: UserSigningPubkey, +} + +#[derive(Clone, Debug)] +pub struct PrivateCrossSigningIdentity { + user_id: Arc, + shared: Arc, + master_key: Arc>>, + user_signing_key: Arc>>, + self_signing_key: Arc>>, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct PickledCrossSigningIdentity { + user_id: UserId, + shared: bool, + + master_key: Option, + user_signing_key: Option, + self_signing_key: Option, +} + +#[derive(Debug, Clone)] +pub struct Signature(String); + +#[derive(Debug, Clone, Serialize, Deserialize)] +struct PickledSigning(String); + +#[derive(Debug, Clone, Serialize, Deserialize)] +struct InnerPickle { + version: u8, + nonce: String, + ciphertext: String, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct PublicSigningKey(Arc); + +impl Signature { + fn as_str(&self) -> &str { + &self.0 + } +} + +impl PickledSigning { + fn as_str(&self) -> &str { + &self.0 + } +} + +impl PublicSigningKey { + fn as_str(&self) -> &str { + &self.0 + } + + fn to_string(&self) -> String { + self.0.to_string() + } +} + +impl Signing { + fn new() -> Self { + let seed = OlmPkSigning::generate_seed(); + Self::from_seed(seed) + } + + fn from_seed(seed: Vec) -> Self { + let inner = OlmPkSigning::new(seed.clone()).expect("Unable to create pk signing object"); + let public_key = PublicSigningKey(Arc::new(inner.public_key().to_owned())); + + Signing { + inner: Arc::new(Mutex::new(inner)), + seed: Arc::new(Zeroizing::from(seed)), + public_key, + } + } + + fn from_pickle(pickle: PickledSigning, pickle_mode: PicklingMode) -> Self { + let mode = if let PicklingMode::Encrypted { key } = &pickle_mode { + key + } else { + "DEFAULT_PICKLE_PASSPHRASE_123456".as_bytes() + }; + + let pickled: InnerPickle = serde_json::from_str(pickle.as_str()).unwrap(); + + let key = GenericArray::from_slice(mode); + let cipher = Aes256Gcm::new(key); + + let nonce = decode(pickled.nonce).unwrap(); + let nonce = GenericArray::from_slice(&nonce); + let ciphertext = &decode(pickled.ciphertext).unwrap(); + + let seed = cipher + .decrypt(&nonce, ciphertext.as_slice()) + .expect("Can't decrypt pickle"); + + Self::from_seed(seed) + } + + async fn pickle(&self, pickle_mode: PicklingMode) -> PickledSigning { + let mode = if let PicklingMode::Encrypted { key } = &pickle_mode { + key + } else { + "DEFAULT_PICKLE_PASSPHRASE_123456".as_bytes() + }; + + let key = GenericArray::from_slice(mode); + let cipher = Aes256Gcm::new(key); + let nonce = GenericArray::from_slice(b"unique nonce"); + let ciphertext = cipher + .encrypt(nonce, self.seed.as_slice()) + .expect("Can't encrypt signing pickle"); + + let ciphertext = encode(ciphertext); + + let pickle = InnerPickle { + version: 1, + nonce: encode(nonce.as_slice()), + ciphertext, + }; + + PickledSigning(serde_json::to_string(&pickle).expect("Can't encode pickled signing")) + } + + fn public_key(&self) -> &PublicSigningKey { + &self.public_key + } + + fn cross_signing_key(&self, user_id: UserId, usage: KeyUsage) -> CrossSigningKey { + let mut keys = BTreeMap::new(); + + keys.insert( + format!("ed25519:{}", self.public_key().as_str()), + self.public_key().to_string(), + ); + + CrossSigningKey { + user_id, + usage: vec![usage], + keys, + signatures: BTreeMap::new(), + } + } + + async fn verify(&self, message: &str, signature: &Signature) -> Result { + let utility = OlmUtility::new(); + utility.ed25519_verify(self.public_key.as_str(), message, signature.as_str()) + } + + async fn sign(&self, message: &str) -> Signature { + Signature(self.inner.lock().await.sign(message)) + } +} + +impl PrivateCrossSigningIdentity { + async fn new(user_id: UserId) -> Self { + let master = Signing::new(); + + let public_key = master.cross_signing_key(user_id.clone(), KeyUsage::Master); + let master = MasterSigning { + inner: master, + public_key: public_key.into(), + }; + + let user = Signing::new(); + let mut public_key = user.cross_signing_key(user_id.clone(), KeyUsage::UserSigning); + master.sign_subkey(&mut public_key).await; + + let user = UserSigning { + inner: user, + public_key: public_key.into(), + }; + + let self_signing = Signing::new(); + let mut public_key = self_signing.cross_signing_key(user_id.clone(), KeyUsage::SelfSigning); + master.sign_subkey(&mut public_key).await; + + let self_signing = SelfSigning { + inner: self_signing, + public_key: public_key.into(), + }; + + Self { + user_id: Arc::new(user_id), + shared: Arc::new(AtomicBool::new(false)), + master_key: Arc::new(Mutex::new(Some(master))), + self_signing_key: Arc::new(Mutex::new(Some(self_signing))), + user_signing_key: Arc::new(Mutex::new(Some(user))), + } + } + + fn mark_as_shared(&self) { + self.shared.store(true, Ordering::SeqCst) + } + + pub fn shared(&self) -> bool { + self.shared.load(Ordering::SeqCst) + } + + async fn pickle(&self, pickle_mode: PicklingMode) -> PickledCrossSigningIdentity { + let pickle_key = if let PicklingMode::Encrypted { key } = &pickle_mode { + key + } else { + "DEFAULT_PICKLE_PASSPHRASE_123456".as_bytes() + } + .to_vec(); + + let master_key = if let Some(m) = self.master_key.lock().await.as_ref() { + Some( + m.pickle(PicklingMode::Encrypted { + key: pickle_key.clone(), + }) + .await, + ) + } else { + None + }; + + let self_signing_key = if let Some(m) = self.self_signing_key.lock().await.as_ref() { + Some( + m.pickle(PicklingMode::Encrypted { + key: pickle_key.clone(), + }) + .await, + ) + } else { + None + }; + + let user_signing_key = if let Some(m) = self.user_signing_key.lock().await.as_ref() { + Some( + m.pickle(PicklingMode::Encrypted { + key: pickle_key.clone(), + }) + .await, + ) + } else { + None + }; + + PickledCrossSigningIdentity { + user_id: self.user_id.as_ref().to_owned(), + shared: self.shared(), + master_key, + self_signing_key, + user_signing_key, + } + } + + async fn from_pickle(pickle: PickledCrossSigningIdentity, pickle_mode: PicklingMode) -> Self { + let pickle_key = if let PicklingMode::Encrypted { key } = &pickle_mode { + key + } else { + "DEFAULT_PICKLE_PASSPHRASE_123456".as_bytes() + } + .to_vec(); + + let master = if let Some(m) = pickle.master_key { + Some(MasterSigning::from_pickle( + m, + PicklingMode::Encrypted { + key: pickle_key.clone(), + }, + )) + } else { + None + }; + + let self_signing = if let Some(s) = pickle.self_signing_key { + Some(SelfSigning::from_pickle( + s, + PicklingMode::Encrypted { + key: pickle_key.clone(), + }, + )) + } else { + None + }; + + let user_signing = if let Some(u) = pickle.user_signing_key { + Some(UserSigning::from_pickle( + u, + PicklingMode::Encrypted { key: pickle_key }, + )) + } else { + None + }; + + Self { + user_id: Arc::new(pickle.user_id), + shared: Arc::new(AtomicBool::from(pickle.shared)), + master_key: Arc::new(Mutex::new(master)), + self_signing_key: Arc::new(Mutex::new(self_signing)), + user_signing_key: Arc::new(Mutex::new(user_signing)), + } + } + + async fn as_upload_request(&self) -> UploadRequest<'_> { + UploadRequest { + auth: None, + master_key: self + .master_key + .lock() + .await + .as_ref() + .cloned() + .map(|k| k.public_key.into()), + user_signing_key: self + .user_signing_key + .lock() + .await + .as_ref() + .cloned() + .map(|k| k.public_key.into()), + self_signing_key: self + .self_signing_key + .lock() + .await + .as_ref() + .cloned() + .map(|k| k.public_key.into()), + } + } +} + +#[cfg(test)] +mod test { + use super::{PrivateCrossSigningIdentity, Signing}; + + use matrix_sdk_common::identifiers::{user_id, UserId}; + use matrix_sdk_test::async_test; + use olm_rs::PicklingMode; + + fn user_id() -> UserId { + user_id!("@example:localhost") + } + + #[test] + fn signing_creation() { + let signing = Signing::new(); + assert!(!signing.public_key().as_str().is_empty()); + } + + #[async_test] + async fn signature_verification() { + let signing = Signing::new(); + + let message = "Hello world"; + + let signature = signing.sign(message).await; + assert!(signing.verify(message, &signature).await.is_ok()); + } + + #[async_test] + async fn pickling_signing() { + let signing = Signing::new(); + let pickled = signing.pickle(PicklingMode::Unencrypted).await; + + let unpickled = Signing::from_pickle(pickled, PicklingMode::Unencrypted); + + assert_eq!(signing.public_key(), unpickled.public_key()); + } + + #[async_test] + async fn private_identity_creation() { + let identity = PrivateCrossSigningIdentity::new(user_id()).await; + + assert!(identity + .master_key + .lock() + .await + .as_ref() + .unwrap() + .public_key + .verify_subkey( + &identity + .self_signing_key + .lock() + .await + .as_ref() + .unwrap() + .public_key, + ) + .is_ok()); + + assert!(identity + .master_key + .lock() + .await + .as_ref() + .unwrap() + .public_key + .verify_subkey( + &identity + .user_signing_key + .lock() + .await + .as_ref() + .unwrap() + .public_key, + ) + .is_ok()); + } + + #[async_test] + async fn identity_pickling() { + let identity = PrivateCrossSigningIdentity::new(user_id()).await; + + let pickled = identity.pickle(PicklingMode::Unencrypted).await; + + let unpickled = + PrivateCrossSigningIdentity::from_pickle(pickled, PicklingMode::Unencrypted).await; + + assert_eq!(identity.user_id, unpickled.user_id); + assert_eq!( + &*identity.master_key.lock().await, + &*unpickled.master_key.lock().await + ); + assert_eq!( + &*identity.user_signing_key.lock().await, + &*unpickled.user_signing_key.lock().await + ); + assert_eq!( + &*identity.self_signing_key.lock().await, + &*unpickled.self_signing_key.lock().await + ); + } +} diff --git a/matrix_sdk_test/Cargo.toml b/matrix_sdk_test/Cargo.toml index 108ad599..b6a7bcb9 100644 --- a/matrix_sdk_test/Cargo.toml +++ b/matrix_sdk_test/Cargo.toml @@ -11,7 +11,7 @@ repository = "https://github.com/matrix-org/matrix-rust-sdk" version = "0.1.0" [dependencies] -serde_json = "1.0.58" +serde_json = "1.0.59" http = "0.2.1" matrix-sdk-common = { version = "0.1.0", path = "../matrix_sdk_common" } matrix-sdk-test-macros = { version = "0.1.0", path = "../matrix_sdk_test_macros" } From 4e8ce4cb5d12682cce6e9df060e3c88ba532460a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 14 Oct 2020 16:01:52 +0200 Subject: [PATCH 05/34] crypto: Fix clippy warnings and don't use the PickleMode for signing pickling. --- matrix_sdk_crypto/src/olm/signing.rs | 167 ++++++++++----------------- 1 file changed, 61 insertions(+), 106 deletions(-) diff --git a/matrix_sdk_crypto/src/olm/signing.rs b/matrix_sdk_crypto/src/olm/signing.rs index db15bd95..95dc5e65 100644 --- a/matrix_sdk_crypto/src/olm/signing.rs +++ b/matrix_sdk_crypto/src/olm/signing.rs @@ -29,7 +29,7 @@ use std::{ }; use zeroize::Zeroizing; -use olm_rs::{errors::OlmUtilityError, pk::OlmPkSigning, utility::OlmUtility, PicklingMode}; +use olm_rs::{errors::OlmUtilityError, pk::OlmPkSigning, utility::OlmUtility}; use matrix_sdk_common::{ api::r0::keys::{upload_signing_keys::Request as UploadRequest, CrossSigningKey, KeyUsage}, @@ -93,14 +93,14 @@ struct PickledSelfSigning { } impl MasterSigning { - async fn pickle(&self, pickle_mode: PicklingMode) -> PickledMasterSigning { - let pickle = self.inner.pickle(pickle_mode).await; + async fn pickle(&self, pickle_key: &[u8]) -> PickledMasterSigning { + let pickle = self.inner.pickle(pickle_key).await; let public_key = self.public_key.clone().into(); PickledMasterSigning { pickle, public_key } } - fn from_pickle(pickle: PickledMasterSigning, pickle_mode: PicklingMode) -> Self { - let inner = Signing::from_pickle(pickle.pickle, pickle_mode); + fn from_pickle(pickle: PickledMasterSigning, pickle_key: &[u8]) -> Self { + let inner = Signing::from_pickle(pickle.pickle, pickle_key); Self { inner, @@ -133,14 +133,14 @@ impl MasterSigning { } impl UserSigning { - async fn pickle(&self, pickle_mode: PicklingMode) -> PickledUserSigning { - let pickle = self.inner.pickle(pickle_mode).await; + async fn pickle(&self, pickle_key: &[u8]) -> PickledUserSigning { + let pickle = self.inner.pickle(pickle_key).await; let public_key = self.public_key.clone().into(); PickledUserSigning { pickle, public_key } } - fn from_pickle(pickle: PickledUserSigning, pickle_mode: PicklingMode) -> Self { - let inner = Signing::from_pickle(pickle.pickle, pickle_mode); + fn from_pickle(pickle: PickledUserSigning, pickle_key: &[u8]) -> Self { + let inner = Signing::from_pickle(pickle.pickle, pickle_key); Self { inner, @@ -150,14 +150,14 @@ impl UserSigning { } impl SelfSigning { - async fn pickle(&self, pickle_mode: PicklingMode) -> PickledSelfSigning { - let pickle = self.inner.pickle(pickle_mode).await; + async fn pickle(&self, pickle_key: &[u8]) -> PickledSelfSigning { + let pickle = self.inner.pickle(pickle_key).await; let public_key = self.public_key.clone().into(); PickledSelfSigning { pickle, public_key } } - fn from_pickle(pickle: PickledSelfSigning, pickle_mode: PicklingMode) -> Self { - let inner = Signing::from_pickle(pickle.pickle, pickle_mode); + fn from_pickle(pickle: PickledSelfSigning, pickle_key: &[u8]) -> Self { + let inner = Signing::from_pickle(pickle.pickle, pickle_key); Self { inner, @@ -230,6 +230,7 @@ impl PublicSigningKey { &self.0 } + #[allow(clippy::inherent_to_string)] fn to_string(&self) -> String { self.0.to_string() } @@ -252,16 +253,10 @@ impl Signing { } } - fn from_pickle(pickle: PickledSigning, pickle_mode: PicklingMode) -> Self { - let mode = if let PicklingMode::Encrypted { key } = &pickle_mode { - key - } else { - "DEFAULT_PICKLE_PASSPHRASE_123456".as_bytes() - }; - + fn from_pickle(pickle: PickledSigning, pickle_key: &[u8]) -> Self { let pickled: InnerPickle = serde_json::from_str(pickle.as_str()).unwrap(); - let key = GenericArray::from_slice(mode); + let key = GenericArray::from_slice(pickle_key); let cipher = Aes256Gcm::new(key); let nonce = decode(pickled.nonce).unwrap(); @@ -275,14 +270,8 @@ impl Signing { Self::from_seed(seed) } - async fn pickle(&self, pickle_mode: PicklingMode) -> PickledSigning { - let mode = if let PicklingMode::Encrypted { key } = &pickle_mode { - key - } else { - "DEFAULT_PICKLE_PASSPHRASE_123456".as_bytes() - }; - - let key = GenericArray::from_slice(mode); + async fn pickle(&self, pickle_key: &[u8]) -> PickledSigning { + let key = GenericArray::from_slice(pickle_key); let cipher = Aes256Gcm::new(key); let nonce = GenericArray::from_slice(b"unique nonce"); let ciphertext = cipher @@ -375,43 +364,21 @@ impl PrivateCrossSigningIdentity { self.shared.load(Ordering::SeqCst) } - async fn pickle(&self, pickle_mode: PicklingMode) -> PickledCrossSigningIdentity { - let pickle_key = if let PicklingMode::Encrypted { key } = &pickle_mode { - key - } else { - "DEFAULT_PICKLE_PASSPHRASE_123456".as_bytes() - } - .to_vec(); - + async fn pickle(&self, pickle_key: &[u8]) -> PickledCrossSigningIdentity { let master_key = if let Some(m) = self.master_key.lock().await.as_ref() { - Some( - m.pickle(PicklingMode::Encrypted { - key: pickle_key.clone(), - }) - .await, - ) + Some(m.pickle(pickle_key).await) } else { None }; let self_signing_key = if let Some(m) = self.self_signing_key.lock().await.as_ref() { - Some( - m.pickle(PicklingMode::Encrypted { - key: pickle_key.clone(), - }) - .await, - ) + Some(m.pickle(pickle_key).await) } else { None }; let user_signing_key = if let Some(m) = self.user_signing_key.lock().await.as_ref() { - Some( - m.pickle(PicklingMode::Encrypted { - key: pickle_key.clone(), - }) - .await, - ) + Some(m.pickle(pickle_key).await) } else { None }; @@ -425,41 +392,21 @@ impl PrivateCrossSigningIdentity { } } - async fn from_pickle(pickle: PickledCrossSigningIdentity, pickle_mode: PicklingMode) -> Self { - let pickle_key = if let PicklingMode::Encrypted { key } = &pickle_mode { - key - } else { - "DEFAULT_PICKLE_PASSPHRASE_123456".as_bytes() - } - .to_vec(); - + async fn from_pickle(pickle: PickledCrossSigningIdentity, pickle_key: &[u8]) -> Self { let master = if let Some(m) = pickle.master_key { - Some(MasterSigning::from_pickle( - m, - PicklingMode::Encrypted { - key: pickle_key.clone(), - }, - )) + Some(MasterSigning::from_pickle(m, pickle_key)) } else { None }; let self_signing = if let Some(s) = pickle.self_signing_key { - Some(SelfSigning::from_pickle( - s, - PicklingMode::Encrypted { - key: pickle_key.clone(), - }, - )) + Some(SelfSigning::from_pickle(s, pickle_key)) } else { None }; let user_signing = if let Some(u) = pickle.user_signing_key { - Some(UserSigning::from_pickle( - u, - PicklingMode::Encrypted { key: pickle_key }, - )) + Some(UserSigning::from_pickle(u, pickle_key)) } else { None }; @@ -474,29 +421,35 @@ impl PrivateCrossSigningIdentity { } async fn as_upload_request(&self) -> UploadRequest<'_> { + let master_key = self + .master_key + .lock() + .await + .as_ref() + .cloned() + .map(|k| k.public_key.into()); + + let user_signing_key = self + .user_signing_key + .lock() + .await + .as_ref() + .cloned() + .map(|k| k.public_key.into()); + + let self_signing_key = self + .self_signing_key + .lock() + .await + .as_ref() + .cloned() + .map(|k| k.public_key.into()); + UploadRequest { auth: None, - master_key: self - .master_key - .lock() - .await - .as_ref() - .cloned() - .map(|k| k.public_key.into()), - user_signing_key: self - .user_signing_key - .lock() - .await - .as_ref() - .cloned() - .map(|k| k.public_key.into()), - self_signing_key: self - .self_signing_key - .lock() - .await - .as_ref() - .cloned() - .map(|k| k.public_key.into()), + master_key, + user_signing_key, + self_signing_key, } } } @@ -507,12 +460,15 @@ mod test { use matrix_sdk_common::identifiers::{user_id, UserId}; use matrix_sdk_test::async_test; - use olm_rs::PicklingMode; fn user_id() -> UserId { user_id!("@example:localhost") } + fn pickle_key() -> &'static [u8] { + &[0u8; 32] + } + #[test] fn signing_creation() { let signing = Signing::new(); @@ -532,9 +488,9 @@ mod test { #[async_test] async fn pickling_signing() { let signing = Signing::new(); - let pickled = signing.pickle(PicklingMode::Unencrypted).await; + let pickled = signing.pickle(pickle_key()).await; - let unpickled = Signing::from_pickle(pickled, PicklingMode::Unencrypted); + let unpickled = Signing::from_pickle(pickled, pickle_key()); assert_eq!(signing.public_key(), unpickled.public_key()); } @@ -584,10 +540,9 @@ mod test { async fn identity_pickling() { let identity = PrivateCrossSigningIdentity::new(user_id()).await; - let pickled = identity.pickle(PicklingMode::Unencrypted).await; + let pickled = identity.pickle(pickle_key()).await; - let unpickled = - PrivateCrossSigningIdentity::from_pickle(pickled, PicklingMode::Unencrypted).await; + let unpickled = PrivateCrossSigningIdentity::from_pickle(pickled, pickle_key()).await; assert_eq!(identity.user_id, unpickled.user_id); assert_eq!( From d1313b8614d29442852cff0344fe1910e7eb9572 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 14 Oct 2020 16:15:26 +0200 Subject: [PATCH 06/34] crypto: Fix another clippy warning. --- matrix_sdk_crypto/src/olm/signing.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/matrix_sdk_crypto/src/olm/signing.rs b/matrix_sdk_crypto/src/olm/signing.rs index 95dc5e65..0f47a81f 100644 --- a/matrix_sdk_crypto/src/olm/signing.rs +++ b/matrix_sdk_crypto/src/olm/signing.rs @@ -499,12 +499,10 @@ mod test { async fn private_identity_creation() { let identity = PrivateCrossSigningIdentity::new(user_id()).await; - assert!(identity - .master_key - .lock() - .await - .as_ref() - .unwrap() + let master_key = identity.master_key.lock().await; + let master_key = master_key.as_ref().unwrap(); + + assert!(master_key .public_key .verify_subkey( &identity @@ -517,12 +515,7 @@ mod test { ) .is_ok()); - assert!(identity - .master_key - .lock() - .await - .as_ref() - .unwrap() + assert!(master_key .public_key .verify_subkey( &identity From 59a719920226d1304a7cdcc2a240f295208dda41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 15 Oct 2020 13:58:35 +0200 Subject: [PATCH 07/34] crypto: Initial test for the session manager. --- matrix_sdk_crypto/src/olm/account.rs | 17 ++-- matrix_sdk_crypto/src/session_manager.rs | 110 +++++++++++++++++++++++ 2 files changed, 121 insertions(+), 6 deletions(-) diff --git a/matrix_sdk_crypto/src/olm/account.rs b/matrix_sdk_crypto/src/olm/account.rs index a6522ba1..0960038b 100644 --- a/matrix_sdk_crypto/src/olm/account.rs +++ b/matrix_sdk_crypto/src/olm/account.rs @@ -683,14 +683,9 @@ impl ReadOnlyAccount { self.sign(&canonical_json).await } - /// Generate, sign and prepare one-time keys to be uploaded. - /// - /// If no one-time keys need to be uploaded returns an empty error. - pub(crate) async fn signed_one_time_keys( + pub(crate) async fn signed_one_time_keys_helper( &self, ) -> Result, ()> { - let _ = self.generate_one_time_keys().await?; - let one_time_keys = self.one_time_keys().await; let mut one_time_key_map = BTreeMap::new(); @@ -728,6 +723,16 @@ impl ReadOnlyAccount { Ok(one_time_key_map) } + /// Generate, sign and prepare one-time keys to be uploaded. + /// + /// If no one-time keys need to be uploaded returns an empty error. + pub(crate) async fn signed_one_time_keys( + &self, + ) -> Result, ()> { + let _ = self.generate_one_time_keys().await?; + self.signed_one_time_keys_helper().await + } + /// Create a new session with another account given a one-time key. /// /// Returns the newly created session or a `OlmSessionError` if creating a diff --git a/matrix_sdk_crypto/src/session_manager.rs b/matrix_sdk_crypto/src/session_manager.rs index 792caa81..69a86722 100644 --- a/matrix_sdk_crypto/src/session_manager.rs +++ b/matrix_sdk_crypto/src/session_manager.rs @@ -303,3 +303,113 @@ impl SessionManager { Ok(()) } } + +#[cfg(test)] +mod test { + use dashmap::DashMap; + use std::{collections::BTreeMap, sync::Arc}; + + use matrix_sdk_common::{ + api::r0::keys::claim_keys::Response as KeyClaimResponse, + identifiers::{user_id, DeviceIdBox, UserId}, + }; + use matrix_sdk_test::async_test; + + use super::SessionManager; + use crate::{ + identities::ReadOnlyDevice, + key_request::KeyRequestMachine, + olm::{Account, ReadOnlyAccount}, + store::{CryptoStore, MemoryStore, Store}, + verification::VerificationMachine, + }; + + fn user_id() -> UserId { + user_id!("@example:localhost") + } + + fn device_id() -> DeviceIdBox { + "DEVICEID".into() + } + + fn bob_account() -> ReadOnlyAccount { + ReadOnlyAccount::new(&user_id!("@bob:localhost"), "BOBDEVICE".into()) + } + + async fn session_manager() -> SessionManager { + let user_id = user_id(); + let device_id = device_id(); + + let outbound_sessions = Arc::new(DashMap::new()); + let users_for_key_claim = Arc::new(DashMap::new()); + let account = ReadOnlyAccount::new(&user_id, &device_id); + let store: Arc> = Arc::new(Box::new(MemoryStore::new())); + store.save_account(account.clone()).await.unwrap(); + + let verification = VerificationMachine::new(account.clone(), store.clone()); + + let user_id = Arc::new(user_id); + let device_id = Arc::new(device_id); + + let store = Store::new(user_id.clone(), store, verification); + + let account = Account { + inner: account, + store: store.clone(), + }; + + let key_request = KeyRequestMachine::new( + user_id, + device_id, + store.clone(), + outbound_sessions, + users_for_key_claim.clone(), + ); + + SessionManager::new(account, users_for_key_claim, key_request, store) + } + + #[async_test] + async fn session_creation() { + let manager = session_manager().await; + let bob = bob_account(); + + let bob_device = ReadOnlyDevice::from_account(&bob).await; + + manager.store.save_devices(&[bob_device]).await.unwrap(); + + let (_, request) = manager + .get_missing_sessions(&mut [bob.user_id().clone()].iter()) + .await + .unwrap() + .unwrap(); + + assert!(request.one_time_keys.contains_key(bob.user_id())); + + bob.generate_one_time_keys_helper(1).await; + let one_time = bob.signed_one_time_keys_helper().await.unwrap(); + bob.mark_keys_as_published().await; + + let mut one_time_keys = BTreeMap::new(); + one_time_keys + .entry(bob.user_id().clone()) + .or_insert_with(BTreeMap::new) + .insert(bob.device_id().into(), one_time); + + let response = KeyClaimResponse { + failures: BTreeMap::new(), + one_time_keys, + }; + + manager + .receive_keys_claim_response(&response) + .await + .unwrap(); + + assert!(manager + .get_missing_sessions(&mut [bob.user_id().clone()].iter()) + .await + .unwrap() + .is_none()); + } +} From 59d7b532429c12c05107b0753085efd1acecd542 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 15 Oct 2020 15:02:02 +0200 Subject: [PATCH 08/34] crypto: Add an user for a key request if the device was marked as wedged. --- matrix_sdk_crypto/src/session_manager.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/matrix_sdk_crypto/src/session_manager.rs b/matrix_sdk_crypto/src/session_manager.rs index 69a86722..954f2be5 100644 --- a/matrix_sdk_crypto/src/session_manager.rs +++ b/matrix_sdk_crypto/src/session_manager.rs @@ -34,7 +34,7 @@ use crate::{ olm::Account, requests::{OutgoingRequest, ToDeviceRequest}, store::{Result as StoreResult, Store}, - Device, + ReadOnlyDevice, }; #[derive(Debug, Clone)] @@ -92,6 +92,10 @@ impl SessionManager { if let Some(session) = session { if session.creation_time.elapsed() > Self::UNWEDGING_INTERVAL { + self.users_for_key_claim + .entry(device.user_id().clone()) + .or_insert_with(DashSet::new) + .insert(device.device_id().into()); self.wedged_devices .entry(device.user_id().to_owned()) .or_insert_with(DashSet::new) @@ -105,7 +109,7 @@ impl SessionManager { } #[allow(dead_code)] - pub fn is_device_wedged(&self, device: &Device) -> bool { + pub fn is_device_wedged(&self, device: &ReadOnlyDevice) -> bool { self.wedged_devices .get(device.user_id()) .map(|d| d.contains(device.device_id())) From b5c947342484ff180ca4d175c894e5f692feebbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 15 Oct 2020 15:03:22 +0200 Subject: [PATCH 09/34] crypto: Test the session unwedging logic. --- matrix_sdk_crypto/src/session_manager.rs | 78 +++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/matrix_sdk_crypto/src/session_manager.rs b/matrix_sdk_crypto/src/session_manager.rs index 954f2be5..ab3976ab 100644 --- a/matrix_sdk_crypto/src/session_manager.rs +++ b/matrix_sdk_crypto/src/session_manager.rs @@ -315,7 +315,8 @@ mod test { use matrix_sdk_common::{ api::r0::keys::claim_keys::Response as KeyClaimResponse, - identifiers::{user_id, DeviceIdBox, UserId}, + identifiers::{user_id, DeviceIdBox, DeviceKeyAlgorithm, UserId}, + instant::{Duration, Instant}, }; use matrix_sdk_test::async_test; @@ -416,4 +417,79 @@ mod test { .unwrap() .is_none()); } + + // This test doesn't run on macos because we're modifying the session + // creation time so we can get around the UNWEDGING_INTERVAL. + #[async_test] + #[cfg(not(target_os = "macos"))] + async fn session_unwedging() { + let manager = session_manager().await; + let bob = bob_account(); + let (_, mut session) = bob.create_session_for(&manager.account).await; + + let bob_device = ReadOnlyDevice::from_account(&bob).await; + session.creation_time = Arc::new(Instant::now() - Duration::from_secs(3601)); + + manager + .store + .save_devices(&[bob_device.clone()]) + .await + .unwrap(); + manager.store.save_sessions(&[session]).await.unwrap(); + + assert!(manager + .get_missing_sessions(&mut [bob.user_id().clone()].iter()) + .await + .unwrap() + .is_none()); + + let curve_key = bob_device.get_key(DeviceKeyAlgorithm::Curve25519).unwrap(); + + assert!(!manager.users_for_key_claim.contains_key(bob.user_id())); + assert!(!manager.is_device_wedged(&bob_device)); + manager + .mark_device_as_wedged(bob_device.user_id(), &curve_key) + .await + .unwrap(); + assert!(manager.is_device_wedged(&bob_device)); + assert!(manager.users_for_key_claim.contains_key(bob.user_id())); + + let (_, request) = manager + .get_missing_sessions(&mut [bob.user_id().clone()].iter()) + .await + .unwrap() + .unwrap(); + + assert!(request.one_time_keys.contains_key(bob.user_id())); + + bob.generate_one_time_keys_helper(1).await; + let one_time = bob.signed_one_time_keys_helper().await.unwrap(); + bob.mark_keys_as_published().await; + + let mut one_time_keys = BTreeMap::new(); + one_time_keys + .entry(bob.user_id().clone()) + .or_insert_with(BTreeMap::new) + .insert(bob.device_id().into(), one_time); + + let response = KeyClaimResponse { + failures: BTreeMap::new(), + one_time_keys, + }; + + assert!(manager.outgoing_to_device_requests.is_empty()); + + manager + .receive_keys_claim_response(&response) + .await + .unwrap(); + + assert!(!manager.is_device_wedged(&bob_device)); + assert!(manager + .get_missing_sessions(&mut [bob.user_id().clone()].iter()) + .await + .unwrap() + .is_none()); + assert!(!manager.outgoing_to_device_requests.is_empty()) + } } From e7a24d5e68490f66dede85ce3d07de43729cc410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 16 Oct 2020 11:09:55 +0200 Subject: [PATCH 10/34] crypto: Move the session managers under a common module. --- matrix_sdk_crypto/src/identities/manager.rs | 4 ++-- matrix_sdk_crypto/src/lib.rs | 1 - matrix_sdk_crypto/src/machine.rs | 3 +-- .../group_sessions.rs} | 0 matrix_sdk_crypto/src/session_manager/mod.rs | 19 +++++++++++++++++++ .../sessions.rs} | 0 6 files changed, 22 insertions(+), 5 deletions(-) rename matrix_sdk_crypto/src/{group_manager.rs => session_manager/group_sessions.rs} (100%) create mode 100644 matrix_sdk_crypto/src/session_manager/mod.rs rename matrix_sdk_crypto/src/{session_manager.rs => session_manager/sessions.rs} (100%) diff --git a/matrix_sdk_crypto/src/identities/manager.rs b/matrix_sdk_crypto/src/identities/manager.rs index d80c8c2e..ef9d60db 100644 --- a/matrix_sdk_crypto/src/identities/manager.rs +++ b/matrix_sdk_crypto/src/identities/manager.rs @@ -27,12 +27,12 @@ use matrix_sdk_common::{ use crate::{ error::OlmResult, - group_manager::GroupSessionManager, identities::{ MasterPubkey, OwnUserIdentity, ReadOnlyDevice, SelfSigningPubkey, UserIdentities, UserIdentity, UserSigningPubkey, }, requests::KeysQueryRequest, + session_manager::GroupSessionManager, store::{Result as StoreResult, Store}, }; @@ -376,10 +376,10 @@ pub(crate) mod test { use serde_json::json; use crate::{ - group_manager::GroupSessionManager, identities::IdentityManager, machine::test::response_from_file, olm::{Account, ReadOnlyAccount}, + session_manager::GroupSessionManager, store::{CryptoStore, MemoryStore, Store}, verification::VerificationMachine, }; diff --git a/matrix_sdk_crypto/src/lib.rs b/matrix_sdk_crypto/src/lib.rs index 6f168126..0a1f0300 100644 --- a/matrix_sdk_crypto/src/lib.rs +++ b/matrix_sdk_crypto/src/lib.rs @@ -29,7 +29,6 @@ mod error; mod file_encryption; -mod group_manager; mod identities; mod key_request; mod machine; diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 3e64e48a..03127c25 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -46,7 +46,6 @@ use matrix_sdk_common::{ use crate::store::sqlite::SqliteStore; use crate::{ error::{EventError, MegolmError, MegolmResult, OlmError, OlmResult}, - group_manager::GroupSessionManager, identities::{Device, IdentityManager, ReadOnlyDevice, UserDevices, UserIdentities}, key_request::KeyRequestMachine, olm::{ @@ -54,7 +53,7 @@ use crate::{ InboundGroupSession, ReadOnlyAccount, }, requests::{IncomingResponse, OutgoingRequest}, - session_manager::SessionManager, + session_manager::{GroupSessionManager, SessionManager}, store::{CryptoStore, MemoryStore, Result as StoreResult, Store}, verification::{Sas, VerificationMachine}, ToDeviceRequest, diff --git a/matrix_sdk_crypto/src/group_manager.rs b/matrix_sdk_crypto/src/session_manager/group_sessions.rs similarity index 100% rename from matrix_sdk_crypto/src/group_manager.rs rename to matrix_sdk_crypto/src/session_manager/group_sessions.rs diff --git a/matrix_sdk_crypto/src/session_manager/mod.rs b/matrix_sdk_crypto/src/session_manager/mod.rs new file mode 100644 index 00000000..7750262e --- /dev/null +++ b/matrix_sdk_crypto/src/session_manager/mod.rs @@ -0,0 +1,19 @@ +// Copyright 2020 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. + +mod group_sessions; +mod sessions; + +pub(crate) use group_sessions::GroupSessionManager; +pub(crate) use sessions::SessionManager; diff --git a/matrix_sdk_crypto/src/session_manager.rs b/matrix_sdk_crypto/src/session_manager/sessions.rs similarity index 100% rename from matrix_sdk_crypto/src/session_manager.rs rename to matrix_sdk_crypto/src/session_manager/sessions.rs From fc54c63a4c684afd2529a8e67e4673b193e7c956 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 16 Oct 2020 15:05:53 +0200 Subject: [PATCH 11/34] crypto: Upgrade sqlx to the beta release. This change is much needed to enable transactions in our sqlite store, before this release creating a transaction would take ownership of the connection, now it just mutably borrows it. --- matrix_sdk_crypto/Cargo.toml | 2 +- matrix_sdk_crypto/src/machine.rs | 2 +- matrix_sdk_crypto/src/store/mod.rs | 5 -- matrix_sdk_crypto/src/store/sqlite.rs | 71 ++++++++++++++------------- 4 files changed, 40 insertions(+), 40 deletions(-) diff --git a/matrix_sdk_crypto/Cargo.toml b/matrix_sdk_crypto/Cargo.toml index c437da65..cb882341 100644 --- a/matrix_sdk_crypto/Cargo.toml +++ b/matrix_sdk_crypto/Cargo.toml @@ -52,7 +52,7 @@ default-features = false features = ["std", "std-future"] [target.'cfg(not(target_arch = "wasm32"))'.dependencies.sqlx] -version = "0.3.5" +version = "0.4.0-beta.1" optional = true default-features = false features = ["runtime-tokio", "sqlite", "macros"] diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 03127c25..a2b270d9 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -1576,7 +1576,7 @@ pub(crate) mod test { } } - #[tokio::test] + #[tokio::test(threaded_scheduler)] #[cfg(feature = "sqlite_cryptostore")] async fn test_machine_with_default_store() { let tmpdir = tempdir().unwrap(); diff --git a/matrix_sdk_crypto/src/store/mod.rs b/matrix_sdk_crypto/src/store/mod.rs index 767bbae4..7dad4d07 100644 --- a/matrix_sdk_crypto/src/store/mod.rs +++ b/matrix_sdk_crypto/src/store/mod.rs @@ -55,7 +55,6 @@ use olm_rs::errors::{OlmAccountError, OlmGroupSessionError, OlmSessionError}; use serde::{Deserialize, Serialize}; use serde_json::Error as SerdeError; use thiserror::Error; -use url::ParseError; #[cfg_attr(feature = "docs", doc(cfg(r#sqlite_cryptostore)))] #[cfg(not(target_arch = "wasm32"))] @@ -245,10 +244,6 @@ pub enum CryptoStoreError { /// The store failed to (de)serialize a data type. #[error(transparent)] Serialization(#[from] SerdeError), - - /// An error occurred while parsing an URL. - #[error(transparent)] - UrlParse(#[from] ParseError), } /// Trait abstracting a store that the `OlmMachine` uses to store cryptographic diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index fe67e2d6..e469948c 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -30,8 +30,7 @@ use matrix_sdk_common::{ instant::Duration, locks::Mutex, }; -use sqlx::{query, query_as, sqlite::SqliteQueryAs, Connect, Executor, SqliteConnection}; -use url::Url; +use sqlx::{query, query_as, sqlite::SqliteConnectOptions, Connection, Executor, SqliteConnection}; use zeroize::Zeroizing; use super::{ @@ -131,20 +130,20 @@ impl SqliteStore { .await } - fn path_to_url(path: &Path) -> Result { - // TODO this returns an empty error if the path isn't absolute. - let url = Url::from_directory_path(path).expect("Invalid path"); - Ok(url.join(DATABASE_NAME)?) - } - async fn open_helper>( user_id: &UserId, device_id: &DeviceId, path: P, passphrase: Option>, ) -> Result { - let url = SqliteStore::path_to_url(path.as_ref())?; - let connection = SqliteConnection::connect(url.as_ref()).await?; + let path = path.as_ref().join(DATABASE_NAME); + let options = SqliteConnectOptions::new() + .foreign_keys(true) + .create_if_missing(true) + .read_only(false) + .filename(&path); + + let connection = SqliteConnection::connect_with(&options).await?; let store = SqliteStore { user_id: Arc::new(user_id.to_owned()), @@ -153,7 +152,7 @@ impl SqliteStore { sessions: SessionStore::new(), inbound_group_sessions: GroupSessionStore::new(), devices: DeviceStore::new(), - path: Arc::new(path.as_ref().to_owned()), + path: Arc::new(path), connection: Arc::new(Mutex::new(connection)), pickle_passphrase: Arc::new(passphrase), tracked_users: Arc::new(DashSet::new()), @@ -1249,9 +1248,14 @@ impl CryptoStore for SqliteStore { async fn save_sessions(&self, sessions: &[Session]) -> Result<()> { let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; - // TODO turn this into a transaction for session in sessions { self.lazy_load_sessions(&session.sender_key).await?; + } + + let mut connection = self.connection.lock().await; + let mut transaction = connection.begin().await?; + + for session in sessions { self.sessions.add(session.clone()).await; let pickle = session.pickle(self.get_pickle_mode()).await; @@ -1260,8 +1264,6 @@ impl CryptoStore for SqliteStore { let creation_time = serde_json::to_string(&pickle.creation_time)?; let last_use_time = serde_json::to_string(&pickle.last_use_time)?; - let mut connection = self.connection.lock().await; - query( "REPLACE INTO sessions ( session_id, account_id, creation_time, last_use_time, sender_key, pickle @@ -1273,10 +1275,12 @@ impl CryptoStore for SqliteStore { .bind(&*last_use_time) .bind(&pickle.sender_key) .bind(&pickle.pickle.as_str()) - .execute(&mut *connection) + .execute(&mut *transaction) .await?; } + transaction.commit().await?; + Ok(()) } @@ -1287,15 +1291,16 @@ impl CryptoStore for SqliteStore { async fn save_inbound_group_sessions(&self, sessions: &[InboundGroupSession]) -> Result<()> { let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; let mut connection = self.connection.lock().await; - - // FIXME use a transaction here once sqlx gets better support for them. + let mut transaction = connection.begin().await?; for session in sessions { - self.save_inbound_group_session_helper(account_id, &mut connection, session) + self.save_inbound_group_session_helper(account_id, &mut transaction, session) .await?; self.inbound_group_sessions.add(session.clone()); } + transaction.commit().await?; + Ok(()) } @@ -1549,7 +1554,7 @@ mod test { (alice, session) } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn create_store() { let tmpdir = tempdir().unwrap(); let tmpdir_path = tmpdir.path().to_str().unwrap(); @@ -1558,7 +1563,7 @@ mod test { .expect("Can't create store"); } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn save_account() { let (store, _dir) = get_store(None).await; assert!(store.load_account().await.unwrap().is_none()); @@ -1570,7 +1575,7 @@ mod test { .expect("Can't save account"); } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn load_account() { let (store, _dir) = get_store(None).await; let account = get_account(); @@ -1586,7 +1591,7 @@ mod test { assert_eq!(account, loaded_account); } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn load_account_with_passphrase() { let (store, _dir) = get_store(Some("secret_passphrase")).await; let account = get_account(); @@ -1602,7 +1607,7 @@ mod test { assert_eq!(account, loaded_account); } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn save_and_share_account() { let (store, _dir) = get_store(None).await; let account = get_account(); @@ -1630,7 +1635,7 @@ mod test { ); } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn save_session() { let (store, _dir) = get_store(None).await; let (account, session) = get_account_and_session().await; @@ -1645,7 +1650,7 @@ mod test { store.save_sessions(&[session]).await.unwrap(); } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn load_sessions() { let (store, _dir) = get_store(None).await; let (account, session) = get_account_and_session().await; @@ -1664,7 +1669,7 @@ mod test { assert_eq!(&session, loaded_session); } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn add_and_save_session() { let (store, dir) = get_store(None).await; let (account, session) = get_account_and_session().await; @@ -1699,7 +1704,7 @@ mod test { assert_eq!(session_id, session.session_id()); } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn save_inbound_group_session() { let (account, store, _dir) = get_loaded_store().await; @@ -1719,7 +1724,7 @@ mod test { .expect("Can't save group session"); } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn load_inbound_group_session() { let (account, store, dir) = get_loaded_store().await; @@ -1761,7 +1766,7 @@ mod test { assert!(!export.forwarding_curve25519_key_chain.is_empty()) } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn test_tracked_users() { let (_account, store, dir) = get_loaded_store().await; let device = get_device(); @@ -1808,7 +1813,7 @@ mod test { assert!(!store.users_for_key_query().contains(device.user_id())); } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn device_saving() { let (_account, store, dir) = get_loaded_store().await; let device = get_device(); @@ -1842,7 +1847,7 @@ mod test { assert_eq!(user_devices.devices().next().unwrap(), &device); } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn device_deleting() { let (_account, store, dir) = get_loaded_store().await; let device = get_device(); @@ -1864,7 +1869,7 @@ mod test { assert!(loaded_device.is_none()); } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn user_saving() { let dir = tempdir().unwrap(); let tmpdir_path = dir.path().to_str().unwrap(); @@ -1941,7 +1946,7 @@ mod test { assert!(loaded_user.own().unwrap().is_verified()) } - #[tokio::test] + #[tokio::test(threaded_scheduler)] async fn key_value_saving() { let (_, store, _dir) = get_loaded_store().await; let key = "test_key".to_string(); From b5560d3cb678ae2f5ea566d347aff4b1f4b16965 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 16 Oct 2020 15:23:34 +0200 Subject: [PATCH 12/34] crypto: More transactions in the sqlite store. --- matrix_sdk_crypto/src/store/sqlite.rs | 32 +++++++++++++++++++-------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index e469948c..e3aaa303 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -754,11 +754,13 @@ impl SqliteStore { Ok(()) } - async fn save_device_helper(&self, device: ReadOnlyDevice) -> Result<()> { + async fn save_device_helper( + &self, + connection: &mut SqliteConnection, + device: ReadOnlyDevice, + ) -> Result<()> { let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; - let mut connection = self.connection.lock().await; - query( "INSERT INTO devices ( account_id, user_id, device_id, @@ -1107,11 +1109,13 @@ impl SqliteStore { Ok(()) } - async fn save_user_helper(&self, user: &UserIdentities) -> Result<()> { + async fn save_user_helper( + &self, + mut connection: &mut SqliteConnection, + user: &UserIdentities, + ) -> Result<()> { let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; - let mut connection = self.connection.lock().await; - query("REPLACE INTO users (account_id, user_id) VALUES (?1, ?2)") .bind(account_id) .bind(user.user_id().as_str()) @@ -1347,12 +1351,17 @@ impl CryptoStore for SqliteStore { } async fn save_devices(&self, devices: &[ReadOnlyDevice]) -> Result<()> { - // TODO turn this into a bulk transaction. + let mut connection = self.connection.lock().await; + let mut transaction = connection.begin().await?; + for device in devices { self.devices.add(device.clone()); - self.save_device_helper(device.clone()).await? + self.save_device_helper(&mut transaction, device.clone()) + .await? } + transaction.commit().await?; + Ok(()) } @@ -1391,10 +1400,15 @@ impl CryptoStore for SqliteStore { } async fn save_user_identities(&self, users: &[UserIdentities]) -> Result<()> { + let mut connection = self.connection.lock().await; + let mut transaction = connection.begin().await?; + for user in users { - self.save_user_helper(user).await?; + self.save_user_helper(&mut transaction, user).await?; } + transaction.commit().await?; + Ok(()) } From 4262f1d3b0e7682f51591166725e4ec47ba4e78f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 16 Oct 2020 15:54:50 +0200 Subject: [PATCH 13/34] crypto: Don't cache inbound group sessions in the sqlite store. --- matrix_sdk_crypto/src/store/sqlite.rs | 180 ++++++++++++++++++-------- 1 file changed, 124 insertions(+), 56 deletions(-) diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index e3aaa303..6bc710dc 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -34,7 +34,7 @@ use sqlx::{query, query_as, sqlite::SqliteConnectOptions, Connection, Executor, use zeroize::Zeroizing; use super::{ - caches::{DeviceStore, GroupSessionStore, ReadOnlyUserDevices, SessionStore}, + caches::{DeviceStore, ReadOnlyUserDevices, SessionStore}, CryptoStore, CryptoStoreError, Result, }; use crate::{ @@ -56,7 +56,6 @@ pub struct SqliteStore { path: Arc, sessions: SessionStore, - inbound_group_sessions: GroupSessionStore, devices: DeviceStore, tracked_users: Arc>, users_for_key_query: Arc>, @@ -150,7 +149,6 @@ impl SqliteStore { device_id: Arc::new(device_id.into()), account_info: Arc::new(SyncMutex::new(None)), sessions: SessionStore::new(), - inbound_group_sessions: GroupSessionStore::new(), devices: DeviceStore::new(), path: Arc::new(path), connection: Arc::new(Mutex::new(connection)), @@ -525,7 +523,113 @@ impl SqliteStore { .collect::>>()?) } - async fn load_inbound_group_sessions(&self) -> Result<()> { + async fn load_inbound_session_data( + &self, + connection: &mut SqliteConnection, + session_row_id: i64, + pickle: String, + sender_key: String, + room_id: RoomId, + imported: bool, + ) -> Result { + let key_rows: Vec<(String, String)> = + query_as("SELECT algorithm, key FROM group_session_claimed_keys WHERE session_id = ?") + .bind(session_row_id) + .fetch_all(&mut *connection) + .await?; + + let claimed_keys: BTreeMap = key_rows + .into_iter() + .filter_map(|row| { + let algorithm = row.0.parse::().ok()?; + let key = row.1; + + Some((algorithm, key)) + }) + .collect(); + + let mut chain_rows: Vec<(String,)> = + query_as("SELECT key, key FROM group_session_chains WHERE session_id = ?") + .bind(session_row_id) + .fetch_all(&mut *connection) + .await?; + + let chains: Vec = chain_rows.drain(..).map(|r| r.0).collect(); + + let chains = if chains.is_empty() { + None + } else { + Some(chains) + }; + + let pickle = PickledInboundGroupSession { + pickle: InboundGroupSessionPickle::from(pickle), + sender_key, + signing_key: claimed_keys, + room_id, + forwarding_chains: chains, + imported, + }; + + Ok(InboundGroupSession::from_pickle( + pickle, + self.get_pickle_mode(), + )?) + } + + async fn load_inbound_group_session_helper( + &self, + room_id: &RoomId, + sender_key: &str, + session_id: &str, + ) -> Result> { + let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; + let mut connection = self.connection.lock().await; + + let row: Option<(i64, String, bool)> = query_as( + "SELECT id, pickle, imported + FROM inbound_group_sessions + WHERE ( + account_id = ? and + room_id = ? and + sender_key = ? and + session_id = ? + )", + ) + .bind(account_id) + .bind(room_id.as_str()) + .bind(sender_key) + .bind(session_id) + .fetch_optional(&mut *connection) + .await?; + + let row = if let Some(r) = row { + r + } else { + return Ok(None); + }; + + let session_row_id = row.0; + let pickle = row.1; + let imported = row.2; + + let session = self + .load_inbound_session_data( + &mut connection, + session_row_id, + pickle, + sender_key.to_owned(), + room_id.to_owned(), + imported, + ) + .await?; + + Ok(Some(session)) + } + + async fn load_inbound_group_sessions(&self) -> Result> { + let mut sessions = Vec::new(); + let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; let mut connection = self.connection.lock().await; @@ -541,57 +645,24 @@ impl SqliteStore { let session_row_id = row.0; let pickle = row.1; let sender_key = row.2; - let room_id = row.3; + let room_id = RoomId::try_from(row.3)?; let imported = row.4; - let key_rows: Vec<(String, String)> = query_as( - "SELECT algorithm, key FROM group_session_claimed_keys WHERE session_id = ?", - ) - .bind(session_row_id) - .fetch_all(&mut *connection) - .await?; - - let claimed_keys: BTreeMap = key_rows - .into_iter() - .filter_map(|row| { - let algorithm = row.0.parse::().ok()?; - let key = row.1; - - Some((algorithm, key)) - }) - .collect(); - - let mut chain_rows: Vec<(String,)> = - query_as("SELECT key, key FROM group_session_chains WHERE session_id = ?") - .bind(session_row_id) - .fetch_all(&mut *connection) - .await?; - - let chains: Vec = chain_rows.drain(..).map(|r| r.0).collect(); - - let chains = if chains.is_empty() { - None - } else { - Some(chains) - }; - - let pickle = PickledInboundGroupSession { - pickle: InboundGroupSessionPickle::from(pickle), - sender_key, - signing_key: claimed_keys, - room_id: RoomId::try_from(room_id)?, - forwarding_chains: chains, - imported, - }; - - self.inbound_group_sessions - .add(InboundGroupSession::from_pickle( + let session = self + .load_inbound_session_data( + &mut connection, + session_row_id, pickle, - self.get_pickle_mode(), - )?); + sender_key, + room_id.to_owned(), + imported, + ) + .await?; + + sessions.push(session); } - Ok(()) + Ok(sessions) } async fn save_tracked_user(&self, user: &UserId, dirty: bool) -> Result<()> { @@ -1205,7 +1276,6 @@ impl CryptoStore for SqliteStore { drop(connection); - self.load_inbound_group_sessions().await?; self.load_devices().await?; self.load_tracked_users().await?; @@ -1300,7 +1370,6 @@ impl CryptoStore for SqliteStore { for session in sessions { self.save_inbound_group_session_helper(account_id, &mut transaction, session) .await?; - self.inbound_group_sessions.add(session.clone()); } transaction.commit().await?; @@ -1315,12 +1384,12 @@ impl CryptoStore for SqliteStore { session_id: &str, ) -> Result> { Ok(self - .inbound_group_sessions - .get(room_id, sender_key, session_id)) + .load_inbound_group_session_helper(room_id, sender_key, session_id) + .await?) } async fn get_inbound_group_sessions(&self) -> Result> { - Ok(self.inbound_group_sessions.get_all()) + Ok(self.load_inbound_group_sessions().await?) } fn is_user_tracked(&self, user_id: &UserId) -> bool { @@ -1768,7 +1837,6 @@ mod test { .expect("Can't create store"); store.load_account().await.unwrap(); - store.load_inbound_group_sessions().await.unwrap(); let loaded_session = store .get_inbound_group_session(&session.room_id, &session.sender_key, session.session_id()) From 425a07d6708c8933b06304ffe272cbb047cc9cde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 16 Oct 2020 16:53:10 +0200 Subject: [PATCH 14/34] crypto: Don't load all the devices in the sqlite store. --- matrix_sdk/src/device.rs | 5 +- matrix_sdk_crypto/src/identities/device.rs | 16 +- matrix_sdk_crypto/src/identities/manager.rs | 9 +- matrix_sdk_crypto/src/store/caches.rs | 49 +--- matrix_sdk_crypto/src/store/memorystore.rs | 20 +- matrix_sdk_crypto/src/store/mod.rs | 20 +- matrix_sdk_crypto/src/store/sqlite.rs | 266 ++++++++++++-------- 7 files changed, 216 insertions(+), 169 deletions(-) diff --git a/matrix_sdk/src/device.rs b/matrix_sdk/src/device.rs index 5c7b55fb..776b9f0e 100644 --- a/matrix_sdk/src/device.rs +++ b/matrix_sdk/src/device.rs @@ -19,7 +19,8 @@ use matrix_sdk_base::crypto::{ UserDevices as BaseUserDevices, }; use matrix_sdk_common::{ - api::r0::to_device::send_event_to_device::Request as ToDeviceRequest, identifiers::DeviceId, + api::r0::to_device::send_event_to_device::Request as ToDeviceRequest, + identifiers::{DeviceId, DeviceIdBox}, }; use crate::{error::Result, http_client::HttpClient, Sas}; @@ -114,7 +115,7 @@ impl UserDevices { } /// Iterator over all the device ids of the user devices. - pub fn keys(&self) -> impl Iterator { + pub fn keys(&self) -> impl Iterator { self.inner.keys() } diff --git a/matrix_sdk_crypto/src/identities/device.rs b/matrix_sdk_crypto/src/identities/device.rs index 578e7718..a0281217 100644 --- a/matrix_sdk_crypto/src/identities/device.rs +++ b/matrix_sdk_crypto/src/identities/device.rs @@ -13,7 +13,7 @@ // limitations under the License. use std::{ - collections::BTreeMap, + collections::{BTreeMap, HashMap}, convert::{TryFrom, TryInto}, ops::Deref, sync::{ @@ -30,7 +30,9 @@ use matrix_sdk_common::{ forwarded_room_key::ForwardedRoomKeyEventContent, room::encrypted::EncryptedEventContent, EventType, }, - identifiers::{DeviceId, DeviceKeyAlgorithm, DeviceKeyId, EventEncryptionAlgorithm, UserId}, + identifiers::{ + DeviceId, DeviceIdBox, DeviceKeyAlgorithm, DeviceKeyId, EventEncryptionAlgorithm, UserId, + }, locks::Mutex, }; use serde::{Deserialize, Serialize}; @@ -45,7 +47,7 @@ use crate::{ error::{EventError, OlmError, OlmResult, SignatureError}, identities::{OwnUserIdentity, UserIdentities}, olm::Utility, - store::{caches::ReadOnlyUserDevices, CryptoStore, Result as StoreResult}, + store::{CryptoStore, Result as StoreResult}, verification::VerificationMachine, Sas, ToDeviceRequest, }; @@ -168,7 +170,7 @@ impl Device { /// A read only view over all devices belonging to a user. #[derive(Debug)] pub struct UserDevices { - pub(crate) inner: ReadOnlyUserDevices, + pub(crate) inner: HashMap, pub(crate) verification_machine: VerificationMachine, pub(crate) own_identity: Option, pub(crate) device_owner_identity: Option, @@ -178,7 +180,7 @@ impl UserDevices { /// Get the specific device with the given device id. pub fn get(&self, device_id: &DeviceId) -> Option { self.inner.get(device_id).map(|d| Device { - inner: d, + inner: d.clone(), verification_machine: self.verification_machine.clone(), own_identity: self.own_identity.clone(), device_owner_identity: self.device_owner_identity.clone(), @@ -186,13 +188,13 @@ impl UserDevices { } /// Iterator over all the device ids of the user devices. - pub fn keys(&self) -> impl Iterator { + pub fn keys(&self) -> impl Iterator { self.inner.keys() } /// Iterator over all the devices of the user devices. pub fn devices(&self) -> impl Iterator + '_ { - self.inner.devices().map(move |d| Device { + self.inner.values().map(move |d| Device { inner: d.clone(), verification_machine: self.verification_machine.clone(), own_identity: self.own_identity.clone(), diff --git a/matrix_sdk_crypto/src/identities/manager.rs b/matrix_sdk_crypto/src/identities/manager.rs index ef9d60db..ba6fb749 100644 --- a/matrix_sdk_crypto/src/identities/manager.rs +++ b/matrix_sdk_crypto/src/identities/manager.rs @@ -165,18 +165,17 @@ impl IdentityManager { changed_devices.push(device); } - let current_devices: HashSet<&DeviceId> = - device_map.keys().map(|id| id.as_ref()).collect(); + let current_devices: HashSet<&DeviceIdBox> = device_map.keys().collect(); let stored_devices = self.store.get_readonly_devices(&user_id).await?; - let stored_devices_set: HashSet<&DeviceId> = stored_devices.keys().collect(); + let stored_devices_set: HashSet<&DeviceIdBox> = stored_devices.keys().collect(); let deleted_devices = stored_devices_set.difference(¤t_devices); for device_id in deleted_devices { users_with_new_or_deleted_devices.insert(user_id); - if let Some(device) = stored_devices.get(device_id) { + if let Some(device) = stored_devices.get(*device_id) { device.mark_as_deleted(); - self.store.delete_device(device).await?; + self.store.delete_device(device.clone()).await?; } } } diff --git a/matrix_sdk_crypto/src/store/caches.rs b/matrix_sdk_crypto/src/store/caches.rs index eb66198d..3b306d6b 100644 --- a/matrix_sdk_crypto/src/store/caches.rs +++ b/matrix_sdk_crypto/src/store/caches.rs @@ -19,9 +19,9 @@ use std::{collections::HashMap, sync::Arc}; -use dashmap::{DashMap, ReadOnlyView}; +use dashmap::DashMap; use matrix_sdk_common::{ - identifiers::{DeviceId, RoomId, UserId}, + identifiers::{DeviceId, DeviceIdBox, RoomId, UserId}, locks::Mutex, }; @@ -145,29 +145,6 @@ pub struct DeviceStore { entries: Arc, ReadOnlyDevice>>>, } -/// A read only view over all devices belonging to a user. -#[derive(Debug)] -pub struct ReadOnlyUserDevices { - entries: ReadOnlyView, ReadOnlyDevice>, -} - -impl ReadOnlyUserDevices { - /// Get the specific device with the given device id. - pub fn get(&self, device_id: &DeviceId) -> Option { - self.entries.get(device_id).cloned() - } - - /// Iterator over all the device ids of the user devices. - pub fn keys(&self) -> impl Iterator { - self.entries.keys().map(|id| id.as_ref()) - } - - /// Iterator over all the devices of the user devices. - pub fn devices(&self) -> impl Iterator { - self.entries.values() - } -} - impl DeviceStore { /// Create a new empty device store. pub fn new() -> Self { @@ -206,15 +183,13 @@ impl DeviceStore { } /// Get a read-only view over all devices of the given user. - pub fn user_devices(&self, user_id: &UserId) -> ReadOnlyUserDevices { - ReadOnlyUserDevices { - entries: self - .entries - .entry(user_id.clone()) - .or_insert_with(DashMap::new) - .clone() - .into_read_only(), - } + pub fn user_devices(&self, user_id: &UserId) -> HashMap { + self.entries + .entry(user_id.clone()) + .or_insert_with(DashMap::new) + .iter() + .map(|i| (i.key().to_owned(), i.value().clone())) + .collect() } } @@ -305,12 +280,12 @@ mod test { let user_devices = store.user_devices(device.user_id()); - assert_eq!(user_devices.keys().next().unwrap(), device.device_id()); - assert_eq!(user_devices.devices().next().unwrap(), &device); + assert_eq!(&**user_devices.keys().next().unwrap(), device.device_id()); + assert_eq!(user_devices.values().next().unwrap(), &device); let loaded_device = user_devices.get(device.device_id()).unwrap(); - assert_eq!(device, loaded_device); + assert_eq!(&device, loaded_device); store.remove(device.user_id(), device.device_id()); diff --git a/matrix_sdk_crypto/src/store/memorystore.rs b/matrix_sdk_crypto/src/store/memorystore.rs index d7f6a4da..2e5a8762 100644 --- a/matrix_sdk_crypto/src/store/memorystore.rs +++ b/matrix_sdk_crypto/src/store/memorystore.rs @@ -12,17 +12,20 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{collections::HashSet, sync::Arc}; +use std::{ + collections::{HashMap, HashSet}, + sync::Arc, +}; use dashmap::{DashMap, DashSet}; use matrix_sdk_common::{ - identifiers::{DeviceId, RoomId, UserId}, + identifiers::{DeviceId, DeviceIdBox, RoomId, UserId}, locks::Mutex, }; use matrix_sdk_common_macros::async_trait; use super::{ - caches::{DeviceStore, GroupSessionStore, ReadOnlyUserDevices, SessionStore}, + caches::{DeviceStore, GroupSessionStore, SessionStore}, CryptoStore, InboundGroupSession, ReadOnlyAccount, Result, Session, }; use crate::identities::{ReadOnlyDevice, UserIdentities}; @@ -153,7 +156,10 @@ impl CryptoStore for MemoryStore { Ok(()) } - async fn get_user_devices(&self, user_id: &UserId) -> Result { + async fn get_user_devices( + &self, + user_id: &UserId, + ) -> Result> { Ok(self.devices.user_devices(user_id)) } @@ -273,12 +279,12 @@ mod test { let user_devices = store.get_user_devices(device.user_id()).await.unwrap(); - assert_eq!(user_devices.keys().next().unwrap(), device.device_id()); - assert_eq!(user_devices.devices().next().unwrap(), &device); + assert_eq!(&**user_devices.keys().next().unwrap(), device.device_id()); + assert_eq!(user_devices.values().next().unwrap(), &device); let loaded_device = user_devices.get(device.device_id()).unwrap(); - assert_eq!(device, loaded_device); + assert_eq!(&device, loaded_device); store.delete_device(device.clone()).await.unwrap(); assert!(store diff --git a/matrix_sdk_crypto/src/store/mod.rs b/matrix_sdk_crypto/src/store/mod.rs index 7dad4d07..b8b661ef 100644 --- a/matrix_sdk_crypto/src/store/mod.rs +++ b/matrix_sdk_crypto/src/store/mod.rs @@ -43,13 +43,19 @@ mod memorystore; #[cfg(feature = "sqlite_cryptostore")] pub(crate) mod sqlite; -use caches::ReadOnlyUserDevices; +use matrix_sdk_common::identifiers::DeviceIdBox; pub use memorystore::MemoryStore; #[cfg(not(target_arch = "wasm32"))] #[cfg(feature = "sqlite_cryptostore")] pub use sqlite::SqliteStore; -use std::{collections::HashSet, fmt::Debug, io::Error as IoError, ops::Deref, sync::Arc}; +use std::{ + collections::{HashMap, HashSet}, + fmt::Debug, + io::Error as IoError, + ops::Deref, + sync::Arc, +}; use olm_rs::errors::{OlmAccountError, OlmGroupSessionError, OlmSessionError}; use serde::{Deserialize, Serialize}; @@ -115,7 +121,10 @@ impl Store { self.inner.get_device(user_id, device_id).await } - pub async fn get_readonly_devices(&self, user_id: &UserId) -> Result { + pub async fn get_readonly_devices( + &self, + user_id: &UserId, + ) -> Result> { self.inner.get_user_devices(user_id).await } @@ -354,7 +363,10 @@ pub trait CryptoStore: Debug { /// # Arguments /// /// * `user_id` - The user for which we should get all the devices. - async fn get_user_devices(&self, user_id: &UserId) -> Result; + async fn get_user_devices( + &self, + user_id: &UserId, + ) -> Result>; /// Save the given user identities in the store. /// diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index 6bc710dc..f591af79 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -13,7 +13,7 @@ // limitations under the License. use std::{ - collections::{BTreeMap, HashSet}, + collections::{BTreeMap, HashMap, HashSet}, convert::TryFrom, path::{Path, PathBuf}, result::Result as StdResult, @@ -25,7 +25,8 @@ use dashmap::DashSet; use matrix_sdk_common::{ api::r0::keys::{CrossSigningKey, KeyUsage}, identifiers::{ - DeviceId, DeviceKeyAlgorithm, DeviceKeyId, EventEncryptionAlgorithm, RoomId, UserId, + DeviceId, DeviceIdBox, DeviceKeyAlgorithm, DeviceKeyId, EventEncryptionAlgorithm, RoomId, + UserId, }, instant::Duration, locks::Mutex, @@ -33,10 +34,7 @@ use matrix_sdk_common::{ use sqlx::{query, query_as, sqlite::SqliteConnectOptions, Connection, Executor, SqliteConnection}; use zeroize::Zeroizing; -use super::{ - caches::{DeviceStore, ReadOnlyUserDevices, SessionStore}, - CryptoStore, CryptoStoreError, Result, -}; +use super::{caches::SessionStore, CryptoStore, CryptoStoreError, Result}; use crate::{ identities::{LocalTrust, OwnUserIdentity, ReadOnlyDevice, UserIdentities, UserIdentity}, olm::{ @@ -56,7 +54,6 @@ pub struct SqliteStore { path: Arc, sessions: SessionStore, - devices: DeviceStore, tracked_users: Arc>, users_for_key_query: Arc>, @@ -149,7 +146,6 @@ impl SqliteStore { device_id: Arc::new(device_id.into()), account_info: Arc::new(SyncMutex::new(None)), sessions: SessionStore::new(), - devices: DeviceStore::new(), path: Arc::new(path), connection: Arc::new(Mutex::new(connection)), pickle_passphrase: Arc::new(passphrase), @@ -717,112 +713,167 @@ impl SqliteStore { Ok(()) } - async fn load_devices(&self) -> Result<()> { - let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; - let mut connection = self.connection.lock().await; + async fn load_device_data( + &self, + connection: &mut SqliteConnection, + device_row_id: i64, + user_id: &UserId, + device_id: DeviceIdBox, + trust_state: LocalTrust, + display_name: Option, + ) -> Result { + let algorithm_rows: Vec<(String,)> = + query_as("SELECT algorithm FROM algorithms WHERE device_id = ?") + .bind(device_row_id) + .fetch_all(&mut *connection) + .await?; - let rows: Vec<(i64, String, String, Option, i64)> = query_as( - "SELECT id, user_id, device_id, display_name, trust_state - FROM devices WHERE account_id = ?", + let algorithms = algorithm_rows + .iter() + .map(|row| { + let algorithm: &str = &row.0; + EventEncryptionAlgorithm::from(algorithm) + }) + .collect::>(); + + let key_rows: Vec<(String, String)> = + query_as("SELECT algorithm, key FROM device_keys WHERE device_id = ?") + .bind(device_row_id) + .fetch_all(&mut *connection) + .await?; + + let keys: BTreeMap = key_rows + .into_iter() + .filter_map(|row| { + let algorithm = row.0.parse::().ok()?; + let key = row.1; + + Some((DeviceKeyId::from_parts(algorithm, &device_id), key)) + }) + .collect(); + + let signature_rows: Vec<(String, String, String)> = query_as( + "SELECT user_id, key_algorithm, signature + FROM device_signatures WHERE device_id = ?", ) - .bind(account_id) + .bind(device_row_id) .fetch_all(&mut *connection) .await?; - for row in rows { - let device_row_id = row.0; - let user_id: &str = &row.1; - let user_id = if let Ok(u) = UserId::try_from(user_id) { + let mut signatures: BTreeMap> = BTreeMap::new(); + + for row in signature_rows { + let user_id = if let Ok(u) = UserId::try_from(&*row.0) { u } else { continue; }; - let device_id = &row.2.to_string(); - let display_name = &row.3; - let trust_state = LocalTrust::from(row.4); + let key_algorithm = if let Ok(k) = row.1.parse::() { + k + } else { + continue; + }; - let algorithm_rows: Vec<(String,)> = - query_as("SELECT algorithm FROM algorithms WHERE device_id = ?") - .bind(device_row_id) - .fetch_all(&mut *connection) - .await?; + let signature = row.2; - let algorithms = algorithm_rows - .iter() - .map(|row| { - let algorithm: &str = &row.0; - EventEncryptionAlgorithm::from(algorithm) - }) - .collect::>(); - - let key_rows: Vec<(String, String)> = - query_as("SELECT algorithm, key FROM device_keys WHERE device_id = ?") - .bind(device_row_id) - .fetch_all(&mut *connection) - .await?; - - let keys: BTreeMap = key_rows - .into_iter() - .filter_map(|row| { - let algorithm = row.0.parse::().ok()?; - let key = row.1; - - Some(( - DeviceKeyId::from_parts(algorithm, device_id.as_str().into()), - key, - )) - }) - .collect(); - - let signature_rows: Vec<(String, String, String)> = query_as( - "SELECT user_id, key_algorithm, signature - FROM device_signatures WHERE device_id = ?", - ) - .bind(device_row_id) - .fetch_all(&mut *connection) - .await?; - - let mut signatures: BTreeMap> = BTreeMap::new(); - - for row in signature_rows { - let user_id = if let Ok(u) = UserId::try_from(&*row.0) { - u - } else { - continue; - }; - - let key_algorithm = if let Ok(k) = row.1.parse::() { - k - } else { - continue; - }; - - let signature = row.2; - - signatures - .entry(user_id) - .or_insert_with(BTreeMap::new) - .insert( - DeviceKeyId::from_parts(key_algorithm, device_id.as_str().into()), - signature.to_owned(), - ); - } - - let device = ReadOnlyDevice::new( - user_id, - device_id.as_str().into(), - display_name.clone(), - trust_state, - algorithms, - keys, - signatures, - ); - - self.devices.add(device); + signatures + .entry(user_id) + .or_insert_with(BTreeMap::new) + .insert( + DeviceKeyId::from_parts(key_algorithm, device_id.as_str().into()), + signature.to_owned(), + ); } - Ok(()) + Ok(ReadOnlyDevice::new( + user_id.to_owned(), + device_id, + display_name.clone(), + trust_state, + algorithms, + keys, + signatures, + )) + } + + async fn get_single_device( + &self, + user_id: &UserId, + device_id: &DeviceId, + ) -> Result> { + let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; + let mut connection = self.connection.lock().await; + + let row: Option<(i64, Option, i64)> = query_as( + "SELECT id, display_name, trust_state + FROM devices WHERE account_id = ? and user_id = ? and device_id = ?", + ) + .bind(account_id) + .bind(user_id.as_str()) + .bind(device_id.as_str()) + .fetch_optional(&mut *connection) + .await?; + + let row = if let Some(r) = row { + r + } else { + return Ok(None); + }; + + let device_row_id = row.0; + let display_name = row.1; + let trust_state = LocalTrust::from(row.2); + let device = self + .load_device_data( + &mut connection, + device_row_id, + user_id, + device_id.into(), + trust_state, + display_name, + ) + .await?; + + Ok(Some(device)) + } + + async fn load_devices(&self, user_id: &UserId) -> Result> { + let mut devices = HashMap::new(); + + let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; + let mut connection = self.connection.lock().await; + + let mut rows: Vec<(i64, String, Option, i64)> = query_as( + "SELECT id, device_id, display_name, trust_state + FROM devices WHERE account_id = ? and user_id = ?", + ) + .bind(account_id) + .bind(user_id.as_str()) + .fetch_all(&mut *connection) + .await?; + + for row in rows.drain(..) { + let device_row_id = row.0; + let device_id: DeviceIdBox = row.1.into(); + let display_name = row.2; + let trust_state = LocalTrust::from(row.3); + + let device = self + .load_device_data( + &mut connection, + device_row_id, + user_id, + device_id.clone(), + trust_state, + display_name, + ) + .await?; + + devices.insert(device_id, device); + } + + Ok(devices) } async fn save_device_helper( @@ -1276,7 +1327,6 @@ impl CryptoStore for SqliteStore { drop(connection); - self.load_devices().await?; self.load_tracked_users().await?; Ok(result) @@ -1424,7 +1474,6 @@ impl CryptoStore for SqliteStore { let mut transaction = connection.begin().await?; for device in devices { - self.devices.add(device.clone()); self.save_device_helper(&mut transaction, device.clone()) .await? } @@ -1457,11 +1506,14 @@ impl CryptoStore for SqliteStore { user_id: &UserId, device_id: &DeviceId, ) -> Result> { - Ok(self.devices.get(user_id, device_id)) + self.get_single_device(user_id, device_id).await } - async fn get_user_devices(&self, user_id: &UserId) -> Result { - Ok(self.devices.user_devices(user_id)) + async fn get_user_devices( + &self, + user_id: &UserId, + ) -> Result> { + Ok(self.load_devices(user_id).await?) } async fn get_user_identity(&self, user_id: &UserId) -> Result> { @@ -1925,8 +1977,8 @@ mod test { assert_eq!(device.keys(), loaded_device.keys()); let user_devices = store.get_user_devices(device.user_id()).await.unwrap(); - assert_eq!(user_devices.keys().next().unwrap(), device.device_id()); - assert_eq!(user_devices.devices().next().unwrap(), &device); + assert_eq!(&**user_devices.keys().next().unwrap(), device.device_id()); + assert_eq!(user_devices.values().next().unwrap(), &device); } #[tokio::test(threaded_scheduler)] From b1c8c64205a56191c8f2de3aaf5fc3add2263071 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 16 Oct 2020 17:27:00 +0200 Subject: [PATCH 15/34] matrix-sdk: Add support to delete devices. --- matrix_sdk/src/client.rs | 130 ++++++++++++++++++++++++++++++++++++++- matrix_sdk/src/error.rs | 31 +++++++++- 2 files changed, 157 insertions(+), 4 deletions(-) diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index 5b6fc4b5..19d6a551 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -65,7 +65,7 @@ pub enum LoopCtrl { use matrix_sdk_common::{ api::r0::{ account::register, - device::get_devices, + device::{delete_devices, get_devices}, directory::{get_public_rooms, get_public_rooms_filtered}, media::create_content, membership::{ @@ -82,6 +82,7 @@ use matrix_sdk_common::{ typing::create_typing_event::{ Request as TypingRequest, Response as TypingResponse, Typing, }, + uiaa::AuthData, }, assign, events::{ @@ -1383,6 +1384,71 @@ impl Client { self.send(request).await } + /// Delete the given devices from the server. + /// + /// # Arguments + /// + /// * `devices` - The list of devices that should be deleted from the + /// server. + /// + /// * `auth_data` - This request requires user interactive auth, the first + /// request needs to set this to `None` and will always fail with an + /// `UiaaResponse`. The response will contain information for the + /// interactive auth and the same request needs to be made but this time + /// with some `auth_data` provided. + /// + /// ```no_run + /// # use matrix_sdk::{ + /// # api::r0::uiaa::{UiaaResponse, AuthData}, + /// # Client, SyncSettings, Error, FromHttpResponseError, ServerError, + /// # }; + /// # use futures::executor::block_on; + /// # use serde_json::json; + /// # use url::Url; + /// # use std::{collections::BTreeMap, convert::TryFrom}; + /// # block_on(async { + /// # let homeserver = Url::parse("http://localhost:8080").unwrap(); + /// # let mut client = Client::new(homeserver).unwrap(); + /// let devices = &["DEVICEID".into()]; + /// + /// if let Err(e) = client.delete_devices(devices, None).await { + /// if let Some(info) = e.uiaa_response() { + /// let mut auth_parameters = BTreeMap::new(); + /// + /// let identifier = json!({ + /// "type": "m.id.user", + /// "user": "example", + /// }); + /// auth_parameters.insert("identifier".to_owned(), identifier); + /// auth_parameters.insert("password".to_owned(), "wordpass".into()); + /// + /// // This is needed because of https://github.com/matrix-org/synapse/issues/5665 + /// auth_parameters.insert("user".to_owned(), "@example:localhost".into()); + /// + /// let auth_data = AuthData::DirectRequest { + /// kind: "m.login.password", + /// auth_parameters, + /// session: info.session.as_deref(), + /// }; + /// + /// client + /// .delete_devices(devices, Some(auth_data)) + /// .await + /// .expect("Can't delete devices"); + /// } + /// } + /// # }); + pub async fn delete_devices( + &self, + devices: &[DeviceIdBox], + auth_data: Option>, + ) -> Result { + let mut request = delete_devices::Request::new(devices); + request.auth = auth_data; + + self.send(request).await + } + /// Synchronize the client's state with the latest state on the server. /// /// **Note**: You should not use this method to repeatedly sync if encryption @@ -1956,7 +2022,10 @@ mod test { use serde_json::json; use tempfile::tempdir; - use std::{convert::TryInto, io::Cursor, path::Path, str::FromStr, time::Duration}; + use std::{ + collections::BTreeMap, convert::TryInto, io::Cursor, path::Path, str::FromStr, + time::Duration, + }; async fn logged_in_client() -> Client { let session = Session { @@ -2755,4 +2824,61 @@ mod test { assert_eq!("tutorial".to_string(), room.read().await.display_name()); } + + #[tokio::test] + async fn delete_devices() { + let homeserver = Url::from_str(&mockito::server_url()).unwrap(); + let client = Client::new(homeserver).unwrap(); + + let _m = mock("POST", "/_matrix/client/r0/delete_devices") + .with_status(401) + .with_body( + json!({ + "flows": [ + { + "stages": [ + "m.login.password" + ] + } + ], + "params": {}, + "session": "vBslorikviAjxzYBASOBGfPp" + }) + .to_string(), + ) + .create(); + + let _m = mock("POST", "/_matrix/client/r0/delete_devices") + .with_status(401) + // empty response + // TODO rename that response type. + .with_body(test_json::LOGOUT.to_string()) + .create(); + + let devices = &["DEVICEID".into()]; + + if let Err(e) = client.delete_devices(devices, None).await { + if let Some(info) = e.uiaa_response() { + let mut auth_parameters = BTreeMap::new(); + + let identifier = json!({ + "type": "m.id.user", + "user": "example", + }); + auth_parameters.insert("identifier".to_owned(), identifier); + auth_parameters.insert("password".to_owned(), "wordpass".into()); + + let auth_data = AuthData::DirectRequest { + kind: "m.login.password", + auth_parameters, + session: info.session.as_deref(), + }; + + client + .delete_devices(devices, Some(auth_data)) + .await + .unwrap(); + } + } + } } diff --git a/matrix_sdk/src/error.rs b/matrix_sdk/src/error.rs index bdca9912..eda93c96 100644 --- a/matrix_sdk/src/error.rs +++ b/matrix_sdk/src/error.rs @@ -16,8 +16,11 @@ use matrix_sdk_base::Error as MatrixError; use matrix_sdk_common::{ - api::{r0::uiaa::UiaaResponse as UiaaError, Error as RumaClientError}, - FromHttpResponseError as RumaResponseError, IntoHttpError as RumaIntoHttpError, + api::{ + r0::uiaa::{UiaaInfo, UiaaResponse as UiaaError}, + Error as RumaClientError, + }, + FromHttpResponseError as RumaResponseError, IntoHttpError as RumaIntoHttpError, ServerError, }; use reqwest::Error as ReqwestError; use serde_json::Error as JsonError; @@ -79,6 +82,30 @@ pub enum Error { UiaaError(RumaResponseError), } +impl Error { + /// Try to destructure the error into an universal interactive auth info. + /// + /// Some requests require universal interactive auth, doing such a request + /// will always fail the first time with a 401 status code, the response + /// body will contain info how the client can authenticate. + /// + /// The request will need to be retried, this time containing additional + /// authentication data. + /// + /// This method is an convenience method to get to the info the server + /// returned on the first, failed request. + pub fn uiaa_response(&self) -> Option<&UiaaInfo> { + if let Error::UiaaError(RumaResponseError::Http(ServerError::Known( + UiaaError::AuthResponse(i), + ))) = self + { + Some(i) + } else { + None + } + } +} + impl From> for Error { fn from(error: RumaResponseError) -> Self { Self::UiaaError(error) From 93f49265a6aca09ff689ee7eeee0253359369ade Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 16 Oct 2020 19:39:50 +0200 Subject: [PATCH 16/34] crypto: Use a git version of sqlx. The beta release has a nasty bug where one thread would consume 100% of CPU. --- matrix_sdk_crypto/Cargo.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/matrix_sdk_crypto/Cargo.toml b/matrix_sdk_crypto/Cargo.toml index cb882341..4e7ee157 100644 --- a/matrix_sdk_crypto/Cargo.toml +++ b/matrix_sdk_crypto/Cargo.toml @@ -52,7 +52,8 @@ default-features = false features = ["std", "std-future"] [target.'cfg(not(target_arch = "wasm32"))'.dependencies.sqlx] -version = "0.4.0-beta.1" +git = "https://github.com/launchbadge/sqlx/" +rev = "fd25a7530cf087e1529553ff854f192738db3461" optional = true default-features = false features = ["runtime-tokio", "sqlite", "macros"] From 17cc4fcb81ff59902b0e1df025ebfb0fb813b628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 16 Oct 2020 19:45:38 +0200 Subject: [PATCH 17/34] matrix-sdk: Fix an import for the non-crypto case. --- matrix_sdk/src/client.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index 19d6a551..a0018e1e 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -95,7 +95,7 @@ use matrix_sdk_common::{ }, AnyMessageEventContent, }, - identifiers::{EventId, RoomId, RoomIdOrAliasId, ServerName, UserId}, + identifiers::{DeviceIdBox, EventId, RoomId, RoomIdOrAliasId, ServerName, UserId}, instant::{Duration, Instant}, js_int::UInt, locks::RwLock, @@ -112,7 +112,6 @@ use matrix_sdk_common::{ Request as RumaToDeviceRequest, Response as ToDeviceResponse, }, }, - identifiers::DeviceIdBox, locks::Mutex, }; From 404cc410ccc74c7cffe185a3072c7dd0400799b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Sat, 17 Oct 2020 14:39:19 +0200 Subject: [PATCH 18/34] crypto: Fix the docs and return value of the import_keys method. --- matrix_sdk/src/client.rs | 19 +++++++++++++++---- matrix_sdk_crypto/src/machine.rs | 5 +++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index a0018e1e..63b37bc5 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -1879,6 +1879,8 @@ impl Client { /// /// # Panics /// + /// This method will panic if it isn't run on a Tokio runtime. + /// /// This method will panic if it can't get enough randomness from the OS to /// encrypt the exported keys securely. /// @@ -1947,6 +1949,17 @@ impl Client { /// Import E2EE keys from the given file path. /// + /// # Arguments + /// + /// * `path` - The file path where the exported key file will can be found. + /// + /// * `passphrase` - The passphrase that should be used to decrypt the + /// exported room keys. + /// + /// # Panics + /// + /// This method will panic if it isn't run on a Tokio runtime. + /// /// ```no_run /// # use std::{path::PathBuf, time::Duration}; /// # use matrix_sdk::{ @@ -1972,7 +1985,7 @@ impl Client { feature = "docs", doc(cfg(all(encryption, not(target_arch = "wasm32")))) )] - pub async fn import_keys(&self, path: PathBuf, passphrase: &str) -> Result<()> { + pub async fn import_keys(&self, path: PathBuf, passphrase: &str) -> Result { let olm = self .base_client .olm_machine() @@ -1988,9 +2001,7 @@ impl Client { let task = tokio::task::spawn_blocking(decrypt); let import = task.await.expect("Task join error").unwrap(); - olm.import_keys(import).await.unwrap(); - - Ok(()) + Ok(olm.import_keys(import).await?) } } diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index a2b270d9..8206daff 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -906,6 +906,7 @@ impl OlmMachine { // Only import the session if we didn't have this session or if it's // a better version of the same session, that is the first known // index is lower. + // TODO load all sessions so we don't do a thousand small loads. if let Some(existing_session) = self .store .get_inbound_group_session( @@ -929,6 +930,10 @@ impl OlmMachine { let num_sessions = sessions.len(); self.store.save_inbound_group_sessions(&sessions).await?; + info!( + "Successfully imported {} inbound group sessions", + num_sessions + ); Ok(num_sessions) } From 0682292b91da71650b2952888735eb9081face08 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Sun, 18 Oct 2020 02:01:39 +0200 Subject: [PATCH 19/34] Upgrade ruma --- matrix_sdk/src/client.rs | 8 +++++--- matrix_sdk_common/Cargo.toml | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index 5b6fc4b5..b667e4a3 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -826,7 +826,7 @@ impl Client { /// let mut client = Client::new(homeserver).unwrap(); /// /// let generic_search_term = Some("matrix-rust-sdk"); - /// let filter = Some(assign!(Filter::new(), { generic_search_term })); + /// let filter = assign!(Filter::new(), { generic_search_term }); /// let request = assign!(PublicRoomsFilterRequest::new(), { filter }); /// /// client.public_rooms_filtered(request).await; @@ -1292,7 +1292,9 @@ impl Client { let mut data = Vec::new(); reader.read_to_end(&mut data)?; - let request = create_content::Request::new(content_type.essence_str(), data); + let request = assign!(create_content::Request::new(data), { + content_type: Some(content_type.essence_str()), + }); self.http_client.upload(request).await } @@ -2325,7 +2327,7 @@ mod test { .create(); let generic_search_term = Some("cheese"); - let filter = Some(assign!(Filter::new(), { generic_search_term })); + let filter = assign!(Filter::new(), { generic_search_term }); let request = assign!(PublicRoomsFilterRequest::new(), { filter }); let get_public_rooms_filtered::Response { chunk, .. } = diff --git a/matrix_sdk_common/Cargo.toml b/matrix_sdk_common/Cargo.toml index 6eb38e5d..252c7807 100644 --- a/matrix_sdk_common/Cargo.toml +++ b/matrix_sdk_common/Cargo.toml @@ -21,7 +21,7 @@ js_int = "0.1.9" [dependencies.ruma] version = "0.0.1" git = "https://github.com/ruma/ruma" -rev = "3869d75837b7aab60eef58fc834e498317d1e4a4" +rev = "50eb700571480d1440e15a387d10f98be8abab59" features = ["client-api", "unstable-pre-spec", "unstable-exhaustive-types"] [target.'cfg(not(target_arch = "wasm32"))'.dependencies] From 728d80ed0612b5003078e2e85ea32d658b953365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 19 Oct 2020 16:03:01 +0200 Subject: [PATCH 20/34] crypto: Connect the cross signing to the main state machine. --- matrix_sdk_crypto/src/machine.rs | 52 +++++++++++++++++++++++++--- matrix_sdk_crypto/src/olm/mod.rs | 1 + matrix_sdk_crypto/src/olm/signing.rs | 30 +++++++++++----- matrix_sdk_crypto/src/requests.rs | 20 +++++++++++ 4 files changed, 90 insertions(+), 13 deletions(-) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 8206daff..675d5be7 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -17,6 +17,7 @@ use std::path::Path; use std::{collections::BTreeMap, mem, sync::Arc}; use dashmap::DashMap; +use matrix_sdk_common::locks::Mutex; use tracing::{debug, error, info, instrument, trace, warn}; use matrix_sdk_common::{ @@ -50,9 +51,9 @@ use crate::{ key_request::KeyRequestMachine, olm::{ Account, EncryptionSettings, ExportedRoomKey, GroupSessionKey, IdentityKeys, - InboundGroupSession, ReadOnlyAccount, + InboundGroupSession, PrivateCrossSigningIdentity, ReadOnlyAccount, }, - requests::{IncomingResponse, OutgoingRequest}, + requests::{IncomingResponse, OutgoingRequest, UploadSigningKeysRequest}, session_manager::{GroupSessionManager, SessionManager}, store::{CryptoStore, MemoryStore, Result as StoreResult, Store}, verification::{Sas, VerificationMachine}, @@ -69,6 +70,11 @@ pub struct OlmMachine { device_id: Arc>, /// Our underlying Olm Account holding our identity keys. account: Account, + /// The private part of our cross signing identity. + /// Used to sign devices and other users, might be missing if some other + /// device bootstraped cross signing or cross signing isn't bootstrapped at + /// all. + user_identity: Arc>, /// Store for the encryption keys. /// Persists all the encryption keys so a client can resume the session /// without the need to create new keys. @@ -114,7 +120,13 @@ impl OlmMachine { let device_id: DeviceIdBox = device_id.into(); let account = ReadOnlyAccount::new(&user_id, &device_id); - OlmMachine::new_helper(user_id, device_id, store, account) + OlmMachine::new_helper( + user_id, + device_id, + store, + account, + PrivateCrossSigningIdentity::empty(user_id.to_owned()), + ) } fn new_helper( @@ -122,6 +134,7 @@ impl OlmMachine { device_id: DeviceIdBox, store: Box, account: ReadOnlyAccount, + user_identity: PrivateCrossSigningIdentity, ) -> Self { let user_id = Arc::new(user_id.clone()); @@ -169,6 +182,7 @@ impl OlmMachine { verification_machine, key_request_machine, identity_manager, + user_identity: Arc::new(Mutex::new(user_identity)), } } @@ -207,7 +221,12 @@ impl OlmMachine { } }; - Ok(OlmMachine::new_helper(&user_id, device_id, store, account)) + // TODO load the identity like we load an account. + let identity = PrivateCrossSigningIdentity::empty(user_id.clone()); + + Ok(OlmMachine::new_helper( + &user_id, device_id, store, account, identity, + )) } /// Create a new machine with the default crypto store. @@ -311,11 +330,36 @@ impl OlmMachine { IncomingResponse::ToDevice(_) => { self.mark_to_device_request_as_sent(&request_id).await?; } + IncomingResponse::SigningKeysUpload(_) => { + self.receive_cross_signing_upload_response().await?; + } }; Ok(()) } + /// Mark the cross signing identity as shared. + async fn receive_cross_signing_upload_response(&self) -> StoreResult<()> { + self.user_identity.lock().await.mark_as_shared(); + // TOOD save the identity. + Ok(()) + } + + /// Create a new cross signing identity and get the upload request to push + /// the new public keys to the server. + /// + /// **Warning**: This will delete any existing cross signing keys that might + /// exist on the server and thus will reset the trust between all the + /// devices. + /// + /// Uploading these keys will require user interactive auth. + pub async fn bootstrap_cross_signing(&self) -> UploadSigningKeysRequest { + let mut lock = self.user_identity.lock().await; + *lock = PrivateCrossSigningIdentity::new(self.user_id().to_owned()).await; + // TODO save the identity. + lock.as_upload_request().await + } + /// Should device or one-time keys be uploaded to the server. /// /// This needs to be checked periodically, ideally after every sync request. diff --git a/matrix_sdk_crypto/src/olm/mod.rs b/matrix_sdk_crypto/src/olm/mod.rs index f2e39094..df2b387a 100644 --- a/matrix_sdk_crypto/src/olm/mod.rs +++ b/matrix_sdk_crypto/src/olm/mod.rs @@ -32,6 +32,7 @@ pub use group_sessions::{ pub(crate) use group_sessions::{GroupSessionKey, OutboundGroupSession}; pub use olm_rs::{account::IdentityKeys, PicklingMode}; pub use session::{PickledSession, Session, SessionPickle}; +pub(crate) use signing::PrivateCrossSigningIdentity; pub(crate) use utility::Utility; #[cfg(test)] diff --git a/matrix_sdk_crypto/src/olm/signing.rs b/matrix_sdk_crypto/src/olm/signing.rs index 0f47a81f..8395db9c 100644 --- a/matrix_sdk_crypto/src/olm/signing.rs +++ b/matrix_sdk_crypto/src/olm/signing.rs @@ -32,12 +32,15 @@ use zeroize::Zeroizing; use olm_rs::{errors::OlmUtilityError, pk::OlmPkSigning, utility::OlmUtility}; use matrix_sdk_common::{ - api::r0::keys::{upload_signing_keys::Request as UploadRequest, CrossSigningKey, KeyUsage}, + api::r0::keys::{CrossSigningKey, KeyUsage}, identifiers::UserId, locks::Mutex, }; -use crate::identities::{MasterPubkey, SelfSigningPubkey, UserSigningPubkey}; +use crate::{ + identities::{MasterPubkey, SelfSigningPubkey, UserSigningPubkey}, + requests::UploadSigningKeysRequest, +}; fn encode>(input: T) -> String { encode_config(input, URL_SAFE_NO_PAD) @@ -320,7 +323,17 @@ impl Signing { } impl PrivateCrossSigningIdentity { - async fn new(user_id: UserId) -> Self { + pub(crate) fn empty(user_id: UserId) -> Self { + Self { + user_id: Arc::new(user_id), + shared: Arc::new(AtomicBool::new(false)), + master_key: Arc::new(Mutex::new(None)), + self_signing_key: Arc::new(Mutex::new(None)), + user_signing_key: Arc::new(Mutex::new(None)), + } + } + + pub(crate) async fn new(user_id: UserId) -> Self { let master = Signing::new(); let public_key = master.cross_signing_key(user_id.clone(), KeyUsage::Master); @@ -356,7 +369,7 @@ impl PrivateCrossSigningIdentity { } } - fn mark_as_shared(&self) { + pub fn mark_as_shared(&self) { self.shared.store(true, Ordering::SeqCst) } @@ -364,7 +377,7 @@ impl PrivateCrossSigningIdentity { self.shared.load(Ordering::SeqCst) } - async fn pickle(&self, pickle_key: &[u8]) -> PickledCrossSigningIdentity { + pub async fn pickle(&self, pickle_key: &[u8]) -> PickledCrossSigningIdentity { let master_key = if let Some(m) = self.master_key.lock().await.as_ref() { Some(m.pickle(pickle_key).await) } else { @@ -392,7 +405,7 @@ impl PrivateCrossSigningIdentity { } } - async fn from_pickle(pickle: PickledCrossSigningIdentity, pickle_key: &[u8]) -> Self { + pub async fn from_pickle(pickle: PickledCrossSigningIdentity, pickle_key: &[u8]) -> Self { let master = if let Some(m) = pickle.master_key { Some(MasterSigning::from_pickle(m, pickle_key)) } else { @@ -420,7 +433,7 @@ impl PrivateCrossSigningIdentity { } } - async fn as_upload_request(&self) -> UploadRequest<'_> { + pub(crate) async fn as_upload_request(&self) -> UploadSigningKeysRequest { let master_key = self .master_key .lock() @@ -445,8 +458,7 @@ impl PrivateCrossSigningIdentity { .cloned() .map(|k| k.public_key.into()); - UploadRequest { - auth: None, + UploadSigningKeysRequest { master_key, user_signing_key, self_signing_key, diff --git a/matrix_sdk_crypto/src/requests.rs b/matrix_sdk_crypto/src/requests.rs index 3a6ba4d9..5b70c744 100644 --- a/matrix_sdk_crypto/src/requests.rs +++ b/matrix_sdk_crypto/src/requests.rs @@ -20,6 +20,8 @@ use matrix_sdk_common::{ claim_keys::Response as KeysClaimResponse, get_keys::Response as KeysQueryResponse, upload_keys::{Request as KeysUploadRequest, Response as KeysUploadResponse}, + upload_signing_keys::Response as SigningKeysUploadResponse, + CrossSigningKey, }, to_device::{send_event_to_device::Response as ToDeviceResponse, DeviceIdOrAllDevices}, }, @@ -56,6 +58,21 @@ impl ToDeviceRequest { } } +/// Request that will publish a cross signing identity. +/// +/// This uploads the public cross signing key triplet. +#[derive(Debug, Clone)] +pub struct UploadSigningKeysRequest { + /// The user's master key. + pub master_key: Option, + /// The user's self-signing key. Must be signed with the accompanied master, or by the + /// user's most recently uploaded master key if no master key is included in the request. + pub self_signing_key: Option, + /// The user's user-signing key. Must be signed with the accompanied master, or by the + /// user's most recently uploaded master key if no master key is included in the request. + pub user_signing_key: Option, +} + /// Customized version of `ruma_client_api::r0::keys::get_keys::Request`, without any references. #[derive(Clone, Debug)] pub struct KeysQueryRequest { @@ -141,6 +158,9 @@ pub enum IncomingResponse<'a> { /// The key claiming requests, giving us new one-time keys of other users so /// new Olm sessions can be created. KeysClaim(&'a KeysClaimResponse), + /// The cross signing keys upload response, marking our private cross + /// signing identity as shared. + SigningKeysUpload(&'a SigningKeysUploadResponse), } impl<'a> From<&'a KeysUploadResponse> for IncomingResponse<'a> { From 7cab7cadc9ed88312081f4de389c3b9e3037f175 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 20 Oct 2020 17:19:37 +0200 Subject: [PATCH 21/34] crypto: Rework the cryptostore. This modifies the cryptostore and storage logic in two ways: * The cryptostore trait has only one main save method. * The receive_sync method tries to save all the objects in one `save_changes()` call. This means that all the changes a sync makes get commited to the store in one transaction, leaving us in a consistent state. This also means that we can pass the Changes struct the receive sync method collects to our caller if the caller wishes to store the room state and crypto state changes in a single transaction. --- matrix_sdk_base/src/client.rs | 6 +- matrix_sdk_crypto/src/identities/device.rs | 29 +- matrix_sdk_crypto/src/identities/manager.rs | 68 ++-- matrix_sdk_crypto/src/key_request.rs | 153 ++++---- matrix_sdk_crypto/src/machine.rs | 169 +++++---- matrix_sdk_crypto/src/olm/account.rs | 68 ++-- .../src/session_manager/group_sessions.rs | 29 +- .../src/session_manager/sessions.rs | 16 +- matrix_sdk_crypto/src/store/memorystore.rs | 88 ++--- matrix_sdk_crypto/src/store/mod.rs | 96 +++-- matrix_sdk_crypto/src/store/sqlite.rs | 339 ++++++++++++------ matrix_sdk_crypto/src/verification/machine.rs | 30 +- matrix_sdk_crypto/src/verification/sas/mod.rs | 97 +++-- 13 files changed, 711 insertions(+), 477 deletions(-) diff --git a/matrix_sdk_base/src/client.rs b/matrix_sdk_base/src/client.rs index 6557a69b..e4cae5bc 100644 --- a/matrix_sdk_base/src/client.rs +++ b/matrix_sdk_base/src/client.rs @@ -971,8 +971,6 @@ impl BaseClient { return Ok(()); } - *self.sync_token.write().await = Some(response.next_batch.clone()); - #[cfg(feature = "encryption")] { let olm = self.olm.lock().await; @@ -982,10 +980,12 @@ impl BaseClient { // decryptes to-device events, but leaves room events alone. // This makes sure that we have the deryption keys for the room // events at hand. - o.receive_sync_response(response).await; + o.receive_sync_response(response).await?; } } + *self.sync_token.write().await = Some(response.next_batch.clone()); + // when events change state, updated_* signals to StateStore to update database self.iter_joined_rooms(response).await?; self.iter_invited_rooms(response).await?; diff --git a/matrix_sdk_crypto/src/identities/device.rs b/matrix_sdk_crypto/src/identities/device.rs index a0281217..0a75ad29 100644 --- a/matrix_sdk_crypto/src/identities/device.rs +++ b/matrix_sdk_crypto/src/identities/device.rs @@ -39,7 +39,10 @@ use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; use tracing::warn; -use crate::olm::{InboundGroupSession, Session}; +use crate::{ + olm::{InboundGroupSession, Session}, + store::{Changes, DeviceChanges}, +}; #[cfg(test)] use crate::{OlmMachine, ReadOnlyAccount}; @@ -118,10 +121,15 @@ impl Device { pub async fn set_local_trust(&self, trust_state: LocalTrust) -> StoreResult<()> { self.inner.set_trust_state(trust_state); - self.verification_machine - .store - .save_devices(&[self.inner.clone()]) - .await + let changes = Changes { + devices: DeviceChanges { + changed: vec![self.inner.clone()], + ..Default::default() + }, + ..Default::default() + }; + + self.verification_machine.store.save_changes(changes).await } /// Encrypt the given content for this `Device`. @@ -135,7 +143,7 @@ impl Device { &self, event_type: EventType, content: Value, - ) -> OlmResult { + ) -> OlmResult<(Session, EncryptedEventContent)> { self.inner .encrypt(&**self.verification_machine.store, event_type, content) .await @@ -146,7 +154,7 @@ impl Device { pub async fn encrypt_session( &self, session: InboundGroupSession, - ) -> OlmResult { + ) -> OlmResult<(Session, EncryptedEventContent)> { let export = session.export().await; let content: ForwardedRoomKeyEventContent = if let Ok(c) = export.try_into() { @@ -364,7 +372,7 @@ impl ReadOnlyDevice { store: &dyn CryptoStore, event_type: EventType, content: Value, - ) -> OlmResult { + ) -> OlmResult<(Session, EncryptedEventContent)> { let sender_key = if let Some(k) = self.get_key(DeviceKeyAlgorithm::Curve25519) { k } else { @@ -396,10 +404,9 @@ impl ReadOnlyDevice { return Err(OlmError::MissingSession); }; - let message = session.encrypt(&self, event_type, content).await; - store.save_sessions(&[session]).await?; + let message = session.encrypt(&self, event_type, content).await?; - message + Ok((session, message)) } /// Update a device with a new device keys struct. diff --git a/matrix_sdk_crypto/src/identities/manager.rs b/matrix_sdk_crypto/src/identities/manager.rs index ba6fb749..d91ffa2c 100644 --- a/matrix_sdk_crypto/src/identities/manager.rs +++ b/matrix_sdk_crypto/src/identities/manager.rs @@ -33,7 +33,7 @@ use crate::{ }, requests::KeysQueryRequest, session_manager::GroupSessionManager, - store::{Result as StoreResult, Store}, + store::{Changes, DeviceChanges, IdentityChanges, Result as StoreResult, Store}, }; #[derive(Debug, Clone)] @@ -79,7 +79,7 @@ impl IdentityManager { pub async fn receive_keys_query_response( &self, response: &KeysQueryResponse, - ) -> OlmResult<(Vec, Vec)> { + ) -> OlmResult<(DeviceChanges, IdentityChanges)> { // TODO create a enum that tells us how the device/identity changed, // e.g. new/deleted/display name change. // @@ -92,9 +92,15 @@ impl IdentityManager { let changed_devices = self .handle_devices_from_key_query(&response.device_keys) .await?; - self.store.save_devices(&changed_devices).await?; let changed_identities = self.handle_cross_singing_keys(response).await?; - self.store.save_user_identities(&changed_identities).await?; + + let changes = Changes { + identities: changed_identities.clone(), + devices: changed_devices.clone(), + ..Default::default() + }; + + self.store.save_changes(changes).await?; Ok((changed_devices, changed_identities)) } @@ -111,9 +117,10 @@ impl IdentityManager { async fn handle_devices_from_key_query( &self, device_keys_map: &BTreeMap>, - ) -> StoreResult> { + ) -> StoreResult { let mut users_with_new_or_deleted_devices = HashSet::new(); - let mut changed_devices = Vec::new(); + + let mut changes = DeviceChanges::default(); for (user_id, device_map) in device_keys_map { // TODO move this out into the handle keys query response method @@ -137,7 +144,7 @@ impl IdentityManager { let device = self.store.get_readonly_device(&user_id, device_id).await?; - let device = if let Some(mut device) = device { + if let Some(mut device) = device { if let Err(e) = device.update_device(device_keys) { warn!( "Failed to update the device keys for {} {}: {:?}", @@ -145,7 +152,7 @@ impl IdentityManager { ); continue; } - device + changes.changed.push(device); } else { let device = match ReadOnlyDevice::try_from(device_keys) { Ok(d) => d, @@ -159,23 +166,21 @@ impl IdentityManager { }; info!("Adding a new device to the device store {:?}", device); users_with_new_or_deleted_devices.insert(user_id); - device - }; - - changed_devices.push(device); + changes.new.push(device); + } } let current_devices: HashSet<&DeviceIdBox> = device_map.keys().collect(); let stored_devices = self.store.get_readonly_devices(&user_id).await?; let stored_devices_set: HashSet<&DeviceIdBox> = stored_devices.keys().collect(); - let deleted_devices = stored_devices_set.difference(¤t_devices); + let deleted_devices_set = stored_devices_set.difference(¤t_devices); - for device_id in deleted_devices { + for device_id in deleted_devices_set { users_with_new_or_deleted_devices.insert(user_id); if let Some(device) = stored_devices.get(*device_id) { device.mark_as_deleted(); - self.store.delete_device(device.clone()).await?; + changes.deleted.push(device.clone()); } } } @@ -183,7 +188,7 @@ impl IdentityManager { self.group_manager .invalidate_sessions_new_devices(&users_with_new_or_deleted_devices); - Ok(changed_devices) + Ok(changes) } /// Handle the device keys part of a key query response. @@ -197,8 +202,8 @@ impl IdentityManager { async fn handle_cross_singing_keys( &self, response: &KeysQueryResponse, - ) -> StoreResult> { - let mut changed = Vec::new(); + ) -> StoreResult { + let mut changes = IdentityChanges::default(); for (user_id, master_key) in &response.master_keys { let master_key = MasterPubkey::from(master_key); @@ -213,7 +218,7 @@ impl IdentityManager { continue; }; - let identity = if let Some(mut i) = self.store.get_user_identity(user_id).await? { + let result = if let Some(mut i) = self.store.get_user_identity(user_id).await? { match &mut i { UserIdentities::Own(ref mut identity) => { let user_signing = if let Some(s) = response.user_signing_keys.get(user_id) @@ -230,11 +235,11 @@ impl IdentityManager { identity .update(master_key, self_signing, user_signing) - .map(|_| i) - } - UserIdentities::Other(ref mut identity) => { - identity.update(master_key, self_signing).map(|_| i) + .map(|_| (i, false)) } + UserIdentities::Other(ref mut identity) => identity + .update(master_key, self_signing) + .map(|_| (i, false)), } } else if user_id == self.user_id() { if let Some(s) = response.user_signing_keys.get(user_id) { @@ -252,7 +257,7 @@ impl IdentityManager { } OwnUserIdentity::new(master_key, self_signing, user_signing) - .map(UserIdentities::Own) + .map(|i| (UserIdentities::Own(i), true)) } else { warn!( "User identity for our own user {} didn't contain a \ @@ -268,17 +273,22 @@ impl IdentityManager { ); continue; } else { - UserIdentity::new(master_key, self_signing).map(UserIdentities::Other) + UserIdentity::new(master_key, self_signing) + .map(|i| (UserIdentities::Other(i), true)) }; - match identity { - Ok(i) => { + match result { + Ok((i, new)) => { trace!( "Updated or created new user identity for {}: {:?}", user_id, i ); - changed.push(i); + if new { + changes.new.push(i); + } else { + changes.changed.push(i); + } } Err(e) => { warn!( @@ -290,7 +300,7 @@ impl IdentityManager { } } - Ok(changed) + Ok(changes) } /// Get a key query request if one is needed. diff --git a/matrix_sdk_crypto/src/key_request.rs b/matrix_sdk_crypto/src/key_request.rs index 060a85cb..3a4d26cf 100644 --- a/matrix_sdk_crypto/src/key_request.rs +++ b/matrix_sdk_crypto/src/key_request.rs @@ -41,7 +41,7 @@ use matrix_sdk_common::{ use crate::{ error::{OlmError, OlmResult}, - olm::{InboundGroupSession, OutboundGroupSession}, + olm::{InboundGroupSession, OutboundGroupSession, Session}, requests::{OutgoingRequest, ToDeviceRequest}, store::{CryptoStoreError, Store}, Device, @@ -235,15 +235,18 @@ impl KeyRequestMachine { /// Handle all the incoming key requests that are queued up and empty our /// key request queue. - pub async fn collect_incoming_key_requests(&self) -> OlmResult<()> { + pub async fn collect_incoming_key_requests(&self) -> OlmResult> { + let mut changed_sessions = Vec::new(); for item in self.incoming_key_requests.iter() { let event = item.value(); - self.handle_key_request(event).await?; + if let Some(s) = self.handle_key_request(event).await? { + changed_sessions.push(s); + } } self.incoming_key_requests.clear(); - Ok(()) + Ok(changed_sessions) } /// Store the key share request for later, once we get an Olm session with @@ -294,7 +297,7 @@ impl KeyRequestMachine { async fn handle_key_request( &self, event: &ToDeviceEvent, - ) -> OlmResult<()> { + ) -> OlmResult> { let key_info = match event.content.action { Action::Request => { if let Some(info) = &event.content.body { @@ -305,11 +308,11 @@ impl KeyRequestMachine { action, but no key info was found", event.sender, event.content.requesting_device_id ); - return Ok(()); + return Ok(None); } } // We ignore cancellations here since there's nothing to serve. - Action::CancelRequest => return Ok(()), + Action::CancelRequest => return Ok(None), }; let session = self @@ -328,7 +331,7 @@ impl KeyRequestMachine { "Received a key request from {} {} for an unknown inbound group session {}.", &event.sender, &event.content.requesting_device_id, &key_info.session_id ); - return Ok(()); + return Ok(None); }; let device = self @@ -349,6 +352,8 @@ impl KeyRequestMachine { device.device_id(), e ); + + Ok(None) } else { info!( "Serving a key request for {} from {} {}.", @@ -357,20 +362,20 @@ impl KeyRequestMachine { device.device_id() ); - if let Err(e) = self.share_session(&session, &device).await { - match e { - OlmError::MissingSession => { - info!( - "Key request from {} {} is missing an Olm session, \ - putting the request in the wait queue", - device.user_id(), - device.device_id() - ); - self.handle_key_share_without_session(device, event); - return Ok(()); - } - e => return Err(e), + match self.share_session(&session, &device).await { + Ok(s) => Ok(Some(s)), + Err(OlmError::MissingSession) => { + info!( + "Key request from {} {} is missing an Olm session, \ + putting the request in the wait queue", + device.user_id(), + device.device_id() + ); + self.handle_key_share_without_session(device, event); + + Ok(None) } + Err(e) => Err(e), } } } else { @@ -379,13 +384,17 @@ impl KeyRequestMachine { &event.sender, &event.content.requesting_device_id ); self.store.update_tracked_user(&event.sender, true).await?; - } - Ok(()) + Ok(None) + } } - async fn share_session(&self, session: &InboundGroupSession, device: &Device) -> OlmResult<()> { - let content = device.encrypt_session(session.clone()).await?; + async fn share_session( + &self, + session: &InboundGroupSession, + device: &Device, + ) -> OlmResult { + let (used_session, content) = device.encrypt_session(session.clone()).await?; let id = Uuid::new_v4(); let mut messages = BTreeMap::new(); @@ -412,7 +421,7 @@ impl KeyRequestMachine { self.outgoing_to_device_requests.insert(id, request); - Ok(()) + Ok(used_session) } /// Check if it's ok to share a session with the given device. @@ -569,23 +578,20 @@ impl KeyRequestMachine { Ok(()) } - /// Save an inbound group session we received using a key forward. + /// Mark the given outgoing key info as done. /// - /// At the same time delete the key info since we received the wanted key. - async fn save_session( - &self, - key_info: OugoingKeyInfo, - session: InboundGroupSession, - ) -> Result<(), CryptoStoreError> { + /// This will queue up a request cancelation. + async fn mark_as_done(&self, key_info: OugoingKeyInfo) -> Result<(), CryptoStoreError> { // TODO perhaps only remove the key info if the first known index is 0. trace!( "Successfully received a forwarded room key for {:#?}", key_info ); - self.store.save_inbound_group_sessions(&[session]).await?; self.outgoing_to_device_requests .remove(&key_info.request_id); + // TODO return the key info instead of deleting it so the sync handler + // can delete it in one transaction. self.delete_key_info(&key_info).await?; let content = RoomKeyRequestEventContent { @@ -609,7 +615,8 @@ impl KeyRequestMachine { &self, sender_key: &str, event: &mut ToDeviceEvent, - ) -> Result>, CryptoStoreError> { + ) -> Result<(Option>, Option), CryptoStoreError> + { let key_info = self.get_key_info(&event.content).await?; if let Some(info) = key_info { @@ -626,27 +633,32 @@ impl KeyRequestMachine { // If we have a previous session, check if we have a better version // and store the new one if so. - if let Some(old_session) = old_session { + let session = if let Some(old_session) = old_session { let first_old_index = old_session.first_known_index().await; let first_index = session.first_known_index().await; if first_old_index > first_index { - self.save_session(info, session).await?; + self.mark_as_done(info).await?; + Some(session) + } else { + None } // If we didn't have a previous session, store it. } else { - self.save_session(info, session).await?; - } + self.mark_as_done(info).await?; + Some(session) + }; - Ok(Some(Raw::from(AnyToDeviceEvent::ForwardedRoomKey( - event.clone(), - )))) + Ok(( + Some(Raw::from(AnyToDeviceEvent::ForwardedRoomKey(event.clone()))), + session, + )) } else { info!( "Received a forwarded room key from {}, but no key info was found.", event.sender, ); - Ok(None) + Ok((None, None)) } } } @@ -831,20 +843,20 @@ mod test { .is_none() ); - machine + let (_, first_session) = machine .receive_forwarded_room_key(&session.sender_key, &mut event) .await .unwrap(); - - let first_session = machine - .store - .get_inbound_group_session(session.room_id(), &session.sender_key, session.session_id()) - .await - .unwrap() - .unwrap(); + let first_session = first_session.unwrap(); assert_eq!(first_session.first_known_index().await, 10); + machine + .store + .save_inbound_group_sessions(&[first_session.clone()]) + .await + .unwrap(); + // Get the cancel request. let request = machine.outgoing_to_device_requests.iter().next().unwrap(); let id = request.request_id; @@ -875,19 +887,12 @@ mod test { content, }; - machine + let (_, second_session) = machine .receive_forwarded_room_key(&session.sender_key, &mut event) .await .unwrap(); - let second_session = machine - .store - .get_inbound_group_session(session.room_id(), &session.sender_key, session.session_id()) - .await - .unwrap() - .unwrap(); - - assert_eq!(second_session.first_known_index().await, 10); + assert!(second_session.is_none()); let export = session.export_at_index(0).await.unwrap(); @@ -898,18 +903,12 @@ mod test { content, }; - machine + let (_, second_session) = machine .receive_forwarded_room_key(&session.sender_key, &mut event) .await .unwrap(); - let second_session = machine - .store - .get_inbound_group_session(session.room_id(), &session.sender_key, session.session_id()) - .await - .unwrap() - .unwrap(); - assert_eq!(second_session.first_known_index().await, 0); + assert_eq!(second_session.unwrap().first_known_index().await, 0); } #[async_test] @@ -1132,14 +1131,19 @@ mod test { .unwrap() .is_none()); - let (decrypted, sender_key, _) = + let (_, decrypted, sender_key, _) = alice_account.decrypt_to_device_event(&event).await.unwrap(); if let AnyToDeviceEvent::ForwardedRoomKey(mut e) = decrypted.deserialize().unwrap() { - alice_machine + let (_, session) = alice_machine .receive_forwarded_room_key(&sender_key, &mut e) .await .unwrap(); + alice_machine + .store + .save_inbound_group_sessions(&[session.unwrap()]) + .await + .unwrap(); } else { panic!("Invalid decrypted event type"); } @@ -1315,14 +1319,19 @@ mod test { .unwrap() .is_none()); - let (decrypted, sender_key, _) = + let (_, decrypted, sender_key, _) = alice_account.decrypt_to_device_event(&event).await.unwrap(); if let AnyToDeviceEvent::ForwardedRoomKey(mut e) = decrypted.deserialize().unwrap() { - alice_machine + let (_, session) = alice_machine .receive_forwarded_room_key(&sender_key, &mut e) .await .unwrap(); + alice_machine + .store + .save_inbound_group_sessions(&[session.unwrap()]) + .await + .unwrap(); } else { panic!("Invalid decrypted event type"); } diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 675d5be7..0db52888 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -47,15 +47,18 @@ use matrix_sdk_common::{ use crate::store::sqlite::SqliteStore; use crate::{ error::{EventError, MegolmError, MegolmResult, OlmError, OlmResult}, - identities::{Device, IdentityManager, ReadOnlyDevice, UserDevices, UserIdentities}, + identities::{Device, IdentityManager, UserDevices}, key_request::KeyRequestMachine, olm::{ Account, EncryptionSettings, ExportedRoomKey, GroupSessionKey, IdentityKeys, - InboundGroupSession, PrivateCrossSigningIdentity, ReadOnlyAccount, + InboundGroupSession, PrivateCrossSigningIdentity, ReadOnlyAccount, Session, }, requests::{IncomingResponse, OutgoingRequest, UploadSigningKeysRequest}, session_manager::{GroupSessionManager, SessionManager}, - store::{CryptoStore, MemoryStore, Result as StoreResult, Store}, + store::{ + Changes, CryptoStore, DeviceChanges, IdentityChanges, MemoryStore, Result as StoreResult, + Store, + }, verification::{Sas, VerificationMachine}, ToDeviceRequest, }; @@ -467,7 +470,7 @@ impl OlmMachine { async fn receive_keys_query_response( &self, response: &KeysQueryResponse, - ) -> OlmResult<(Vec, Vec)> { + ) -> OlmResult<(DeviceChanges, IdentityChanges)> { self.identity_manager .receive_keys_query_response(response) .await @@ -498,12 +501,12 @@ impl OlmMachine { async fn decrypt_to_device_event( &self, event: &ToDeviceEvent, - ) -> OlmResult> { - let (decrypted_event, sender_key, signing_key) = + ) -> OlmResult<(Session, Raw, Option)> { + let (session, decrypted_event, sender_key, signing_key) = self.account.decrypt_to_device_event(event).await?; // Handle the decrypted event, e.g. fetch out Megolm sessions out of // the event. - if let Some(event) = self + if let (Some(event), group_session) = self .handle_decrypted_to_device_event(&sender_key, &signing_key, &decrypted_event) .await? { @@ -512,9 +515,9 @@ impl OlmMachine { // don't want them to be able to do silly things with it. Handling // events modifies them and returns a modified one, so replace it // here if we get one. - Ok(event) + Ok((session, event, group_session)) } else { - Ok(decrypted_event) + Ok((session, decrypted_event, None)) } } @@ -524,7 +527,7 @@ impl OlmMachine { sender_key: &str, signing_key: &str, event: &mut ToDeviceEvent, - ) -> OlmResult>> { + ) -> OlmResult<(Option>, Option)> { match event.content.algorithm { EventEncryptionAlgorithm::MegolmV1AesSha2 => { let session_key = GroupSessionKey(mem::take(&mut event.content.session_key)); @@ -535,17 +538,15 @@ impl OlmMachine { &event.content.room_id, session_key, )?; - let _ = self.store.save_inbound_group_sessions(&[session]).await?; - let event = Raw::from(AnyToDeviceEvent::RoomKey(event.clone())); - Ok(Some(event)) + Ok((Some(event), Some(session))) } _ => { warn!( "Received room key with unsupported key algorithm {}", event.content.algorithm ); - Ok(None) + Ok((None, None)) } } } @@ -555,9 +556,14 @@ impl OlmMachine { &self, room_id: &RoomId, ) -> OlmResult<()> { - self.group_session_manager + let (_, session) = self + .group_session_manager .create_outbound_group_session(room_id, EncryptionSettings::default()) - .await + .await?; + + self.store.save_inbound_group_sessions(&[session]).await?; + + Ok(()) } /// Encrypt a room message for the given room. @@ -647,12 +653,12 @@ impl OlmMachine { sender_key: &str, signing_key: &str, event: &Raw, - ) -> OlmResult>> { + ) -> OlmResult<(Option>, Option)> { let event = if let Ok(e) = event.deserialize() { e } else { warn!("Decrypted to-device event failed to be parsed correctly"); - return Ok(None); + return Ok((None, None)); }; match event { @@ -665,7 +671,7 @@ impl OlmMachine { .await?), _ => { warn!("Received a unexpected encrypted to-device event"); - Ok(None) + Ok((None, None)) } } } @@ -699,11 +705,8 @@ impl OlmMachine { self.verification_machine.get_sas(flow_id) } - async fn update_one_time_key_count( - &self, - key_count: &BTreeMap, - ) -> StoreResult<()> { - self.account.update_uploaded_key_count(key_count).await + async fn update_one_time_key_count(&self, key_count: &BTreeMap) { + self.account.update_uploaded_key_count(key_count).await; } /// Handle a sync response and update the internal state of the Olm machine. @@ -719,15 +722,19 @@ impl OlmMachine { /// /// [`decrypt_room_event`]: #method.decrypt_room_event #[instrument(skip(response))] - pub async fn receive_sync_response(&self, response: &mut SyncResponse) { + pub async fn receive_sync_response(&self, response: &mut SyncResponse) -> OlmResult<()> { + // Remove verification objects that have expired or are done. self.verification_machine.garbage_collect(); - if let Err(e) = self - .update_one_time_key_count(&response.device_one_time_keys_count) - .await - { - error!("Error updating the one-time key count {:?}", e); - } + // Always save the account, a new session might get created which also + // touches the account. + let mut changes = Changes { + account: Some(self.account.inner.clone()), + ..Default::default() + }; + + self.update_one_time_key_count(&response.device_one_time_keys_count) + .await; for user_id in &response.device_lists.changed { if let Err(e) = self.identity_manager.mark_user_as_changed(&user_id).await { @@ -748,29 +755,36 @@ impl OlmMachine { match &mut event { AnyToDeviceEvent::RoomEncrypted(e) => { - let decrypted_event = match self.decrypt_to_device_event(e).await { - Ok(e) => e, - Err(err) => { - warn!( - "Failed to decrypt to-device event from {} {}", - e.sender, err - ); + let (session, decrypted_event, group_session) = + match self.decrypt_to_device_event(e).await { + Ok(e) => e, + Err(err) => { + warn!( + "Failed to decrypt to-device event from {} {}", + e.sender, err + ); - if let OlmError::SessionWedged(sender, curve_key) = err { - if let Err(e) = self - .session_manager - .mark_device_as_wedged(&sender, &curve_key) - .await - { - error!( - "Couldn't mark device from {} to be unwedged {:?}", - sender, e - ); + if let OlmError::SessionWedged(sender, curve_key) = err { + if let Err(e) = self + .session_manager + .mark_device_as_wedged(&sender, &curve_key) + .await + { + error!( + "Couldn't mark device from {} to be unwedged {:?}", + sender, e + ); + } } + continue; } - continue; - } - }; + }; + + changes.sessions.push(session); + + if let Some(group_session) = group_session { + changes.inbound_group_sessions.push(group_session); + } *event_result = decrypted_event; } @@ -789,13 +803,14 @@ impl OlmMachine { } } - if let Err(e) = self + let changed_sessions = self .key_request_machine .collect_incoming_key_requests() - .await - { - error!("Error collecting our key share requests {:?}", e); - } + .await?; + + changes.sessions.extend(changed_sessions); + + Ok(self.store.save_changes(changes).await?) } /// Decrypt an event from a room timeline. @@ -973,7 +988,13 @@ impl OlmMachine { let num_sessions = sessions.len(); - self.store.save_inbound_group_sessions(&sessions).await?; + let changes = Changes { + inbound_group_sessions: sessions, + ..Default::default() + }; + + self.store.save_changes(changes).await?; + info!( "Successfully imported {} inbound group sessions", num_sessions @@ -1198,15 +1219,19 @@ pub(crate) mod test { .unwrap() .unwrap(); + let (session, content) = bob_device + .encrypt(EventType::Dummy, json!({})) + .await + .unwrap(); + alice.store.save_sessions(&[session]).await.unwrap(); + let event = ToDeviceEvent { sender: alice.user_id().clone(), - content: bob_device - .encrypt(EventType::Dummy, json!({})) - .await - .unwrap(), + content, }; - bob.decrypt_to_device_event(&event).await.unwrap(); + let (session, _, _) = bob.decrypt_to_device_event(&event).await.unwrap(); + bob.store.save_sessions(&[session]).await.unwrap(); (alice, bob) } @@ -1492,13 +1517,15 @@ pub(crate) mod test { content: bob_device .encrypt(EventType::Dummy, json!({})) .await - .unwrap(), + .unwrap() + .1, }; let event = bob .decrypt_to_device_event(&event) .await .unwrap() + .1 .deserialize() .unwrap(); @@ -1534,12 +1561,14 @@ pub(crate) mod test { .get_outbound_group_session(&room_id) .unwrap(); - let event = bob - .decrypt_to_device_event(&event) + let (session, event, group_session) = bob.decrypt_to_device_event(&event).await.unwrap(); + + bob.store.save_sessions(&[session]).await.unwrap(); + bob.store + .save_inbound_group_sessions(&[group_session.unwrap()]) .await - .unwrap() - .deserialize() .unwrap(); + let event = event.deserialize().unwrap(); if let AnyToDeviceEvent::RoomKey(event) = event { assert_eq!(&event.sender, alice.user_id()); @@ -1579,7 +1608,11 @@ pub(crate) mod test { content: to_device_requests_to_content(to_device_requests), }; - bob.decrypt_to_device_event(&event).await.unwrap(); + let (_, _, group_session) = bob.decrypt_to_device_event(&event).await.unwrap(); + bob.store + .save_inbound_group_sessions(&[group_session.unwrap()]) + .await + .unwrap(); let plaintext = "It is a secret to everybody"; diff --git a/matrix_sdk_crypto/src/olm/account.rs b/matrix_sdk_crypto/src/olm/account.rs index 0960038b..422d127f 100644 --- a/matrix_sdk_crypto/src/olm/account.rs +++ b/matrix_sdk_crypto/src/olm/account.rs @@ -52,7 +52,7 @@ use olm_rs::{ use crate::{ error::{EventError, OlmResult, SessionCreationError}, identities::ReadOnlyDevice, - store::{Result as StoreResult, Store}, + store::Store, OlmError, }; @@ -76,7 +76,7 @@ impl Account { pub async fn decrypt_to_device_event( &self, event: &ToDeviceEvent, - ) -> OlmResult<(Raw, String, String)> { + ) -> OlmResult<(Session, Raw, String, String)> { debug!("Decrypting to-device event"); let content = if let EncryptedEventContent::OlmV1Curve25519AesSha2(c) = &event.content { @@ -103,27 +103,28 @@ impl Account { .map_err(|_| EventError::UnsupportedOlmType)?; // Decrypt the OlmMessage and get a Ruma event out of it. - let (decrypted_event, signing_key) = self + let (session, decrypted_event, signing_key) = self .decrypt_olm_message(&event.sender, &content.sender_key, message) .await?; debug!("Decrypted a to-device event {:?}", decrypted_event); - Ok((decrypted_event, content.sender_key.clone(), signing_key)) + Ok(( + session, + decrypted_event, + content.sender_key.clone(), + signing_key, + )) } else { warn!("Olm event doesn't contain a ciphertext for our key"); Err(EventError::MissingCiphertext.into()) } } - pub async fn update_uploaded_key_count( - &self, - key_count: &BTreeMap, - ) -> StoreResult<()> { + pub async fn update_uploaded_key_count(&self, key_count: &BTreeMap) { let one_time_key_count = key_count.get(&DeviceKeyAlgorithm::SignedCurve25519); let count: u64 = one_time_key_count.map_or(0, |c| (*c).into()); self.inner.update_uploaded_key_count(count); - self.store.save_account(self.inner.clone()).await } pub async fn receive_keys_upload_response( @@ -161,7 +162,7 @@ impl Account { sender: &UserId, sender_key: &str, message: &OlmMessage, - ) -> OlmResult> { + ) -> OlmResult> { let s = self.store.get_sessions(sender_key).await?; // We don't have any existing sessions, return early. @@ -171,8 +172,7 @@ impl Account { return Ok(None); }; - let mut session_to_save = None; - let mut plaintext = None; + let mut decrypted: Option<(Session, String)> = None; for session in &mut *sessions.lock().await { let mut matches = false; @@ -191,9 +191,7 @@ impl Account { match ret { Ok(p) => { - plaintext = Some(p); - session_to_save = Some(session.clone()); - + decrypted = Some((session.clone(), p)); break; } Err(e) => { @@ -214,14 +212,7 @@ impl Account { } } - if let Some(session) = session_to_save { - // Decryption was successful, save the new ratchet state of the - // session that was used to decrypt the message. - trace!("Saved the new session state for {}", sender); - self.store.save_sessions(&[session]).await?; - } - - Ok(plaintext) + Ok(decrypted) } /// Decrypt an Olm message, creating a new Olm session if possible. @@ -230,15 +221,15 @@ impl Account { sender: &UserId, sender_key: &str, message: OlmMessage, - ) -> OlmResult<(Raw, String)> { + ) -> OlmResult<(Session, Raw, String)> { // First try to decrypt using an existing session. - let plaintext = if let Some(p) = self + let (session, plaintext) = if let Some(d) = self .try_decrypt_olm_message(sender, sender_key, &message) .await? { - // Decryption succeeded, de-structure the plaintext out of the - // Option. - p + // Decryption succeeded, de-structure the session/plaintext out of + // the Option. + d } else { // Decryption failed with every known session, let's try to create a // new session. @@ -278,9 +269,6 @@ impl Account { } }; - // Save the account since we remove the one-time key that - // was used to create this session. - self.store.save_account(self.inner.clone()).await?; session } }; @@ -288,15 +276,23 @@ impl Account { // Decrypt our message, this shouldn't fail since we're using a // newly created Session. let plaintext = session.decrypt(message).await?; - - // Save the new ratcheted state of the session. - self.store.save_sessions(&[session]).await?; - plaintext + (session, plaintext) }; trace!("Successfully decrypted a Olm message: {}", plaintext); - self.parse_decrypted_to_device_event(sender, &plaintext) + let (event, signing_key) = match self.parse_decrypted_to_device_event(sender, &plaintext) { + Ok(r) => r, + Err(e) => { + // We might created a new session but decryption might still + // have failed, store it for the error case here, this is fine + // since we don't expect this to happen often or at all. + self.store.save_sessions(&[session]).await?; + return Err(e); + } + }; + + Ok((session, event, signing_key)) } /// Parse a decrypted Olm message, check that the plaintext and encrypted diff --git a/matrix_sdk_crypto/src/session_manager/group_sessions.rs b/matrix_sdk_crypto/src/session_manager/group_sessions.rs index d20f8f8e..374610fd 100644 --- a/matrix_sdk_crypto/src/session_manager/group_sessions.rs +++ b/matrix_sdk_crypto/src/session_manager/group_sessions.rs @@ -28,8 +28,8 @@ use tracing::{debug, info}; use crate::{ error::{EventError, MegolmResult, OlmResult}, - olm::{Account, OutboundGroupSession}, - store::Store, + olm::{Account, InboundGroupSession, OutboundGroupSession}, + store::{Changes, Store}, Device, EncryptionSettings, OlmError, ToDeviceRequest, }; @@ -140,19 +140,17 @@ impl GroupSessionManager { &self, room_id: &RoomId, settings: EncryptionSettings, - ) -> OlmResult<()> { + ) -> OlmResult<(OutboundGroupSession, InboundGroupSession)> { let (outbound, inbound) = self .account .create_group_session_pair(room_id, settings) .await .map_err(|_| EventError::UnsupportedAlgorithm)?; - let _ = self.store.save_inbound_group_sessions(&[inbound]).await?; - let _ = self .outbound_group_sessions - .insert(room_id.to_owned(), outbound); - Ok(()) + .insert(room_id.to_owned(), outbound.clone()); + Ok((outbound, inbound)) } /// Get to-device requests to share a group session with users in a room. @@ -169,13 +167,12 @@ impl GroupSessionManager { users: impl Iterator, encryption_settings: impl Into, ) -> OlmResult>> { - self.create_outbound_group_session(room_id, encryption_settings.into()) - .await?; - let session = self.outbound_group_sessions.get(room_id).unwrap(); + let mut changes = Changes::default(); - if session.shared() { - panic!("Session is already shared"); - } + let (session, inbound_session) = self + .create_outbound_group_session(room_id, encryption_settings.into()) + .await?; + changes.inbound_group_sessions.push(inbound_session); let mut devices: Vec = Vec::new(); @@ -196,7 +193,7 @@ impl GroupSessionManager { .encrypt(EventType::RoomKey, key_content.clone()) .await; - let encrypted = match encrypted { + let (used_session, encrypted) = match encrypted { Ok(c) => c, Err(OlmError::MissingSession) | Err(OlmError::EventError(EventError::MissingSenderKey)) => { @@ -205,6 +202,8 @@ impl GroupSessionManager { Err(e) => return Err(e), }; + changes.sessions.push(used_session); + messages .entry(device.user_id().clone()) .or_insert_with(BTreeMap::new) @@ -237,6 +236,8 @@ impl GroupSessionManager { session.mark_as_shared(); } + self.store.save_changes(changes).await?; + Ok(requests) } } diff --git a/matrix_sdk_crypto/src/session_manager/sessions.rs b/matrix_sdk_crypto/src/session_manager/sessions.rs index ab3976ab..c86c09ac 100644 --- a/matrix_sdk_crypto/src/session_manager/sessions.rs +++ b/matrix_sdk_crypto/src/session_manager/sessions.rs @@ -33,7 +33,7 @@ use crate::{ key_request::KeyRequestMachine, olm::Account, requests::{OutgoingRequest, ToDeviceRequest}, - store::{Result as StoreResult, Store}, + store::{Changes, Result as StoreResult, Store}, ReadOnlyDevice, }; @@ -128,7 +128,7 @@ impl SessionManager { .is_some() { if let Some(device) = self.store.get_device(user_id, device_id).await? { - let content = device.encrypt(EventType::Dummy, json!({})).await?; + let (_, content) = device.encrypt(EventType::Dummy, json!({})).await?; let id = Uuid::new_v4(); let mut messages = BTreeMap::new(); @@ -258,6 +258,8 @@ impl SessionManager { pub async fn receive_keys_claim_response(&self, response: &KeysClaimResponse) -> OlmResult<()> { // TODO log the failures here + let mut changes = Changes::default(); + for (user_id, user_devices) in &response.one_time_keys { for (device_id, key_map) in user_devices { let device = match self.store.get_readonly_device(&user_id, device_id).await { @@ -284,15 +286,12 @@ impl SessionManager { let session = match self.account.create_outbound_session(device, &key_map).await { Ok(s) => s, Err(e) => { - warn!("{:?}", e); + warn!("Error creating new outbound session {:?}", e); continue; } }; - if let Err(e) = self.store.save_sessions(&[session]).await { - error!("Failed to store newly created Olm session {}", e); - continue; - } + changes.sessions.push(session); self.key_request_machine.retry_keyshare(&user_id, device_id); @@ -304,7 +303,8 @@ impl SessionManager { } } } - Ok(()) + + Ok(self.store.save_changes(changes).await?) } } diff --git a/matrix_sdk_crypto/src/store/memorystore.rs b/matrix_sdk_crypto/src/store/memorystore.rs index 2e5a8762..f41d4579 100644 --- a/matrix_sdk_crypto/src/store/memorystore.rs +++ b/matrix_sdk_crypto/src/store/memorystore.rs @@ -26,7 +26,7 @@ use matrix_sdk_common_macros::async_trait; use super::{ caches::{DeviceStore, GroupSessionStore, SessionStore}, - CryptoStore, InboundGroupSession, ReadOnlyAccount, Result, Session, + Changes, CryptoStore, InboundGroupSession, ReadOnlyAccount, Result, Session, }; use crate::identities::{ReadOnlyDevice, UserIdentities}; @@ -61,6 +61,30 @@ impl MemoryStore { pub fn new() -> Self { Self::default() } + + pub(crate) async fn save_devices(&self, mut devices: Vec) { + for device in devices.drain(..) { + let _ = self.devices.add(device); + } + } + + async fn delete_devices(&self, mut devices: Vec) { + for device in devices.drain(..) { + let _ = self.devices.remove(device.user_id(), device.device_id()); + } + } + + async fn save_sessions(&self, mut sessions: Vec) { + for session in sessions.drain(..) { + let _ = self.sessions.add(session.clone()).await; + } + } + + async fn save_inbound_group_sessions(&self, mut sessions: Vec) { + for session in sessions.drain(..) { + self.inbound_group_sessions.add(session); + } + } } #[async_trait] @@ -73,9 +97,24 @@ impl CryptoStore for MemoryStore { Ok(()) } - async fn save_sessions(&self, sessions: &[Session]) -> Result<()> { - for session in sessions { - let _ = self.sessions.add(session.clone()).await; + async fn save_changes(&self, mut changes: Changes) -> Result<()> { + self.save_sessions(changes.sessions).await; + self.save_inbound_group_sessions(changes.inbound_group_sessions) + .await; + + self.save_devices(changes.devices.new).await; + self.save_devices(changes.devices.changed).await; + self.delete_devices(changes.devices.deleted).await; + + for identity in changes + .identities + .new + .drain(..) + .chain(changes.identities.changed) + { + let _ = self + .identities + .insert(identity.user_id().to_owned(), identity.clone()); } Ok(()) @@ -85,14 +124,6 @@ impl CryptoStore for MemoryStore { Ok(self.sessions.get(sender_key)) } - async fn save_inbound_group_sessions(&self, sessions: &[InboundGroupSession]) -> Result<()> { - for session in sessions { - self.inbound_group_sessions.add(session.clone()); - } - - Ok(()) - } - async fn get_inbound_group_session( &self, room_id: &RoomId, @@ -151,11 +182,6 @@ impl CryptoStore for MemoryStore { Ok(self.devices.get(user_id, device_id)) } - async fn delete_device(&self, device: ReadOnlyDevice) -> Result<()> { - let _ = self.devices.remove(device.user_id(), device.device_id()); - Ok(()) - } - async fn get_user_devices( &self, user_id: &UserId, @@ -163,28 +189,11 @@ impl CryptoStore for MemoryStore { Ok(self.devices.user_devices(user_id)) } - async fn save_devices(&self, devices: &[ReadOnlyDevice]) -> Result<()> { - for device in devices { - let _ = self.devices.add(device.clone()); - } - - Ok(()) - } - async fn get_user_identity(&self, user_id: &UserId) -> Result> { #[allow(clippy::map_clone)] Ok(self.identities.get(user_id).map(|i| i.clone())) } - async fn save_user_identities(&self, identities: &[UserIdentities]) -> Result<()> { - for identity in identities { - let _ = self - .identities - .insert(identity.user_id().to_owned(), identity.clone()); - } - Ok(()) - } - async fn save_value(&self, key: String, value: String) -> Result<()> { self.values.insert(key, value); Ok(()) @@ -217,7 +226,7 @@ mod test { assert!(store.load_account().await.unwrap().is_none()); store.save_account(account).await.unwrap(); - store.save_sessions(&[session.clone()]).await.unwrap(); + store.save_sessions(vec![session.clone()]).await; let sessions = store .get_sessions(&session.sender_key) @@ -250,9 +259,8 @@ mod test { let store = MemoryStore::new(); let _ = store - .save_inbound_group_sessions(&[inbound.clone()]) - .await - .unwrap(); + .save_inbound_group_sessions(vec![inbound.clone()]) + .await; let loaded_session = store .get_inbound_group_session(&room_id, "test_key", outbound.session_id()) @@ -267,7 +275,7 @@ mod test { let device = get_device(); let store = MemoryStore::new(); - store.save_devices(&[device.clone()]).await.unwrap(); + store.save_devices(vec![device.clone()]).await; let loaded_device = store .get_device(device.user_id(), device.device_id()) @@ -286,7 +294,7 @@ mod test { assert_eq!(&device, loaded_device); - store.delete_device(device.clone()).await.unwrap(); + store.delete_devices(vec![device.clone()]).await; assert!(store .get_device(device.user_id(), device.device_id()) .await diff --git a/matrix_sdk_crypto/src/store/mod.rs b/matrix_sdk_crypto/src/store/mod.rs index b8b661ef..478cb709 100644 --- a/matrix_sdk_crypto/src/store/mod.rs +++ b/matrix_sdk_crypto/src/store/mod.rs @@ -100,6 +100,31 @@ pub(crate) struct Store { verification_machine: VerificationMachine, } +#[derive(Debug, Default)] +#[allow(missing_docs)] +pub struct Changes { + pub account: Option, + pub sessions: Vec, + pub inbound_group_sessions: Vec, + pub identities: IdentityChanges, + pub devices: DeviceChanges, +} + +#[derive(Debug, Clone, Default)] +#[allow(missing_docs)] +pub struct IdentityChanges { + pub new: Vec, + pub changed: Vec, +} + +#[derive(Debug, Clone, Default)] +#[allow(missing_docs)] +pub struct DeviceChanges { + pub new: Vec, + pub changed: Vec, + pub deleted: Vec, +} + impl Store { pub fn new( user_id: Arc, @@ -121,6 +146,41 @@ impl Store { self.inner.get_device(user_id, device_id).await } + pub async fn save_sessions(&self, sessions: &[Session]) -> Result<()> { + let changes = Changes { + sessions: sessions.to_vec(), + ..Default::default() + }; + + self.save_changes(changes).await + } + + #[cfg(test)] + pub async fn save_devices(&self, devices: &[ReadOnlyDevice]) -> Result<()> { + let changes = Changes { + devices: DeviceChanges { + changed: devices.to_vec(), + ..Default::default() + }, + ..Default::default() + }; + + self.save_changes(changes).await + } + + #[cfg(test)] + pub async fn save_inbound_group_sessions( + &self, + sessions: &[InboundGroupSession], + ) -> Result<()> { + let changes = Changes { + inbound_group_sessions: sessions.to_vec(), + ..Default::default() + }; + + self.save_changes(changes).await + } + pub async fn get_readonly_devices( &self, user_id: &UserId, @@ -271,12 +331,8 @@ pub trait CryptoStore: Debug { /// * `account` - The account that should be stored. async fn save_account(&self, account: ReadOnlyAccount) -> Result<()>; - /// Save the given sessions in the store. - /// - /// # Arguments - /// - /// * `session` - The sessions that should be stored. - async fn save_sessions(&self, session: &[Session]) -> Result<()>; + /// TODO + async fn save_changes(&self, changes: Changes) -> Result<()>; /// Get all the sessions that belong to the given sender key. /// @@ -285,13 +341,6 @@ pub trait CryptoStore: Debug { /// * `sender_key` - The sender key that was used to establish the sessions. async fn get_sessions(&self, sender_key: &str) -> Result>>>>; - /// Save the given inbound group sessions in the store. - /// - /// # Arguments - /// - /// * `sessions` - The sessions that should be stored. - async fn save_inbound_group_sessions(&self, session: &[InboundGroupSession]) -> Result<()>; - /// Get the inbound group session from our store. /// /// # Arguments @@ -331,20 +380,6 @@ pub trait CryptoStore: Debug { /// * `dirty` - Should the user be also marked for a key query. async fn update_tracked_user(&self, user: &UserId, dirty: bool) -> Result; - /// Save the given devices in the store. - /// - /// # Arguments - /// - /// * `device` - The device that should be stored. - async fn save_devices(&self, devices: &[ReadOnlyDevice]) -> Result<()>; - - /// Delete the given device from the store. - /// - /// # Arguments - /// - /// * `device` - The device that should be stored. - async fn delete_device(&self, device: ReadOnlyDevice) -> Result<()>; - /// Get the device for the given user with the given device id. /// /// # Arguments @@ -368,13 +403,6 @@ pub trait CryptoStore: Debug { user_id: &UserId, ) -> Result>; - /// Save the given user identities in the store. - /// - /// # Arguments - /// - /// * `identities` - The identities that should be saved in the store. - async fn save_user_identities(&self, identities: &[UserIdentities]) -> Result<()>; - /// Get the user identity that is attached to the given user id. /// /// # Arguments diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index f591af79..6759abb5 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -34,7 +34,7 @@ use matrix_sdk_common::{ use sqlx::{query, query_as, sqlite::SqliteConnectOptions, Connection, Executor, SqliteConnection}; use zeroize::Zeroizing; -use super::{caches::SessionStore, CryptoStore, CryptoStoreError, Result}; +use super::{caches::SessionStore, Changes, CryptoStore, CryptoStoreError, Result}; use crate::{ identities::{LocalTrust, OwnUserIdentity, ReadOnlyDevice, UserIdentities, UserIdentity}, olm::{ @@ -456,11 +456,17 @@ impl SqliteStore { Ok(()) } - async fn lazy_load_sessions(&self, sender_key: &str) -> Result<()> { + async fn lazy_load_sessions( + &self, + connection: &mut SqliteConnection, + sender_key: &str, + ) -> Result<()> { let loaded_sessions = self.sessions.get(sender_key).is_some(); if !loaded_sessions { - let sessions = self.load_sessions_for(sender_key).await?; + let sessions = self + .load_sessions_for_helper(connection, sender_key) + .await?; if !sessions.is_empty() { self.sessions.set_for_sender(sender_key, sessions); @@ -470,20 +476,33 @@ impl SqliteStore { Ok(()) } - async fn get_sessions_for(&self, sender_key: &str) -> Result>>>> { - self.lazy_load_sessions(sender_key).await?; + async fn get_sessions_for( + &self, + connection: &mut SqliteConnection, + sender_key: &str, + ) -> Result>>>> { + self.lazy_load_sessions(connection, sender_key).await?; Ok(self.sessions.get(sender_key)) } + #[cfg(test)] async fn load_sessions_for(&self, sender_key: &str) -> Result> { + let mut connection = self.connection.lock().await; + self.load_sessions_for_helper(&mut connection, sender_key) + .await + } + + async fn load_sessions_for_helper( + &self, + connection: &mut SqliteConnection, + sender_key: &str, + ) -> Result> { let account_info = self .account_info .lock() .unwrap() .clone() .ok_or(CryptoStoreError::AccountUnset)?; - let mut connection = self.connection.lock().await; - let mut rows: Vec<(String, String, String, String)> = query_as( "SELECT pickle, sender_key, creation_time, last_use_time FROM sessions WHERE account_id = ? and sender_key = ?", @@ -1231,6 +1250,134 @@ impl SqliteStore { Ok(()) } + #[cfg(test)] + async fn save_sessions(&self, sessions: &[Session]) -> Result<()> { + let mut connection = self.connection.lock().await; + let mut transaction = connection.begin().await?; + + self.save_sessions_helper(&mut transaction, sessions) + .await?; + transaction.commit().await?; + + Ok(()) + } + + async fn save_sessions_helper( + &self, + connection: &mut SqliteConnection, + sessions: &[Session], + ) -> Result<()> { + let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; + + for session in sessions { + self.lazy_load_sessions(connection, &session.sender_key) + .await?; + } + + for session in sessions { + self.sessions.add(session.clone()).await; + + let pickle = session.pickle(self.get_pickle_mode()).await; + + let session_id = session.session_id(); + let creation_time = serde_json::to_string(&pickle.creation_time)?; + let last_use_time = serde_json::to_string(&pickle.last_use_time)?; + + query( + "REPLACE INTO sessions ( + session_id, account_id, creation_time, last_use_time, sender_key, pickle + ) VALUES (?, ?, ?, ?, ?, ?)", + ) + .bind(&session_id) + .bind(&account_id) + .bind(&*creation_time) + .bind(&*last_use_time) + .bind(&pickle.sender_key) + .bind(&pickle.pickle.as_str()) + .execute(&mut *connection) + .await?; + } + + Ok(()) + } + + async fn save_devices( + &self, + mut connection: &mut SqliteConnection, + devices: &[ReadOnlyDevice], + ) -> Result<()> { + for device in devices { + self.save_device_helper(&mut connection, device.clone()) + .await? + } + + Ok(()) + } + + async fn delete_devices( + &self, + connection: &mut SqliteConnection, + devices: &[ReadOnlyDevice], + ) -> Result<()> { + let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; + + for device in devices { + query( + "DELETE FROM devices + WHERE account_id = ?1 and user_id = ?2 and device_id = ?3 + ", + ) + .bind(account_id) + .bind(&device.user_id().to_string()) + .bind(device.device_id().as_str()) + .execute(&mut *connection) + .await?; + } + + Ok(()) + } + + #[cfg(test)] + async fn save_inbound_group_sessions_test( + &self, + sessions: &[InboundGroupSession], + ) -> Result<()> { + let mut connection = self.connection.lock().await; + let mut transaction = connection.begin().await?; + + self.save_inbound_group_sessions(&mut transaction, sessions) + .await?; + + transaction.commit().await?; + Ok(()) + } + + async fn save_inbound_group_sessions( + &self, + connection: &mut SqliteConnection, + sessions: &[InboundGroupSession], + ) -> Result<()> { + let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; + + for session in sessions { + self.save_inbound_group_session_helper(account_id, connection, session) + .await?; + } + + Ok(()) + } + + async fn save_user_identities( + &self, + mut connection: &mut SqliteConnection, + users: &[UserIdentities], + ) -> Result<()> { + for user in users { + self.save_user_helper(&mut connection, user).await?; + } + Ok(()) + } + async fn save_user_helper( &self, mut connection: &mut SqliteConnection, @@ -1369,39 +1516,26 @@ impl CryptoStore for SqliteStore { Ok(()) } - async fn save_sessions(&self, sessions: &[Session]) -> Result<()> { - let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; - - for session in sessions { - self.lazy_load_sessions(&session.sender_key).await?; - } - + async fn save_changes(&self, changes: Changes) -> Result<()> { let mut connection = self.connection.lock().await; let mut transaction = connection.begin().await?; - for session in sessions { - self.sessions.add(session.clone()).await; - - let pickle = session.pickle(self.get_pickle_mode()).await; - - let session_id = session.session_id(); - let creation_time = serde_json::to_string(&pickle.creation_time)?; - let last_use_time = serde_json::to_string(&pickle.last_use_time)?; - - query( - "REPLACE INTO sessions ( - session_id, account_id, creation_time, last_use_time, sender_key, pickle - ) VALUES (?, ?, ?, ?, ?, ?)", - ) - .bind(&session_id) - .bind(&account_id) - .bind(&*creation_time) - .bind(&*last_use_time) - .bind(&pickle.sender_key) - .bind(&pickle.pickle.as_str()) - .execute(&mut *transaction) + self.save_sessions_helper(&mut transaction, &changes.sessions) + .await?; + self.save_inbound_group_sessions(&mut transaction, &changes.inbound_group_sessions) + .await?; + + self.save_devices(&mut transaction, &changes.devices.new) + .await?; + self.save_devices(&mut transaction, &changes.devices.changed) + .await?; + self.delete_devices(&mut transaction, &changes.devices.deleted) + .await?; + + self.save_user_identities(&mut transaction, &changes.identities.new) + .await?; + self.save_user_identities(&mut transaction, &changes.identities.changed) .await?; - } transaction.commit().await?; @@ -1409,22 +1543,8 @@ impl CryptoStore for SqliteStore { } async fn get_sessions(&self, sender_key: &str) -> Result>>>> { - Ok(self.get_sessions_for(sender_key).await?) - } - - async fn save_inbound_group_sessions(&self, sessions: &[InboundGroupSession]) -> Result<()> { - let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; let mut connection = self.connection.lock().await; - let mut transaction = connection.begin().await?; - - for session in sessions { - self.save_inbound_group_session_helper(account_id, &mut transaction, session) - .await?; - } - - transaction.commit().await?; - - Ok(()) + Ok(self.get_sessions_for(&mut connection, sender_key).await?) } async fn get_inbound_group_session( @@ -1469,38 +1589,6 @@ impl CryptoStore for SqliteStore { Ok(already_added) } - async fn save_devices(&self, devices: &[ReadOnlyDevice]) -> Result<()> { - let mut connection = self.connection.lock().await; - let mut transaction = connection.begin().await?; - - for device in devices { - self.save_device_helper(&mut transaction, device.clone()) - .await? - } - - transaction.commit().await?; - - Ok(()) - } - - async fn delete_device(&self, device: ReadOnlyDevice) -> Result<()> { - let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; - let mut connection = self.connection.lock().await; - - query( - "DELETE FROM devices - WHERE account_id = ?1 and user_id = ?2 and device_id = ?3 - ", - ) - .bind(account_id) - .bind(&device.user_id().to_string()) - .bind(device.device_id().as_str()) - .execute(&mut *connection) - .await?; - - Ok(()) - } - async fn get_device( &self, user_id: &UserId, @@ -1520,19 +1608,6 @@ impl CryptoStore for SqliteStore { self.load_user(user_id).await } - async fn save_user_identities(&self, users: &[UserIdentities]) -> Result<()> { - let mut connection = self.connection.lock().await; - let mut transaction = connection.begin().await?; - - for user in users { - self.save_user_helper(&mut transaction, user).await?; - } - - transaction.commit().await?; - - Ok(()) - } - async fn save_value(&self, key: String, value: String) -> Result<()> { let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; let mut connection = self.connection.lock().await; @@ -1598,6 +1673,7 @@ mod test { user::test::{get_other_identity, get_own_identity}, }, olm::{GroupSessionKey, InboundGroupSession, ReadOnlyAccount, Session}, + store::{Changes, DeviceChanges, IdentityChanges}, }; use matrix_sdk_common::{ api::r0::keys::SignedKey, @@ -1854,7 +1930,7 @@ mod test { .expect("Can't create session"); store - .save_inbound_group_sessions(&[session]) + .save_inbound_group_sessions_test(&[session]) .await .expect("Can't save group session"); } @@ -1880,7 +1956,7 @@ mod test { let session = InboundGroupSession::from_export(export).unwrap(); store - .save_inbound_group_sessions(&[session.clone()]) + .save_inbound_group_sessions_test(&[session.clone()]) .await .expect("Can't save group session"); @@ -1952,7 +2028,15 @@ mod test { let (_account, store, dir) = get_loaded_store().await; let device = get_device(); - store.save_devices(&[device.clone()]).await.unwrap(); + let changes = Changes { + devices: DeviceChanges { + changed: vec![device.clone()], + ..Default::default() + }, + ..Default::default() + }; + + store.save_changes(changes).await.unwrap(); drop(store); @@ -1986,8 +2070,25 @@ mod test { let (_account, store, dir) = get_loaded_store().await; let device = get_device(); - store.save_devices(&[device.clone()]).await.unwrap(); - store.delete_device(device.clone()).await.unwrap(); + let changes = Changes { + devices: DeviceChanges { + changed: vec![device.clone()], + ..Default::default() + }, + ..Default::default() + }; + + store.save_changes(changes).await.unwrap(); + + let changes = Changes { + devices: DeviceChanges { + deleted: vec![device.clone()], + ..Default::default() + }, + ..Default::default() + }; + + store.save_changes(changes).await.unwrap(); let store = SqliteStore::open(&alice_id(), &alice_device_id(), dir.path()) .await @@ -2024,8 +2125,16 @@ mod test { let own_identity = get_own_identity(); + let changes = Changes { + identities: IdentityChanges { + changed: vec![own_identity.clone().into()], + ..Default::default() + }, + ..Default::default() + }; + store - .save_user_identities(&[own_identity.clone().into()]) + .save_changes(changes) .await .expect("Can't save identity"); @@ -2052,10 +2161,15 @@ mod test { let other_identity = get_other_identity(); - store - .save_user_identities(&[other_identity.clone().into()]) - .await - .unwrap(); + let changes = Changes { + identities: IdentityChanges { + changed: vec![other_identity.clone().into()], + ..Default::default() + }, + ..Default::default() + }; + + store.save_changes(changes).await.unwrap(); let loaded_user = store .load_user(other_identity.user_id()) @@ -2072,10 +2186,15 @@ mod test { own_identity.mark_as_verified(); - store - .save_user_identities(&[own_identity.into()]) - .await - .unwrap(); + let changes = Changes { + identities: IdentityChanges { + changed: vec![own_identity.into()], + ..Default::default() + }, + ..Default::default() + }; + + store.save_changes(changes).await.unwrap(); let loaded_user = store.load_user(&user_id).await.unwrap().unwrap(); assert!(loaded_user.own().unwrap().is_verified()) } diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index ee12d562..71e3cb19 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -194,18 +194,14 @@ impl VerificationMachine { self.receive_event_helper(&s, event); if s.is_done() { - if !s.mark_device_as_verified().await? { - if let Some(r) = s.cancel() { - self.outgoing_to_device_messages.insert( - r.txn_id, - OutgoingRequest { - request_id: r.txn_id, - request: Arc::new(r.into()), - }, - ); - } - } else { - s.mark_identity_as_verified().await?; + if let Some(r) = s.mark_as_done().await? { + self.outgoing_to_device_messages.insert( + r.txn_id, + OutgoingRequest { + request_id: r.txn_id, + request: Arc::new(r.into()), + }, + ); } } }; @@ -258,17 +254,15 @@ mod test { let alice = ReadOnlyAccount::new(&alice_id(), &alice_device_id()); let bob = ReadOnlyAccount::new(&bob_id(), &bob_device_id()); let store = MemoryStore::new(); - let bob_store: Arc> = Arc::new(Box::new(MemoryStore::new())); + let bob_store = MemoryStore::new(); let bob_device = ReadOnlyDevice::from_account(&bob).await; let alice_device = ReadOnlyDevice::from_account(&alice).await; - store.save_devices(&[bob_device]).await.unwrap(); - bob_store - .save_devices(&[alice_device.clone()]) - .await - .unwrap(); + store.save_devices(vec![bob_device]).await; + bob_store.save_devices(vec![alice_device.clone()]).await; + let bob_store: Arc> = Arc::new(Box::new(bob_store)); let machine = VerificationMachine::new(alice, Arc::new(Box::new(store))); let (bob_sas, start_content) = Sas::start(bob, alice_device, bob_store, None); machine diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index 95d61541..b71070a7 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -34,7 +34,7 @@ use matrix_sdk_common::{ use crate::{ identities::{LocalTrust, ReadOnlyDevice, UserIdentities}, - store::{CryptoStore, CryptoStoreError}, + store::{Changes, CryptoStore, CryptoStoreError, DeviceChanges}, ReadOnlyAccount, ToDeviceRequest, }; @@ -189,34 +189,64 @@ impl Sas { (content, guard.is_done()) }; - if done { - // TODO move the logic that marks and stores the device into the - // else branch and only after the identity was verified as well. We - // dont' want to verify one without the other. - if !self.mark_device_as_verified().await? { - return Ok(self.cancel()); - } else { - self.mark_identity_as_verified().await?; - } - } + let cancel = if done { + self.mark_as_done().await? + } else { + None + }; - Ok(content.map(|c| { - let content = AnyToDeviceEventContent::KeyVerificationMac(c); - self.content_to_request(content) - })) + if cancel.is_some() { + Ok(cancel) + } else { + Ok(content.map(|c| { + let content = AnyToDeviceEventContent::KeyVerificationMac(c); + self.content_to_request(content) + })) + } } - pub(crate) async fn mark_identity_as_verified(&self) -> Result { + pub(crate) async fn mark_as_done(&self) -> Result, CryptoStoreError> { + if let Some(device) = self.mark_device_as_verified().await? { + let identity = self.mark_identity_as_verified().await?; + + let mut changes = Changes { + devices: DeviceChanges { + changed: vec![device], + ..Default::default() + }, + ..Default::default() + }; + + if let Some(i) = identity { + changes.identities.changed.push(i); + } + + self.store.save_changes(changes).await?; + Ok(None) + } else { + Ok(self.cancel()) + } + } + + pub(crate) async fn mark_identity_as_verified( + &self, + ) -> Result, CryptoStoreError> { // If there wasn't an identity available during the verification flow // return early as there's nothing to do. if self.other_identity.is_none() { - return Ok(false); + return Ok(None); } + // TODO signal an error, e.g. when the identity got deleted so we don't + // verify/save the device either. let identity = self.store.get_user_identity(self.other_user_id()).await?; if let Some(identity) = identity { - if identity.master_key() == self.other_identity.as_ref().unwrap().master_key() { + if self + .other_identity + .as_ref() + .map_or(false, |i| i.master_key() == identity.master_key()) + { if self .verified_identities() .map_or(false, |i| i.contains(&identity)) @@ -228,13 +258,12 @@ impl Sas { if let UserIdentities::Own(i) = &identity { i.mark_as_verified(); - self.store.save_user_identities(&[identity]).await?; } // TODO if we have the private part of the user signing // key we should sign and upload a signature for this // identity. - Ok(true) + Ok(Some(identity)) } else { info!( "The interactive verification process didn't contain a \ @@ -243,7 +272,7 @@ impl Sas { self.verified_identities(), ); - Ok(false) + Ok(None) } } else { warn!( @@ -252,7 +281,7 @@ impl Sas { identity.user_id(), ); - Ok(false) + Ok(None) } } else { info!( @@ -260,11 +289,13 @@ impl Sas { verification was going on.", self.other_user_id(), ); - Ok(false) + Ok(None) } } - pub(crate) async fn mark_device_as_verified(&self) -> Result { + pub(crate) async fn mark_device_as_verified( + &self, + ) -> Result, CryptoStoreError> { let device = self .store .get_device(self.other_user_id(), self.other_device_id()) @@ -283,12 +314,11 @@ impl Sas { ); device.set_trust_state(LocalTrust::Verified); - self.store.save_devices(&[device]).await?; // TODO if this is a device from our own user and we have // the private part of the self signing key, we should sign // the device and upload the signature. - Ok(true) + Ok(Some(device)) } else { info!( "The interactive verification process didn't contain a \ @@ -297,7 +327,7 @@ impl Sas { device.device_id() ); - Ok(false) + Ok(None) } } else { warn!( @@ -306,7 +336,7 @@ impl Sas { device.user_id(), device.device_id() ); - Ok(false) + Ok(None) } } else { let device = self.other_device(); @@ -317,7 +347,7 @@ impl Sas { device.user_id(), device.device_id() ); - Ok(false) + Ok(None) } } @@ -777,12 +807,11 @@ mod test { let bob_device = ReadOnlyDevice::from_account(&bob).await; let alice_store: Arc> = Arc::new(Box::new(MemoryStore::new())); - let bob_store: Arc> = Arc::new(Box::new(MemoryStore::new())); + let bob_store = MemoryStore::new(); - bob_store - .save_devices(&[alice_device.clone()]) - .await - .unwrap(); + bob_store.save_devices(vec![alice_device.clone()]).await; + + let bob_store: Arc> = Arc::new(Box::new(bob_store)); let (alice, content) = Sas::start(alice, bob_device, alice_store, None); let event = wrap_to_device_event(alice.user_id(), content); From 6a7da5a8b6edb3439a92f82449d04ab31f19ee0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 21 Oct 2020 12:55:45 +0200 Subject: [PATCH 22/34] crypto: Correctly generate a random nonce for pickling of the signing objects. --- matrix_sdk_crypto/src/olm/signing.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/matrix_sdk_crypto/src/olm/signing.rs b/matrix_sdk_crypto/src/olm/signing.rs index 8395db9c..633a18b9 100644 --- a/matrix_sdk_crypto/src/olm/signing.rs +++ b/matrix_sdk_crypto/src/olm/signing.rs @@ -19,6 +19,7 @@ use aes_gcm::{ Aes256Gcm, }; use base64::{decode_config, encode_config, DecodeError, URL_SAFE_NO_PAD}; +use getrandom::getrandom; use serde::{Deserialize, Serialize}; use std::{ collections::BTreeMap, @@ -42,6 +43,8 @@ use crate::{ requests::UploadSigningKeysRequest, }; +const NONCE_SIZE: usize = 12; + fn encode>(input: T) -> String { encode_config(input, URL_SAFE_NO_PAD) } @@ -276,7 +279,11 @@ impl Signing { async fn pickle(&self, pickle_key: &[u8]) -> PickledSigning { let key = GenericArray::from_slice(pickle_key); let cipher = Aes256Gcm::new(key); - let nonce = GenericArray::from_slice(b"unique nonce"); + + let mut nonce = vec![0u8; NONCE_SIZE]; + getrandom(&mut nonce).expect("Can't generate nonce to pickle the signing object"); + let nonce = GenericArray::from_slice(nonce.as_slice()); + let ciphertext = cipher .encrypt(nonce, self.seed.as_slice()) .expect("Can't encrypt signing pickle"); From dd0642cd59e7bc1bea7a787793e60271e76b7a24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 21 Oct 2020 13:21:22 +0200 Subject: [PATCH 23/34] crypto: Add a pickle key struct. --- matrix_sdk_crypto/src/store/mod.rs | 1 + matrix_sdk_crypto/src/store/pickle_key.rs | 154 ++++++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 matrix_sdk_crypto/src/store/pickle_key.rs diff --git a/matrix_sdk_crypto/src/store/mod.rs b/matrix_sdk_crypto/src/store/mod.rs index 478cb709..0753e672 100644 --- a/matrix_sdk_crypto/src/store/mod.rs +++ b/matrix_sdk_crypto/src/store/mod.rs @@ -39,6 +39,7 @@ pub mod caches; mod memorystore; +mod pickle_key; #[cfg(not(target_arch = "wasm32"))] #[cfg(feature = "sqlite_cryptostore")] pub(crate) mod sqlite; diff --git a/matrix_sdk_crypto/src/store/pickle_key.rs b/matrix_sdk_crypto/src/store/pickle_key.rs new file mode 100644 index 00000000..4398f1fe --- /dev/null +++ b/matrix_sdk_crypto/src/store/pickle_key.rs @@ -0,0 +1,154 @@ +// Copyright 2020 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. + +#![allow(dead_code)] + +use aes_gcm::{ + aead::{generic_array::GenericArray, Aead, NewAead}, + Aes256Gcm, +}; +use getrandom::getrandom; +use hmac::Hmac; +use pbkdf2::pbkdf2; +use sha2::Sha256; +use zeroize::{Zeroize, Zeroizing}; + +use serde::{Deserialize, Serialize}; + +const VERSION: u8 = 1; +const KEY_SIZE: usize = 32; +const NONCE_SIZE: usize = 12; +const KDF_SALT_SIZE: usize = 32; +const KDF_ROUNDS: u32 = 10000; + +/// An encrypted version of our pickle key, this can be safely stored in a +/// database. +#[derive(Debug, Serialize, Deserialize, PartialEq)] +pub struct EncryptedPickleKey { + /// The version of the encrypted pickle. + pub version: u8, + /// The salt that was used when the passphrase was expanded into a AES key. + pub kdf_salt: Vec, + /// The nonce that was used to encrypt the pickle key. + pub nonce: Vec, + /// The encrypted pickle key. + pub ciphertext: Vec, +} + +/// A pickle key that will be used to encrypt all the private keys for Olm. +/// +/// Olm uses AES256 to encrypt accounts, sessions, inbound group sessions. We +/// also implement our own pickling for the cross-signing types using +/// AES256-GCM so the key sizes match. +#[derive(Debug, Zeroize, PartialEq)] +pub struct PickleKey { + version: u8, + aes256_key: Vec, +} + +impl PickleKey { + /// Generate a new random pickle key. + pub fn new() -> Self { + let mut key = vec![0u8; KEY_SIZE]; + getrandom(&mut key).expect("Can't generate new pickle key"); + + Self { + version: VERSION, + aes256_key: key, + } + } + + fn expand_key(passphrase: &str, salt: &[u8]) -> Zeroizing> { + let mut key = Zeroizing::from(vec![0u8; KEY_SIZE]); + pbkdf2::>(passphrase.as_bytes(), &salt, KDF_ROUNDS, &mut *key); + key + } + + /// Encrypt and export our pickle key using the given passphrase. + /// + /// # Arguments + /// + /// * `passphrase` - The passphrase that should be used to encrypt the + /// pickle key. + pub fn encrypt(&self, passphrase: &str) -> EncryptedPickleKey { + let mut salt = vec![0u8; KDF_SALT_SIZE]; + getrandom(&mut salt).expect("Can't generate new random pickle key"); + + let key = PickleKey::expand_key(passphrase, &salt); + let key = GenericArray::from_slice(key.as_ref()); + let cipher = Aes256Gcm::new(&key); + + let mut nonce = vec![0u8; NONCE_SIZE]; + getrandom(&mut nonce).expect("Can't generate new random nonce for the pickle key"); + + let ciphertext = cipher + .encrypt( + &GenericArray::from_slice(nonce.as_ref()), + self.aes256_key.as_slice(), + ) + .expect("Can't encrypt pickle key"); + + EncryptedPickleKey { + version: self.version, + kdf_salt: salt, + nonce, + ciphertext, + } + } + + /// Restore a pickle key from an encrypted export. + /// + /// # Arguments + /// + /// * `passphrase` - The passphrase that should be used to encrypt the + /// pickle key. + /// + /// * `encrypted` - The exported and encrypted version of the pickle key. + pub fn from_encrypted(passphrase: &str, encrypted: EncryptedPickleKey) -> Self { + let key = PickleKey::expand_key(passphrase, &encrypted.kdf_salt); + let key = GenericArray::from_slice(key.as_ref()); + let cipher = Aes256Gcm::new(&key); + let nonce = GenericArray::from_slice(&encrypted.nonce); + + let decrypted_key = cipher + .decrypt(nonce, encrypted.ciphertext.as_ref()) + .expect("Can't decrypt pickle"); + + Self { + version: encrypted.version, + aes256_key: decrypted_key, + } + } +} + +#[cfg(test)] +mod test { + use super::PickleKey; + + #[test] + fn generating() { + PickleKey::new(); + } + + #[test] + fn encrypting() { + let passphrase = "it's a secret to everybody"; + let pickle_key = PickleKey::new(); + + let encrypted = pickle_key.encrypt(passphrase); + let decrypted = PickleKey::from_encrypted(passphrase, encrypted); + + assert_eq!(pickle_key, decrypted); + } +} From 959e8450affd1b9b5a6828fd0b0a202cbf6af45a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 21 Oct 2020 14:01:27 +0200 Subject: [PATCH 24/34] crypto: Use a transaction to create sqlite tables. --- matrix_sdk_crypto/src/store/memorystore.rs | 1 + matrix_sdk_crypto/src/store/mod.rs | 1 + matrix_sdk_crypto/src/store/sqlite.rs | 36 ++++++++++++---------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/matrix_sdk_crypto/src/store/memorystore.rs b/matrix_sdk_crypto/src/store/memorystore.rs index f41d4579..f7845dba 100644 --- a/matrix_sdk_crypto/src/store/memorystore.rs +++ b/matrix_sdk_crypto/src/store/memorystore.rs @@ -26,6 +26,7 @@ use matrix_sdk_common_macros::async_trait; use super::{ caches::{DeviceStore, GroupSessionStore, SessionStore}, + pickle_key::EncryptedPickleKey, Changes, CryptoStore, InboundGroupSession, ReadOnlyAccount, Result, Session, }; use crate::identities::{ReadOnlyDevice, UserIdentities}; diff --git a/matrix_sdk_crypto/src/store/mod.rs b/matrix_sdk_crypto/src/store/mod.rs index 0753e672..23c1aeda 100644 --- a/matrix_sdk_crypto/src/store/mod.rs +++ b/matrix_sdk_crypto/src/store/mod.rs @@ -46,6 +46,7 @@ pub(crate) mod sqlite; use matrix_sdk_common::identifiers::DeviceIdBox; pub use memorystore::MemoryStore; +use pickle_key::EncryptedPickleKey; #[cfg(not(target_arch = "wasm32"))] #[cfg(feature = "sqlite_cryptostore")] pub use sqlite::SqliteStore; diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index 6759abb5..db9430f2 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -167,7 +167,9 @@ impl SqliteStore { async fn create_tables(&self) -> Result<()> { let mut connection = self.connection.lock().await; - connection + let mut transaction = connection.begin().await?; + + transaction .execute( r#" CREATE TABLE IF NOT EXISTS accounts ( @@ -183,7 +185,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS sessions ( @@ -202,7 +204,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS tracked_users ( @@ -220,7 +222,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS inbound_group_sessions ( @@ -241,7 +243,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS group_session_claimed_keys ( @@ -259,7 +261,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS group_session_chains ( @@ -276,7 +278,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS devices ( @@ -296,7 +298,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS algorithms ( @@ -313,7 +315,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS device_keys ( @@ -331,7 +333,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS device_signatures ( @@ -350,7 +352,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS users ( @@ -367,7 +369,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS users_trust_state ( @@ -382,7 +384,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS cross_signing_keys ( @@ -399,7 +401,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS user_keys ( @@ -416,7 +418,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS user_key_signatures ( @@ -435,7 +437,7 @@ impl SqliteStore { ) .await?; - connection + transaction .execute( r#" CREATE TABLE IF NOT EXISTS key_value ( @@ -453,6 +455,8 @@ impl SqliteStore { ) .await?; + transaction.commit().await?; + Ok(()) } From d175c47a05c1cb2b02af6ae0de520f0cdfb5c9b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 21 Oct 2020 15:13:21 +0200 Subject: [PATCH 25/34] crypto: Use a random pickle key in the sqlite store. --- matrix_sdk_crypto/src/store/memorystore.rs | 1 - matrix_sdk_crypto/src/store/mod.rs | 2 +- matrix_sdk_crypto/src/store/pickle_key.rs | 24 +++- matrix_sdk_crypto/src/store/sqlite.rs | 146 +++++++++++++++------ 4 files changed, 128 insertions(+), 45 deletions(-) diff --git a/matrix_sdk_crypto/src/store/memorystore.rs b/matrix_sdk_crypto/src/store/memorystore.rs index f7845dba..f41d4579 100644 --- a/matrix_sdk_crypto/src/store/memorystore.rs +++ b/matrix_sdk_crypto/src/store/memorystore.rs @@ -26,7 +26,6 @@ use matrix_sdk_common_macros::async_trait; use super::{ caches::{DeviceStore, GroupSessionStore, SessionStore}, - pickle_key::EncryptedPickleKey, Changes, CryptoStore, InboundGroupSession, ReadOnlyAccount, Result, Session, }; use crate::identities::{ReadOnlyDevice, UserIdentities}; diff --git a/matrix_sdk_crypto/src/store/mod.rs b/matrix_sdk_crypto/src/store/mod.rs index 23c1aeda..c7f39f7d 100644 --- a/matrix_sdk_crypto/src/store/mod.rs +++ b/matrix_sdk_crypto/src/store/mod.rs @@ -46,7 +46,7 @@ pub(crate) mod sqlite; use matrix_sdk_common::identifiers::DeviceIdBox; pub use memorystore::MemoryStore; -use pickle_key::EncryptedPickleKey; +pub use pickle_key::{EncryptedPickleKey, PickleKey}; #[cfg(not(target_arch = "wasm32"))] #[cfg(feature = "sqlite_cryptostore")] pub use sqlite::SqliteStore; diff --git a/matrix_sdk_crypto/src/store/pickle_key.rs b/matrix_sdk_crypto/src/store/pickle_key.rs index 4398f1fe..e4b01fe8 100644 --- a/matrix_sdk_crypto/src/store/pickle_key.rs +++ b/matrix_sdk_crypto/src/store/pickle_key.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -#![allow(dead_code)] +use std::convert::TryFrom; use aes_gcm::{ aead::{generic_array::GenericArray, Aead, NewAead}, @@ -20,6 +20,7 @@ use aes_gcm::{ }; use getrandom::getrandom; use hmac::Hmac; +use olm_rs::PicklingMode; use pbkdf2::pbkdf2; use sha2::Sha256; use zeroize::{Zeroize, Zeroizing}; @@ -57,6 +58,20 @@ pub struct PickleKey { aes256_key: Vec, } +impl TryFrom> for PickleKey { + type Error = (); + fn try_from(value: Vec) -> Result { + if value.len() != KEY_SIZE { + Err(()) + } else { + Ok(Self { + version: VERSION, + aes256_key: value, + }) + } + } +} + impl PickleKey { /// Generate a new random pickle key. pub fn new() -> Self { @@ -75,6 +90,13 @@ impl PickleKey { key } + /// Get a `PicklingMode` version of this pickle key. + pub fn pickle_mode(&self) -> PicklingMode { + PicklingMode::Encrypted { + key: self.aes256_key.clone(), + } + } + /// Encrypt and export our pickle key using the given passphrase. /// /// # Arguments diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index db9430f2..84e720a5 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -32,9 +32,12 @@ use matrix_sdk_common::{ locks::Mutex, }; use sqlx::{query, query_as, sqlite::SqliteConnectOptions, Connection, Executor, SqliteConnection}; -use zeroize::Zeroizing; -use super::{caches::SessionStore, Changes, CryptoStore, CryptoStoreError, Result}; +use super::{ + caches::SessionStore, + pickle_key::{EncryptedPickleKey, PickleKey}, + Changes, CryptoStore, CryptoStoreError, Result, +}; use crate::{ identities::{LocalTrust, OwnUserIdentity, ReadOnlyDevice, UserIdentities, UserIdentity}, olm::{ @@ -44,6 +47,10 @@ use crate::{ }, }; +/// This needs to be 32 bytes long since AES-GCM requires it, otherwise we will +/// panic once we try to pickle a Signing object. +const DEFAULT_PICKLE: &'static str = "DEFAULT_PICKLE_PASSPHRASE_123456"; + /// SQLite based implementation of a `CryptoStore`. #[derive(Clone)] #[cfg_attr(feature = "docs", doc(cfg(r#sqlite_cryptostore)))] @@ -58,7 +65,7 @@ pub struct SqliteStore { users_for_key_query: Arc>, connection: Arc>, - pickle_passphrase: Arc>>, + pickle_key: Arc, } #[derive(Clone)] @@ -117,20 +124,14 @@ impl SqliteStore { path: P, passphrase: &str, ) -> Result { - SqliteStore::open_helper( - user_id, - device_id, - path, - Some(Zeroizing::new(passphrase.to_owned())), - ) - .await + SqliteStore::open_helper(user_id, device_id, path, Some(passphrase)).await } async fn open_helper>( user_id: &UserId, device_id: &DeviceId, path: P, - passphrase: Option>, + passphrase: Option<&str>, ) -> Result { let path = path.as_ref().join(DATABASE_NAME); let options = SqliteConnectOptions::new() @@ -139,7 +140,15 @@ impl SqliteStore { .read_only(false) .filename(&path); - let connection = SqliteConnection::connect_with(&options).await?; + let mut connection = SqliteConnection::connect_with(&options).await?; + Self::create_tables(&mut connection).await?; + + let pickle_key = if let Some(passphrase) = passphrase { + Self::get_or_create_pickle_key(user_id, device_id, &passphrase, &mut connection).await? + } else { + PickleKey::try_from(DEFAULT_PICKLE.as_bytes().to_vec()) + .expect("Can't create default pickle key") + }; let store = SqliteStore { user_id: Arc::new(user_id.to_owned()), @@ -148,11 +157,10 @@ impl SqliteStore { sessions: SessionStore::new(), path: Arc::new(path), connection: Arc::new(Mutex::new(connection)), - pickle_passphrase: Arc::new(passphrase), tracked_users: Arc::new(DashSet::new()), users_for_key_query: Arc::new(DashSet::new()), + pickle_key: Arc::new(pickle_key), }; - store.create_tables().await?; Ok(store) } @@ -165,11 +173,8 @@ impl SqliteStore { .map(|i| i.account_id) } - async fn create_tables(&self) -> Result<()> { - let mut connection = self.connection.lock().await; - let mut transaction = connection.begin().await?; - - transaction + async fn create_tables(connection: &mut SqliteConnection) -> Result<()> { + connection .execute( r#" CREATE TABLE IF NOT EXISTS accounts ( @@ -185,7 +190,21 @@ impl SqliteStore { ) .await?; - transaction + connection + .execute( + r#" + CREATE TABLE IF NOT EXISTS pickle_keys ( + "id" INTEGER NOT NULL PRIMARY KEY, + "user_id" TEXT NOT NULL, + "device_id" TEXT NOT NULL, + "key" TEXT NOT NULL, + UNIQUE(user_id,device_id) + ); + "#, + ) + .await?; + + connection .execute( r#" CREATE TABLE IF NOT EXISTS sessions ( @@ -204,7 +223,7 @@ impl SqliteStore { ) .await?; - transaction + connection .execute( r#" CREATE TABLE IF NOT EXISTS tracked_users ( @@ -222,7 +241,7 @@ impl SqliteStore { ) .await?; - transaction + connection .execute( r#" CREATE TABLE IF NOT EXISTS inbound_group_sessions ( @@ -243,7 +262,7 @@ impl SqliteStore { ) .await?; - transaction + connection .execute( r#" CREATE TABLE IF NOT EXISTS group_session_claimed_keys ( @@ -261,7 +280,7 @@ impl SqliteStore { ) .await?; - transaction + connection .execute( r#" CREATE TABLE IF NOT EXISTS group_session_chains ( @@ -278,7 +297,7 @@ impl SqliteStore { ) .await?; - transaction + connection .execute( r#" CREATE TABLE IF NOT EXISTS devices ( @@ -298,7 +317,7 @@ impl SqliteStore { ) .await?; - transaction + connection .execute( r#" CREATE TABLE IF NOT EXISTS algorithms ( @@ -315,7 +334,7 @@ impl SqliteStore { ) .await?; - transaction + connection .execute( r#" CREATE TABLE IF NOT EXISTS device_keys ( @@ -333,7 +352,7 @@ impl SqliteStore { ) .await?; - transaction + connection .execute( r#" CREATE TABLE IF NOT EXISTS device_signatures ( @@ -352,7 +371,7 @@ impl SqliteStore { ) .await?; - transaction + connection .execute( r#" CREATE TABLE IF NOT EXISTS users ( @@ -369,7 +388,7 @@ impl SqliteStore { ) .await?; - transaction + connection .execute( r#" CREATE TABLE IF NOT EXISTS users_trust_state ( @@ -384,7 +403,7 @@ impl SqliteStore { ) .await?; - transaction + connection .execute( r#" CREATE TABLE IF NOT EXISTS cross_signing_keys ( @@ -401,7 +420,7 @@ impl SqliteStore { ) .await?; - transaction + connection .execute( r#" CREATE TABLE IF NOT EXISTS user_keys ( @@ -418,7 +437,7 @@ impl SqliteStore { ) .await?; - transaction + connection .execute( r#" CREATE TABLE IF NOT EXISTS user_key_signatures ( @@ -437,7 +456,7 @@ impl SqliteStore { ) .await?; - transaction + connection .execute( r#" CREATE TABLE IF NOT EXISTS key_value ( @@ -455,11 +474,59 @@ impl SqliteStore { ) .await?; - transaction.commit().await?; + Ok(()) + } + + async fn save_pickle_key( + user_id: &UserId, + device_id: &DeviceId, + key: EncryptedPickleKey, + connection: &mut SqliteConnection, + ) -> Result<()> { + let key = serde_json::to_string(&key)?; + + query( + "INSERT INTO pickle_keys ( + user_id, device_id, key + ) VALUES (?1, ?2, ?3) + ON CONFLICT(user_id, device_id) DO UPDATE SET + key = excluded.key + ", + ) + .bind(user_id.as_str()) + .bind(device_id.as_str()) + .bind(key) + .execute(&mut *connection) + .await?; Ok(()) } + async fn get_or_create_pickle_key( + user_id: &UserId, + device_id: &DeviceId, + passphrase: &str, + connection: &mut SqliteConnection, + ) -> Result { + let row: Option<(String,)> = + query_as("SELECT key FROM pickle_keys WHERE user_id = ? and device_id = ?") + .bind(user_id.as_str()) + .bind(device_id.as_str()) + .fetch_optional(&mut *connection) + .await?; + + Ok(if let Some(row) = row { + let encrypted: EncryptedPickleKey = serde_json::from_str(&row.0).unwrap(); + PickleKey::from_encrypted(passphrase, encrypted) + } else { + let key = PickleKey::new(); + let encrypted = key.encrypt(passphrase); + Self::save_pickle_key(user_id, device_id, encrypted, connection).await?; + + key + }) + } + async fn lazy_load_sessions( &self, connection: &mut SqliteConnection, @@ -983,12 +1050,7 @@ impl SqliteStore { } fn get_pickle_mode(&self) -> PicklingMode { - match &*self.pickle_passphrase { - Some(p) => PicklingMode::Encrypted { - key: p.as_bytes().to_vec(), - }, - None => PicklingMode::Unencrypted, - } + self.pickle_key.pickle_mode() } async fn save_inbound_group_session_helper( @@ -1484,8 +1546,8 @@ impl CryptoStore for SqliteStore { } async fn save_account(&self, account: ReadOnlyAccount) -> Result<()> { - let pickle = account.pickle(self.get_pickle_mode()).await; let mut connection = self.connection.lock().await; + let pickle = account.pickle(self.get_pickle_mode()).await; query( "INSERT INTO accounts ( From ac0df5dea9d1d556f2fb9032cceb6dd257e700da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 21 Oct 2020 15:28:43 +0200 Subject: [PATCH 26/34] crypto: Properly handle errors in the pickle key decryption. --- matrix_sdk_crypto/src/store/mod.rs | 4 +++ matrix_sdk_crypto/src/store/pickle_key.rs | 37 ++++++++++++++--------- matrix_sdk_crypto/src/store/sqlite.rs | 3 +- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/matrix_sdk_crypto/src/store/mod.rs b/matrix_sdk_crypto/src/store/mod.rs index c7f39f7d..1f24a49d 100644 --- a/matrix_sdk_crypto/src/store/mod.rs +++ b/matrix_sdk_crypto/src/store/mod.rs @@ -308,6 +308,10 @@ pub enum CryptoStoreError { #[error(transparent)] SessionUnpickling(#[from] SessionUnpicklingError), + /// Failed to decrypt an pickled object. + #[error("An object failed to be decrypted while unpickling")] + UnpicklingError, + /// A Matirx identifier failed to be validated. #[error(transparent)] IdentifierValidation(#[from] IdentifierValidationError), diff --git a/matrix_sdk_crypto/src/store/pickle_key.rs b/matrix_sdk_crypto/src/store/pickle_key.rs index e4b01fe8..b4ee98b8 100644 --- a/matrix_sdk_crypto/src/store/pickle_key.rs +++ b/matrix_sdk_crypto/src/store/pickle_key.rs @@ -16,7 +16,7 @@ use std::convert::TryFrom; use aes_gcm::{ aead::{generic_array::GenericArray, Aead, NewAead}, - Aes256Gcm, + Aes256Gcm, Error as DecryptionError, }; use getrandom::getrandom; use hmac::Hmac; @@ -58,6 +58,18 @@ pub struct PickleKey { aes256_key: Vec, } +impl Default for PickleKey { + fn default() -> Self { + let mut key = vec![0u8; KEY_SIZE]; + getrandom(&mut key).expect("Can't generate new pickle key"); + + Self { + version: VERSION, + aes256_key: key, + } + } +} + impl TryFrom> for PickleKey { type Error = (); fn try_from(value: Vec) -> Result { @@ -75,13 +87,7 @@ impl TryFrom> for PickleKey { impl PickleKey { /// Generate a new random pickle key. pub fn new() -> Self { - let mut key = vec![0u8; KEY_SIZE]; - getrandom(&mut key).expect("Can't generate new pickle key"); - - Self { - version: VERSION, - aes256_key: key, - } + Default::default() } fn expand_key(passphrase: &str, salt: &[u8]) -> Zeroizing> { @@ -137,20 +143,21 @@ impl PickleKey { /// pickle key. /// /// * `encrypted` - The exported and encrypted version of the pickle key. - pub fn from_encrypted(passphrase: &str, encrypted: EncryptedPickleKey) -> Self { + pub fn from_encrypted( + passphrase: &str, + encrypted: EncryptedPickleKey, + ) -> Result { let key = PickleKey::expand_key(passphrase, &encrypted.kdf_salt); let key = GenericArray::from_slice(key.as_ref()); let cipher = Aes256Gcm::new(&key); let nonce = GenericArray::from_slice(&encrypted.nonce); - let decrypted_key = cipher - .decrypt(nonce, encrypted.ciphertext.as_ref()) - .expect("Can't decrypt pickle"); + let decrypted_key = cipher.decrypt(nonce, encrypted.ciphertext.as_ref())?; - Self { + Ok(Self { version: encrypted.version, aes256_key: decrypted_key, - } + }) } } @@ -169,7 +176,7 @@ mod test { let pickle_key = PickleKey::new(); let encrypted = pickle_key.encrypt(passphrase); - let decrypted = PickleKey::from_encrypted(passphrase, encrypted); + let decrypted = PickleKey::from_encrypted(passphrase, encrypted).unwrap(); assert_eq!(pickle_key, decrypted); } diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index 84e720a5..c2cbcf80 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -516,8 +516,9 @@ impl SqliteStore { .await?; Ok(if let Some(row) = row { - let encrypted: EncryptedPickleKey = serde_json::from_str(&row.0).unwrap(); + let encrypted: EncryptedPickleKey = serde_json::from_str(&row.0)?; PickleKey::from_encrypted(passphrase, encrypted) + .map_err(|_| CryptoStoreError::UnpicklingError)? } else { let key = PickleKey::new(); let encrypted = key.encrypt(passphrase); From c9db63509f44b71139c5db29c318480c86e6a80f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 21 Oct 2020 16:24:10 +0200 Subject: [PATCH 27/34] crypto: Add error handling to the signing module. --- matrix_sdk_crypto/src/olm/signing.rs | 75 ++++++++++++++++++---------- 1 file changed, 49 insertions(+), 26 deletions(-) diff --git a/matrix_sdk_crypto/src/olm/signing.rs b/matrix_sdk_crypto/src/olm/signing.rs index 633a18b9..d6272a20 100644 --- a/matrix_sdk_crypto/src/olm/signing.rs +++ b/matrix_sdk_crypto/src/olm/signing.rs @@ -21,6 +21,7 @@ use aes_gcm::{ use base64::{decode_config, encode_config, DecodeError, URL_SAFE_NO_PAD}; use getrandom::getrandom; use serde::{Deserialize, Serialize}; +use serde_json::Error as JsonError; use std::{ collections::BTreeMap, sync::{ @@ -28,6 +29,7 @@ use std::{ Arc, }, }; +use thiserror::Error; use zeroize::Zeroizing; use olm_rs::{errors::OlmUtilityError, pk::OlmPkSigning, utility::OlmUtility}; @@ -53,6 +55,22 @@ fn decode>(input: T) -> Result, DecodeError> { decode_config(input, URL_SAFE_NO_PAD) } +/// Error type reporting failures in the Signign operations. +#[derive(Debug, Error)] +pub enum SigningError { + /// Error decoding the base64 encoded pickle data. + #[error(transparent)] + Decode(#[from] DecodeError), + + /// Error decrypting the pickled signing seed + #[error("Error decrypting the pickled signign seed")] + Decryption(String), + + /// Error deserializing the pickle data. + #[error(transparent)] + Json(#[from] JsonError), +} + #[derive(Clone)] pub struct Signing { inner: Arc>, @@ -105,13 +123,13 @@ impl MasterSigning { PickledMasterSigning { pickle, public_key } } - fn from_pickle(pickle: PickledMasterSigning, pickle_key: &[u8]) -> Self { - let inner = Signing::from_pickle(pickle.pickle, pickle_key); + fn from_pickle(pickle: PickledMasterSigning, pickle_key: &[u8]) -> Result { + let inner = Signing::from_pickle(pickle.pickle, pickle_key)?; - Self { + Ok(Self { inner, public_key: pickle.public_key.into(), - } + }) } async fn sign_subkey<'a>(&self, subkey: &mut CrossSigningKey) { @@ -145,13 +163,13 @@ impl UserSigning { PickledUserSigning { pickle, public_key } } - fn from_pickle(pickle: PickledUserSigning, pickle_key: &[u8]) -> Self { - let inner = Signing::from_pickle(pickle.pickle, pickle_key); + fn from_pickle(pickle: PickledUserSigning, pickle_key: &[u8]) -> Result { + let inner = Signing::from_pickle(pickle.pickle, pickle_key)?; - Self { + Ok(Self { inner, public_key: pickle.public_key.into(), - } + }) } } @@ -162,13 +180,13 @@ impl SelfSigning { PickledSelfSigning { pickle, public_key } } - fn from_pickle(pickle: PickledSelfSigning, pickle_key: &[u8]) -> Self { - let inner = Signing::from_pickle(pickle.pickle, pickle_key); + fn from_pickle(pickle: PickledSelfSigning, pickle_key: &[u8]) -> Result { + let inner = Signing::from_pickle(pickle.pickle, pickle_key)?; - Self { + Ok(Self { inner, public_key: pickle.public_key.into(), - } + }) } } @@ -259,21 +277,21 @@ impl Signing { } } - fn from_pickle(pickle: PickledSigning, pickle_key: &[u8]) -> Self { - let pickled: InnerPickle = serde_json::from_str(pickle.as_str()).unwrap(); + fn from_pickle(pickle: PickledSigning, pickle_key: &[u8]) -> Result { + let pickled: InnerPickle = serde_json::from_str(pickle.as_str())?; let key = GenericArray::from_slice(pickle_key); let cipher = Aes256Gcm::new(key); - let nonce = decode(pickled.nonce).unwrap(); + let nonce = decode(pickled.nonce)?; let nonce = GenericArray::from_slice(&nonce); - let ciphertext = &decode(pickled.ciphertext).unwrap(); + let ciphertext = &decode(pickled.ciphertext)?; let seed = cipher .decrypt(&nonce, ciphertext.as_slice()) - .expect("Can't decrypt pickle"); + .map_err(|e| SigningError::Decryption(e.to_string()))?; - Self::from_seed(seed) + Ok(Self::from_seed(seed)) } async fn pickle(&self, pickle_key: &[u8]) -> PickledSigning { @@ -412,32 +430,35 @@ impl PrivateCrossSigningIdentity { } } - pub async fn from_pickle(pickle: PickledCrossSigningIdentity, pickle_key: &[u8]) -> Self { + pub async fn from_pickle( + pickle: PickledCrossSigningIdentity, + pickle_key: &[u8], + ) -> Result { let master = if let Some(m) = pickle.master_key { - Some(MasterSigning::from_pickle(m, pickle_key)) + Some(MasterSigning::from_pickle(m, pickle_key)?) } else { None }; let self_signing = if let Some(s) = pickle.self_signing_key { - Some(SelfSigning::from_pickle(s, pickle_key)) + Some(SelfSigning::from_pickle(s, pickle_key)?) } else { None }; let user_signing = if let Some(u) = pickle.user_signing_key { - Some(UserSigning::from_pickle(u, pickle_key)) + Some(UserSigning::from_pickle(u, pickle_key)?) } else { None }; - Self { + Ok(Self { user_id: Arc::new(pickle.user_id), shared: Arc::new(AtomicBool::from(pickle.shared)), master_key: Arc::new(Mutex::new(master)), self_signing_key: Arc::new(Mutex::new(self_signing)), user_signing_key: Arc::new(Mutex::new(user_signing)), - } + }) } pub(crate) async fn as_upload_request(&self) -> UploadSigningKeysRequest { @@ -509,7 +530,7 @@ mod test { let signing = Signing::new(); let pickled = signing.pickle(pickle_key()).await; - let unpickled = Signing::from_pickle(pickled, pickle_key()); + let unpickled = Signing::from_pickle(pickled, pickle_key()).unwrap(); assert_eq!(signing.public_key(), unpickled.public_key()); } @@ -554,7 +575,9 @@ mod test { let pickled = identity.pickle(pickle_key()).await; - let unpickled = PrivateCrossSigningIdentity::from_pickle(pickled, pickle_key()).await; + let unpickled = PrivateCrossSigningIdentity::from_pickle(pickled, pickle_key()) + .await + .unwrap(); assert_eq!(identity.user_id, unpickled.user_id); assert_eq!( From fa25ca4475b865d919f2995ba30e1c88fa9a29c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 21 Oct 2020 16:50:59 +0200 Subject: [PATCH 28/34] crypto: Make the pickle key encryption future proof. --- matrix_sdk_crypto/src/store/pickle_key.rs | 78 ++++++++++++++--------- 1 file changed, 48 insertions(+), 30 deletions(-) diff --git a/matrix_sdk_crypto/src/store/pickle_key.rs b/matrix_sdk_crypto/src/store/pickle_key.rs index b4ee98b8..1781e363 100644 --- a/matrix_sdk_crypto/src/store/pickle_key.rs +++ b/matrix_sdk_crypto/src/store/pickle_key.rs @@ -27,24 +27,44 @@ use zeroize::{Zeroize, Zeroizing}; use serde::{Deserialize, Serialize}; -const VERSION: u8 = 1; const KEY_SIZE: usize = 32; const NONCE_SIZE: usize = 12; const KDF_SALT_SIZE: usize = 32; const KDF_ROUNDS: u32 = 10000; +/// Version specific info for the key derivation method that is used. +#[derive(Debug, Serialize, Deserialize, PartialEq)] +pub enum KdfInfo { + Pbkdf2 { + /// The number of PBKDF rounds that were used when deriving the AES key. + rounds: u32, + }, +} + +/// Version specific info for encryption method that is used to encrypt our +/// pickle key. +#[derive(Debug, Serialize, Deserialize, PartialEq)] +pub enum CipherTextInfo { + Aes256Gcm { + /// The nonce that was used to encrypt the ciphertext. + nonce: Vec, + /// The encrypted pickle key. + ciphertext: Vec, + }, +} + /// An encrypted version of our pickle key, this can be safely stored in a /// database. #[derive(Debug, Serialize, Deserialize, PartialEq)] pub struct EncryptedPickleKey { - /// The version of the encrypted pickle. - pub version: u8, + /// Info about the key derivation method that was used to expand the + /// passphrase into an encryption key. + pub kdf_info: KdfInfo, + /// The ciphertext with it's accompanying additional data that is needed to + /// decrypt the pickle key. + pub ciphertext_info: CipherTextInfo, /// The salt that was used when the passphrase was expanded into a AES key. - pub kdf_salt: Vec, - /// The nonce that was used to encrypt the pickle key. - pub nonce: Vec, - /// The encrypted pickle key. - pub ciphertext: Vec, + kdf_salt: Vec, } /// A pickle key that will be used to encrypt all the private keys for Olm. @@ -54,7 +74,6 @@ pub struct EncryptedPickleKey { /// AES256-GCM so the key sizes match. #[derive(Debug, Zeroize, PartialEq)] pub struct PickleKey { - version: u8, aes256_key: Vec, } @@ -63,10 +82,7 @@ impl Default for PickleKey { let mut key = vec![0u8; KEY_SIZE]; getrandom(&mut key).expect("Can't generate new pickle key"); - Self { - version: VERSION, - aes256_key: key, - } + Self { aes256_key: key } } } @@ -76,10 +92,7 @@ impl TryFrom> for PickleKey { if value.len() != KEY_SIZE { Err(()) } else { - Ok(Self { - version: VERSION, - aes256_key: value, - }) + Ok(Self { aes256_key: value }) } } } @@ -90,9 +103,9 @@ impl PickleKey { Default::default() } - fn expand_key(passphrase: &str, salt: &[u8]) -> Zeroizing> { + fn expand_key(passphrase: &str, salt: &[u8], rounds: u32) -> Zeroizing> { let mut key = Zeroizing::from(vec![0u8; KEY_SIZE]); - pbkdf2::>(passphrase.as_bytes(), &salt, KDF_ROUNDS, &mut *key); + pbkdf2::>(passphrase.as_bytes(), &salt, rounds, &mut *key); key } @@ -113,7 +126,7 @@ impl PickleKey { let mut salt = vec![0u8; KDF_SALT_SIZE]; getrandom(&mut salt).expect("Can't generate new random pickle key"); - let key = PickleKey::expand_key(passphrase, &salt); + let key = PickleKey::expand_key(passphrase, &salt, KDF_ROUNDS); let key = GenericArray::from_slice(key.as_ref()); let cipher = Aes256Gcm::new(&key); @@ -128,10 +141,9 @@ impl PickleKey { .expect("Can't encrypt pickle key"); EncryptedPickleKey { - version: self.version, + kdf_info: KdfInfo::Pbkdf2 { rounds: KDF_ROUNDS }, kdf_salt: salt, - nonce, - ciphertext, + ciphertext_info: CipherTextInfo::Aes256Gcm { nonce, ciphertext }, } } @@ -147,16 +159,22 @@ impl PickleKey { passphrase: &str, encrypted: EncryptedPickleKey, ) -> Result { - let key = PickleKey::expand_key(passphrase, &encrypted.kdf_salt); - let key = GenericArray::from_slice(key.as_ref()); - let cipher = Aes256Gcm::new(&key); - let nonce = GenericArray::from_slice(&encrypted.nonce); + let key = match encrypted.kdf_info { + KdfInfo::Pbkdf2 { rounds } => Self::expand_key(passphrase, &encrypted.kdf_salt, rounds), + }; - let decrypted_key = cipher.decrypt(nonce, encrypted.ciphertext.as_ref())?; + let key = GenericArray::from_slice(key.as_ref()); + + let decrypted = match encrypted.ciphertext_info { + CipherTextInfo::Aes256Gcm { nonce, ciphertext } => { + let cipher = Aes256Gcm::new(&key); + let nonce = GenericArray::from_slice(&nonce); + cipher.decrypt(nonce, ciphertext.as_ref())? + } + }; Ok(Self { - version: encrypted.version, - aes256_key: decrypted_key, + aes256_key: decrypted, }) } } From 78d7f6c10b66dd45b84db23bfaa20d1a6604c6b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 21 Oct 2020 17:05:36 +0200 Subject: [PATCH 29/34] crypto: Fix a clippy issue. --- matrix_sdk_crypto/src/store/sqlite.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index c2cbcf80..d2d8d77e 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -49,7 +49,7 @@ use crate::{ /// This needs to be 32 bytes long since AES-GCM requires it, otherwise we will /// panic once we try to pickle a Signing object. -const DEFAULT_PICKLE: &'static str = "DEFAULT_PICKLE_PASSPHRASE_123456"; +const DEFAULT_PICKLE: &str = "DEFAULT_PICKLE_PASSPHRASE_123456"; /// SQLite based implementation of a `CryptoStore`. #[derive(Clone)] From f60dc7ed781f5e282abbbcbd20ad7cf92477b6c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 22 Oct 2020 14:52:19 +0200 Subject: [PATCH 30/34] crypto: Allow cross signing identities to be stored/restored. --- matrix_sdk_common/Cargo.toml | 2 +- matrix_sdk_crypto/src/olm/mod.rs | 2 +- matrix_sdk_crypto/src/olm/signing.rs | 51 ++++++++---- matrix_sdk_crypto/src/store/memorystore.rs | 9 +++ matrix_sdk_crypto/src/store/mod.rs | 7 ++ matrix_sdk_crypto/src/store/pickle_key.rs | 5 ++ matrix_sdk_crypto/src/store/sqlite.rs | 91 ++++++++++++++++++++++ 7 files changed, 152 insertions(+), 15 deletions(-) diff --git a/matrix_sdk_common/Cargo.toml b/matrix_sdk_common/Cargo.toml index 7302ebfc..4a074fb7 100644 --- a/matrix_sdk_common/Cargo.toml +++ b/matrix_sdk_common/Cargo.toml @@ -21,7 +21,7 @@ js_int = "0.1.9" [dependencies.ruma] version = "0.0.1" git = "https://github.com/ruma/ruma" -rev = "50eb700571480d1440e15a387d10f98be8abab59" +rev = "db2f58032953ccb6d8ae712d64d713ebdf412598" features = ["client-api", "unstable-pre-spec", "unstable-exhaustive-types"] [target.'cfg(not(target_arch = "wasm32"))'.dependencies] diff --git a/matrix_sdk_crypto/src/olm/mod.rs b/matrix_sdk_crypto/src/olm/mod.rs index df2b387a..d87b1092 100644 --- a/matrix_sdk_crypto/src/olm/mod.rs +++ b/matrix_sdk_crypto/src/olm/mod.rs @@ -32,7 +32,7 @@ pub use group_sessions::{ pub(crate) use group_sessions::{GroupSessionKey, OutboundGroupSession}; pub use olm_rs::{account::IdentityKeys, PicklingMode}; pub use session::{PickledSession, Session, SessionPickle}; -pub(crate) use signing::PrivateCrossSigningIdentity; +pub use signing::{PrivateCrossSigningIdentity, PickledCrossSigningIdentity}; pub(crate) use utility::Utility; #[cfg(test)] diff --git a/matrix_sdk_crypto/src/olm/signing.rs b/matrix_sdk_crypto/src/olm/signing.rs index d6272a20..f92f1246 100644 --- a/matrix_sdk_crypto/src/olm/signing.rs +++ b/matrix_sdk_crypto/src/olm/signing.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -#![allow(dead_code)] +#![allow(dead_code,missing_docs)] use aes_gcm::{ aead::{generic_array::GenericArray, Aead, NewAead}, @@ -213,9 +213,13 @@ pub struct PrivateCrossSigningIdentity { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct PickledCrossSigningIdentity { - user_id: UserId, - shared: bool, + pub user_id: UserId, + pub shared: bool, + pub pickle: String, +} +#[derive(Debug, Clone, Serialize, Deserialize)] +struct PickledSignings { master_key: Option, user_signing_key: Option, self_signing_key: Option, @@ -348,6 +352,10 @@ impl Signing { } impl PrivateCrossSigningIdentity { + pub fn user_id(&self) -> &UserId { + &self.user_id + } + pub(crate) fn empty(user_id: UserId) -> Self { Self { user_id: Arc::new(user_id), @@ -402,7 +410,7 @@ impl PrivateCrossSigningIdentity { self.shared.load(Ordering::SeqCst) } - pub async fn pickle(&self, pickle_key: &[u8]) -> PickledCrossSigningIdentity { + pub async fn pickle(&self, pickle_key: &[u8]) -> Result { let master_key = if let Some(m) = self.master_key.lock().await.as_ref() { Some(m.pickle(pickle_key).await) } else { @@ -421,32 +429,49 @@ impl PrivateCrossSigningIdentity { None }; - PickledCrossSigningIdentity { + let pickle = PickledSignings { + master_key, + user_signing_key, + self_signing_key, + }; + + println!("HELOOO {:#?}", pickle); + + let pickle = serde_json::to_string(&pickle)?; + + Ok(PickledCrossSigningIdentity { user_id: self.user_id.as_ref().to_owned(), shared: self.shared(), - master_key, - self_signing_key, - user_signing_key, - } + pickle, + }) } + /// Restore the private cross signing identity from a pickle. + /// + /// # Panic + /// + /// Panics if the pickle_key isn't 32 bytes long. pub async fn from_pickle( pickle: PickledCrossSigningIdentity, pickle_key: &[u8], ) -> Result { - let master = if let Some(m) = pickle.master_key { + println!("HELOOO UNPICKLED {:#?}", pickle.pickle); + let signings: PickledSignings = serde_json::from_str(&pickle.pickle)?; + + + let master = if let Some(m) = signings.master_key { Some(MasterSigning::from_pickle(m, pickle_key)?) } else { None }; - let self_signing = if let Some(s) = pickle.self_signing_key { + let self_signing = if let Some(s) = signings.self_signing_key { Some(SelfSigning::from_pickle(s, pickle_key)?) } else { None }; - let user_signing = if let Some(u) = pickle.user_signing_key { + let user_signing = if let Some(u) = signings.user_signing_key { Some(UserSigning::from_pickle(u, pickle_key)?) } else { None @@ -573,7 +598,7 @@ mod test { async fn identity_pickling() { let identity = PrivateCrossSigningIdentity::new(user_id()).await; - let pickled = identity.pickle(pickle_key()).await; + let pickled = identity.pickle(pickle_key()).await.unwrap(); let unpickled = PrivateCrossSigningIdentity::from_pickle(pickled, pickle_key()) .await diff --git a/matrix_sdk_crypto/src/store/memorystore.rs b/matrix_sdk_crypto/src/store/memorystore.rs index f41d4579..487f1d24 100644 --- a/matrix_sdk_crypto/src/store/memorystore.rs +++ b/matrix_sdk_crypto/src/store/memorystore.rs @@ -29,6 +29,7 @@ use super::{ Changes, CryptoStore, InboundGroupSession, ReadOnlyAccount, Result, Session, }; use crate::identities::{ReadOnlyDevice, UserIdentities}; +use crate::olm::PrivateCrossSigningIdentity; /// An in-memory only store that will forget all the E2EE key once it's dropped. #[derive(Debug, Clone)] @@ -207,6 +208,14 @@ impl CryptoStore for MemoryStore { async fn get_value(&self, key: &str) -> Result> { Ok(self.values.get(key).map(|v| v.to_owned())) } + + async fn save_identity(&self, _: PrivateCrossSigningIdentity) -> Result<()> { + Ok(()) + } + + async fn load_identity(&self) -> Result> { + Ok(None) + } } #[cfg(test)] diff --git a/matrix_sdk_crypto/src/store/mod.rs b/matrix_sdk_crypto/src/store/mod.rs index 1f24a49d..c4e4365a 100644 --- a/matrix_sdk_crypto/src/store/mod.rs +++ b/matrix_sdk_crypto/src/store/mod.rs @@ -79,6 +79,7 @@ use matrix_sdk_common_macros::async_trait; #[cfg(not(target_arch = "wasm32"))] use matrix_sdk_common_macros::send_sync; +use crate::olm::PrivateCrossSigningIdentity; use crate::{ error::SessionUnpicklingError, identities::{Device, ReadOnlyDevice, UserDevices, UserIdentities}, @@ -337,6 +338,12 @@ pub trait CryptoStore: Debug { /// * `account` - The account that should be stored. async fn save_account(&self, account: ReadOnlyAccount) -> Result<()>; + /// TODO + async fn save_identity(&self, identity: PrivateCrossSigningIdentity) -> Result<()>; + + /// TODO + async fn load_identity(&self) -> Result>; + /// TODO async fn save_changes(&self, changes: Changes) -> Result<()>; diff --git a/matrix_sdk_crypto/src/store/pickle_key.rs b/matrix_sdk_crypto/src/store/pickle_key.rs index 1781e363..c49f6352 100644 --- a/matrix_sdk_crypto/src/store/pickle_key.rs +++ b/matrix_sdk_crypto/src/store/pickle_key.rs @@ -116,6 +116,11 @@ impl PickleKey { } } + /// Get the raw AES256 key. + pub fn key(&self) -> &[u8] { + &self.aes256_key + } + /// Encrypt and export our pickle key using the given passphrase. /// /// # Arguments diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index d2d8d77e..d4182bad 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -38,6 +38,8 @@ use super::{ pickle_key::{EncryptedPickleKey, PickleKey}, Changes, CryptoStore, CryptoStoreError, Result, }; +use crate::olm::PickledCrossSigningIdentity; +use crate::olm::PrivateCrossSigningIdentity; use crate::{ identities::{LocalTrust, OwnUserIdentity, ReadOnlyDevice, UserIdentities, UserIdentity}, olm::{ @@ -190,6 +192,23 @@ impl SqliteStore { ) .await?; + connection + .execute( + r#" + CREATE TABLE IF NOT EXISTS private_identities ( + "id" INTEGER NOT NULL PRIMARY KEY, + "account_id" INTEGER NOT NULL, + "user_id" TEXT NOT NULL, + "pickle" TEXT NOT NULL, + "shared" INTEGER NOT NULL, + FOREIGN KEY ("account_id") REFERENCES "accounts" ("id") + ON DELETE CASCADE + UNIQUE(account_id, user_id) + ); + "#, + ) + .await?; + connection .execute( r#" @@ -1054,6 +1073,10 @@ impl SqliteStore { self.pickle_key.pickle_mode() } + fn get_pickle_key(&self) -> &[u8] { + self.pickle_key.key() + } + async fn save_inbound_group_session_helper( &self, account_id: i64, @@ -1583,6 +1606,62 @@ impl CryptoStore for SqliteStore { Ok(()) } + async fn save_identity(&self, identity: PrivateCrossSigningIdentity) -> Result<()> { + let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; + + let pickle = identity.pickle(self.get_pickle_key()).await?; + + let mut connection = self.connection.lock().await; + + query( + "INSERT INTO private_identities ( + account_id, user_id, pickle, shared + ) VALUES (?1, ?2, ?3, ?4) + ON CONFLICT(account_id, user_id) DO UPDATE SET + pickle = excluded.pickle, + shared = excluded.shared + ", + ) + .bind(account_id) + .bind(pickle.user_id.as_str()) + .bind(pickle.pickle) + .bind(pickle.shared) + .execute(&mut *connection) + .await?; + + Ok(()) + } + + async fn load_identity(&self) -> Result> { + let account_id = self.account_id().ok_or(CryptoStoreError::AccountUnset)?; + let mut connection = self.connection.lock().await; + + let row: Option<(String, bool)> = query_as( + "SELECT pickle, shared FROM private_identities + WHERE account_id = ?", + ) + .bind(account_id) + .fetch_optional(&mut *connection) + .await?; + + if let Some(row) = row { + let pickle = PickledCrossSigningIdentity { + user_id: (&*self.user_id).clone(), + pickle: row.0, + shared: row.1, + }; + + // TODO remove this unwrap + let identity = PrivateCrossSigningIdentity::from_pickle(pickle, self.get_pickle_key()) + .await + .unwrap(); + + Ok(Some(identity)) + } else { + Ok(None) + } + } + async fn save_changes(&self, changes: Changes) -> Result<()> { let mut connection = self.connection.lock().await; let mut transaction = connection.begin().await?; @@ -1734,6 +1813,7 @@ impl std::fmt::Debug for SqliteStore { #[cfg(test)] mod test { + use crate::olm::PrivateCrossSigningIdentity; use crate::{ identities::{ device::test::get_device, @@ -2266,6 +2346,17 @@ mod test { assert!(loaded_user.own().unwrap().is_verified()) } + #[tokio::test(threaded_scheduler)] + async fn private_identity_saving() { + let (_, store, _dir) = get_loaded_store().await; + assert!(store.load_identity().await.unwrap().is_none()); + let identity = PrivateCrossSigningIdentity::new((&*store.user_id).clone()).await; + + store.save_identity(identity.clone()).await.unwrap(); + let loaded_identity = store.load_identity().await.unwrap().unwrap(); + assert_eq!(identity.user_id(), loaded_identity.user_id()); + } + #[tokio::test(threaded_scheduler)] async fn key_value_saving() { let (_, store, _dir) = get_loaded_store().await; From 7de002b128f7a5f4390234ea2d5acf07c59fdff2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 22 Oct 2020 16:40:05 +0200 Subject: [PATCH 31/34] crypto: Fix some lint issues. --- matrix_sdk_crypto/src/olm/mod.rs | 2 +- matrix_sdk_crypto/src/olm/signing.rs | 13 ++++++------- matrix_sdk_crypto/src/store/memorystore.rs | 6 ++++-- matrix_sdk_crypto/src/store/mod.rs | 3 +-- matrix_sdk_crypto/src/store/sqlite.rs | 14 +++++++------- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/matrix_sdk_crypto/src/olm/mod.rs b/matrix_sdk_crypto/src/olm/mod.rs index d87b1092..116f9b82 100644 --- a/matrix_sdk_crypto/src/olm/mod.rs +++ b/matrix_sdk_crypto/src/olm/mod.rs @@ -32,7 +32,7 @@ pub use group_sessions::{ pub(crate) use group_sessions::{GroupSessionKey, OutboundGroupSession}; pub use olm_rs::{account::IdentityKeys, PicklingMode}; pub use session::{PickledSession, Session, SessionPickle}; -pub use signing::{PrivateCrossSigningIdentity, PickledCrossSigningIdentity}; +pub use signing::{PickledCrossSigningIdentity, PrivateCrossSigningIdentity}; pub(crate) use utility::Utility; #[cfg(test)] diff --git a/matrix_sdk_crypto/src/olm/signing.rs b/matrix_sdk_crypto/src/olm/signing.rs index f92f1246..dfde208b 100644 --- a/matrix_sdk_crypto/src/olm/signing.rs +++ b/matrix_sdk_crypto/src/olm/signing.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -#![allow(dead_code,missing_docs)] +#![allow(dead_code, missing_docs)] use aes_gcm::{ aead::{generic_array::GenericArray, Aead, NewAead}, @@ -410,7 +410,10 @@ impl PrivateCrossSigningIdentity { self.shared.load(Ordering::SeqCst) } - pub async fn pickle(&self, pickle_key: &[u8]) -> Result { + pub async fn pickle( + &self, + pickle_key: &[u8], + ) -> Result { let master_key = if let Some(m) = self.master_key.lock().await.as_ref() { Some(m.pickle(pickle_key).await) } else { @@ -435,10 +438,8 @@ impl PrivateCrossSigningIdentity { self_signing_key, }; - println!("HELOOO {:#?}", pickle); - let pickle = serde_json::to_string(&pickle)?; - + Ok(PickledCrossSigningIdentity { user_id: self.user_id.as_ref().to_owned(), shared: self.shared(), @@ -455,10 +456,8 @@ impl PrivateCrossSigningIdentity { pickle: PickledCrossSigningIdentity, pickle_key: &[u8], ) -> Result { - println!("HELOOO UNPICKLED {:#?}", pickle.pickle); let signings: PickledSignings = serde_json::from_str(&pickle.pickle)?; - let master = if let Some(m) = signings.master_key { Some(MasterSigning::from_pickle(m, pickle_key)?) } else { diff --git a/matrix_sdk_crypto/src/store/memorystore.rs b/matrix_sdk_crypto/src/store/memorystore.rs index 487f1d24..952522e5 100644 --- a/matrix_sdk_crypto/src/store/memorystore.rs +++ b/matrix_sdk_crypto/src/store/memorystore.rs @@ -28,8 +28,10 @@ use super::{ caches::{DeviceStore, GroupSessionStore, SessionStore}, Changes, CryptoStore, InboundGroupSession, ReadOnlyAccount, Result, Session, }; -use crate::identities::{ReadOnlyDevice, UserIdentities}; -use crate::olm::PrivateCrossSigningIdentity; +use crate::{ + identities::{ReadOnlyDevice, UserIdentities}, + olm::PrivateCrossSigningIdentity, +}; /// An in-memory only store that will forget all the E2EE key once it's dropped. #[derive(Debug, Clone)] diff --git a/matrix_sdk_crypto/src/store/mod.rs b/matrix_sdk_crypto/src/store/mod.rs index c4e4365a..b19e3b07 100644 --- a/matrix_sdk_crypto/src/store/mod.rs +++ b/matrix_sdk_crypto/src/store/mod.rs @@ -79,11 +79,10 @@ use matrix_sdk_common_macros::async_trait; #[cfg(not(target_arch = "wasm32"))] use matrix_sdk_common_macros::send_sync; -use crate::olm::PrivateCrossSigningIdentity; use crate::{ error::SessionUnpicklingError, identities::{Device, ReadOnlyDevice, UserDevices, UserIdentities}, - olm::{InboundGroupSession, ReadOnlyAccount, Session}, + olm::{InboundGroupSession, PrivateCrossSigningIdentity, ReadOnlyAccount, Session}, verification::VerificationMachine, }; diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index d4182bad..3df06ab2 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -38,14 +38,12 @@ use super::{ pickle_key::{EncryptedPickleKey, PickleKey}, Changes, CryptoStore, CryptoStoreError, Result, }; -use crate::olm::PickledCrossSigningIdentity; -use crate::olm::PrivateCrossSigningIdentity; use crate::{ identities::{LocalTrust, OwnUserIdentity, ReadOnlyDevice, UserIdentities, UserIdentity}, olm::{ AccountPickle, IdentityKeys, InboundGroupSession, InboundGroupSessionPickle, - PickledAccount, PickledInboundGroupSession, PickledSession, PicklingMode, ReadOnlyAccount, - Session, SessionPickle, + PickledAccount, PickledCrossSigningIdentity, PickledInboundGroupSession, PickledSession, + PicklingMode, PrivateCrossSigningIdentity, ReadOnlyAccount, Session, SessionPickle, }, }; @@ -1615,7 +1613,7 @@ impl CryptoStore for SqliteStore { query( "INSERT INTO private_identities ( - account_id, user_id, pickle, shared + account_id, user_id, pickle, shared ) VALUES (?1, ?2, ?3, ?4) ON CONFLICT(account_id, user_id) DO UPDATE SET pickle = excluded.pickle, @@ -1813,13 +1811,15 @@ impl std::fmt::Debug for SqliteStore { #[cfg(test)] mod test { - use crate::olm::PrivateCrossSigningIdentity; use crate::{ identities::{ device::test::get_device, user::test::{get_other_identity, get_own_identity}, }, - olm::{GroupSessionKey, InboundGroupSession, ReadOnlyAccount, Session}, + olm::{ + GroupSessionKey, InboundGroupSession, PrivateCrossSigningIdentity, ReadOnlyAccount, + Session, + }, store::{Changes, DeviceChanges, IdentityChanges}, }; use matrix_sdk_common::{ From 5fd004bae56a20e353ba7a84b3f69c546df16c45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 22 Oct 2020 18:04:29 +0200 Subject: [PATCH 32/34] crypto: Connect the private identity to the verification machine. --- matrix_sdk_crypto/src/identities/manager.rs | 6 ++-- matrix_sdk_crypto/src/identities/user.rs | 8 +++-- matrix_sdk_crypto/src/key_request.rs | 9 +++-- matrix_sdk_crypto/src/machine.rs | 34 +++++++++++++------ .../src/session_manager/sessions.rs | 9 +++-- matrix_sdk_crypto/src/verification/machine.rs | 18 ++++++++-- 6 files changed, 60 insertions(+), 24 deletions(-) diff --git a/matrix_sdk_crypto/src/identities/manager.rs b/matrix_sdk_crypto/src/identities/manager.rs index d91ffa2c..993dd5a4 100644 --- a/matrix_sdk_crypto/src/identities/manager.rs +++ b/matrix_sdk_crypto/src/identities/manager.rs @@ -378,6 +378,7 @@ pub(crate) mod test { use matrix_sdk_common::{ api::r0::keys::get_keys::Response as KeyQueryResponse, identifiers::{room_id, user_id, DeviceIdBox, RoomId, UserId}, + locks::Mutex, }; use matrix_sdk_test::async_test; @@ -387,7 +388,7 @@ pub(crate) mod test { use crate::{ identities::IdentityManager, machine::test::response_from_file, - olm::{Account, ReadOnlyAccount}, + olm::{Account, PrivateCrossSigningIdentity, ReadOnlyAccount}, session_manager::GroupSessionManager, store::{CryptoStore, MemoryStore, Store}, verification::VerificationMachine, @@ -410,10 +411,11 @@ pub(crate) mod test { } fn manager() -> IdentityManager { + let identity = Arc::new(Mutex::new(PrivateCrossSigningIdentity::empty(user_id()))); let user_id = Arc::new(user_id()); let account = ReadOnlyAccount::new(&user_id, &device_id()); let store: Arc> = Arc::new(Box::new(MemoryStore::new())); - let verification = VerificationMachine::new(account.clone(), store); + let verification = VerificationMachine::new(account.clone(), identity, store); let store = Store::new( user_id.clone(), Arc::new(Box::new(MemoryStore::new())), diff --git a/matrix_sdk_crypto/src/identities/user.rs b/matrix_sdk_crypto/src/identities/user.rs index 83055ab2..70026de0 100644 --- a/matrix_sdk_crypto/src/identities/user.rs +++ b/matrix_sdk_crypto/src/identities/user.rs @@ -684,13 +684,13 @@ pub(crate) mod test { manager::test::{other_key_query, own_key_query}, Device, ReadOnlyDevice, }, - olm::ReadOnlyAccount, + olm::{PrivateCrossSigningIdentity, ReadOnlyAccount}, store::MemoryStore, verification::VerificationMachine, }; use matrix_sdk_common::{ - api::r0::keys::get_keys::Response as KeyQueryResponse, identifiers::user_id, + api::r0::keys::get_keys::Response as KeyQueryResponse, identifiers::user_id, locks::Mutex, }; use super::{OwnUserIdentity, UserIdentities, UserIdentity}; @@ -752,8 +752,12 @@ pub(crate) mod test { assert!(identity.is_device_signed(&first).is_err()); assert!(identity.is_device_signed(&second).is_ok()); + let private_identity = Arc::new(Mutex::new(PrivateCrossSigningIdentity::empty( + second.user_id().clone(), + ))); let verification_machine = VerificationMachine::new( ReadOnlyAccount::new(second.user_id(), second.device_id()), + private_identity, Arc::new(Box::new(MemoryStore::new())), ); diff --git a/matrix_sdk_crypto/src/key_request.rs b/matrix_sdk_crypto/src/key_request.rs index 3a4d26cf..ba98f62e 100644 --- a/matrix_sdk_crypto/src/key_request.rs +++ b/matrix_sdk_crypto/src/key_request.rs @@ -674,13 +674,14 @@ mod test { AnyToDeviceEvent, ToDeviceEvent, }, identifiers::{room_id, user_id, DeviceIdBox, RoomId, UserId}, + locks::Mutex, }; use matrix_sdk_test::async_test; use std::{convert::TryInto, sync::Arc}; use crate::{ identities::{LocalTrust, ReadOnlyDevice}, - olm::{Account, ReadOnlyAccount}, + olm::{Account, PrivateCrossSigningIdentity, ReadOnlyAccount}, store::{CryptoStore, MemoryStore, Store}, verification::VerificationMachine, }; @@ -719,7 +720,8 @@ mod test { let user_id = Arc::new(bob_id()); let account = ReadOnlyAccount::new(&user_id, &alice_device_id()); let store: Arc> = Arc::new(Box::new(MemoryStore::new())); - let verification = VerificationMachine::new(account, store.clone()); + let identity = Arc::new(Mutex::new(PrivateCrossSigningIdentity::empty(bob_id()))); + let verification = VerificationMachine::new(account, identity, store.clone()); let store = Store::new(user_id.clone(), store, verification); KeyRequestMachine::new( @@ -736,7 +738,8 @@ mod test { let account = ReadOnlyAccount::new(&user_id, &alice_device_id()); let device = ReadOnlyDevice::from_account(&account).await; let store: Arc> = Arc::new(Box::new(MemoryStore::new())); - let verification = VerificationMachine::new(account, store.clone()); + let identity = Arc::new(Mutex::new(PrivateCrossSigningIdentity::empty(alice_id()))); + let verification = VerificationMachine::new(account, identity, store.clone()); let store = Store::new(user_id.clone(), store, verification); store.save_devices(&[device]).await.unwrap(); diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 0db52888..ba381ee8 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -140,9 +140,11 @@ impl OlmMachine { user_identity: PrivateCrossSigningIdentity, ) -> Self { let user_id = Arc::new(user_id.clone()); + let user_identity = Arc::new(Mutex::new(user_identity)); let store = Arc::new(store); - let verification_machine = VerificationMachine::new(account.clone(), store.clone()); + let verification_machine = + VerificationMachine::new(account.clone(), user_identity.clone(), store.clone()); let store = Store::new(user_id.clone(), store, verification_machine.clone()); let device_id: Arc = Arc::new(device_id); let outbound_group_sessions = Arc::new(DashMap::new()); @@ -185,7 +187,7 @@ impl OlmMachine { verification_machine, key_request_machine, identity_manager, - user_identity: Arc::new(Mutex::new(user_identity)), + user_identity, } } @@ -224,8 +226,16 @@ impl OlmMachine { } }; - // TODO load the identity like we load an account. - let identity = PrivateCrossSigningIdentity::empty(user_id.clone()); + let identity = match store.load_identity().await? { + Some(i) => { + debug!("Restored the cross signing identity"); + i + } + None => { + debug!("Creating an empty cross signing identity stub"); + PrivateCrossSigningIdentity::empty(user_id.clone()) + } + }; Ok(OlmMachine::new_helper( &user_id, device_id, store, account, identity, @@ -344,8 +354,9 @@ impl OlmMachine { /// Mark the cross signing identity as shared. async fn receive_cross_signing_upload_response(&self) -> StoreResult<()> { self.user_identity.lock().await.mark_as_shared(); - // TOOD save the identity. - Ok(()) + self.store + .save_identity((&*self.user_identity.lock().await).clone()) + .await } /// Create a new cross signing identity and get the upload request to push @@ -356,11 +367,12 @@ impl OlmMachine { /// devices. /// /// Uploading these keys will require user interactive auth. - pub async fn bootstrap_cross_signing(&self) -> UploadSigningKeysRequest { - let mut lock = self.user_identity.lock().await; - *lock = PrivateCrossSigningIdentity::new(self.user_id().to_owned()).await; - // TODO save the identity. - lock.as_upload_request().await + pub async fn bootstrap_cross_signing(&self) -> StoreResult { + // TODO should we save the request until we get a response? + let mut identity = self.user_identity.lock().await; + *identity = PrivateCrossSigningIdentity::new(self.user_id().to_owned()).await; + self.store.save_identity(identity.clone()).await?; + Ok(identity.as_upload_request().await) } /// Should device or one-time keys be uploaded to the server. diff --git a/matrix_sdk_crypto/src/session_manager/sessions.rs b/matrix_sdk_crypto/src/session_manager/sessions.rs index c86c09ac..29af3b6f 100644 --- a/matrix_sdk_crypto/src/session_manager/sessions.rs +++ b/matrix_sdk_crypto/src/session_manager/sessions.rs @@ -311,6 +311,7 @@ impl SessionManager { #[cfg(test)] mod test { use dashmap::DashMap; + use matrix_sdk_common::locks::Mutex; use std::{collections::BTreeMap, sync::Arc}; use matrix_sdk_common::{ @@ -324,7 +325,7 @@ mod test { use crate::{ identities::ReadOnlyDevice, key_request::KeyRequestMachine, - olm::{Account, ReadOnlyAccount}, + olm::{Account, PrivateCrossSigningIdentity, ReadOnlyAccount}, store::{CryptoStore, MemoryStore, Store}, verification::VerificationMachine, }; @@ -350,8 +351,10 @@ mod test { let account = ReadOnlyAccount::new(&user_id, &device_id); let store: Arc> = Arc::new(Box::new(MemoryStore::new())); store.save_account(account.clone()).await.unwrap(); - - let verification = VerificationMachine::new(account.clone(), store.clone()); + let identity = Arc::new(Mutex::new(PrivateCrossSigningIdentity::empty( + user_id.clone(), + ))); + let verification = VerificationMachine::new(account.clone(), identity, store.clone()); let user_id = Arc::new(user_id); let device_id = Arc::new(device_id); diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index 71e3cb19..ba098ddc 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -16,6 +16,7 @@ use std::sync::Arc; use dashmap::DashMap; +use matrix_sdk_common::locks::Mutex; use tracing::{trace, warn}; use matrix_sdk_common::{ @@ -26,6 +27,7 @@ use matrix_sdk_common::{ use super::sas::{content_to_request, Sas}; use crate::{ + olm::PrivateCrossSigningIdentity, requests::{OutgoingRequest, ToDeviceRequest}, store::{CryptoStore, CryptoStoreError}, ReadOnlyAccount, ReadOnlyDevice, @@ -34,15 +36,21 @@ use crate::{ #[derive(Clone, Debug)] pub struct VerificationMachine { account: ReadOnlyAccount, + user_identity: Arc>, pub(crate) store: Arc>, verifications: Arc>, outgoing_to_device_messages: Arc>, } impl VerificationMachine { - pub(crate) fn new(account: ReadOnlyAccount, store: Arc>) -> Self { + pub(crate) fn new( + account: ReadOnlyAccount, + identity: Arc>, + store: Arc>, + ) -> Self { Self { account, + user_identity: identity, store, verifications: Arc::new(DashMap::new()), outgoing_to_device_messages: Arc::new(DashMap::new()), @@ -224,10 +232,12 @@ mod test { use matrix_sdk_common::{ events::AnyToDeviceEventContent, identifiers::{DeviceId, UserId}, + locks::Mutex, }; use super::{Sas, VerificationMachine}; use crate::{ + olm::PrivateCrossSigningIdentity, requests::OutgoingRequests, store::{CryptoStore, MemoryStore}, verification::test::{get_content_from_request, wrap_any_to_device_content}, @@ -263,7 +273,8 @@ mod test { bob_store.save_devices(vec![alice_device.clone()]).await; let bob_store: Arc> = Arc::new(Box::new(bob_store)); - let machine = VerificationMachine::new(alice, Arc::new(Box::new(store))); + let identity = Arc::new(Mutex::new(PrivateCrossSigningIdentity::empty(alice_id()))); + let machine = VerificationMachine::new(alice, identity, Arc::new(Box::new(store))); let (bob_sas, start_content) = Sas::start(bob, alice_device, bob_store, None); machine .receive_event(&mut wrap_any_to_device_content( @@ -279,8 +290,9 @@ mod test { #[test] fn create() { let alice = ReadOnlyAccount::new(&alice_id(), &alice_device_id()); + let identity = Arc::new(Mutex::new(PrivateCrossSigningIdentity::empty(alice_id()))); let store = MemoryStore::new(); - let _ = VerificationMachine::new(alice, Arc::new(Box::new(store))); + let _ = VerificationMachine::new(alice, identity, Arc::new(Box::new(store))); } #[tokio::test] From 8ed1e37cef1b5ac33c81920c0569f8b70a9fb3ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 23 Oct 2020 11:17:37 +0200 Subject: [PATCH 33/34] crypto: Save the account if we create a new one. --- matrix_sdk_crypto/src/machine.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index ba381ee8..cb80a008 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -222,7 +222,9 @@ impl OlmMachine { } None => { debug!("Creating a new account"); - ReadOnlyAccount::new(&user_id, &device_id) + let account = ReadOnlyAccount::new(&user_id, &device_id); + store.save_account(account.clone()).await?; + account } }; From 4cc803fe2767360ae84d97969a8fea9fe79483e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Sat, 24 Oct 2020 10:32:17 +0200 Subject: [PATCH 34/34] crypto: WIP cross signing bootstrap. --- .../examples/cross_signing_bootstrap.rs | 119 +++++++++++++ matrix_sdk/src/client.rs | 29 +++- matrix_sdk_common/Cargo.toml | 2 +- matrix_sdk_crypto/src/error.rs | 3 + matrix_sdk_crypto/src/machine.rs | 35 +++- matrix_sdk_crypto/src/olm/account.rs | 80 +++++---- matrix_sdk_crypto/src/olm/signing.rs | 156 +++++++++++++++++- 7 files changed, 382 insertions(+), 42 deletions(-) create mode 100644 matrix_sdk/examples/cross_signing_bootstrap.rs diff --git a/matrix_sdk/examples/cross_signing_bootstrap.rs b/matrix_sdk/examples/cross_signing_bootstrap.rs new file mode 100644 index 00000000..54def1cb --- /dev/null +++ b/matrix_sdk/examples/cross_signing_bootstrap.rs @@ -0,0 +1,119 @@ +use std::{ + collections::BTreeMap, + env, io, + process::exit, + sync::atomic::{AtomicBool, Ordering}, +}; + +use serde_json::json; +use url::Url; + +use matrix_sdk::{ + self, + api::r0::uiaa::AuthData, + identifiers::{user_id, UserId}, + Client, ClientConfig, LoopCtrl, SyncSettings, +}; + +fn auth_data<'a>(user: &UserId, password: &str, session: Option<&'a str>) -> AuthData<'a> { + let mut auth_parameters = BTreeMap::new(); + let identifier = json!({ + "type": "m.id.user", + "user": user, + }); + + auth_parameters.insert("identifier".to_owned(), identifier); + auth_parameters.insert("password".to_owned(), password.to_owned().into()); + + // This is needed because of https://github.com/matrix-org/synapse/issues/5665 + auth_parameters.insert("user".to_owned(), user.as_str().into()); + + AuthData::DirectRequest { + kind: "m.login.password", + auth_parameters, + session, + } +} + +async fn bootstrap(client: Client) { + println!("Bootstrapping a new cross signing identity, press enter to continue."); + + let mut input = String::new(); + + io::stdin() + .read_line(&mut input) + .expect("error: unable to read user input"); + + if let Err(e) = client.bootstrap_cross_signing(None).await { + if let Some(response) = e.uiaa_response() { + let auth_data = auth_data( + &user_id!("@example:localhost"), + "wordpass", + response.session.as_deref(), + ); + client + .bootstrap_cross_signing(Some(auth_data)) + .await + .expect("Couldn't bootstrap cross signing") + } else { + panic!("Error durign cross signing bootstrap {:#?}", e); + } + } +} + +async fn login( + homeserver_url: String, + username: &str, + password: &str, +) -> Result<(), matrix_sdk::Error> { + let client_config = ClientConfig::new() + .disable_ssl_verification() + .proxy("http://localhost:8080") + .unwrap(); + let homeserver_url = Url::parse(&homeserver_url).expect("Couldn't parse the homeserver URL"); + let client = Client::new_with_config(homeserver_url, client_config).unwrap(); + + client + .login(username, password, None, Some("rust-sdk")) + .await?; + + let client_ref = &client; + let asked = AtomicBool::new(false); + let asked_ref = &asked; + + client + .sync_with_callback(SyncSettings::new(), |_| async move { + let asked = asked_ref; + let client = &client_ref; + + // Wait for sync to be done then ask the user to bootstrap. + if !asked.load(Ordering::SeqCst) { + tokio::spawn(bootstrap((*client).clone())); + } + + asked.store(true, Ordering::SeqCst); + LoopCtrl::Continue + }) + .await; + + Ok(()) +} + +#[tokio::main] +async fn main() -> Result<(), matrix_sdk::Error> { + tracing_subscriber::fmt::init(); + + let (homeserver_url, username, password) = + match (env::args().nth(1), env::args().nth(2), env::args().nth(3)) { + (Some(a), Some(b), Some(c)) => (a, b, c), + _ => { + eprintln!( + "Usage: {} ", + env::args().next().unwrap() + ); + exit(1) + } + }; + + login(homeserver_url, &username, &password).await +} diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index c5786eff..2f592d73 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -107,7 +107,7 @@ use matrix_sdk_common::{ #[cfg(feature = "encryption")] use matrix_sdk_common::{ api::r0::{ - keys::{get_keys, upload_keys}, + keys::{get_keys, upload_keys, upload_signing_keys::Request as UploadSigningKeysRequest}, to_device::send_event_to_device::{ Request as RumaToDeviceRequest, Response as ToDeviceResponse, }, @@ -1823,6 +1823,33 @@ impl Client { })) } + /// TODO + #[cfg(feature = "encryption")] + #[cfg_attr(feature = "docs", doc(cfg(encryption)))] + pub async fn bootstrap_cross_signing(&self, auth_data: Option>) -> Result<()> { + let olm = self + .base_client + .olm_machine() + .await + .ok_or(Error::AuthenticationRequired)?; + + let (request, signature_request) = olm.bootstrap_cross_signing(false).await?; + + println!("HELLOOO MAKING REQUEST {:#?}", request); + + let request = UploadSigningKeysRequest { + auth: auth_data, + master_key: request.master_key, + self_signing_key: request.self_signing_key, + user_signing_key: request.user_signing_key, + }; + + self.send(request).await?; + self.send(signature_request).await?; + + Ok(()) + } + /// Get a map holding all the devices of an user. /// /// This will always return an empty map if the client hasn't been logged diff --git a/matrix_sdk_common/Cargo.toml b/matrix_sdk_common/Cargo.toml index 4a074fb7..10094d0d 100644 --- a/matrix_sdk_common/Cargo.toml +++ b/matrix_sdk_common/Cargo.toml @@ -20,7 +20,7 @@ js_int = "0.1.9" [dependencies.ruma] version = "0.0.1" -git = "https://github.com/ruma/ruma" +path = "/home/poljar/werk/priv/ruma/ruma" rev = "db2f58032953ccb6d8ae712d64d713ebdf412598" features = ["client-api", "unstable-pre-spec", "unstable-exhaustive-types"] diff --git a/matrix_sdk_crypto/src/error.rs b/matrix_sdk_crypto/src/error.rs index 39a554bb..1692bd54 100644 --- a/matrix_sdk_crypto/src/error.rs +++ b/matrix_sdk_crypto/src/error.rs @@ -148,6 +148,9 @@ pub enum SignatureError { #[error("the provided JSON object can't be converted to a canonical representation")] CanonicalJsonError(CjsonError), + #[error(transparent)] + JsonError(#[from] SerdeError), + #[error("the signature didn't match the provided key")] VerificationError, } diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index cb80a008..b3d5589a 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -26,6 +26,7 @@ use matrix_sdk_common::{ claim_keys::{Request as KeysClaimRequest, Response as KeysClaimResponse}, get_keys::Response as KeysQueryResponse, upload_keys, + upload_signatures::Request as UploadSignaturesRequest, }, sync::sync_events::Response as SyncResponse, }, @@ -95,6 +96,7 @@ pub struct OlmMachine { /// State machine handling public user identities and devices, keeping track /// of when a key query needs to be done and handling one. identity_manager: IdentityManager, + cross_signing_request: Arc>>, } #[cfg(not(tarpaulin_include))] @@ -181,13 +183,14 @@ impl OlmMachine { user_id, device_id, account, + user_identity, store, session_manager, group_session_manager, verification_machine, key_request_machine, identity_manager, - user_identity, + cross_signing_request: Arc::new(Mutex::new(None)), } } @@ -369,12 +372,32 @@ impl OlmMachine { /// devices. /// /// Uploading these keys will require user interactive auth. - pub async fn bootstrap_cross_signing(&self) -> StoreResult { - // TODO should we save the request until we get a response? + pub async fn bootstrap_cross_signing( + &self, + reset: bool, + ) -> StoreResult<(UploadSigningKeysRequest, UploadSignaturesRequest)> { let mut identity = self.user_identity.lock().await; - *identity = PrivateCrossSigningIdentity::new(self.user_id().to_owned()).await; - self.store.save_identity(identity.clone()).await?; - Ok(identity.as_upload_request().await) + + if identity.is_empty().await || reset { + info!("Creating new cross signing identity"); + let (id, signature_request) = self.account.bootstrap_cross_signing().await; + let request = id.as_upload_request().await; + + *identity = id; + + self.store.save_identity(identity.clone()).await?; + Ok((request, signature_request)) + } else { + info!("Trying to upload the existing cross signing identity"); + let request = identity.as_upload_request().await; + let device_keys = self.account.unsigned_device_keys(); + // TODO remove this expect. + let signature_request = identity + .sign_device(device_keys) + .await + .expect("Can't sign device keys"); + Ok((request, signature_request)) + } } /// Should device or one-time keys be uploaded to the server. diff --git a/matrix_sdk_crypto/src/olm/account.rs b/matrix_sdk_crypto/src/olm/account.rs index 422d127f..49ce924f 100644 --- a/matrix_sdk_crypto/src/olm/account.rs +++ b/matrix_sdk_crypto/src/olm/account.rs @@ -30,7 +30,9 @@ use tracing::{debug, trace, warn}; #[cfg(test)] use matrix_sdk_common::events::EventType; use matrix_sdk_common::{ - api::r0::keys::{upload_keys, OneTimeKey, SignedKey}, + api::r0::keys::{ + upload_keys, upload_signatures::Request as SignatureUploadRequest, OneTimeKey, SignedKey, + }, encryption::DeviceKeys, events::{room::encrypted::EncryptedEventContent, AnyToDeviceEvent}, identifiers::{ @@ -50,13 +52,16 @@ use olm_rs::{ }; use crate::{ - error::{EventError, OlmResult, SessionCreationError}, + error::{EventError, OlmResult, SessionCreationError, SignatureError}, identities::ReadOnlyDevice, store::Store, OlmError, }; -use super::{EncryptionSettings, InboundGroupSession, OutboundGroupSession, Session}; +use super::{ + EncryptionSettings, InboundGroupSession, OutboundGroupSession, PrivateCrossSigningIdentity, + Session, +}; #[derive(Debug, Clone)] pub struct Account { @@ -618,9 +623,7 @@ impl ReadOnlyAccount { }) } - /// Sign the device keys of the account and return them so they can be - /// uploaded. - pub(crate) async fn device_keys(&self) -> DeviceKeys { + pub(crate) fn unsigned_device_keys(&self) -> DeviceKeys { let identity_keys = self.identity_keys(); let mut keys = BTreeMap::new(); @@ -634,34 +637,41 @@ impl ReadOnlyAccount { identity_keys.ed25519().to_owned(), ); - let device_keys = json!({ - "user_id": (*self.user_id).clone(), - "device_id": (*self.device_id).clone(), - "algorithms": Self::ALGORITHMS, - "keys": keys, - }); - - let mut signatures = BTreeMap::new(); - - let mut signature = BTreeMap::new(); - signature.insert( - DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, &self.device_id), - self.sign_json(&device_keys).await, - ); - signatures.insert((*self.user_id).clone(), signature); - DeviceKeys::new( (*self.user_id).clone(), (*self.device_id).clone(), - vec![ - EventEncryptionAlgorithm::OlmV1Curve25519AesSha2, - EventEncryptionAlgorithm::MegolmV1AesSha2, - ], + Self::ALGORITHMS.iter().map(|a| (&**a).clone()).collect(), keys, - signatures, + BTreeMap::new(), ) } + /// Sign the device keys of the account and return them so they can be + /// uploaded. + pub(crate) async fn device_keys(&self) -> DeviceKeys { + let mut device_keys = self.unsigned_device_keys(); + let jsond_device_keys = serde_json::to_value(&device_keys).unwrap(); + + device_keys + .signatures + .entry(self.user_id().clone()) + .or_insert_with(BTreeMap::new) + .insert( + DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, &self.device_id), + self.sign_json(jsond_device_keys) + .await + .expect("Can't sign own device keys"), + ); + + device_keys + } + + pub(crate) async fn bootstrap_cross_signing( + &self, + ) -> (PrivateCrossSigningIdentity, SignatureUploadRequest) { + PrivateCrossSigningIdentity::new_with_account(self).await + } + /// Convert a JSON value to the canonical representation and sign the JSON /// string. /// @@ -673,10 +683,13 @@ impl ReadOnlyAccount { /// # Panic /// /// Panics if the json value can't be serialized. - pub async fn sign_json(&self, json: &Value) -> String { - let canonical_json = cjson::to_string(json) - .unwrap_or_else(|_| panic!(format!("Can't serialize {} to canonical JSON", json))); - self.sign(&canonical_json).await + pub async fn sign_json(&self, mut json: Value) -> Result { + let json_object = json.as_object_mut().ok_or(SignatureError::NotAnObject)?; + let _ = json_object.remove("unsigned"); + let _ = json_object.remove("signatures"); + + let canonical_json = cjson::to_string(&json)?; + Ok(self.sign(&canonical_json).await) } pub(crate) async fn signed_one_time_keys_helper( @@ -690,7 +703,10 @@ impl ReadOnlyAccount { "key": key, }); - let signature = self.sign_json(&key_json).await; + let signature = self + .sign_json(key_json) + .await + .expect("Can't sign own one-time keys"); let mut signature_map = BTreeMap::new(); diff --git a/matrix_sdk_crypto/src/olm/signing.rs b/matrix_sdk_crypto/src/olm/signing.rs index dfde208b..d4332501 100644 --- a/matrix_sdk_crypto/src/olm/signing.rs +++ b/matrix_sdk_crypto/src/olm/signing.rs @@ -20,8 +20,12 @@ use aes_gcm::{ }; use base64::{decode_config, encode_config, DecodeError, URL_SAFE_NO_PAD}; use getrandom::getrandom; +use matrix_sdk_common::{ + encryption::DeviceKeys, + identifiers::{DeviceKeyAlgorithm, DeviceKeyId}, +}; use serde::{Deserialize, Serialize}; -use serde_json::Error as JsonError; +use serde_json::{Error as JsonError, Value}; use std::{ collections::BTreeMap, sync::{ @@ -35,16 +39,22 @@ use zeroize::Zeroizing; use olm_rs::{errors::OlmUtilityError, pk::OlmPkSigning, utility::OlmUtility}; use matrix_sdk_common::{ - api::r0::keys::{CrossSigningKey, KeyUsage}, + api::r0::keys::{ + upload_signatures::Request as SignatureUploadRequest, CrossSigningKey, KeyUsage, + }, identifiers::UserId, locks::Mutex, }; use crate::{ + error::SignatureError, identities::{MasterPubkey, SelfSigningPubkey, UserSigningPubkey}, requests::UploadSigningKeysRequest, + UserIdentity, }; +use crate::ReadOnlyAccount; + const NONCE_SIZE: usize = 12; fn encode>(input: T) -> String { @@ -163,6 +173,10 @@ impl UserSigning { PickledUserSigning { pickle, public_key } } + async fn sign_user(&self, _: &UserIdentity) -> BTreeMap> { + todo!(); + } + fn from_pickle(pickle: PickledUserSigning, pickle_key: &[u8]) -> Result { let inner = Signing::from_pickle(pickle.pickle, pickle_key)?; @@ -180,6 +194,29 @@ impl SelfSigning { PickledSelfSigning { pickle, public_key } } + async fn sign_device_raw(&self, value: Value) -> Result { + self.inner.sign_json(value).await + } + + async fn sign_device(&self, device_keys: &mut DeviceKeys) -> Result<(), SignatureError> { + let json_device = serde_json::to_value(&device_keys)?; + let signature = self.sign_device_raw(json_device).await?; + + device_keys + .signatures + .entry(self.public_key.user_id().to_owned()) + .or_insert_with(BTreeMap::new) + .insert( + DeviceKeyId::from_parts( + DeviceKeyAlgorithm::Ed25519, + self.inner.public_key.as_str().into(), + ), + signature.0, + ); + + Ok(()) + } + fn from_pickle(pickle: PickledSelfSigning, pickle_key: &[u8]) -> Result { let inner = Signing::from_pickle(pickle.pickle, pickle_key)?; @@ -346,6 +383,13 @@ impl Signing { utility.ed25519_verify(self.public_key.as_str(), message, signature.as_str()) } + async fn sign_json(&self, mut json: Value) -> Result { + let json_object = json.as_object_mut().ok_or(SignatureError::NotAnObject)?; + let _ = json_object.remove("signatures"); + let canonical_json = cjson::to_string(json_object)?; + Ok(self.sign(&canonical_json).await) + } + async fn sign(&self, message: &str) -> Signature { Signature(self.inner.lock().await.sign(message)) } @@ -356,6 +400,14 @@ impl PrivateCrossSigningIdentity { &self.user_id } + pub async fn is_empty(&self) -> bool { + let has_master = self.master_key.lock().await.is_some(); + let has_user = self.user_signing_key.lock().await.is_some(); + let has_self = self.self_signing_key.lock().await.is_some(); + + !(has_master && has_user && has_self) + } + pub(crate) fn empty(user_id: UserId) -> Self { Self { user_id: Arc::new(user_id), @@ -366,6 +418,94 @@ impl PrivateCrossSigningIdentity { } } + pub(crate) async fn sign_device( + &self, + mut device_keys: DeviceKeys, + ) -> Result { + self.self_signing_key + .lock() + .await + .as_ref() + .ok_or(SignatureError::MissingSigningKey)? + .sign_device(&mut device_keys) + .await?; + + let mut signed_keys = BTreeMap::new(); + signed_keys + .entry((&*self.user_id).to_owned()) + .or_insert_with(BTreeMap::new) + .insert( + device_keys.device_id.to_string(), + serde_json::to_value(device_keys)?, + ); + + Ok(SignatureUploadRequest { signed_keys }) + } + + pub(crate) async fn new_with_account( + account: &ReadOnlyAccount, + ) -> (Self, SignatureUploadRequest) { + let master = Signing::new(); + + let mut public_key = + master.cross_signing_key(account.user_id().to_owned(), KeyUsage::Master); + let signature = account + .sign_json( + serde_json::to_value(&public_key) + .expect("Can't convert own public master key to json"), + ) + .await + .expect("Can't sign own public master key"); + public_key + .signatures + .entry(account.user_id().to_owned()) + .or_insert_with(BTreeMap::new) + .insert(format!("ed25519:{}", account.device_id()), signature); + + let master = MasterSigning { + inner: master, + public_key: public_key.into(), + }; + + let identity = Self::new_helper(account.user_id(), master).await; + let device_keys = account.unsigned_device_keys(); + let request = identity + .sign_device(device_keys) + .await + .expect("Can't sign own device with new cross signign keys"); + + (identity, request) + } + + async fn new_helper(user_id: &UserId, master: MasterSigning) -> Self { + let user = Signing::new(); + let mut public_key = user.cross_signing_key(user_id.to_owned(), KeyUsage::UserSigning); + master.sign_subkey(&mut public_key).await; + + let user = UserSigning { + inner: user, + public_key: public_key.into(), + }; + + let self_signing = Signing::new(); + let mut public_key = + self_signing.cross_signing_key(user_id.to_owned(), KeyUsage::SelfSigning); + master.sign_subkey(&mut public_key).await; + + let self_signing = SelfSigning { + inner: self_signing, + public_key: public_key.into(), + }; + + Self { + user_id: Arc::new(user_id.to_owned()), + shared: Arc::new(AtomicBool::new(false)), + master_key: Arc::new(Mutex::new(Some(master))), + self_signing_key: Arc::new(Mutex::new(Some(self_signing))), + user_signing_key: Arc::new(Mutex::new(Some(user))), + } + } + pub(crate) async fn new(user_id: UserId) -> Self { let master = Signing::new(); @@ -520,6 +660,8 @@ impl PrivateCrossSigningIdentity { #[cfg(test)] mod test { + use crate::olm::ReadOnlyAccount; + use super::{PrivateCrossSigningIdentity, Signing}; use matrix_sdk_common::identifiers::{user_id, UserId}; @@ -617,4 +759,14 @@ mod test { &*unpickled.self_signing_key.lock().await ); } + + #[async_test] + async fn private_identity_signed_by_accound() { + let account = ReadOnlyAccount::new(&user_id(), "DEVICEID".into()); + let (identity, _) = PrivateCrossSigningIdentity::new_with_account(&account).await; + let master = identity.master_key.lock().await; + let master = master.as_ref().unwrap(); + + assert!(!master.public_key.signatures().is_empty()); + } }