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 <dkasak@termina.org.uk>

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<Ed25519PublicKey>,
+        /// The Ed25519 identity key of the device sending the room key.
+        device_ed25519: Option<Box<Ed25519PublicKey>>,
+        /// The Curve25519 key recorded in the room key's to-device message.
+        key_curve25519: Box<Curve25519PublicKey>,
+        /// The Curve25519 identity key of the device sending the room key.
+        device_curve25519: Option<Box<Curve25519PublicKey>>,
+    },
+
     /// The encrypted megolm message couldn't be decoded.
     #[error(transparent)]
     Decode(#[from] vodozemac::DecodeError),

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<bool, MegolmError> {
         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)
         }
     }
 

crates/matrix-sdk-crypto/src/machine.rs

@@ -1064,7 +1064,7 @@ impl OlmMachine {
         &self,
         session: &InboundGroupSession,
         sender: &UserId,
-    ) -> StoreResult<(VerificationState, Option<OwnedDeviceId>)> {
+    ) -> MegolmResult<(VerificationState, Option<OwnedDeviceId>)> {
         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<EncryptionInfo> {
+    ) -> MegolmResult<EncryptionInfo> {
         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 { .. })
+        );
+    }
 }