From 8fe1eda169e19ad91adc26523c38adf4250292f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 29 Sep 2020 14:18:03 +0200 Subject: [PATCH] crypto: Test the full key share flow. --- matrix_sdk/src/client.rs | 5 +- matrix_sdk/src/device.rs | 5 +- matrix_sdk_base/src/client.rs | 3 +- matrix_sdk_crypto/src/key_request.rs | 106 ++++++++++++++++++++------- matrix_sdk_crypto/src/machine.rs | 73 +++++------------- matrix_sdk_crypto/src/olm/account.rs | 90 ++++++++++++++++++----- matrix_sdk_crypto/src/olm/mod.rs | 1 - 7 files changed, 175 insertions(+), 108 deletions(-) diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index c3bfa63f..fb62ee74 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -1653,7 +1653,10 @@ impl Client { /// # let homeserver = Url::parse("http://example.com").unwrap(); /// # let client = Client::new(homeserver).unwrap(); /// # block_on(async { - /// let device = client.get_device(&alice, "DEVICEID".into()).await.unwrap(); + /// let device = client.get_device(&alice, "DEVICEID".into()) + /// .await + /// .unwrap() + /// .unwrap(); /// /// println!("{:?}", device.is_trusted()); /// diff --git a/matrix_sdk/src/device.rs b/matrix_sdk/src/device.rs index c9c14e44..5c7b55fb 100644 --- a/matrix_sdk/src/device.rs +++ b/matrix_sdk/src/device.rs @@ -55,7 +55,10 @@ impl Device { /// # let homeserver = Url::parse("http://example.com").unwrap(); /// # let client = Client::new(homeserver).unwrap(); /// # block_on(async { - /// let device = client.get_device(&alice, "DEVICEID".into()).await.unwrap(); + /// let device = client.get_device(&alice, "DEVICEID".into()) + /// .await + /// .unwrap() + /// .unwrap(); /// /// let verification = device.start_verification().await.unwrap(); /// # }); diff --git a/matrix_sdk_base/src/client.rs b/matrix_sdk_base/src/client.rs index f6731803..97965999 100644 --- a/matrix_sdk_base/src/client.rs +++ b/matrix_sdk_base/src/client.rs @@ -1819,8 +1819,7 @@ impl BaseClient { if let Some(olm) = olm.as_ref() { olm.get_device(user_id, device_id).await } else { - // TODO remove this panic. - panic!("The client hasn't been logged in") + Ok(None) } } diff --git a/matrix_sdk_crypto/src/key_request.rs b/matrix_sdk_crypto/src/key_request.rs index 842c6467..ace40b6e 100644 --- a/matrix_sdk_crypto/src/key_request.rs +++ b/matrix_sdk_crypto/src/key_request.rs @@ -14,10 +14,6 @@ // TODO // -// Incoming key requests: -// First handle the easy case, if we trust the device and have a session, queue -// up a to-device request. -// // If we don't have a session, queue up a key claim request, once we get a // session send out the key if we trust the device. // @@ -31,7 +27,7 @@ use serde::{Deserialize, Serialize}; use serde_json::{value::to_raw_value, Value}; use std::{collections::BTreeMap, convert::TryInto, ops::Deref, sync::Arc}; use thiserror::Error; -use tracing::{info, instrument, trace, warn}; +use tracing::{error, info, instrument, trace, warn}; use matrix_sdk_common::{ api::r0::to_device::DeviceIdOrAllDevices, @@ -62,12 +58,37 @@ struct Device { } 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(crate) async fn encrypt( + async fn encrypt( &self, event_type: EventType, content: Value, @@ -208,7 +229,7 @@ impl KeyRequestMachine { /// Handle all the incoming key requests that are queued up and empty our /// key request queue. - pub async fn collect_incoming_key_requests(&self) -> Result<(), CryptoStoreError> { + pub async fn collect_incoming_key_requests(&self) -> OlmResult<()> { for item in self.incoming_key_requests.iter() { let event = item.value(); self.handle_key_request(event).await?; @@ -224,7 +245,7 @@ impl KeyRequestMachine { async fn handle_key_request( &self, event: &ToDeviceEvent, - ) -> Result<(), CryptoStoreError> { + ) -> OlmResult<()> { let key_info = match event.content.action { Action::Request => { if let Some(info) = &event.content.body { @@ -273,7 +294,6 @@ impl KeyRequestMachine { }); if let Some(device) = device { - // TODO get the matching outbound session. if let Err(e) = self.should_share_session( &device, self.outbound_group_sessions @@ -294,7 +314,8 @@ impl KeyRequestMachine { device.device_id() ); - self.share_session(session, device).await; + // TODO the missing session error here. + self.share_session(session, device).await?; } } else { warn!( @@ -307,17 +328,10 @@ impl KeyRequestMachine { Ok(()) } - async fn share_session(&self, session: InboundGroupSession, device: Device) { - let export = session.export().await; - let content: ForwardedRoomKeyEventContent = export.try_into().unwrap(); - let content = serde_json::to_value(content).unwrap(); - let content = device - .encrypt(EventType::ForwardedRoomKey, content) - .await - .unwrap(); + async fn share_session(&self, session: InboundGroupSession, device: Device) -> OlmResult<()> { + let content = device.encrypt_session(session).await?; let id = Uuid::new_v4(); - let mut messages = BTreeMap::new(); messages @@ -325,7 +339,7 @@ impl KeyRequestMachine { .or_insert_with(BTreeMap::new) .insert( DeviceIdOrAllDevices::DeviceId(device.device_id().into()), - to_raw_value(&content).unwrap(), + to_raw_value(&content)?, ); let request = OutgoingRequest { @@ -341,6 +355,8 @@ impl KeyRequestMachine { }; self.outgoing_to_device_requests.insert(id, request); + + Ok(()) } /// Check if it's ok to share a session with the given device. @@ -590,7 +606,7 @@ mod test { events::{ forwarded_room_key::ForwardedRoomKeyEventContent, room::encrypted::EncryptedEventContent, room_key_request::RoomKeyRequestEventContent, - ToDeviceEvent, + AnyToDeviceEvent, ToDeviceEvent, }, identifiers::{room_id, user_id, DeviceIdBox, RoomId, UserId}, }; @@ -599,7 +615,7 @@ mod test { use crate::{ identities::{LocalTrust, ReadOnlyDevice}, - olm::ReadOnlyAccount, + olm::{Account, ReadOnlyAccount}, store::{MemoryStore, Store}, }; @@ -905,7 +921,10 @@ mod test { #[async_test] async fn key_share_cycle() { let alice_machine = get_machine(); - let alice_account = account(); + let alice_account = Account { + inner: account(), + store: alice_machine.store.clone(), + }; let bob_machine = bob_machine(); let bob_account = bob_account(); @@ -964,7 +983,7 @@ mod test { // Put the outbound session into bobs store. bob_machine .outbound_group_sessions - .insert(room_id(), group_session); + .insert(room_id(), group_session.clone()); // Get the request and convert it into a event. let request = alice_machine @@ -1029,12 +1048,45 @@ mod test { .await .unwrap(); - let _event = ToDeviceEvent { + let mut event = ToDeviceEvent { sender: bob_id(), content, }; - // TODO test that alice can receive, decrypt and add the requested key - // to the store. + // Check that alice doesn't have the session. + assert!(alice_machine + .store + .get_inbound_group_session( + &room_id(), + &bob_account.identity_keys().curve25519(), + group_session.session_id() + ) + .await + .unwrap() + .is_none()); + + let (decrypted, sender_key, _) = alice_account + .decrypt_to_device_event(&mut event) + .await + .unwrap(); + + if let AnyToDeviceEvent::ForwardedRoomKey(mut e) = decrypted.deserialize().unwrap() { + alice_machine + .receive_forwarded_room_key(&sender_key, &mut e) + .await + .unwrap(); + } else { + panic!("Invalid decrypted event type"); + } + + // Check that alice now does have the session. + let session = alice_machine + .store + .get_inbound_group_session(&room_id(), &sender_key, group_session.session_id()) + .await + .unwrap() + .unwrap(); + + assert_eq!(session.session_id(), group_session.session_id()) } } diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index d148b9d2..ebe67f70 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -14,7 +14,7 @@ #[cfg(feature = "sqlite_cryptostore")] use std::path::Path; -use std::{collections::BTreeMap, convert::TryInto, mem, sync::Arc, time::Duration}; +use std::{collections::BTreeMap, mem, sync::Arc, time::Duration}; use dashmap::DashMap; use tracing::{debug, error, info, instrument, trace, warn}; @@ -51,7 +51,7 @@ use super::{ key_request::KeyRequestMachine, olm::{ Account, EncryptionSettings, ExportedRoomKey, GroupSessionKey, IdentityKeys, - InboundGroupSession, OlmMessage, OutboundGroupSession, ReadOnlyAccount, + InboundGroupSession, OutboundGroupSession, ReadOnlyAccount, }, requests::{IncomingResponse, OutgoingRequest, ToDeviceRequest}, store::{CryptoStore, MemoryStore, Result as StoreResult, Store}, @@ -513,61 +513,22 @@ impl OlmMachine { &self, event: &ToDeviceEvent, ) -> OlmResult> { - info!("Decrypting to-device event"); - - let content = if let EncryptedEventContent::OlmV1Curve25519AesSha2(c) = &event.content { - c + let (decrypted_event, sender_key, signing_key) = + self.account.decrypt_to_device_event(event).await?; + // Handle the decrypted event, e.g. fetch out Megolm sessions out of + // the event. + if let Some(event) = self + .handle_decrypted_to_device_event(&sender_key, &signing_key, &decrypted_event) + .await? + { + // Some events may have sensitive data e.g. private keys, while we + // want to notify our users that a private key was received we + // don't want them to be able to do silly things with it. Handling + // events modifies them and returns a modified one, so replace it + // here if we get one. + Ok(event) } else { - warn!("Error, unsupported encryption algorithm"); - return Err(EventError::UnsupportedAlgorithm.into()); - }; - - let identity_keys = self.account.identity_keys(); - let own_key = identity_keys.curve25519(); - let own_ciphertext = content.ciphertext.get(own_key); - - // Try to find a ciphertext that was meant for our device. - if let Some(ciphertext) = own_ciphertext { - let message_type: u8 = ciphertext - .message_type - .try_into() - .map_err(|_| EventError::UnsupportedOlmType)?; - - // Create a OlmMessage from the ciphertext and the type. - let message = - OlmMessage::from_type_and_ciphertext(message_type.into(), ciphertext.body.clone()) - .map_err(|_| EventError::UnsupportedOlmType)?; - - // Decrypt the OlmMessage and get a Ruma event out of it. - let (decrypted_event, signing_key) = self - .account - .decrypt_olm_message(&event.sender, &content.sender_key, message) - .await?; - - debug!("Decrypted a to-device event {:?}", decrypted_event); - - // Handle the decrypted event, e.g. fetch out Megolm sessions out of - // the event. - if let Some(event) = self - .handle_decrypted_to_device_event( - &content.sender_key, - &signing_key, - &decrypted_event, - ) - .await? - { - // Some events may have sensitive data e.g. private keys, while we - // want to notify our users that a private key was received we - // don't want them to be able to do silly things with it. Handling - // events modifies them and returns a modified one, so replace it - // here if we get one. - Ok(event) - } else { - Ok(decrypted_event) - } - } else { - warn!("Olm event doesn't contain a ciphertext for our key"); - Err(EventError::MissingCiphertext.into()) + Ok(decrypted_event) } } diff --git a/matrix_sdk_crypto/src/olm/account.rs b/matrix_sdk_crypto/src/olm/account.rs index 23deef9e..de20784b 100644 --- a/matrix_sdk_crypto/src/olm/account.rs +++ b/matrix_sdk_crypto/src/olm/account.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use matrix_sdk_common::events::ToDeviceEvent; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; use std::{ @@ -27,11 +28,11 @@ use std::{ use tracing::{debug, trace, warn}; #[cfg(test)] -use matrix_sdk_common::events::{room::encrypted::EncryptedEventContent, EventType}; +use matrix_sdk_common::events::EventType; use matrix_sdk_common::{ api::r0::keys::{upload_keys, OneTimeKey, SignedKey}, encryption::DeviceKeys, - events::AnyToDeviceEvent, + events::{room::encrypted::EncryptedEventContent, AnyToDeviceEvent}, identifiers::{ DeviceId, DeviceIdBox, DeviceKeyAlgorithm, DeviceKeyId, EventEncryptionAlgorithm, RoomId, UserId, @@ -72,6 +73,48 @@ impl Deref for Account { } impl Account { + pub async fn decrypt_to_device_event( + &self, + event: &ToDeviceEvent, + ) -> OlmResult<(Raw, String, String)> { + debug!("Decrypting to-device event"); + + let content = if let EncryptedEventContent::OlmV1Curve25519AesSha2(c) = &event.content { + c + } else { + warn!("Error, unsupported encryption algorithm"); + return Err(EventError::UnsupportedAlgorithm.into()); + }; + + let identity_keys = self.inner.identity_keys(); + let own_key = identity_keys.curve25519(); + let own_ciphertext = content.ciphertext.get(own_key); + + // Try to find a ciphertext that was meant for our device. + if let Some(ciphertext) = own_ciphertext { + let message_type: u8 = ciphertext + .message_type + .try_into() + .map_err(|_| EventError::UnsupportedOlmType)?; + + // Create a OlmMessage from the ciphertext and the type. + let message = + OlmMessage::from_type_and_ciphertext(message_type.into(), ciphertext.body.clone()) + .map_err(|_| EventError::UnsupportedOlmType)?; + + // Decrypt the OlmMessage and get a Ruma event out of it. + let (decrypted_event, signing_key) = self + .decrypt_olm_message(&event.sender, &content.sender_key, message) + .await?; + + debug!("Decrypted a to-device event {:?}", decrypted_event); + Ok((decrypted_event, content.sender_key.clone(), signing_key)) + } else { + warn!("Olm event doesn't contain a ciphertext for our key"); + Err(EventError::MissingCiphertext.into()) + } + } + pub async fn update_uploaded_key_count( &self, key_count: &BTreeMap, @@ -146,21 +189,24 @@ impl Account { let ret = session.decrypt(message.clone()).await; - if let Ok(p) = ret { - plaintext = Some(p); - session_to_save = Some(session.clone()); + match ret { + Ok(p) => { + plaintext = Some(p); + session_to_save = Some(session.clone()); - break; - } else { - // Decryption failed with a matching session, the session is - // likely wedged and needs to be rotated. - if matches { - warn!( - "Found a matching Olm session yet decryption failed - for sender {} and sender_key {}", - sender, sender_key - ); - return Err(OlmError::SessionWedged); + break; + } + Err(e) => { + // Decryption failed with a matching session, the session is + // likely wedged and needs to be rotated. + if matches { + warn!( + "Found a matching Olm session yet decryption failed + for sender {} and sender_key {} {:?}", + sender, sender_key, e + ); + return Err(OlmError::SessionWedged); + } } } } @@ -176,7 +222,7 @@ impl Account { } /// Decrypt an Olm message, creating a new Olm session if possible. - pub async fn decrypt_olm_message( + async fn decrypt_olm_message( &self, sender: &UserId, sender_key: &str, @@ -882,6 +928,8 @@ impl ReadOnlyAccount { .await .unwrap(); + other.mark_keys_as_published().await; + let message = our_session .encrypt(&device, EventType::Dummy, json!({})) .await @@ -901,14 +949,14 @@ impl ReadOnlyAccount { let message = OlmMessage::from_type_and_ciphertext(message_type.into(), own_ciphertext.body.clone()) .unwrap(); - let message = if let OlmMessage::PreKey(m) = message { + let prekey = if let OlmMessage::PreKey(m) = message.clone() { m } else { panic!("Wrong Olm message type"); }; let our_device = ReadOnlyDevice::from_account(self).await; - let other_session = other + let mut other_session = other .create_inbound_session( our_device .keys() @@ -917,11 +965,13 @@ impl ReadOnlyAccount { our_device.device_id(), )) .unwrap(), - message, + prekey, ) .await .unwrap(); + other_session.decrypt(message).await.unwrap(); + (our_session, other_session) } } diff --git a/matrix_sdk_crypto/src/olm/mod.rs b/matrix_sdk_crypto/src/olm/mod.rs index dda23c76..eef28ff1 100644 --- a/matrix_sdk_crypto/src/olm/mod.rs +++ b/matrix_sdk_crypto/src/olm/mod.rs @@ -30,7 +30,6 @@ pub use group_sessions::{ }; pub(crate) use group_sessions::{GroupSessionKey, OutboundGroupSession}; pub use olm_rs::{account::IdentityKeys, PicklingMode}; -pub(crate) use session::OlmMessage; pub use session::{PickledSession, Session, SessionPickle}; pub(crate) use utility::Utility;