diff --git a/crates/matrix-sdk-crypto/src/machine/mod.rs b/crates/matrix-sdk-crypto/src/machine/mod.rs index 95a435a25dd..c2201a169ea 100644 --- a/crates/matrix-sdk-crypto/src/machine/mod.rs +++ b/crates/matrix-sdk-crypto/src/machine/mod.rs @@ -857,9 +857,9 @@ impl OlmMachine { #[instrument( skip_all, - // This function is only ever called by add_room_key via - // handle_decrypted_to_device_event, so sender, sender_key, and algorithm are - // already recorded. + // This function is only ever called by add_room_key via + // handle_decrypted_to_device_event, so sender, sender_key, and algorithm are + // already recorded. fields(room_id = ? content.room_id, session_id) )] async fn handle_key( @@ -888,17 +888,21 @@ impl OlmMachine { session.sender_data = sender_data; - if self.store().compare_group_session(&session).await? == SessionOrdering::Better { - info!("Received a new megolm room key"); + match self.store().compare_group_session(&session).await? { + SessionOrdering::Better => { + info!("Received a new megolm room key"); - Ok(Some(session)) - } else { - warn!( - "Received a megolm room key that we already have a better version of, \ - discarding", - ); - - Ok(None) + Ok(Some(session)) + } + comparison_result => { + warn!( + ?comparison_result, + "Received a megolm room key that we already have a better version \ + of, discarding" + ); + + Ok(None) + } } } Err(e) => { diff --git a/crates/matrix-sdk-crypto/src/olm/group_sessions/inbound.rs b/crates/matrix-sdk-crypto/src/olm/group_sessions/inbound.rs index f43e013e442..c42d7ba889d 100644 --- a/crates/matrix-sdk-crypto/src/olm/group_sessions/inbound.rs +++ b/crates/matrix-sdk-crypto/src/olm/group_sessions/inbound.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::{ + cmp::Ordering, fmt, ops::Deref, sync::{ @@ -375,8 +376,8 @@ impl InboundGroupSession { self.imported } - /// Check if the `InboundGroupSession` is better than the given other - /// `InboundGroupSession` + /// Check if the [`InboundGroupSession`] is better than the given other + /// [`InboundGroupSession`] pub async fn compare(&self, other: &InboundGroupSession) -> SessionOrdering { // If this is the same object the ordering is the same, we can't compare because // we would deadlock while trying to acquire the same lock twice. @@ -389,8 +390,18 @@ impl InboundGroupSession { { SessionOrdering::Unconnected } else { - let mut other = other.inner.lock().await; - self.inner.lock().await.compare(&mut other) + let mut other_inner = other.inner.lock().await; + + match self.inner.lock().await.compare(&mut other_inner) { + SessionOrdering::Equal => { + match self.sender_data.compare_trust_level(&other.sender_data) { + Ordering::Less => SessionOrdering::Worse, + Ordering::Equal => SessionOrdering::Equal, + Ordering::Greater => SessionOrdering::Better, + } + } + result => result, + } } } @@ -648,7 +659,7 @@ mod tests { }; use crate::{ - olm::{InboundGroupSession, SenderData}, + olm::{InboundGroupSession, KnownSenderData, SenderData}, types::EventEncryptionAlgorithm, Account, }; @@ -868,6 +879,29 @@ mod tests { assert_eq!(inbound.compare(©).await, SessionOrdering::Unconnected); } + #[async_test] + async fn test_session_comparison_sender_data() { + let alice = Account::with_device_id(alice_id(), alice_device_id()); + let room_id = room_id!("!test:localhost"); + + let (_, mut inbound) = alice.create_group_session_pair_with_defaults(room_id).await; + + let sender_data = SenderData::SenderVerified(KnownSenderData { + user_id: alice.user_id().into(), + device_id: Some(alice.device_id().into()), + master_key: alice.identity_keys().ed25519.into(), + }); + + let mut better = InboundGroupSession::from_pickle(inbound.pickle().await).unwrap(); + better.sender_data = sender_data.clone(); + + assert_eq!(inbound.compare(&better).await, SessionOrdering::Worse); + assert_eq!(better.compare(&inbound).await, SessionOrdering::Better); + + inbound.sender_data = sender_data; + assert_eq!(better.compare(&inbound).await, SessionOrdering::Equal); + } + fn create_session_key() -> SessionKey { SessionKey::from_base64( "\