feat(crypto): Key distribution errors for Pin violations

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

@@ -69,8 +69,34 @@ pub enum OlmError {
             have a valid Olm session with us"
     )]
     MissingSession,
+    #[error(transparent)]
+    /// The room key that should be shared was not due to an error.
+    KeyDistributionError(RoomKeyDistributionError),
 }
 
+/// Depending on the sharing strategy for room keys, the distribution of the
+/// room key could fail.
+#[derive(Error, Debug)]
+pub enum RoomKeyDistributionError {
+    /// When encrypting using the IdentityBased strategy.
+    /// Will be thrown when sharing room keys when there is a new identity for a
+    /// user that has not been confirmed by the user.
+    /// Application should display identity changes to the user as soon as
+    /// possible to avoid hitting this case. If it happens the app might
+    /// just retry automatically after the identity change has been
+    /// notified, or offer option to cancel.
+    #[error("Encryption failed because there are key pinning violation, please re-pin or verify the problematic users")]
+    KeyPinningViolation(Vec<OwnedUserId>),
+
+    /// Cross-signing is required for encryption with invisible crypto
+    #[error("Encryption failed: Setup cross-signing on your account")]
+    CrossSigningNotSetup,
+    /// The current device needs to be verified when encrypting using the
+    /// IdentityBased strategy. Apps should prevent sending in the UI to
+    /// avoid hitting this case.
+    #[error("Encryption failed: Verify your device to send encrypted messages")]
+    SendingFromUnverifiedDevice,
+}
 /// Error representing a failure during a group encryption operation.
 #[derive(Error, Debug)]
 pub enum MegolmError {

crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs

@@ -24,8 +24,10 @@ use tracing::{debug, instrument, trace};
 
 use super::OutboundGroupSession;
 use crate::{
-    error::OlmResult, store::Store, types::events::room_key_withheld::WithheldCode,
-    EncryptionSettings, ReadOnlyDevice, ReadOnlyOwnUserIdentity, ReadOnlyUserIdentities,
+    error::{OlmResult, RoomKeyDistributionError},
+    store::Store,
+    types::events::room_key_withheld::WithheldCode,
+    EncryptionSettings, OlmError, ReadOnlyDevice, ReadOnlyOwnUserIdentity, ReadOnlyUserIdentities,
 };
 
 /// Strategy to collect the devices that should receive room keys for the
@@ -166,12 +168,42 @@ pub(crate) async fn collect_session_recipients(
             }
         }
         CollectStrategy::IdentityBasedStrategy => {
-            let _maybe_own_identity =
+            let maybe_own_identity =
                 store.get_user_identity(store.user_id()).await?.and_then(|i| i.into_own());
 
+            let own_verified_identity = match maybe_own_identity {
+                None => {
+                    return Err(OlmError::KeyDistributionError(
+                        RoomKeyDistributionError::CrossSigningNotSetup,
+                    ))
+                }
+                Some(identity) if !identity.is_verified() => {
+                    return Err(OlmError::KeyDistributionError(
+                        RoomKeyDistributionError::SendingFromUnverifiedDevice,
+                    ))
+                }
+                Some(identity) => identity,
+            };
+
+            // Collect all potential identity mismatch and report at the end if any.
+            let mut users_with_identity_mismatch: Vec<OwnedUserId> = Vec::default();
             for user_id in users {
-                let user_devices = store.get_readonly_devices_filtered(user_id).await?;
                 let device_owner_identity = store.get_user_identity(user_id).await?;
+                // debug!(?device_owner_identity, "Other Checking identity");
+
+                if let Some(other_identity) = device_owner_identity.as_ref().and_then(|i| i.other())
+                {
+                    if other_identity.has_pin_violation()
+                        && own_verified_identity.is_identity_signed(other_identity).is_err()
+                    {
+                        debug!(?other_identity, "Identity Mismatch detected");
+                        // Identity mismatch
+                        users_with_identity_mismatch.push(other_identity.user_id().to_owned());
+                        continue;
+                    }
+                }
+                let user_devices = store.get_readonly_devices_filtered(user_id).await?;
+
                 let recipient_devices = split_recipients_withhelds_for_user_based_on_identity(
                     user_devices,
                     &device_owner_identity,
@@ -195,6 +227,13 @@ pub(crate) async fn collect_session_recipients(
                     .extend(recipient_devices.allowed_devices);
                 withheld_devices.extend(recipient_devices.denied_devices_with_code);
             }
+
+            // Abort sharing and fail when there are identities mismatch.
+            if !users_with_identity_mismatch.is_empty() {
+                return Err(OlmError::KeyDistributionError(
+                    RoomKeyDistributionError::KeyPinningViolation(users_with_identity_mismatch),
+                ));
+            }
         }
     };
 
@@ -217,7 +256,7 @@ pub(crate) async fn collect_session_recipients(
 // blacklisted in the meantime. If it's the case the session should be rotated.
 fn should_rotate_due_to_left_device(
     user_id: &UserId,
-    future_recipients: &Vec<ReadOnlyDevice>,
+    future_recipients: &[ReadOnlyDevice],
     outbound: &OutboundGroupSession,
 ) -> bool {
     // Device IDs that should receive this session
@@ -324,7 +363,12 @@ mod tests {
 
     use std::sync::Arc;
 
-    use matrix_sdk_test::{async_test, test_json::keys_query_sets::KeyDistributionTestData};
+    use matrix_sdk_test::{
+        async_test,
+        test_json::keys_query_sets::{
+            IdentityChangeDataSet, KeyDistributionTestData, MaloIdentityChangeDataSet,
+        },
+    };
     use ruma::{events::room::history_visibility::HistoryVisibility, room_id, TransactionId};
 
     use crate::{
@@ -333,7 +377,7 @@ mod tests {
             group_sessions::share_strategy::collect_session_recipients, CollectStrategy,
         },
         types::events::room_key_withheld::WithheldCode,
-        CrossSigningKeyExport, EncryptionSettings, OlmMachine,
+        CrossSigningKeyExport, EncryptionSettings, OlmError, OlmMachine,
     };
 
     async fn set_up_test_machine() -> OlmMachine {
@@ -571,6 +615,172 @@ mod tests {
         assert_eq!(code, &WithheldCode::Unauthorised);
     }
 
+    #[async_test]
+    async fn test_share_identity_strategy_no_cross_signing() {
+        let machine: OlmMachine = OlmMachine::new(
+            KeyDistributionTestData::me_id(),
+            KeyDistributionTestData::me_device_id(),
+        )
+        .await;
+
+        let keys_query = KeyDistributionTestData::dan_keys_query_response();
+        let txn_id = TransactionId::new();
+        machine.mark_request_as_sent(&txn_id, &keys_query).await.unwrap();
+
+        let fake_room_id = room_id!("!roomid:localhost");
+
+        let strategy = CollectStrategy::new_identity_based();
+
+        let encryption_settings =
+            EncryptionSettings { sharing_strategy: strategy.clone(), ..Default::default() };
+
+        let request_result = machine
+            .share_room_key(
+                fake_room_id,
+                vec![KeyDistributionTestData::dan_id()].into_iter(),
+                encryption_settings.clone(),
+            )
+            .await;
+
+        assert!(request_result.is_err());
+        let err = request_result.unwrap_err();
+        match err {
+            OlmError::KeyDistributionError(
+                crate::error::RoomKeyDistributionError::CrossSigningNotSetup,
+            ) => {
+                // ok
+            }
+            _ => panic!("Unexpected share error"),
+        }
+
+        // Let's now have cross-signing, but the identity is not trusted
+        let keys_query = KeyDistributionTestData::me_keys_query_response();
+        let txn_id = TransactionId::new();
+        machine.mark_request_as_sent(&txn_id, &keys_query).await.unwrap();
+
+        let request_result = machine
+            .share_room_key(
+                fake_room_id,
+                vec![KeyDistributionTestData::dan_id()].into_iter(),
+                encryption_settings.clone(),
+            )
+            .await;
+
+        assert!(request_result.is_err());
+        let err = request_result.unwrap_err();
+        match err {
+            OlmError::KeyDistributionError(
+                crate::error::RoomKeyDistributionError::SendingFromUnverifiedDevice,
+            ) => {
+                // ok
+            }
+            _ => panic!("Unexpected share error"),
+        }
+
+        // If we are verified it should then work
+        machine
+            .import_cross_signing_keys(CrossSigningKeyExport {
+                master_key: KeyDistributionTestData::MASTER_KEY_PRIVATE_EXPORT.to_owned().into(),
+                self_signing_key: KeyDistributionTestData::SELF_SIGNING_KEY_PRIVATE_EXPORT
+                    .to_owned()
+                    .into(),
+                user_signing_key: KeyDistributionTestData::USER_SIGNING_KEY_PRIVATE_EXPORT
+                    .to_owned()
+                    .into(),
+            })
+            .await
+            .unwrap();
+
+        let request_result = machine
+            .share_room_key(
+                fake_room_id,
+                vec![KeyDistributionTestData::dan_id()].into_iter(),
+                encryption_settings.clone(),
+            )
+            .await;
+
+        assert!(request_result.is_ok());
+    }
+
+    #[async_test]
+    async fn test_share_identity_strategy_report_pinning_violation() {
+        let machine: OlmMachine = OlmMachine::new(
+            KeyDistributionTestData::me_id(),
+            KeyDistributionTestData::me_device_id(),
+        )
+        .await;
+
+        machine.bootstrap_cross_signing(false).await.unwrap();
+
+        let keys_query = IdentityChangeDataSet::key_query_with_identity_a();
+        let txn_id = TransactionId::new();
+        machine.mark_request_as_sent(&txn_id, &keys_query).await.unwrap();
+
+        let keys_query = MaloIdentityChangeDataSet::initial_key_query();
+        let txn_id = TransactionId::new();
+        machine.mark_request_as_sent(&txn_id, &keys_query).await.unwrap();
+
+        // Simulate pinning violation
+        let keys_query = IdentityChangeDataSet::key_query_with_identity_b();
+        let txn_id = TransactionId::new();
+        machine.mark_request_as_sent(&txn_id, &keys_query).await.unwrap();
+
+        let keys_query = MaloIdentityChangeDataSet::updated_key_query();
+        let txn_id = TransactionId::new();
+        machine.mark_request_as_sent(&txn_id, &keys_query).await.unwrap();
+
+        let fake_room_id = room_id!("!roomid:localhost");
+        let strategy = CollectStrategy::new_identity_based();
+
+        let encryption_settings =
+            EncryptionSettings { sharing_strategy: strategy.clone(), ..Default::default() };
+
+        let request_result = machine
+            .share_room_key(
+                fake_room_id,
+                vec![IdentityChangeDataSet::user_id(), MaloIdentityChangeDataSet::user_id()]
+                    .into_iter(),
+                encryption_settings.clone(),
+            )
+            .await;
+
+        assert!(request_result.is_err());
+        let err = request_result.unwrap_err();
+        match err {
+            OlmError::KeyDistributionError(
+                crate::error::RoomKeyDistributionError::KeyPinningViolation(affected_users),
+            ) => {
+                assert_eq!(2, affected_users.len());
+
+                // resolve by pinning the new identity
+                for user_id in affected_users {
+                    machine
+                        .get_identity(&user_id, None)
+                        .await
+                        .unwrap()
+                        .unwrap()
+                        .other()
+                        .unwrap()
+                        .pin_current_master_key()
+                        .await
+                        .unwrap()
+                }
+
+                let request_result = machine
+                    .share_room_key(
+                        fake_room_id,
+                        // vec![KeyDistributionTestData::dan_id()].into_iter(),
+                        vec![IdentityChangeDataSet::user_id()].into_iter(),
+                        encryption_settings.clone(),
+                    )
+                    .await;
+
+                assert!(request_result.is_ok());
+            }
+            _ => panic!("Unexpected share error"),
+        }
+    }
+
     #[async_test]
     async fn test_should_rotate_based_on_visibility() {
         let machine = set_up_test_machine().await;