From 3c59e186d9795079dd0be504b2c0bd79d841bcfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 27 Oct 2021 14:33:11 +0200 Subject: [PATCH] improvement(crypto): Improve the logging when we decrypt room events --- crates/matrix-sdk-crypto/src/error.rs | 8 +- .../src/gossiping/machine.rs | 11 +- crates/matrix-sdk-crypto/src/machine.rs | 129 +++++++++++++----- 3 files changed, 104 insertions(+), 44 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/error.rs b/crates/matrix-sdk-crypto/src/error.rs index c3c6aeda6..d44bc574d 100644 --- a/crates/matrix-sdk-crypto/src/error.rs +++ b/crates/matrix-sdk-crypto/src/error.rs @@ -77,10 +77,10 @@ pub enum MegolmError { #[error(transparent)] JsonError(#[from] SerdeError), - /// Decryption failed because the session needed to decrypt the event is - /// missing. - #[error("decryption failed because the session to decrypt the message is missing")] - MissingSession, + /// Decryption failed because we're missing the room key that was to encrypt + /// the event. + #[error("decryption failed because the room key is missing")] + MissingRoomKey, /// The underlying group session operation returned an error. #[error("can't finish Olm group session operation {0}")] diff --git a/crates/matrix-sdk-crypto/src/gossiping/machine.rs b/crates/matrix-sdk-crypto/src/gossiping/machine.rs index 7a8958001..8f8e7619a 100644 --- a/crates/matrix-sdk-crypto/src/gossiping/machine.rs +++ b/crates/matrix-sdk-crypto/src/gossiping/machine.rs @@ -661,7 +661,7 @@ impl GossipMachine { room_id: &RoomId, sender_key: &str, session_id: &str, - ) -> Result<(), CryptoStoreError> { + ) -> Result { let key_info = RequestedKeyInfo::new( EventEncryptionAlgorithm::MegolmV1AesSha2, room_id.to_owned(), @@ -670,11 +670,12 @@ impl GossipMachine { ) .into(); - if self.should_request_key(&key_info).await? { + Ok(if self.should_request_key(&key_info).await? { self.request_key_helper(key_info).await?; - } - - Ok(()) + true + } else { + false + }) } /// Save an outgoing key info. diff --git a/crates/matrix-sdk-crypto/src/machine.rs b/crates/matrix-sdk-crypto/src/machine.rs index d1436586a..68b27ca31 100644 --- a/crates/matrix-sdk-crypto/src/machine.rs +++ b/crates/matrix-sdk-crypto/src/machine.rs @@ -39,7 +39,8 @@ use ruma::{ assign, events::{ room::encrypted::{ - EncryptedEventContent, EncryptedEventScheme, SyncEncryptedEvent, ToDeviceEncryptedEvent, + EncryptedEventContent, EncryptedEventScheme, MegolmV1AesSha2Content, + SyncEncryptedEvent, ToDeviceEncryptedEvent, }, room_key::ToDeviceRoomKeyEvent, secret::request::SecretName, @@ -1055,6 +1056,62 @@ impl OlmMachine { }) } + async fn decrypt_megolm_v1_event( + &self, + room_id: &RoomId, + event: &SyncEncryptedEvent, + content: &MegolmV1AesSha2Content, + ) -> MegolmResult { + if let Some(session) = self + .store + .get_inbound_group_session(room_id, &content.sender_key, &content.session_id) + .await? + { + // TODO check the message index. + let (decrypted_event, _) = session.decrypt(event).await?; + + match decrypted_event.deserialize() { + Ok(e) => { + // TODO log the event type once `AnySyncRoomEvent` has the + // method as well + trace!( + sender = event.sender.as_str(), + room_id = room_id.as_str(), + session_id = session.session_id(), + sender_key = session.sender_key(), + "Successfully decrypted a room event" + ); + let event = e.into_full_event(room_id.to_owned()); + + if let AnyRoomEvent::Message(e) = event { + self.verification_machine.receive_any_event(&e).await?; + } + } + Err(e) => { + warn!( + sender = event.sender.as_str(), + room_id = room_id.as_str(), + session_id = session.session_id(), + sender_key = session.sender_key(), + error =? e, + "Event was successfully decrypted but has an invalid format" + ); + } + } + + let encryption_info = + self.get_encryption_info(&session, &event.sender, &content.device_id).await?; + + Ok(SyncRoomEvent { encryption_info: Some(encryption_info), event: decrypted_event }) + } else { + self.key_request_machine + .create_outgoing_key_request(room_id, &content.sender_key, &content.session_id) + .await?; + + Err(MegolmError::MissingRoomKey) + } + } + /// Decrypt an event from a room timeline. /// /// # Arguments @@ -1067,43 +1124,45 @@ impl OlmMachine { event: &SyncEncryptedEvent, room_id: &RoomId, ) -> MegolmResult { - let content = match &event.content.scheme { - EncryptedEventScheme::MegolmV1AesSha2(c) => c, - _ => return Err(EventError::UnsupportedAlgorithm.into()), - }; + match &event.content.scheme { + EncryptedEventScheme::MegolmV1AesSha2(c) => { + match self.decrypt_megolm_v1_event(room_id, event, c).await { + Ok(r) => Ok(r), + Err(e) => { + if let MegolmError::MissingRoomKey = e { + // TODO log the withheld reason if we have one. + debug!( + sender = event.sender.as_str(), + room_id = room_id.as_str(), + sender_key = c.sender_key.as_str(), + session_id = c.session_id.as_str(), + "Failed to decrypt a room event, the room key is missing" + ); + } else { + warn!( + sender = event.sender.as_str(), + room_id = room_id.as_str(), + sender_key = c.sender_key.as_str(), + session_id = c.session_id.as_str(), + error =? e, + "Failed to decrypt a room event" + ); + } - let session = self - .store - .get_inbound_group_session(room_id, &content.sender_key, &content.session_id) - .await?; - // TODO check if the Olm session is wedged and re-request the key. - let session = if let Some(s) = session { - s - } else { - self.key_request_machine - .create_outgoing_key_request(room_id, &content.sender_key, &content.session_id) - .await?; - return Err(MegolmError::MissingSession); - }; - - // TODO check the message index. - // TODO check if this is from a verified device. - let (decrypted_event, _) = session.decrypt(event).await?; - - trace!("Successfully decrypted a Megolm event {:?}", decrypted_event); - - if let Ok(e) = decrypted_event.deserialize() { - let event = e.into_full_event(room_id.to_owned()); - - if let AnyRoomEvent::Message(e) = event { - self.verification_machine.receive_any_event(&e).await?; + Err(e) + } + } + } + algorithm => { + warn!( + sender = event.sender.as_str(), + room_id = room_id.as_str(), + ?algorithm, + "Received an encrypted room event with an unsupported algorithm" + ); + Err(EventError::UnsupportedAlgorithm.into()) } } - - let encryption_info = - self.get_encryption_info(&session, &event.sender, &content.device_id).await?; - - Ok(SyncRoomEvent { encryption_info: Some(encryption_info), event: decrypted_event }) } /// Update the tracked users.