From c78406ceb937229db834a0539eb5264f98e891a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 24 Jun 2021 12:12:13 +0200 Subject: [PATCH] crypto: Clean up the method to check if a device is verified. --- matrix_sdk/examples/emoji_verification.rs | 2 +- matrix_sdk/src/client.rs | 2 +- matrix_sdk/src/device.rs | 7 +- matrix_sdk_crypto/src/identities/device.rs | 86 +++++++++++--------- matrix_sdk_crypto/src/identities/user.rs | 16 ++-- matrix_sdk_crypto/src/key_request.rs | 2 +- matrix_sdk_crypto/src/machine.rs | 10 +-- matrix_sdk_crypto/src/verification/qrcode.rs | 4 +- 8 files changed, 68 insertions(+), 61 deletions(-) diff --git a/matrix_sdk/examples/emoji_verification.rs b/matrix_sdk/examples/emoji_verification.rs index 90c7fd43..1307d9a2 100644 --- a/matrix_sdk/examples/emoji_verification.rs +++ b/matrix_sdk/examples/emoji_verification.rs @@ -58,7 +58,7 @@ async fn print_devices(user_id: &UserId, client: &Client) { " {:<10} {:<30} {:<}", device.device_id(), device.display_name().as_deref().unwrap_or_default(), - device.is_trusted() + device.verified() ); } } diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index c977755c..3d1d2713 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -2251,7 +2251,7 @@ impl Client { /// .unwrap() /// .unwrap(); /// - /// println!("{:?}", device.is_trusted()); + /// println!("{:?}", device.verified()); /// /// let verification = device.start_verification().await.unwrap(); /// # }); diff --git a/matrix_sdk/src/device.rs b/matrix_sdk/src/device.rs index c9e329c6..92c4fdaa 100644 --- a/matrix_sdk/src/device.rs +++ b/matrix_sdk/src/device.rs @@ -69,9 +69,10 @@ impl Device { Ok(SasVerification { inner: sas, client: self.client.clone() }) } - /// Is the device trusted. - pub fn is_trusted(&self) -> bool { - self.inner.trust_state() + /// Is the device considered to be verified, either by locally trusting it + /// or using cross signing. + pub fn verified(&self) -> bool { + self.inner.verified() } /// Set the local trust state of the device to the given state. diff --git a/matrix_sdk_crypto/src/identities/device.rs b/matrix_sdk_crypto/src/identities/device.rs index fecccaa2..d9f6e8cc 100644 --- a/matrix_sdk_crypto/src/identities/device.rs +++ b/matrix_sdk_crypto/src/identities/device.rs @@ -145,9 +145,20 @@ impl Device { } } - /// Get the trust state of the device. - pub fn trust_state(&self) -> bool { - self.inner.trust_state(&self.own_identity, &self.device_owner_identity) + /// Is this device considered to be verified. + /// + /// This method returns true if either [`is_locally_trusted()`] returns true + /// or if [`is_cross_signing_trusted()`] returns true. + /// + /// [`is_locally_trusted()`]: #method.is_locally_trusted + /// [`is_cross_signing_trusted()`]: #method.is_cross_signing_trusted + pub fn verified(&self) -> bool { + self.inner.verified(&self.own_identity, &self.device_owner_identity) + } + + /// Is this device considered to be verified using cross signing. + pub fn is_cross_signing_trusted(&self) -> bool { + self.inner.is_cross_signing_trusted(&self.own_identity, &self.device_owner_identity) } /// Set the local trust state of the device to the given state. @@ -236,7 +247,7 @@ impl UserDevices { /// Returns true if there is at least one devices of this user that is /// considered to be verified, false otherwise. pub fn is_any_verified(&self) -> bool { - self.inner.values().any(|d| d.trust_state(&self.own_identity, &self.device_owner_identity)) + self.inner.values().any(|d| d.verified(&self.own_identity, &self.device_owner_identity)) } /// Iterator over all the device ids of the user devices. @@ -340,7 +351,7 @@ impl ReadOnlyDevice { } /// Is the device locally marked as trusted. - pub fn is_trusted(&self) -> bool { + pub fn is_locally_trusted(&self) -> bool { self.local_trust_state() == LocalTrust::Verified } @@ -369,46 +380,41 @@ impl ReadOnlyDevice { self.deleted.load(Ordering::Relaxed) } - pub(crate) fn trust_state( + pub(crate) fn verified( &self, own_identity: &Option, device_owner: &Option, ) -> bool { - // TODO we want to return an enum mentioning if the trust is local, if - // only the identity is trusted, if the identity and the device are - // trusted. - if self.is_trusted() { - // If the device is locally marked as verified just return so, no - // need to check signatures. - true - } else { - own_identity.as_ref().map_or(false, |own_identity| { - // Our own identity needs to be marked as verified. - own_identity.is_verified() - && device_owner - .as_ref() - .map(|device_identity| match device_identity { - // If it's one of our own devices, just check that - // we signed the device. - UserIdentities::Own(_) => { - own_identity.is_device_signed(self).map_or(false, |_| true) - } + self.is_locally_trusted() || self.is_cross_signing_trusted(own_identity, device_owner) + } - // If it's a device from someone else, first check - // that our user has signed the other user and then - // check if the other user has signed this device. - UserIdentities::Other(device_identity) => { - own_identity - .is_identity_signed(device_identity) - .map_or(false, |_| true) - && device_identity - .is_device_signed(self) - .map_or(false, |_| true) - } - }) - .unwrap_or(false) - }) - } + pub(crate) fn is_cross_signing_trusted( + &self, + own_identity: &Option, + device_owner: &Option, + ) -> bool { + own_identity.as_ref().map_or(false, |own_identity| { + // Our own identity needs to be marked as verified. + own_identity.is_verified() + && device_owner + .as_ref() + .map(|device_identity| match device_identity { + // If it's one of our own devices, just check that + // we signed the device. + UserIdentities::Own(_) => { + own_identity.is_device_signed(self).map_or(false, |_| true) + } + + // If it's a device from someone else, first check + // that our user has signed the other user and then + // check if the other user has signed this device. + UserIdentities::Other(device_identity) => { + own_identity.is_identity_signed(device_identity).map_or(false, |_| true) + && device_identity.is_device_signed(self).map_or(false, |_| true) + } + }) + .unwrap_or(false) + }) } pub(crate) async fn encrypt( diff --git a/matrix_sdk_crypto/src/identities/user.rs b/matrix_sdk_crypto/src/identities/user.rs index e983f786..184d64c2 100644 --- a/matrix_sdk_crypto/src/identities/user.rs +++ b/matrix_sdk_crypto/src/identities/user.rs @@ -784,15 +784,15 @@ pub(crate) mod test { device_owner_identity: Some(UserIdentities::Own(identity.clone())), }; - assert!(!second.trust_state()); - assert!(!second.is_trusted()); + assert!(!second.is_locally_trusted()); + assert!(!second.is_cross_signing_trusted()); - assert!(!first.trust_state()); - assert!(!first.is_trusted()); + assert!(!first.is_locally_trusted()); + assert!(!first.is_cross_signing_trusted()); identity.mark_as_verified(); - assert!(second.trust_state()); - assert!(!first.trust_state()); + assert!(second.verified()); + assert!(!first.verified()); } #[async_test] @@ -821,12 +821,12 @@ pub(crate) mod test { device_owner_identity: Some(public_identity.clone().into()), }; - assert!(!device.trust_state()); + assert!(!device.verified()); let mut device_keys = device.as_device_keys(); identity.sign_device_keys(&mut device_keys).await.unwrap(); device.inner.signatures = Arc::new(device_keys.signatures); - assert!(device.trust_state()); + assert!(device.verified()); } } diff --git a/matrix_sdk_crypto/src/key_request.rs b/matrix_sdk_crypto/src/key_request.rs index d089853f..570837d0 100644 --- a/matrix_sdk_crypto/src/key_request.rs +++ b/matrix_sdk_crypto/src/key_request.rs @@ -479,7 +479,7 @@ impl KeyRequestMachine { .flatten(); let own_device_check = || { - if device.trust_state() { + if device.verified() { Ok(None) } else { Err(KeyshareDecision::UntrustedDevice) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index cb02d64e..75b1fd40 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -928,7 +928,7 @@ impl OlmMachine { .unwrap_or(false) }) { if (self.user_id() == device.user_id() && self.device_id() == device.device_id()) - || device.is_trusted() + || device.verified() { VerificationState::Trusted } else { @@ -1771,7 +1771,7 @@ pub(crate) mod test { let bob_device = alice.get_device(bob.user_id(), bob.device_id()).await.unwrap().unwrap(); - assert!(!bob_device.is_trusted()); + assert!(!bob_device.verified()); let (alice_sas, request) = bob_device.start_verification().await.unwrap(); @@ -1834,14 +1834,14 @@ pub(crate) mod test { .unwrap(); assert!(alice_sas.is_done()); - assert!(bob_device.is_trusted()); + assert!(bob_device.verified()); let alice_device = bob.get_device(alice.user_id(), alice.device_id()).await.unwrap().unwrap(); - assert!(!alice_device.is_trusted()); + assert!(!alice_device.verified()); bob.handle_verification_event(&event).await; assert!(bob_sas.is_done()); - assert!(alice_device.is_trusted()); + assert!(alice_device.verified()); } } diff --git a/matrix_sdk_crypto/src/verification/qrcode.rs b/matrix_sdk_crypto/src/verification/qrcode.rs index 37a60917..33578544 100644 --- a/matrix_sdk_crypto/src/verification/qrcode.rs +++ b/matrix_sdk_crypto/src/verification/qrcode.rs @@ -871,8 +871,8 @@ mod test { let identity = identity.own().unwrap(); - assert!(!bob_device.is_trusted()); - assert!(alice_device.is_trusted()); + assert!(!bob_device.is_locally_trusted()); + assert!(alice_device.is_locally_trusted()); assert!(identity.is_verified()); };