From 600fd62dc5cb249599c773822a47ba676002843d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 24 Jan 2023 09:14:42 +0100 Subject: [PATCH] crypto(fix): Fail to decrypt if there's a key mismatch A mismatch between the recorded Ed25519 and Curve25519 keys of the room key and the identity keys of a Device used to be just a warning. Considering that many clients will aggressively try to hide E2EE related warnings let's upgrade this clear error into a full blown decryption failure. Co-authored-by: Denis Kasak --- crates/matrix-sdk-crypto/src/error.rs | 17 ++++++++ .../src/identities/device.rs | 35 +++++++++++---- crates/matrix-sdk-crypto/src/machine.rs | 43 +++++++++++++++++-- 3 files changed, 82 insertions(+), 13 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/error.rs b/crates/matrix-sdk-crypto/src/error.rs index ca07a696c..f469f28dd 100644 --- a/crates/matrix-sdk-crypto/src/error.rs +++ b/crates/matrix-sdk-crypto/src/error.rs @@ -88,6 +88,23 @@ pub enum MegolmError { #[error("Can't find the room key to decrypt the event")] MissingRoomKey, + /// Decryption failed because of a mismatch between the identity keys of the + /// device we received the room key from and the identity keys recorded in + /// the plaintext of the room key to-device message. + #[error( + "decryption failed because of mismatched identity keys of the sending device and those recorded in the to-device message" + )] + MismatchedIdentityKeys { + /// The Ed25519 key recorded in the room key's to-device message. + key_ed25519: Box, + /// The Ed25519 identity key of the device sending the room key. + device_ed25519: Option>, + /// The Curve25519 key recorded in the room key's to-device message. + key_curve25519: Box, + /// The Curve25519 identity key of the device sending the room key. + device_curve25519: Option>, + }, + /// The encrypted megolm message couldn't be decoded. #[error(transparent)] Decode(#[from] vodozemac::DecodeError), diff --git a/crates/matrix-sdk-crypto/src/identities/device.rs b/crates/matrix-sdk-crypto/src/identities/device.rs index 53a6bcad3..91d8cd20f 100644 --- a/crates/matrix-sdk-crypto/src/identities/device.rs +++ b/crates/matrix-sdk-crypto/src/identities/device.rs @@ -47,10 +47,11 @@ use crate::{ forwarded_room_key::ForwardedRoomKeyContent, room::encrypted::ToDeviceEncryptedEventContent, EventType, }, - DeviceKey, DeviceKeys, EventEncryptionAlgorithm, Signatures, SignedKey, SigningKey, + DeviceKey, DeviceKeys, EventEncryptionAlgorithm, Signatures, SignedKey, }, verification::VerificationMachine, - OutgoingVerificationRequest, ReadOnlyAccount, Sas, ToDeviceRequest, VerificationRequest, + MegolmError, OutgoingVerificationRequest, ReadOnlyAccount, Sas, ToDeviceRequest, + VerificationRequest, }; /// A read-only version of a `Device`. @@ -157,16 +158,32 @@ impl Device { /// An `InboundGroupSession` is exchanged between devices as an Olm /// encrypted `m.room_key` event. This method determines if this `Device` /// can be confirmed as the creator and owner of the `m.room_key`. - pub fn is_owner_of_session(&self, session: &InboundGroupSession) -> bool { + pub fn is_owner_of_session(&self, session: &InboundGroupSession) -> Result { if session.has_been_imported() { - false - } else if let Some(SigningKey::Ed25519(key)) = - session.signing_keys.get(&DeviceKeyAlgorithm::Ed25519) + Ok(false) + } else if let Some(key) = + session.signing_keys.get(&DeviceKeyAlgorithm::Ed25519).and_then(|k| k.ed25519()) { - self.ed25519_key().map(|k| k == *key).unwrap_or(false) - && self.curve25519_key().map(|k| k == session.sender_key).unwrap_or(false) + let ed25519_comparison = self.ed25519_key().map(|k| k == key); + let curve25519_comparison = self.curve25519_key().map(|k| k == session.sender_key); + + match (ed25519_comparison, curve25519_comparison) { + // If we have any of the keys but they don't turn out to match, refuse to decrypt + // instead. + (_, Some(false)) | (Some(false), _) => Err(MegolmError::MismatchedIdentityKeys { + key_ed25519: key.into(), + device_ed25519: self.ed25519_key().map(Into::into), + key_curve25519: session.sender_key().into(), + device_curve25519: self.curve25519_key().map(Into::into), + }), + // If both keys match, we have ourselves an owner. + (Some(true), Some(true)) => Ok(true), + // In the remaining cases, the device is missing at least one of the required + // identity keys, so we default to a negative answer. + _ => Ok(false), + } } else { - false + Ok(false) } } diff --git a/crates/matrix-sdk-crypto/src/machine.rs b/crates/matrix-sdk-crypto/src/machine.rs index 97db5b7a4..5f6bc8e68 100644 --- a/crates/matrix-sdk-crypto/src/machine.rs +++ b/crates/matrix-sdk-crypto/src/machine.rs @@ -1064,7 +1064,7 @@ impl OlmMachine { &self, session: &InboundGroupSession, sender: &UserId, - ) -> StoreResult<(VerificationState, Option)> { + ) -> MegolmResult<(VerificationState, Option)> { Ok( // First find the device corresponding to the Curve25519 identity // key that sent us the session (recorded upon successful @@ -1082,7 +1082,7 @@ impl OlmMachine { // // a) This is our own device, or // b) The device itself is considered to be trusted. - if device.is_owner_of_session(session) + if device.is_owner_of_session(session)? && (device.is_our_own_device() || device.is_verified()) { (VerificationState::Trusted, Some(device.device_id().to_owned())) @@ -1106,7 +1106,7 @@ impl OlmMachine { &self, session: &InboundGroupSession, sender: &UserId, - ) -> StoreResult { + ) -> MegolmResult { let (verification_state, device_id) = self.get_verification_state(session, sender).await?; let sender = sender.to_owned(); @@ -1625,7 +1625,7 @@ pub(crate) mod tests { room::encrypted::{EncryptedToDeviceEvent, ToDeviceEncryptedEventContent}, ToDeviceEvent, }, - DeviceKeys, SignedKey, + DeviceKeys, SignedKey, SigningKeys, }, utilities::json_convert, verification::tests::{outgoing_request_to_event, request_to_event}, @@ -2454,4 +2454,39 @@ pub(crate) mod tests { assert!(session.unwrap().is_none()); } + + #[async_test] + async fn room_key_with_fake_identity_keys() { + let room_id = room_id!("!test:localhost"); + let (alice, _) = get_machine_pair_with_setup_sessions().await; + let device = ReadOnlyDevice::from_machine(&alice).await; + alice.store.save_devices(&[device]).await.unwrap(); + + let (outbound, mut inbound) = + alice.account.create_group_session_pair(room_id, Default::default()).await.unwrap(); + + let fake_key = Ed25519PublicKey::from_base64("ee3Ek+J2LkkPmjGPGLhMxiKnhiX//xcqaVL4RP6EypE") + .unwrap() + .into(); + let signing_keys = SigningKeys::from([(DeviceKeyAlgorithm::Ed25519, fake_key)]); + inbound.signing_keys = signing_keys.into(); + + let content = json!({}); + let content = outbound.encrypt(content, "m.dummy").await; + alice.store.save_inbound_group_sessions(&[inbound]).await.unwrap(); + + let event = json!({ + "sender": alice.user_id(), + "event_id": "$xxxxx:example.org", + "origin_server_ts": MilliSecondsSinceUnixEpoch::now(), + "type": "m.room.encrypted", + "content": content, + }); + let event = json_convert(&event).unwrap(); + + assert_matches!( + alice.decrypt_room_event(&event, room_id).await, + Err(MegolmError::MismatchedIdentityKeys { .. }) + ); + } }