From 24592adbba3c7d3574f631bdbadc67f85192e18e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 1 Dec 2020 12:41:11 +0100 Subject: [PATCH] crypto: Return a higher level struct when decrypting olm messages instead of tuples --- matrix_sdk_crypto/src/file_encryption/mod.rs | 4 +- matrix_sdk_crypto/src/key_request.rs | 26 +++-- matrix_sdk_crypto/src/machine.rs | 114 ++++++++++--------- matrix_sdk_crypto/src/olm/account.rs | 35 ++++-- matrix_sdk_crypto/src/olm/mod.rs | 2 +- matrix_sdk_crypto/src/olm/session.rs | 4 +- 6 files changed, 107 insertions(+), 78 deletions(-) diff --git a/matrix_sdk_crypto/src/file_encryption/mod.rs b/matrix_sdk_crypto/src/file_encryption/mod.rs index 3c644bd9..45ccd310 100644 --- a/matrix_sdk_crypto/src/file_encryption/mod.rs +++ b/matrix_sdk_crypto/src/file_encryption/mod.rs @@ -14,10 +14,10 @@ fn decode_url_safe(input: impl AsRef<[u8]>) -> Result, DecodeError> { decode_config(input, URL_SAFE_NO_PAD) } -fn encode(input: impl AsRef<[u8]>) -> String { +pub fn encode(input: impl AsRef<[u8]>) -> String { encode_config(input, STANDARD_NO_PAD) } -fn encode_url_safe(input: impl AsRef<[u8]>) -> String { +pub fn encode_url_safe(input: impl AsRef<[u8]>) -> String { encode_config(input, URL_SAFE_NO_PAD) } diff --git a/matrix_sdk_crypto/src/key_request.rs b/matrix_sdk_crypto/src/key_request.rs index 55f5b569..32cbd69f 100644 --- a/matrix_sdk_crypto/src/key_request.rs +++ b/matrix_sdk_crypto/src/key_request.rs @@ -1137,12 +1137,11 @@ mod test { .unwrap() .is_none()); - let (_, decrypted, sender_key, _) = - alice_account.decrypt_to_device_event(&event).await.unwrap(); + let decrypted = alice_account.decrypt_to_device_event(&event).await.unwrap(); - if let AnyToDeviceEvent::ForwardedRoomKey(mut e) = decrypted.deserialize().unwrap() { + if let AnyToDeviceEvent::ForwardedRoomKey(mut e) = decrypted.event.deserialize().unwrap() { let (_, session) = alice_machine - .receive_forwarded_room_key(&sender_key, &mut e) + .receive_forwarded_room_key(&decrypted.sender_key, &mut e) .await .unwrap(); alice_machine @@ -1157,7 +1156,11 @@ mod test { // 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()) + .get_inbound_group_session( + &room_id(), + &decrypted.sender_key, + group_session.session_id(), + ) .await .unwrap() .unwrap(); @@ -1325,12 +1328,11 @@ mod test { .unwrap() .is_none()); - let (_, decrypted, sender_key, _) = - alice_account.decrypt_to_device_event(&event).await.unwrap(); + let decrypted = alice_account.decrypt_to_device_event(&event).await.unwrap(); - if let AnyToDeviceEvent::ForwardedRoomKey(mut e) = decrypted.deserialize().unwrap() { + if let AnyToDeviceEvent::ForwardedRoomKey(mut e) = decrypted.event.deserialize().unwrap() { let (_, session) = alice_machine - .receive_forwarded_room_key(&sender_key, &mut e) + .receive_forwarded_room_key(&decrypted.sender_key, &mut e) .await .unwrap(); alice_machine @@ -1345,7 +1347,11 @@ mod test { // 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()) + .get_inbound_group_session( + &room_id(), + &decrypted.sender_key, + group_session.session_id(), + ) .await .unwrap() .unwrap(); diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 83b5939a..dbea9a0a 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -52,7 +52,7 @@ use crate::{ key_request::KeyRequestMachine, olm::{ Account, EncryptionSettings, ExportedRoomKey, GroupSessionKey, IdentityKeys, - InboundGroupSession, PrivateCrossSigningIdentity, ReadOnlyAccount, Session, + InboundGroupSession, OlmDecryptionInfo, PrivateCrossSigningIdentity, ReadOnlyAccount, }, requests::{IncomingResponse, OutgoingRequest, UploadSigningKeysRequest}, session_manager::{GroupSessionManager, SessionManager}, @@ -555,24 +555,23 @@ impl OlmMachine { async fn decrypt_to_device_event( &self, event: &ToDeviceEvent, - ) -> OlmResult<(Session, Raw, Option)> { - let (session, decrypted_event, sender_key, signing_key) = - self.account.decrypt_to_device_event(event).await?; + ) -> OlmResult { + let mut decrypted = 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), group_session) = self - .handle_decrypted_to_device_event(&sender_key, &signing_key, &decrypted_event) - .await? + if let (Some(event), group_session) = + self.handle_decrypted_to_device_event(&decrypted).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((session, event, group_session)) - } else { - Ok((session, decrypted_event, None)) + decrypted.event = event; + decrypted.inbound_group_session = group_session; } + + Ok(decrypted) } /// Create a group session from a room key and add it to our crypto store. @@ -704,27 +703,29 @@ impl OlmMachine { /// * `event` - The decrypted to-device event. async fn handle_decrypted_to_device_event( &self, - sender_key: &str, - signing_key: &str, - event: &Raw, + decrypted: &OlmDecryptionInfo, ) -> OlmResult<(Option>, Option)> { - let event = if let Ok(e) = event.deserialize() { - e - } else { - warn!("Decrypted to-device event failed to be parsed correctly"); - return Ok((None, None)); + let event = match decrypted.event.deserialize() { + Ok(e) => e, + Err(e) => { + warn!( + "Decrypted to-device event failed to be parsed correctly {:?}", + e + ); + return Ok((None, None)); + } }; match event { - AnyToDeviceEvent::RoomKey(mut e) => { - Ok(self.add_room_key(sender_key, signing_key, &mut e).await?) - } + AnyToDeviceEvent::RoomKey(mut e) => Ok(self + .add_room_key(&decrypted.sender_key, &decrypted.signing_key, &mut e) + .await?), AnyToDeviceEvent::ForwardedRoomKey(mut e) => Ok(self .key_request_machine - .receive_forwarded_room_key(sender_key, &mut e) + .receive_forwarded_room_key(&decrypted.sender_key, &mut e) .await?), _ => { - warn!("Received a unexpected encrypted to-device event"); + warn!("Received an unexpected encrypted to-device event"); Ok((None, None)) } } @@ -808,38 +809,37 @@ impl OlmMachine { match &mut event { AnyToDeviceEvent::RoomEncrypted(e) => { - let (session, decrypted_event, group_session) = - match self.decrypt_to_device_event(e).await { - Ok(e) => e, - Err(err) => { - warn!( - "Failed to decrypt to-device event from {} {}", - e.sender, err - ); + let decrypted = match self.decrypt_to_device_event(e).await { + Ok(e) => e, + Err(err) => { + warn!( + "Failed to decrypt to-device event from {} {}", + e.sender, err + ); - if let OlmError::SessionWedged(sender, curve_key) = err { - if let Err(e) = self - .session_manager - .mark_device_as_wedged(&sender, &curve_key) - .await - { - error!( - "Couldn't mark device from {} to be unwedged {:?}", - sender, e - ); - } + if let OlmError::SessionWedged(sender, curve_key) = err { + if let Err(e) = self + .session_manager + .mark_device_as_wedged(&sender, &curve_key) + .await + { + error!( + "Couldn't mark device from {} to be unwedged {:?}", + sender, e + ); } - continue; } - }; + continue; + } + }; - changes.sessions.push(session); + changes.sessions.push(decrypted.session); - if let Some(group_session) = group_session { + if let Some(group_session) = decrypted.inbound_group_session { changes.inbound_group_sessions.push(group_session); } - *event_result = decrypted_event; + *event_result = decrypted.event; } AnyToDeviceEvent::RoomKeyRequest(e) => { self.key_request_machine.receive_incoming_key_request(e) @@ -1283,8 +1283,8 @@ pub(crate) mod test { content, }; - let (session, _, _) = bob.decrypt_to_device_event(&event).await.unwrap(); - bob.store.save_sessions(&[session]).await.unwrap(); + let decrypted = bob.decrypt_to_device_event(&event).await.unwrap(); + bob.store.save_sessions(&[decrypted.session]).await.unwrap(); (alice, bob) } @@ -1578,7 +1578,7 @@ pub(crate) mod test { .decrypt_to_device_event(&event) .await .unwrap() - .1 + .event .deserialize() .unwrap(); @@ -1614,14 +1614,14 @@ pub(crate) mod test { .get_outbound_group_session(&room_id) .unwrap(); - let (session, event, group_session) = bob.decrypt_to_device_event(&event).await.unwrap(); + let decrypted = bob.decrypt_to_device_event(&event).await.unwrap(); - bob.store.save_sessions(&[session]).await.unwrap(); + bob.store.save_sessions(&[decrypted.session]).await.unwrap(); bob.store - .save_inbound_group_sessions(&[group_session.unwrap()]) + .save_inbound_group_sessions(&[decrypted.inbound_group_session.unwrap()]) .await .unwrap(); - let event = event.deserialize().unwrap(); + let event = decrypted.event.deserialize().unwrap(); if let AnyToDeviceEvent::RoomKey(event) = event { assert_eq!(&event.sender, alice.user_id()); @@ -1661,7 +1661,11 @@ pub(crate) mod test { content: to_device_requests_to_content(to_device_requests), }; - let (_, _, group_session) = bob.decrypt_to_device_event(&event).await.unwrap(); + let group_session = bob + .decrypt_to_device_event(&event) + .await + .unwrap() + .inbound_group_session; bob.store .save_inbound_group_sessions(&[group_session.unwrap()]) .await diff --git a/matrix_sdk_crypto/src/olm/account.rs b/matrix_sdk_crypto/src/olm/account.rs index ed57b893..d9a9e387 100644 --- a/matrix_sdk_crypto/src/olm/account.rs +++ b/matrix_sdk_crypto/src/olm/account.rs @@ -15,6 +15,7 @@ use matrix_sdk_common::events::ToDeviceEvent; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; +use sha2::{Digest, Sha256}; use std::{ collections::BTreeMap, convert::{TryFrom, TryInto}, @@ -53,6 +54,7 @@ use olm_rs::{ use crate::{ error::{EventError, OlmResult, SessionCreationError}, + file_encryption::encode, identities::ReadOnlyDevice, requests::UploadSigningKeysRequest, store::Store, @@ -70,6 +72,15 @@ pub struct Account { pub(crate) store: Store, } +pub struct OlmDecryptionInfo { + pub session: Session, + pub message_hash: String, + pub event: Raw, + pub signing_key: String, + pub sender_key: String, + pub inbound_group_session: Option, +} + impl Deref for Account { type Target = ReadOnlyAccount; @@ -82,7 +93,7 @@ impl Account { pub async fn decrypt_to_device_event( &self, event: &ToDeviceEvent, - ) -> OlmResult<(Session, Raw, String, String)> { + ) -> OlmResult { debug!("Decrypting to-device event"); let content = if let EncryptedEventContent::OlmV1Curve25519AesSha2(c) = &event.content { @@ -103,23 +114,33 @@ impl Account { .try_into() .map_err(|_| EventError::UnsupportedOlmType)?; + let sha = Sha256::new() + .chain(&content.sender_key) + .chain(&[message_type]) + .chain(&ciphertext.body); + + let message_hash = encode(sha.finalize().as_slice()); + // 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 (session, decrypted_event, signing_key) = self + let (session, event, signing_key) = self .decrypt_olm_message(&event.sender, &content.sender_key, message) .await?; - debug!("Decrypted a to-device event {:?}", decrypted_event); - Ok(( + debug!("Decrypted a to-device event {:?}", event); + + Ok(OlmDecryptionInfo { session, - decrypted_event, - content.sender_key.clone(), + message_hash, + event, signing_key, - )) + sender_key: content.sender_key.clone(), + inbound_group_session: None, + }) } else { warn!("Olm event doesn't contain a ciphertext for our key"); Err(EventError::MissingCiphertext.into()) diff --git a/matrix_sdk_crypto/src/olm/mod.rs b/matrix_sdk_crypto/src/olm/mod.rs index 116f9b82..22520f47 100644 --- a/matrix_sdk_crypto/src/olm/mod.rs +++ b/matrix_sdk_crypto/src/olm/mod.rs @@ -23,7 +23,7 @@ mod session; mod signing; mod utility; -pub(crate) use account::Account; +pub(crate) use account::{Account, OlmDecryptionInfo}; pub use account::{AccountPickle, PickledAccount, ReadOnlyAccount}; pub use group_sessions::{ EncryptionSettings, ExportedRoomKey, InboundGroupSession, InboundGroupSessionPickle, diff --git a/matrix_sdk_crypto/src/olm/session.rs b/matrix_sdk_crypto/src/olm/session.rs index a68f8230..8d031f63 100644 --- a/matrix_sdk_crypto/src/olm/session.rs +++ b/matrix_sdk_crypto/src/olm/session.rs @@ -126,9 +126,7 @@ impl Session { "content": content, }); - let plaintext = serde_json::to_string(&payload) - .unwrap_or_else(|_| panic!(format!("Can't serialize {} to canonical JSON", payload))); - + let plaintext = serde_json::to_string(&payload)?; let ciphertext = self.encrypt_helper(&plaintext).await.to_tuple(); let message_type = ciphertext.0;