From 3f3f5998778fd6098c8b3cbdbfda26005c02f1c7 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Wed, 6 Sep 2023 10:03:15 +0100 Subject: [PATCH] Add `OlmMachine::get_room_event_encryption_info` (#2510) Since the verification status of an event can change, we need to be able to refetch the verification status without doing the whole decryption dance. Hence, we expose a new `get_room_event_encryption_info` method. --- crates/matrix-sdk-crypto/CHANGELOG.md | 2 + crates/matrix-sdk-crypto/src/machine.rs | 199 +++++++++++++++++------- 2 files changed, 145 insertions(+), 56 deletions(-) diff --git a/crates/matrix-sdk-crypto/CHANGELOG.md b/crates/matrix-sdk-crypto/CHANGELOG.md index b993597e4..f774cb117 100644 --- a/crates/matrix-sdk-crypto/CHANGELOG.md +++ b/crates/matrix-sdk-crypto/CHANGELOG.md @@ -61,3 +61,5 @@ - Change the returned success value type of `BackupMachine::backup` from `OutgoingRequest` to `(OwnedTransactionId, KeysBackupRequest)`. + +- Expose new `OlmMachine::get_room_event_encryption_info` method. diff --git a/crates/matrix-sdk-crypto/src/machine.rs b/crates/matrix-sdk-crypto/src/machine.rs index 66f826e15..389a40381 100644 --- a/crates/matrix-sdk-crypto/src/machine.rs +++ b/crates/matrix-sdk-crypto/src/machine.rs @@ -1320,63 +1320,85 @@ impl OlmMachine { }) } + async fn get_megolm_encryption_info( + &self, + room_id: &RoomId, + event: &EncryptedEvent, + content: &SupportedEventEncryptionSchemes<'_>, + ) -> MegolmResult { + let session = + self.get_inbound_group_session_or_error(room_id, content.session_id()).await?; + self.get_encryption_info(&session, &event.sender).await + } + async fn decrypt_megolm_events( &self, room_id: &RoomId, event: &EncryptedEvent, content: &SupportedEventEncryptionSchemes<'_>, ) -> MegolmResult { - if let Some(session) = - self.store().get_inbound_group_session(room_id, content.session_id()).await? - { - // This function is only ever called by decrypt_room_event, so - // room_id, sender, algorithm and session_id are recorded already - // - // While we already record the sender key in some cases from the event, the - // sender key in the event is deprecated, so let's record it now. - tracing::Span::current().record("sender_key", debug(session.sender_key())); + let session = + self.get_inbound_group_session_or_error(room_id, content.session_id()).await?; - let result = session.decrypt(event).await; - match result { - Ok((decrypted_event, _)) => { - let encryption_info = self.get_encryption_info(&session, &event.sender).await?; - Ok(TimelineEvent { - encryption_info: Some(encryption_info), - event: decrypted_event, - push_actions: None, - }) - } - Err(error) => Err( - if let MegolmError::Decryption(DecryptionError::UnknownMessageIndex(_, _)) = - error - { - let withheld_code = self - .inner - .store - .get_withheld_info(room_id, content.session_id()) - .await? - .map(|e| e.content.withheld_code()); + // This function is only ever called by decrypt_room_event, so + // room_id, sender, algorithm and session_id are recorded already + // + // While we already record the sender key in some cases from the event, the + // sender key in the event is deprecated, so let's record it now. + tracing::Span::current().record("sender_key", debug(session.sender_key())); - if withheld_code.is_some() { - // Partially withheld, report with a withheld code if we have one. - MegolmError::MissingRoomKey(withheld_code) - } else { - error - } + let result = session.decrypt(event).await; + match result { + Ok((decrypted_event, _)) => { + let encryption_info = self.get_encryption_info(&session, &event.sender).await?; + Ok(TimelineEvent { + encryption_info: Some(encryption_info), + event: decrypted_event, + push_actions: None, + }) + } + Err(error) => Err( + if let MegolmError::Decryption(DecryptionError::UnknownMessageIndex(_, _)) = error { + let withheld_code = self + .inner + .store + .get_withheld_info(room_id, content.session_id()) + .await? + .map(|e| e.content.withheld_code()); + + if withheld_code.is_some() { + // Partially withheld, report with a withheld code if we have one. + MegolmError::MissingRoomKey(withheld_code) } else { error - }, - ), - } - } else { - let withheld_code = self - .inner - .store - .get_withheld_info(room_id, content.session_id()) - .await? - .map(|e| e.content.withheld_code()); + } + } else { + error + }, + ), + } + } - Err(MegolmError::MissingRoomKey(withheld_code)) + /// Attempt to retrieve an inbound group session from the store. + /// + /// If the session is not found, checks for withheld reports, and returns a + /// [`MegolmError::MissingRoomKey`] error. + async fn get_inbound_group_session_or_error( + &self, + room_id: &RoomId, + session_id: &str, + ) -> MegolmResult { + match self.store().get_inbound_group_session(room_id, session_id).await? { + Some(session) => Ok(session), + None => { + let withheld_code = self + .inner + .store + .get_withheld_info(room_id, session_id) + .await? + .map(|e| e.content.withheld_code()); + Err(MegolmError::MissingRoomKey(withheld_code)) + } } } @@ -1437,6 +1459,37 @@ impl OlmMachine { result } + /// Get encryption info for a decrypted timeline event. + /// + /// This recalculates the [`EncryptionInfo`] data that is returned by + /// [`OlmMachine::decrypt_room_event`], based on the current + /// verification status of the sender, etc. + /// + /// Returns an error for an unencrypted event. + /// + /// # Arguments + /// + /// * `event` - The event to get information for. + /// * `room_id` - The ID of the room where the event was sent to. + pub async fn get_room_event_encryption_info( + &self, + event: &Raw, + room_id: &RoomId, + ) -> MegolmResult { + let event = event.deserialize()?; + + let content: SupportedEventEncryptionSchemes<'_> = match &event.content.scheme { + RoomEventEncryptionScheme::MegolmV1AesSha2(c) => c.into(), + #[cfg(feature = "experimental-algorithms")] + RoomEventEncryptionScheme::MegolmV2AesSha2(c) => c.into(), + RoomEventEncryptionScheme::Unknown(_) => { + return Err(EventError::UnsupportedAlgorithm.into()); + } + }; + + self.get_megolm_encryption_info(room_id, &event, &content).await + } + /// Update the list of tracked users. /// /// The OlmMachine maintains a list of users whose devices we are keeping @@ -2780,6 +2833,14 @@ pub(crate) mod tests { assert_shield!(encryption_info, Red, Red); + // get_room_event_encryption_info should return the same information + let encryption_info = bob.get_room_event_encryption_info(&event, room_id).await.unwrap(); + assert_eq!( + VerificationState::Unverified(VerificationLevel::UnsignedDevice), + encryption_info.verification_state + ); + assert_shield!(encryption_info, Red, Red); + // Local trust state has no effect bob.get_device(alice.user_id(), alice_device_id(), None) .await @@ -2787,8 +2848,7 @@ pub(crate) mod tests { .unwrap() .set_trust_state(LocalTrust::Verified); - let encryption_info = - bob.decrypt_room_event(&event, room_id).await.unwrap().encryption_info.unwrap(); + let encryption_info = bob.get_room_event_encryption_info(&event, room_id).await.unwrap(); assert_eq!( VerificationState::Unverified(VerificationLevel::UnsignedDevice), @@ -2803,8 +2863,7 @@ pub(crate) mod tests { assert_matches!(alice_id_from_bob, Some(UserIdentities::Other(_))); // we setup cross signing but nothing is signed yet - let encryption_info = - bob.decrypt_room_event(&event, room_id).await.unwrap().encryption_info.unwrap(); + let encryption_info = bob.get_room_event_encryption_info(&event, room_id).await.unwrap(); assert_eq!( VerificationState::Unverified(VerificationLevel::UnsignedDevice), @@ -2815,8 +2874,7 @@ pub(crate) mod tests { // Let alice sign her device sign_alice_device_for_machine(&alice, &bob).await; - let encryption_info = - bob.decrypt_room_event(&event, room_id).await.unwrap().encryption_info.unwrap(); + let encryption_info = bob.get_room_event_encryption_info(&event, room_id).await.unwrap(); assert_eq!( VerificationState::Unverified(VerificationLevel::UnverifiedIdentity), @@ -2826,8 +2884,7 @@ pub(crate) mod tests { assert_shield!(encryption_info, Red, None); mark_alice_identity_as_verified(&alice, &bob).await; - let encryption_info = - bob.decrypt_room_event(&event, room_id).await.unwrap().encryption_info.unwrap(); + let encryption_info = bob.get_room_event_encryption_info(&event, room_id).await.unwrap(); assert_eq!(VerificationState::Verified, encryption_info.verification_state); assert_shield!(encryption_info, None, None); @@ -2835,8 +2892,7 @@ pub(crate) mod tests { let imported = InboundGroupSession::from_export(&export).unwrap(); bob.store().save_inbound_group_sessions(&[imported]).await.unwrap(); - let encryption_info = - bob.decrypt_room_event(&event, room_id).await.unwrap().encryption_info.unwrap(); + let encryption_info = bob.get_room_event_encryption_info(&event, room_id).await.unwrap(); // As soon as the key source is unsafe the verification state (or existence) of // the device is meaningless @@ -2850,6 +2906,37 @@ pub(crate) mod tests { assert_shield!(encryption_info, Red, Grey); } + /// Test what happens when we feed an unencrypted event into the decryption + /// functions + #[async_test] + async fn test_decrypt_unencrypted_event() { + let (bob, _) = get_prepared_machine(user_id(), false).await; + let room_id = room_id!("!test:example.org"); + + let event = json!({ + "event_id": "$xxxxx:example.org", + "origin_server_ts": MilliSecondsSinceUnixEpoch::now(), + "sender": user_id(), + // it's actually the lack of an `algorithm` that upsets it, rather than the event type. + "type": "m.room.encrypted", + "content": RoomMessageEventContent::text_plain("plain"), + }); + + let event = json_convert(&event).unwrap(); + + // decrypt_room_event should return an error + assert_matches!( + bob.decrypt_room_event(&event, room_id).await, + Err(MegolmError::JsonError(..)) + ); + + // so should get_room_event_encryption_info + assert_matches!( + bob.get_room_event_encryption_info(&event, room_id).await, + Err(MegolmError::JsonError(..)) + ); + } + async fn setup_cross_signing_for_machine(alice: &OlmMachine, bob: &OlmMachine) { let (alice_upload_signing, _) = alice.bootstrap_cross_signing(false).await.expect("Expect Alice x-signing key request");