From f96437a242fe30c32e692bb93359f5616c8b5f92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 18 Aug 2020 12:50:03 +0200 Subject: [PATCH 01/49] crypto: Initial scaffolding for handling user identities in key queries. --- matrix_sdk_crypto/src/machine.rs | 69 +++++++++++++++++++++- matrix_sdk_crypto/src/store/memorystore.rs | 5 ++ matrix_sdk_crypto/src/store/mod.rs | 8 +++ matrix_sdk_crypto/src/store/sqlite.rs | 5 ++ matrix_sdk_crypto/src/user_identity.rs | 52 ++++++++++++++-- 5 files changed, 131 insertions(+), 8 deletions(-) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 4fd34053..2543b518 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -60,6 +60,10 @@ use super::{ OlmMessage, OutboundGroupSession, }, store::{memorystore::MemoryStore, Result as StoreResult}, + user_identity::{ + MasterPubkey, OwnUserIdentity, SelfSigningPubkey, UserIdentities, UserIdentity, + UserSigningPubkey, + }, verification::{Sas, VerificationMachine}, CryptoStore, }; @@ -429,6 +433,9 @@ impl OlmMachine { let mut changed_devices = Vec::new(); for (user_id, device_map) in device_keys_map { + // TODO move this out into the handle keys query response method + // since we might fail handle the new device at any point here or + // when updating the user identities. self.store.update_tracked_user(user_id, false).await?; for (device_id, device_keys) in device_map.iter() { @@ -492,6 +499,63 @@ impl OlmMachine { Ok(changed_devices) } + async fn handle_cross_singing_keys( + &self, + response: &get_keys::Response, + ) -> StoreResult> { + let mut changed = Vec::new(); + + for (user_id, master_key) in &response.master_keys { + let master_key = MasterPubkey::from(master_key); + + let self_signing = if let Some(s) = response.self_signing_keys.get(user_id) { + SelfSigningPubkey::from(s) + } else { + continue; + }; + + let identity = 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) + { + UserSigningPubkey::from(s) + } else { + continue; + }; + + identity + .update(master_key, self_signing, user_signing) + .map(|_| i) + } + UserIdentities::Other(ref mut identity) => { + identity.update(master_key, self_signing).map(|_| i) + } + } + } else if user_id == self.user_id() { + if let Some(s) = response.user_signing_keys.get(user_id) { + let user_signing = UserSigningPubkey::from(s); + OwnUserIdentity::new(master_key, self_signing, user_signing) + .map(UserIdentities::Own) + } else { + continue; + } + } else { + UserIdentity::new(master_key, self_signing).map(UserIdentities::Other) + }; + + match identity { + Ok(i) => changed.push(i), + Err(_e) => { + // TODO log some error here + continue; + } + } + } + + Ok(changed) + } + /// Receive a successful keys query response. /// /// Returns a list of devices newly discovered devices and devices that @@ -504,13 +568,14 @@ impl OlmMachine { pub async fn receive_keys_query_response( &self, response: &get_keys::Response, - ) -> OlmResult> { + ) -> OlmResult<(Vec, Vec)> { 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?; - Ok(changed_devices) + Ok((changed_devices, changed_identities)) } /// Get a request to upload E2EE keys to the server. diff --git a/matrix_sdk_crypto/src/store/memorystore.rs b/matrix_sdk_crypto/src/store/memorystore.rs index 660b3e90..4cbe31db 100644 --- a/matrix_sdk_crypto/src/store/memorystore.rs +++ b/matrix_sdk_crypto/src/store/memorystore.rs @@ -25,6 +25,7 @@ use super::{Account, CryptoStore, InboundGroupSession, Result, Session}; use crate::{ device::ReadOnlyDevice, memory_stores::{DeviceStore, GroupSessionStore, ReadOnlyUserDevices, SessionStore}, + user_identity::UserIdentities, }; #[derive(Debug, Clone)] pub struct MemoryStore { @@ -131,6 +132,10 @@ impl CryptoStore for MemoryStore { Ok(()) } + + async fn get_user_identity(&self, _user_id: &UserId) -> Result> { + Ok(None) + } } #[cfg(test)] diff --git a/matrix_sdk_crypto/src/store/mod.rs b/matrix_sdk_crypto/src/store/mod.rs index 5e01d8a1..6273a9ba 100644 --- a/matrix_sdk_crypto/src/store/mod.rs +++ b/matrix_sdk_crypto/src/store/mod.rs @@ -30,6 +30,7 @@ use super::{ device::ReadOnlyDevice, memory_stores::ReadOnlyUserDevices, olm::{Account, InboundGroupSession, Session}, + user_identity::UserIdentities, }; pub mod memorystore; @@ -197,4 +198,11 @@ pub trait CryptoStore: Debug { /// /// * `user_id` - The user for which we should get all the devices. async fn get_user_devices(&self, user_id: &UserId) -> Result; + + /// Get the user identity that is attached to the given user id. + /// + /// # Arguments + /// + /// * `user_id` - The user for which we should get the identity. + async fn get_user_identity(&self, user_id: &UserId) -> Result>; } diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index 1946a376..a44417aa 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -38,6 +38,7 @@ use super::{CryptoStore, CryptoStoreError, Result}; use crate::{ device::{ReadOnlyDevice, TrustState}, memory_stores::{DeviceStore, GroupSessionStore, ReadOnlyUserDevices, SessionStore}, + user_identity::UserIdentities, Account, IdentityKeys, InboundGroupSession, Session, }; @@ -883,6 +884,10 @@ impl CryptoStore for SqliteStore { async fn get_user_devices(&self, user_id: &UserId) -> Result { Ok(self.devices.user_devices(user_id)) } + + async fn get_user_identity(&self, _user_id: &UserId) -> Result> { + Ok(None) + } } #[cfg(not(tarpaulin_include))] diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index 77f349d0..cabd1c04 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -77,12 +77,6 @@ impl<'a> CrossSigningSubKeys<'a> { } } -pub struct UserIdentity { - user_id: Arc, - master_key: MasterPubkey, - self_signing_key: SelfSigningPubkey, -} - impl MasterPubkey { fn verify_subkey<'a>( &self, @@ -151,6 +145,19 @@ impl SelfSigningPubkey { } } +#[derive(Debug, Clone)] +pub enum UserIdentities { + Own(OwnUserIdentity), + Other(UserIdentity), +} + +#[derive(Debug, Clone)] +pub struct UserIdentity { + user_id: Arc, + master_key: MasterPubkey, + self_signing_key: SelfSigningPubkey, +} + impl UserIdentity { pub fn new( master_key: MasterPubkey, @@ -165,11 +172,25 @@ impl UserIdentity { }) } + pub fn update( + &mut self, + master_key: MasterPubkey, + self_signing_key: SelfSigningPubkey, + ) -> Result<(), SignatureError> { + master_key.verify_subkey(&self_signing_key)?; + + self.master_key = master_key; + self.self_signing_key = self_signing_key; + + Ok(()) + } + pub fn is_device_signed(&self, device: &ReadOnlyDevice) -> Result<(), SignatureError> { self.self_signing_key.verify_device(device) } } +#[derive(Debug, Clone)] pub struct OwnUserIdentity { user_id: Arc, master_key: MasterPubkey, @@ -196,6 +217,25 @@ impl OwnUserIdentity { }) } + pub fn update( + &mut self, + master_key: MasterPubkey, + self_signing_key: SelfSigningPubkey, + user_signing_key: UserSigningPubkey, + ) -> Result<(), SignatureError> { + master_key.verify_subkey(&self_signing_key)?; + master_key.verify_subkey(&user_signing_key)?; + + self.self_signing_key = self_signing_key; + self.user_signing_key = user_signing_key; + + self.master_key = master_key; + + // FIXME reset the verification state if the master key changed. + + Ok(()) + } + pub fn is_identity_signed(&self, identity: &UserIdentity) -> Result<(), SignatureError> { self.user_signing_key .verify_master_key(&identity.master_key) From 6d0b73cb3db8cab1c6eeec8786f64a289fa8fcc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 18 Aug 2020 13:37:02 +0200 Subject: [PATCH 02/49] crypto: Pass the user identity to the SAS object when doing verifications. --- matrix_sdk/src/device.rs | 2 +- matrix_sdk_crypto/src/device.rs | 6 +++-- matrix_sdk_crypto/src/machine.rs | 2 +- matrix_sdk_crypto/src/verification/machine.rs | 27 +++++++++++++++---- matrix_sdk_crypto/src/verification/sas/mod.rs | 14 +++++++--- 5 files changed, 39 insertions(+), 12 deletions(-) diff --git a/matrix_sdk/src/device.rs b/matrix_sdk/src/device.rs index 44812ba1..1ffb51c5 100644 --- a/matrix_sdk/src/device.rs +++ b/matrix_sdk/src/device.rs @@ -61,7 +61,7 @@ impl Device { /// # }); /// ``` pub async fn start_verification(&self) -> Result { - let (sas, request) = self.inner.start_verification(); + let (sas, request) = self.inner.start_verification().await?; let request = ToDeviceRequest { event_type: request.event_type, txn_id: &request.txn_id, diff --git a/matrix_sdk_crypto/src/device.rs b/matrix_sdk_crypto/src/device.rs index f8eb6e49..b6fbcbd9 100644 --- a/matrix_sdk_crypto/src/device.rs +++ b/matrix_sdk_crypto/src/device.rs @@ -72,8 +72,10 @@ impl Device { /// Start a interactive verification with this `Device` /// /// Returns a `Sas` object and to-device request that needs to be sent out. - pub fn start_verification(&self) -> (Sas, OwnedToDeviceRequest) { - self.verification_machine.start_sas(self.inner.clone()) + pub async fn start_verification(&self) -> StoreResult<(Sas, OwnedToDeviceRequest)> { + self.verification_machine + .start_sas(self.inner.clone()) + .await } /// Set the trust state of the device to the given state. diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 2543b518..3fb2e912 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -2063,7 +2063,7 @@ pub(crate) mod test { assert!(!bob_device.is_trusted()); - let (alice_sas, request) = bob_device.start_verification(); + let (alice_sas, request) = bob_device.start_verification().await.unwrap(); let mut event = request_to_event(alice.user_id(), &request); bob.handle_verification_event(&mut event).await; diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index d6382b7a..d496fcad 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -45,8 +45,19 @@ impl VerificationMachine { } } - pub fn start_sas(&self, device: ReadOnlyDevice) -> (Sas, OwnedToDeviceRequest) { - let (sas, content) = Sas::start(self.account.clone(), device.clone(), self.store.clone()); + pub async fn start_sas( + &self, + device: ReadOnlyDevice, + ) -> Result<(Sas, OwnedToDeviceRequest), CryptoStoreError> { + let identity = self.store.get_user_identity(device.user_id()).await?; + + let (sas, content) = Sas::start( + self.account.clone(), + device.clone(), + self.store.clone(), + identity, + ); + let request = content_to_request( device.user_id(), device.device_id(), @@ -56,7 +67,7 @@ impl VerificationMachine { self.verifications .insert(sas.flow_id().to_owned(), sas.clone()); - (sas, request) + Ok((sas, request)) } pub fn get_sas(&self, transaction_id: &str) -> Option { @@ -128,7 +139,13 @@ impl VerificationMachine { .get_device(&e.sender, &e.content.from_device) .await? { - match Sas::from_start_event(self.account.clone(), d, self.store.clone(), e) { + match Sas::from_start_event( + self.account.clone(), + d, + self.store.clone(), + e, + None, + ) { Ok(s) => { self.verifications .insert(e.content.transaction_id.clone(), s); @@ -231,7 +248,7 @@ mod test { .unwrap(); let machine = VerificationMachine::new(alice, Arc::new(Box::new(store))); - let (bob_sas, start_content) = Sas::start(bob, alice_device, bob_store); + let (bob_sas, start_content) = Sas::start(bob, alice_device, bob_store, None); machine .receive_event(&mut wrap_any_to_device_content( bob_sas.user_id(), diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index 6e79a4e5..eebcac46 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -33,7 +33,10 @@ use matrix_sdk_common::{ identifiers::{DeviceId, UserId}, }; -use crate::{Account, CryptoStore, CryptoStoreError, ReadOnlyDevice, TrustState}; +use crate::{ + user_identity::UserIdentities, Account, CryptoStore, CryptoStoreError, ReadOnlyDevice, + TrustState, +}; pub use helpers::content_to_request; use sas_state::{ @@ -47,6 +50,7 @@ pub struct Sas { store: Arc>, account: Account, other_device: ReadOnlyDevice, + other_identity: Option, flow_id: Arc, } @@ -101,6 +105,7 @@ impl Sas { account: Account, other_device: ReadOnlyDevice, store: Arc>, + other_identity: Option, ) -> (Sas, StartEventContent) { let (inner, content) = InnerSas::start(account.clone(), other_device.clone()); let flow_id = inner.verification_flow_id(); @@ -111,6 +116,7 @@ impl Sas { store, other_device, flow_id, + other_identity, }; (sas, content) @@ -131,6 +137,7 @@ impl Sas { other_device: ReadOnlyDevice, store: Arc>, event: &ToDeviceEvent, + other_identity: Option, ) -> Result { let inner = InnerSas::from_start_event(account.clone(), other_device.clone(), event)?; let flow_id = inner.verification_flow_id(); @@ -138,6 +145,7 @@ impl Sas { inner: Arc::new(Mutex::new(inner)), account, other_device, + other_identity, store, flow_id, }) @@ -683,10 +691,10 @@ mod test { .await .unwrap(); - let (alice, content) = Sas::start(alice, bob_device, alice_store); + let (alice, content) = Sas::start(alice, bob_device, alice_store, None); let event = wrap_to_device_event(alice.user_id(), content); - let bob = Sas::from_start_event(bob, alice_device, bob_store, &event).unwrap(); + let bob = Sas::from_start_event(bob, alice_device, bob_store, &event, None).unwrap(); let mut event = wrap_any_to_device_content( bob.user_id(), get_content_from_request(&bob.accept().unwrap()), From 38cf771f1fc82860e89fd9e2527670ce6732d137 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 18 Aug 2020 14:24:27 +0200 Subject: [PATCH 03/49] crypto: Pass the identity further through the SAS layer and try to verify it. --- matrix_sdk_crypto/src/user_identity.rs | 18 ++++++++++++ .../src/verification/sas/helpers.rs | 23 ++++++++++++++- matrix_sdk_crypto/src/verification/sas/mod.rs | 29 ++++++++++++++----- .../src/verification/sas/sas_state.rs | 23 ++++++++++----- 4 files changed, 78 insertions(+), 15 deletions(-) diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index cabd1c04..6141ea67 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::{ + collections::BTreeMap, convert::TryFrom, sync::{atomic::AtomicBool, Arc}, }; @@ -151,6 +152,15 @@ pub enum UserIdentities { Other(UserIdentity), } +impl UserIdentities { + pub fn master_key(&self) -> &BTreeMap { + match self { + UserIdentities::Own(i) => i.master_key(), + UserIdentities::Other(i) => i.master_key(), + } + } +} + #[derive(Debug, Clone)] pub struct UserIdentity { user_id: Arc, @@ -172,6 +182,10 @@ impl UserIdentity { }) } + pub fn master_key(&self) -> &BTreeMap { + &self.master_key.0.keys + } + pub fn update( &mut self, master_key: MasterPubkey, @@ -236,6 +250,10 @@ impl OwnUserIdentity { Ok(()) } + pub fn master_key(&self) -> &BTreeMap { + &self.master_key.0.keys + } + pub fn is_identity_signed(&self, identity: &UserIdentity) -> Result<(), SignatureError> { self.user_signing_key .verify_master_key(&identity.master_key) diff --git a/matrix_sdk_crypto/src/verification/sas/helpers.rs b/matrix_sdk_crypto/src/verification/sas/helpers.rs index ac364ba7..d644884f 100644 --- a/matrix_sdk_crypto/src/verification/sas/helpers.rs +++ b/matrix_sdk_crypto/src/verification/sas/helpers.rs @@ -30,12 +30,13 @@ use matrix_sdk_common::{ uuid::Uuid, }; -use crate::{Account, ReadOnlyDevice}; +use crate::{user_identity::UserIdentities, Account, ReadOnlyDevice}; #[derive(Clone, Debug)] pub struct SasIds { pub account: Account, pub other_device: ReadOnlyDevice, + pub other_identity: Option, } /// Get a tuple of an emoji and a description of the emoji using a number. @@ -160,6 +161,7 @@ pub fn receive_mac_event( event: &ToDeviceEvent, ) -> Result<(Vec, Vec), CancelCode> { let mut verified_devices = Vec::new(); + let mut verified_identities = Vec::new(); let info = extra_mac_info_receive(&ids, flow_id); @@ -201,6 +203,25 @@ pub fn receive_mac_event( } else { return Err(CancelCode::KeyMismatch); } + } else if let Some(identity) = &ids.other_identity { + if let Some(key) = identity.master_key().get(key_id.as_str()) { + // TODO we should check that the master key signs the device, + // this way we know the master key also trusts the device + if key_mac + == &sas + .calculate_mac(key, &format!("{}{}", info, key_id)) + .expect("Can't calculate SAS MAC") + { + trace!( + "Successfully verified the master key {} from {}", + key_id, + event.sender + ); + verified_identities.push(identity) + } else { + return Err(CancelCode::KeyMismatch); + } + } } else { warn!( "Key ID {} in MAC event from {} {} doesn't belong to any device \ diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index eebcac46..c3837203 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -107,7 +107,11 @@ impl Sas { store: Arc>, other_identity: Option, ) -> (Sas, StartEventContent) { - let (inner, content) = InnerSas::start(account.clone(), other_device.clone()); + let (inner, content) = InnerSas::start( + account.clone(), + other_device.clone(), + other_identity.clone(), + ); let flow_id = inner.verification_flow_id(); let sas = Sas { @@ -139,7 +143,12 @@ impl Sas { event: &ToDeviceEvent, other_identity: Option, ) -> Result { - let inner = InnerSas::from_start_event(account.clone(), other_device.clone(), event)?; + let inner = InnerSas::from_start_event( + account.clone(), + other_device.clone(), + event, + other_identity.clone(), + )?; let flow_id = inner.verification_flow_id(); Ok(Sas { inner: Arc::new(Mutex::new(inner)), @@ -346,8 +355,12 @@ enum InnerSas { } impl InnerSas { - fn start(account: Account, other_device: ReadOnlyDevice) -> (InnerSas, StartEventContent) { - let sas = SasState::::new(account, other_device); + fn start( + account: Account, + other_device: ReadOnlyDevice, + other_identity: Option, + ) -> (InnerSas, StartEventContent) { + let sas = SasState::::new(account, other_device, other_identity); let content = sas.as_content(); (InnerSas::Created(sas), content) } @@ -356,8 +369,9 @@ impl InnerSas { account: Account, other_device: ReadOnlyDevice, event: &ToDeviceEvent, + other_identity: Option, ) -> Result { - match SasState::::from_start_event(account, other_device, event) { + match SasState::::from_start_event(account, other_device, event, other_identity) { Ok(s) => Ok(InnerSas::Started(s)), Err(s) => Err(s.as_content()), } @@ -599,12 +613,13 @@ mod test { let bob = Account::new(&bob_id(), &bob_device_id()); let bob_device = ReadOnlyDevice::from_account(&bob).await; - let alice_sas = SasState::::new(alice.clone(), bob_device); + let alice_sas = SasState::::new(alice.clone(), bob_device, None); let start_content = alice_sas.as_content(); let event = wrap_to_device_event(alice_sas.user_id(), start_content); - let bob_sas = SasState::::from_start_event(bob.clone(), alice_device, &event); + let bob_sas = + SasState::::from_start_event(bob.clone(), alice_device, &event, None); (alice_sas, bob_sas.unwrap()) } diff --git a/matrix_sdk_crypto/src/verification/sas/sas_state.rs b/matrix_sdk_crypto/src/verification/sas/sas_state.rs index 0ff6d61d..5c072800 100644 --- a/matrix_sdk_crypto/src/verification/sas/sas_state.rs +++ b/matrix_sdk_crypto/src/verification/sas/sas_state.rs @@ -43,7 +43,7 @@ use matrix_sdk_common::{ use super::helpers::{get_decimal, get_emoji, get_mac_content, receive_mac_event, SasIds}; -use crate::{Account, ReadOnlyDevice}; +use crate::{user_identity::UserIdentities, Account, ReadOnlyDevice}; const KEY_AGREEMENT_PROTOCOLS: &[KeyAgreementProtocol] = &[KeyAgreementProtocol::Curve25519HkdfSha256]; @@ -286,7 +286,11 @@ impl SasState { /// * `account` - Our own account. /// /// * `other_device` - The other device which we are going to verify. - pub fn new(account: Account, other_device: ReadOnlyDevice) -> SasState { + pub fn new( + account: Account, + other_device: ReadOnlyDevice, + other_identity: Option, + ) -> SasState { let verification_flow_id = Uuid::new_v4().to_string(); SasState { @@ -294,6 +298,7 @@ impl SasState { ids: SasIds { account, other_device, + other_identity, }, verification_flow_id: Arc::new(verification_flow_id), @@ -382,6 +387,7 @@ impl SasState { account: Account, other_device: ReadOnlyDevice, event: &ToDeviceEvent, + other_identity: Option, ) -> Result, SasState> { if let StartMethod::MSasV1(content) = &event.content.method { let sas = OlmSas::new(); @@ -397,6 +403,7 @@ impl SasState { ids: SasIds { account, other_device, + other_identity, }, creation_time: Arc::new(Instant::now()), @@ -438,6 +445,7 @@ impl SasState { ids: SasIds { account, other_device, + other_identity, }, verification_flow_id: Arc::new(event.content.transaction_id.clone()), @@ -871,12 +879,13 @@ mod test { let bob = Account::new(&bob_id(), &bob_device_id()); let bob_device = ReadOnlyDevice::from_account(&bob).await; - let alice_sas = SasState::::new(alice.clone(), bob_device); + let alice_sas = SasState::::new(alice.clone(), bob_device, None); let start_content = alice_sas.as_content(); let event = wrap_to_device_event(alice_sas.user_id(), start_content); - let bob_sas = SasState::::from_start_event(bob.clone(), alice_device, &event); + let bob_sas = + SasState::::from_start_event(bob.clone(), alice_device, &event, None); (alice_sas, bob_sas.unwrap()) } @@ -1027,7 +1036,7 @@ mod test { let bob = Account::new(&bob_id(), &bob_device_id()); let bob_device = ReadOnlyDevice::from_account(&bob).await; - let alice_sas = SasState::::new(alice.clone(), bob_device); + let alice_sas = SasState::::new(alice.clone(), bob_device, None); let mut start_content = alice_sas.as_content(); @@ -1039,7 +1048,7 @@ mod test { } let event = wrap_to_device_event(alice_sas.user_id(), start_content); - SasState::::from_start_event(bob.clone(), alice_device.clone(), &event) + SasState::::from_start_event(bob.clone(), alice_device.clone(), &event, None) .expect_err("Didn't cancel on invalid MAC method"); let mut start_content = alice_sas.as_content(); @@ -1050,7 +1059,7 @@ mod test { }); let event = wrap_to_device_event(alice_sas.user_id(), start_content); - SasState::::from_start_event(bob.clone(), alice_device, &event) + SasState::::from_start_event(bob.clone(), alice_device, &event, None) .expect_err("Didn't cancel on unknown sas method"); } } From 37a7f69e03c9cc3f69dddfa491e487085cd53649 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 18 Aug 2020 15:13:56 +0200 Subject: [PATCH 04/49] crypto: Implement storage for the user identities in the memory store. --- matrix_sdk_crypto/src/store/memorystore.rs | 18 +++++++++++++++--- matrix_sdk_crypto/src/store/mod.rs | 7 +++++++ matrix_sdk_crypto/src/store/sqlite.rs | 4 ++++ matrix_sdk_crypto/src/user_identity.rs | 15 +++++++++++++++ 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/matrix_sdk_crypto/src/store/memorystore.rs b/matrix_sdk_crypto/src/store/memorystore.rs index 4cbe31db..2bccfb38 100644 --- a/matrix_sdk_crypto/src/store/memorystore.rs +++ b/matrix_sdk_crypto/src/store/memorystore.rs @@ -15,7 +15,7 @@ use std::{collections::HashSet, sync::Arc}; use async_trait::async_trait; -use dashmap::DashSet; +use dashmap::{DashMap, DashSet}; use matrix_sdk_common::{ identifiers::{DeviceId, RoomId, UserId}, locks::Mutex, @@ -34,6 +34,7 @@ pub struct MemoryStore { tracked_users: Arc>, users_for_key_query: Arc>, devices: DeviceStore, + identities: Arc>, } impl MemoryStore { @@ -44,6 +45,7 @@ impl MemoryStore { tracked_users: Arc::new(DashSet::new()), users_for_key_query: Arc::new(DashSet::new()), devices: DeviceStore::new(), + identities: Arc::new(DashMap::new()), } } } @@ -133,8 +135,18 @@ impl CryptoStore for MemoryStore { Ok(()) } - async fn get_user_identity(&self, _user_id: &UserId) -> Result> { - Ok(None) + 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(()) } } diff --git a/matrix_sdk_crypto/src/store/mod.rs b/matrix_sdk_crypto/src/store/mod.rs index 6273a9ba..0bea21fc 100644 --- a/matrix_sdk_crypto/src/store/mod.rs +++ b/matrix_sdk_crypto/src/store/mod.rs @@ -199,6 +199,13 @@ pub trait CryptoStore: Debug { /// * `user_id` - The user for which we should get all the devices. async fn get_user_devices(&self, 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 a44417aa..218c2ea2 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -888,6 +888,10 @@ impl CryptoStore for SqliteStore { async fn get_user_identity(&self, _user_id: &UserId) -> Result> { Ok(None) } + + async fn save_user_identities(&self, _users: &[UserIdentities]) -> Result<()> { + Ok(()) + } } #[cfg(not(tarpaulin_include))] diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index 6141ea67..4c0f8197 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -153,6 +153,13 @@ pub enum UserIdentities { } impl UserIdentities { + pub fn user_id(&self) -> &UserId { + match self { + UserIdentities::Own(i) => i.user_id(), + UserIdentities::Other(i) => i.user_id(), + } + } + pub fn master_key(&self) -> &BTreeMap { match self { UserIdentities::Own(i) => i.master_key(), @@ -182,6 +189,10 @@ impl UserIdentity { }) } + pub fn user_id(&self) -> &UserId { + &self.user_id + } + pub fn master_key(&self) -> &BTreeMap { &self.master_key.0.keys } @@ -231,6 +242,10 @@ impl OwnUserIdentity { }) } + pub fn user_id(&self) -> &UserId { + &self.user_id + } + pub fn update( &mut self, master_key: MasterPubkey, From f626f2b24e8b8a7877bc4587433ff8b1650caff1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 18 Aug 2020 15:22:30 +0200 Subject: [PATCH 05/49] crypto: Add some logging for the user identity update logic. --- matrix_sdk_crypto/src/machine.rs | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 3fb2e912..18ad8a5d 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -511,6 +511,10 @@ impl OlmMachine { let self_signing = if let Some(s) = response.self_signing_keys.get(user_id) { SelfSigningPubkey::from(s) } else { + warn!( + "User identity for user {} didn't contain a self signing pubkey", + user_id + ); continue; }; @@ -521,6 +525,7 @@ impl OlmMachine { { UserSigningPubkey::from(s) } else { + warn!("User identity for our own user {} didn't contain a user signing pubkey", user_id); continue; }; @@ -538,6 +543,10 @@ impl OlmMachine { OwnUserIdentity::new(master_key, self_signing, user_signing) .map(UserIdentities::Own) } else { + warn!( + "User identity for our own user {} didn't contain a user signing pubkey", + user_id + ); continue; } } else { @@ -545,9 +554,19 @@ impl OlmMachine { }; match identity { - Ok(i) => changed.push(i), - Err(_e) => { - // TODO log some error here + Ok(i) => { + trace!( + "Updated or created new user identity for {}: {:?}", + user_id, + i + ); + changed.push(i); + } + Err(e) => { + warn!( + "Coulnd't update or create new user identity for {}: {:?}", + user_id, e + ); continue; } } From c21517c61ebab489148fcac57a9a67737eff31c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 18 Aug 2020 15:23:37 +0200 Subject: [PATCH 06/49] crypto: Store the changed user identities. --- matrix_sdk_crypto/src/machine.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 18ad8a5d..b8dbf9e7 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -593,6 +593,7 @@ impl OlmMachine { .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?; Ok((changed_devices, changed_identities)) } From 27e1fb9a355c25d9d8127c9be3152347dba75d74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 18 Aug 2020 15:25:00 +0200 Subject: [PATCH 07/49] crypto: Pass the user identity to the SAS object when a start event is received. --- matrix_sdk_crypto/src/verification/machine.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index d496fcad..a2699127 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -144,7 +144,7 @@ impl VerificationMachine { d, self.store.clone(), e, - None, + self.store.get_user_identity(&e.sender).await?, ) { Ok(s) => { self.verifications From f63a01a85bc4711d8763018af3cacbc3843688f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 18 Aug 2020 15:36:04 +0200 Subject: [PATCH 08/49] crypto: Remove a stale TODO. --- matrix_sdk_crypto/src/verification/sas/helpers.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/matrix_sdk_crypto/src/verification/sas/helpers.rs b/matrix_sdk_crypto/src/verification/sas/helpers.rs index d644884f..893451f7 100644 --- a/matrix_sdk_crypto/src/verification/sas/helpers.rs +++ b/matrix_sdk_crypto/src/verification/sas/helpers.rs @@ -231,7 +231,6 @@ pub fn receive_mac_event( ids.other_device.device_id() ); } - // TODO add an else if branch for the master key here } Ok((verified_devices, vec![])) From a42af5da69da1da461040143042eb707c2ceddd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 19 Aug 2020 10:54:26 +0200 Subject: [PATCH 09/49] crypto: Let the device hold on to identities. This makes it possible to check the verification state of the device directly. --- matrix_sdk_crypto/src/device.rs | 17 +++++++++--- matrix_sdk_crypto/src/machine.rs | 38 ++++++++++++++++++++++++++ matrix_sdk_crypto/src/user_identity.rs | 14 ++++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/matrix_sdk_crypto/src/device.rs b/matrix_sdk_crypto/src/device.rs index b6fbcbd9..6e517279 100644 --- a/matrix_sdk_crypto/src/device.rs +++ b/matrix_sdk_crypto/src/device.rs @@ -36,7 +36,10 @@ use serde_json::{json, Value}; use super::{Account, OlmMachine}; use crate::{ - error::SignatureError, store::Result as StoreResult, verification::VerificationMachine, + error::SignatureError, + store::Result as StoreResult, + user_identity::{OwnUserIdentity, UserIdentity}, + verification::VerificationMachine, verify_json, ReadOnlyUserDevices, Sas, }; @@ -58,6 +61,8 @@ pub struct ReadOnlyDevice { pub struct Device { pub(crate) inner: ReadOnlyDevice, pub(crate) verification_machine: VerificationMachine, + pub(crate) own_identity: Option, + pub(crate) device_owner_identity: Option, } impl Deref for Device { @@ -97,6 +102,8 @@ impl Device { pub struct UserDevices { pub(crate) inner: ReadOnlyUserDevices, pub(crate) verification_machine: VerificationMachine, + pub(crate) own_identity: Option, + pub(crate) device_owner_identity: Option, } impl UserDevices { @@ -105,6 +112,8 @@ impl UserDevices { self.inner.get(device_id).map(|d| Device { inner: d, verification_machine: self.verification_machine.clone(), + own_identity: self.own_identity.clone(), + device_owner_identity: self.device_owner_identity.clone(), }) } @@ -115,11 +124,11 @@ impl UserDevices { /// Iterator over all the devices of the user devices. pub fn devices(&self) -> impl Iterator + '_ { - let machine = self.verification_machine.clone(); - self.inner.devices().map(move |d| Device { inner: d.clone(), - verification_machine: machine.clone(), + verification_machine: self.verification_machine.clone(), + own_identity: self.own_identity.clone(), + device_owner_identity: self.device_owner_identity.clone(), }) } } diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index b8dbf9e7..65a53c8d 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -1423,9 +1423,28 @@ impl OlmMachine { .ok() .flatten()?; + let own_identity = self + .store + .get_user_identity(self.user_id()) + .await + .ok() + .flatten() + .map(|i| i.own().cloned()) + .flatten(); + let device_owner_identity = self + .store + .get_user_identity(user_id) + .await + .ok() + .flatten() + .map(|i| i.other().cloned()) + .flatten(); + Some(Device { inner: device, verification_machine: self.verification_machine.clone(), + own_identity, + device_owner_identity, }) } @@ -1455,9 +1474,28 @@ impl OlmMachine { pub async fn get_user_devices(&self, user_id: &UserId) -> StoreResult { let devices = self.store.get_user_devices(user_id).await?; + let own_identity = self + .store + .get_user_identity(self.user_id()) + .await + .ok() + .flatten() + .map(|i| i.own().cloned()) + .flatten(); + let device_owner_identity = self + .store + .get_user_identity(user_id) + .await + .ok() + .flatten() + .map(|i| i.other().cloned()) + .flatten(); + Ok(UserDevices { inner: devices, verification_machine: self.verification_machine.clone(), + own_identity, + device_owner_identity, }) } } diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index 4c0f8197..21bf3ee5 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -166,6 +166,20 @@ impl UserIdentities { UserIdentities::Other(i) => i.master_key(), } } + + pub fn own(&self) -> Option<&OwnUserIdentity> { + match self { + UserIdentities::Own(i) => Some(i), + _ => None, + } + } + + pub fn other(&self) -> Option<&UserIdentity> { + match self { + UserIdentities::Other(i) => Some(i), + _ => None, + } + } } #[derive(Debug, Clone)] From 90ea0229f225e68c6e4eb8d9ffa60f1c3a94ce3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 19 Aug 2020 11:20:08 +0200 Subject: [PATCH 10/49] crypto: Rename TrustState to LocalTrust since. We might still trust the device event if our local trust isn't set, so rename the enum to better reflect that meaning. --- matrix_sdk/src/device.rs | 4 +-- matrix_sdk/src/lib.rs | 2 +- matrix_sdk_base/src/lib.rs | 2 +- matrix_sdk_crypto/src/device.rs | 36 +++++++++---------- matrix_sdk_crypto/src/lib.rs | 2 +- matrix_sdk_crypto/src/store/sqlite.rs | 4 +-- matrix_sdk_crypto/src/verification/sas/mod.rs | 6 ++-- 7 files changed, 28 insertions(+), 28 deletions(-) diff --git a/matrix_sdk/src/device.rs b/matrix_sdk/src/device.rs index 1ffb51c5..dd4449da 100644 --- a/matrix_sdk/src/device.rs +++ b/matrix_sdk/src/device.rs @@ -15,7 +15,7 @@ use std::{ops::Deref, result::Result as StdResult}; use matrix_sdk_base::{ - CryptoStoreError, Device as BaseDevice, ReadOnlyDevice, TrustState, + CryptoStoreError, Device as BaseDevice, LocalTrust, ReadOnlyDevice, UserDevices as BaseUserDevices, }; use matrix_sdk_common::{ @@ -83,7 +83,7 @@ impl Device { /// * `trust_state` - The new trust state that should be set for the device. pub async fn set_trust_state( &self, - trust_state: TrustState, + trust_state: LocalTrust, ) -> StdResult<(), CryptoStoreError> { self.inner.set_trust_state(trust_state).await } diff --git a/matrix_sdk/src/lib.rs b/matrix_sdk/src/lib.rs index f6e81259..6784df65 100644 --- a/matrix_sdk/src/lib.rs +++ b/matrix_sdk/src/lib.rs @@ -41,7 +41,7 @@ pub use matrix_sdk_base::JsonStore; #[cfg(feature = "encryption")] #[cfg_attr(feature = "docs", doc(cfg(encryption)))] -pub use matrix_sdk_base::TrustState; +pub use matrix_sdk_base::LocalTrust; pub use matrix_sdk_base::{ CustomEvent, Error as BaseError, EventEmitter, Room, RoomState, Session, StateStore, SyncRoom, }; diff --git a/matrix_sdk_base/src/lib.rs b/matrix_sdk_base/src/lib.rs index a49b6906..4bf782ce 100644 --- a/matrix_sdk_base/src/lib.rs +++ b/matrix_sdk_base/src/lib.rs @@ -57,7 +57,7 @@ pub use state::{AllRooms, ClientState}; #[cfg(feature = "encryption")] #[cfg_attr(feature = "docs", doc(cfg(encryption)))] pub use matrix_sdk_crypto::{ - CryptoStoreError, Device, ReadOnlyDevice, Sas, TrustState, UserDevices, + CryptoStoreError, Device, LocalTrust, ReadOnlyDevice, Sas, UserDevices, }; #[cfg(feature = "messages")] diff --git a/matrix_sdk_crypto/src/device.rs b/matrix_sdk_crypto/src/device.rs index 6e517279..958510a4 100644 --- a/matrix_sdk_crypto/src/device.rs +++ b/matrix_sdk_crypto/src/device.rs @@ -53,7 +53,7 @@ pub struct ReadOnlyDevice { signatures: Arc>>, display_name: Arc>, deleted: Arc, - trust_state: Arc>, + trust_state: Arc>, } #[derive(Debug, Clone)] @@ -88,7 +88,7 @@ impl Device { /// # Arguments /// /// * `trust_state` - The new trust state that should be set for the device. - pub async fn set_trust_state(&self, trust_state: TrustState) -> StoreResult<()> { + pub async fn set_trust_state(&self, trust_state: LocalTrust) -> StoreResult<()> { self.inner.set_trust_state(trust_state); self.verification_machine .store @@ -134,8 +134,8 @@ impl UserDevices { } #[derive(Debug, Clone, Copy, PartialEq)] -/// The trust state of a device. -pub enum TrustState { +/// The local trust state of a device. +pub enum LocalTrust { /// The device has been verified and is trusted. Verified = 0, /// The device been blacklisted from communicating. @@ -146,14 +146,14 @@ pub enum TrustState { Unset = 3, } -impl From for TrustState { +impl From for LocalTrust { fn from(state: i64) -> Self { match state { - 0 => TrustState::Verified, - 1 => TrustState::BlackListed, - 2 => TrustState::Ignored, - 3 => TrustState::Unset, - _ => TrustState::Unset, + 0 => LocalTrust::Verified, + 1 => LocalTrust::BlackListed, + 2 => LocalTrust::Ignored, + 3 => LocalTrust::Unset, + _ => LocalTrust::Unset, } } } @@ -164,7 +164,7 @@ impl ReadOnlyDevice { user_id: UserId, device_id: Box, display_name: Option, - trust_state: TrustState, + trust_state: LocalTrust, algorithms: Vec, keys: BTreeMap, signatures: BTreeMap>, @@ -213,27 +213,27 @@ impl ReadOnlyDevice { } /// Get the trust state of the device. - pub fn trust_state(&self) -> TrustState { + pub fn trust_state(&self) -> LocalTrust { self.trust_state.load(Ordering::Relaxed) } /// Is the device locally marked as trusted. pub fn is_trusted(&self) -> bool { - self.trust_state() == TrustState::Verified + self.trust_state() == LocalTrust::Verified } /// Is the device locally marked as blacklisted. /// /// Blacklisted devices won't receive any group sessions. pub fn is_blacklisted(&self) -> bool { - self.trust_state() == TrustState::BlackListed + self.trust_state() == LocalTrust::BlackListed } /// Set the trust state of the device to the given state. /// /// Note: This should only done in the cryptostore where the trust state can /// be stored. - pub(crate) fn set_trust_state(&self, state: TrustState) { + pub(crate) fn set_trust_state(&self, state: LocalTrust) { self.trust_state.store(state, Ordering::Relaxed) } @@ -339,7 +339,7 @@ impl TryFrom<&DeviceKeys> for ReadOnlyDevice { .flatten(), ), deleted: Arc::new(AtomicBool::new(false)), - trust_state: Arc::new(Atomic::new(TrustState::Unset)), + trust_state: Arc::new(Atomic::new(LocalTrust::Unset)), }; device.verify_device_keys(device_keys)?; @@ -358,7 +358,7 @@ pub(crate) mod test { use serde_json::json; use std::convert::TryFrom; - use crate::device::{ReadOnlyDevice, TrustState}; + use crate::device::{LocalTrust, ReadOnlyDevice}; use matrix_sdk_common::{ encryption::DeviceKeys, identifiers::{user_id, DeviceKeyAlgorithm}, @@ -404,7 +404,7 @@ pub(crate) mod test { assert_eq!(&user_id, device.user_id()); assert_eq!(device_id, device.device_id()); assert_eq!(device.algorithms.len(), 2); - assert_eq!(TrustState::Unset, device.trust_state()); + assert_eq!(LocalTrust::Unset, device.trust_state()); assert_eq!( "Alice's mobile phone", device.display_name().as_ref().unwrap() diff --git a/matrix_sdk_crypto/src/lib.rs b/matrix_sdk_crypto/src/lib.rs index 8c41e88e..21a9b487 100644 --- a/matrix_sdk_crypto/src/lib.rs +++ b/matrix_sdk_crypto/src/lib.rs @@ -37,7 +37,7 @@ mod store; mod user_identity; mod verification; -pub use device::{Device, ReadOnlyDevice, TrustState, UserDevices}; +pub use device::{Device, LocalTrust, ReadOnlyDevice, UserDevices}; pub use error::{MegolmError, OlmError}; pub use machine::{OlmMachine, OneTimeKeys}; pub use memory_stores::{DeviceStore, GroupSessionStore, ReadOnlyUserDevices, SessionStore}; diff --git a/matrix_sdk_crypto/src/store/sqlite.rs b/matrix_sdk_crypto/src/store/sqlite.rs index 218c2ea2..7ea291d0 100644 --- a/matrix_sdk_crypto/src/store/sqlite.rs +++ b/matrix_sdk_crypto/src/store/sqlite.rs @@ -36,7 +36,7 @@ use zeroize::Zeroizing; use super::{CryptoStore, CryptoStoreError, Result}; use crate::{ - device::{ReadOnlyDevice, TrustState}, + device::{LocalTrust, ReadOnlyDevice}, memory_stores::{DeviceStore, GroupSessionStore, ReadOnlyUserDevices, SessionStore}, user_identity::UserIdentities, Account, IdentityKeys, InboundGroupSession, Session, @@ -486,7 +486,7 @@ impl SqliteStore { let device_id = &row.2.to_string(); let display_name = &row.3; - let trust_state = TrustState::from(row.4); + let trust_state = LocalTrust::from(row.4); let algorithm_rows: Vec<(String,)> = query_as("SELECT algorithm FROM algorithms WHERE device_id = ?") diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index c3837203..0c92d44e 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -34,8 +34,8 @@ use matrix_sdk_common::{ }; use crate::{ - user_identity::UserIdentities, Account, CryptoStore, CryptoStoreError, ReadOnlyDevice, - TrustState, + user_identity::UserIdentities, Account, CryptoStore, CryptoStoreError, LocalTrust, + ReadOnlyDevice, }; pub use helpers::content_to_request; @@ -216,7 +216,7 @@ impl Sas { device.device_id() ); - device.set_trust_state(TrustState::Verified); + device.set_trust_state(LocalTrust::Verified); self.store.save_devices(&[device]).await?; Ok(true) From 3990e50ca6f0209b83ec0189204ee60143622cfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 19 Aug 2020 14:28:16 +0200 Subject: [PATCH 11/49] crypto: Store the verified identities in the SAS states. --- matrix_sdk_crypto/src/verification/sas/helpers.rs | 6 +++--- matrix_sdk_crypto/src/verification/sas/mod.rs | 14 ++++++++++++++ .../src/verification/sas/sas_state.rs | 9 +++++++-- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/sas/helpers.rs b/matrix_sdk_crypto/src/verification/sas/helpers.rs index 893451f7..c3afcf86 100644 --- a/matrix_sdk_crypto/src/verification/sas/helpers.rs +++ b/matrix_sdk_crypto/src/verification/sas/helpers.rs @@ -159,7 +159,7 @@ pub fn receive_mac_event( ids: &SasIds, flow_id: &str, event: &ToDeviceEvent, -) -> Result<(Vec, Vec), CancelCode> { +) -> Result<(Vec, Vec), CancelCode> { let mut verified_devices = Vec::new(); let mut verified_identities = Vec::new(); @@ -217,7 +217,7 @@ pub fn receive_mac_event( key_id, event.sender ); - verified_identities.push(identity) + verified_identities.push(identity.clone()) } else { return Err(CancelCode::KeyMismatch); } @@ -233,7 +233,7 @@ pub fn receive_mac_event( } } - Ok((verified_devices, vec![])) + Ok((verified_devices, verified_identities)) } /// Get the extra info that will be used when we generate a MAC and need to send diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index 0c92d44e..74a33e78 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -150,6 +150,7 @@ impl Sas { other_identity.clone(), )?; let flow_id = inner.verification_flow_id(); + Ok(Sas { inner: Arc::new(Mutex::new(inner)), account, @@ -334,6 +335,11 @@ impl Sas { self.inner.lock().unwrap().verified_devices() } + #[allow(dead_code)] + pub(crate) fn verified_identities(&self) -> Option>> { + self.inner.lock().unwrap().verified_identities() + } + pub(crate) fn content_to_request( &self, content: AnyToDeviceEventContent, @@ -564,6 +570,14 @@ impl InnerSas { None } } + + fn verified_identities(&self) -> Option>> { + if let InnerSas::Done(s) = self { + Some(s.verified_identities()) + } else { + None + } + } } #[cfg(test)] diff --git a/matrix_sdk_crypto/src/verification/sas/sas_state.rs b/matrix_sdk_crypto/src/verification/sas/sas_state.rs index 5c072800..85201fa9 100644 --- a/matrix_sdk_crypto/src/verification/sas/sas_state.rs +++ b/matrix_sdk_crypto/src/verification/sas/sas_state.rs @@ -207,7 +207,7 @@ pub struct MacReceived { we_started: bool, their_pubkey: String, verified_devices: Arc>, - verified_master_keys: Arc>, + verified_master_keys: Arc>, } /// The SAS state indicating that the verification finished successfully. @@ -217,7 +217,7 @@ pub struct MacReceived { #[derive(Clone, Debug)] pub struct Done { verified_devices: Arc>, - verified_master_keys: Arc>, + verified_master_keys: Arc>, } #[derive(Clone, Debug)] @@ -791,6 +791,11 @@ impl SasState { pub fn verified_devices(&self) -> Arc> { self.state.verified_devices.clone() } + + /// Get the list of verified identities. + pub fn verified_identities(&self) -> Arc> { + self.state.verified_master_keys.clone() + } } impl Canceled { From 317a141e077b5fcff63e08f2819e57f10393cecb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 19 Aug 2020 14:34:18 +0200 Subject: [PATCH 12/49] crypto: If our own identity passed a SAS flow, mark it as verified. --- matrix_sdk_crypto/src/user_identity.rs | 21 +++++- matrix_sdk_crypto/src/verification/sas/mod.rs | 67 ++++++++++++++++++- 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index 21bf3ee5..0ba74205 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -15,7 +15,10 @@ use std::{ collections::BTreeMap, convert::TryFrom, - sync::{atomic::AtomicBool, Arc}, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }, }; use serde_json::to_value; @@ -29,8 +32,10 @@ use crate::{error::SignatureError, verify_json, ReadOnlyDevice}; #[derive(Debug, Clone)] pub struct MasterPubkey(Arc); + #[derive(Debug, Clone)] pub struct SelfSigningPubkey(Arc); + #[derive(Debug, Clone)] pub struct UserSigningPubkey(Arc); @@ -182,6 +187,12 @@ impl UserIdentities { } } +impl PartialEq for UserIdentities { + fn eq(&self, other: &UserIdentities) -> bool { + self.user_id() == other.user_id() + } +} + #[derive(Debug, Clone)] pub struct UserIdentity { user_id: Arc, @@ -287,6 +298,14 @@ impl OwnUserIdentity { self.user_signing_key .verify_master_key(&identity.master_key) } + + pub fn mark_as_verified(&self) { + self.verified.store(true, Ordering::SeqCst) + } + + pub fn is_verified(&self) -> bool { + self.verified.load(Ordering::SeqCst) + } } #[cfg(test)] diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index 74a33e78..c3e49687 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -189,8 +189,15 @@ impl Sas { (content, guard.is_done()) }; - if done && !self.mark_device_as_verified().await? { - return Ok(self.cancel()); + 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?; + } } Ok(content.map(|c| { @@ -199,6 +206,61 @@ impl Sas { })) } + pub(crate) async fn mark_identity_as_verified(&self) -> Result { + // 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); + } + + 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 + .verified_identities() + .map_or(false, |i| i.contains(&identity)) + { + trace!( + "Marking user identity of {} as verified.", + identity.user_id(), + ); + + match &identity { + UserIdentities::Own(i) => { + 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) + } else { + info!( + "The interactive verification process didn't contain a \ + MAC for the user identity of {} {:?}", + identity.user_id(), + self.verified_identities(), + ); + + Ok(false) + } + } else { + Ok(false) + } + } else { + info!( + "The identity for {} was deleted while an interactive \ + verification was going on.", + self.other_user_id(), + ); + Ok(false) + } + } + pub(crate) async fn mark_device_as_verified(&self) -> Result { let device = self .store @@ -335,7 +397,6 @@ impl Sas { self.inner.lock().unwrap().verified_devices() } - #[allow(dead_code)] pub(crate) fn verified_identities(&self) -> Option>> { self.inner.lock().unwrap().verified_identities() } From c2a386b889728f8aa91ae067b0112af43ebf2252 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 19 Aug 2020 14:40:04 +0200 Subject: [PATCH 13/49] crypto: Fix a clippy warning. --- matrix_sdk_crypto/src/verification/sas/mod.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index c3e49687..c876ea8f 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -226,16 +226,13 @@ impl Sas { identity.user_id(), ); - match &identity { - UserIdentities::Own(i) => { - 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. - _ => {} + 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) } else { From c3e593d99844b23de87c40991f97fb194e92509e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 19 Aug 2020 14:43:49 +0200 Subject: [PATCH 14/49] crypto: The device identity can be our own, so store the identity enum instead. --- matrix_sdk_crypto/src/device.rs | 6 +++--- matrix_sdk_crypto/src/machine.rs | 18 ++---------------- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/matrix_sdk_crypto/src/device.rs b/matrix_sdk_crypto/src/device.rs index 958510a4..587f0ecc 100644 --- a/matrix_sdk_crypto/src/device.rs +++ b/matrix_sdk_crypto/src/device.rs @@ -38,7 +38,7 @@ use super::{Account, OlmMachine}; use crate::{ error::SignatureError, store::Result as StoreResult, - user_identity::{OwnUserIdentity, UserIdentity}, + user_identity::{OwnUserIdentity, UserIdentities}, verification::VerificationMachine, verify_json, ReadOnlyUserDevices, Sas, }; @@ -62,7 +62,7 @@ pub struct Device { pub(crate) inner: ReadOnlyDevice, pub(crate) verification_machine: VerificationMachine, pub(crate) own_identity: Option, - pub(crate) device_owner_identity: Option, + pub(crate) device_owner_identity: Option, } impl Deref for Device { @@ -103,7 +103,7 @@ pub struct UserDevices { pub(crate) inner: ReadOnlyUserDevices, pub(crate) verification_machine: VerificationMachine, pub(crate) own_identity: Option, - pub(crate) device_owner_identity: Option, + pub(crate) device_owner_identity: Option, } impl UserDevices { diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 65a53c8d..a65373b9 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -1431,14 +1431,7 @@ impl OlmMachine { .flatten() .map(|i| i.own().cloned()) .flatten(); - let device_owner_identity = self - .store - .get_user_identity(user_id) - .await - .ok() - .flatten() - .map(|i| i.other().cloned()) - .flatten(); + let device_owner_identity = self.store.get_user_identity(user_id).await.ok().flatten(); Some(Device { inner: device, @@ -1482,14 +1475,7 @@ impl OlmMachine { .flatten() .map(|i| i.own().cloned()) .flatten(); - let device_owner_identity = self - .store - .get_user_identity(user_id) - .await - .ok() - .flatten() - .map(|i| i.other().cloned()) - .flatten(); + let device_owner_identity = self.store.get_user_identity(user_id).await.ok().flatten(); Ok(UserDevices { inner: devices, From 3153a81cd274a0dbbac7aa8504c0629caa21eeea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 19 Aug 2020 14:47:22 +0200 Subject: [PATCH 15/49] crypto: Add support to check the cross signing verification state of a device. --- matrix_sdk_crypto/src/device.rs | 32 ++++++++++++++++++++++++++ matrix_sdk_crypto/src/user_identity.rs | 4 ++++ 2 files changed, 36 insertions(+) diff --git a/matrix_sdk_crypto/src/device.rs b/matrix_sdk_crypto/src/device.rs index 587f0ecc..b760f085 100644 --- a/matrix_sdk_crypto/src/device.rs +++ b/matrix_sdk_crypto/src/device.rs @@ -83,6 +83,37 @@ impl Device { .await } + /// Get the trust state of the device. + pub fn trust_state(&self) -> bool { + // TODO we want to return an enum mentioning if the trust is local, if + // only the identity is trusted, if the identity and the device are + // trusted. + if self.inner.trust_state() == LocalTrust::Verified { + true + } else { + self.own_identity.as_ref().map_or(false, |own_identity| { + own_identity.is_verified() + && self + .device_owner_identity + .as_ref() + .map(|device_identity| match device_identity { + UserIdentities::Own(_) => own_identity + .is_device_signed(&self.inner) + .map_or(false, |_| true), + UserIdentities::Other(device_identity) => { + own_identity + .is_identity_signed(&device_identity) + .map_or(false, |_| true) + && device_identity + .is_device_signed(&self.inner) + .map_or(false, |_| true) + } + }) + .unwrap_or(false) + }) + } + } + /// Set the trust state of the device to the given state. /// /// # Arguments @@ -90,6 +121,7 @@ impl Device { /// * `trust_state` - The new trust state that should be set for the device. pub async fn set_trust_state(&self, trust_state: LocalTrust) -> StoreResult<()> { self.inner.set_trust_state(trust_state); + self.verification_machine .store .save_devices(&[self.inner.clone()]) diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index 0ba74205..4ed1d4cb 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -299,6 +299,10 @@ impl OwnUserIdentity { .verify_master_key(&identity.master_key) } + pub fn is_device_signed(&self, device: &ReadOnlyDevice) -> Result<(), SignatureError> { + self.self_signing_key.verify_device(device) + } + pub fn mark_as_verified(&self) { self.verified.store(true, Ordering::SeqCst) } From 7f23cbbeb5c741b4d5be5a62319dd192779a004f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 19 Aug 2020 14:49:40 +0200 Subject: [PATCH 16/49] crypto: Add a TODO about cross signing signatures. --- matrix_sdk_crypto/src/verification/sas/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index c876ea8f..c116d5b4 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -278,6 +278,9 @@ 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) } else { @@ -303,7 +306,7 @@ impl Sas { let device = self.other_device(); info!( - "The device {} {} was deleted while a interactive \ + "The device {} {} was deleted while an interactive \ verification was going on.", device.user_id(), device.device_id() From 9fe0717cee8b7bcd6401c973ca168ed84e243bb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 19 Aug 2020 14:50:35 +0200 Subject: [PATCH 17/49] examples: Update the emoji verification example tho show a list of devices. This may showcase that cross signing verification works if the other device uploads valid signatures. --- matrix_sdk/examples/emoji_verification.rs | 42 ++++++++++++++++++----- matrix_sdk/src/device.rs | 5 +++ 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/matrix_sdk/examples/emoji_verification.rs b/matrix_sdk/examples/emoji_verification.rs index 992e29ed..4b05d9c3 100644 --- a/matrix_sdk/examples/emoji_verification.rs +++ b/matrix_sdk/examples/emoji_verification.rs @@ -1,10 +1,12 @@ use std::{env, io, process::exit}; use url::Url; -use matrix_sdk::{self, events::AnyToDeviceEvent, Client, ClientConfig, Sas, SyncSettings}; +use matrix_sdk::{ + self, events::AnyToDeviceEvent, identifiers::UserId, Client, ClientConfig, Sas, SyncSettings, +}; -async fn wait_for_confirmation(sas: Sas) { - println!("Emoji: {:?}", sas.emoji()); +async fn wait_for_confirmation(client: Client, sas: Sas) { + println!("Does the emoji match: {:?}", sas.emoji()); let mut input = String::new(); io::stdin() @@ -16,14 +18,15 @@ async fn wait_for_confirmation(sas: Sas) { sas.confirm().await.unwrap(); if sas.is_done() { - print_result(sas); + print_result(&sas); + print_devices(sas.other_device().user_id(), &client).await; } } _ => sas.cancel().await.unwrap(), } } -fn print_result(sas: Sas) { +fn print_result(sas: &Sas) { let device = sas.other_device(); println!( @@ -34,12 +37,28 @@ fn print_result(sas: Sas) { ); } +async fn print_devices(user_id: &UserId, client: &Client) { + println!("Devices of user {}", user_id); + + for device in client.get_user_devices(user_id).await.unwrap().devices() { + println!( + " {:<10} {:<30} {:<}", + device.device_id(), + device.display_name().as_deref().unwrap_or_default(), + device.is_trusted() + ); + } +} + async fn login( homeserver_url: String, username: &str, password: &str, ) -> Result<(), matrix_sdk::Error> { - let client_config = ClientConfig::new(); + 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(); @@ -64,6 +83,12 @@ async fn login( .get_verification(&e.content.transaction_id) .await .expect("Sas object wasn't created"); + println!( + "Starting verification with {} {}", + &sas.other_device().user_id(), + &sas.other_device().device_id() + ); + print_devices(&e.sender, &client).await; sas.accept().await.unwrap(); } @@ -73,7 +98,7 @@ async fn login( .await .expect("Sas object wasn't created"); - tokio::spawn(wait_for_confirmation(sas)); + tokio::spawn(wait_for_confirmation((*client).clone(), sas)); } AnyToDeviceEvent::KeyVerificationMac(e) => { @@ -83,7 +108,8 @@ async fn login( .expect("Sas object wasn't created"); if sas.is_done() { - print_result(sas); + print_result(&sas); + print_devices(&e.sender, &client).await; } } diff --git a/matrix_sdk/src/device.rs b/matrix_sdk/src/device.rs index dd4449da..241c883f 100644 --- a/matrix_sdk/src/device.rs +++ b/matrix_sdk/src/device.rs @@ -76,6 +76,11 @@ impl Device { }) } + /// Is the device trusted. + pub fn is_trusted(&self) -> bool { + self.inner.trust_state() + } + /// Set the trust state of the device to the given state. /// /// # Arguments From 56309ae12cb35eb5c804f933dc12d8142b459a15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 19 Aug 2020 14:52:11 +0200 Subject: [PATCH 18/49] matrix-sdk: Bump the versions of our deps. --- matrix_sdk/Cargo.toml | 4 ++-- matrix_sdk_base/Cargo.toml | 2 +- matrix_sdk_crypto/Cargo.toml | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/matrix_sdk/Cargo.toml b/matrix_sdk/Cargo.toml index 7f50d180..a353b06e 100644 --- a/matrix_sdk/Cargo.toml +++ b/matrix_sdk/Cargo.toml @@ -24,7 +24,7 @@ sqlite_cryptostore = ["matrix-sdk-base/sqlite_cryptostore"] docs = ["encryption", "sqlite_cryptostore", "messages"] [dependencies] -async-trait = "0.1.37" +async-trait = "0.1.38" dashmap = { version = "3.11.10", optional = true } http = "0.2.1" # FIXME: Revert to regular dependency once 0.10.8 or 0.11.0 is released @@ -55,7 +55,7 @@ version = "3.0.2" features = ["wasm-bindgen"] [dev-dependencies] -async-trait = "0.1.37" +async-trait = "0.1.38" dirs = "3.0.1" matrix-sdk-test = { version = "0.1.0", path = "../matrix_sdk_test" } tokio = { version = "0.2.22", features = ["rt-threaded", "macros"] } diff --git a/matrix_sdk_base/Cargo.toml b/matrix_sdk_base/Cargo.toml index efeedc4c..40855e62 100644 --- a/matrix_sdk_base/Cargo.toml +++ b/matrix_sdk_base/Cargo.toml @@ -23,7 +23,7 @@ sqlite_cryptostore = ["matrix-sdk-crypto/sqlite_cryptostore"] docs = ["encryption", "sqlite_cryptostore", "messages"] [dependencies] -async-trait = "0.1.37" +async-trait = "0.1.38" serde = "1.0.115" serde_json = "1.0.57" zeroize = "1.1.0" diff --git a/matrix_sdk_crypto/Cargo.toml b/matrix_sdk_crypto/Cargo.toml index 766d3e17..e1ec78b2 100644 --- a/matrix_sdk_crypto/Cargo.toml +++ b/matrix_sdk_crypto/Cargo.toml @@ -20,7 +20,7 @@ sqlite_cryptostore = ["sqlx"] docs = ["sqlite_cryptostore"] [dependencies] -async-trait = "0.1.37" +async-trait = "0.1.38" matrix-sdk-common-macros = { version = "0.1.0", path = "../matrix_sdk_common_macros" } matrix-sdk-common = { version = "0.1.0", path = "../matrix_sdk_common" } @@ -52,7 +52,7 @@ features = ["runtime-tokio", "sqlite"] [dev-dependencies] tokio = { version = "0.2.22", features = ["rt-threaded", "macros"] } futures = "0.3.5" -proptest = "0.10.0" +proptest = "0.10.1" serde_json = "1.0.57" tempfile = "3.1.0" http = "0.2.1" From eb16737d3baf0a5b9113e0e4800f583c96c50658 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 19 Aug 2020 15:35:34 +0200 Subject: [PATCH 19/49] crypto: Add some comments about the order of signature checks. --- matrix_sdk_crypto/src/device.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/matrix_sdk_crypto/src/device.rs b/matrix_sdk_crypto/src/device.rs index b760f085..3f96736d 100644 --- a/matrix_sdk_crypto/src/device.rs +++ b/matrix_sdk_crypto/src/device.rs @@ -89,17 +89,26 @@ impl Device { // only the identity is trusted, if the identity and the device are // trusted. if self.inner.trust_state() == LocalTrust::Verified { + // If the device is localy marked as verified just return so, no + // need to check signatures. true } else { self.own_identity.as_ref().map_or(false, |own_identity| { + // Our own identity needs to be marked as verified. own_identity.is_verified() && self .device_owner_identity .as_ref() .map(|device_identity| match device_identity { + // If it's one of our own devices, just check that + // we signed the device. UserIdentities::Own(_) => own_identity .is_device_signed(&self.inner) .map_or(false, |_| true), + + // If it's a device from someone else, first check + // that our user has signed the other user and then + // checkif the other user has signed this device. UserIdentities::Other(device_identity) => { own_identity .is_identity_signed(&device_identity) From 6f5352b9a90a824e14d0a1400b5fbcb2f34abd8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 19 Aug 2020 17:52:38 +0200 Subject: [PATCH 20/49] crypto: Test the signature checking of user identities. --- matrix_sdk_crypto/src/user_identity.rs | 194 +++++++++++++++++++------ 1 file changed, 149 insertions(+), 45 deletions(-) diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index 4ed1d4cb..2ecd549d 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -315,15 +315,21 @@ impl OwnUserIdentity { #[cfg(test)] mod test { use serde_json::json; - use std::convert::TryFrom; + use std::{convert::TryFrom, sync::Arc}; use matrix_sdk_common::{ api::r0::keys::get_keys::Response as KeyQueryResponse, identifiers::user_id, }; - use crate::machine::test::response_from_file; + use crate::{ + device::{Device, ReadOnlyDevice}, + machine::test::response_from_file, + olm::Account, + store::memorystore::MemoryStore, + verification::VerificationMachine, + }; - use super::{OwnUserIdentity, UserIdentity}; + use super::{OwnUserIdentity, UserIdentities, UserIdentity}; fn other_key_query() -> KeyQueryResponse { let data = response_from_file(&json!({ @@ -386,61 +392,120 @@ mod test { fn own_key_query() -> KeyQueryResponse { let data = response_from_file(&json!({ "device_keys": { - }, - "master_keys": { - "@example:localhost": { - "keys": { - "ed25519:rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0": "rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0" - }, - "signatures": { - "@example:localhost": { - "ed25519:WSKKLTJZCL": "ZzJp1wtmRdykXAUEItEjNiFlBrxx8L6/Vaen9am8AuGwlxxJtOkuY4m+4MPLvDPOgavKHLsrRuNLAfCeakMlCQ" - } - }, - "usage": [ - "master" - ], - "user_id": "@example:localhost" + "@example:localhost": { + "WSKKLTJZCL": { + "algorithms": [ + "m.olm.v1.curve25519-aes-sha2", + "m.megolm.v1.aes-sha2" + ], + "device_id": "WSKKLTJZCL", + "keys": { + "curve25519:WSKKLTJZCL": "wnip2tbJBJxrFayC88NNJpm61TeSNgYcqBH4T9yEDhU", + "ed25519:WSKKLTJZCL": "lQ+eshkhgKoo+qp9Qgnj3OX5PBoWMU5M9zbuEevwYqE" + }, + "signatures": { + "@example:localhost": { + "ed25519:WSKKLTJZCL": "SKpIUnq7QK0xleav0PrIQyKjVm+TgZr7Yi8cKjLeZDtkgyToE2d4/e3Aj79dqOlLB92jFVE4d1cM/Ry04wFwCA", + "ed25519:0C8lCBxrvrv/O7BQfsKnkYogHZX3zAgw3RfJuyiq210": "9UGu1iC5YhFCdELGfB29YaV+QE0t/X5UDSsPf4QcdZyXIwyp9zBbHX2lh9vWudNQ+akZpaq7ZRaaM+4TCnw/Ag" + } + }, + "user_id": "@example:localhost", + "unsigned": { + "device_display_name": "Cross signing capable" } + }, + "LVWOVGOXME": { + "algorithms": [ + "m.olm.v1.curve25519-aes-sha2", + "m.megolm.v1.aes-sha2" + ], + "device_id": "LVWOVGOXME", + "keys": { + "curve25519:LVWOVGOXME": "KMfWKUhnDW1D11hNzATs/Ax1FQRsJxKCWzq0NyGtIiI", + "ed25519:LVWOVGOXME": "k+NC3L7CBD6fBClcHBrKLOkqCyGNSKhWXiH5Q2STRnA" + }, + "signatures": { + "@example:localhost": { + "ed25519:LVWOVGOXME": "39Ir5Bttpc5+bQwzLj7rkjm5E5/cp/JTbMJ/t0enj6J5w9MXVBFOUqqM2hpaRaRwILMMpwYbJ8IOGjl0Y/MGAw" + } + }, + "user_id": "@example:localhost", + "unsigned": { + "device_display_name": "Non-cross signing" + } + } + } + }, + "failures": {}, + "master_keys": { + "@example:localhost": { + "user_id": "@example:localhost", + "usage": [ + "master" + ], + "keys": { + "ed25519:rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0": "rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0" + }, + "signatures": { + "@example:localhost": { + "ed25519:WSKKLTJZCL": "ZzJp1wtmRdykXAUEItEjNiFlBrxx8L6/Vaen9am8AuGwlxxJtOkuY4m+4MPLvDPOgavKHLsrRuNLAfCeakMlCQ" + } + } + } }, "self_signing_keys": { + "@example:localhost": { + "user_id": "@example:localhost", + "usage": [ + "self_signing" + ], + "keys": { + "ed25519:0C8lCBxrvrv/O7BQfsKnkYogHZX3zAgw3RfJuyiq210": "0C8lCBxrvrv/O7BQfsKnkYogHZX3zAgw3RfJuyiq210" + }, + "signatures": { "@example:localhost": { - "keys": { - "ed25519:0C8lCBxrvrv/O7BQfsKnkYogHZX3zAgw3RfJuyiq210": "0C8lCBxrvrv/O7BQfsKnkYogHZX3zAgw3RfJuyiq210" - }, - "signatures": { - "@example:localhost": { - "ed25519:rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0": "AC7oDUW4rUhtInwb4lAoBJ0wAuu4a5k+8e34B5+NKsDB8HXRwgVwUWN/MRWc/sJgtSbVlhzqS9THEmQQ1C51Bw" - } - }, - "usage": [ - "self_signing" - ], - "user_id": "@example:localhost" + "ed25519:rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0": "AC7oDUW4rUhtInwb4lAoBJ0wAuu4a5k+8e34B5+NKsDB8HXRwgVwUWN/MRWc/sJgtSbVlhzqS9THEmQQ1C51Bw" } + } + } }, "user_signing_keys": { + "@example:localhost": { + "user_id": "@example:localhost", + "usage": [ + "user_signing" + ], + "keys": { + "ed25519:DU9z4gBFKFKCk7a13sW9wjT0Iyg7Hqv5f0BPM7DEhPo": "DU9z4gBFKFKCk7a13sW9wjT0Iyg7Hqv5f0BPM7DEhPo" + }, + "signatures": { "@example:localhost": { - "keys": { - "ed25519:DU9z4gBFKFKCk7a13sW9wjT0Iyg7Hqv5f0BPM7DEhPo": "DU9z4gBFKFKCk7a13sW9wjT0Iyg7Hqv5f0BPM7DEhPo" - }, - "signatures": { - "@example:localhost": { - "ed25519:rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0": "C4L2sx9frGqj8w41KyynHGqwUbbwBYRZpYCB+6QWnvQFA5Oi/1PJj8w5anwzEsoO0TWmLYmf7FXuAGewanOWDg" - } - }, - "usage": [ - "user_signing" - ], - "user_id": "@example:localhost" + "ed25519:rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0": "C4L2sx9frGqj8w41KyynHGqwUbbwBYRZpYCB+6QWnvQFA5Oi/1PJj8w5anwzEsoO0TWmLYmf7FXuAGewanOWDg" } - }, - "failures": {} + } + } + } })); - KeyQueryResponse::try_from(data).expect("Can't parse the keys upload response") } + fn device(response: &KeyQueryResponse) -> (ReadOnlyDevice, ReadOnlyDevice) { + let mut devices = response.device_keys.values().next().unwrap().values(); + let first = ReadOnlyDevice::try_from(devices.next().unwrap()).unwrap(); + let second = ReadOnlyDevice::try_from(devices.next().unwrap()).unwrap(); + (first, second) + } + + fn own_identity(response: &KeyQueryResponse) -> OwnUserIdentity { + let user_id = user_id!("@example:localhost"); + + let master_key = response.master_keys.get(&user_id).unwrap(); + let user_signing = response.user_signing_keys.get(&user_id).unwrap(); + let self_signing = response.self_signing_keys.get(&user_id).unwrap(); + + OwnUserIdentity::new(master_key.into(), self_signing.into(), user_signing.into()).unwrap() + } + #[test] fn own_identity_create() { let user_id = user_id!("@example:localhost"); @@ -463,4 +528,43 @@ mod test { UserIdentity::new(master_key.into(), self_signing.into()).unwrap(); } + + #[test] + fn own_identity_check_signatures() { + let response = own_key_query(); + let identity = own_identity(&response); + let (first, second) = device(&response); + + assert!(identity.is_device_signed(&first).is_err()); + assert!(identity.is_device_signed(&second).is_ok()); + + let verification_machine = VerificationMachine::new( + Account::new(second.user_id(), second.device_id()), + Arc::new(Box::new(MemoryStore::new())), + ); + + let first = Device { + inner: first.clone(), + verification_machine: verification_machine.clone(), + own_identity: Some(identity.clone()), + device_owner_identity: Some(UserIdentities::Own(identity.clone())), + }; + + let second = Device { + inner: second.clone(), + verification_machine, + own_identity: Some(identity.clone()), + device_owner_identity: Some(UserIdentities::Own(identity.clone())), + }; + + assert!(!second.trust_state()); + assert!(!second.is_trusted()); + + assert!(!first.trust_state()); + assert!(!first.is_trusted()); + + identity.mark_as_verified(); + assert!(second.trust_state()); + assert!(!first.trust_state()); + } } From 23126c4e4878fc1bbabcd9fd0a93eeb8ac7cb5a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 19 Aug 2020 17:55:28 +0200 Subject: [PATCH 21/49] crypto: Disable the sqlite store test if the feature is disabled. --- matrix_sdk_crypto/src/machine.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index a65373b9..d4b5755b 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -1499,6 +1499,7 @@ pub(crate) mod test { use http::Response; use serde_json::json; + #[cfg(feature = "sqlite_cryptostore")] use tempfile::tempdir; use crate::{ @@ -2059,6 +2060,7 @@ pub(crate) mod test { } #[tokio::test] + #[cfg(feature = "sqlite_cryptostore")] async fn test_machine_with_default_store() { let tmpdir = tempdir().unwrap(); From 1bd15b9fdd883c3755356d2e02156463f6df4ac6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 19 Aug 2020 18:04:06 +0200 Subject: [PATCH 22/49] crypto: Remove some unneeded clones. --- matrix_sdk_crypto/src/user_identity.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index 2ecd549d..5946ace6 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -544,14 +544,14 @@ mod test { ); let first = Device { - inner: first.clone(), + inner: first, verification_machine: verification_machine.clone(), own_identity: Some(identity.clone()), device_owner_identity: Some(UserIdentities::Own(identity.clone())), }; let second = Device { - inner: second.clone(), + inner: second, verification_machine, own_identity: Some(identity.clone()), device_owner_identity: Some(UserIdentities::Own(identity.clone())), From 58185e08e8b6543c5c18977372aabdfcac054cfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 10:18:36 +0200 Subject: [PATCH 23/49] crypto: Move the olm_encrypt() method into the higher level Device. --- matrix_sdk_crypto/src/device.rs | 55 ++++++++++++++++++++++++++- matrix_sdk_crypto/src/machine.rs | 65 ++++---------------------------- 2 files changed, 61 insertions(+), 59 deletions(-) diff --git a/matrix_sdk_crypto/src/device.rs b/matrix_sdk_crypto/src/device.rs index 3f96736d..4ac12dd4 100644 --- a/matrix_sdk_crypto/src/device.rs +++ b/matrix_sdk_crypto/src/device.rs @@ -28,15 +28,17 @@ use matrix_sdk_common::{ keys::SignedKey, to_device::send_event_to_device::IncomingRequest as OwnedToDeviceRequest, }, encryption::DeviceKeys, + events::{room::encrypted::EncryptedEventContent, EventType}, identifiers::{DeviceId, DeviceKeyAlgorithm, DeviceKeyId, EventEncryptionAlgorithm, UserId}, }; use serde_json::{json, Value}; +use tracing::warn; #[cfg(test)] use super::{Account, OlmMachine}; use crate::{ - error::SignatureError, + error::{EventError, OlmError, OlmResult, SignatureError}, store::Result as StoreResult, user_identity::{OwnUserIdentity, UserIdentities}, verification::VerificationMachine, @@ -136,6 +138,57 @@ impl Device { .save_devices(&[self.inner.clone()]) .await } + + /// Encrypt the given content for this `Device`. + /// + /// # Arguments + /// + /// * `event_type` - The type of the event. + /// + /// * `content` - The content of the event that should be encrypted. + pub(crate) async fn encrypt( + &self, + event_type: EventType, + content: Value, + ) -> OlmResult { + let sender_key = if let Some(k) = self.inner.get_key(DeviceKeyAlgorithm::Curve25519) { + k + } else { + warn!( + "Trying to encrypt a Megolm session for user {} on device {}, \ + but the device doesn't have a curve25519 key", + self.user_id(), + self.device_id() + ); + return Err(EventError::MissingSenderKey.into()); + }; + + let mut session = if let Some(s) = self + .verification_machine + .store + .get_sessions(sender_key) + .await? + { + let session = &s.lock().await[0]; + session.clone() + } else { + warn!( + "Trying to encrypt a Megolm session for user {} on device {}, \ + but no Olm session is found", + self.user_id(), + self.device_id() + ); + return Err(OlmError::MissingSession); + }; + + let message = session.encrypt(&self.inner, event_type, content).await; + self.verification_machine + .store + .save_sessions(&[session]) + .await?; + + message + } } /// A read only view over all devices belonging to a user. diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index d4b5755b..ada96b3d 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -993,53 +993,6 @@ impl OlmMachine { Ok(session.encrypt(content).await) } - /// Encrypt the given event for the given Device - /// - /// # Arguments - /// - /// * `reciepient_device` - The device that the event should be encrypted - /// for. - /// - /// * `event_type` - The type of the event. - /// - /// * `content` - The content of the event that should be encrypted. - async fn olm_encrypt( - &self, - recipient_device: &ReadOnlyDevice, - event_type: EventType, - content: Value, - ) -> OlmResult { - let sender_key = if let Some(k) = recipient_device.get_key(DeviceKeyAlgorithm::Curve25519) { - k - } else { - warn!( - "Trying to encrypt a Megolm session for user {} on device {}, \ - but the device doesn't have a curve25519 key", - recipient_device.user_id(), - recipient_device.device_id() - ); - return Err(EventError::MissingSenderKey.into()); - }; - - let mut session = if let Some(s) = self.store.get_sessions(sender_key).await? { - let session = &s.lock().await[0]; - session.clone() - } else { - warn!( - "Trying to encrypt a Megolm session for user {} on device {}, \ - but no Olm session is found", - recipient_device.user_id(), - recipient_device.device_id() - ); - return Err(OlmError::MissingSession); - }; - - let message = session.encrypt(recipient_device, event_type, content).await; - self.store.save_sessions(&[session]).await?; - - message - } - /// Should the client share a group session for the given room. /// /// Returns true if a session needs to be shared before room messages can be @@ -1100,7 +1053,7 @@ impl OlmMachine { let mut devices = Vec::new(); for user_id in users { - for device in self.store.get_user_devices(user_id).await?.devices() { + for device in self.get_user_devices(user_id).await?.devices() { if !device.is_blacklisted() { devices.push(device.clone()); } @@ -1114,8 +1067,8 @@ impl OlmMachine { let mut messages = BTreeMap::new(); for device in device_map_chunk { - let encrypted = self - .olm_encrypt(&device, EventType::RoomKey, key_content.clone()) + let encrypted = device + .encrypt(EventType::RoomKey, key_content.clone()) .await; let encrypted = match encrypted { @@ -1643,16 +1596,14 @@ pub(crate) mod test { let (alice, bob) = get_machine_pair_with_session().await; let bob_device = alice - .store .get_device(&bob.user_id, &bob.device_id) .await - .unwrap() .unwrap(); let event = ToDeviceEvent { sender: alice.user_id.clone(), - content: alice - .olm_encrypt(&bob_device, EventType::Dummy, json!({})) + content: bob_device + .encrypt(EventType::Dummy, json!({})) .await .unwrap(), }; @@ -1924,16 +1875,14 @@ pub(crate) mod test { let (alice, bob) = get_machine_pair_with_session().await; let bob_device = alice - .store .get_device(&bob.user_id, &bob.device_id) .await - .unwrap() .unwrap(); let event = ToDeviceEvent { sender: alice.user_id.clone(), - content: alice - .olm_encrypt(&bob_device, EventType::Dummy, json!({})) + content: bob_device + .encrypt(EventType::Dummy, json!({})) .await .unwrap(), }; From aaa15c768c8fe6833b36831a515329380cc75903 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 10:19:55 +0200 Subject: [PATCH 24/49] crypto: Simplify the Olm message map construction. --- matrix_sdk_crypto/src/machine.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index ada96b3d..9becc8ad 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -1080,16 +1080,13 @@ impl OlmMachine { Err(e) => return Err(e), }; - if !messages.contains_key(device.user_id()) { - messages.insert(device.user_id().clone(), BTreeMap::new()); - }; - - let user_messages = messages.get_mut(device.user_id()).unwrap(); - - user_messages.insert( - DeviceIdOrAllDevices::DeviceId(device.device_id().into()), - serde_json::value::to_raw_value(&encrypted)?, - ); + messages + .entry(device.user_id().clone()) + .or_insert(BTreeMap::new()) + .insert( + DeviceIdOrAllDevices::DeviceId(device.device_id().into()), + serde_json::value::to_raw_value(&encrypted)?, + ); } requests.push(OwnedToDeviceRequest { From 69fbe65ac463caa11d819a78ebe48f0363ebe6f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 10:21:00 +0200 Subject: [PATCH 25/49] crypto: Add some docs for the cross signing keys handling method. --- matrix_sdk_crypto/src/machine.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 9becc8ad..0ceeebde 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -499,6 +499,14 @@ impl OlmMachine { Ok(changed_devices) } + /// Handle the device keys part of a key query response. + /// + /// # Arguments + /// + /// * `response` - The keys query response. + /// + /// Returns a list of identities that changed. Changed here means either + /// they are new, one of their properties has changed or they got deleted. async fn handle_cross_singing_keys( &self, response: &get_keys::Response, From a99e47c310ac69ba2e04a79dc7026be3cfb66900 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 10:23:16 +0200 Subject: [PATCH 26/49] crypto: Shorten some log lines. --- matrix_sdk_crypto/src/machine.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 0ceeebde..ea06e097 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -533,7 +533,11 @@ impl OlmMachine { { UserSigningPubkey::from(s) } else { - warn!("User identity for our own user {} didn't contain a user signing pubkey", user_id); + warn!( + "User identity for our own user {} didn't \ + contain a user signing pubkey", + user_id + ); continue; }; @@ -552,7 +556,8 @@ impl OlmMachine { .map(UserIdentities::Own) } else { warn!( - "User identity for our own user {} didn't contain a user signing pubkey", + "User identity for our own user {} didn't contain a \ + user signing pubkey", user_id ); continue; From ea49a35b436a553d57316c661796294cf410eeaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 10:25:05 +0200 Subject: [PATCH 27/49] crypto: Simplify the function signature of share_group_session. --- matrix_sdk_crypto/src/machine.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index ea06e097..3ee383f7 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -761,7 +761,7 @@ impl OlmMachine { trace!("Successfully decrypted a Olm message: {}", plaintext); - Ok(self.parse_decrypted_to_device_event(sender, &plaintext)?) + self.parse_decrypted_to_device_event(sender, &plaintext) } /// Parse a decrypted Olm message, check that the plaintext and encrypted @@ -1040,15 +1040,12 @@ impl OlmMachine { /// used. /// /// `users` - The list of users that should receive the group session. - pub async fn share_group_session( + pub async fn share_group_session( &self, room_id: &RoomId, users: impl Iterator, - encryption_settings: S, - ) -> OlmResult> - where - S: Into + Sized, - { + 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(); From c3eb4d81064172220ab04c5f0bc0c3f7c3146cf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 10:36:58 +0200 Subject: [PATCH 28/49] crypto: Simplify some more function definitions. --- matrix_sdk_crypto/src/machine.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 3ee383f7..4ffde40f 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -192,10 +192,10 @@ impl OlmMachine { #[cfg(feature = "sqlite_cryptostore")] #[instrument(skip(path, passphrase))] #[cfg_attr(feature = "docs", doc(cfg(r#sqlite_cryptostore)))] - pub async fn new_with_default_store>( + pub async fn new_with_default_store( user_id: &UserId, device_id: &DeviceId, - path: P, + path: impl AsRef, passphrase: &str, ) -> StoreResult { let store = @@ -1316,10 +1316,7 @@ impl OlmMachine { /// /// If the user is already known to the Olm machine it will not be /// considered for a key query. - pub async fn update_tracked_users<'a, I>(&self, users: I) - where - I: IntoIterator, - { + pub async fn update_tracked_users(&self, users: impl IntoIterator) { for user in users { if self.store.is_user_tracked(user) { continue; From b97e3d7bae0ab2007b2881cf37bca4849b2d162a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 10:49:14 +0200 Subject: [PATCH 29/49] crypto: Fix a clippy warning. --- matrix_sdk_crypto/src/machine.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 4ffde40f..fb9e87f0 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -1092,7 +1092,7 @@ impl OlmMachine { messages .entry(device.user_id().clone()) - .or_insert(BTreeMap::new()) + .or_insert_with(BTreeMap::new) .insert( DeviceIdOrAllDevices::DeviceId(device.device_id().into()), serde_json::value::to_raw_value(&encrypted)?, From 74dd0a00d3f4371da62b97e865a52c8ab84d2ff3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 12:23:18 +0200 Subject: [PATCH 30/49] crypto: Simplify the default hashmaps in the memory stores. --- matrix_sdk_crypto/src/memory_stores.rs | 60 +++++++++++--------------- 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/matrix_sdk_crypto/src/memory_stores.rs b/matrix_sdk_crypto/src/memory_stores.rs index 380ac173..307d339b 100644 --- a/matrix_sdk_crypto/src/memory_stores.rs +++ b/matrix_sdk_crypto/src/memory_stores.rs @@ -44,16 +44,15 @@ impl SessionStore { /// Returns true if the the session was added, false if the session was /// already in the store. pub async fn add(&self, session: Session) -> bool { - if !self.entries.contains_key(&*session.sender_key) { - self.entries.insert( - session.sender_key.to_string(), - Arc::new(Mutex::new(Vec::new())), - ); - } - let sessions = self.entries.get_mut(&*session.sender_key).unwrap(); + let sessions_lock = self + .entries + .entry(session.sender_key.to_string()) + .or_insert_with(|| Arc::new(Mutex::new(Vec::new()))); - if !sessions.lock().await.contains(&session) { - sessions.lock().await.push(session); + let mut sessions = sessions_lock.lock().await; + + if !sessions.contains(&session) { + sessions.push(session); true } else { false @@ -93,22 +92,13 @@ impl GroupSessionStore { /// Returns true if the the session was added, false if the session was /// already in the store. pub fn add(&self, session: InboundGroupSession) -> bool { - if !self.entries.contains_key(&session.room_id) { - let room_id = &*session.room_id; - self.entries.insert(room_id.clone(), HashMap::new()); - } - - let mut room_map = self.entries.get_mut(&session.room_id).unwrap(); - - if !room_map.contains_key(&*session.sender_key) { - let sender_key = &*session.sender_key; - room_map.insert(sender_key.to_owned(), HashMap::new()); - } - - let sender_map = room_map.get_mut(&*session.sender_key).unwrap(); - let ret = sender_map.insert(session.session_id().to_owned(), session); - - ret.is_none() + self.entries + .entry((&*session.room_id).clone()) + .or_insert_with(HashMap::new) + .entry(session.sender_key.to_string()) + .or_insert_with(HashMap::new) + .insert(session.session_id().to_owned(), session) + .is_none() } /// Get a inbound group session from our store. @@ -173,13 +163,9 @@ impl DeviceStore { /// Returns true if the device was already in the store, false otherwise. pub fn add(&self, device: ReadOnlyDevice) -> bool { let user_id = device.user_id(); - - if !self.entries.contains_key(&user_id) { - self.entries.insert(user_id.clone(), DashMap::new()); - } - let device_map = self.entries.get_mut(&user_id).unwrap(); - - device_map + self.entries + .entry(user_id.to_owned()) + .or_insert_with(DashMap::new) .insert(device.device_id().into(), device) .is_none() } @@ -203,11 +189,13 @@ impl DeviceStore { /// Get a read-only view over all devices of the given user. pub fn user_devices(&self, user_id: &UserId) -> ReadOnlyUserDevices { - if !self.entries.contains_key(user_id) { - self.entries.insert(user_id.clone(), DashMap::new()); - } ReadOnlyUserDevices { - entries: self.entries.get(user_id).unwrap().clone().into_read_only(), + entries: self + .entries + .entry(user_id.clone()) + .or_insert_with(DashMap::new) + .clone() + .into_read_only(), } } } From a57f63d61416c625d9cd9071628795d7e6e69dc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 14:44:16 +0200 Subject: [PATCH 31/49] crypto: Document the user identities. --- matrix_sdk_crypto/src/user_identity.rs | 186 ++++++++++++++++++++++--- 1 file changed, 163 insertions(+), 23 deletions(-) diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index 5946ace6..fd190039 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -30,12 +30,15 @@ use matrix_sdk_common::{ use crate::{error::SignatureError, verify_json, ReadOnlyDevice}; +/// Wrapper for a cross signing key marking it as the master key. #[derive(Debug, Clone)] pub struct MasterPubkey(Arc); +/// Wrapper for a cross signing key marking it as a self signing key. #[derive(Debug, Clone)] pub struct SelfSigningPubkey(Arc); +/// Wrapper for a cross signing key marking it as a user signing key. #[derive(Debug, Clone)] pub struct UserSigningPubkey(Arc); @@ -69,12 +72,24 @@ impl<'a> From<&'a UserSigningPubkey> for CrossSigningSubKeys<'a> { } } +/// Enum over the cross signing sub-keys. enum CrossSigningSubKeys<'a> { + /// The self signing subkey. SelfSigning(&'a SelfSigningPubkey), + /// The user signing subkey. UserSigning(&'a UserSigningPubkey), } impl<'a> CrossSigningSubKeys<'a> { + /// Get the id of the user that owns this cross signing subkey. + fn user_id(&self) -> &UserId { + match self { + CrossSigningSubKeys::SelfSigning(key) => &key.0.user_id, + CrossSigningSubKeys::UserSigning(key) => &key.0.user_id, + } + } + + /// Get the `CrossSigningKey` from an sub-keys enum fn cross_signing_key(&self) -> &CrossSigningKey { match self { CrossSigningSubKeys::SelfSigning(key) => &key.0, @@ -84,6 +99,23 @@ impl<'a> CrossSigningSubKeys<'a> { } impl MasterPubkey { + /// Get the master key with the given key id. + /// + /// # Arguments + /// + /// * `key_id` - The id of the key that should be fetched. + pub fn get_key(&self, key_id: &DeviceKeyId) -> Option<&str> { + self.0.keys.get(key_id.as_str()).map(|k| k.as_str()) + } + + /// Check if the given cross signing sub-key is signed by the master key. + /// + /// # Arguments + /// + /// * `subkey` - The subkey that should be checked for a valid signature. + /// + /// Returns an empty result if the signature check succeeded, otherwise a + /// SignatureError indicating why the check failed. fn verify_subkey<'a>( &self, subkey: impl Into>, @@ -112,6 +144,15 @@ impl MasterPubkey { } impl UserSigningPubkey { + /// Check if the given master key is signed by this user signing key. + /// + /// # Arguments + /// + /// * `master_key` - The master key that should be checked for a valid + /// signature. + /// + /// Returns an empty result if the signature check succeeded, otherwise a + /// SignatureError indicating why the check failed. fn verify_master_key(&self, master_key: &MasterPubkey) -> Result<(), SignatureError> { let (key_id, key) = self .0 @@ -132,6 +173,14 @@ impl UserSigningPubkey { } impl SelfSigningPubkey { + /// Check if the given device is signed by this self signing key. + /// + /// # Arguments + /// + /// * `device` - The device that should be checked for a valid signature. + /// + /// Returns an empty result if the signature check succeeded, otherwise a + /// SignatureError indicating why the check failed. fn verify_device(&self, device: &ReadOnlyDevice) -> Result<(), SignatureError> { let (key_id, key) = self .0 @@ -151,13 +200,17 @@ impl SelfSigningPubkey { } } +/// Enum over the different user identity types we can have. #[derive(Debug, Clone)] pub enum UserIdentities { + /// Our own user identity. Own(OwnUserIdentity), + /// Identities of other users. Other(UserIdentity), } impl UserIdentities { + /// The unique user id of this identity. pub fn user_id(&self) -> &UserId { match self { UserIdentities::Own(i) => i.user_id(), @@ -172,6 +225,8 @@ impl UserIdentities { } } + /// Destructure the enum into an `OwnUserIdentity` if it's of the correct + /// type. pub fn own(&self) -> Option<&OwnUserIdentity> { match self { UserIdentities::Own(i) => Some(i), @@ -193,6 +248,11 @@ impl PartialEq for UserIdentities { } } +/// Struct representing a cross signing identity of a user. +/// +/// This is the user identity of a user that isn't our own. Other users will +/// only contain a master key and a self signing key, meaning that only device +/// signatures can be checked with this identity. #[derive(Debug, Clone)] pub struct UserIdentity { user_id: Arc, @@ -201,6 +261,16 @@ pub struct UserIdentity { } impl UserIdentity { + /// Create a new user identity with the given master and self signing key. + /// + /// # Arguments + /// + /// * `master_key` - The master key of the user identity. + /// + /// * `self signing key` - The self signing key of user identity. + /// + /// Returns a `SignatureError` if the self signing key fails to be correctly + /// verified by the given master key. pub fn new( master_key: MasterPubkey, self_signing_key: SelfSigningPubkey, @@ -214,6 +284,7 @@ impl UserIdentity { }) } + /// Get the user id of this identity. pub fn user_id(&self) -> &UserId { &self.user_id } @@ -222,6 +293,15 @@ impl UserIdentity { &self.master_key.0.keys } + /// Update the identity with a new master key and self signing key. + /// + /// # Arguments + /// + /// * `master_key` - The new master key of the user identity. + /// + /// * `self_signing_key` - The new self signing key of user identity. + /// + /// Returns a `SignatureError` if we failed to update the identity. pub fn update( &mut self, master_key: MasterPubkey, @@ -235,6 +315,18 @@ impl UserIdentity { Ok(()) } + /// Check if the given device has been signed by this identity. + /// + /// The user_id of the user identity and the user_id of the device need to + /// match for the signature check to succeed as we don't trust users to sign + /// devices of other users. + /// + /// # Arguments + /// + /// * `device` - The device that should be checked for a valid signature. + /// + /// Returns an empty result if the signature check succeeded, otherwise a + /// SignatureError indicating why the check failed. pub fn is_device_signed(&self, device: &ReadOnlyDevice) -> Result<(), SignatureError> { self.self_signing_key.verify_device(device) } @@ -250,6 +342,19 @@ pub struct OwnUserIdentity { } impl OwnUserIdentity { + /// Create a new own user identity with the given master, self signing, and + /// user signing key. + /// + /// # Arguments + /// + /// * `master_key` - The master key of the user identity. + /// + /// * `self_signing_key` - The self signing key of user identity. + /// + /// * `user_signing_key` - The user signing key of user identity. + /// + /// Returns a `SignatureError` if the self signing key fails to be correctly + /// verified by the given master key. pub fn new( master_key: MasterPubkey, self_signing_key: SelfSigningPubkey, @@ -267,10 +372,68 @@ impl OwnUserIdentity { }) } + /// Get the user id of this identity. pub fn user_id(&self) -> &UserId { &self.user_id } + pub fn master_key(&self) -> &BTreeMap { + &self.master_key.0.keys + } + + /// Check if the given identity has been signed by this identity. + /// + /// # Arguments + /// + /// * `identity` - The identity of another user that we want to check if + /// it's has been signed. + /// + /// Returns an empty result if the signature check succeeded, otherwise a + /// SignatureError indicating why the check failed. + pub fn is_identity_signed(&self, identity: &UserIdentity) -> Result<(), SignatureError> { + self.user_signing_key + .verify_master_key(&identity.master_key) + } + + /// Check if the given device has been signed by this identity. + /// + /// Only devices of our own user should be checked with this method, if a + /// device of a different user is given the signature check will always fail + /// even if a valid signature exists. + /// + /// # Arguments + /// + /// * `device` - The device that should be checked for a valid signature. + /// + /// Returns an empty result if the signature check succeeded, otherwise a + /// SignatureError indicating why the check failed. + pub fn is_device_signed(&self, device: &ReadOnlyDevice) -> Result<(), SignatureError> { + self.self_signing_key.verify_device(device) + } + + /// Mark our identity as verified. + pub fn mark_as_verified(&self) { + self.verified.store(true, Ordering::SeqCst) + } + + /// Check if our identity is verified. + pub fn is_verified(&self) -> bool { + self.verified.load(Ordering::SeqCst) + } + + /// Update the identity with a new master key and self signing key. + /// + /// Note: This will reset the verification state if the master keys differ. + /// + /// # Arguments + /// + /// * `master_key` - The new master key of the user identity. + /// + /// * `self_signing_key` - The new self signing key of user identity. + /// + /// * `user_signing_key` - The new user signing key of user identity. + /// + /// Returns a `SignatureError` if we failed to update the identity. pub fn update( &mut self, master_key: MasterPubkey, @@ -285,31 +448,8 @@ impl OwnUserIdentity { self.master_key = master_key; - // FIXME reset the verification state if the master key changed. - Ok(()) } - - pub fn master_key(&self) -> &BTreeMap { - &self.master_key.0.keys - } - - pub fn is_identity_signed(&self, identity: &UserIdentity) -> Result<(), SignatureError> { - self.user_signing_key - .verify_master_key(&identity.master_key) - } - - pub fn is_device_signed(&self, device: &ReadOnlyDevice) -> Result<(), SignatureError> { - self.self_signing_key.verify_device(device) - } - - pub fn mark_as_verified(&self) { - self.verified.store(true, Ordering::SeqCst) - } - - pub fn is_verified(&self) -> bool { - self.verified.load(Ordering::SeqCst) - } } #[cfg(test)] From 89b56b5af85f92c4f046e47eee183b2ddaaef04b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 15:06:49 +0200 Subject: [PATCH 32/49] crypto: Don't expose the btree map of the master key dirrectly. This implements PartialEq for the master key so we can check if they have changed when doing SAS. --- matrix_sdk_crypto/src/user_identity.rs | 28 ++++++++++--------- .../src/verification/sas/helpers.rs | 2 +- matrix_sdk_crypto/src/verification/sas/mod.rs | 6 ++++ 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index fd190039..94e00024 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -13,7 +13,6 @@ // limitations under the License. use std::{ - collections::BTreeMap, convert::TryFrom, sync::{ atomic::{AtomicBool, Ordering}, @@ -42,6 +41,13 @@ pub struct SelfSigningPubkey(Arc); #[derive(Debug, Clone)] pub struct UserSigningPubkey(Arc); +impl PartialEq for MasterPubkey { + fn eq(&self, other: &MasterPubkey) -> bool { + self.0.user_id == other.0.user_id && self.0.keys == other.0.keys + // TODO check the usage once `KeyUsage` gets PartialEq. + } +} + impl From<&CrossSigningKey> for MasterPubkey { fn from(key: &CrossSigningKey) -> Self { Self(Arc::new(key.clone())) @@ -218,7 +224,8 @@ impl UserIdentities { } } - pub fn master_key(&self) -> &BTreeMap { + /// Get the master key of the identity. + pub fn master_key(&self) -> &MasterPubkey { match self { UserIdentities::Own(i) => i.master_key(), UserIdentities::Other(i) => i.master_key(), @@ -233,13 +240,6 @@ impl UserIdentities { _ => None, } } - - pub fn other(&self) -> Option<&UserIdentity> { - match self { - UserIdentities::Other(i) => Some(i), - _ => None, - } - } } impl PartialEq for UserIdentities { @@ -289,8 +289,9 @@ impl UserIdentity { &self.user_id } - pub fn master_key(&self) -> &BTreeMap { - &self.master_key.0.keys + /// Get the public master key of the identity. + pub fn master_key(&self) -> &MasterPubkey { + &self.master_key } /// Update the identity with a new master key and self signing key. @@ -377,8 +378,9 @@ impl OwnUserIdentity { &self.user_id } - pub fn master_key(&self) -> &BTreeMap { - &self.master_key.0.keys + /// Get the public master key of the identity. + pub fn master_key(&self) -> &MasterPubkey { + &self.master_key } /// Check if the given identity has been signed by this identity. diff --git a/matrix_sdk_crypto/src/verification/sas/helpers.rs b/matrix_sdk_crypto/src/verification/sas/helpers.rs index c3afcf86..1c66fcd4 100644 --- a/matrix_sdk_crypto/src/verification/sas/helpers.rs +++ b/matrix_sdk_crypto/src/verification/sas/helpers.rs @@ -204,7 +204,7 @@ pub fn receive_mac_event( return Err(CancelCode::KeyMismatch); } } else if let Some(identity) = &ids.other_identity { - if let Some(key) = identity.master_key().get(key_id.as_str()) { + if let Some(key) = identity.master_key().get_key(&key_id) { // TODO we should check that the master key signs the device, // this way we know the master key also trusts the device if key_mac diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index c116d5b4..9bf9d153 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -246,6 +246,12 @@ impl Sas { Ok(false) } } else { + warn!( + "The master keys of {} have changed while an interactive \ + verification was going on, not marking the identity as verified.", + identity.user_id(), + ); + Ok(false) } } else { From 398edbbe0c85a8d237c1324e1a7d0befaef72008 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 15:13:55 +0200 Subject: [PATCH 33/49] crypto: Reset the verification state of our identity if the master keys change. --- matrix_sdk_crypto/src/user_identity.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index 94e00024..c06e75d4 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -448,6 +448,10 @@ impl OwnUserIdentity { self.self_signing_key = self_signing_key; self.user_signing_key = user_signing_key; + if self.master_key != master_key { + self.verified.store(false, Ordering::SeqCst) + } + self.master_key = master_key; Ok(()) From 9edc87616073bed643cee8f9cdc8069f356cd30d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 15:14:58 +0200 Subject: [PATCH 34/49] crypto: Check that the master key and subkeys have the same user id. --- matrix_sdk_crypto/src/error.rs | 3 +++ matrix_sdk_crypto/src/user_identity.rs | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/matrix_sdk_crypto/src/error.rs b/matrix_sdk_crypto/src/error.rs index d18ab476..cc679c0c 100644 --- a/matrix_sdk_crypto/src/error.rs +++ b/matrix_sdk_crypto/src/error.rs @@ -126,6 +126,9 @@ pub enum SignatureError { #[error("the signing key is missing from the object that signed the message")] MissingSigningKey, + #[error("the user id of the signing differs from the subkey user id")] + UserIdMissmatch, + #[error("the provided JSON value isn't an object")] NotAnObject, diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index c06e75d4..138cbd17 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -137,9 +137,12 @@ impl MasterPubkey { // if self.0.usage.contains(&KeyUsage::Master) { // return Err(SignatureError::MissingSigningKey); // } - let subkey: CrossSigningSubKeys = subkey.into(); + if &self.0.user_id != subkey.user_id() { + return Err(SignatureError::UserIdMissmatch); + } + verify_json( &self.0.user_id, &DeviceKeyId::try_from(key_id.as_str())?, From d908d0f817615c2bc8b732050c6bc8576d88f31a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 15:17:19 +0200 Subject: [PATCH 35/49] crypto: Don't allow user identities to verify devices of other users. --- matrix_sdk_crypto/src/user_identity.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index 138cbd17..2a1206ca 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -332,6 +332,10 @@ impl UserIdentity { /// Returns an empty result if the signature check succeeded, otherwise a /// SignatureError indicating why the check failed. pub fn is_device_signed(&self, device: &ReadOnlyDevice) -> Result<(), SignatureError> { + if self.user_id() != device.user_id() { + return Err(SignatureError::UserIdMissmatch); + } + self.self_signing_key.verify_device(device) } } @@ -413,6 +417,10 @@ impl OwnUserIdentity { /// Returns an empty result if the signature check succeeded, otherwise a /// SignatureError indicating why the check failed. pub fn is_device_signed(&self, device: &ReadOnlyDevice) -> Result<(), SignatureError> { + if self.user_id() != device.user_id() { + return Err(SignatureError::UserIdMissmatch); + } + self.self_signing_key.verify_device(device) } From c2ad298963c1db6f687b671f87dedf258e798172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 15:40:49 +0200 Subject: [PATCH 36/49] crypto: Check that the user ids match for the cross signing keys. --- matrix_sdk_crypto/src/machine.rs | 24 ++++++++++++++++++++++-- matrix_sdk_crypto/src/user_identity.rs | 20 ++++++++++++++++++-- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index fb9e87f0..c36e1ab1 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -552,6 +552,18 @@ impl OlmMachine { } else if user_id == self.user_id() { if let Some(s) = response.user_signing_keys.get(user_id) { let user_signing = UserSigningPubkey::from(s); + + if master_key.user_id() != user_id + || self_signing.user_id() != user_id + || user_signing.user_id() != user_id + { + warn!( + "User id missmatch in one of the cross signing keys for user {}", + user_id + ); + continue; + } + OwnUserIdentity::new(master_key, self_signing, user_signing) .map(UserIdentities::Own) } else { @@ -563,7 +575,15 @@ impl OlmMachine { continue; } } else { - UserIdentity::new(master_key, self_signing).map(UserIdentities::Other) + if master_key.user_id() != user_id || self_signing.user_id() != user_id { + warn!( + "User id missmatch in one of the cross signing keys for user {}", + user_id + ); + continue; + } else { + UserIdentity::new(master_key, self_signing).map(UserIdentities::Other) + } }; match identity { @@ -577,7 +597,7 @@ impl OlmMachine { } Err(e) => { warn!( - "Coulnd't update or create new user identity for {}: {:?}", + "Couldn't update or create new user identity for {}: {:?}", user_id, e ); continue; diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index 2a1206ca..b6c4159a 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -44,7 +44,6 @@ pub struct UserSigningPubkey(Arc); impl PartialEq for MasterPubkey { fn eq(&self, other: &MasterPubkey) -> bool { self.0.user_id == other.0.user_id && self.0.keys == other.0.keys - // TODO check the usage once `KeyUsage` gets PartialEq. } } @@ -105,6 +104,11 @@ impl<'a> CrossSigningSubKeys<'a> { } impl MasterPubkey { + /// Get the user id of the master key's owner. + pub fn user_id(&self) -> &UserId { + &self.0.user_id + } + /// Get the master key with the given key id. /// /// # Arguments @@ -133,6 +137,8 @@ impl MasterPubkey { .next() .ok_or(SignatureError::MissingSigningKey)?; + let key_id = DeviceKeyId::try_from(key_id.as_str())?; + // FIXME `KeyUsage is missing PartialEq. // if self.0.usage.contains(&KeyUsage::Master) { // return Err(SignatureError::MissingSigningKey); @@ -145,7 +151,7 @@ impl MasterPubkey { verify_json( &self.0.user_id, - &DeviceKeyId::try_from(key_id.as_str())?, + &key_id, key, &mut to_value(subkey.cross_signing_key()).map_err(|_| SignatureError::NotAnObject)?, ) @@ -153,6 +159,11 @@ impl MasterPubkey { } impl UserSigningPubkey { + /// Get the user id of the user signing key's owner. + pub fn user_id(&self) -> &UserId { + &self.0.user_id + } + /// Check if the given master key is signed by this user signing key. /// /// # Arguments @@ -182,6 +193,11 @@ impl UserSigningPubkey { } impl SelfSigningPubkey { + /// Get the user id of the self signing key's owner. + pub fn user_id(&self) -> &UserId { + &self.0.user_id + } + /// Check if the given device is signed by this self signing key. /// /// # Arguments From 552a12eeed34919129619f13ff5748377afcc8c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 15:52:40 +0200 Subject: [PATCH 37/49] crypto: More docs for the user identities. --- matrix_sdk_crypto/src/user_identity.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/matrix_sdk_crypto/src/user_identity.rs b/matrix_sdk_crypto/src/user_identity.rs index b6c4159a..4594b6f9 100644 --- a/matrix_sdk_crypto/src/user_identity.rs +++ b/matrix_sdk_crypto/src/user_identity.rs @@ -30,14 +30,21 @@ use matrix_sdk_common::{ use crate::{error::SignatureError, verify_json, ReadOnlyDevice}; /// Wrapper for a cross signing key marking it as the master key. +/// +/// Master keys are used to sign other cross signing keys, the self signing and +/// user signing keys of an user will be signed by their master key. #[derive(Debug, Clone)] pub struct MasterPubkey(Arc); /// Wrapper for a cross signing key marking it as a self signing key. +/// +/// Self signing keys are used to sign the user's own devices. #[derive(Debug, Clone)] pub struct SelfSigningPubkey(Arc); /// Wrapper for a cross signing key marking it as a user signing key. +/// +/// User signing keys are used to sign the master keys of other users. #[derive(Debug, Clone)] pub struct UserSigningPubkey(Arc); @@ -356,6 +363,13 @@ impl UserIdentity { } } +/// Struct representing a cross signing identity of our own user. +/// +/// This is the user identity of our own user. This user identity will contain a +/// master key, self signing key as well as a user signing key. +/// +/// This identity can verify other identities as well as devices belonging to +/// the identity. #[derive(Debug, Clone)] pub struct OwnUserIdentity { user_id: Arc, From c307690c2e68c5972009d729823f90abe2504619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 16:06:06 +0200 Subject: [PATCH 38/49] crypto: Fix a clippy warning and some spelling. --- matrix_sdk_crypto/src/machine.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index c36e1ab1..12c5c431 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -558,7 +558,7 @@ impl OlmMachine { || user_signing.user_id() != user_id { warn!( - "User id missmatch in one of the cross signing keys for user {}", + "User id mismatch in one of the cross signing keys for user {}", user_id ); continue; @@ -574,16 +574,14 @@ impl OlmMachine { ); continue; } + } else if master_key.user_id() != user_id || self_signing.user_id() != user_id { + warn!( + "User id mismatch in one of the cross signing keys for user {}", + user_id + ); + continue; } else { - if master_key.user_id() != user_id || self_signing.user_id() != user_id { - warn!( - "User id missmatch in one of the cross signing keys for user {}", - user_id - ); - continue; - } else { - UserIdentity::new(master_key, self_signing).map(UserIdentities::Other) - } + UserIdentity::new(master_key, self_signing).map(UserIdentities::Other) }; match identity { From 202c20feda2b1b5dab88cb7a58ded42c2c7b330b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 20 Aug 2020 18:01:34 +0200 Subject: [PATCH 39/49] crypto: Rename the method to set the local trust of a device. --- matrix_sdk/src/device.rs | 9 ++++++--- matrix_sdk_crypto/src/device.rs | 7 +++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/matrix_sdk/src/device.rs b/matrix_sdk/src/device.rs index 241c883f..49dfd5ab 100644 --- a/matrix_sdk/src/device.rs +++ b/matrix_sdk/src/device.rs @@ -81,16 +81,19 @@ impl Device { self.inner.trust_state() } - /// Set the trust state of the device to the given state. + /// Set the local trust state of the device to the given state. + /// + /// This won't affect any cross signing trust state, this only sets a flag + /// marking to have the given trust state. /// /// # Arguments /// /// * `trust_state` - The new trust state that should be set for the device. - pub async fn set_trust_state( + pub async fn set_local_trust( &self, trust_state: LocalTrust, ) -> StdResult<(), CryptoStoreError> { - self.inner.set_trust_state(trust_state).await + self.inner.set_local_trust(trust_state).await } } diff --git a/matrix_sdk_crypto/src/device.rs b/matrix_sdk_crypto/src/device.rs index 4ac12dd4..47589595 100644 --- a/matrix_sdk_crypto/src/device.rs +++ b/matrix_sdk_crypto/src/device.rs @@ -125,12 +125,15 @@ impl Device { } } - /// Set the trust state of the device to the given state. + /// Set the local trust state of the device to the given state. + /// + /// This won't affect any cross signing trust state, this only sets a flag + /// marking to have the given trust state. /// /// # Arguments /// /// * `trust_state` - The new trust state that should be set for the device. - pub async fn set_trust_state(&self, trust_state: LocalTrust) -> StoreResult<()> { + pub async fn set_local_trust(&self, trust_state: LocalTrust) -> StoreResult<()> { self.inner.set_trust_state(trust_state); self.verification_machine From ce93869915800ad401336093c4c525c340009d43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 21 Aug 2020 09:50:01 +0200 Subject: [PATCH 40/49] crypto: Return an Option instead of an empty result for the key uploads. --- matrix_sdk_base/src/client.rs | 4 ++-- matrix_sdk_crypto/src/machine.rs | 24 +++++++++++------------- matrix_sdk_crypto/src/olm/account.rs | 17 +++++++---------- 3 files changed, 20 insertions(+), 25 deletions(-) diff --git a/matrix_sdk_base/src/client.rs b/matrix_sdk_base/src/client.rs index ebaf9af3..f6d15d22 100644 --- a/matrix_sdk_base/src/client.rs +++ b/matrix_sdk_base/src/client.rs @@ -1337,12 +1337,12 @@ impl BaseClient { /// Returns an empty error if no keys need to be uploaded. #[cfg(feature = "encryption")] #[cfg_attr(feature = "docs", doc(cfg(encryption)))] - pub async fn keys_for_upload(&self) -> StdResult { + pub async fn keys_for_upload(&self) -> Option { let olm = self.olm.lock().await; match &*olm { Some(o) => o.keys_for_upload().await, - None => Err(()), + None => None, } } diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 12c5c431..6c71ba28 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -18,7 +18,6 @@ use std::{ collections::{BTreeMap, HashSet}, convert::{TryFrom, TryInto}, mem, - result::Result as StdResult, sync::Arc, }; @@ -26,15 +25,14 @@ use dashmap::DashMap; use serde_json::Value; use tracing::{debug, error, info, instrument, trace, warn}; -use api::r0::{ - keys::{claim_keys, get_keys, upload_keys, OneTimeKey}, - sync::sync_events::Response as SyncResponse, - to_device::{ - send_event_to_device::IncomingRequest as OwnedToDeviceRequest, DeviceIdOrAllDevices, - }, -}; use matrix_sdk_common::{ - api, + api::r0::{ + keys::{claim_keys, get_keys, upload_keys, OneTimeKey}, + sync::sync_events::Response as SyncResponse, + to_device::{ + send_event_to_device::IncomingRequest as OwnedToDeviceRequest, DeviceIdOrAllDevices, + }, + }, encryption::DeviceKeys, events::{ forwarded_room_key::ForwardedRoomKeyEventContent, @@ -631,17 +629,17 @@ impl OlmMachine { /// Get a request to upload E2EE keys to the server. /// - /// Returns an empty error if no keys need to be uploaded. + /// Returns None if no keys need to be uploaded. /// /// The response of a successful key upload requests needs to be passed to /// the [`OlmMachine`] with the [`receive_keys_upload_response`]. /// /// [`receive_keys_upload_response`]: #method.receive_keys_upload_response /// [`OlmMachine`]: struct.OlmMachine.html - pub async fn keys_for_upload(&self) -> StdResult { + pub async fn keys_for_upload(&self) -> Option { let (device_keys, one_time_keys) = self.account.keys_for_upload().await?; - Ok(upload_keys::Request { + Some(upload_keys::Request { device_keys, one_time_keys, }) @@ -1813,7 +1811,7 @@ pub(crate) mod test { .unwrap(); let ret = machine.keys_for_upload().await; - assert!(ret.is_err()); + assert!(ret.is_none()); } #[tokio::test] diff --git a/matrix_sdk_crypto/src/olm/account.rs b/matrix_sdk_crypto/src/olm/account.rs index 2e60b493..86cd7d54 100644 --- a/matrix_sdk_crypto/src/olm/account.rs +++ b/matrix_sdk_crypto/src/olm/account.rs @@ -200,18 +200,15 @@ impl Account { /// Get a tuple of device and one-time keys that need to be uploaded. /// - /// Returns an empty error if no keys need to be uploaded. + /// Returns None if no keys need to be uploaded. pub(crate) async fn keys_for_upload( &self, - ) -> Result< - ( - Option, - Option>, - ), - (), - > { + ) -> Option<( + Option, + Option>, + )> { if !self.should_upload_keys().await { - return Err(()); + return None; } let device_keys = if !self.shared() { @@ -222,7 +219,7 @@ impl Account { let one_time_keys = self.signed_one_time_keys().await.ok(); - Ok((device_keys, one_time_keys)) + Some((device_keys, one_time_keys)) } /// Mark the current set of one-time keys as being published. From 9fe23227af6fdbbdda8a2d07463ee1bc908c3a2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 21 Aug 2020 12:44:14 +0200 Subject: [PATCH 41/49] base: Fix the encryption settings Into implementation. --- matrix_sdk_base/src/models/room.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix_sdk_base/src/models/room.rs b/matrix_sdk_base/src/models/room.rs index 3015fd8f..4aa509f6 100644 --- a/matrix_sdk_base/src/models/room.rs +++ b/matrix_sdk_base/src/models/room.rs @@ -165,7 +165,7 @@ impl Into for EncryptionInfo { fn into(self) -> EncryptionSettings { EncryptionSettings { algorithm: self.algorithm, - rotation_period: Duration::from_millis(self.rotation_period_messages), + rotation_period: Duration::from_millis(self.rotation_period_ms), rotation_period_msgs: self.rotation_period_messages, } } From aee40977a3304857b7beff8c33830d57f0653ffd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 21 Aug 2020 12:46:11 +0200 Subject: [PATCH 42/49] crypto: Clamp the rotation period ms so users can't wedge E2E. Users may set a very small rotation period this might mean that a session might expire by the time it's shared ending up in a loop where we constantly need to share a group session yet never manage to send a message. --- matrix_sdk_crypto/src/olm/group_sessions.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/matrix_sdk_crypto/src/olm/group_sessions.rs b/matrix_sdk_crypto/src/olm/group_sessions.rs index a2710c38..40949632 100644 --- a/matrix_sdk_crypto/src/olm/group_sessions.rs +++ b/matrix_sdk_crypto/src/olm/group_sessions.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::{ + cmp::min, convert::TryInto, fmt, sync::{ @@ -406,7 +407,11 @@ impl OutboundGroupSession { let count = self.message_count.load(Ordering::SeqCst); count >= self.settings.rotation_period_msgs - || self.creation_time.elapsed() >= self.settings.rotation_period + || self.creation_time.elapsed() + // Since the encryption settings are provided by users and not + // checked someone could set a really low rotation perdiod so + // clamp it at a minute. + >= min(self.settings.rotation_period, Duration::from_secs(3600)) } /// Mark the session as shared. From 93e1967119808fbedb73d0f80049b45544cbeb62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 21 Aug 2020 13:34:25 +0200 Subject: [PATCH 43/49] crypto: Initial refactor to switch to the outgoing_requests queue. --- matrix_sdk/src/client.rs | 86 +++++++++------------- matrix_sdk_base/src/client.rs | 115 +++++++----------------------- matrix_sdk_base/src/lib.rs | 3 +- matrix_sdk_crypto/src/lib.rs | 2 + matrix_sdk_crypto/src/machine.rs | 92 +++++++++++++++++++----- matrix_sdk_crypto/src/requests.rs | 88 +++++++++++++++++++++++ 6 files changed, 228 insertions(+), 158 deletions(-) create mode 100644 matrix_sdk_crypto/src/requests.rs diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index 674ae84d..5fe8dbc3 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -38,7 +38,7 @@ use tracing::{error, info, instrument}; use matrix_sdk_base::{BaseClient, BaseClientConfig, Room, Session, StateStore}; #[cfg(feature = "encryption")] -use matrix_sdk_base::CryptoStoreError; +use matrix_sdk_base::{CryptoStoreError, OutgoingRequests}; use matrix_sdk_common::{ api::r0::{ @@ -1235,29 +1235,27 @@ impl Client { #[cfg(feature = "encryption")] { - if self.base_client.should_upload_keys().await { - let response = self.keys_upload().await; + for r in self.base_client.outgoing_requests().await { + match r.request { + OutgoingRequests::KeysQuery(request) => { + if let Err(e) = self.keys_query(&r.request_id, request).await { + warn!("Error while querying device keys {:?}", e); + } + } - if let Err(e) = response { - warn!("Error while uploading E2EE keys {:?}", e); - } - } - - if self.base_client.should_query_keys().await { - let response = self.keys_query().await; - - if let Err(e) = response { - warn!("Error while querying device keys {:?}", e); - } - } - - for request in self.base_client.outgoing_to_device_requests().await { - let txn_id = request.txn_id.clone(); - - if self.send_to_device(request).await.is_ok() { - self.base_client - .mark_to_device_request_as_sent(&txn_id) - .await; + OutgoingRequests::KeysUpload(request) => { + if let Err(e) = self.keys_upload(&r.request_id, request).await { + warn!("Error while querying device keys {:?}", e); + } + } + OutgoingRequests::ToDeviceRequest(request) => { + if let Ok(resp) = self.send_to_device(request).await { + self.base_client + .mark_request_as_sent(&r.request_id, &resp) + .await + .unwrap(); + } + } } } } @@ -1356,13 +1354,11 @@ impl Client { #[cfg(feature = "encryption")] #[cfg_attr(feature = "docs", doc(cfg(encryption)))] #[instrument] - async fn keys_upload(&self) -> Result { - let request = self - .base_client - .keys_for_upload() - .await - .expect("Keys don't need to be uploaded"); - + async fn keys_upload( + &self, + request_id: &Uuid, + request: upload_keys::Request, + ) -> Result { debug!( "Uploading encryption keys device keys: {}, one-time-keys: {}", request.device_keys.is_some(), @@ -1371,8 +1367,9 @@ impl Client { let response = self.send(request).await?; self.base_client - .receive_keys_upload_response(&response) + .mark_request_as_sent(request_id, &response) .await?; + Ok(response) } @@ -1390,33 +1387,20 @@ impl Client { #[cfg(feature = "encryption")] #[cfg_attr(feature = "docs", doc(cfg(encryption)))] #[instrument] - async fn keys_query(&self) -> Result { - let mut users_for_query = self - .base_client - .users_for_key_query() - .await - .expect("Keys don't need to be uploaded"); - - debug!( - "Querying device keys device for users: {:?}", - users_for_query - ); - - let mut device_keys: BTreeMap>> = BTreeMap::new(); - - for user in users_for_query.drain() { - device_keys.insert(user, Vec::new()); - } - + async fn keys_query( + &self, + request_id: &Uuid, + request: get_keys::IncomingRequest, + ) -> Result { let request = get_keys::Request { timeout: None, - device_keys, + device_keys: request.device_keys, token: None, }; let response = self.send(request).await?; self.base_client - .receive_keys_query_response(&response) + .mark_request_as_sent(request_id, &response) .await?; Ok(response) diff --git a/matrix_sdk_base/src/client.rs b/matrix_sdk_base/src/client.rs index f6d15d22..57761618 100644 --- a/matrix_sdk_base/src/client.rs +++ b/matrix_sdk_base/src/client.rs @@ -14,7 +14,7 @@ // limitations under the License. #[cfg(feature = "encryption")] -use std::collections::{BTreeMap, HashSet}; +use std::collections::BTreeMap; use std::{ collections::HashMap, fmt, @@ -36,15 +36,12 @@ use matrix_sdk_common::{ identifiers::{RoomId, UserId}, locks::RwLock, push::Ruleset, + uuid::Uuid, Raw, }; #[cfg(feature = "encryption")] use matrix_sdk_common::{ - api::r0::keys::{ - claim_keys::Response as KeysClaimResponse, - get_keys::Response as KeysQueryResponse, - upload_keys::{Request as KeysUploadRequest, Response as KeysUploadResponse}, - }, + api::r0::keys::claim_keys::Response as KeysClaimResponse, api::r0::to_device::send_event_to_device::IncomingRequest as OwnedToDeviceRequest, events::room::{ encrypted::EncryptedEventContent, message::MessageEventContent as MsgEventContent, @@ -53,7 +50,8 @@ use matrix_sdk_common::{ }; #[cfg(feature = "encryption")] use matrix_sdk_crypto::{ - CryptoStore, CryptoStoreError, Device, OlmError, OlmMachine, Sas, UserDevices, + CryptoStore, CryptoStoreError, Device, IncomingResponse, OlmError, OlmMachine, OutgoingRequest, + Sas, UserDevices, }; use zeroize::Zeroizing; @@ -1229,18 +1227,6 @@ impl BaseClient { Ok(updated) } - /// Should account or one-time keys be uploaded to the server. - #[cfg(feature = "encryption")] - #[cfg_attr(feature = "docs", doc(cfg(encryption)))] - pub async fn should_upload_keys(&self) -> bool { - let olm = self.olm.lock().await; - - match &*olm { - Some(o) => o.should_upload_keys().await, - None => false, - } - } - /// Should the client share a group session for the given room. /// /// Returns true if a session needs to be shared before room messages can be @@ -1260,15 +1246,31 @@ impl BaseClient { } } - /// Should users be queried for their device keys. + /// Get the list of outgoing requests that need to be sent out. #[cfg(feature = "encryption")] #[cfg_attr(feature = "docs", doc(cfg(encryption)))] - pub async fn should_query_keys(&self) -> bool { + pub async fn outgoing_requests(&self) -> Vec { let olm = self.olm.lock().await; match &*olm { - Some(o) => o.should_query_keys().await, - None => false, + Some(o) => o.outgoing_requests().await, + None => vec![], + } + } + + /// Get the list of outgoing requests that need to be sent out. + #[cfg(feature = "encryption")] + #[cfg_attr(feature = "docs", doc(cfg(encryption)))] + pub async fn mark_request_as_sent<'a>( + &self, + request_id: &Uuid, + response: impl Into>, + ) -> Result<()> { + let olm = self.olm.lock().await; + + match &*olm { + Some(o) => Ok(o.mark_requests_as_sent(request_id, response).await?), + None => Ok(()), } } @@ -1332,53 +1334,6 @@ impl BaseClient { } } - /// Get a tuple of device and one-time keys that need to be uploaded. - /// - /// Returns an empty error if no keys need to be uploaded. - #[cfg(feature = "encryption")] - #[cfg_attr(feature = "docs", doc(cfg(encryption)))] - pub async fn keys_for_upload(&self) -> Option { - let olm = self.olm.lock().await; - - match &*olm { - Some(o) => o.keys_for_upload().await, - None => None, - } - } - - /// Get the users that we need to query keys for. - /// - /// Returns an empty error if no keys need to be queried. - #[cfg(feature = "encryption")] - #[cfg_attr(feature = "docs", doc(cfg(encryption)))] - pub async fn users_for_key_query(&self) -> StdResult, ()> { - let olm = self.olm.lock().await; - - match &*olm { - Some(o) => Ok(o.users_for_key_query().await), - None => Err(()), - } - } - - /// Receive a successful keys upload response. - /// - /// # Arguments - /// - /// * `response` - The keys upload response of the request that the client - /// performed. - /// - /// # Panics - /// Panics if the client hasn't been logged in. - #[cfg(feature = "encryption")] - #[cfg_attr(feature = "docs", doc(cfg(encryption)))] - pub async fn receive_keys_upload_response(&self, response: &KeysUploadResponse) -> Result<()> { - let olm = self.olm.lock().await; - - let o = olm.as_ref().expect("Client isn't logged in."); - o.receive_keys_upload_response(response).await?; - Ok(()) - } - /// Receive a successful keys claim response. /// /// # Arguments @@ -1398,26 +1353,6 @@ impl BaseClient { Ok(()) } - /// Receive a successful keys query response. - /// - /// # Arguments - /// - /// * `response` - The keys query response of the request that the client - /// performed. - /// - /// # Panics - /// Panics if the client hasn't been logged in. - #[cfg(feature = "encryption")] - #[cfg_attr(feature = "docs", doc(cfg(encryption)))] - pub async fn receive_keys_query_response(&self, response: &KeysQueryResponse) -> Result<()> { - let olm = self.olm.lock().await; - - let o = olm.as_ref().expect("Client isn't logged in."); - o.receive_keys_query_response(response).await?; - // TODO notify our callers of new devices via some callback. - Ok(()) - } - /// Invalidate the currently active outbound group session for the given /// room. /// diff --git a/matrix_sdk_base/src/lib.rs b/matrix_sdk_base/src/lib.rs index 4bf782ce..f96d1728 100644 --- a/matrix_sdk_base/src/lib.rs +++ b/matrix_sdk_base/src/lib.rs @@ -57,7 +57,8 @@ pub use state::{AllRooms, ClientState}; #[cfg(feature = "encryption")] #[cfg_attr(feature = "docs", doc(cfg(encryption)))] pub use matrix_sdk_crypto::{ - CryptoStoreError, Device, LocalTrust, ReadOnlyDevice, Sas, UserDevices, + CryptoStoreError, Device, IncomingResponse, LocalTrust, OutgoingRequest, OutgoingRequests, + ReadOnlyDevice, Sas, UserDevices, }; #[cfg(feature = "messages")] diff --git a/matrix_sdk_crypto/src/lib.rs b/matrix_sdk_crypto/src/lib.rs index 21a9b487..5fcc422b 100644 --- a/matrix_sdk_crypto/src/lib.rs +++ b/matrix_sdk_crypto/src/lib.rs @@ -32,6 +32,7 @@ mod error; mod machine; mod memory_stores; mod olm; +mod requests; mod store; #[allow(dead_code)] mod user_identity; @@ -44,6 +45,7 @@ pub use memory_stores::{DeviceStore, GroupSessionStore, ReadOnlyUserDevices, Ses pub use olm::{ Account, EncryptionSettings, IdentityKeys, InboundGroupSession, OutboundGroupSession, Session, }; +pub use requests::{IncomingResponse, OutgoingRequest, OutgoingRequests}; #[cfg(feature = "sqlite_cryptostore")] pub use store::sqlite::SqliteStore; pub use store::{CryptoStore, CryptoStoreError}; diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 6c71ba28..21745032 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -27,7 +27,11 @@ use tracing::{debug, error, info, instrument, trace, warn}; use matrix_sdk_common::{ api::r0::{ - keys::{claim_keys, get_keys, upload_keys, OneTimeKey}, + keys::{ + claim_keys, + get_keys::{IncomingRequest as KeysQueryRequest, Response as KeysQueryResponse}, + upload_keys, OneTimeKey, + }, sync::sync_events::Response as SyncResponse, to_device::{ send_event_to_device::IncomingRequest as OwnedToDeviceRequest, DeviceIdOrAllDevices, @@ -57,6 +61,7 @@ use super::{ Account, EncryptionSettings, GroupSessionKey, IdentityKeys, InboundGroupSession, OlmMessage, OutboundGroupSession, }, + requests::{IncomingResponse, OutgoingRequest}, store::{memorystore::MemoryStore, Result as StoreResult}, user_identity::{ MasterPubkey, OwnUserIdentity, SelfSigningPubkey, UserIdentities, UserIdentity, @@ -217,6 +222,48 @@ impl OlmMachine { self.account.identity_keys() } + /// Get the outgoing requests that need to be sent out. + pub async fn outgoing_requests(&self) -> Vec { + let mut requests = Vec::new(); + + if let Some(r) = self.keys_for_upload().await.map(|r| OutgoingRequest { + request_id: Uuid::new_v4(), + request: r.into(), + }) { + requests.push(r); + } + + if let Some(r) = self.users_for_key_query().await.map(|r| OutgoingRequest { + request_id: Uuid::new_v4(), + request: r.into(), + }) { + requests.push(r); + } + + requests + } + + /// Mark the request with the given request id as sent. + pub async fn mark_requests_as_sent<'a>( + &self, + request_id: &Uuid, + response: impl Into>, + ) -> OlmResult<()> { + match response.into() { + IncomingResponse::KeysQuery(response) => { + self.receive_keys_query_response(response).await?; + } + IncomingResponse::KeysUpload(response) => { + self.receive_keys_upload_response(response).await?; + } + IncomingResponse::ToDevice(_) => { + self.mark_to_device_request_as_sent(&request_id.to_string()); + } + }; + + Ok(()) + } + /// Should device or one-time keys be uploaded to the server. /// /// This needs to be checked periodically, ideally after every sync request. @@ -241,7 +288,8 @@ impl OlmMachine { /// } /// # }); /// ``` - pub async fn should_upload_keys(&self) -> bool { + #[cfg(test)] + async fn should_upload_keys(&self) -> bool { self.account.should_upload_keys().await } @@ -269,7 +317,7 @@ impl OlmMachine { /// * `response` - The keys upload response of the request that the client /// performed. #[instrument] - pub async fn receive_keys_upload_response( + async fn receive_keys_upload_response( &self, response: &upload_keys::Response, ) -> OlmResult<()> { @@ -507,7 +555,7 @@ impl OlmMachine { /// they are new, one of their properties has changed or they got deleted. async fn handle_cross_singing_keys( &self, - response: &get_keys::Response, + response: &KeysQueryResponse, ) -> StoreResult> { let mut changed = Vec::new(); @@ -613,9 +661,9 @@ impl OlmMachine { /// /// * `response` - The keys query response of the request that the client /// performed. - pub async fn receive_keys_query_response( + async fn receive_keys_query_response( &self, - response: &get_keys::Response, + response: &KeysQueryResponse, ) -> OlmResult<(Vec, Vec)> { let changed_devices = self .handle_devices_from_key_query(&response.device_keys) @@ -636,7 +684,7 @@ impl OlmMachine { /// /// [`receive_keys_upload_response`]: #method.receive_keys_upload_response /// [`OlmMachine`]: struct.OlmMachine.html - pub async fn keys_for_upload(&self) -> Option { + async fn keys_for_upload(&self) -> Option { let (device_keys, one_time_keys) = self.account.keys_for_upload().await?; Some(upload_keys::Request { @@ -1344,22 +1392,34 @@ impl OlmMachine { } } - /// Should the client perform a key query request. - pub async fn should_query_keys(&self) -> bool { - self.store.has_users_for_key_query() - } - - /// Get the set of users that we need to query keys for. + /// Get a key query request if one is needed. /// - /// Returns a hash set of users that need to be queried for keys. + /// Returns a key query reqeust if the client should query E2E keys, + /// otherwise None. /// /// The response of a successful key query requests needs to be passed to /// the [`OlmMachine`] with the [`receive_keys_query_response`]. /// /// [`OlmMachine`]: struct.OlmMachine.html /// [`receive_keys_query_response`]: #method.receive_keys_query_response - pub async fn users_for_key_query(&self) -> HashSet { - self.store.users_for_key_query() + async fn users_for_key_query(&self) -> Option { + let mut users = self.store.users_for_key_query(); + + if users.is_empty() { + None + } else { + let mut device_keys: BTreeMap>> = BTreeMap::new(); + + for user in users.drain() { + device_keys.insert(user, Vec::new()); + } + + Some(KeysQueryRequest { + timeout: None, + device_keys, + token: None, + }) + } } /// Get a specific device of a user. diff --git a/matrix_sdk_crypto/src/requests.rs b/matrix_sdk_crypto/src/requests.rs new file mode 100644 index 00000000..18b6e12d --- /dev/null +++ b/matrix_sdk_crypto/src/requests.rs @@ -0,0 +1,88 @@ +// 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. + +use matrix_sdk_common::{ + api::r0::{ + keys::{ + get_keys::{IncomingRequest as KeysQueryRequest, Response as KeysQueryResponse}, + upload_keys::{Request as KeysUploadRequest, Response as KeysUploadResponse}, + }, + to_device::send_event_to_device::{ + IncomingRequest as ToDeviceRequest, Response as ToDeviceResponse, + }, + }, + uuid::Uuid, +}; + +/// TODO +#[derive(Debug)] +pub enum OutgoingRequests { + /// TODO + KeysUpload(KeysUploadRequest), + /// TODO + KeysQuery(KeysQueryRequest), + /// TODO + ToDeviceRequest(ToDeviceRequest), +} + +impl From for OutgoingRequests { + fn from(request: KeysQueryRequest) -> Self { + OutgoingRequests::KeysQuery(request) + } +} + +impl From for OutgoingRequests { + fn from(request: KeysUploadRequest) -> Self { + OutgoingRequests::KeysUpload(request) + } +} + +/// TODO +#[derive(Debug)] +pub enum IncomingResponse<'a> { + /// TODO + KeysUpload(&'a KeysUploadResponse), + /// TODO + KeysQuery(&'a KeysQueryResponse), + /// TODO + ToDevice(&'a ToDeviceResponse), +} + +impl<'a> From<&'a KeysUploadResponse> for IncomingResponse<'a> { + fn from(response: &'a KeysUploadResponse) -> Self { + IncomingResponse::KeysUpload(response) + } +} + +impl<'a> From<&'a KeysQueryResponse> for IncomingResponse<'a> { + fn from(response: &'a KeysQueryResponse) -> Self { + IncomingResponse::KeysQuery(response) + } +} + +impl<'a> From<&'a ToDeviceResponse> for IncomingResponse<'a> { + fn from(response: &'a ToDeviceResponse) -> Self { + IncomingResponse::ToDevice(response) + } +} + +/// TODO +#[derive(Debug)] +pub struct OutgoingRequest { + /// The unique id of a request, needs to be passed when receiving a + /// response. + pub request_id: Uuid, + /// TODO + pub request: OutgoingRequests, +} From e38bfc64f417642c23215bcd202b1acd2b35e236 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 21 Aug 2020 14:40:49 +0200 Subject: [PATCH 44/49] crypto: Streamline the key claiming so we use the new mark request as sent method. --- matrix_sdk/src/client.rs | 18 ++----- matrix_sdk_base/src/client.rs | 29 ++-------- matrix_sdk_crypto/src/lib.rs | 2 +- matrix_sdk_crypto/src/machine.rs | 88 +++++++++++++++++-------------- matrix_sdk_crypto/src/requests.rs | 9 ++++ 5 files changed, 68 insertions(+), 78 deletions(-) diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index 5fe8dbc3..a13fddc2 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -13,8 +13,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#[cfg(feature = "encryption")] -use std::collections::BTreeMap; use std::{ collections::HashMap, convert::{TryFrom, TryInto}, @@ -76,7 +74,6 @@ use matrix_sdk_common::{ Response as ToDeviceResponse, }, }, - identifiers::DeviceKeyAlgorithm, locks::Mutex, }; @@ -970,10 +967,9 @@ impl Client { self.base_client.get_missing_sessions(members).await? }; - if !missing_sessions.is_empty() { - self.claim_one_time_keys(missing_sessions).await?; + if let Some((request_id, request)) = missing_sessions { + self.claim_one_time_keys(&request_id, request).await?; } - let response = self.share_group_session(room_id).await; self.group_session_locks.remove(room_id); @@ -1301,16 +1297,12 @@ impl Client { #[instrument] async fn claim_one_time_keys( &self, - one_time_keys: BTreeMap, DeviceKeyAlgorithm>>, + request_id: &Uuid, + request: claim_keys::Request, ) -> Result { - let request = claim_keys::Request { - timeout: None, - one_time_keys, - }; - let response = self.send(request).await?; self.base_client - .receive_keys_claim_response(&response) + .mark_request_as_sent(request_id, &response) .await?; Ok(response) } diff --git a/matrix_sdk_base/src/client.rs b/matrix_sdk_base/src/client.rs index 57761618..d5554002 100644 --- a/matrix_sdk_base/src/client.rs +++ b/matrix_sdk_base/src/client.rs @@ -13,8 +13,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#[cfg(feature = "encryption")] -use std::collections::BTreeMap; use std::{ collections::HashMap, fmt, @@ -41,12 +39,12 @@ use matrix_sdk_common::{ }; #[cfg(feature = "encryption")] use matrix_sdk_common::{ - api::r0::keys::claim_keys::Response as KeysClaimResponse, + api::r0::keys::claim_keys::Request as KeysClaimRequest, api::r0::to_device::send_event_to_device::IncomingRequest as OwnedToDeviceRequest, events::room::{ encrypted::EncryptedEventContent, message::MessageEventContent as MsgEventContent, }, - identifiers::{DeviceId, DeviceKeyAlgorithm}, + identifiers::DeviceId, }; #[cfg(feature = "encryption")] use matrix_sdk_crypto::{ @@ -1282,12 +1280,12 @@ impl BaseClient { pub async fn get_missing_sessions( &self, users: impl Iterator, - ) -> Result, DeviceKeyAlgorithm>>> { + ) -> Result> { let olm = self.olm.lock().await; match &*olm { Some(o) => Ok(o.get_missing_sessions(users).await?), - None => Ok(BTreeMap::new()), + None => Ok(None), } } @@ -1334,25 +1332,6 @@ impl BaseClient { } } - /// Receive a successful keys claim response. - /// - /// # Arguments - /// - /// * `response` - The keys claim response of the request that the client - /// performed. - /// - /// # Panics - /// Panics if the client hasn't been logged in. - #[cfg(feature = "encryption")] - #[cfg_attr(feature = "docs", doc(cfg(encryption)))] - pub async fn receive_keys_claim_response(&self, response: &KeysClaimResponse) -> Result<()> { - let olm = self.olm.lock().await; - - let o = olm.as_ref().expect("Client isn't logged in."); - o.receive_keys_claim_response(response).await?; - Ok(()) - } - /// Invalidate the currently active outbound group session for the given /// room. /// diff --git a/matrix_sdk_crypto/src/lib.rs b/matrix_sdk_crypto/src/lib.rs index 5fcc422b..58d9c4e1 100644 --- a/matrix_sdk_crypto/src/lib.rs +++ b/matrix_sdk_crypto/src/lib.rs @@ -40,7 +40,7 @@ mod verification; pub use device::{Device, LocalTrust, ReadOnlyDevice, UserDevices}; pub use error::{MegolmError, OlmError}; -pub use machine::{OlmMachine, OneTimeKeys}; +pub use machine::OlmMachine; pub use memory_stores::{DeviceStore, GroupSessionStore, ReadOnlyUserDevices, SessionStore}; pub use olm::{ Account, EncryptionSettings, IdentityKeys, InboundGroupSession, OutboundGroupSession, Session, diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 21745032..79957a5b 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -19,6 +19,7 @@ use std::{ convert::{TryFrom, TryInto}, mem, sync::Arc, + time::Duration, }; use dashmap::DashMap; @@ -28,9 +29,9 @@ use tracing::{debug, error, info, instrument, trace, warn}; use matrix_sdk_common::{ api::r0::{ keys::{ - claim_keys, + claim_keys::{Request as KeysClaimRequest, Response as KeysClaimResponse}, get_keys::{IncomingRequest as KeysQueryRequest, Response as KeysQueryResponse}, - upload_keys, OneTimeKey, + upload_keys, }, sync::sync_events::Response as SyncResponse, to_device::{ @@ -45,9 +46,7 @@ use matrix_sdk_common::{ room_key_request::RoomKeyRequestEventContent, AnySyncRoomEvent, AnyToDeviceEvent, EventType, SyncMessageEvent, ToDeviceEvent, }, - identifiers::{ - DeviceId, DeviceKeyAlgorithm, DeviceKeyId, EventEncryptionAlgorithm, RoomId, UserId, - }, + identifiers::{DeviceId, DeviceKeyAlgorithm, EventEncryptionAlgorithm, RoomId, UserId}, uuid::Uuid, Raw, }; @@ -71,11 +70,6 @@ use super::{ CryptoStore, }; -/// A map from the algorithm and device id to a one-time key. -/// -/// These keys need to be periodically uploaded to the server. -pub type OneTimeKeys = BTreeMap; - /// State machine implementation of the Olm/Megolm encryption protocol used for /// Matrix end to end encryption. #[derive(Clone)] @@ -109,6 +103,7 @@ impl std::fmt::Debug for OlmMachine { impl OlmMachine { const MAX_TO_DEVICE_MESSAGES: usize = 20; + const KEY_CLAIM_TIMEOUT: Duration = Duration::from_secs(10); /// Create a new memory based OlmMachine. /// @@ -250,11 +245,14 @@ impl OlmMachine { response: impl Into>, ) -> OlmResult<()> { match response.into() { + IncomingResponse::KeysUpload(response) => { + self.receive_keys_upload_response(response).await?; + } IncomingResponse::KeysQuery(response) => { self.receive_keys_query_response(response).await?; } - IncomingResponse::KeysUpload(response) => { - self.receive_keys_upload_response(response).await?; + IncomingResponse::KeysClaim(response) => { + self.receive_keys_claim_response(response).await?; } IncomingResponse::ToDevice(_) => { self.mark_to_device_request_as_sent(&request_id.to_string()); @@ -344,12 +342,10 @@ impl OlmMachine { Ok(()) } - /// Get the user/device pairs for which no Olm session exists. + /// Get the a key claiming request for the user/device pairs that we are + /// missing Olm sessions for. /// - /// Returns a map from the user id, to a map from the device id to a key - /// algorithm. - /// - /// This can be used to make a key claiming request to the server. + /// Returns None if no key claiming request needs to be sent out. /// /// Sessions need to be established between devices so group sessions for a /// room can be shared with them. @@ -357,18 +353,18 @@ impl OlmMachine { /// This should be called every time a group session needs to be shared. /// /// The response of a successful key claiming requests needs to be passed to - /// the `OlmMachine` with the [`receive_keys_claim_response`]. + /// the `OlmMachine` with the [`mark_requests_as_sent`]. /// /// # Arguments /// /// `users` - The list of users that we should check if we lack a session /// with one of their devices. /// - /// [`receive_keys_claim_response`]: #method.receive_keys_claim_response + /// [`mark_requests_as_sent`]: #method.mark_requests_as_sent pub async fn get_missing_sessions( &self, users: impl Iterator, - ) -> OlmResult, DeviceKeyAlgorithm>>> { + ) -> OlmResult> { let mut missing = BTreeMap::new(); for user_id in users { @@ -403,7 +399,17 @@ impl OlmMachine { } } - Ok(missing) + if missing.is_empty() { + Ok(None) + } else { + Ok(Some(( + Uuid::new_v4(), + KeysClaimRequest { + timeout: Some(OlmMachine::KEY_CLAIM_TIMEOUT), + one_time_keys: missing, + }, + ))) + } } /// Receive a successful key claim response and create new Olm sessions with @@ -412,10 +418,7 @@ impl OlmMachine { /// # Arguments /// /// * `response` - The response containing the claimed one-time keys. - pub async fn receive_keys_claim_response( - &self, - response: &claim_keys::Response, - ) -> OlmResult<()> { + async fn receive_keys_claim_response(&self, response: &KeysClaimResponse) -> OlmResult<()> { // TODO log the failures here for (user_id, user_devices) in &response.one_time_keys { @@ -1523,7 +1526,6 @@ impl OlmMachine { pub(crate) mod test { static USER_ID: &str = "@bob:example.org"; - use matrix_sdk_common::js_int::uint; use std::{ collections::BTreeMap, convert::{TryFrom, TryInto}, @@ -1536,13 +1538,15 @@ pub(crate) mod test { use tempfile::tempdir; use crate::{ - machine::{OlmMachine, OneTimeKeys}, - verification::test::request_to_event, - verify_json, EncryptionSettings, ReadOnlyDevice, + machine::OlmMachine, verification::test::request_to_event, verify_json, EncryptionSettings, + ReadOnlyDevice, }; use matrix_sdk_common::{ - api::r0::{keys, to_device::send_event_to_device::IncomingRequest as OwnedToDeviceRequest}, + api::r0::{ + keys::{claim_keys, get_keys, upload_keys, OneTimeKey}, + to_device::send_event_to_device::IncomingRequest as OwnedToDeviceRequest, + }, events::{ room::{ encrypted::EncryptedEventContent, @@ -1558,6 +1562,11 @@ pub(crate) mod test { }; use matrix_sdk_test::test_json; + /// These keys need to be periodically uploaded to the server. + type OneTimeKeys = BTreeMap; + + use matrix_sdk_common::js_int::uint; + fn alice_id() -> UserId { user_id!("@alice:example.org") } @@ -1577,14 +1586,14 @@ pub(crate) mod test { .unwrap() } - fn keys_upload_response() -> keys::upload_keys::Response { + fn keys_upload_response() -> upload_keys::Response { let data = response_from_file(&test_json::KEYS_UPLOAD); - keys::upload_keys::Response::try_from(data).expect("Can't parse the keys upload response") + upload_keys::Response::try_from(data).expect("Can't parse the keys upload response") } - fn keys_query_response() -> keys::get_keys::Response { + fn keys_query_response() -> get_keys::Response { let data = response_from_file(&test_json::KEYS_QUERY); - keys::get_keys::Response::try_from(data).expect("Can't parse the keys upload response") + get_keys::Response::try_from(data).expect("Can't parse the keys upload response") } fn to_device_requests_to_content(requests: Vec) -> EncryptedEventContent { @@ -1662,7 +1671,7 @@ pub(crate) mod test { let mut one_time_keys = BTreeMap::new(); one_time_keys.insert(bob.user_id.clone(), bob_keys); - let response = keys::claim_keys::Response { + let response = claim_keys::Response { failures: BTreeMap::new(), one_time_keys, }; @@ -1906,13 +1915,14 @@ pub(crate) mod test { let alice = alice_id(); let alice_device = alice_device_id(); - let missing_sessions = machine + let (_, missing_sessions) = machine .get_missing_sessions([alice.clone()].iter()) .await + .unwrap() .unwrap(); - assert!(missing_sessions.contains_key(&alice)); - let user_sessions = missing_sessions.get(&alice).unwrap(); + assert!(missing_sessions.one_time_keys.contains_key(&alice)); + let user_sessions = missing_sessions.one_time_keys.get(&alice).unwrap(); assert!(user_sessions.contains_key(&alice_device)); } @@ -1930,7 +1940,7 @@ pub(crate) mod test { let mut one_time_keys = BTreeMap::new(); one_time_keys.insert(bob_machine.user_id.clone(), bob_keys); - let response = keys::claim_keys::Response { + let response = claim_keys::Response { failures: BTreeMap::new(), one_time_keys, }; diff --git a/matrix_sdk_crypto/src/requests.rs b/matrix_sdk_crypto/src/requests.rs index 18b6e12d..07816601 100644 --- a/matrix_sdk_crypto/src/requests.rs +++ b/matrix_sdk_crypto/src/requests.rs @@ -15,6 +15,7 @@ use matrix_sdk_common::{ api::r0::{ keys::{ + claim_keys::Response as KeysClaimResponse, get_keys::{IncomingRequest as KeysQueryRequest, Response as KeysQueryResponse}, upload_keys::{Request as KeysUploadRequest, Response as KeysUploadResponse}, }, @@ -57,6 +58,8 @@ pub enum IncomingResponse<'a> { KeysQuery(&'a KeysQueryResponse), /// TODO ToDevice(&'a ToDeviceResponse), + /// + KeysClaim(&'a KeysClaimResponse), } impl<'a> From<&'a KeysUploadResponse> for IncomingResponse<'a> { @@ -77,6 +80,12 @@ impl<'a> From<&'a ToDeviceResponse> for IncomingResponse<'a> { } } +impl<'a> From<&'a KeysClaimResponse> for IncomingResponse<'a> { + fn from(response: &'a KeysClaimResponse) -> Self { + IncomingResponse::KeysClaim(response) + } +} + /// TODO #[derive(Debug)] pub struct OutgoingRequest { From 002531349edcd4b8ed448f89ebb2fa8a09afbf9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 21 Aug 2020 15:06:54 +0200 Subject: [PATCH 45/49] crypto: Decluter the main doc page a bit. --- matrix_sdk_crypto/src/lib.rs | 11 +++++------ matrix_sdk_crypto/src/memory_stores.rs | 5 +++++ matrix_sdk_crypto/src/olm/mod.rs | 13 +++++++++---- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/matrix_sdk_crypto/src/lib.rs b/matrix_sdk_crypto/src/lib.rs index 58d9c4e1..7d31aceb 100644 --- a/matrix_sdk_crypto/src/lib.rs +++ b/matrix_sdk_crypto/src/lib.rs @@ -30,8 +30,8 @@ mod device; mod error; mod machine; -mod memory_stores; -mod olm; +pub mod memory_stores; +pub mod olm; mod requests; mod store; #[allow(dead_code)] @@ -41,10 +41,9 @@ mod verification; pub use device::{Device, LocalTrust, ReadOnlyDevice, UserDevices}; pub use error::{MegolmError, OlmError}; pub use machine::OlmMachine; -pub use memory_stores::{DeviceStore, GroupSessionStore, ReadOnlyUserDevices, SessionStore}; -pub use olm::{ - Account, EncryptionSettings, IdentityKeys, InboundGroupSession, OutboundGroupSession, Session, -}; +use memory_stores::ReadOnlyUserDevices; +pub use olm::EncryptionSettings; +pub(crate) use olm::{Account, IdentityKeys, InboundGroupSession, Session}; pub use requests::{IncomingResponse, OutgoingRequest, OutgoingRequests}; #[cfg(feature = "sqlite_cryptostore")] pub use store::sqlite::SqliteStore; diff --git a/matrix_sdk_crypto/src/memory_stores.rs b/matrix_sdk_crypto/src/memory_stores.rs index 307d339b..5fcf71f3 100644 --- a/matrix_sdk_crypto/src/memory_stores.rs +++ b/matrix_sdk_crypto/src/memory_stores.rs @@ -12,6 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! Collection of small in-memory stores that can be used to cache Olm objects. +//! +//! Note: You'll only be interested in these if you are implementing a custom +//! `CryptoStore`. + use std::{collections::HashMap, sync::Arc}; use dashmap::{DashMap, ReadOnlyView}; diff --git a/matrix_sdk_crypto/src/olm/mod.rs b/matrix_sdk_crypto/src/olm/mod.rs index af1416a4..af597d95 100644 --- a/matrix_sdk_crypto/src/olm/mod.rs +++ b/matrix_sdk_crypto/src/olm/mod.rs @@ -12,15 +12,20 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! The crypto specific Olm objects. +//! +//! Note: You'll only be interested in these if you are implementing a custom +//! `CryptoStore`. + mod account; mod group_sessions; mod session; pub use account::{Account, IdentityKeys}; -pub use group_sessions::{ - EncryptionSettings, GroupSessionKey, InboundGroupSession, OutboundGroupSession, -}; -pub use session::{OlmMessage, Session}; +pub use group_sessions::{EncryptionSettings, InboundGroupSession}; +pub(crate) use group_sessions::{GroupSessionKey, OutboundGroupSession}; +pub(crate) use session::OlmMessage; +pub use session::Session; #[cfg(test)] pub(crate) mod test { From de90da4adcdeb6e4079e65b2f141f74d12649c64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 21 Aug 2020 16:26:34 +0200 Subject: [PATCH 46/49] crypto: Make the verification machine compatible with how we queue up requests. --- matrix_sdk/src/client.rs | 24 ++++---- matrix_sdk/src/sas.rs | 2 +- matrix_sdk_base/src/client.rs | 21 ------- matrix_sdk_crypto/src/machine.rs | 21 ++++--- matrix_sdk_crypto/src/olm/group_sessions.rs | 10 +++- matrix_sdk_crypto/src/requests.rs | 26 ++++++++- matrix_sdk_crypto/src/verification/machine.rs | 55 +++++++++++++------ matrix_sdk_crypto/src/verification/mod.rs | 11 ++++ .../src/verification/sas/helpers.rs | 17 ++++-- matrix_sdk_crypto/src/verification/sas/mod.rs | 13 +++-- 10 files changed, 121 insertions(+), 79 deletions(-) diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index a13fddc2..d44ab146 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -1109,11 +1109,11 @@ impl Client { } #[cfg(feature = "encryption")] - async fn send_to_device(&self, request: OwnedToDeviceRequest) -> Result { + async fn send_to_device(&self, request: &OwnedToDeviceRequest) -> Result { let request = ToDeviceRequest { - event_type: request.event_type, + event_type: request.event_type.clone(), txn_id: &request.txn_id, - messages: request.messages, + messages: request.messages.clone(), }; self.send(request).await @@ -1232,22 +1232,22 @@ impl Client { #[cfg(feature = "encryption")] { for r in self.base_client.outgoing_requests().await { - match r.request { + match r.request() { OutgoingRequests::KeysQuery(request) => { - if let Err(e) = self.keys_query(&r.request_id, request).await { + if let Err(e) = self.keys_query(r.request_id(), request).await { warn!("Error while querying device keys {:?}", e); } } OutgoingRequests::KeysUpload(request) => { - if let Err(e) = self.keys_upload(&r.request_id, request).await { + if let Err(e) = self.keys_upload(&r.request_id(), request).await { warn!("Error while querying device keys {:?}", e); } } OutgoingRequests::ToDeviceRequest(request) => { if let Ok(resp) = self.send_to_device(request).await { self.base_client - .mark_request_as_sent(&r.request_id, &resp) + .mark_request_as_sent(&r.request_id(), &resp) .await .unwrap(); } @@ -1328,7 +1328,7 @@ impl Client { .expect("Keys don't need to be uploaded"); for request in requests.drain(..) { - self.send_to_device(request).await?; + self.send_to_device(&request).await?; } Ok(()) @@ -1349,7 +1349,7 @@ impl Client { async fn keys_upload( &self, request_id: &Uuid, - request: upload_keys::Request, + request: &upload_keys::Request, ) -> Result { debug!( "Uploading encryption keys device keys: {}, one-time-keys: {}", @@ -1357,7 +1357,7 @@ impl Client { request.one_time_keys.as_ref().map_or(0, |k| k.len()) ); - let response = self.send(request).await?; + let response = self.send(request.clone()).await?; self.base_client .mark_request_as_sent(request_id, &response) .await?; @@ -1382,11 +1382,11 @@ impl Client { async fn keys_query( &self, request_id: &Uuid, - request: get_keys::IncomingRequest, + request: &get_keys::IncomingRequest, ) -> Result { let request = get_keys::Request { timeout: None, - device_keys: request.device_keys, + device_keys: request.device_keys.clone(), token: None, }; diff --git a/matrix_sdk/src/sas.rs b/matrix_sdk/src/sas.rs index 3092c5aa..cadaddd1 100644 --- a/matrix_sdk/src/sas.rs +++ b/matrix_sdk/src/sas.rs @@ -56,7 +56,7 @@ impl Sas { /// Cancel the interactive verification flow. pub async fn cancel(&self) -> Result<()> { - if let Some(req) = self.inner.cancel() { + if let Some((_, req)) = self.inner.cancel() { let request = ToDeviceRequest { event_type: req.event_type, txn_id: &req.txn_id, diff --git a/matrix_sdk_base/src/client.rs b/matrix_sdk_base/src/client.rs index d5554002..d057bd78 100644 --- a/matrix_sdk_base/src/client.rs +++ b/matrix_sdk_base/src/client.rs @@ -1748,27 +1748,6 @@ impl BaseClient { } } - /// Get the to-device requests that need to be sent out. - #[cfg(feature = "encryption")] - #[cfg_attr(feature = "docs", doc(cfg(encryption)))] - pub async fn outgoing_to_device_requests(&self) -> Vec { - self.olm - .lock() - .await - .as_ref() - .map(|o| o.outgoing_to_device_requests()) - .unwrap_or_default() - } - - /// Mark an outgoing to-device requests as sent. - #[cfg(feature = "encryption")] - #[cfg_attr(feature = "docs", doc(cfg(encryption)))] - pub async fn mark_to_device_request_as_sent(&self, request_id: &str) { - if let Some(olm) = self.olm.lock().await.as_ref() { - olm.mark_to_device_request_as_sent(request_id) - } - } - /// Get a `Sas` verification object with the given flow id. /// /// # Arguments diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 79957a5b..b9755a03 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -223,18 +223,20 @@ impl OlmMachine { if let Some(r) = self.keys_for_upload().await.map(|r| OutgoingRequest { request_id: Uuid::new_v4(), - request: r.into(), + request: Arc::new(r.into()), }) { requests.push(r); } if let Some(r) = self.users_for_key_query().await.map(|r| OutgoingRequest { request_id: Uuid::new_v4(), - request: r.into(), + request: Arc::new(r.into()), }) { requests.push(r); } + requests.append(&mut self.outgoing_to_device_requests()); + requests } @@ -255,7 +257,7 @@ impl OlmMachine { self.receive_keys_claim_response(response).await?; } IncomingResponse::ToDevice(_) => { - self.mark_to_device_request_as_sent(&request_id.to_string()); + self.mark_to_device_request_as_sent(&request_id); } }; @@ -1234,12 +1236,12 @@ impl OlmMachine { } /// Get the to-device requests that need to be sent out. - pub fn outgoing_to_device_requests(&self) -> Vec { + fn outgoing_to_device_requests(&self) -> Vec { self.verification_machine.outgoing_to_device_requests() } /// Mark an outgoing to-device requests as sent. - pub fn mark_to_device_request_as_sent(&self, request_id: &str) { + fn mark_to_device_request_as_sent(&self, request_id: &Uuid) { self.verification_machine.mark_requests_as_sent(request_id); } @@ -1538,8 +1540,9 @@ pub(crate) mod test { use tempfile::tempdir; use crate::{ - machine::OlmMachine, verification::test::request_to_event, verify_json, EncryptionSettings, - ReadOnlyDevice, + machine::OlmMachine, + verification::test::{outgoing_request_to_event, request_to_event}, + verify_json, EncryptionSettings, ReadOnlyDevice, }; use matrix_sdk_common::{ @@ -2169,7 +2172,7 @@ pub(crate) mod test { .outgoing_to_device_requests() .iter() .next() - .map(|r| request_to_event(alice.user_id(), &r)) + .map(|r| outgoing_request_to_event(alice.user_id(), r)) .unwrap(); bob.handle_verification_event(&mut event).await; @@ -2177,7 +2180,7 @@ pub(crate) mod test { .outgoing_to_device_requests() .iter() .next() - .map(|r| request_to_event(bob.user_id(), &r)) + .map(|r| outgoing_request_to_event(bob.user_id(), r)) .unwrap(); alice.handle_verification_event(&mut event).await; diff --git a/matrix_sdk_crypto/src/olm/group_sessions.rs b/matrix_sdk_crypto/src/olm/group_sessions.rs index 40949632..a677380c 100644 --- a/matrix_sdk_crypto/src/olm/group_sessions.rs +++ b/matrix_sdk_crypto/src/olm/group_sessions.rs @@ -476,7 +476,10 @@ impl std::fmt::Debug for OutboundGroupSession { #[cfg(test)] mod test { - use std::{thread::sleep, time::Duration}; + use std::{ + sync::Arc, + time::{Duration, Instant}, + }; use matrix_sdk_common::{ events::room::message::{MessageEventContent, TextMessageEventContent}, @@ -487,6 +490,7 @@ mod test { use crate::Account; #[tokio::test] + #[cfg(not(target_os = "macos"))] async fn expiration() { let settings = EncryptionSettings { rotation_period_msgs: 1, @@ -512,13 +516,13 @@ mod test { ..Default::default() }; - let (session, _) = account + let (mut session, _) = account .create_group_session_pair(&room_id!("!test_room:example.org"), settings) .await .unwrap(); assert!(!session.expired()); - sleep(Duration::from_millis(110)); + session.creation_time = Arc::new(Instant::now() - Duration::from_secs(60 * 60)); assert!(session.expired()); } } diff --git a/matrix_sdk_crypto/src/requests.rs b/matrix_sdk_crypto/src/requests.rs index 07816601..0b102076 100644 --- a/matrix_sdk_crypto/src/requests.rs +++ b/matrix_sdk_crypto/src/requests.rs @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::sync::Arc; + use matrix_sdk_common::{ api::r0::{ keys::{ @@ -49,6 +51,12 @@ impl From for OutgoingRequests { } } +impl From for OutgoingRequests { + fn from(request: ToDeviceRequest) -> Self { + OutgoingRequests::ToDeviceRequest(request) + } +} + /// TODO #[derive(Debug)] pub enum IncomingResponse<'a> { @@ -87,11 +95,23 @@ impl<'a> From<&'a KeysClaimResponse> for IncomingResponse<'a> { } /// TODO -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct OutgoingRequest { /// The unique id of a request, needs to be passed when receiving a /// response. - pub request_id: Uuid, + pub(crate) request_id: Uuid, /// TODO - pub request: OutgoingRequests, + pub(crate) request: Arc, +} + +impl OutgoingRequest { + /// Get the unique id of this request. + pub fn request_id(&self) -> &Uuid { + &self.request_id + } + + /// Get the underlying outgoing request. + pub fn request(&self) -> &OutgoingRequests { + &self.request + } } diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index a2699127..3590dced 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -22,17 +22,18 @@ use matrix_sdk_common::{ api::r0::to_device::send_event_to_device::IncomingRequest as OwnedToDeviceRequest, events::{AnyToDeviceEvent, AnyToDeviceEventContent}, identifiers::{DeviceId, UserId}, + uuid::Uuid, }; use super::sas::{content_to_request, Sas}; -use crate::{Account, CryptoStore, CryptoStoreError, ReadOnlyDevice}; +use crate::{requests::OutgoingRequest, Account, CryptoStore, CryptoStoreError, ReadOnlyDevice}; #[derive(Clone, Debug)] pub struct VerificationMachine { account: Account, pub(crate) store: Arc>, verifications: Arc>, - outgoing_to_device_messages: Arc>, + outgoing_to_device_messages: Arc>, } impl VerificationMachine { @@ -58,7 +59,7 @@ impl VerificationMachine { identity, ); - let request = content_to_request( + let (_, request) = content_to_request( device.user_id(), device.device_id(), AnyToDeviceEventContent::KeyVerificationStart(content), @@ -81,10 +82,14 @@ impl VerificationMachine { recipient_device: &DeviceId, content: AnyToDeviceEventContent, ) { - let request = content_to_request(recipient, recipient_device, content); + let (request_id, request) = content_to_request(recipient, recipient_device, content); - self.outgoing_to_device_messages - .insert(request.txn_id.clone(), request); + let request = OutgoingRequest { + request_id: request_id.clone(), + request: Arc::new(request.into()), + }; + + self.outgoing_to_device_messages.insert(request_id, request); } fn receive_event_helper(&self, sas: &Sas, event: &mut AnyToDeviceEvent) { @@ -93,19 +98,15 @@ impl VerificationMachine { } } - pub fn mark_requests_as_sent(&self, uuid: &str) { + pub fn mark_requests_as_sent(&self, uuid: &Uuid) { self.outgoing_to_device_messages.remove(uuid); } - pub fn outgoing_to_device_requests(&self) -> Vec { + pub fn outgoing_to_device_requests(&self) -> Vec { #[allow(clippy::map_clone)] self.outgoing_to_device_messages .iter() - .map(|r| OwnedToDeviceRequest { - event_type: r.event_type.clone(), - txn_id: r.txn_id.clone(), - messages: r.messages.clone(), - }) + .map(|r| (*r).clone()) .collect() } @@ -115,7 +116,13 @@ impl VerificationMachine { for sas in self.verifications.iter() { if let Some(r) = sas.cancel_if_timed_out() { - self.outgoing_to_device_messages.insert(r.txn_id.clone(), r); + self.outgoing_to_device_messages.insert( + r.0.clone(), + OutgoingRequest { + request_id: r.0, + request: Arc::new(r.1.into()), + }, + ); } } } @@ -184,7 +191,13 @@ impl VerificationMachine { if s.is_done() && !s.mark_device_as_verified().await? { if let Some(r) = s.cancel() { - self.outgoing_to_device_messages.insert(r.txn_id.clone(), r); + self.outgoing_to_device_messages.insert( + r.0.clone(), + OutgoingRequest { + request_id: r.0, + request: Arc::new(r.1.into()), + }, + ); } } }; @@ -211,6 +224,7 @@ mod test { use super::{Sas, VerificationMachine}; use crate::{ + requests::OutgoingRequests, store::memorystore::MemoryStore, verification::test::{get_content_from_request, wrap_any_to_device_content}, Account, CryptoStore, ReadOnlyDevice, @@ -293,10 +307,15 @@ mod test { .next() .unwrap(); - let txn_id = request.txn_id.clone(); + let txn_id = request.request_id().clone(); - let mut event = - wrap_any_to_device_content(alice.user_id(), get_content_from_request(&request)); + let r = if let OutgoingRequests::ToDeviceRequest(r) = request.request() { + r + } else { + panic!("Invalid request type"); + }; + + let mut event = wrap_any_to_device_content(alice.user_id(), get_content_from_request(r)); drop(request); alice_machine.mark_requests_as_sent(&txn_id); diff --git a/matrix_sdk_crypto/src/verification/mod.rs b/matrix_sdk_crypto/src/verification/mod.rs index 9e503f4e..ab79c0f7 100644 --- a/matrix_sdk_crypto/src/verification/mod.rs +++ b/matrix_sdk_crypto/src/verification/mod.rs @@ -20,6 +20,7 @@ pub use sas::Sas; #[cfg(test)] pub(crate) mod test { + use crate::requests::{OutgoingRequest, OutgoingRequests}; use serde_json::Value; use matrix_sdk_common::{ @@ -36,6 +37,16 @@ pub(crate) mod test { wrap_any_to_device_content(sender, content) } + pub(crate) fn outgoing_request_to_event( + sender: &UserId, + request: &OutgoingRequest, + ) -> AnyToDeviceEvent { + match request.request() { + OutgoingRequests::ToDeviceRequest(r) => request_to_event(sender, r), + _ => panic!("Unsupported outgoing request"), + } + } + pub(crate) fn wrap_any_to_device_content( sender: &UserId, content: AnyToDeviceEventContent, diff --git a/matrix_sdk_crypto/src/verification/sas/helpers.rs b/matrix_sdk_crypto/src/verification/sas/helpers.rs index 1c66fcd4..259248e1 100644 --- a/matrix_sdk_crypto/src/verification/sas/helpers.rs +++ b/matrix_sdk_crypto/src/verification/sas/helpers.rs @@ -464,7 +464,7 @@ pub fn content_to_request( recipient: &UserId, recipient_device: &DeviceId, content: AnyToDeviceEventContent, -) -> OwnedToDeviceRequest { +) -> (Uuid, OwnedToDeviceRequest) { let mut messages = BTreeMap::new(); let mut user_messages = BTreeMap::new(); @@ -483,11 +483,16 @@ pub fn content_to_request( _ => unreachable!(), }; - OwnedToDeviceRequest { - txn_id: Uuid::new_v4().to_string(), - event_type, - messages, - } + let request_id = Uuid::new_v4(); + + ( + request_id, + OwnedToDeviceRequest { + txn_id: request_id.to_string(), + event_type, + messages, + }, + ) } #[cfg(test)] diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index 9bf9d153..3e4db5e3 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -31,6 +31,7 @@ use matrix_sdk_common::{ AnyToDeviceEvent, AnyToDeviceEventContent, ToDeviceEvent, }, identifiers::{DeviceId, UserId}, + uuid::Uuid, }; use crate::{ @@ -168,7 +169,7 @@ impl Sas { pub fn accept(&self) -> Option { self.inner.lock().unwrap().accept().map(|c| { let content = AnyToDeviceEventContent::KeyVerificationAccept(c); - self.content_to_request(content) + self.content_to_request(content).1 }) } @@ -194,7 +195,7 @@ impl Sas { // 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()); + return Ok(self.cancel().map(|r| r.1)); } else { self.mark_identity_as_verified().await?; } @@ -202,7 +203,7 @@ impl Sas { Ok(content.map(|c| { let content = AnyToDeviceEventContent::KeyVerificationMac(c); - self.content_to_request(content) + self.content_to_request(content).1 })) } @@ -327,7 +328,7 @@ impl Sas { /// /// Returns None if the `Sas` object is already in a canceled state, /// otherwise it returns a request that needs to be sent out. - pub fn cancel(&self) -> Option { + pub fn cancel(&self) -> Option<(Uuid, OwnedToDeviceRequest)> { let mut guard = self.inner.lock().unwrap(); let sas: InnerSas = (*guard).clone(); let (sas, content) = sas.cancel(CancelCode::User); @@ -336,7 +337,7 @@ impl Sas { content.map(|c| self.content_to_request(c)) } - pub(crate) fn cancel_if_timed_out(&self) -> Option { + pub(crate) fn cancel_if_timed_out(&self) -> Option<(Uuid, OwnedToDeviceRequest)> { if self.is_canceled() || self.is_done() { None } else if self.timed_out() { @@ -410,7 +411,7 @@ impl Sas { pub(crate) fn content_to_request( &self, content: AnyToDeviceEventContent, - ) -> OwnedToDeviceRequest { + ) -> (Uuid, OwnedToDeviceRequest) { content_to_request(self.other_user_id(), self.other_device_id(), content) } } From c3c642871792d0d6d7bc76ad03cc83e38e844674 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 21 Aug 2020 16:31:02 +0200 Subject: [PATCH 47/49] crypto: Remove some clippy warnings. --- matrix_sdk_crypto/src/verification/machine.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index 3590dced..d71e85d8 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -85,7 +85,7 @@ impl VerificationMachine { let (request_id, request) = content_to_request(recipient, recipient_device, content); let request = OutgoingRequest { - request_id: request_id.clone(), + request_id, request: Arc::new(request.into()), }; @@ -117,7 +117,7 @@ impl VerificationMachine { for sas in self.verifications.iter() { if let Some(r) = sas.cancel_if_timed_out() { self.outgoing_to_device_messages.insert( - r.0.clone(), + r.0, OutgoingRequest { request_id: r.0, request: Arc::new(r.1.into()), @@ -192,7 +192,7 @@ impl VerificationMachine { if s.is_done() && !s.mark_device_as_verified().await? { if let Some(r) = s.cancel() { self.outgoing_to_device_messages.insert( - r.0.clone(), + r.0, OutgoingRequest { request_id: r.0, request: Arc::new(r.1.into()), From b3941ca2543161fb48bc68362865cededc83e4c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 21 Aug 2020 16:39:15 +0200 Subject: [PATCH 48/49] crypto: Verify user identities when we're the first one to confirm as well. --- matrix_sdk_crypto/src/verification/machine.rs | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index d71e85d8..e69d5e21 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -189,15 +189,19 @@ impl VerificationMachine { if let Some(s) = self.get_sas(&e.content.transaction_id) { self.receive_event_helper(&s, event); - if s.is_done() && !s.mark_device_as_verified().await? { - if let Some(r) = s.cancel() { - self.outgoing_to_device_messages.insert( - r.0, - OutgoingRequest { - request_id: r.0, - request: Arc::new(r.1.into()), - }, - ); + if s.is_done() { + if !s.mark_device_as_verified().await? { + if let Some(r) = s.cancel() { + self.outgoing_to_device_messages.insert( + r.0, + OutgoingRequest { + request_id: r.0, + request: Arc::new(r.1.into()), + }, + ); + } + } else { + s.mark_identity_as_verified().await?; } } }; From edea5e1c51ccd6210942719689bda2011a0c3982 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 21 Aug 2020 16:46:28 +0200 Subject: [PATCH 49/49] crypto: Fix a clippy warning. --- matrix_sdk_crypto/src/verification/machine.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index e69d5e21..f6aebff8 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -311,7 +311,7 @@ mod test { .next() .unwrap(); - let txn_id = request.request_id().clone(); + let txn_id = *request.request_id(); let r = if let OutgoingRequests::ToDeviceRequest(r) = request.request() { r