From 1d8f01ef11c6fb885a42e7f69ae1826a06cba119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 1 Oct 2020 12:15:13 +0200 Subject: [PATCH] crypto: Remove the third Device variant. --- matrix_sdk_crypto/src/group_manager.rs | 38 ++---- matrix_sdk_crypto/src/identities/device.rs | 34 ++++- matrix_sdk_crypto/src/identities/manager.rs | 21 ++- matrix_sdk_crypto/src/key_request.rs | 134 ++++++-------------- matrix_sdk_crypto/src/machine.rs | 32 +---- matrix_sdk_crypto/src/store/mod.rs | 95 +++++++++----- 6 files changed, 162 insertions(+), 192 deletions(-) diff --git a/matrix_sdk_crypto/src/group_manager.rs b/matrix_sdk_crypto/src/group_manager.rs index 1c258d46..7336a0d0 100644 --- a/matrix_sdk_crypto/src/group_manager.rs +++ b/matrix_sdk_crypto/src/group_manager.rs @@ -24,10 +24,9 @@ use matrix_sdk_common::{ use crate::{ error::{EventError, MegolmResult, OlmResult}, - key_request::Device, olm::{Account, OutboundGroupSession}, - store::{Result as StoreResult, Store}, - EncryptionSettings, OlmError, ToDeviceRequest, + store::Store, + Device, EncryptionSettings, OlmError, ToDeviceRequest, }; #[derive(Clone)] @@ -54,28 +53,6 @@ impl GroupSessionManager { } } - 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.account.user_id()) - .await? - .map(|i| i.own().cloned()) - .flatten(); - let device_owner_identity = self.store.get_user_identity(user_id).await.ok().flatten(); - - Ok(devices - .devices() - .map(|d| Device { - inner: d.clone(), - store: self.store.clone(), - own_identity: own_identity.clone(), - device_owner_identity: device_owner_identity.clone(), - }) - .collect()) - } - pub fn invalidate_group_session(&self, room_id: &RoomId) -> bool { self.outbound_group_sessions.remove(room_id).is_some() } @@ -178,11 +155,16 @@ impl GroupSessionManager { // caller mark them as sent using an UUID. session.mark_as_shared(); - let mut devices = Vec::new(); + let mut devices: Vec = Vec::new(); for user_id in session.users_to_share_with() { - let mut user_devices = self.get_user_devices(&user_id).await?; - devices.extend(user_devices.drain(..).filter(|d| !d.is_blacklisted())) + let user_devices = self.store.get_user_devices(&user_id).await?; + devices.extend( + user_devices + .devices() + .map(|d| d.clone()) + .filter(|d| !d.is_blacklisted()), + ) } let mut requests = Vec::new(); diff --git a/matrix_sdk_crypto/src/identities/device.rs b/matrix_sdk_crypto/src/identities/device.rs index 71b36418..8ab5f2ca 100644 --- a/matrix_sdk_crypto/src/identities/device.rs +++ b/matrix_sdk_crypto/src/identities/device.rs @@ -14,7 +14,7 @@ use std::{ collections::BTreeMap, - convert::TryFrom, + convert::{TryFrom, TryInto}, ops::Deref, sync::{ atomic::{AtomicBool, Ordering}, @@ -26,13 +26,17 @@ use atomic::Atomic; use matrix_sdk_common::{ api::r0::keys::SignedKey, encryption::DeviceKeys, - events::{room::encrypted::EncryptedEventContent, EventType}, + events::{ + forwarded_room_key::ForwardedRoomKeyEventContent, room::encrypted::EncryptedEventContent, + EventType, + }, identifiers::{DeviceId, DeviceKeyAlgorithm, DeviceKeyId, EventEncryptionAlgorithm, UserId}, }; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; use tracing::warn; +use crate::olm::InboundGroupSession; #[cfg(test)] use crate::{OlmMachine, ReadOnlyAccount}; @@ -115,7 +119,6 @@ impl Device { /// * `event_type` - The type of the event. /// /// * `content` - The content of the event that should be encrypted. - #[cfg(test)] pub(crate) async fn encrypt( &self, event_type: EventType, @@ -125,6 +128,31 @@ impl Device { .encrypt(&**self.verification_machine.store, event_type, content) .await } + + /// Encrypt the given inbound group session as a forwarded room key for this + /// device. + pub async fn encrypt_session( + &self, + session: InboundGroupSession, + ) -> OlmResult { + let export = session.export().await; + + let content: ForwardedRoomKeyEventContent = if let Ok(c) = export.try_into() { + c + } else { + // TODO remove this panic. + panic!( + "Can't share session {} with device {} {}, key export can't \ + be converted to a forwarded room key content", + session.session_id(), + self.user_id(), + self.device_id() + ); + }; + + let content = serde_json::to_value(content)?; + self.encrypt(EventType::ForwardedRoomKey, content).await + } } /// A read only view over all devices belonging to a user. diff --git a/matrix_sdk_crypto/src/identities/manager.rs b/matrix_sdk_crypto/src/identities/manager.rs index 572ed522..509c3269 100644 --- a/matrix_sdk_crypto/src/identities/manager.rs +++ b/matrix_sdk_crypto/src/identities/manager.rs @@ -126,7 +126,7 @@ impl IdentityManager { continue; } - let device = self.store.get_device(&user_id, device_id).await?; + let device = self.store.get_readonly_device(&user_id, device_id).await?; let device = if let Some(mut device) = device { if let Err(e) = device.update_device(device_keys) { @@ -157,7 +157,7 @@ impl IdentityManager { let current_devices: HashSet<&DeviceId> = device_map.keys().map(|id| id.as_ref()).collect(); - let stored_devices = self.store.get_user_devices(&user_id).await.unwrap(); + let stored_devices = self.store.get_readonly_devices(&user_id).await?; let stored_devices_set: HashSet<&DeviceId> = stored_devices.keys().collect(); let deleted_devices = stored_devices_set.difference(¤t_devices); @@ -364,7 +364,9 @@ pub(crate) mod test { use crate::{ identities::IdentityManager, machine::test::response_from_file, - store::{MemoryStore, Store}, + olm::ReadOnlyAccount, + store::{CryptoStore, MemoryStore, Store}, + verification::VerificationMachine, }; fn user_id() -> UserId { @@ -381,7 +383,14 @@ pub(crate) mod test { fn manager() -> IdentityManager { let user_id = Arc::new(user_id()); - let store = Store::new(user_id.clone(), Arc::new(Box::new(MemoryStore::new()))); + let account = ReadOnlyAccount::new(&user_id, &device_id()); + let store: Arc> = Arc::new(Box::new(MemoryStore::new())); + let verification = VerificationMachine::new(account, store); + let store = Store::new( + user_id.clone(), + Arc::new(Box::new(MemoryStore::new())), + verification, + ); IdentityManager::new(user_id, Arc::new(device_id()), store) } @@ -566,7 +575,7 @@ pub(crate) mod test { let device = manager .store - .get_device(&other_user, "SKISMLNIMH".into()) + .get_readonly_device(&other_user, "SKISMLNIMH".into()) .await .unwrap() .unwrap(); @@ -598,7 +607,7 @@ pub(crate) mod test { let device = manager .store - .get_device(&other_user, "SKISMLNIMH".into()) + .get_readonly_device(&other_user, "SKISMLNIMH".into()) .await .unwrap() .unwrap(); diff --git a/matrix_sdk_crypto/src/key_request.rs b/matrix_sdk_crypto/src/key_request.rs index d45c9c16..831ce0e0 100644 --- a/matrix_sdk_crypto/src/key_request.rs +++ b/matrix_sdk_crypto/src/key_request.rs @@ -20,12 +20,10 @@ // If we don't trust the device store an object that remembers the request and // let the users introspect that object. -#![allow(dead_code)] - use dashmap::DashMap; use serde::{Deserialize, Serialize}; -use serde_json::{value::to_raw_value, Value}; -use std::{collections::BTreeMap, convert::TryInto, ops::Deref, sync::Arc}; +use serde_json::value::to_raw_value; +use std::{collections::BTreeMap, sync::Arc}; use thiserror::Error; use tracing::{error, info, instrument, trace, warn}; @@ -33,7 +31,6 @@ use matrix_sdk_common::{ api::r0::to_device::DeviceIdOrAllDevices, events::{ forwarded_room_key::ForwardedRoomKeyEventContent, - room::encrypted::EncryptedEventContent, room_key_request::{Action, RequestedKeyInfo, RoomKeyRequestEventContent}, AnyToDeviceEvent, EventType, ToDeviceEvent, }, @@ -44,67 +41,12 @@ use matrix_sdk_common::{ use crate::{ error::OlmResult, - identities::{OwnUserIdentity, ReadOnlyDevice, UserIdentities}, olm::{InboundGroupSession, OutboundGroupSession}, requests::{OutgoingRequest, ToDeviceRequest}, store::{CryptoStoreError, Store}, + Device, }; -pub struct Device { - pub(crate) inner: ReadOnlyDevice, - pub(crate) store: Store, - pub(crate) own_identity: Option, - pub(crate) device_owner_identity: Option, -} - -impl Device { - /// Encrypt the given inbound group session as a forwarded room key for this - /// device. - pub async fn encrypt_session( - &self, - session: InboundGroupSession, - ) -> OlmResult { - let export = session.export().await; - - let content: ForwardedRoomKeyEventContent = if let Ok(c) = export.try_into() { - c - } else { - // TODO remove this panic. - panic!( - "Can't share session {} with device {} {}, key export can't \ - be converted to a forwarded room key content", - session.session_id(), - self.user_id(), - self.device_id() - ); - }; - - let content = serde_json::to_value(content)?; - self.encrypt(EventType::ForwardedRoomKey, content).await - } - - fn trust_state(&self) -> bool { - self.inner - .trust_state(&self.own_identity, &self.device_owner_identity) - } - - pub async fn encrypt( - &self, - event_type: EventType, - content: Value, - ) -> OlmResult { - self.inner.encrypt(&*self.store, event_type, content).await - } -} - -impl Deref for Device { - type Target = ReadOnlyDevice; - - fn deref(&self) -> &Self::Target { - &self.inner - } -} - /// An error describing why a key share request won't be honored. #[derive(Debug, Clone, Error, PartialEq)] pub enum KeyshareDecision { @@ -282,14 +224,8 @@ impl KeyRequestMachine { let device = self .store - .get_device_and_users(&event.sender, &event.content.requesting_device_id) - .await? - .map(|(d, o, u)| Device { - inner: d, - store: self.store.clone(), - own_identity: o, - device_owner_identity: u, - }); + .get_device(&event.sender, &event.content.requesting_device_id) + .await?; if let Some(device) = device { if let Err(e) = self.should_share_session( @@ -614,10 +550,11 @@ mod test { use crate::{ identities::{LocalTrust, ReadOnlyDevice}, olm::{Account, ReadOnlyAccount}, - store::{MemoryStore, Store}, + store::{CryptoStore, MemoryStore, Store}, + verification::VerificationMachine, }; - use super::{Device, KeyRequestMachine, KeyshareDecision}; + use super::{KeyRequestMachine, KeyshareDecision}; fn alice_id() -> UserId { user_id!("@alice:example.org") @@ -649,7 +586,10 @@ mod test { fn bob_machine() -> KeyRequestMachine { let user_id = Arc::new(bob_id()); - let store = Store::new(user_id.clone(), Arc::new(Box::new(MemoryStore::new()))); + let account = ReadOnlyAccount::new(&user_id, &alice_device_id()); + let store: Arc> = Arc::new(Box::new(MemoryStore::new())); + let verification = VerificationMachine::new(account, store.clone()); + let store = Store::new(user_id.clone(), store, verification); KeyRequestMachine::new( user_id, @@ -659,9 +599,14 @@ mod test { ) } - fn get_machine() -> KeyRequestMachine { + async fn get_machine() -> KeyRequestMachine { let user_id = Arc::new(alice_id()); - let store = Store::new(user_id.clone(), Arc::new(Box::new(MemoryStore::new()))); + let account = ReadOnlyAccount::new(&user_id, &alice_device_id()); + let device = ReadOnlyDevice::from_account(&account).await; + let store: Arc> = Arc::new(Box::new(MemoryStore::new())); + let verification = VerificationMachine::new(account, store.clone()); + let store = Store::new(user_id.clone(), store, verification); + store.save_devices(&[device]).await.unwrap(); KeyRequestMachine::new( user_id, @@ -671,16 +616,16 @@ mod test { ) } - #[test] - fn create_machine() { - let machine = get_machine(); + #[async_test] + async fn create_machine() { + let machine = get_machine().await; assert!(machine.outgoing_to_device_requests().is_empty()); } #[async_test] async fn create_key_request() { - let machine = get_machine(); + let machine = get_machine().await; let account = account(); let (_, session) = account @@ -721,7 +666,7 @@ mod test { #[async_test] async fn receive_forwarded_key() { - let machine = get_machine(); + let machine = get_machine().await; let account = account(); let (_, session) = account @@ -848,15 +793,15 @@ mod test { #[async_test] async fn should_share_key_test() { - let machine = get_machine(); + let machine = get_machine().await; let account = account(); - let own_device = Device { - store: machine.store.clone(), - inner: ReadOnlyDevice::from_account(&account).await, - own_identity: None, - device_owner_identity: None, - }; + let own_device = machine + .store + .get_device(&alice_id(), &alice_device_id()) + .await + .unwrap() + .unwrap(); // We don't share keys with untrusted devices. assert_eq!( @@ -869,12 +814,15 @@ mod test { // Now we do want to share the keys. assert!(machine.should_share_session(&own_device, None).is_ok()); - let bob_device = Device { - store: machine.store.clone(), - inner: ReadOnlyDevice::from_account(&bob_account()).await, - own_identity: None, - device_owner_identity: None, - }; + let bob_device = ReadOnlyDevice::from_account(&bob_account()).await; + machine.store.save_devices(&[bob_device]).await.unwrap(); + + let bob_device = machine + .store + .get_device(&bob_id(), &bob_device_id()) + .await + .unwrap() + .unwrap(); // We don't share sessions with other user's devices if no outbound // session was provided. @@ -918,7 +866,7 @@ mod test { #[async_test] async fn key_share_cycle() { - let alice_machine = get_machine(); + let alice_machine = get_machine().await; let alice_account = Account { inner: account(), store: alice_machine.store.clone(), diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 23e5878e..b173da6a 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -127,7 +127,7 @@ impl OlmMachine { let store = Arc::new(store); let verification_machine = VerificationMachine::new(account.clone(), store.clone()); - let store = Store::new(user_id.clone(), store); + let store = Store::new(user_id.clone(), store, verification_machine.clone()); let device_id: Arc = Arc::new(device_id); let outbound_group_sessions = Arc::new(DashMap::new()); let key_request_machine = KeyRequestMachine::new( @@ -430,7 +430,7 @@ impl OlmMachine { for (user_id, user_devices) in &response.one_time_keys { for (device_id, key_map) in user_devices { - let device = match self.store.get_device(&user_id, device_id).await { + let device = match self.store.get_readonly_device(&user_id, device_id).await { Ok(Some(d)) => d, Ok(None) => { warn!( @@ -889,16 +889,7 @@ impl OlmMachine { user_id: &UserId, device_id: &DeviceId, ) -> StoreResult> { - Ok(self - .store - .get_device_and_users(user_id, device_id) - .await? - .map(|(d, o, u)| Device { - inner: d, - verification_machine: self.verification_machine.clone(), - own_identity: o, - device_owner_identity: u, - })) + self.store.get_device(user_id, device_id).await } /// Get a map holding all the devices of an user. @@ -925,22 +916,7 @@ 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? - .map(|i| i.own().cloned()) - .flatten(); - let device_owner_identity = self.store.get_user_identity(user_id).await.ok().flatten(); - - Ok(UserDevices { - inner: devices, - verification_machine: self.verification_machine.clone(), - own_identity, - device_owner_identity, - }) + self.store.get_user_devices(user_id).await } /// Import the given room keys into our store. diff --git a/matrix_sdk_crypto/src/store/mod.rs b/matrix_sdk_crypto/src/store/mod.rs index a412f657..83c2a0f9 100644 --- a/matrix_sdk_crypto/src/store/mod.rs +++ b/matrix_sdk_crypto/src/store/mod.rs @@ -70,13 +70,13 @@ use matrix_sdk_common_macros::async_trait; #[cfg(not(target_arch = "wasm32"))] use matrix_sdk_common_macros::send_sync; -use super::{ - identities::{OwnUserIdentity, ReadOnlyDevice, UserIdentities}, +use crate::{ + error::SessionUnpicklingError, + identities::{Device, ReadOnlyDevice, UserDevices, UserIdentities}, olm::{InboundGroupSession, ReadOnlyAccount, Session}, + verification::VerificationMachine, }; -use crate::error::SessionUnpicklingError; - /// A `CryptoStore` specific result type. pub type Result = std::result::Result; @@ -90,48 +90,77 @@ pub type Result = std::result::Result; pub(crate) struct Store { user_id: Arc, inner: Arc>, + verification_machine: VerificationMachine, } impl Store { - pub fn new(user_id: Arc, store: Arc>) -> Self { + pub fn new( + user_id: Arc, + store: Arc>, + verification_machine: VerificationMachine, + ) -> Self { Self { user_id, inner: store, + verification_machine, } } - pub async fn get_device_and_users( + pub async fn get_readonly_device( &self, user_id: &UserId, device_id: &DeviceId, - ) -> Result< - Option<( - ReadOnlyDevice, - Option, - Option, - )>, - > { - let device = self.get_device(user_id, device_id).await?; - - let device = if let Some(d) = device { - d - } else { - return Ok(None); - }; - - let own_identity = self - .get_user_identity(&self.user_id) - .await - .ok() - .flatten() - .map(|i| i.own().cloned()) - .flatten(); - let device_owner_identity = self.get_user_identity(user_id).await.ok().flatten(); - - Ok(Some((device, own_identity, device_owner_identity))) + ) -> Result> { + self.inner.get_device(user_id, device_id).await + } + + pub async fn get_readonly_devices(&self, user_id: &UserId) -> Result { + self.inner.get_user_devices(user_id).await + } + + pub async fn get_user_devices(&self, user_id: &UserId) -> Result { + let devices = self.inner.get_user_devices(user_id).await?; + + let own_identity = self + .inner + .get_user_identity(&self.user_id) + .await? + .map(|i| i.own().cloned()) + .flatten(); + let device_owner_identity = self.inner.get_user_identity(user_id).await.ok().flatten(); + + Ok(UserDevices { + inner: devices, + verification_machine: self.verification_machine.clone(), + own_identity, + device_owner_identity, + }) + } + + pub async fn get_device( + &self, + user_id: &UserId, + device_id: &DeviceId, + ) -> Result> { + let own_identity = self + .get_user_identity(&self.user_id) + .await? + .map(|i| i.own().cloned()) + .flatten(); + let device_owner_identity = self.get_user_identity(user_id).await?; + + Ok(self + .inner + .get_device(user_id, device_id) + .await? + .map(|d| Device { + inner: d, + verification_machine: self.verification_machine.clone(), + own_identity, + device_owner_identity, + })) } - #[allow(dead_code)] pub async fn get_object Deserialize<'b>>(&self, key: &str) -> Result> { if let Some(value) = self.get_value(key).await? { Ok(Some(serde_json::from_str(&value)?)) @@ -140,13 +169,11 @@ impl Store { } } - #[allow(dead_code)] pub async fn save_object(&self, key: &str, value: &impl Serialize) -> Result<()> { let value = serde_json::to_string(value)?; self.save_value(key.to_owned(), value).await } - #[allow(dead_code)] pub async fn delete_object(&self, key: &str) -> Result<()> { self.inner.remove_value(key).await?; Ok(())