From 8c9e93e9dedc6969a1e675f226c2e1b45e1ff275 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 4 Oct 2024 12:03:04 +0100 Subject: [PATCH 1/2] crypto: Expose a way to pin a user's identity --- bindings/matrix-sdk-ffi/CHANGELOG.md | 1 + bindings/matrix-sdk-ffi/src/encryption.rs | 44 +++++++++++++++++++ .../matrix-sdk-crypto/src/identities/user.rs | 42 +++++++++++++++++- crates/matrix-sdk/CHANGELOG.md | 1 + .../src/encryption/identities/users.rs | 18 ++++++++ .../src/room/identity_status_changes.rs | 5 ++- 6 files changed, 108 insertions(+), 3 deletions(-) diff --git a/bindings/matrix-sdk-ffi/CHANGELOG.md b/bindings/matrix-sdk-ffi/CHANGELOG.md index 44eaaa65ef..55a9474eb6 100644 --- a/bindings/matrix-sdk-ffi/CHANGELOG.md +++ b/bindings/matrix-sdk-ffi/CHANGELOG.md @@ -31,4 +31,5 @@ Breaking changes: Additions: +- Add `Encryption::get_user_identity` - Add `ClientBuilder::room_key_recipient_strategy` diff --git a/bindings/matrix-sdk-ffi/src/encryption.rs b/bindings/matrix-sdk-ffi/src/encryption.rs index 34a0e79494..501097483f 100644 --- a/bindings/matrix-sdk-ffi/src/encryption.rs +++ b/bindings/matrix-sdk-ffi/src/encryption.rs @@ -410,6 +410,50 @@ impl Encryption { pub async fn wait_for_e2ee_initialization_tasks(&self) { self.inner.wait_for_e2ee_initialization_tasks().await; } + + /// Get the E2EE identity of a user. + /// + /// Returns an error if this user does not exist, if there is an error + /// contacting the crypto store, or if our client is not logged in. + pub async fn get_user_identity( + &self, + user_id: String, + ) -> Result, ClientError> { + Ok(Arc::new(UserIdentity { + inner: self + .inner + .get_user_identity(user_id.as_str().try_into()?) + .await? + .ok_or(ClientError::new("User not found"))?, + })) + } +} + +/// The E2EE identity of a user. +#[derive(uniffi::Object)] +pub struct UserIdentity { + inner: matrix_sdk::encryption::identities::UserIdentity, +} + +#[uniffi::export] +impl UserIdentity { + /// Remember this identity, ensuring it does not result in a pin violation. + /// + /// When we first see a user, we assume their cryptographic identity has not + /// been tampered with by the homeserver or another entity with + /// man-in-the-middle capabilities. We remember this identity and call this + /// action "pinning". + /// + /// If the identity presented for the user changes later on, the newly + /// presented identity is considered to be in "pin violation". This + /// method explicitly accepts the new identity, allowing it to replace + /// the previously pinned one and bringing it out of pin violation. + /// + /// UIs should display a warning to the user when encountering an identity + /// which is not verified and is in pin violation. + pub(crate) async fn pin(&self) -> Result<(), ClientError> { + Ok(self.inner.pin().await?) + } } #[derive(uniffi::Object)] diff --git a/crates/matrix-sdk-crypto/src/identities/user.rs b/crates/matrix-sdk-crypto/src/identities/user.rs index e70c120138..96c7e5677d 100644 --- a/crates/matrix-sdk-crypto/src/identities/user.rs +++ b/crates/matrix-sdk-crypto/src/identities/user.rs @@ -125,6 +125,32 @@ impl UserIdentity { } } + /// Remember this identity, ensuring it does not result in a pin violation. + /// + /// When we first see a user, we assume their cryptographic identity has not + /// been tampered with by the homeserver or another entity with + /// man-in-the-middle capabilities. We remember this identity and call this + /// action "pinning". + /// + /// If the identity presented for the user changes later on, the newly + /// presented identity is considered to be in "pin violation". This + /// method explicitly accepts the new identity, allowing it to replace + /// the previously pinned one and bringing it out of pin violation. + /// + /// UIs should display a warning to the user when encountering an identity + /// which is not verified and is in pin violation. See + /// [`OtherUserIdentity::identity_needs_user_approval`]. + pub async fn pin(&self) -> Result<(), CryptoStoreError> { + match self { + UserIdentity::Own(_) => { + // Nothing to be done for our own identity: we already + // consider it trusted in this sense. + Ok(()) + } + UserIdentity::Other(u) => u.pin_current_master_key().await, + } + } + /// Was this identity previously verified, and is no longer? pub fn has_verification_violation(&self) -> bool { match self { @@ -737,7 +763,21 @@ impl OtherUserIdentityData { &self.self_signing_key } - /// Pin the current identity + /// Remember this identity, ensuring it does not result in a pin violation. + /// + /// When we first see a user, we assume their cryptographic identity has not + /// been tampered with by the homeserver or another entity with + /// man-in-the-middle capabilities. We remember this identity and call this + /// action "pinning". + /// + /// If the identity presented for the user changes later on, the newly + /// presented identity is considered to be in "pin violation". This + /// method explicitly accepts the new identity, allowing it to replace + /// the previously pinned one and bringing it out of pin violation. + /// + /// UIs should display a warning to the user when encountering an identity + /// which is not verified and is in pin violation. See + /// [`OtherUserIdentity::identity_needs_user_approval`]. pub(crate) fn pin(&self) { let mut m = self.pinned_master_key.write().unwrap(); *m = self.master_key.as_ref().clone() diff --git a/crates/matrix-sdk/CHANGELOG.md b/crates/matrix-sdk/CHANGELOG.md index 8610707e99..915838e009 100644 --- a/crates/matrix-sdk/CHANGELOG.md +++ b/crates/matrix-sdk/CHANGELOG.md @@ -28,6 +28,7 @@ Breaking changes: Additions: +- new `UserIdentity::pin` method. - new `ClientBuilder::with_decryption_trust_requirement` method. - new `ClientBuilder::with_room_key_recipient_strategy` method - new `Room.set_account_data` and `Room.set_account_data_raw` RoomAccountData setters, analogous to the GlobalAccountData diff --git a/crates/matrix-sdk/src/encryption/identities/users.rs b/crates/matrix-sdk/src/encryption/identities/users.rs index d71ca1e7e2..0ce4e303e5 100644 --- a/crates/matrix-sdk/src/encryption/identities/users.rs +++ b/crates/matrix-sdk/src/encryption/identities/users.rs @@ -422,6 +422,24 @@ impl UserIdentity { self.inner.withdraw_verification().await } + /// Remember this identity, ensuring it does not result in a pin violation. + /// + /// When we first see a user, we assume their cryptographic identity has not + /// been tampered with by the homeserver or another entity with + /// man-in-the-middle capabilities. We remember this identity and call this + /// action "pinning". + /// + /// If the identity presented for the user changes later on, the newly + /// presented identity is considered to be in "pin violation". This + /// method explicitly accepts the new identity, allowing it to replace + /// the previously pinned one and bringing it out of pin violation. + /// + /// UIs should display a warning to the user when encountering an identity + /// which is not verified and is in pin violation. + pub async fn pin(&self) -> Result<(), CryptoStoreError> { + self.inner.pin().await + } + /// Get the public part of the Master key of this user identity. /// /// The public part of the Master key is usually used to uniquely identify diff --git a/crates/matrix-sdk/src/room/identity_status_changes.rs b/crates/matrix-sdk/src/room/identity_status_changes.rs index ce93f326a4..feab79e416 100644 --- a/crates/matrix-sdk/src/room/identity_status_changes.rs +++ b/crates/matrix-sdk/src/room/identity_status_changes.rs @@ -430,9 +430,10 @@ mod tests { ); // Pin it - self.crypto_other_identity() + self.user_identity() .await - .pin_current_master_key() + .expect("User should exist") + .pin() .await .expect("Should not fail to pin"); } else { From ee67ca81bb4b34ff752ae601d6dbb1856f43040f Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 4 Oct 2024 15:13:37 +0100 Subject: [PATCH 2/2] ffi: Expose the master_key method on UserIdentity --- bindings/matrix-sdk-ffi/CHANGELOG.md | 2 +- bindings/matrix-sdk-ffi/src/encryption.rs | 27 ++++++++++++++--------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/bindings/matrix-sdk-ffi/CHANGELOG.md b/bindings/matrix-sdk-ffi/CHANGELOG.md index 55a9474eb6..922f8f305c 100644 --- a/bindings/matrix-sdk-ffi/CHANGELOG.md +++ b/bindings/matrix-sdk-ffi/CHANGELOG.md @@ -31,5 +31,5 @@ Breaking changes: Additions: -- Add `Encryption::get_user_identity` +- Add `Encryption::get_user_identity` which returns `UserIdentity` - Add `ClientBuilder::room_key_recipient_strategy` diff --git a/bindings/matrix-sdk-ffi/src/encryption.rs b/bindings/matrix-sdk-ffi/src/encryption.rs index 501097483f..579387b485 100644 --- a/bindings/matrix-sdk-ffi/src/encryption.rs +++ b/bindings/matrix-sdk-ffi/src/encryption.rs @@ -413,19 +413,16 @@ impl Encryption { /// Get the E2EE identity of a user. /// - /// Returns an error if this user does not exist, if there is an error - /// contacting the crypto store, or if our client is not logged in. + /// Returns Ok(None) if this user does not exist. + /// + /// Returns an error if there was a problem contacting the crypto store, or + /// if our client is not logged in. pub async fn get_user_identity( &self, user_id: String, - ) -> Result, ClientError> { - Ok(Arc::new(UserIdentity { - inner: self - .inner - .get_user_identity(user_id.as_str().try_into()?) - .await? - .ok_or(ClientError::new("User not found"))?, - })) + ) -> Result>, ClientError> { + let identity = self.inner.get_user_identity(user_id.as_str().try_into()?).await?; + Ok(identity.map(|i| Arc::new(UserIdentity { inner: i }))) } } @@ -454,6 +451,16 @@ impl UserIdentity { pub(crate) async fn pin(&self) -> Result<(), ClientError> { Ok(self.inner.pin().await?) } + + /// Get the public part of the Master key of this user identity. + /// + /// The public part of the Master key is usually used to uniquely identify + /// the identity. + /// + /// Returns None if the master key does not actually contain any keys. + pub(crate) fn master_key(&self) -> Option { + self.inner.master_key().get_first_key().map(|k| k.to_base64()) + } } #[derive(uniffi::Object)]