crypto: Remove private cross signing keys if we detect that they changed

matrix_sdk_crypto/src/identities/manager.rs

@@ -24,7 +24,7 @@ use ruma::{
     api::client::r0::keys::get_keys::Response as KeysQueryResponse, encryption::DeviceKeys,
     DeviceId, DeviceIdBox, UserId,
 };
-use tracing::{trace, warn};
+use tracing::{info, trace, warn};
 
 use crate::{
     error::OlmResult,
@@ -32,6 +32,7 @@ use crate::{
         MasterPubkey, ReadOnlyDevice, ReadOnlyOwnUserIdentity, ReadOnlyUserIdentities,
         ReadOnlyUserIdentity, SelfSigningPubkey, UserSigningPubkey,
     },
+    olm::PrivateCrossSigningIdentity,
     requests::KeysQueryRequest,
     store::{Changes, DeviceChanges, IdentityChanges, Result as StoreResult, Store},
 };
@@ -75,11 +76,13 @@ impl IdentityManager {
     ) -> OlmResult<(DeviceChanges, IdentityChanges)> {
         let changed_devices =
             self.handle_devices_from_key_query(response.device_keys.clone()).await?;
-        let changed_identities = self.handle_cross_singing_keys(response).await?;
+        let (changed_identities, cross_signing_identity) =
+            self.handle_cross_singing_keys(response).await?;
 
         let changes = Changes {
             identities: changed_identities.clone(),
             devices: changed_devices.clone(),
+            private_identity: cross_signing_identity,
             ..Default::default()
         };
 
@@ -234,8 +237,9 @@ impl IdentityManager {
     async fn handle_cross_singing_keys(
         &self,
         response: &KeysQueryResponse,
-    ) -> StoreResult<IdentityChanges> {
+    ) -> StoreResult<(IdentityChanges, Option<PrivateCrossSigningIdentity>)> {
         let mut changes = IdentityChanges::default();
+        let mut changed_identity = None;
 
         for (user_id, master_key) in &response.master_keys {
             let master_key = MasterPubkey::from(master_key);
@@ -243,7 +247,10 @@ impl IdentityManager {
             let self_signing = if let Some(s) = response.self_signing_keys.get(user_id) {
                 SelfSigningPubkey::from(s)
             } else {
-                warn!("User identity for user {} didn't contain a self signing pubkey", user_id);
+                warn!(
+                    user_id = user_id.as_str(),
+                    "A user identity didn't contain a self signing pubkey"
+                );
                 continue;
             };
 
@@ -255,9 +262,9 @@ impl IdentityManager {
                             UserSigningPubkey::from(s)
                         } else {
                             warn!(
-                                "User identity for our own user {} didn't \
+                                user_id = user_id.as_str(),
-                                  contain a user signing pubkey",
+                                "User identity for our own user didn't \
-                                user_id
+                                contain a user signing pubkey",
                             );
                             continue;
                         };
@@ -277,8 +284,8 @@ impl IdentityManager {
                         || user_signing.user_id() != user_id
                     {
                         warn!(
-                            "User id mismatch in one of the cross signing keys for user {}",
+                            user_id = user_id.as_str(),
-                            user_id
+                            "User ID mismatch in one of the cross signing keys",
                         );
                         continue;
                     }
@@ -287,14 +294,14 @@ impl IdentityManager {
                         .map(|i| (ReadOnlyUserIdentities::Own(i), true))
                 } else {
                     warn!(
-                        "User identity for our own user {} didn't contain a \
+                        user_id = user_id.as_str(),
+                        "User identity for our own user didn't contain a \
                         user signing pubkey",
-                        user_id
                     );
                     continue;
                 }
             } else if master_key.user_id() != user_id || self_signing.user_id() != user_id {
-                warn!("User id mismatch in one of the cross signing keys for user {}", user_id);
+                warn!(user = user_id.as_str(), "User ID mismatch in one of the cross signing keys",);
                 continue;
             } else {
                 ReadOnlyUserIdentity::new(master_key, self_signing)
@@ -303,21 +310,38 @@ impl IdentityManager {
 
             match result {
                 Ok((i, new)) => {
-                    trace!("Updated or created new user identity for {}: {:?}", user_id, i);
+                    if let Some(identity) = i.own() {
+                        let private_identity = self.store.private_identity();
+                        let private_identity = private_identity.lock().await;
+
+                        let result = private_identity.clear_if_differs(identity).await;
+
+                        if result.any_cleared() {
+                            changed_identity = Some((&*private_identity).clone());
+                            info!(cleared =? result, "Removed some or all of our private cross signing keys");
+                        }
+                    }
+
                     if new {
+                        trace!(user_id = user_id.as_str(), identity =? i, "Created new user identity");
                         changes.new.push(i);
                     } else {
+                        trace!(user_id = user_id.as_str(), identity =? i, "Updated a user identity");
                         changes.changed.push(i);
                     }
                 }
                 Err(e) => {
-                    warn!("Couldn't update or create new user identity for {}: {:?}", user_id, e);
+                    warn!(
+                        user_id = user_id.as_str(),
+                        error =? e,
+                        "Couldn't update or create new user identity for"
+                    );
                     continue;
                 }
             }
         }
 
-        Ok(changes)
+        Ok((changes, changed_identity))
     }
 
     /// Get a key query request if one is needed.

matrix_sdk_crypto/src/olm/signing/mod.rs

@@ -34,9 +34,13 @@ use serde::{Deserialize, Serialize};
 use serde_json::Error as JsonError;
 
 use crate::{
-    error::SignatureError, identities::MasterPubkey, requests::UploadSigningKeysRequest,
+    error::SignatureError,
-    store::SecretImportError, utilities::decode, OwnUserIdentity, ReadOnlyAccount, ReadOnlyDevice,
+    identities::{MasterPubkey, SelfSigningPubkey, UserSigningPubkey},
-    ReadOnlyOwnUserIdentity, ReadOnlyUserIdentity,
+    requests::UploadSigningKeysRequest,
+    store::SecretImportError,
+    utilities::decode,
+    OwnUserIdentity, ReadOnlyAccount, ReadOnlyDevice, ReadOnlyOwnUserIdentity,
+    ReadOnlyUserIdentity,
 };
 
 /// Private cross signing identity.
@@ -57,6 +61,26 @@ pub struct PrivateCrossSigningIdentity {
     pub(crate) self_signing_key: Arc<Mutex<Option<SelfSigning>>>,
 }
 
+/// A struct containing information if any of our cross signing keys were
+/// cleared because the public keys differ from the keys that are uploaded to
+/// the server.
+#[derive(Debug, Clone)]
+pub struct ClearResult {
+    /// Was the master key cleared.
+    master_cleared: bool,
+    /// Was the self-signing key cleared.
+    self_signing_cleared: bool,
+    /// Was the user-signing key cleared.
+    user_signing_cleared: bool,
+}
+
+impl ClearResult {
+    /// Did we clear any of the private cross signing keys.
+    pub fn any_cleared(&self) -> bool {
+        self.master_cleared || self.self_signing_cleared || self.user_signing_cleared
+    }
+}
+
 /// The pickled version of a `PrivateCrossSigningIdentity`.
 ///
 /// Can be used to store the identity.
@@ -137,6 +161,16 @@ impl PrivateCrossSigningIdentity {
         self.master_key.lock().await.as_ref().map(|m| m.public_key.to_owned())
     }
 
+    /// Get the public part of the self-signing key, if we have one.
+    pub async fn self_signing_public_key(&self) -> Option<SelfSigningPubkey> {
+        self.self_signing_key.lock().await.as_ref().map(|k| k.public_key.to_owned())
+    }
+
+    /// Get the public part of the user-signing key, if we have one.
+    pub async fn user_signing_public_key(&self) -> Option<UserSigningPubkey> {
+        self.user_signing_key.lock().await.as_ref().map(|k| k.public_key.to_owned())
+    }
+
     /// Export the seed of the private cross signing key
     ///
     /// The exported seed will be encoded as unpadded base64.
@@ -190,7 +224,6 @@ impl PrivateCrossSigningIdentity {
 
             if public_identity.master_key() == &master.public_key {
                 Ok(Some(master))
-                // *self.master_key.lock().await = Some(master);
             } else {
                 Err(SecretImportError::MissmatchedPublicKeys)
             }
@@ -203,7 +236,6 @@ impl PrivateCrossSigningIdentity {
             let subkey = UserSigning::from_seed(self.user_id().clone(), seed);
 
             if public_identity.user_signing_key() == &subkey.public_key {
-                // *self.user_signing_key.lock().await = Some(subkey);
                 Ok(Some(subkey))
             } else {
                 Err(SecretImportError::MissmatchedPublicKeys)
@@ -217,7 +249,6 @@ impl PrivateCrossSigningIdentity {
             let subkey = SelfSigning::from_seed(self.user_id().clone(), seed);
 
             if public_identity.self_signing_key() == &subkey.public_key {
-                // *self.self_signing_key.lock().await = Some(subkey);
                 Ok(Some(subkey))
             } else {
                 Err(SecretImportError::MissmatchedPublicKeys)
@@ -241,6 +272,58 @@ impl PrivateCrossSigningIdentity {
         Ok(())
     }
 
+    /// Remove our private cross signing key if the public keys differ from
+    /// what's found in the `ReadOnlyOwnUserIdentity`.
+    pub(crate) async fn clear_if_differs(
+        &self,
+        public_identity: &ReadOnlyOwnUserIdentity,
+    ) -> ClearResult {
+        let result = self.get_public_identity_diff(public_identity).await;
+
+        if result.master_cleared {
+            *self.master_key.lock().await = None;
+        }
+
+        if result.user_signing_cleared {
+            *self.user_signing_key.lock().await = None;
+        }
+
+        if result.self_signing_cleared {
+            *self.self_signing_key.lock().await = None;
+        }
+
+        result
+    }
+
+    async fn get_public_identity_diff(
+        &self,
+        public_identity: &ReadOnlyOwnUserIdentity,
+    ) -> ClearResult {
+        let master_differs = self
+            .master_public_key()
+            .await
+            .map(|master| &master != public_identity.master_key())
+            .unwrap_or(false);
+
+        let user_signing_differs = self
+            .user_signing_public_key()
+            .await
+            .map(|subkey| &subkey != public_identity.user_signing_key())
+            .unwrap_or(false);
+
+        let self_signing_differs = self
+            .self_signing_public_key()
+            .await
+            .map(|subkey| &subkey != public_identity.self_signing_key())
+            .unwrap_or(false);
+
+        ClearResult {
+            master_cleared: master_differs,
+            user_signing_cleared: user_signing_differs,
+            self_signing_cleared: self_signing_differs,
+        }
+    }
+
     /// Get the names of the secrets we are missing.
     pub(crate) async fn get_missing_secrets(&self) -> Vec<SecretName> {
         let mut missing = Vec::new();
@@ -282,6 +365,7 @@ impl PrivateCrossSigningIdentity {
             .ok_or(SignatureError::MissingSigningKey)?
             .public_key
             .clone();
+
         let self_signing = self
             .self_signing_key
             .lock()
@@ -290,6 +374,7 @@ impl PrivateCrossSigningIdentity {
             .ok_or(SignatureError::MissingSigningKey)?
             .public_key
             .clone();
+
         let user_signing = self
             .user_signing_key
             .lock()
@@ -298,6 +383,7 @@ impl PrivateCrossSigningIdentity {
             .ok_or(SignatureError::MissingSigningKey)?
             .public_key
             .clone();
+
         let identity = ReadOnlyOwnUserIdentity::new(master, self_signing, user_signing)?;
         identity.mark_as_verified();
 
@@ -506,23 +592,18 @@ impl PrivateCrossSigningIdentity {
     ) -> Result<Self, SigningError> {
         let signings: PickledSignings = serde_json::from_str(&pickle.pickle)?;
 
-        let master = if let Some(m) = signings.master_key {
+        let master =
-            Some(MasterSigning::from_pickle(m, pickle_key)?)
+            signings.master_key.map(|m| MasterSigning::from_pickle(m, pickle_key)).transpose()?;
-        } else {
-            None
-        };
 
-        let self_signing = if let Some(s) = signings.self_signing_key {
+        let self_signing = signings
-            Some(SelfSigning::from_pickle(s, pickle_key)?)
+            .self_signing_key
-        } else {
+            .map(|s| SelfSigning::from_pickle(s, pickle_key))
-            None
+            .transpose()?;
-        };
 
-        let user_signing = if let Some(u) = signings.user_signing_key {
+        let user_signing = signings
-            Some(UserSigning::from_pickle(u, pickle_key)?)
+            .user_signing_key
-        } else {
+            .map(|s| UserSigning::from_pickle(s, pickle_key))
-            None
+            .transpose()?;
-        };
 
         Ok(Self {
             user_id: Arc::new(pickle.user_id),
@@ -537,13 +618,13 @@ impl PrivateCrossSigningIdentity {
     /// identity.
     pub(crate) async fn as_upload_request(&self) -> UploadSigningKeysRequest {
         let master_key =
-            self.master_key.lock().await.as_ref().cloned().map(|k| k.public_key.into());
+            self.master_key.lock().await.as_ref().map(|k| k.public_key.to_owned().into());
 
         let user_signing_key =
-            self.user_signing_key.lock().await.as_ref().cloned().map(|k| k.public_key.into());
+            self.user_signing_key.lock().await.as_ref().map(|k| k.public_key.to_owned().into());
 
         let self_signing_key =
-            self.self_signing_key.lock().await.as_ref().cloned().map(|k| k.public_key.into());
+            self.self_signing_key.lock().await.as_ref().map(|k| k.public_key.to_owned().into());
 
         UploadSigningKeysRequest { master_key, self_signing_key, user_signing_key }
     }

matrix_sdk_crypto/src/store/mod.rs

@@ -202,6 +202,10 @@ impl Store {
         self.identity.lock().await.reset().await;
     }
 
+    pub fn private_identity(&self) -> Arc<Mutex<PrivateCrossSigningIdentity>> {
+        self.identity.clone()
+    }
+
     pub async fn save_sessions(&self, sessions: &[Session]) -> Result<()> {
         let changes = Changes { sessions: sessions.to_vec(), ..Default::default() };