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 01/35] 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()); }; From 9a685d685c35930bc33477cb2cda6e7325791a43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 24 Jun 2021 12:13:06 +0200 Subject: [PATCH 02/35] crypto: Add a couple more getters for the SAS verification --- matrix_sdk_crypto/src/verification/sas/inner_sas.rs | 12 ++++++++++++ matrix_sdk_crypto/src/verification/sas/mod.rs | 10 ++++++++++ 2 files changed, 22 insertions(+) diff --git a/matrix_sdk_crypto/src/verification/sas/inner_sas.rs b/matrix_sdk_crypto/src/verification/sas/inner_sas.rs index 67b5302e..7ca37d5a 100644 --- a/matrix_sdk_crypto/src/verification/sas/inner_sas.rs +++ b/matrix_sdk_crypto/src/verification/sas/inner_sas.rs @@ -312,6 +312,18 @@ impl InnerSas { matches!(self, InnerSas::Cancelled(_)) } + pub fn have_we_confirmed(&self) -> bool { + matches!(self, InnerSas::Confirmed(_) | InnerSas::WaitingForDone(_) | InnerSas::Done(_)) + } + + pub fn cancel_code(&self) -> Option { + if let InnerSas::Cancelled(c) = self { + Some(c.state.cancel_code.clone()) + } else { + None + } + } + pub fn timed_out(&self) -> bool { match self { InnerSas::Created(s) => s.timed_out(), diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index fb9a5be6..3339c8e4 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -104,6 +104,16 @@ impl Sas { self.identities_being_verified.is_self_verification() } + /// Have we confirmed that the short auth string matches. + pub fn have_we_confirmed(&self) -> bool { + self.inner.lock().unwrap().have_we_confirmed() + } + + /// Get the cancel code of this SAS verification if it has been cancelled + pub fn cancel_code(&self) -> Option { + self.inner.lock().unwrap().cancel_code() + } + #[cfg(test)] #[allow(dead_code)] pub(crate) fn set_creation_time(&self, time: Instant) { From 55690ddd54137639457755bfc573b3a4dfbbd4dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 24 Jun 2021 12:14:09 +0200 Subject: [PATCH 03/35] crypto: Allow canceling a SAS verification with a specific cancel code --- matrix_sdk_crypto/src/verification/sas/mod.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index 3339c8e4..471be4aa 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -359,7 +359,20 @@ impl Sas { self.cancel_with_code(CancelCode::User) } - pub(crate) fn cancel_with_code(&self, code: CancelCode) -> Option { + /// Cancel the verification. + /// + /// This cancels the verification with given `CancelCode`. + /// + /// **Note**: This method should generally not be used, the [`cancel()`] + /// method should be preferred. The SDK will automatically cancel with the + /// approprate cancel code, user initiated cancellations should only cancel + /// with the `CancelCode::User` + /// + /// Returns None if the `Sas` object is already in a canceled state, + /// otherwise it returns a request that needs to be sent out. + /// + /// [`cancel()`]: #method.cancel + pub fn cancel_with_code(&self, code: CancelCode) -> Option { let mut guard = self.inner.lock().unwrap(); let sas: InnerSas = (*guard).clone(); let (sas, content) = sas.cancel(code); From d4e80883dcb18040a0725d16478ea1ff3b19763f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 25 Jun 2021 12:51:45 +0200 Subject: [PATCH 04/35] crypto: Propagate the we_started info to the SAS verification --- matrix_sdk_crypto/src/verification/machine.rs | 3 + .../src/verification/requests.rs | 58 +++++++++++-------- matrix_sdk_crypto/src/verification/sas/mod.rs | 17 ++++++ 3 files changed, 54 insertions(+), 24 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index b5d1d60c..494f9b65 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -72,6 +72,7 @@ impl VerificationMachine { self.store.clone(), identity, None, + true, ); let request = match content { @@ -325,6 +326,7 @@ impl VerificationMachine { device, identity, false, + false, ) { Ok(sas) => { self.verifications.insert_sas(sas); @@ -461,6 +463,7 @@ mod test { bob_store, None, None, + true, ); machine diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index 7eb621c6..2d26fd22 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -414,7 +414,7 @@ impl VerificationRequest { let inner = self.inner.lock().unwrap().clone(); if let InnerRequest::Ready(s) = inner { - s.receive_start(sender, content).await?; + s.receive_start(sender, content, self.we_started).await?; } else { warn!( sender = sender.as_str(), @@ -454,6 +454,7 @@ impl VerificationRequest { s.store.clone(), s.account.clone(), s.private_cross_signing_identity.clone(), + self.we_started, ) .await? { @@ -565,9 +566,10 @@ impl InnerRequest { content: &StartContent, other_device: ReadOnlyDevice, other_identity: Option, + we_started: bool, ) -> Result, OutgoingContent> { if let InnerRequest::Ready(s) = self { - Ok(Some(s.to_started_sas(content, other_device, other_identity)?)) + Ok(Some(s.to_started_sas(content, other_device, other_identity, we_started)?)) } else { Ok(None) } @@ -762,6 +764,7 @@ impl RequestState { content: &StartContent<'a>, other_device: ReadOnlyDevice, other_identity: Option, + we_started: bool, ) -> Result { Sas::from_start_event( (&*self.flow_id).to_owned(), @@ -772,6 +775,7 @@ impl RequestState { other_device, other_identity, true, + we_started, ) } @@ -868,6 +872,7 @@ impl RequestState { &self, sender: &UserId, content: &StartContent<'_>, + we_started: bool, ) -> Result<(), CryptoStoreError> { info!( sender = sender.as_str(), @@ -890,29 +895,31 @@ impl RequestState { let identity = self.store.get_user_identity(sender).await?; match content.method() { - StartMethod::SasV1(_) => match self.to_started_sas(content, device.clone(), identity) { - // TODO check if there is already a SAS verification, i.e. we - // already started one before the other side tried to do the - // same; ignore it if we did and we're the lexicographically - // smaller user ID, otherwise auto-accept the newly started one. - Ok(s) => { - info!("Started a new SAS verification."); - self.verification_cache.insert_sas(s); + StartMethod::SasV1(_) => { + match self.to_started_sas(content, device.clone(), identity, we_started) { + // TODO check if there is already a SAS verification, i.e. we + // already started one before the other side tried to do the + // same; ignore it if we did and we're the lexicographically + // smaller user ID, otherwise auto-accept the newly started one. + Ok(s) => { + info!("Started a new SAS verification."); + self.verification_cache.insert_sas(s); + } + Err(c) => { + warn!( + user_id = device.user_id().as_str(), + device_id = device.device_id().as_str(), + content =? c, + "Can't start key verification, canceling.", + ); + self.verification_cache.queue_up_content( + device.user_id(), + device.device_id(), + c, + ) + } } - Err(c) => { - warn!( - user_id = device.user_id().as_str(), - device_id = device.device_id().as_str(), - content =? c, - "Can't start key verification, canceling.", - ); - self.verification_cache.queue_up_content( - device.user_id(), - device.device_id(), - c, - ) - } - }, + } StartMethod::ReciprocateV1(_) => { if let Some(qr_verification) = self.verification_cache.get_qr(sender, content.flow_id()) @@ -941,6 +948,7 @@ impl RequestState { store: Arc, account: ReadOnlyAccount, private_identity: PrivateCrossSigningIdentity, + we_started: bool, ) -> Result, CryptoStoreError> { if !self.state.their_methods.contains(&VerificationMethod::SasV1) { return Ok(None); @@ -972,6 +980,7 @@ impl RequestState { store, other_identity, Some(t.to_owned()), + we_started, ); (sas, content) } @@ -984,6 +993,7 @@ impl RequestState { device, store, other_identity, + we_started, ); (sas, content) } diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index 471be4aa..1755cd03 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -55,6 +55,7 @@ pub struct Sas { account: ReadOnlyAccount, identities_being_verified: IdentitiesBeingVerified, flow_id: Arc, + we_started: bool, } impl Sas { @@ -114,6 +115,11 @@ impl Sas { self.inner.lock().unwrap().cancel_code() } + /// Did we initiate the verification flow. + pub fn we_started(&self) -> bool { + self.we_started + } + #[cfg(test)] #[allow(dead_code)] pub(crate) fn set_creation_time(&self, time: Instant) { @@ -127,6 +133,7 @@ impl Sas { other_device: ReadOnlyDevice, store: Arc, other_identity: Option, + we_started: bool, ) -> Sas { let flow_id = inner_sas.verification_flow_id(); @@ -142,6 +149,7 @@ impl Sas { account, identities_being_verified: identities, flow_id, + we_started, } } @@ -162,6 +170,7 @@ impl Sas { store: Arc, other_identity: Option, transaction_id: Option, + we_started: bool, ) -> (Sas, OutgoingContent) { let (inner, content) = InnerSas::start( account.clone(), @@ -178,6 +187,7 @@ impl Sas { other_device, store, other_identity, + we_started, ), content, ) @@ -193,6 +203,7 @@ impl Sas { /// /// Returns the new `Sas` object and a `StartEventContent` that needs to be /// sent out through the server to the other device. + #[allow(clippy::too_many_arguments)] pub(crate) fn start_in_room( flow_id: EventId, room_id: RoomId, @@ -201,6 +212,7 @@ impl Sas { other_device: ReadOnlyDevice, store: Arc, other_identity: Option, + we_started: bool, ) -> (Sas, OutgoingContent) { let (inner, content) = InnerSas::start_in_room( flow_id, @@ -218,6 +230,7 @@ impl Sas { other_device, store, other_identity, + we_started, ), content, ) @@ -243,6 +256,7 @@ impl Sas { other_device: ReadOnlyDevice, other_identity: Option, started_from_request: bool, + we_started: bool, ) -> Result { let inner = InnerSas::from_start_event( account.clone(), @@ -260,6 +274,7 @@ impl Sas { other_device, store, other_identity, + we_started, )) } @@ -568,6 +583,7 @@ mod test { alice_store, None, None, + true, ); let flow_id = alice.flow_id().to_owned(); @@ -582,6 +598,7 @@ mod test { alice_device, None, false, + false, ) .unwrap(); From af084528c8924c359bb42b7a359f288ccd9e8cec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 25 Jun 2021 12:54:00 +0200 Subject: [PATCH 05/35] crypto: Remember who cancelled the verification This seems to be of limited use considering that people can just see the sender of the cancellation event or they perform the cancellation themselves using the cancel() method but EA seems to want this. --- matrix_sdk_crypto/src/verification/mod.rs | 5 +-- matrix_sdk_crypto/src/verification/qrcode.rs | 12 +++---- .../src/verification/requests.rs | 22 +++++++----- .../src/verification/sas/inner_sas.rs | 26 ++++++++++---- matrix_sdk_crypto/src/verification/sas/mod.rs | 7 +++- .../src/verification/sas/sas_state.rs | 34 +++++++++---------- 6 files changed, 64 insertions(+), 42 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/mod.rs b/matrix_sdk_crypto/src/verification/mod.rs index fd9850e2..b77d758b 100644 --- a/matrix_sdk_crypto/src/verification/mod.rs +++ b/matrix_sdk_crypto/src/verification/mod.rs @@ -167,12 +167,13 @@ impl Done { #[derive(Clone, Debug)] pub struct Cancelled { + cancelled_by_us: bool, cancel_code: CancelCode, reason: &'static str, } impl Cancelled { - fn new(code: CancelCode) -> Self { + fn new(cancelled_by_us: bool, code: CancelCode) -> Self { let reason = match code { CancelCode::Accepted => { "A m.key.verification.request was accepted by a different device." @@ -192,7 +193,7 @@ impl Cancelled { _ => "Unknown cancel reason", }; - Self { cancel_code: code, reason } + Self { cancelled_by_us, cancel_code: code, reason } } pub fn as_content(&self, flow_id: &FlowId) -> OutgoingContent { diff --git a/matrix_sdk_crypto/src/verification/qrcode.rs b/matrix_sdk_crypto/src/verification/qrcode.rs index 33578544..e57e88fe 100644 --- a/matrix_sdk_crypto/src/verification/qrcode.rs +++ b/matrix_sdk_crypto/src/verification/qrcode.rs @@ -210,7 +210,7 @@ impl QrVerification { } fn cancel_with_code(&self, code: CancelCode) -> Option { - let new_state = QrState::::new(code); + let new_state = QrState::::new(true, code); let content = new_state.as_content(self.flow_id()); let mut state = self.state.lock().unwrap(); @@ -243,7 +243,7 @@ impl QrVerification { match self.identities.mark_as_done(Some(&devices), Some(&identities)).await? { VerificationResult::Ok => (None, None), VerificationResult::Cancel(c) => { - let canceled = QrState::::new(c); + let canceled = QrState::::new(false, c); let content = canceled.as_content(self.flow_id()); new_state = InnerState::Cancelled(canceled); (Some(content), None) @@ -594,8 +594,8 @@ impl QrState { } impl QrState { - fn new(cancel_code: CancelCode) -> Self { - QrState { state: Cancelled::new(cancel_code) } + fn new(cancelled_by_us: bool, cancel_code: CancelCode) -> Self { + QrState { state: Cancelled::new(cancelled_by_us, cancel_code) } } fn as_content(&self, flow_id: &FlowId) -> OutgoingContent { @@ -614,10 +614,10 @@ impl QrState { if self.state.secret == m.secret { Ok(QrState { state: Scanned {} }) } else { - Err(QrState::::new(CancelCode::KeyMismatch)) + Err(QrState::::new(false, CancelCode::KeyMismatch)) } } - _ => Err(QrState::::new(CancelCode::UnknownMethod)), + _ => Err(QrState::::new(false, CancelCode::UnknownMethod)), } } } diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index 2d26fd22..14ed1f90 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -376,7 +376,7 @@ impl VerificationRequest { /// Cancel the verification request pub fn cancel(&self) -> Option { let mut inner = self.inner.lock().unwrap(); - inner.cancel(&CancelCode::User); + inner.cancel(true, &CancelCode::User); let content = if let InnerRequest::Cancelled(c) = &*inner { Some(c.state.as_content(self.flow_id())) @@ -436,7 +436,7 @@ impl VerificationRequest { pub(crate) fn receive_cancel(&self, sender: &UserId, content: &CancelContent<'_>) { if sender == self.other_user() { let mut inner = self.inner.lock().unwrap().clone(); - inner.cancel(content.cancel_code()); + inner.cancel(false, content.cancel_code()); } } @@ -539,12 +539,12 @@ impl InnerRequest { }) } - fn cancel(&mut self, cancel_code: &CancelCode) { + fn cancel(&mut self, cancelled_by_us: bool, cancel_code: &CancelCode) { *self = InnerRequest::Cancelled(match self { - InnerRequest::Created(s) => s.clone().into_canceled(cancel_code), - InnerRequest::Requested(s) => s.clone().into_canceled(cancel_code), - InnerRequest::Ready(s) => s.clone().into_canceled(cancel_code), - InnerRequest::Passive(s) => s.clone().into_canceled(cancel_code), + InnerRequest::Created(s) => s.clone().into_canceled(cancelled_by_us, cancel_code), + InnerRequest::Requested(s) => s.clone().into_canceled(cancelled_by_us, cancel_code), + InnerRequest::Ready(s) => s.clone().into_canceled(cancelled_by_us, cancel_code), + InnerRequest::Passive(s) => s.clone().into_canceled(cancelled_by_us, cancel_code), InnerRequest::Done(_) => return, InnerRequest::Cancelled(_) => return, }) @@ -604,7 +604,11 @@ impl RequestState { } } - fn into_canceled(self, cancel_code: &CancelCode) -> RequestState { + fn into_canceled( + self, + cancelled_by_us: bool, + cancel_code: &CancelCode, + ) -> RequestState { RequestState:: { account: self.account, private_cross_signing_identity: self.private_cross_signing_identity, @@ -612,7 +616,7 @@ impl RequestState { store: self.store, flow_id: self.flow_id, other_user_id: self.other_user_id, - state: Cancelled::new(cancel_code.clone()), + state: Cancelled::new(cancelled_by_us, cancel_code.clone()), } } } diff --git a/matrix_sdk_crypto/src/verification/sas/inner_sas.rs b/matrix_sdk_crypto/src/verification/sas/inner_sas.rs index 7ca37d5a..d8f35332 100644 --- a/matrix_sdk_crypto/src/verification/sas/inner_sas.rs +++ b/matrix_sdk_crypto/src/verification/sas/inner_sas.rs @@ -168,13 +168,17 @@ impl InnerSas { } } - pub fn cancel(self, code: CancelCode) -> (InnerSas, Option) { + pub fn cancel( + self, + cancelled_by_us: bool, + code: CancelCode, + ) -> (InnerSas, Option) { let sas = match self { - InnerSas::Created(s) => s.cancel(code), - InnerSas::Started(s) => s.cancel(code), - InnerSas::Accepted(s) => s.cancel(code), - InnerSas::KeyReceived(s) => s.cancel(code), - InnerSas::MacReceived(s) => s.cancel(code), + InnerSas::Created(s) => s.cancel(cancelled_by_us, code), + InnerSas::Started(s) => s.cancel(cancelled_by_us, code), + InnerSas::Accepted(s) => s.cancel(cancelled_by_us, code), + InnerSas::KeyReceived(s) => s.cancel(cancelled_by_us, code), + InnerSas::MacReceived(s) => s.cancel(cancelled_by_us, code), _ => return (self, None), }; @@ -230,7 +234,7 @@ impl InnerSas { } } AnyVerificationContent::Cancel(c) => { - let (sas, _) = self.cancel(c.cancel_code().to_owned()); + let (sas, _) = self.cancel(false, c.cancel_code().to_owned()); (sas, None) } AnyVerificationContent::Key(c) => match self { @@ -324,6 +328,14 @@ impl InnerSas { } } + pub fn cancelled_by_us(&self) -> Option { + if let InnerSas::Cancelled(c) = self { + Some(c.state.cancelled_by_us) + } else { + None + } + } + pub fn timed_out(&self) -> bool { match self { InnerSas::Created(s) => s.timed_out(), diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index 1755cd03..6ef72715 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -115,6 +115,11 @@ impl Sas { self.inner.lock().unwrap().cancel_code() } + /// Has the verification flow been cancelled by us. + pub fn cancelled_by_us(&self) -> Option { + self.inner.lock().unwrap().cancelled_by_us() + } + /// Did we initiate the verification flow. pub fn we_started(&self) -> bool { self.we_started @@ -390,7 +395,7 @@ impl Sas { pub fn cancel_with_code(&self, code: CancelCode) -> Option { let mut guard = self.inner.lock().unwrap(); let sas: InnerSas = (*guard).clone(); - let (sas, content) = sas.cancel(code); + let (sas, content) = sas.cancel(true, code); *guard = sas; content.map(|c| match c { OutgoingContent::Room(room_id, content) => { diff --git a/matrix_sdk_crypto/src/verification/sas/sas_state.rs b/matrix_sdk_crypto/src/verification/sas/sas_state.rs index 2ab1beb6..eb9d78c8 100644 --- a/matrix_sdk_crypto/src/verification/sas/sas_state.rs +++ b/matrix_sdk_crypto/src/verification/sas/sas_state.rs @@ -298,14 +298,14 @@ impl SasState { self.ids.other_device.clone() } - pub fn cancel(self, cancel_code: CancelCode) -> SasState { + pub fn cancel(self, cancelled_by_us: bool, cancel_code: CancelCode) -> SasState { SasState { inner: self.inner, ids: self.ids, creation_time: self.creation_time, last_event_time: self.last_event_time, verification_flow_id: self.verification_flow_id, - state: Arc::new(Cancelled::new(cancel_code)), + state: Arc::new(Cancelled::new(cancelled_by_us, cancel_code)), started_from_request: self.started_from_request, } } @@ -444,11 +444,11 @@ impl SasState { sender: &UserId, content: &AcceptContent, ) -> Result, SasState> { - self.check_event(sender, content.flow_id()).map_err(|c| self.clone().cancel(c))?; + self.check_event(sender, content.flow_id()).map_err(|c| self.clone().cancel(true, c))?; if let AcceptMethod::SasV1(content) = content.method() { - let accepted_protocols = - AcceptedProtocols::try_from(content.clone()).map_err(|c| self.clone().cancel(c))?; + let accepted_protocols = AcceptedProtocols::try_from(content.clone()) + .map_err(|c| self.clone().cancel(true, c))?; let start_content = self.as_content().into(); @@ -466,7 +466,7 @@ impl SasState { }), }) } else { - Err(self.cancel(CancelCode::UnknownMethod)) + Err(self.cancel(true, CancelCode::UnknownMethod)) } } } @@ -509,7 +509,7 @@ impl SasState { }, verification_flow_id: flow_id.clone(), - state: Arc::new(Cancelled::new(CancelCode::UnknownMethod)), + state: Arc::new(Cancelled::new(true, CancelCode::UnknownMethod)), }; if let StartMethod::SasV1(method_content) = content.method() { @@ -600,7 +600,7 @@ impl SasState { sender: &UserId, content: &KeyContent, ) -> Result, SasState> { - self.check_event(sender, content.flow_id()).map_err(|c| self.clone().cancel(c))?; + self.check_event(sender, content.flow_id()).map_err(|c| self.clone().cancel(true, c))?; let their_pubkey = content.public_key().to_owned(); @@ -640,7 +640,7 @@ impl SasState { sender: &UserId, content: &KeyContent, ) -> Result, SasState> { - self.check_event(sender, content.flow_id()).map_err(|c| self.clone().cancel(c))?; + self.check_event(sender, content.flow_id()).map_err(|c| self.clone().cancel(true, c))?; let commitment = calculate_commitment( content.public_key(), @@ -648,7 +648,7 @@ impl SasState { ); if self.state.commitment != commitment { - Err(self.cancel(CancelCode::InvalidMessage)) + Err(self.cancel(true, CancelCode::InvalidMessage)) } else { let their_pubkey = content.public_key().to_owned(); @@ -777,7 +777,7 @@ impl SasState { sender: &UserId, content: &MacContent, ) -> Result, SasState> { - self.check_event(sender, content.flow_id()).map_err(|c| self.clone().cancel(c))?; + self.check_event(sender, content.flow_id()).map_err(|c| self.clone().cancel(true, c))?; let (devices, master_keys) = receive_mac_event( &self.inner.lock().unwrap(), @@ -786,7 +786,7 @@ impl SasState { sender, content, ) - .map_err(|c| self.clone().cancel(c))?; + .map_err(|c| self.clone().cancel(true, c))?; Ok(SasState { inner: self.inner, @@ -837,7 +837,7 @@ impl SasState { sender: &UserId, content: &MacContent, ) -> Result, SasState> { - self.check_event(sender, content.flow_id()).map_err(|c| self.clone().cancel(c))?; + self.check_event(sender, content.flow_id()).map_err(|c| self.clone().cancel(true, c))?; let (devices, master_keys) = receive_mac_event( &self.inner.lock().unwrap(), @@ -846,7 +846,7 @@ impl SasState { sender, content, ) - .map_err(|c| self.clone().cancel(c))?; + .map_err(|c| self.clone().cancel(true, c))?; Ok(SasState { inner: self.inner, @@ -877,7 +877,7 @@ impl SasState { sender: &UserId, content: &MacContent, ) -> Result, SasState> { - self.check_event(sender, content.flow_id()).map_err(|c| self.clone().cancel(c))?; + self.check_event(sender, content.flow_id()).map_err(|c| self.clone().cancel(true, c))?; let (devices, master_keys) = receive_mac_event( &self.inner.lock().unwrap(), @@ -886,7 +886,7 @@ impl SasState { sender, content, ) - .map_err(|c| self.clone().cancel(c))?; + .map_err(|c| self.clone().cancel(true, c))?; Ok(SasState { inner: self.inner, @@ -1032,7 +1032,7 @@ impl SasState { sender: &UserId, content: &DoneContent, ) -> Result, SasState> { - self.check_event(sender, content.flow_id()).map_err(|c| self.clone().cancel(c))?; + self.check_event(sender, content.flow_id()).map_err(|c| self.clone().cancel(true, c))?; Ok(SasState { inner: self.inner, From 728d2988104914fb2d835f2117a46de247ddf8e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 25 Jun 2021 12:56:30 +0200 Subject: [PATCH 06/35] crypto: Add a getter for the room id for the SAS verifications --- matrix_sdk_crypto/src/verification/sas/mod.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index 6ef72715..0079a4b0 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -89,6 +89,15 @@ impl Sas { &self.flow_id } + /// Get the room id if the verification is happening inside a room. + pub fn room_id(&self) -> Option<&RoomId> { + if let FlowId::InRoom(r, _) = self.flow_id() { + Some(r) + } else { + None + } + } + /// Does this verification flow support displaying emoji for the short /// authentication string. pub fn supports_emoji(&self) -> bool { From 80a30bcdd6de8972b7cefb9712c979cb6fb07418 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 25 Jun 2021 17:55:39 +0200 Subject: [PATCH 07/35] crypto: Add all the common accessors to the qr code verification --- matrix_sdk_crypto/src/verification/qrcode.rs | 109 ++++++++++++++---- .../src/verification/requests.rs | 15 ++- 2 files changed, 97 insertions(+), 27 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/qrcode.rs b/matrix_sdk_crypto/src/verification/qrcode.rs index e57e88fe..0b414b80 100644 --- a/matrix_sdk_crypto/src/verification/qrcode.rs +++ b/matrix_sdk_crypto/src/verification/qrcode.rs @@ -33,7 +33,7 @@ use ruma::{ }, AnyMessageEventContent, AnyToDeviceEventContent, }, - DeviceIdBox, DeviceKeyAlgorithm, UserId, + DeviceId, DeviceIdBox, DeviceKeyAlgorithm, RoomId, UserId, }; use thiserror::Error; @@ -83,6 +83,7 @@ pub struct QrVerification { inner: Arc, state: Arc>, identities: IdentitiesBeingVerified, + we_started: bool, } impl std::fmt::Debug for QrVerification { @@ -115,6 +116,34 @@ impl QrVerification { self.identities.other_user_id() } + /// Get the device id of the other side. + pub fn other_device_id(&self) -> &DeviceId { + self.identities.other_device_id() + } + + /// Did we initiate the verification request + pub fn we_started(&self) -> bool { + self.we_started + } + + /// Get the `CancelCode` that cancelled this verification request. + pub fn cancel_code(&self) -> Option { + if let InnerState::Cancelled(c) = &*self.state.lock().unwrap() { + Some(c.state.cancel_code.to_owned()) + } else { + None + } + } + + /// Has the verification flow been cancelled by us. + pub fn cancelled_by_us(&self) -> Option { + if let InnerState::Cancelled(c) = &*self.state.lock().unwrap() { + Some(c.state.cancelled_by_us) + } else { + None + } + } + /// Has the verification flow completed. pub fn is_done(&self) -> bool { matches!(&*self.state.lock().unwrap(), InnerState::Done(_)) @@ -135,6 +164,14 @@ impl QrVerification { &self.flow_id } + /// Get the room id if the verification is happening inside a room. + pub fn room_id(&self) -> Option<&RoomId> { + match self.flow_id() { + FlowId::ToDevice(_) => None, + FlowId::InRoom(r, _) => Some(r), + } + } + /// Generate a QR code object that is representing this verification flow. /// /// The `QrCode` can then be rendered as an image or as an unicode string. @@ -159,6 +196,38 @@ impl QrVerification { self.cancel_with_code(CancelCode::User).map(|c| self.content_to_request(c)) } + /// Cancel the verification. + /// + /// This cancels the verification with given `CancelCode`. + /// + /// **Note**: This method should generally not be used, the [`cancel()`] + /// method should be preferred. The SDK will automatically cancel with the + /// approprate cancel code, user initiated cancellations should only cancel + /// with the `CancelCode::User` + /// + /// Returns None if the `Sas` object is already in a canceled state, + /// otherwise it returns a request that needs to be sent out. + /// + /// [`cancel()`]: #method.cancel + pub fn cancel_with_code(&self, code: CancelCode) -> Option { + let new_state = QrState::::new(true, code); + let content = new_state.as_content(self.flow_id()); + + let mut state = self.state.lock().unwrap(); + + match &*state { + InnerState::Confirmed(_) + | InnerState::Created(_) + | InnerState::Scanned(_) + | InnerState::Reciprocated(_) + | InnerState::Done(_) => { + *state = InnerState::Cancelled(new_state); + Some(content) + } + InnerState::Cancelled(_) => None, + } + } + /// Notify the other side that we have successfully scanned the QR code and /// that the QR verification flow can start. /// @@ -209,25 +278,6 @@ impl QrVerification { } } - fn cancel_with_code(&self, code: CancelCode) -> Option { - let new_state = QrState::::new(true, code); - let content = new_state.as_content(self.flow_id()); - - let mut state = self.state.lock().unwrap(); - - match &*state { - InnerState::Confirmed(_) - | InnerState::Created(_) - | InnerState::Scanned(_) - | InnerState::Reciprocated(_) - | InnerState::Done(_) => { - *state = InnerState::Cancelled(new_state); - Some(content) - } - InnerState::Cancelled(_) => None, - } - } - async fn mark_as_done( &self, new_state: QrState, @@ -351,6 +401,7 @@ impl QrVerification { own_master_key: String, other_device_key: String, identities: IdentitiesBeingVerified, + we_started: bool, ) -> Self { let secret = Self::generate_secret(); @@ -362,7 +413,7 @@ impl QrVerification { ) .into(); - Self::new_helper(store, flow_id, inner, identities) + Self::new_helper(store, flow_id, inner, identities, we_started) } pub(crate) fn new_self_no_master( @@ -371,6 +422,7 @@ impl QrVerification { flow_id: FlowId, own_master_key: String, identities: IdentitiesBeingVerified, + we_started: bool, ) -> QrVerification { let secret = Self::generate_secret(); @@ -382,7 +434,7 @@ impl QrVerification { ) .into(); - Self::new_helper(store, flow_id, inner, identities) + Self::new_helper(store, flow_id, inner, identities, we_started) } pub(crate) fn new_cross( @@ -391,6 +443,7 @@ impl QrVerification { own_master_key: String, other_master_key: String, identities: IdentitiesBeingVerified, + we_started: bool, ) -> Self { let secret = Self::generate_secret(); @@ -403,9 +456,10 @@ impl QrVerification { let inner: QrVerificationData = VerificationData::new(event_id, own_master_key, other_master_key, secret).into(); - Self::new_helper(store, flow_id, inner, identities) + Self::new_helper(store, flow_id, inner, identities, we_started) } + #[allow(clippy::too_many_arguments)] pub(crate) async fn from_scan( store: Arc, own_account: ReadOnlyAccount, @@ -414,6 +468,7 @@ impl QrVerification { other_device_id: DeviceIdBox, flow_id: FlowId, qr_code: QrVerificationData, + we_started: bool, ) -> Result { if flow_id.as_str() != qr_code.flow_id() { return Err(ScanError::FlowIdMismatch { @@ -510,6 +565,7 @@ impl QrVerification { })) .into(), identities, + we_started, }) } @@ -518,6 +574,7 @@ impl QrVerification { flow_id: FlowId, inner: QrVerificationData, identities: IdentitiesBeingVerified, + we_started: bool, ) -> Self { let secret = inner.secret().to_owned(); @@ -527,6 +584,7 @@ impl QrVerification { inner: inner.into(), state: Mutex::new(InnerState::Created(QrState { state: Created { secret } })).into(), identities, + we_started, } } } @@ -747,6 +805,7 @@ mod test { flow_id.clone(), master_key.clone(), identities.clone(), + false, ); assert_eq!(verification.inner.first_key(), &device_key); @@ -758,6 +817,7 @@ mod test { master_key.clone(), device_key.clone(), identities.clone(), + false, ); assert_eq!(verification.inner.first_key(), &master_key); @@ -775,6 +835,7 @@ mod test { master_key.clone(), bob_master_key.clone(), identities, + false, ); assert_eq!(verification.inner.first_key(), &master_key); @@ -818,6 +879,7 @@ mod test { flow_id.clone(), master_key.clone(), identities, + false, ); let bob_store = memory_store(); @@ -838,6 +900,7 @@ mod test { alice_account.device_id().to_owned(), flow_id, qr_code, + false, ) .await .unwrap(); diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index 14ed1f90..e858ceca 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -277,7 +277,7 @@ impl VerificationRequest { /// Generate a QR code that can be used by another client to start a QR code /// based verification. pub async fn generate_qr_code(&self) -> Result, CryptoStoreError> { - self.inner.lock().unwrap().generate_qr_code().await + self.inner.lock().unwrap().generate_qr_code(self.we_started).await } /// Start a QR code verification by providing a scanned QR code for this @@ -303,6 +303,7 @@ impl VerificationRequest { r.state.other_device_id.clone(), r.flow_id.as_ref().to_owned(), data, + self.we_started, ) .await?, )) @@ -550,11 +551,11 @@ impl InnerRequest { }) } - async fn generate_qr_code(&self) -> Result, CryptoStoreError> { + async fn generate_qr_code(&self, we_started: bool) -> Result, CryptoStoreError> { match self { InnerRequest::Created(_) => Ok(None), InnerRequest::Requested(_) => Ok(None), - InnerRequest::Ready(s) => s.generate_qr_code().await, + InnerRequest::Ready(s) => s.generate_qr_code(we_started).await, InnerRequest::Passive(_) => Ok(None), InnerRequest::Done(_) => Ok(None), InnerRequest::Cancelled(_) => Ok(None), @@ -783,7 +784,10 @@ impl RequestState { ) } - async fn generate_qr_code(&self) -> Result, CryptoStoreError> { + async fn generate_qr_code( + &self, + we_started: bool, + ) -> Result, CryptoStoreError> { // TODO return an error explaining why we can't generate a QR code? // If we didn't state that we support showing QR codes or if the other @@ -829,6 +833,7 @@ impl RequestState { .unwrap() .to_owned(), identites, + we_started, )) } else { Some(QrVerification::new_self_no_master( @@ -837,6 +842,7 @@ impl RequestState { self.flow_id.as_ref().to_owned(), i.master_key().get_first_key().unwrap().to_owned(), identites, + we_started, )) } } @@ -852,6 +858,7 @@ impl RequestState { .to_owned(), i.master_key().get_first_key().unwrap().to_owned(), identites, + we_started, )), } } else { From 100a936f1bfe2d404dc35e19cb7920dab90733aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 28 Jun 2021 09:53:56 +0200 Subject: [PATCH 08/35] crypto: Add a method to check if the scanning has been confirmed --- matrix_sdk_crypto/src/verification/qrcode.rs | 7 ++++++- matrix_sdk_crypto/src/verification/requests.rs | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/qrcode.rs b/matrix_sdk_crypto/src/verification/qrcode.rs index 0b414b80..b1e7adfd 100644 --- a/matrix_sdk_crypto/src/verification/qrcode.rs +++ b/matrix_sdk_crypto/src/verification/qrcode.rs @@ -101,10 +101,15 @@ impl QrVerification { /// /// When the verification object is in this state it's required that the /// user confirms that the other side has scanned the QR code. - pub fn is_scanned(&self) -> bool { + pub fn has_been_scanned(&self) -> bool { matches!(&*self.state.lock().unwrap(), InnerState::Scanned(_)) } + /// Has the scanning of the QR code been confirmed by us. + pub fn has_been_confirmed(&self) -> bool { + matches!(&*self.state.lock().unwrap(), InnerState::Confirmed(_)) + } + /// Get our own user id. pub fn user_id(&self) -> &UserId { self.identities.user_id() diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index e858ceca..ac3cbf8e 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -551,7 +551,10 @@ impl InnerRequest { }) } - async fn generate_qr_code(&self, we_started: bool) -> Result, CryptoStoreError> { + async fn generate_qr_code( + &self, + we_started: bool, + ) -> Result, CryptoStoreError> { match self { InnerRequest::Created(_) => Ok(None), InnerRequest::Requested(_) => Ok(None), From 63659c960476c28acece2e7b0e5d5767eac84355 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 28 Jun 2021 15:27:01 +0200 Subject: [PATCH 09/35] crypto: Fix verification requests getting cancelled --- matrix_sdk_crypto/src/verification/requests.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index ac3cbf8e..1db22561 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -436,7 +436,12 @@ impl VerificationRequest { pub(crate) fn receive_cancel(&self, sender: &UserId, content: &CancelContent<'_>) { if sender == self.other_user() { - let mut inner = self.inner.lock().unwrap().clone(); + trace!( + sender = sender.as_str(), + code = content.cancel_code().as_str(), + "Cancelling a verification request, other user has cancelled" + ); + let mut inner = self.inner.lock().unwrap(); inner.cancel(false, content.cancel_code()); } } @@ -548,7 +553,7 @@ impl InnerRequest { InnerRequest::Passive(s) => s.clone().into_canceled(cancelled_by_us, cancel_code), InnerRequest::Done(_) => return, InnerRequest::Cancelled(_) => return, - }) + }); } async fn generate_qr_code( From ee6b804804215276f35c56afc34093ef2a134355 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 28 Jun 2021 15:28:30 +0200 Subject: [PATCH 10/35] crypto: Allow QR code verifications to get into the cancelled state as well. --- matrix_sdk_crypto/src/verification/machine.rs | 13 ++++++-- matrix_sdk_crypto/src/verification/qrcode.rs | 31 ++++++++++++++++++- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index 494f9b65..a2a4abab 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -287,9 +287,16 @@ impl VerificationMachine { verification.receive_cancel(event.sender(), c); } - if let Some(sas) = self.get_sas(event.sender(), flow_id.as_str()) { - // This won't produce an outgoing content - let _ = sas.receive_any_event(event.sender(), &content); + if let Some(verification) = + self.get_verification(event.sender(), flow_id.as_str()) + { + match verification { + Verification::SasV1(sas) => { + // This won't produce an outgoing content + let _ = sas.receive_any_event(event.sender(), &content); + } + Verification::QrV1(qr) => qr.receive_cancel(event.sender(), c), + } } } AnyVerificationContent::Ready(c) => { diff --git a/matrix_sdk_crypto/src/verification/qrcode.rs b/matrix_sdk_crypto/src/verification/qrcode.rs index b1e7adfd..0dd4b576 100644 --- a/matrix_sdk_crypto/src/verification/qrcode.rs +++ b/matrix_sdk_crypto/src/verification/qrcode.rs @@ -36,9 +36,10 @@ use ruma::{ DeviceId, DeviceIdBox, DeviceKeyAlgorithm, RoomId, UserId, }; use thiserror::Error; +use tracing::trace; use super::{ - event_enums::{DoneContent, OutgoingContent, OwnedStartContent, StartContent}, + event_enums::{CancelContent, DoneContent, OutgoingContent, OwnedStartContent, StartContent}, Cancelled, Done, FlowId, IdentitiesBeingVerified, VerificationResult, }; use crate::{ @@ -393,6 +394,28 @@ impl QrVerification { } } + pub(crate) fn receive_cancel(&self, sender: &UserId, content: &CancelContent<'_>) { + if sender == self.other_user_id() { + let mut state = self.state.lock().unwrap(); + + let new_state = match &*state { + InnerState::Created(s) => s.clone().into_cancelled(content), + InnerState::Scanned(s) => s.clone().into_cancelled(content), + InnerState::Confirmed(s) => s.clone().into_cancelled(content), + InnerState::Reciprocated(s) => s.clone().into_cancelled(content), + InnerState::Done(_) | InnerState::Cancelled(_) => return, + }; + + trace!( + sender = sender.as_str(), + code = content.cancel_code().as_str(), + "Cancelling a QR verification, other user has cancelled" + ); + + *state = InnerState::Cancelled(new_state); + } + } + fn generate_secret() -> String { let mut shared_secret = [0u8; SECRET_SIZE]; getrandom::getrandom(&mut shared_secret) @@ -609,6 +632,12 @@ struct QrState { state: S, } +impl QrState { + pub fn into_cancelled(self, content: &CancelContent<'_>) -> QrState { + QrState { state: Cancelled::new(false, content.cancel_code().to_owned()) } + } +} + #[derive(Clone, Debug)] struct Created { secret: String, From 113587247eed9a20aec2835c1faed97767ccbb00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 28 Jun 2021 16:30:41 +0200 Subject: [PATCH 11/35] crypto: Notify our users that a verification timed out on our end --- matrix_sdk_crypto/src/machine.rs | 4 +-- .../src/verification/event_enums.rs | 6 ++--- matrix_sdk_crypto/src/verification/machine.rs | 26 ++++++++++++++++--- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 75b1fd40..68b4426b 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -789,7 +789,7 @@ impl OlmMachine { one_time_keys_counts: &BTreeMap, ) -> OlmResult { // Remove verification objects that have expired or are done. - self.verification_machine.garbage_collect(); + let mut events = self.verification_machine.garbage_collect(); // Always save the account, a new session might get created which also // touches the account. @@ -804,8 +804,6 @@ impl OlmMachine { } } - let mut events = Vec::new(); - for mut raw_event in to_device_events.events { let event = match raw_event.deserialize() { Ok(e) => e, diff --git a/matrix_sdk_crypto/src/verification/event_enums.rs b/matrix_sdk_crypto/src/verification/event_enums.rs index 39b9d195..9c63f540 100644 --- a/matrix_sdk_crypto/src/verification/event_enums.rs +++ b/matrix_sdk_crypto/src/verification/event_enums.rs @@ -666,7 +666,8 @@ impl From<(RoomId, AnyMessageEventContent)> for OutgoingContent { } #[cfg(test)] -use crate::{OutgoingRequest, OutgoingVerificationRequest, RoomMessageRequest, ToDeviceRequest}; +use crate::OutgoingVerificationRequest; +use crate::{OutgoingRequest, RoomMessageRequest, ToDeviceRequest}; #[cfg(test)] impl From for OutgoingContent { @@ -678,14 +679,12 @@ impl From for OutgoingContent { } } -#[cfg(test)] impl From for OutgoingContent { fn from(value: RoomMessageRequest) -> Self { (value.room_id, value.content).into() } } -#[cfg(test)] impl TryFrom for OutgoingContent { type Error = String; @@ -736,7 +735,6 @@ impl TryFrom for OutgoingContent { } } -#[cfg(test)] impl TryFrom for OutgoingContent { type Error = String; diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index a2a4abab..330f02cc 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -12,11 +12,18 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{convert::TryFrom, sync::Arc}; +use std::{ + convert::{TryFrom, TryInto}, + sync::Arc, +}; use dashmap::DashMap; use matrix_sdk_common::{locks::Mutex, uuid::Uuid}; -use ruma::{DeviceId, MilliSecondsSinceUnixEpoch, UserId}; +use ruma::{ + events::{AnyToDeviceEvent, AnyToDeviceEventContent, ToDeviceEvent}, + serde::Raw, + DeviceId, MilliSecondsSinceUnixEpoch, UserId, +}; use tracing::{info, trace, warn}; use super::{ @@ -165,15 +172,28 @@ impl VerificationMachine { self.verifications.outgoing_requests() } - pub fn garbage_collect(&self) { + pub fn garbage_collect(&self) -> Vec> { + let mut events = vec![]; + for user_verification in self.requests.iter() { user_verification.retain(|_, v| !(v.is_done() || v.is_cancelled())); } self.requests.retain(|_, v| !v.is_empty()); for request in self.verifications.garbage_collect() { + if let Ok(OutgoingContent::ToDevice(AnyToDeviceEventContent::KeyVerificationCancel( + content, + ))) = request.clone().try_into() + { + let event = ToDeviceEvent { content, sender: self.account.user_id().to_owned() }; + + events.push(AnyToDeviceEvent::KeyVerificationCancel(event).into()); + } + self.verifications.add_request(request) } + + events } async fn mark_sas_as_done( From 9052843acbc762388f4ae59e6dad0d580914cf59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 29 Jun 2021 09:16:22 +0200 Subject: [PATCH 12/35] crypto: Add another SAS state so we know when both parties accepted --- .../src/verification/sas/inner_sas.rs | 33 ++++++++++++++-- matrix_sdk_crypto/src/verification/sas/mod.rs | 39 +++++++++++++------ .../src/verification/sas/sas_state.rs | 34 ++++++++++++++-- 3 files changed, 88 insertions(+), 18 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/sas/inner_sas.rs b/matrix_sdk_crypto/src/verification/sas/inner_sas.rs index d8f35332..fd6fe7a1 100644 --- a/matrix_sdk_crypto/src/verification/sas/inner_sas.rs +++ b/matrix_sdk_crypto/src/verification/sas/inner_sas.rs @@ -24,6 +24,7 @@ use ruma::{ use super::{ sas_state::{ Accepted, Confirmed, Created, KeyReceived, MacReceived, SasState, Started, WaitingForDone, + WeAccepted, }, FlowId, }; @@ -41,6 +42,7 @@ pub enum InnerSas { Created(SasState), Started(SasState), Accepted(SasState), + WeAccepted(SasState), KeyReceived(SasState), Confirmed(SasState), MacReceived(SasState), @@ -65,6 +67,7 @@ impl InnerSas { match self { InnerSas::Created(s) => s.started_from_request, InnerSas::Started(s) => s.started_from_request, + InnerSas::WeAccepted(s) => s.started_from_request, InnerSas::Accepted(s) => s.started_from_request, InnerSas::KeyReceived(s) => s.started_from_request, InnerSas::Confirmed(s) => s.started_from_request, @@ -75,6 +78,19 @@ impl InnerSas { } } + pub fn has_been_accepted(&self) -> bool { + match self { + InnerSas::Created(_) | InnerSas::Started(_) | InnerSas::Cancelled(_) => false, + InnerSas::Accepted(_) + | InnerSas::WeAccepted(_) + | InnerSas::KeyReceived(_) + | InnerSas::Confirmed(_) + | InnerSas::MacReceived(_) + | InnerSas::WaitingForDone(_) + | InnerSas::Done(_) => true, + } + } + pub fn supports_emoji(&self) -> bool { match self { InnerSas::Created(_) => false, @@ -83,6 +99,11 @@ impl InnerSas { .accepted_protocols .short_auth_string .contains(&ShortAuthenticationString::Emoji), + InnerSas::WeAccepted(s) => s + .state + .accepted_protocols + .short_auth_string + .contains(&ShortAuthenticationString::Emoji), InnerSas::Accepted(s) => s .state .accepted_protocols @@ -144,9 +165,11 @@ impl InnerSas { } } - pub fn accept(&self) -> Option { + pub fn accept(self) -> Option<(InnerSas, OwnedAcceptContent)> { if let InnerSas::Started(s) = self { - Some(s.as_content()) + let sas = s.into_accepted(); + let content = sas.as_content(); + Some((InnerSas::WeAccepted(sas), content)) } else { None } @@ -165,6 +188,7 @@ impl InnerSas { InnerSas::MacReceived(s) => s.set_creation_time(time), InnerSas::Done(s) => s.set_creation_time(time), InnerSas::WaitingForDone(s) => s.set_creation_time(time), + InnerSas::WeAccepted(s) => s.set_creation_time(time), } } @@ -177,6 +201,7 @@ impl InnerSas { InnerSas::Created(s) => s.cancel(cancelled_by_us, code), InnerSas::Started(s) => s.cancel(cancelled_by_us, code), InnerSas::Accepted(s) => s.cancel(cancelled_by_us, code), + InnerSas::WeAccepted(s) => s.cancel(cancelled_by_us, code), InnerSas::KeyReceived(s) => s.cancel(cancelled_by_us, code), InnerSas::MacReceived(s) => s.cancel(cancelled_by_us, code), _ => return (self, None), @@ -245,7 +270,7 @@ impl InnerSas { (InnerSas::Cancelled(s), Some(content)) } }, - InnerSas::Started(s) => match s.into_key_received(sender, c) { + InnerSas::WeAccepted(s) => match s.into_key_received(sender, c) { Ok(s) => { let content = s.as_content(); (InnerSas::KeyReceived(s), Some(content)) @@ -347,6 +372,7 @@ impl InnerSas { InnerSas::MacReceived(s) => s.timed_out(), InnerSas::WaitingForDone(s) => s.timed_out(), InnerSas::Done(s) => s.timed_out(), + InnerSas::WeAccepted(s) => s.timed_out(), } } @@ -361,6 +387,7 @@ impl InnerSas { InnerSas::MacReceived(s) => s.verification_flow_id.clone(), InnerSas::WaitingForDone(s) => s.verification_flow_id.clone(), InnerSas::Done(s) => s.verification_flow_id.clone(), + InnerSas::WeAccepted(s) => s.verification_flow_id.clone(), } } diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index 0079a4b0..eb4b78af 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -119,6 +119,11 @@ impl Sas { self.inner.lock().unwrap().have_we_confirmed() } + /// Has the verification been accepted by both parties. + pub fn has_been_accepted(&self) -> bool { + self.inner.lock().unwrap().has_been_accepted() + } + /// Get the cancel code of this SAS verification if it has been cancelled pub fn cancel_code(&self) -> Option { self.inner.lock().unwrap().cancel_code() @@ -310,18 +315,28 @@ impl Sas { &self, settings: AcceptSettings, ) -> Option { - self.inner.lock().unwrap().accept().map(|c| match settings.apply(c) { - OwnedAcceptContent::ToDevice(c) => { - let content = AnyToDeviceEventContent::KeyVerificationAccept(c); - self.content_to_request(content).into() - } - OwnedAcceptContent::Room(room_id, content) => RoomMessageRequest { - room_id, - txn_id: Uuid::new_v4(), - content: AnyMessageEventContent::KeyVerificationAccept(content), - } - .into(), - }) + let mut guard = self.inner.lock().unwrap(); + let sas: InnerSas = (*guard).clone(); + + if let Some((sas, content)) = sas.accept() { + *guard = sas; + let content = settings.apply(content); + + Some(match content { + OwnedAcceptContent::ToDevice(c) => { + let content = AnyToDeviceEventContent::KeyVerificationAccept(c); + self.content_to_request(content).into() + } + OwnedAcceptContent::Room(room_id, content) => RoomMessageRequest { + room_id, + txn_id: Uuid::new_v4(), + content: AnyMessageEventContent::KeyVerificationAccept(content), + } + .into(), + }) + } else { + None + } } /// Confirm the Sas verification. diff --git a/matrix_sdk_crypto/src/verification/sas/sas_state.rs b/matrix_sdk_crypto/src/verification/sas/sas_state.rs index eb9d78c8..af5dad17 100644 --- a/matrix_sdk_crypto/src/verification/sas/sas_state.rs +++ b/matrix_sdk_crypto/src/verification/sas/sas_state.rs @@ -241,6 +241,15 @@ pub struct Accepted { commitment: String, } +/// The SAS state we're going to be in after we accepted our +/// verification start event. +#[derive(Clone, Debug)] +pub struct WeAccepted { + we_started: bool, + pub accepted_protocols: Arc, + commitment: String, +} + /// The SAS state we're going to be in after we received the public key of the /// other participant. /// @@ -548,6 +557,24 @@ impl SasState { } } + pub fn into_accepted(self) -> SasState { + SasState { + inner: self.inner, + ids: self.ids, + verification_flow_id: self.verification_flow_id, + creation_time: self.creation_time, + last_event_time: self.last_event_time, + started_from_request: self.started_from_request, + state: Arc::new(WeAccepted { + we_started: false, + accepted_protocols: self.state.accepted_protocols.clone(), + commitment: self.state.commitment.clone(), + }), + } + } +} + +impl SasState { /// Get the content for the accept event. /// /// The content needs to be sent to the other device. @@ -1093,7 +1120,7 @@ mod test { }; use serde_json::json; - use super::{Accepted, Created, SasState, Started}; + use super::{Accepted, Created, SasState, Started, WeAccepted}; use crate::{ verification::event_enums::{AcceptContent, KeyContent, MacContent, StartContent}, ReadOnlyAccount, ReadOnlyDevice, @@ -1115,7 +1142,7 @@ mod test { "BOBDEVCIE".into() } - async fn get_sas_pair() -> (SasState, SasState) { + async fn get_sas_pair() -> (SasState, SasState) { let alice = ReadOnlyAccount::new(&alice_id(), &alice_device_id()); let alice_device = ReadOnlyDevice::from_account(&alice).await; @@ -1135,8 +1162,9 @@ mod test { &start_content.as_start_content(), false, ); + let bob_sas = bob_sas.unwrap().into_accepted(); - (alice_sas, bob_sas.unwrap()) + (alice_sas, bob_sas) } #[tokio::test] From db0843a47abd794f272a0d9fc220402d8e2264e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 1 Jul 2021 10:27:45 +0200 Subject: [PATCH 13/35] crypto: Don't panic if we don't have the keys to generate a QR code --- .../src/verification/requests.rs | 114 ++++++++++++------ 1 file changed, 77 insertions(+), 37 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index 1db22561..e84193e1 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -796,8 +796,6 @@ impl RequestState { &self, we_started: bool, ) -> Result, CryptoStoreError> { - // TODO return an error explaining why we can't generate a QR code? - // If we didn't state that we support showing QR codes or if the other // side doesn't support scanning QR codes bail early. if !self.state.our_methods.contains(&VerificationMethod::QrCodeShowV1) @@ -830,44 +828,86 @@ impl RequestState { let verification = if let Some(identity) = &identites.identity_being_verified { match &identity { UserIdentities::Own(i) => { - if identites.can_sign_devices().await { - Some(QrVerification::new_self( - self.store.clone(), - self.flow_id.as_ref().to_owned(), - i.master_key().get_first_key().unwrap().to_owned(), - identites - .other_device() - .get_key(DeviceKeyAlgorithm::Ed25519) - .unwrap() - .to_owned(), - identites, - we_started, - )) + if let Some(master_key) = i.master_key().get_first_key() { + if identites.can_sign_devices().await { + if let Some(device_key) = + identites.other_device().get_key(DeviceKeyAlgorithm::Ed25519) + { + Some(QrVerification::new_self( + self.store.clone(), + self.flow_id.as_ref().to_owned(), + master_key.to_owned(), + device_key.to_owned(), + identites, + we_started, + )) + } else { + warn!( + user_id = self.other_user_id.as_str(), + device_id = self.state.other_device_id.as_str(), + "Can't create a QR code, the other device \ + doesn't have a valid device key" + ); + None + } + } else { + Some(QrVerification::new_self_no_master( + self.account.clone(), + self.store.clone(), + self.flow_id.as_ref().to_owned(), + master_key.to_owned(), + identites, + we_started, + )) + } } else { - Some(QrVerification::new_self_no_master( - self.account.clone(), - self.store.clone(), - self.flow_id.as_ref().to_owned(), - i.master_key().get_first_key().unwrap().to_owned(), - identites, - we_started, - )) + warn!( + user_id = self.other_user_id.as_str(), + device_id = self.state.other_device_id.as_str(), + "Can't create a QR code, our cross signing identity \ + doesn't contain a valid master key" + ); + None + } + } + UserIdentities::Other(i) => { + if let Some(other_master) = i.master_key().get_first_key() { + // TODO we can get the master key from the public + // identity if we don't have the private one and we + // trust the public one. + if let Some(own_master) = self + .private_cross_signing_identity + .master_public_key() + .await + .and_then(|m| m.get_first_key().map(|m| m.to_owned())) + { + Some(QrVerification::new_cross( + self.store.clone(), + self.flow_id.as_ref().to_owned(), + own_master, + other_master.to_owned(), + identites, + we_started, + )) + } else { + warn!( + user_id = self.other_user_id.as_str(), + device_id = self.state.other_device_id.as_str(), + "Can't create a QR code, we don't trust our own \ + master key" + ); + None + } + } else { + warn!( + user_id = self.other_user_id.as_str(), + device_id = self.state.other_device_id.as_str(), + "Can't create a QR code, the user's identity \ + doesn't have a valid master key" + ); + None } } - UserIdentities::Other(i) => Some(QrVerification::new_cross( - self.store.clone(), - self.flow_id.as_ref().to_owned(), - self.private_cross_signing_identity - .master_public_key() - .await - .unwrap() - .get_first_key() - .unwrap() - .to_owned(), - i.master_key().get_first_key().unwrap().to_owned(), - identites, - we_started, - )), } } else { warn!( From c5df7c5356c2540ede95f41e3565e91ca7de5777 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 8 Jul 2021 12:30:30 +0200 Subject: [PATCH 14/35] crypto: Add methods to request verification from users --- matrix_sdk_crypto/src/identities/device.rs | 24 +- matrix_sdk_crypto/src/identities/manager.rs | 16 +- matrix_sdk_crypto/src/identities/mod.rs | 4 +- matrix_sdk_crypto/src/identities/user.rs | 239 +++++++++++++++--- matrix_sdk_crypto/src/lib.rs | 3 +- matrix_sdk_crypto/src/machine.rs | 14 +- matrix_sdk_crypto/src/olm/signing/mod.rs | 14 +- .../src/olm/signing/pk_signing.rs | 4 +- matrix_sdk_crypto/src/store/memorystore.rs | 6 +- matrix_sdk_crypto/src/store/mod.rs | 29 ++- matrix_sdk_crypto/src/store/sled.rs | 4 +- matrix_sdk_crypto/src/verification/machine.rs | 75 +++++- matrix_sdk_crypto/src/verification/mod.rs | 14 +- matrix_sdk_crypto/src/verification/qrcode.rs | 12 +- .../src/verification/requests.rs | 171 ++++++------- .../src/verification/sas/helpers.rs | 6 +- .../src/verification/sas/inner_sas.rs | 14 +- matrix_sdk_crypto/src/verification/sas/mod.rs | 12 +- .../src/verification/sas/sas_state.rs | 16 +- 19 files changed, 455 insertions(+), 222 deletions(-) diff --git a/matrix_sdk_crypto/src/identities/device.rs b/matrix_sdk_crypto/src/identities/device.rs index d9f6e8cc..79884c84 100644 --- a/matrix_sdk_crypto/src/identities/device.rs +++ b/matrix_sdk_crypto/src/identities/device.rs @@ -39,11 +39,11 @@ use tracing::warn; use super::{atomic_bool_deserializer, atomic_bool_serializer}; use crate::{ error::{EventError, OlmError, OlmResult, SignatureError}, - identities::{OwnUserIdentity, UserIdentities}, + identities::{ReadOnlyOwnUserIdentity, ReadOnlyUserIdentities}, olm::{InboundGroupSession, PrivateCrossSigningIdentity, Session, Utility}, store::{Changes, CryptoStore, DeviceChanges, Result as StoreResult}, verification::VerificationMachine, - OutgoingVerificationRequest, Sas, ToDeviceRequest, + OutgoingVerificationRequest, Sas, ToDeviceRequest, VerificationRequest, }; #[cfg(test)] use crate::{OlmMachine, ReadOnlyAccount}; @@ -104,8 +104,8 @@ pub struct Device { pub(crate) inner: ReadOnlyDevice, pub(crate) private_identity: Arc>, pub(crate) verification_machine: VerificationMachine, - pub(crate) own_identity: Option, - pub(crate) device_owner_identity: Option, + pub(crate) own_identity: Option, + pub(crate) device_owner_identity: Option, } impl std::fmt::Debug for Device { @@ -228,8 +228,8 @@ pub struct UserDevices { pub(crate) inner: HashMap, pub(crate) private_identity: Arc>, pub(crate) verification_machine: VerificationMachine, - pub(crate) own_identity: Option, - pub(crate) device_owner_identity: Option, + pub(crate) own_identity: Option, + pub(crate) device_owner_identity: Option, } impl UserDevices { @@ -382,16 +382,16 @@ impl ReadOnlyDevice { pub(crate) fn verified( &self, - own_identity: &Option, - device_owner: &Option, + own_identity: &Option, + device_owner: &Option, ) -> bool { self.is_locally_trusted() || self.is_cross_signing_trusted(own_identity, device_owner) } pub(crate) fn is_cross_signing_trusted( &self, - own_identity: &Option, - device_owner: &Option, + 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. @@ -401,14 +401,14 @@ impl ReadOnlyDevice { .map(|device_identity| match device_identity { // If it's one of our own devices, just check that // we signed the device. - UserIdentities::Own(_) => { + ReadOnlyUserIdentities::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) => { + ReadOnlyUserIdentities::Other(device_identity) => { own_identity.is_identity_signed(device_identity).map_or(false, |_| true) && device_identity.is_device_signed(self).map_or(false, |_| true) } diff --git a/matrix_sdk_crypto/src/identities/manager.rs b/matrix_sdk_crypto/src/identities/manager.rs index e8e7fedc..58763126 100644 --- a/matrix_sdk_crypto/src/identities/manager.rs +++ b/matrix_sdk_crypto/src/identities/manager.rs @@ -29,8 +29,8 @@ use tracing::{trace, warn}; use crate::{ error::OlmResult, identities::{ - MasterPubkey, OwnUserIdentity, ReadOnlyDevice, SelfSigningPubkey, UserIdentities, - UserIdentity, UserSigningPubkey, + MasterPubkey, ReadOnlyDevice, ReadOnlyOwnUserIdentity, ReadOnlyUserIdentities, + ReadOnlyUserIdentity, SelfSigningPubkey, UserSigningPubkey, }, requests::KeysQueryRequest, store::{Changes, DeviceChanges, IdentityChanges, Result as StoreResult, Store}, @@ -246,7 +246,7 @@ impl IdentityManager { let result = if let Some(mut i) = self.store.get_user_identity(user_id).await? { match &mut i { - UserIdentities::Own(ref mut identity) => { + ReadOnlyUserIdentities::Own(ref mut identity) => { let user_signing = if let Some(s) = response.user_signing_keys.get(user_id) { UserSigningPubkey::from(s) @@ -261,7 +261,7 @@ impl IdentityManager { identity.update(master_key, self_signing, user_signing).map(|_| (i, false)) } - UserIdentities::Other(ref mut identity) => { + ReadOnlyUserIdentities::Other(ref mut identity) => { identity.update(master_key, self_signing).map(|_| (i, false)) } } @@ -280,8 +280,8 @@ impl IdentityManager { continue; } - OwnUserIdentity::new(master_key, self_signing, user_signing) - .map(|i| (UserIdentities::Own(i), true)) + ReadOnlyOwnUserIdentity::new(master_key, self_signing, user_signing) + .map(|i| (ReadOnlyUserIdentities::Own(i), true)) } else { warn!( "User identity for our own user {} didn't contain a \ @@ -294,8 +294,8 @@ impl IdentityManager { warn!("User id mismatch in one of the cross signing keys for user {}", user_id); continue; } else { - UserIdentity::new(master_key, self_signing) - .map(|i| (UserIdentities::Other(i), true)) + ReadOnlyUserIdentity::new(master_key, self_signing) + .map(|i| (ReadOnlyUserIdentities::Other(i), true)) }; match result { diff --git a/matrix_sdk_crypto/src/identities/mod.rs b/matrix_sdk_crypto/src/identities/mod.rs index 873cecfa..d90206b3 100644 --- a/matrix_sdk_crypto/src/identities/mod.rs +++ b/matrix_sdk_crypto/src/identities/mod.rs @@ -53,8 +53,8 @@ pub use device::{Device, LocalTrust, ReadOnlyDevice, UserDevices}; pub(crate) use manager::IdentityManager; use serde::{Deserialize, Deserializer, Serializer}; pub use user::{ - MasterPubkey, OwnUserIdentity, SelfSigningPubkey, UserIdentities, UserIdentity, - UserSigningPubkey, + MasterPubkey, OwnUserIdentity, ReadOnlyOwnUserIdentity, ReadOnlyUserIdentities, + ReadOnlyUserIdentity, SelfSigningPubkey, UserIdentities, UserIdentity, UserSigningPubkey, }; // These methods are only here because Serialize and Deserialize don't seem to diff --git a/matrix_sdk_crypto/src/identities/user.rs b/matrix_sdk_crypto/src/identities/user.rs index 184d64c2..c124d4e4 100644 --- a/matrix_sdk_crypto/src/identities/user.rs +++ b/matrix_sdk_crypto/src/identities/user.rs @@ -15,6 +15,7 @@ use std::{ collections::{btree_map::Iter, BTreeMap}, convert::TryFrom, + ops::Deref, sync::{ atomic::{AtomicBool, Ordering}, Arc, @@ -23,7 +24,10 @@ use std::{ use ruma::{ encryption::{CrossSigningKey, KeyUsage}, - DeviceKeyId, UserId, + events::{ + key::verification::VerificationMethod, room::message::KeyVerificationRequestEventContent, + }, + DeviceKeyId, EventId, RoomId, UserId, }; use serde::{Deserialize, Serialize}; use serde_json::to_value; @@ -31,7 +35,153 @@ use serde_json::to_value; use super::{atomic_bool_deserializer, atomic_bool_serializer}; #[cfg(test)] use crate::olm::PrivateCrossSigningIdentity; -use crate::{error::SignatureError, olm::Utility, ReadOnlyDevice}; +use crate::{ + error::SignatureError, olm::Utility, verification::VerificationMachine, CryptoStoreError, + OutgoingVerificationRequest, ReadOnlyDevice, VerificationRequest, +}; + +/// Enum over the different user identity types we can have. +#[derive(Debug, Clone)] +pub enum UserIdentities { + /// Our own user identity. + Own(OwnUserIdentity), + /// An identity belonging to another user. + Other(UserIdentity), +} + +impl UserIdentities { + /// Destructure the enum into an `OwnUserIdentity` if it's of the correct + /// type. + pub fn own(self) -> Option { + match self { + Self::Own(i) => Some(i), + _ => None, + } + } + + /// Destructure the enum into an `UserIdentity` if it's of the correct + /// type. + pub fn other(self) -> Option { + match self { + Self::Other(i) => Some(i), + _ => None, + } + } +} + +impl From for UserIdentities { + fn from(i: OwnUserIdentity) -> Self { + Self::Own(i) + } +} + +impl From for UserIdentities { + fn from(i: UserIdentity) -> Self { + Self::Other(i) + } +} + +/// Struct representing a cross signing identity of a user. +/// +/// This is the user identity of a user that isn't our own. Other users will +/// only contain a master key and a self signing key, meaning that only device +/// signatures can be checked with this identity. +/// +/// This struct wraps a read-only version of the struct and allows verifications +/// to be requested to verify our own device with the user identity. +#[derive(Debug, Clone)] +pub struct OwnUserIdentity { + pub(crate) inner: ReadOnlyOwnUserIdentity, + pub(crate) verification_machine: VerificationMachine, +} + +impl Deref for OwnUserIdentity { + type Target = ReadOnlyOwnUserIdentity; + + fn deref(&self) -> &Self::Target { + &self.inner + } +} + +impl OwnUserIdentity { + /// Send a verification request to our other devices. + pub async fn request_verification( + &self, + ) -> Result<(VerificationRequest, OutgoingVerificationRequest), CryptoStoreError> { + self.verification_machine.request_self_verification(&self.inner, None).await + } + + /// Send a verification request to our other devices while specifying our + /// supported methods. + /// + /// # Arguments + /// + /// * `methods` - The verification methods that we're supporting. + pub async fn request_verification_with_methods( + &self, + methods: Vec, + ) -> Result<(VerificationRequest, OutgoingVerificationRequest), CryptoStoreError> { + self.verification_machine.request_self_verification(&self.inner, Some(methods)).await + } +} + +/// Struct representing a cross signing identity of a user. +/// +/// This is the user identity of a user that isn't our own. Other users will +/// only contain a master key and a self signing key, meaning that only device +/// signatures can be checked with this identity. +/// +/// This struct wraps a read-only version of the struct and allows verifications +/// to be requested to verify our own device with the user identity. +#[derive(Debug, Clone)] +pub struct UserIdentity { + pub(crate) inner: ReadOnlyUserIdentity, + pub(crate) verification_machine: VerificationMachine, +} + +impl Deref for UserIdentity { + type Target = ReadOnlyUserIdentity; + + fn deref(&self) -> &Self::Target { + &self.inner + } +} + +impl UserIdentity { + /// Create a `VerificationRequest` object after the verification request + /// content has been sent out. + pub async fn request_verification( + &self, + room_id: &RoomId, + request_event_id: &EventId, + methods: Option>, + ) -> VerificationRequest { + self.verification_machine + .request_verification(&self.inner, room_id, request_event_id, methods) + .await + } + + /// Send a verification request to the given user. + /// + /// The returned content needs to be sent out into a DM room with the given + /// user. + /// + /// After the content has been sent out a `VerificationRequest` can be + /// started with the [`request_verification`] method. + /// + /// [`request_verification()`]: #method.request_verification + pub async fn verification_request_content( + &self, + methods: Option>, + ) -> KeyVerificationRequestEventContent { + VerificationRequest::request( + self.verification_machine.own_user_id(), + self.verification_machine.own_device_id(), + self.user_id(), + methods, + ) + } +} /// Wrapper for a cross signing key marking it as the master key. /// @@ -358,47 +508,47 @@ impl<'a> IntoIterator for &'a SelfSigningPubkey { /// Enum over the different user identity types we can have. #[derive(Debug, Clone, Serialize, Deserialize)] -pub enum UserIdentities { +pub enum ReadOnlyUserIdentities { /// Our own user identity. - Own(OwnUserIdentity), + Own(ReadOnlyOwnUserIdentity), /// Identities of other users. - Other(UserIdentity), + Other(ReadOnlyUserIdentity), } -impl From for UserIdentities { - fn from(identity: OwnUserIdentity) -> Self { - UserIdentities::Own(identity) +impl From for ReadOnlyUserIdentities { + fn from(identity: ReadOnlyOwnUserIdentity) -> Self { + ReadOnlyUserIdentities::Own(identity) } } -impl From for UserIdentities { - fn from(identity: UserIdentity) -> Self { - UserIdentities::Other(identity) +impl From for ReadOnlyUserIdentities { + fn from(identity: ReadOnlyUserIdentity) -> Self { + ReadOnlyUserIdentities::Other(identity) } } -impl UserIdentities { +impl ReadOnlyUserIdentities { /// The unique user id of this identity. pub fn user_id(&self) -> &UserId { match self { - UserIdentities::Own(i) => i.user_id(), - UserIdentities::Other(i) => i.user_id(), + ReadOnlyUserIdentities::Own(i) => i.user_id(), + ReadOnlyUserIdentities::Other(i) => i.user_id(), } } /// Get the master key of the identity. pub fn master_key(&self) -> &MasterPubkey { match self { - UserIdentities::Own(i) => i.master_key(), - UserIdentities::Other(i) => i.master_key(), + ReadOnlyUserIdentities::Own(i) => i.master_key(), + ReadOnlyUserIdentities::Other(i) => i.master_key(), } } /// Get the self-signing key of the identity. pub fn self_signing_key(&self) -> &SelfSigningPubkey { match self { - UserIdentities::Own(i) => &i.self_signing_key, - UserIdentities::Other(i) => &i.self_signing_key, + ReadOnlyUserIdentities::Own(i) => &i.self_signing_key, + ReadOnlyUserIdentities::Other(i) => &i.self_signing_key, } } @@ -406,32 +556,32 @@ impl UserIdentities { /// own user identity.. pub fn user_signing_key(&self) -> Option<&UserSigningPubkey> { match self { - UserIdentities::Own(i) => Some(&i.user_signing_key), - UserIdentities::Other(_) => None, + ReadOnlyUserIdentities::Own(i) => Some(&i.user_signing_key), + ReadOnlyUserIdentities::Other(_) => None, } } - /// Destructure the enum into an `OwnUserIdentity` if it's of the correct - /// type. - pub fn own(&self) -> Option<&OwnUserIdentity> { + /// Destructure the enum into an `ReadOnlyOwnUserIdentity` if it's of the + /// correct type. + pub fn own(&self) -> Option<&ReadOnlyOwnUserIdentity> { match self { - UserIdentities::Own(i) => Some(i), + ReadOnlyUserIdentities::Own(i) => Some(i), _ => None, } } /// Destructure the enum into an `UserIdentity` if it's of the correct /// type. - pub fn other(&self) -> Option<&UserIdentity> { + pub fn other(&self) -> Option<&ReadOnlyUserIdentity> { match self { - UserIdentities::Other(i) => Some(i), + ReadOnlyUserIdentities::Other(i) => Some(i), _ => None, } } } -impl PartialEq for UserIdentities { - fn eq(&self, other: &UserIdentities) -> bool { +impl PartialEq for ReadOnlyUserIdentities { + fn eq(&self, other: &ReadOnlyUserIdentities) -> bool { self.user_id() == other.user_id() } } @@ -442,13 +592,13 @@ impl PartialEq for UserIdentities { /// only contain a master key and a self signing key, meaning that only device /// signatures can be checked with this identity. #[derive(Debug, Clone, Deserialize, Serialize)] -pub struct UserIdentity { +pub struct ReadOnlyUserIdentity { user_id: Arc, pub(crate) master_key: MasterPubkey, self_signing_key: SelfSigningPubkey, } -impl UserIdentity { +impl ReadOnlyUserIdentity { /// Create a new user identity with the given master and self signing key. /// /// # Arguments @@ -543,7 +693,7 @@ impl UserIdentity { /// This identity can verify other identities as well as devices belonging to /// the identity. #[derive(Debug, Clone, Serialize, Deserialize)] -pub struct OwnUserIdentity { +pub struct ReadOnlyOwnUserIdentity { user_id: Arc, master_key: MasterPubkey, self_signing_key: SelfSigningPubkey, @@ -555,7 +705,7 @@ pub struct OwnUserIdentity { verified: Arc, } -impl OwnUserIdentity { +impl ReadOnlyOwnUserIdentity { /// Create a new own user identity with the given master, self signing, and /// user signing key. /// @@ -615,7 +765,10 @@ impl OwnUserIdentity { /// /// Returns an empty result if the signature check succeeded, otherwise a /// SignatureError indicating why the check failed. - pub fn is_identity_signed(&self, identity: &UserIdentity) -> Result<(), SignatureError> { + pub fn is_identity_signed( + &self, + identity: &ReadOnlyUserIdentity, + ) -> Result<(), SignatureError> { self.user_signing_key.verify_master_key(&identity.master_key) } @@ -692,7 +845,7 @@ pub(crate) mod test { use matrix_sdk_test::async_test; use ruma::{api::client::r0::keys::get_keys::Response as KeyQueryResponse, user_id}; - use super::{OwnUserIdentity, UserIdentities, UserIdentity}; + use super::{ReadOnlyOwnUserIdentity, ReadOnlyUserIdentities, ReadOnlyUserIdentity}; use crate::{ identities::{ manager::test::{other_key_query, own_key_query}, @@ -710,28 +863,29 @@ pub(crate) mod test { (first, second) } - fn own_identity(response: &KeyQueryResponse) -> OwnUserIdentity { + fn own_identity(response: &KeyQueryResponse) -> ReadOnlyOwnUserIdentity { let user_id = user_id!("@example:localhost"); let master_key = response.master_keys.get(&user_id).unwrap(); let user_signing = response.user_signing_keys.get(&user_id).unwrap(); let self_signing = response.self_signing_keys.get(&user_id).unwrap(); - OwnUserIdentity::new(master_key.into(), self_signing.into(), user_signing.into()).unwrap() + ReadOnlyOwnUserIdentity::new(master_key.into(), self_signing.into(), user_signing.into()) + .unwrap() } - pub(crate) fn get_own_identity() -> OwnUserIdentity { + pub(crate) fn get_own_identity() -> ReadOnlyOwnUserIdentity { own_identity(&own_key_query()) } - pub(crate) fn get_other_identity() -> UserIdentity { + pub(crate) fn get_other_identity() -> ReadOnlyUserIdentity { let user_id = user_id!("@example2:localhost"); let response = other_key_query(); let master_key = response.master_keys.get(&user_id).unwrap(); let self_signing = response.self_signing_keys.get(&user_id).unwrap(); - UserIdentity::new(master_key.into(), self_signing.into()).unwrap() + ReadOnlyUserIdentity::new(master_key.into(), self_signing.into()).unwrap() } #[test] @@ -743,7 +897,8 @@ pub(crate) mod test { let user_signing = response.user_signing_keys.get(&user_id).unwrap(); let self_signing = response.self_signing_keys.get(&user_id).unwrap(); - OwnUserIdentity::new(master_key.into(), self_signing.into(), user_signing.into()).unwrap(); + ReadOnlyOwnUserIdentity::new(master_key.into(), self_signing.into(), user_signing.into()) + .unwrap(); } #[test] @@ -773,7 +928,7 @@ pub(crate) mod test { verification_machine: verification_machine.clone(), private_identity: private_identity.clone(), own_identity: Some(identity.clone()), - device_owner_identity: Some(UserIdentities::Own(identity.clone())), + device_owner_identity: Some(ReadOnlyUserIdentities::Own(identity.clone())), }; let second = Device { @@ -781,7 +936,7 @@ pub(crate) mod test { verification_machine, private_identity, own_identity: Some(identity.clone()), - device_owner_identity: Some(UserIdentities::Own(identity.clone())), + device_owner_identity: Some(ReadOnlyUserIdentities::Own(identity.clone())), }; assert!(!second.is_locally_trusted()); diff --git a/matrix_sdk_crypto/src/lib.rs b/matrix_sdk_crypto/src/lib.rs index dcd4db02..a3d7a244 100644 --- a/matrix_sdk_crypto/src/lib.rs +++ b/matrix_sdk_crypto/src/lib.rs @@ -45,7 +45,8 @@ pub use file_encryption::{ DecryptorError, EncryptionInfo, KeyExportError, }; pub use identities::{ - Device, LocalTrust, OwnUserIdentity, ReadOnlyDevice, UserDevices, UserIdentities, UserIdentity, + Device, LocalTrust, OwnUserIdentity, ReadOnlyDevice, ReadOnlyOwnUserIdentity, + ReadOnlyUserIdentities, ReadOnlyUserIdentity, UserDevices, UserIdentities, UserIdentity, }; pub use machine::OlmMachine; pub use matrix_qrcode; diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 68b4426b..f2f4f22e 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -46,7 +46,7 @@ use tracing::{debug, error, info, trace, warn}; use crate::store::sled::SledStore; use crate::{ error::{EventError, MegolmError, MegolmResult, OlmError, OlmResult}, - identities::{Device, IdentityManager, UserDevices}, + identities::{user::UserIdentities, Device, IdentityManager, UserDevices}, key_request::KeyRequestMachine, olm::{ Account, EncryptionSettings, ExportedRoomKey, GroupSessionKey, IdentityKeys, @@ -1052,6 +1052,18 @@ impl OlmMachine { self.store.get_device(user_id, device_id).await } + /// Get the cross signing user identity of a user. + /// + /// # Arguments + /// + /// * `user_id` - The unique id of the user that the identity belongs to + /// + /// Returns a `UserIdentities` enum if one is found and the crypto store + /// didn't throw an error. + pub async fn get_identity(&self, user_id: &UserId) -> StoreResult> { + self.store.get_identity(user_id).await + } + /// Get a map holding all the devices of an user. /// /// # Arguments diff --git a/matrix_sdk_crypto/src/olm/signing/mod.rs b/matrix_sdk_crypto/src/olm/signing/mod.rs index 523c274b..469cdbce 100644 --- a/matrix_sdk_crypto/src/olm/signing/mod.rs +++ b/matrix_sdk_crypto/src/olm/signing/mod.rs @@ -34,7 +34,7 @@ use serde_json::Error as JsonError; use crate::{ error::SignatureError, identities::MasterPubkey, requests::UploadSigningKeysRequest, - OwnUserIdentity, ReadOnlyAccount, ReadOnlyDevice, UserIdentity, + ReadOnlyAccount, ReadOnlyDevice, ReadOnlyOwnUserIdentity, ReadOnlyUserIdentity, }; /// Private cross signing identity. @@ -117,7 +117,9 @@ impl PrivateCrossSigningIdentity { } } - pub(crate) async fn to_public_identity(&self) -> Result { + pub(crate) async fn to_public_identity( + &self, + ) -> Result { let master = self .master_key .lock() @@ -142,7 +144,7 @@ impl PrivateCrossSigningIdentity { .ok_or(SignatureError::MissingSigningKey)? .public_key .clone(); - let identity = OwnUserIdentity::new(master, self_signing, user_signing)?; + let identity = ReadOnlyOwnUserIdentity::new(master, self_signing, user_signing)?; identity.mark_as_verified(); Ok(identity) @@ -151,7 +153,7 @@ impl PrivateCrossSigningIdentity { /// Sign the given public user identity with this private identity. pub(crate) async fn sign_user( &self, - user_identity: &UserIdentity, + user_identity: &ReadOnlyUserIdentity, ) -> Result { let signed_keys = self .user_signing_key @@ -407,7 +409,7 @@ mod test { use super::{PrivateCrossSigningIdentity, Signing}; use crate::{ - identities::{ReadOnlyDevice, UserIdentity}, + identities::{ReadOnlyDevice, ReadOnlyUserIdentity}, olm::ReadOnlyAccount, }; @@ -518,7 +520,7 @@ mod test { let bob_account = ReadOnlyAccount::new(&user_id!("@bob:localhost"), "DEVICEID".into()); let (bob_private, _, _) = PrivateCrossSigningIdentity::new_with_account(&bob_account).await; - let mut bob_public = UserIdentity::from_private(&bob_private).await; + let mut bob_public = ReadOnlyUserIdentity::from_private(&bob_private).await; let user_signing = identity.user_signing_key.lock().await; let user_signing = user_signing.as_ref().unwrap(); diff --git a/matrix_sdk_crypto/src/olm/signing/pk_signing.rs b/matrix_sdk_crypto/src/olm/signing/pk_signing.rs index 00589e6d..61ba5bcd 100644 --- a/matrix_sdk_crypto/src/olm/signing/pk_signing.rs +++ b/matrix_sdk_crypto/src/olm/signing/pk_signing.rs @@ -37,7 +37,7 @@ use crate::{ error::SignatureError, identities::{MasterPubkey, SelfSigningPubkey, UserSigningPubkey}, utilities::{decode_url_safe as decode, encode_url_safe as encode, DecodeError}, - UserIdentity, + ReadOnlyUserIdentity, }; const NONCE_SIZE: usize = 12; @@ -186,7 +186,7 @@ impl UserSigning { pub async fn sign_user( &self, - user: &UserIdentity, + user: &ReadOnlyUserIdentity, ) -> Result>, SignatureError> { let user_master: &CrossSigningKey = user.master_key().as_ref(); let signature = self.inner.sign_json(serde_json::to_value(user_master)?).await?; diff --git a/matrix_sdk_crypto/src/store/memorystore.rs b/matrix_sdk_crypto/src/store/memorystore.rs index 348b0c79..ef52fd44 100644 --- a/matrix_sdk_crypto/src/store/memorystore.rs +++ b/matrix_sdk_crypto/src/store/memorystore.rs @@ -26,7 +26,7 @@ use super::{ Changes, CryptoStore, InboundGroupSession, ReadOnlyAccount, Result, Session, }; use crate::{ - identities::{ReadOnlyDevice, UserIdentities}, + identities::{ReadOnlyDevice, ReadOnlyUserIdentities}, key_request::OutgoingKeyRequest, olm::{OutboundGroupSession, PrivateCrossSigningIdentity}, }; @@ -44,7 +44,7 @@ pub struct MemoryStore { users_for_key_query: Arc>, olm_hashes: Arc>>, devices: DeviceStore, - identities: Arc>, + identities: Arc>, outgoing_key_requests: Arc>, key_requests_by_info: Arc>, } @@ -215,7 +215,7 @@ impl CryptoStore for MemoryStore { Ok(self.devices.user_devices(user_id)) } - async fn get_user_identity(&self, user_id: &UserId) -> Result> { + async fn get_user_identity(&self, user_id: &UserId) -> Result> { #[allow(clippy::map_clone)] Ok(self.identities.get(user_id).map(|i| i.clone())) } diff --git a/matrix_sdk_crypto/src/store/mod.rs b/matrix_sdk_crypto/src/store/mod.rs index 37c413cb..74f5aacf 100644 --- a/matrix_sdk_crypto/src/store/mod.rs +++ b/matrix_sdk_crypto/src/store/mod.rs @@ -66,7 +66,10 @@ use thiserror::Error; pub use self::sled::SledStore; use crate::{ error::SessionUnpicklingError, - identities::{Device, ReadOnlyDevice, UserDevices, UserIdentities}, + identities::{ + user::{OwnUserIdentity, UserIdentities, UserIdentity}, + Device, ReadOnlyDevice, ReadOnlyUserIdentities, UserDevices, + }, key_request::OutgoingKeyRequest, olm::{ InboundGroupSession, OlmMessageHash, OutboundGroupSession, PrivateCrossSigningIdentity, @@ -109,8 +112,8 @@ pub struct Changes { #[derive(Debug, Clone, Default)] #[allow(missing_docs)] pub struct IdentityChanges { - pub new: Vec, - pub changed: Vec, + pub new: Vec, + pub changed: Vec, } #[derive(Debug, Clone, Default)] @@ -215,8 +218,8 @@ impl Store { device_id: &DeviceId, ) -> Result> { let own_identity = - self.get_user_identity(&self.user_id).await?.map(|i| i.own().cloned()).flatten(); - let device_owner_identity = self.get_user_identity(user_id).await?; + self.inner.get_user_identity(&self.user_id).await?.map(|i| i.own().cloned()).flatten(); + let device_owner_identity = self.inner.get_user_identity(user_id).await?; Ok(self.inner.get_device(user_id, device_id).await?.map(|d| Device { inner: d, @@ -226,6 +229,20 @@ impl Store { device_owner_identity, })) } + + pub async fn get_identity(&self, user_id: &UserId) -> Result> { + Ok(self.inner.get_user_identity(user_id).await?.map(|i| match i { + ReadOnlyUserIdentities::Own(i) => OwnUserIdentity { + inner: i, + verification_machine: self.verification_machine.clone(), + } + .into(), + ReadOnlyUserIdentities::Other(i) => { + UserIdentity { inner: i, verification_machine: self.verification_machine.clone() } + .into() + } + })) + } } impl Deref for Store { @@ -388,7 +405,7 @@ pub trait CryptoStore: AsyncTraitDeps { /// # Arguments /// /// * `user_id` - The user for which we should get the identity. - async fn get_user_identity(&self, user_id: &UserId) -> Result>; + async fn get_user_identity(&self, user_id: &UserId) -> Result>; /// Check if a hash for an Olm message stored in the database. async fn is_message_known(&self, message_hash: &OlmMessageHash) -> Result; diff --git a/matrix_sdk_crypto/src/store/sled.rs b/matrix_sdk_crypto/src/store/sled.rs index b803496c..9ed76e07 100644 --- a/matrix_sdk_crypto/src/store/sled.rs +++ b/matrix_sdk_crypto/src/store/sled.rs @@ -35,7 +35,7 @@ use super::{ ReadOnlyAccount, Result, Session, }; use crate::{ - identities::{ReadOnlyDevice, UserIdentities}, + identities::{ReadOnlyDevice, ReadOnlyUserIdentities}, key_request::OutgoingKeyRequest, olm::{OutboundGroupSession, PickledInboundGroupSession, PrivateCrossSigningIdentity}, }; @@ -669,7 +669,7 @@ impl CryptoStore for SledStore { .collect() } - async fn get_user_identity(&self, user_id: &UserId) -> Result> { + async fn get_user_identity(&self, user_id: &UserId) -> Result> { Ok(self .identities .get(user_id.encode())? diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index 330f02cc..374e4410 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -20,9 +20,13 @@ use std::{ use dashmap::DashMap; use matrix_sdk_common::{locks::Mutex, uuid::Uuid}; use ruma::{ - events::{AnyToDeviceEvent, AnyToDeviceEventContent, ToDeviceEvent}, + events::{ + key::verification::VerificationMethod, AnyToDeviceEvent, AnyToDeviceEventContent, + ToDeviceEvent, + }, serde::Raw, - DeviceId, MilliSecondsSinceUnixEpoch, UserId, + to_device::DeviceIdOrAllDevices, + DeviceId, EventId, MilliSecondsSinceUnixEpoch, RoomId, UserId, }; use tracing::{info, trace, warn}; @@ -37,8 +41,8 @@ use crate::{ olm::PrivateCrossSigningIdentity, requests::OutgoingRequest, store::{CryptoStore, CryptoStoreError}, - OutgoingVerificationRequest, ReadOnlyAccount, ReadOnlyDevice, RoomMessageRequest, - ToDeviceRequest, + OutgoingVerificationRequest, ReadOnlyAccount, ReadOnlyDevice, ReadOnlyOwnUserIdentity, + ReadOnlyUserIdentity, RoomMessageRequest, ToDeviceRequest, }; #[derive(Clone, Debug)] @@ -65,6 +69,69 @@ impl VerificationMachine { } } + pub(crate) fn own_user_id(&self) -> &UserId { + self.account.user_id() + } + + pub(crate) fn own_device_id(&self) -> &DeviceId { + self.account.device_id() + } + + pub(crate) async fn request_self_verification( + &self, + identity: &ReadOnlyOwnUserIdentity, + methods: Option>, + ) -> Result<(VerificationRequest, OutgoingVerificationRequest), CryptoStoreError> { + let flow_id = FlowId::from(Uuid::new_v4().to_string()); + + let verification = VerificationRequest::new( + self.verifications.clone(), + self.account.clone(), + self.private_identity.lock().await.clone(), + self.store.clone(), + flow_id, + identity.user_id(), + methods, + ); + + // TODO get all the device ids of the user instead of using AllDevices + // make sure to remember this so we can cancel once someone picks up + let request: OutgoingVerificationRequest = ToDeviceRequest::new( + identity.user_id(), + DeviceIdOrAllDevices::AllDevices, + AnyToDeviceEventContent::KeyVerificationRequest(verification.request_to_device()), + ) + .into(); + + self.insert_request(verification.clone()); + + Ok((verification, request)) + } + + pub async fn request_verification( + &self, + identity: &ReadOnlyUserIdentity, + room_id: &RoomId, + request_event_id: &EventId, + methods: Option>, + ) -> VerificationRequest { + let flow_id = FlowId::InRoom(room_id.to_owned(), request_event_id.to_owned()); + + let request = VerificationRequest::new( + self.verifications.clone(), + self.account.clone(), + self.private_identity.lock().await.clone(), + self.store.clone(), + flow_id, + identity.user_id(), + methods, + ); + + self.insert_request(request.clone()); + + request + } + pub async fn start_sas( &self, device: ReadOnlyDevice, diff --git a/matrix_sdk_crypto/src/verification/mod.rs b/matrix_sdk_crypto/src/verification/mod.rs index b77d758b..025fca1e 100644 --- a/matrix_sdk_crypto/src/verification/mod.rs +++ b/matrix_sdk_crypto/src/verification/mod.rs @@ -44,7 +44,7 @@ use crate::{ error::SignatureError, olm::PrivateCrossSigningIdentity, store::{Changes, CryptoStore}, - CryptoStoreError, LocalTrust, ReadOnlyDevice, UserIdentities, + CryptoStoreError, LocalTrust, ReadOnlyDevice, ReadOnlyUserIdentities, }; /// An enum over the different verification types the SDK supports. @@ -144,7 +144,7 @@ impl From for Verification { #[derive(Clone, Debug)] pub struct Done { verified_devices: Arc<[ReadOnlyDevice]>, - verified_master_keys: Arc<[UserIdentities]>, + verified_master_keys: Arc<[ReadOnlyUserIdentities]>, } impl Done { @@ -277,7 +277,7 @@ pub struct IdentitiesBeingVerified { private_identity: PrivateCrossSigningIdentity, store: Arc, device_being_verified: ReadOnlyDevice, - identity_being_verified: Option, + identity_being_verified: Option, } impl IdentitiesBeingVerified { @@ -308,7 +308,7 @@ impl IdentitiesBeingVerified { pub async fn mark_as_done( &self, verified_devices: Option<&[ReadOnlyDevice]>, - verified_identities: Option<&[UserIdentities]>, + verified_identities: Option<&[ReadOnlyUserIdentities]>, ) -> Result { let device = self.mark_device_as_verified(verified_devices).await?; let identity = self.mark_identity_as_verified(verified_identities).await?; @@ -415,8 +415,8 @@ impl IdentitiesBeingVerified { async fn mark_identity_as_verified( &self, - verified_identities: Option<&[UserIdentities]>, - ) -> Result, CryptoStoreError> { + verified_identities: Option<&[ReadOnlyUserIdentities]>, + ) -> Result, CryptoStoreError> { // If there wasn't an identity available during the verification flow // return early as there's nothing to do. if self.identity_being_verified.is_none() { @@ -437,7 +437,7 @@ impl IdentitiesBeingVerified { "Marking the user identity of as verified." ); - if let UserIdentities::Own(i) = &identity { + if let ReadOnlyUserIdentities::Own(i) = &identity { i.mark_as_verified(); } diff --git a/matrix_sdk_crypto/src/verification/qrcode.rs b/matrix_sdk_crypto/src/verification/qrcode.rs index 0dd4b576..9ded7336 100644 --- a/matrix_sdk_crypto/src/verification/qrcode.rs +++ b/matrix_sdk_crypto/src/verification/qrcode.rs @@ -45,8 +45,8 @@ use super::{ use crate::{ olm::{PrivateCrossSigningIdentity, ReadOnlyAccount}, store::CryptoStore, - CryptoStoreError, OutgoingVerificationRequest, ReadOnlyDevice, RoomMessageRequest, - ToDeviceRequest, UserIdentities, + CryptoStoreError, OutgoingVerificationRequest, ReadOnlyDevice, ReadOnlyUserIdentities, + RoomMessageRequest, ToDeviceRequest, }; const SECRET_SIZE: usize = 16; @@ -518,7 +518,7 @@ impl QrVerification { ScanError::MissingDeviceKeys(other_user_id.clone(), other_device_id.clone()) })?; - let check_master_key = |key, identity: &UserIdentities| { + let check_master_key = |key, identity: &ReadOnlyUserIdentities| { let master_key = identity.master_key().get_first_key().ok_or_else(|| { ScanError::MissingCrossSigningIdentity(identity.user_id().clone()) })?; @@ -719,7 +719,7 @@ impl QrState { self.state.as_content(flow_id) } - fn verified_identities(&self) -> (Arc<[ReadOnlyDevice]>, Arc<[UserIdentities]>) { + fn verified_identities(&self) -> (Arc<[ReadOnlyDevice]>, Arc<[ReadOnlyUserIdentities]>) { (self.state.verified_devices.clone(), self.state.verified_master_keys.clone()) } } @@ -729,7 +729,7 @@ impl QrState { self, _: &DoneContent, verified_device: Option<&ReadOnlyDevice>, - verified_identity: Option<&UserIdentities>, + verified_identity: Option<&ReadOnlyUserIdentities>, ) -> QrState { let devices: Vec<_> = verified_device.into_iter().cloned().collect(); let identities: Vec<_> = verified_identity.into_iter().cloned().collect(); @@ -768,7 +768,7 @@ impl QrState { self, _: &DoneContent, verified_device: Option<&ReadOnlyDevice>, - verified_identity: Option<&UserIdentities>, + verified_identity: Option<&ReadOnlyUserIdentities>, ) -> QrState { let devices: Vec<_> = verified_device.into_iter().cloned().collect(); let identities: Vec<_> = verified_identity.into_iter().cloned().collect(); diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index e84193e1..7c224d55 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -12,8 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#![allow(dead_code)] - use std::sync::{Arc, Mutex}; use matrix_qrcode::QrVerificationData; @@ -31,7 +29,7 @@ use ruma::{ AnyMessageEventContent, AnyToDeviceEventContent, }, to_device::DeviceIdOrAllDevices, - DeviceId, DeviceIdBox, DeviceKeyAlgorithm, EventId, MilliSecondsSinceUnixEpoch, RoomId, UserId, + DeviceId, DeviceIdBox, DeviceKeyAlgorithm, MilliSecondsSinceUnixEpoch, RoomId, UserId, }; use tracing::{info, trace, warn}; @@ -46,8 +44,8 @@ use super::{ use crate::{ olm::{PrivateCrossSigningIdentity, ReadOnlyAccount}, store::CryptoStore, - CryptoStoreError, OutgoingVerificationRequest, ReadOnlyDevice, RoomMessageRequest, Sas, - ToDeviceRequest, UserIdentities, + CryptoStoreError, OutgoingVerificationRequest, ReadOnlyDevice, ReadOnlyUserIdentities, + RoomMessageRequest, Sas, ToDeviceRequest, }; const SUPPORTED_METHODS: &[VerificationMethod] = &[ @@ -79,41 +77,10 @@ impl VerificationRequest { account: ReadOnlyAccount, private_cross_signing_identity: PrivateCrossSigningIdentity, store: Arc, - room_id: &RoomId, - event_id: &EventId, + flow_id: FlowId, other_user: &UserId, + methods: Option>, ) -> Self { - let flow_id = (room_id.to_owned(), event_id.to_owned()).into(); - - let inner = Mutex::new(InnerRequest::Created(RequestState::new( - account.clone(), - private_cross_signing_identity, - cache.clone(), - store, - other_user, - &flow_id, - ))) - .into(); - - Self { - account, - verification_cache: cache, - flow_id: flow_id.into(), - inner, - other_user_id: other_user.to_owned().into(), - we_started: true, - } - } - - pub(crate) fn new_to_device( - cache: VerificationCache, - account: ReadOnlyAccount, - private_cross_signing_identity: PrivateCrossSigningIdentity, - store: Arc, - other_user: &UserId, - ) -> Self { - let flow_id = Uuid::new_v4().to_string().into(); - let inner = Mutex::new(InnerRequest::Created(RequestState::new( account.clone(), private_cross_signing_identity, @@ -121,6 +88,7 @@ impl VerificationRequest { store, other_user, &flow_id, + methods, ))) .into(); @@ -138,11 +106,19 @@ impl VerificationRequest { /// verification from the other side. This should be used only for /// self-verifications and it should be sent to the specific device that we /// want to verify. - pub fn request_to_device(&self) -> RequestToDeviceEventContent { + pub(crate) fn request_to_device(&self) -> RequestToDeviceEventContent { + let inner = self.inner.lock().unwrap(); + + let methods = if let InnerRequest::Created(c) = &*inner { + c.state.our_methods.clone() + } else { + SUPPORTED_METHODS.to_vec() + }; + RequestToDeviceEventContent::new( self.account.device_id().into(), self.flow_id().as_str().to_string(), - SUPPORTED_METHODS.to_vec(), + methods, MilliSecondsSinceUnixEpoch::now(), ) } @@ -155,6 +131,7 @@ impl VerificationRequest { own_user_id: &UserId, own_device_id: &DeviceId, other_user_id: &UserId, + methods: Option>, ) -> KeyVerificationRequestEventContent { KeyVerificationRequestEventContent::new( format!( @@ -163,7 +140,7 @@ impl VerificationRequest { key verification to verify keys.", own_user_id ), - SUPPORTED_METHODS.to_vec(), + methods.unwrap_or_else(|| SUPPORTED_METHODS.to_vec()), own_device_id.into(), other_user_id.to_owned(), ) @@ -376,6 +353,15 @@ impl VerificationRequest { /// Cancel the verification request pub fn cancel(&self) -> Option { + if let Some(verification) = + self.verification_cache.get(self.other_user(), self.flow_id().as_str()) + { + match verification { + crate::Verification::SasV1(s) => s.cancel(), + crate::Verification::QrV1(q) => q.cancel(), + }; + } + let mut inner = self.inner.lock().unwrap(); inner.cancel(true, &CancelCode::User); @@ -398,12 +384,20 @@ impl VerificationRequest { pub(crate) fn receive_ready(&self, sender: &UserId, content: &ReadyContent) { let mut inner = self.inner.lock().unwrap(); - if let InnerRequest::Created(s) = &*inner { - if sender == self.own_user_id() && content.from_device() == self.account.device_id() { - *inner = InnerRequest::Passive(s.clone().into_passive(content)) - } else { + match &*inner { + InnerRequest::Created(s) => { *inner = InnerRequest::Ready(s.clone().into_ready(sender, content)); } + InnerRequest::Requested(s) => { + if sender == self.own_user_id() && content.from_device() != self.account.device_id() + { + *inner = InnerRequest::Passive(s.clone().into_passive(content)) + } + } + InnerRequest::Ready(_) + | InnerRequest::Passive(_) + | InnerRequest::Done(_) + | InnerRequest::Cancelled(_) => {} } } @@ -512,17 +506,6 @@ impl InnerRequest { } } - fn other_user_id(&self) -> &UserId { - match self { - InnerRequest::Created(s) => &s.other_user_id, - InnerRequest::Requested(s) => &s.other_user_id, - InnerRequest::Ready(s) => &s.other_user_id, - InnerRequest::Passive(s) => &s.other_user_id, - InnerRequest::Done(s) => &s.other_user_id, - InnerRequest::Cancelled(s) => &s.other_user_id, - } - } - fn accept(&mut self, methods: Vec) -> Option { if let InnerRequest::Requested(s) = self { let (state, content) = s.clone().accept(methods); @@ -569,20 +552,6 @@ impl InnerRequest { InnerRequest::Cancelled(_) => Ok(None), } } - - fn to_started_sas( - &self, - content: &StartContent, - other_device: ReadOnlyDevice, - other_identity: Option, - we_started: bool, - ) -> Result, OutgoingContent> { - if let InnerRequest::Ready(s) = self { - Ok(Some(s.to_started_sas(content, other_device, other_identity, we_started)?)) - } else { - Ok(None) - } - } } #[derive(Clone, Debug)] @@ -638,30 +607,21 @@ impl RequestState { store: Arc, other_user_id: &UserId, flow_id: &FlowId, + methods: Option>, ) -> Self { + let our_methods = methods.unwrap_or_else(|| SUPPORTED_METHODS.to_vec()); + Self { account, other_user_id: other_user_id.to_owned(), private_cross_signing_identity: private_identity, - state: Created { our_methods: SUPPORTED_METHODS.to_vec() }, + state: Created { our_methods }, verification_cache: cache, store, flow_id: flow_id.to_owned().into(), } } - fn into_passive(self, content: &ReadyContent) -> RequestState { - RequestState { - account: self.account, - flow_id: self.flow_id, - verification_cache: self.verification_cache, - private_cross_signing_identity: self.private_cross_signing_identity, - store: self.store, - other_user_id: self.other_user_id, - state: Passive { other_device_id: content.from_device().to_owned() }, - } - } - fn into_ready(self, _sender: &UserId, content: &ReadyContent) -> RequestState { // TODO check the flow id, and that the methods match what we suggested. RequestState { @@ -720,6 +680,18 @@ impl RequestState { } } + fn into_passive(self, content: &ReadyContent) -> RequestState { + RequestState { + account: self.account, + flow_id: self.flow_id, + verification_cache: self.verification_cache, + private_cross_signing_identity: self.private_cross_signing_identity, + store: self.store, + other_user_id: self.other_user_id, + state: Passive { other_device_id: content.from_device().to_owned() }, + } + } + fn accept(self, methods: Vec) -> (RequestState, OutgoingContent) { let state = RequestState { account: self.account.clone(), @@ -776,7 +748,7 @@ impl RequestState { &self, content: &StartContent<'a>, other_device: ReadOnlyDevice, - other_identity: Option, + other_identity: Option, we_started: bool, ) -> Result { Sas::from_start_event( @@ -827,7 +799,7 @@ impl RequestState { let verification = if let Some(identity) = &identites.identity_being_verified { match &identity { - UserIdentities::Own(i) => { + ReadOnlyUserIdentities::Own(i) => { if let Some(master_key) = i.master_key().get_first_key() { if identites.can_sign_devices().await { if let Some(device_key) = @@ -870,7 +842,7 @@ impl RequestState { None } } - UserIdentities::Other(i) => { + ReadOnlyUserIdentities::Other(i) => { if let Some(other_master) = i.master_key().get_first_key() { // TODO we can get the master key from the public // identity if we don't have the private one and we @@ -1117,20 +1089,21 @@ mod test { let bob_store: Box = Box::new(MemoryStore::new()); let bob_identity = PrivateCrossSigningIdentity::empty(alice_id()); - let content = VerificationRequest::request(bob.user_id(), bob.device_id(), &alice_id()); + let content = + VerificationRequest::request(bob.user_id(), bob.device_id(), &alice_id(), None); + + let flow_id = FlowId::InRoom(room_id, event_id); let bob_request = VerificationRequest::new( VerificationCache::new(), bob, bob_identity, bob_store.into(), - &room_id, - &event_id, + flow_id.clone(), &alice_id(), + None, ); - let flow_id = FlowId::from((room_id, event_id)); - let alice_request = VerificationRequest::from_request( VerificationCache::new(), alice, @@ -1174,20 +1147,20 @@ mod test { changes.devices.new.push(alice_device.clone()); bob_store.save_changes(changes).await.unwrap(); - let content = VerificationRequest::request(bob.user_id(), bob.device_id(), &alice_id()); + let content = + VerificationRequest::request(bob.user_id(), bob.device_id(), &alice_id(), None); + let flow_id = FlowId::from((room_id, event_id)); let bob_request = VerificationRequest::new( VerificationCache::new(), bob, bob_identity, bob_store.into(), - &room_id, - &event_id, + flow_id.clone(), &alice_id(), + None, ); - let flow_id = FlowId::from((room_id, event_id)); - let alice_request = VerificationRequest::from_request( VerificationCache::new(), alice, @@ -1240,12 +1213,16 @@ mod test { changes.devices.new.push(alice_device.clone()); bob_store.save_changes(changes).await.unwrap(); - let bob_request = VerificationRequest::new_to_device( + let flow_id = FlowId::from("TEST_FLOW_ID".to_owned()); + + let bob_request = VerificationRequest::new( VerificationCache::new(), bob, bob_identity, bob_store.into(), + flow_id, &alice_id(), + None, ); let content = bob_request.request_to_device(); diff --git a/matrix_sdk_crypto/src/verification/sas/helpers.rs b/matrix_sdk_crypto/src/verification/sas/helpers.rs index 868df891..f39869b0 100644 --- a/matrix_sdk_crypto/src/verification/sas/helpers.rs +++ b/matrix_sdk_crypto/src/verification/sas/helpers.rs @@ -31,7 +31,7 @@ use tracing::{trace, warn}; use super::{FlowId, OutgoingContent}; use crate::{ - identities::{ReadOnlyDevice, UserIdentities}, + identities::{ReadOnlyDevice, ReadOnlyUserIdentities}, utilities::encode, verification::event_enums::{MacContent, StartContent}, ReadOnlyAccount, @@ -41,7 +41,7 @@ use crate::{ pub struct SasIds { pub account: ReadOnlyAccount, pub other_device: ReadOnlyDevice, - pub other_identity: Option, + pub other_identity: Option, } /// Calculate the commitment for a accept event from the public key and the @@ -182,7 +182,7 @@ pub fn receive_mac_event( flow_id: &str, sender: &UserId, content: &MacContent, -) -> Result<(Vec, Vec), CancelCode> { +) -> Result<(Vec, Vec), CancelCode> { let mut verified_devices = Vec::new(); let mut verified_identities = Vec::new(); diff --git a/matrix_sdk_crypto/src/verification/sas/inner_sas.rs b/matrix_sdk_crypto/src/verification/sas/inner_sas.rs index fd6fe7a1..04267442 100644 --- a/matrix_sdk_crypto/src/verification/sas/inner_sas.rs +++ b/matrix_sdk_crypto/src/verification/sas/inner_sas.rs @@ -29,7 +29,7 @@ use super::{ FlowId, }; use crate::{ - identities::{ReadOnlyDevice, UserIdentities}, + identities::{ReadOnlyDevice, ReadOnlyUserIdentities}, verification::{ event_enums::{AnyVerificationContent, OutgoingContent, OwnedAcceptContent, StartContent}, Cancelled, Done, @@ -55,7 +55,7 @@ impl InnerSas { pub fn start( account: ReadOnlyAccount, other_device: ReadOnlyDevice, - other_identity: Option, + other_identity: Option, transaction_id: Option, ) -> (InnerSas, OutgoingContent) { let sas = SasState::::new(account, other_device, other_identity, transaction_id); @@ -131,7 +131,7 @@ impl InnerSas { room_id: RoomId, account: ReadOnlyAccount, other_device: ReadOnlyDevice, - other_identity: Option, + other_identity: Option, ) -> (InnerSas, OutgoingContent) { let sas = SasState::::new_in_room( room_id, @@ -149,7 +149,7 @@ impl InnerSas { other_device: ReadOnlyDevice, flow_id: FlowId, content: &StartContent, - other_identity: Option, + other_identity: Option, started_from_request: bool, ) -> Result { match SasState::::from_start_event( @@ -204,7 +204,9 @@ impl InnerSas { InnerSas::WeAccepted(s) => s.cancel(cancelled_by_us, code), InnerSas::KeyReceived(s) => s.cancel(cancelled_by_us, code), InnerSas::MacReceived(s) => s.cancel(cancelled_by_us, code), - _ => return (self, None), + InnerSas::Confirmed(s) => s.cancel(cancelled_by_us, code), + InnerSas::WaitingForDone(s) => s.cancel(cancelled_by_us, code), + InnerSas::Done(_) | InnerSas::Cancelled(_) => return (self, None), }; let content = sas.as_content(); @@ -423,7 +425,7 @@ impl InnerSas { } } - pub fn verified_identities(&self) -> Option> { + pub fn verified_identities(&self) -> Option> { if let InnerSas::Done(s) = self { Some(s.verified_identities()) } else { diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index eb4b78af..bdf1f107 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -41,7 +41,7 @@ use super::{ FlowId, IdentitiesBeingVerified, VerificationResult, }; use crate::{ - identities::{ReadOnlyDevice, UserIdentities}, + identities::{ReadOnlyDevice, ReadOnlyUserIdentities}, olm::PrivateCrossSigningIdentity, requests::{OutgoingVerificationRequest, RoomMessageRequest}, store::{CryptoStore, CryptoStoreError}, @@ -151,7 +151,7 @@ impl Sas { private_identity: PrivateCrossSigningIdentity, other_device: ReadOnlyDevice, store: Arc, - other_identity: Option, + other_identity: Option, we_started: bool, ) -> Sas { let flow_id = inner_sas.verification_flow_id(); @@ -187,7 +187,7 @@ impl Sas { private_identity: PrivateCrossSigningIdentity, other_device: ReadOnlyDevice, store: Arc, - other_identity: Option, + other_identity: Option, transaction_id: Option, we_started: bool, ) -> (Sas, OutgoingContent) { @@ -230,7 +230,7 @@ impl Sas { private_identity: PrivateCrossSigningIdentity, other_device: ReadOnlyDevice, store: Arc, - other_identity: Option, + other_identity: Option, we_started: bool, ) -> (Sas, OutgoingContent) { let (inner, content) = InnerSas::start_in_room( @@ -273,7 +273,7 @@ impl Sas { account: ReadOnlyAccount, private_identity: PrivateCrossSigningIdentity, other_device: ReadOnlyDevice, - other_identity: Option, + other_identity: Option, started_from_request: bool, we_started: bool, ) -> Result { @@ -503,7 +503,7 @@ impl Sas { self.inner.lock().unwrap().verified_devices() } - pub(crate) fn verified_identities(&self) -> Option> { + pub(crate) fn verified_identities(&self) -> Option> { self.inner.lock().unwrap().verified_identities() } diff --git a/matrix_sdk_crypto/src/verification/sas/sas_state.rs b/matrix_sdk_crypto/src/verification/sas/sas_state.rs index af5dad17..f4e77c2a 100644 --- a/matrix_sdk_crypto/src/verification/sas/sas_state.rs +++ b/matrix_sdk_crypto/src/verification/sas/sas_state.rs @@ -52,7 +52,7 @@ use super::{ OutgoingContent, }; use crate::{ - identities::{ReadOnlyDevice, UserIdentities}, + identities::{ReadOnlyDevice, ReadOnlyUserIdentities}, verification::{ event_enums::{ AcceptContent, DoneContent, KeyContent, MacContent, OwnedAcceptContent, @@ -277,7 +277,7 @@ pub struct MacReceived { we_started: bool, their_pubkey: String, verified_devices: Arc<[ReadOnlyDevice]>, - verified_master_keys: Arc<[UserIdentities]>, + verified_master_keys: Arc<[ReadOnlyUserIdentities]>, pub accepted_protocols: Arc, } @@ -287,7 +287,7 @@ pub struct MacReceived { #[derive(Clone, Debug)] pub struct WaitingForDone { verified_devices: Arc<[ReadOnlyDevice]>, - verified_master_keys: Arc<[UserIdentities]>, + verified_master_keys: Arc<[ReadOnlyUserIdentities]>, } impl SasState { @@ -362,7 +362,7 @@ impl SasState { pub fn new( account: ReadOnlyAccount, other_device: ReadOnlyDevice, - other_identity: Option, + other_identity: Option, transaction_id: Option, ) -> SasState { let started_from_request = transaction_id.is_some(); @@ -388,7 +388,7 @@ impl SasState { event_id: EventId, account: ReadOnlyAccount, other_device: ReadOnlyDevice, - other_identity: Option, + other_identity: Option, ) -> SasState { let flow_id = FlowId::InRoom(room_id, event_id); Self::new_helper(flow_id, account, other_device, other_identity, false) @@ -398,7 +398,7 @@ impl SasState { flow_id: FlowId, account: ReadOnlyAccount, other_device: ReadOnlyDevice, - other_identity: Option, + other_identity: Option, started_from_request: bool, ) -> SasState { SasState { @@ -497,7 +497,7 @@ impl SasState { pub fn from_start_event( account: ReadOnlyAccount, other_device: ReadOnlyDevice, - other_identity: Option, + other_identity: Option, flow_id: FlowId, content: &StartContent, started_from_request: bool, @@ -1096,7 +1096,7 @@ impl SasState { } /// Get the list of verified identities. - pub fn verified_identities(&self) -> Arc<[UserIdentities]> { + pub fn verified_identities(&self) -> Arc<[ReadOnlyUserIdentities]> { self.state.verified_master_keys.clone() } } From ae37e6ec9d37a59c2f490e13d9bddc55827ab2e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 9 Jul 2021 11:38:10 +0200 Subject: [PATCH 15/35] crypto: Add a state getter where we scanned the QR code --- matrix_sdk_crypto/src/verification/qrcode.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/matrix_sdk_crypto/src/verification/qrcode.rs b/matrix_sdk_crypto/src/verification/qrcode.rs index 9ded7336..455bec20 100644 --- a/matrix_sdk_crypto/src/verification/qrcode.rs +++ b/matrix_sdk_crypto/src/verification/qrcode.rs @@ -165,6 +165,12 @@ impl QrVerification { self.identities.is_self_verification() } + /// Have we successfully scanned the QR code and are able to send a + /// reciprocation event. + pub fn reciprocated(&self) -> bool { + matches!(&*self.state.lock().unwrap(), InnerState::Reciprocated(_)) + } + /// Get the unique ID that identifies this QR code verification flow. pub fn flow_id(&self) -> &FlowId { &self.flow_id From 5c9840daf829eb552f253ca29429884041af20f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 9 Jul 2021 11:39:25 +0200 Subject: [PATCH 16/35] crypto: Return a request when we start QR code verification instead of the content --- matrix_sdk_crypto/src/verification/qrcode.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/qrcode.rs b/matrix_sdk_crypto/src/verification/qrcode.rs index 455bec20..605b248c 100644 --- a/matrix_sdk_crypto/src/verification/qrcode.rs +++ b/matrix_sdk_crypto/src/verification/qrcode.rs @@ -245,9 +245,11 @@ impl QrVerification { /// /// This will return some `OutgoingContent` if the object is in the correct /// state to start the verification flow, otherwise `None`. - pub fn reciprocate(&self) -> Option { + pub fn reciprocate(&self) -> Option { match &*self.state.lock().unwrap() { - InnerState::Reciprocated(s) => Some(s.as_content(self.flow_id())), + InnerState::Reciprocated(s) => { + Some(self.content_to_request(s.as_content(self.flow_id()))) + } InnerState::Created(_) | InnerState::Scanned(_) | InnerState::Confirmed(_) From 76d57baa11b5967b03502595bde06a32ca6a8867 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 9 Jul 2021 11:40:21 +0200 Subject: [PATCH 17/35] crypto: Add the verification to the cache after we scan a QR code --- matrix_sdk_crypto/src/verification/qrcode.rs | 3 +- .../src/verification/requests.rs | 28 ++++++++++--------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/qrcode.rs b/matrix_sdk_crypto/src/verification/qrcode.rs index 605b248c..d460f54b 100644 --- a/matrix_sdk_crypto/src/verification/qrcode.rs +++ b/matrix_sdk_crypto/src/verification/qrcode.rs @@ -947,7 +947,8 @@ mod test { .await .unwrap(); - let content = bob_verification.reciprocate().unwrap(); + let request = bob_verification.reciprocate().unwrap(); + let content = OutgoingContent::from(request); let content = StartContent::try_from(&content).unwrap(); alice_verification.receive_reciprocation(&content); diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index 7c224d55..5c987a13 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -271,19 +271,21 @@ impl VerificationRequest { let state = self.inner.lock().unwrap(); if let InnerRequest::Ready(r) = &*state { - Ok(Some( - QrVerification::from_scan( - r.store.clone(), - r.account.clone(), - r.private_cross_signing_identity.clone(), - r.other_user_id.clone(), - r.state.other_device_id.clone(), - r.flow_id.as_ref().to_owned(), - data, - self.we_started, - ) - .await?, - )) + let qr_verification = QrVerification::from_scan( + r.store.clone(), + r.account.clone(), + r.private_cross_signing_identity.clone(), + r.other_user_id.clone(), + r.state.other_device_id.clone(), + r.flow_id.as_ref().to_owned(), + data, + self.we_started, + ) + .await?; + + self.verification_cache.insert_qr(qr_verification.clone()); + + Ok(Some(qr_verification)) } else { Ok(None) } From 4e5cc036735845278b59e5fb576bc08e235dcd7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 9 Jul 2021 11:50:12 +0200 Subject: [PATCH 18/35] crypto: Only go into a done verification request if we're in the correct state --- matrix_sdk_crypto/src/verification/requests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index 5c987a13..1c661e82 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -521,12 +521,12 @@ impl InnerRequest { fn receive_done(&mut self, content: &DoneContent) { *self = InnerRequest::Done(match self { - InnerRequest::Created(s) => s.clone().into_done(content), - InnerRequest::Requested(s) => s.clone().into_done(content), InnerRequest::Ready(s) => s.clone().into_done(content), InnerRequest::Passive(s) => s.clone().into_done(content), - InnerRequest::Done(s) => s.clone().into_done(content), - InnerRequest::Cancelled(_) => return, + InnerRequest::Done(_) + | InnerRequest::Created(_) + | InnerRequest::Requested(_) + | InnerRequest::Cancelled(_) => return, }) } From b0e8f124265f1359fb2190bd66fd8c08d1ac9ece Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 9 Jul 2021 13:53:47 +0200 Subject: [PATCH 19/35] crypto: Cancel the verification request if the child flow gets cancelled --- matrix_sdk_crypto/src/verification/machine.rs | 2 +- matrix_sdk_crypto/src/verification/qrcode.rs | 28 ++++++++-- .../src/verification/requests.rs | 56 +++++++++++++++++-- matrix_sdk_crypto/src/verification/sas/mod.rs | 20 ++++++- 4 files changed, 92 insertions(+), 14 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index 374e4410..3a89b070 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -419,7 +419,7 @@ impl VerificationMachine { private_identity, device, identity, - false, + None, false, ) { Ok(sas) => { diff --git a/matrix_sdk_crypto/src/verification/qrcode.rs b/matrix_sdk_crypto/src/verification/qrcode.rs index d460f54b..b472db4a 100644 --- a/matrix_sdk_crypto/src/verification/qrcode.rs +++ b/matrix_sdk_crypto/src/verification/qrcode.rs @@ -40,6 +40,7 @@ use tracing::trace; use super::{ event_enums::{CancelContent, DoneContent, OutgoingContent, OwnedStartContent, StartContent}, + requests::RequestHandle, Cancelled, Done, FlowId, IdentitiesBeingVerified, VerificationResult, }; use crate::{ @@ -84,6 +85,7 @@ pub struct QrVerification { inner: Arc, state: Arc>, identities: IdentitiesBeingVerified, + request_handle: Option, we_started: bool, } @@ -222,11 +224,15 @@ impl QrVerification { /// /// [`cancel()`]: #method.cancel pub fn cancel_with_code(&self, code: CancelCode) -> Option { + let mut state = self.state.lock().unwrap(); + + if let Some(request) = &self.request_handle { + request.cancel_with_code(&code); + } + let new_state = QrState::::new(true, code); let content = new_state.as_content(self.flow_id()); - let mut state = self.state.lock().unwrap(); - match &*state { InnerState::Confirmed(_) | InnerState::Created(_) @@ -438,6 +444,7 @@ impl QrVerification { other_device_key: String, identities: IdentitiesBeingVerified, we_started: bool, + request_handle: Option, ) -> Self { let secret = Self::generate_secret(); @@ -449,7 +456,7 @@ impl QrVerification { ) .into(); - Self::new_helper(store, flow_id, inner, identities, we_started) + Self::new_helper(store, flow_id, inner, identities, we_started, request_handle) } pub(crate) fn new_self_no_master( @@ -459,6 +466,7 @@ impl QrVerification { own_master_key: String, identities: IdentitiesBeingVerified, we_started: bool, + request_handle: Option, ) -> QrVerification { let secret = Self::generate_secret(); @@ -470,7 +478,7 @@ impl QrVerification { ) .into(); - Self::new_helper(store, flow_id, inner, identities, we_started) + Self::new_helper(store, flow_id, inner, identities, we_started, request_handle) } pub(crate) fn new_cross( @@ -480,6 +488,7 @@ impl QrVerification { other_master_key: String, identities: IdentitiesBeingVerified, we_started: bool, + request_handle: Option, ) -> Self { let secret = Self::generate_secret(); @@ -492,7 +501,7 @@ impl QrVerification { let inner: QrVerificationData = VerificationData::new(event_id, own_master_key, other_master_key, secret).into(); - Self::new_helper(store, flow_id, inner, identities, we_started) + Self::new_helper(store, flow_id, inner, identities, we_started, request_handle) } #[allow(clippy::too_many_arguments)] @@ -505,6 +514,7 @@ impl QrVerification { flow_id: FlowId, qr_code: QrVerificationData, we_started: bool, + request_handle: Option, ) -> Result { if flow_id.as_str() != qr_code.flow_id() { return Err(ScanError::FlowIdMismatch { @@ -602,6 +612,7 @@ impl QrVerification { .into(), identities, we_started, + request_handle, }) } @@ -611,6 +622,7 @@ impl QrVerification { inner: QrVerificationData, identities: IdentitiesBeingVerified, we_started: bool, + request_handle: Option, ) -> Self { let secret = inner.secret().to_owned(); @@ -621,6 +633,7 @@ impl QrVerification { state: Mutex::new(InnerState::Created(QrState { state: Created { secret } })).into(), identities, we_started, + request_handle, } } } @@ -848,6 +861,7 @@ mod test { master_key.clone(), identities.clone(), false, + None, ); assert_eq!(verification.inner.first_key(), &device_key); @@ -860,6 +874,7 @@ mod test { device_key.clone(), identities.clone(), false, + None, ); assert_eq!(verification.inner.first_key(), &master_key); @@ -878,6 +893,7 @@ mod test { bob_master_key.clone(), identities, false, + None, ); assert_eq!(verification.inner.first_key(), &master_key); @@ -922,6 +938,7 @@ mod test { master_key.clone(), identities, false, + None, ); let bob_store = memory_store(); @@ -943,6 +960,7 @@ mod test { flow_id, qr_code, false, + None, ) .await .unwrap(); diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index 1c661e82..435e7e03 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -71,6 +71,31 @@ pub struct VerificationRequest { we_started: bool, } +/// A handle to a request so child verification flows can cancel the request. +/// +/// A verification flow can branch off into different types of verification +/// flows after the initial request handshake is done. +/// +/// Cancelling a QR code verification should also cancel the request. This +/// `RequestHandle` allows the QR code verification object to cancel the parent +/// `VerificationRequest` object. +#[derive(Clone, Debug)] +pub(crate) struct RequestHandle { + inner: Arc>, +} + +impl RequestHandle { + pub fn cancel_with_code(&self, cancel_code: &CancelCode) { + self.inner.lock().unwrap().cancel(true, cancel_code) + } +} + +impl From>> for RequestHandle { + fn from(inner: Arc>) -> Self { + Self { inner } + } +} + impl VerificationRequest { pub(crate) fn new( cache: VerificationCache, @@ -254,7 +279,11 @@ impl VerificationRequest { /// Generate a QR code that can be used by another client to start a QR code /// based verification. pub async fn generate_qr_code(&self) -> Result, CryptoStoreError> { - self.inner.lock().unwrap().generate_qr_code(self.we_started).await + self.inner + .lock() + .unwrap() + .generate_qr_code(self.we_started, self.inner.clone().into()) + .await } /// Start a QR code verification by providing a scanned QR code for this @@ -280,6 +309,7 @@ impl VerificationRequest { r.flow_id.as_ref().to_owned(), data, self.we_started, + Some(self.inner.clone().into()), ) .await?; @@ -411,7 +441,7 @@ impl VerificationRequest { let inner = self.inner.lock().unwrap().clone(); if let InnerRequest::Ready(s) = inner { - s.receive_start(sender, content, self.we_started).await?; + s.receive_start(sender, content, self.we_started, self.inner.clone().into()).await?; } else { warn!( sender = sender.as_str(), @@ -457,6 +487,7 @@ impl VerificationRequest { s.account.clone(), s.private_cross_signing_identity.clone(), self.we_started, + self.inner.clone().into(), ) .await? { @@ -544,11 +575,12 @@ impl InnerRequest { async fn generate_qr_code( &self, we_started: bool, + request_handle: RequestHandle, ) -> Result, CryptoStoreError> { match self { InnerRequest::Created(_) => Ok(None), InnerRequest::Requested(_) => Ok(None), - InnerRequest::Ready(s) => s.generate_qr_code(we_started).await, + InnerRequest::Ready(s) => s.generate_qr_code(we_started, request_handle).await, InnerRequest::Passive(_) => Ok(None), InnerRequest::Done(_) => Ok(None), InnerRequest::Cancelled(_) => Ok(None), @@ -752,6 +784,7 @@ impl RequestState { other_device: ReadOnlyDevice, other_identity: Option, we_started: bool, + request_handle: RequestHandle, ) -> Result { Sas::from_start_event( (&*self.flow_id).to_owned(), @@ -761,7 +794,7 @@ impl RequestState { self.private_cross_signing_identity.clone(), other_device, other_identity, - true, + Some(request_handle), we_started, ) } @@ -769,6 +802,7 @@ impl RequestState { async fn generate_qr_code( &self, we_started: bool, + request_handle: RequestHandle, ) -> Result, CryptoStoreError> { // If we didn't state that we support showing QR codes or if the other // side doesn't support scanning QR codes bail early. @@ -814,6 +848,7 @@ impl RequestState { device_key.to_owned(), identites, we_started, + Some(request_handle), )) } else { warn!( @@ -832,6 +867,7 @@ impl RequestState { master_key.to_owned(), identites, we_started, + Some(request_handle), )) } } else { @@ -862,6 +898,7 @@ impl RequestState { other_master.to_owned(), identites, we_started, + Some(request_handle), )) } else { warn!( @@ -906,6 +943,7 @@ impl RequestState { sender: &UserId, content: &StartContent<'_>, we_started: bool, + request_handle: RequestHandle, ) -> Result<(), CryptoStoreError> { info!( sender = sender.as_str(), @@ -929,7 +967,13 @@ impl RequestState { match content.method() { StartMethod::SasV1(_) => { - match self.to_started_sas(content, device.clone(), identity, we_started) { + match self.to_started_sas( + content, + device.clone(), + identity, + we_started, + request_handle, + ) { // TODO check if there is already a SAS verification, i.e. we // already started one before the other side tried to do the // same; ignore it if we did and we're the lexicographically @@ -982,6 +1026,7 @@ impl RequestState { account: ReadOnlyAccount, private_identity: PrivateCrossSigningIdentity, we_started: bool, + request_handle: RequestHandle, ) -> Result, CryptoStoreError> { if !self.state.their_methods.contains(&VerificationMethod::SasV1) { return Ok(None); @@ -1027,6 +1072,7 @@ impl RequestState { store, other_identity, we_started, + request_handle, ); (sas, content) } diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index bdf1f107..c0cb4fe4 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -38,6 +38,7 @@ use tracing::trace; use super::{ event_enums::{AnyVerificationContent, OutgoingContent, OwnedAcceptContent, StartContent}, + requests::RequestHandle, FlowId, IdentitiesBeingVerified, VerificationResult, }; use crate::{ @@ -56,6 +57,7 @@ pub struct Sas { identities_being_verified: IdentitiesBeingVerified, flow_id: Arc, we_started: bool, + request_handle: Option, } impl Sas { @@ -145,6 +147,7 @@ impl Sas { self.inner.lock().unwrap().set_creation_time(time) } + #[allow(clippy::too_many_arguments)] fn start_helper( inner_sas: InnerSas, account: ReadOnlyAccount, @@ -153,6 +156,7 @@ impl Sas { store: Arc, other_identity: Option, we_started: bool, + request_handle: Option, ) -> Sas { let flow_id = inner_sas.verification_flow_id(); @@ -169,6 +173,7 @@ impl Sas { identities_being_verified: identities, flow_id, we_started, + request_handle, } } @@ -207,6 +212,7 @@ impl Sas { store, other_identity, we_started, + None, ), content, ) @@ -232,6 +238,7 @@ impl Sas { store: Arc, other_identity: Option, we_started: bool, + request_handle: RequestHandle, ) -> (Sas, OutgoingContent) { let (inner, content) = InnerSas::start_in_room( flow_id, @@ -250,6 +257,7 @@ impl Sas { store, other_identity, we_started, + Some(request_handle), ), content, ) @@ -274,7 +282,7 @@ impl Sas { private_identity: PrivateCrossSigningIdentity, other_device: ReadOnlyDevice, other_identity: Option, - started_from_request: bool, + request_handle: Option, we_started: bool, ) -> Result { let inner = InnerSas::from_start_event( @@ -283,7 +291,7 @@ impl Sas { flow_id, content, other_identity.clone(), - started_from_request, + request_handle.is_some(), )?; Ok(Self::start_helper( @@ -294,6 +302,7 @@ impl Sas { store, other_identity, we_started, + request_handle, )) } @@ -418,6 +427,11 @@ impl Sas { /// [`cancel()`]: #method.cancel pub fn cancel_with_code(&self, code: CancelCode) -> Option { let mut guard = self.inner.lock().unwrap(); + + if let Some(request) = &self.request_handle { + request.cancel_with_code(&code) + } + let sas: InnerSas = (*guard).clone(); let (sas, content) = sas.cancel(true, code); *guard = sas; @@ -626,7 +640,7 @@ mod test { PrivateCrossSigningIdentity::empty(bob_id()), alice_device, None, - false, + None, false, ) .unwrap(); From 7644ceea8a0fd936f6a2830390300bcfb75934a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 9 Jul 2021 14:31:54 +0200 Subject: [PATCH 20/35] crypto: Make sure we don't deadlock when we cancel the verification request --- .../src/verification/requests.rs | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index 435e7e03..7e4fa6df 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -385,15 +385,6 @@ impl VerificationRequest { /// Cancel the verification request pub fn cancel(&self) -> Option { - if let Some(verification) = - self.verification_cache.get(self.other_user(), self.flow_id().as_str()) - { - match verification { - crate::Verification::SasV1(s) => s.cancel(), - crate::Verification::QrV1(q) => q.cancel(), - }; - } - let mut inner = self.inner.lock().unwrap(); inner.cancel(true, &CancelCode::User); @@ -403,14 +394,27 @@ impl VerificationRequest { None }; - content.map(|c| match c { + let request = content.map(|c| match c { OutgoingContent::ToDevice(content) => { ToDeviceRequest::new(&self.other_user(), inner.other_device_id(), content).into() } OutgoingContent::Room(room_id, content) => { RoomMessageRequest { room_id, txn_id: Uuid::new_v4(), content }.into() } - }) + }); + + drop(inner); + + if let Some(verification) = + self.verification_cache.get(self.other_user(), self.flow_id().as_str()) + { + match verification { + crate::Verification::SasV1(s) => s.cancel(), + crate::Verification::QrV1(q) => q.cancel(), + }; + } + + request } pub(crate) fn receive_ready(&self, sender: &UserId, content: &ReadyContent) { From cca73b26225af4928ee156e9872c1be56cf73fdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 9 Jul 2021 16:13:30 +0200 Subject: [PATCH 21/35] crypto: Update the SAS event timeout when we receive events --- .../src/verification/sas/sas_state.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/sas/sas_state.rs b/matrix_sdk_crypto/src/verification/sas/sas_state.rs index f4e77c2a..06b59a7e 100644 --- a/matrix_sdk_crypto/src/verification/sas/sas_state.rs +++ b/matrix_sdk_crypto/src/verification/sas/sas_state.rs @@ -466,7 +466,7 @@ impl SasState { ids: self.ids, verification_flow_id: self.verification_flow_id, creation_time: self.creation_time, - last_event_time: self.last_event_time, + last_event_time: Instant::now().into(), started_from_request: self.started_from_request, state: Arc::new(Accepted { start_content, @@ -642,7 +642,7 @@ impl SasState { ids: self.ids, verification_flow_id: self.verification_flow_id, creation_time: self.creation_time, - last_event_time: self.last_event_time, + last_event_time: Instant::now().into(), started_from_request: self.started_from_request, state: Arc::new(KeyReceived { we_started: false, @@ -690,7 +690,7 @@ impl SasState { ids: self.ids, verification_flow_id: self.verification_flow_id, creation_time: self.creation_time, - last_event_time: self.last_event_time, + last_event_time: Instant::now().into(), started_from_request: self.started_from_request, state: Arc::new(KeyReceived { their_pubkey, @@ -819,7 +819,7 @@ impl SasState { inner: self.inner, verification_flow_id: self.verification_flow_id, creation_time: self.creation_time, - last_event_time: self.last_event_time, + last_event_time: Instant::now().into(), ids: self.ids, started_from_request: self.started_from_request, state: Arc::new(MacReceived { @@ -878,7 +878,7 @@ impl SasState { Ok(SasState { inner: self.inner, creation_time: self.creation_time, - last_event_time: self.last_event_time, + last_event_time: Instant::now().into(), verification_flow_id: self.verification_flow_id, started_from_request: self.started_from_request, ids: self.ids, @@ -918,7 +918,7 @@ impl SasState { Ok(SasState { inner: self.inner, creation_time: self.creation_time, - last_event_time: self.last_event_time, + last_event_time: Instant::now().into(), verification_flow_id: self.verification_flow_id, started_from_request: self.started_from_request, ids: self.ids, @@ -1064,7 +1064,7 @@ impl SasState { Ok(SasState { inner: self.inner, creation_time: self.creation_time, - last_event_time: self.last_event_time, + last_event_time: Instant::now().into(), verification_flow_id: self.verification_flow_id, started_from_request: self.started_from_request, ids: self.ids, From 71c89c26701095a3f4de7582dd9bf34d9d7d83ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 9 Jul 2021 17:01:35 +0200 Subject: [PATCH 22/35] crypto: Time out verification requests as well --- matrix_sdk_crypto/src/verification/cache.rs | 22 ++++++--- .../src/verification/event_enums.rs | 15 +++--- matrix_sdk_crypto/src/verification/machine.rs | 16 +++++- matrix_sdk_crypto/src/verification/mod.rs | 5 +- matrix_sdk_crypto/src/verification/qrcode.rs | 12 ++--- .../src/verification/requests.rs | 49 ++++++++++++++----- 6 files changed, 84 insertions(+), 35 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/cache.rs b/matrix_sdk_crypto/src/verification/cache.rs index 5d029634..2124969b 100644 --- a/matrix_sdk_crypto/src/verification/cache.rs +++ b/matrix_sdk_crypto/src/verification/cache.rs @@ -17,9 +17,13 @@ use std::sync::Arc; use dashmap::DashMap; use matrix_sdk_common::uuid::Uuid; use ruma::{DeviceId, UserId}; +use tracing::trace; use super::{event_enums::OutgoingContent, Sas, Verification}; -use crate::{OutgoingRequest, QrVerification, RoomMessageRequest, ToDeviceRequest}; +use crate::{ + OutgoingRequest, OutgoingVerificationRequest, QrVerification, RoomMessageRequest, + ToDeviceRequest, +}; #[derive(Clone, Debug)] pub struct VerificationCache { @@ -73,7 +77,7 @@ impl VerificationCache { self.outgoing_requests.iter().map(|r| (*r).clone()).collect() } - pub fn garbage_collect(&self) -> Vec { + pub fn garbage_collect(&self) -> Vec { for user_verification in self.verification.iter() { user_verification.retain(|_, s| !(s.is_done() || s.is_cancelled())); } @@ -83,15 +87,12 @@ impl VerificationCache { self.verification .iter() .flat_map(|v| { - let requests: Vec = v + let requests: Vec = v .value() .iter() .filter_map(|s| { if let Verification::SasV1(s) = s.value() { - s.cancel_if_timed_out().map(|r| OutgoingRequest { - request_id: r.request_id(), - request: Arc::new(r.into()), - }) + s.cancel_if_timed_out() } else { None } @@ -114,9 +115,16 @@ impl VerificationCache { } pub fn add_request(&self, request: OutgoingRequest) { + trace!("Adding an outgoing verification request {:?}", request); self.outgoing_requests.insert(request.request_id, request); } + pub fn add_verification_request(&self, request: OutgoingVerificationRequest) { + let request = + OutgoingRequest { request_id: request.request_id(), request: Arc::new(request.into()) }; + self.add_request(request); + } + pub fn queue_up_content( &self, recipient: &UserId, diff --git a/matrix_sdk_crypto/src/verification/event_enums.rs b/matrix_sdk_crypto/src/verification/event_enums.rs index 9c63f540..e7c66725 100644 --- a/matrix_sdk_crypto/src/verification/event_enums.rs +++ b/matrix_sdk_crypto/src/verification/event_enums.rs @@ -665,16 +665,15 @@ impl From<(RoomId, AnyMessageEventContent)> for OutgoingContent { } } -#[cfg(test)] -use crate::OutgoingVerificationRequest; -use crate::{OutgoingRequest, RoomMessageRequest, ToDeviceRequest}; +use crate::{OutgoingRequest, OutgoingVerificationRequest, RoomMessageRequest, ToDeviceRequest}; -#[cfg(test)] -impl From for OutgoingContent { - fn from(request: OutgoingVerificationRequest) -> Self { +impl TryFrom for OutgoingContent { + type Error = String; + + fn try_from(request: OutgoingVerificationRequest) -> Result { match request { - OutgoingVerificationRequest::ToDevice(r) => Self::try_from(r).unwrap(), - OutgoingVerificationRequest::InRoom(r) => Self::from(r), + OutgoingVerificationRequest::ToDevice(r) => Self::try_from(r), + OutgoingVerificationRequest::InRoom(r) => Ok(Self::from(r)), } } } diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index 3a89b070..3bea8480 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -247,7 +247,19 @@ impl VerificationMachine { } self.requests.retain(|_, v| !v.is_empty()); - for request in self.verifications.garbage_collect() { + let mut requests: Vec = self + .requests + .iter() + .flat_map(|v| { + let requests: Vec = + v.value().iter().filter_map(|v| v.cancel_if_timed_out()).collect(); + requests + }) + .collect(); + + requests.extend(self.verifications.garbage_collect().into_iter()); + + for request in requests { if let Ok(OutgoingContent::ToDevice(AnyToDeviceEventContent::KeyVerificationCancel( content, ))) = request.clone().try_into() @@ -257,7 +269,7 @@ impl VerificationMachine { events.push(AnyToDeviceEvent::KeyVerificationCancel(event).into()); } - self.verifications.add_request(request) + self.verifications.add_verification_request(request) } events diff --git a/matrix_sdk_crypto/src/verification/mod.rs b/matrix_sdk_crypto/src/verification/mod.rs index 025fca1e..49ae8e70 100644 --- a/matrix_sdk_crypto/src/verification/mod.rs +++ b/matrix_sdk_crypto/src/verification/mod.rs @@ -525,6 +525,8 @@ impl IdentitiesBeingVerified { #[cfg(test)] pub(crate) mod test { + use std::convert::TryInto; + use ruma::{ events::{AnyToDeviceEvent, AnyToDeviceEventContent, ToDeviceEvent}, UserId, @@ -540,7 +542,8 @@ pub(crate) mod test { sender: &UserId, request: &OutgoingVerificationRequest, ) -> AnyToDeviceEvent { - let content = request.to_owned().into(); + let content = + request.to_owned().try_into().expect("Can't fetch content out of the request"); wrap_any_to_device_content(sender, content) } diff --git a/matrix_sdk_crypto/src/verification/qrcode.rs b/matrix_sdk_crypto/src/verification/qrcode.rs index b472db4a..baff6fab 100644 --- a/matrix_sdk_crypto/src/verification/qrcode.rs +++ b/matrix_sdk_crypto/src/verification/qrcode.rs @@ -207,7 +207,7 @@ impl QrVerification { /// Cancel the verification flow. pub fn cancel(&self) -> Option { - self.cancel_with_code(CancelCode::User).map(|c| self.content_to_request(c)) + self.cancel_with_code(CancelCode::User) } /// Cancel the verification. @@ -223,7 +223,7 @@ impl QrVerification { /// otherwise it returns a request that needs to be sent out. /// /// [`cancel()`]: #method.cancel - pub fn cancel_with_code(&self, code: CancelCode) -> Option { + pub fn cancel_with_code(&self, code: CancelCode) -> Option { let mut state = self.state.lock().unwrap(); if let Some(request) = &self.request_handle { @@ -240,7 +240,7 @@ impl QrVerification { | InnerState::Reciprocated(_) | InnerState::Done(_) => { *state = InnerState::Cancelled(new_state); - Some(content) + Some(self.content_to_request(content)) } InnerState::Cancelled(_) => None, } @@ -966,20 +966,20 @@ mod test { .unwrap(); let request = bob_verification.reciprocate().unwrap(); - let content = OutgoingContent::from(request); + let content = OutgoingContent::try_from(request).unwrap(); let content = StartContent::try_from(&content).unwrap(); alice_verification.receive_reciprocation(&content); let request = alice_verification.confirm_scanning().unwrap(); - let content = OutgoingContent::from(request); + let content = OutgoingContent::try_from(request).unwrap(); let content = DoneContent::try_from(&content).unwrap(); assert!(!alice_verification.is_done()); assert!(!bob_verification.is_done()); let (request, _) = bob_verification.receive_done(&content).await.unwrap(); - let content = OutgoingContent::from(request.unwrap()); + let content = OutgoingContent::try_from(request.unwrap()).unwrap(); let content = DoneContent::try_from(&content).unwrap(); alice_verification.receive_done(&content).await.unwrap(); diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index 7e4fa6df..230407d6 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -12,10 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::sync::{Arc, Mutex}; +use std::{ + sync::{Arc, Mutex}, + time::Duration, +}; use matrix_qrcode::QrVerificationData; -use matrix_sdk_common::uuid::Uuid; +use matrix_sdk_common::{instant::Instant, uuid::Uuid}; use ruma::{ events::{ key::verification::{ @@ -54,6 +57,8 @@ const SUPPORTED_METHODS: &[VerificationMethod] = &[ VerificationMethod::ReciprocateV1, ]; +const VERIFICATION_TIMEOUT: Duration = Duration::from_secs(60 * 10); + /// An object controlling key verification requests. /// /// Interactive verification flows usually start with a verification request, @@ -68,6 +73,7 @@ pub struct VerificationRequest { flow_id: Arc, other_user_id: Arc, inner: Arc>, + creation_time: Arc, we_started: bool, } @@ -123,6 +129,7 @@ impl VerificationRequest { flow_id: flow_id.into(), inner, other_user_id: other_user.to_owned().into(), + creation_time: Instant::now().into(), we_started: true, } } @@ -220,6 +227,11 @@ impl VerificationRequest { matches!(&*self.inner.lock().unwrap(), InnerRequest::Ready(_)) } + /// Has the verification flow timed out. + pub fn timed_out(&self) -> bool { + self.creation_time.elapsed() > VERIFICATION_TIMEOUT + } + /// Get the supported verification methods of the other side. /// /// Will be present only if the other side requested the verification or if @@ -345,6 +357,7 @@ impl VerificationRequest { other_user_id: sender.to_owned().into(), flow_id: flow_id.into(), we_started: false, + creation_time: Instant::now().into(), } } @@ -385,8 +398,12 @@ impl VerificationRequest { /// Cancel the verification request pub fn cancel(&self) -> Option { + self.cancel_with_code(CancelCode::User) + } + + fn cancel_with_code(&self, cancel_code: CancelCode) -> Option { let mut inner = self.inner.lock().unwrap(); - inner.cancel(true, &CancelCode::User); + inner.cancel(true, &cancel_code); let content = if let InnerRequest::Cancelled(c) = &*inner { Some(c.state.as_content(self.flow_id())) @@ -409,14 +426,24 @@ impl VerificationRequest { self.verification_cache.get(self.other_user(), self.flow_id().as_str()) { match verification { - crate::Verification::SasV1(s) => s.cancel(), - crate::Verification::QrV1(q) => q.cancel(), + crate::Verification::SasV1(s) => s.cancel_with_code(cancel_code), + crate::Verification::QrV1(q) => q.cancel_with_code(cancel_code), }; } request } + pub(crate) fn cancel_if_timed_out(&self) -> Option { + if self.is_cancelled() || self.is_done() { + None + } else if self.timed_out() { + self.cancel_with_code(CancelCode::Timeout) + } else { + None + } + } + pub(crate) fn receive_ready(&self, sender: &UserId, content: &ReadyContent) { let mut inner = self.inner.lock().unwrap(); @@ -1095,7 +1122,7 @@ struct Done {} #[cfg(test)] mod test { - use std::convert::TryFrom; + use std::convert::{TryFrom, TryInto}; use matrix_sdk_test::async_test; use ruma::{event_id, room_id, DeviceIdBox, UserId}; @@ -1166,7 +1193,7 @@ mod test { &(&content).into(), ); - let content: OutgoingContent = alice_request.accept().unwrap().into(); + let content: OutgoingContent = alice_request.accept().unwrap().try_into().unwrap(); let content = ReadyContent::try_from(&content).unwrap(); bob_request.receive_ready(&alice_id(), &content); @@ -1223,7 +1250,7 @@ mod test { &(&content).into(), ); - let content: OutgoingContent = alice_request.accept().unwrap().into(); + let content: OutgoingContent = alice_request.accept().unwrap().try_into().unwrap(); let content = ReadyContent::try_from(&content).unwrap(); bob_request.receive_ready(&alice_id(), &content); @@ -1233,7 +1260,7 @@ mod test { let (bob_sas, request) = bob_request.start_sas().await.unwrap().unwrap(); - let content: OutgoingContent = request.into(); + let content: OutgoingContent = request.try_into().unwrap(); let content = StartContent::try_from(&content).unwrap(); let flow_id = content.flow_id().to_owned(); alice_request.receive_start(bob_device.user_id(), &content).await.unwrap(); @@ -1290,7 +1317,7 @@ mod test { &(&content).into(), ); - let content: OutgoingContent = alice_request.accept().unwrap().into(); + let content: OutgoingContent = alice_request.accept().unwrap().try_into().unwrap(); let content = ReadyContent::try_from(&content).unwrap(); bob_request.receive_ready(&alice_id(), &content); @@ -1300,7 +1327,7 @@ mod test { let (bob_sas, request) = bob_request.start_sas().await.unwrap().unwrap(); - let content: OutgoingContent = request.into(); + let content: OutgoingContent = request.try_into().unwrap(); let content = StartContent::try_from(&content).unwrap(); let flow_id = content.flow_id().to_owned(); alice_request.receive_start(bob_device.user_id(), &content).await.unwrap(); From b53518d1b8dd93ac447d1318c2e8aa4e2004dd2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 9 Jul 2021 17:05:23 +0200 Subject: [PATCH 23/35] crypto: Improve a log line --- matrix_sdk_crypto/src/session_manager/group_sessions.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/matrix_sdk_crypto/src/session_manager/group_sessions.rs b/matrix_sdk_crypto/src/session_manager/group_sessions.rs index f4b210a6..e9b6bd3f 100644 --- a/matrix_sdk_crypto/src/session_manager/group_sessions.rs +++ b/matrix_sdk_crypto/src/session_manager/group_sessions.rs @@ -412,10 +412,7 @@ impl GroupSessionManager { users: impl Iterator, encryption_settings: impl Into, ) -> OlmResult>> { - debug!( - room_id = room_id.as_str(), - "Checking if a group session needs to be shared for room {}", room_id - ); + debug!(room_id = room_id.as_str(), "Checking if a room key needs to be shared",); let encryption_settings = encryption_settings.into(); let history_visibility = encryption_settings.history_visibility.clone(); From ead91a1e6b177d1a99b0a5a3232cd68694ff99b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 12 Jul 2021 18:12:02 +0200 Subject: [PATCH 24/35] crypto: Send cancellations if the other device picks up over to-device --- matrix_sdk_base/src/store/memory_store.rs | 3 +- matrix_sdk_crypto/src/identities/user.rs | 26 ++++- matrix_sdk_crypto/src/requests.rs | 23 ++++ .../src/verification/event_enums.rs | 27 +++++ matrix_sdk_crypto/src/verification/machine.rs | 31 +++--- .../src/verification/requests.rs | 105 ++++++++++++++++-- 6 files changed, 183 insertions(+), 32 deletions(-) diff --git a/matrix_sdk_base/src/store/memory_store.rs b/matrix_sdk_base/src/store/memory_store.rs index 6c919a4b..87d94093 100644 --- a/matrix_sdk_base/src/store/memory_store.rs +++ b/matrix_sdk_base/src/store/memory_store.rs @@ -69,7 +69,7 @@ pub struct MemoryStore { } impl MemoryStore { - #[cfg(not(feature = "sled_state_store"))] + #[allow(dead_code)] pub fn new() -> Self { Self { sync_token: Arc::new(RwLock::new(None)), @@ -558,7 +558,6 @@ impl StateStore for MemoryStore { } #[cfg(test)] -#[cfg(not(feature = "sled_state_store"))] mod test { use matrix_sdk_test::async_test; use ruma::{ diff --git a/matrix_sdk_crypto/src/identities/user.rs b/matrix_sdk_crypto/src/identities/user.rs index c124d4e4..13ca15d6 100644 --- a/matrix_sdk_crypto/src/identities/user.rs +++ b/matrix_sdk_crypto/src/identities/user.rs @@ -27,7 +27,7 @@ use ruma::{ events::{ key::verification::VerificationMethod, room::message::KeyVerificationRequestEventContent, }, - DeviceKeyId, EventId, RoomId, UserId, + DeviceIdBox, DeviceKeyId, EventId, RoomId, UserId, }; use serde::{Deserialize, Serialize}; use serde_json::to_value; @@ -108,7 +108,7 @@ impl OwnUserIdentity { pub async fn request_verification( &self, ) -> Result<(VerificationRequest, OutgoingVerificationRequest), CryptoStoreError> { - self.verification_machine.request_self_verification(&self.inner, None).await + self.request_verification_helper(None).await } /// Send a verification request to our other devices while specifying our @@ -121,7 +121,27 @@ impl OwnUserIdentity { &self, methods: Vec, ) -> Result<(VerificationRequest, OutgoingVerificationRequest), CryptoStoreError> { - self.verification_machine.request_self_verification(&self.inner, Some(methods)).await + self.request_verification_helper(Some(methods)).await + } + + async fn request_verification_helper( + &self, + methods: Option>, + ) -> Result<(VerificationRequest, OutgoingVerificationRequest), CryptoStoreError> { + let devices: Vec = self + .verification_machine + .store + .get_user_devices(self.user_id()) + .await? + .into_iter() + .map(|(d, _)| d) + .filter(|d| &**d != self.verification_machine.own_device_id()) + .collect(); + + Ok(self + .verification_machine + .request_to_device_verification(self.user_id(), devices, methods) + .await) } } diff --git a/matrix_sdk_crypto/src/requests.rs b/matrix_sdk_crypto/src/requests.rs index 428edaef..79981b26 100644 --- a/matrix_sdk_crypto/src/requests.rs +++ b/matrix_sdk_crypto/src/requests.rs @@ -78,6 +78,29 @@ impl ToDeviceRequest { Self::new_with_id(recipient, recipient_device, content, Uuid::new_v4()) } + pub(crate) fn new_for_recipients( + recipient: &UserId, + recipient_devices: Vec, + content: AnyToDeviceEventContent, + txn_id: Uuid, + ) -> Self { + let mut messages = BTreeMap::new(); + let event_type = EventType::from(content.event_type()); + + if recipient_devices.is_empty() { + Self::new(recipient, DeviceIdOrAllDevices::AllDevices, content) + } else { + let device_messages = recipient_devices + .into_iter() + .map(|d| (DeviceIdOrAllDevices::DeviceId(d), Raw::from(content.clone()))) + .collect(); + + messages.insert(recipient.clone(), device_messages); + + ToDeviceRequest { event_type, txn_id, messages } + } + } + pub(crate) fn new_with_id( recipient: &UserId, recipient_device: impl Into, diff --git a/matrix_sdk_crypto/src/verification/event_enums.rs b/matrix_sdk_crypto/src/verification/event_enums.rs index e7c66725..0a01c8ad 100644 --- a/matrix_sdk_crypto/src/verification/event_enums.rs +++ b/matrix_sdk_crypto/src/verification/event_enums.rs @@ -379,6 +379,33 @@ try_from_outgoing_content!(MacContent, KeyVerificationMac); try_from_outgoing_content!(CancelContent, KeyVerificationCancel); try_from_outgoing_content!(DoneContent, KeyVerificationDone); +impl<'a> TryFrom<&'a OutgoingContent> for RequestContent<'a> { + type Error = (); + + fn try_from(value: &'a OutgoingContent) -> Result { + match value { + OutgoingContent::Room(_, c) => { + if let AnyMessageEventContent::RoomMessage(m) = c { + if let MessageType::VerificationRequest(c) = &m.msgtype { + Ok(Self::Room(c)) + } else { + Err(()) + } + } else { + Err(()) + } + } + OutgoingContent::ToDevice(c) => { + if let AnyToDeviceEventContent::KeyVerificationRequest(c) = c { + Ok(Self::ToDevice(c)) + } else { + Err(()) + } + } + } + } +} + #[derive(Debug)] pub enum StartContent<'a> { ToDevice(&'a StartToDeviceEventContent), diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index 3bea8480..8d0c4858 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -25,8 +25,7 @@ use ruma::{ ToDeviceEvent, }, serde::Raw, - to_device::DeviceIdOrAllDevices, - DeviceId, EventId, MilliSecondsSinceUnixEpoch, RoomId, UserId, + DeviceId, DeviceIdBox, EventId, MilliSecondsSinceUnixEpoch, RoomId, UserId, }; use tracing::{info, trace, warn}; @@ -41,8 +40,8 @@ use crate::{ olm::PrivateCrossSigningIdentity, requests::OutgoingRequest, store::{CryptoStore, CryptoStoreError}, - OutgoingVerificationRequest, ReadOnlyAccount, ReadOnlyDevice, ReadOnlyOwnUserIdentity, - ReadOnlyUserIdentity, RoomMessageRequest, ToDeviceRequest, + OutgoingVerificationRequest, ReadOnlyAccount, ReadOnlyDevice, ReadOnlyUserIdentity, + RoomMessageRequest, ToDeviceRequest, }; #[derive(Clone, Debug)] @@ -77,11 +76,12 @@ impl VerificationMachine { self.account.device_id() } - pub(crate) async fn request_self_verification( + pub(crate) async fn request_to_device_verification( &self, - identity: &ReadOnlyOwnUserIdentity, + user_id: &UserId, + recipient_devices: Vec, methods: Option>, - ) -> Result<(VerificationRequest, OutgoingVerificationRequest), CryptoStoreError> { + ) -> (VerificationRequest, OutgoingVerificationRequest) { let flow_id = FlowId::from(Uuid::new_v4().to_string()); let verification = VerificationRequest::new( @@ -90,22 +90,16 @@ impl VerificationMachine { self.private_identity.lock().await.clone(), self.store.clone(), flow_id, - identity.user_id(), + user_id, + recipient_devices, methods, ); - // TODO get all the device ids of the user instead of using AllDevices - // make sure to remember this so we can cancel once someone picks up - let request: OutgoingVerificationRequest = ToDeviceRequest::new( - identity.user_id(), - DeviceIdOrAllDevices::AllDevices, - AnyToDeviceEventContent::KeyVerificationRequest(verification.request_to_device()), - ) - .into(); - self.insert_request(verification.clone()); - Ok((verification, request)) + let request = verification.request_to_device(); + + (verification, request.into()) } pub async fn request_verification( @@ -124,6 +118,7 @@ impl VerificationMachine { self.store.clone(), flow_id, identity.user_id(), + vec![], methods, ); diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index 230407d6..450dfeb0 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -75,6 +75,7 @@ pub struct VerificationRequest { inner: Arc>, creation_time: Arc, we_started: bool, + recipient_devices: Arc>, } /// A handle to a request so child verification flows can cancel the request. @@ -110,6 +111,7 @@ impl VerificationRequest { store: Arc, flow_id: FlowId, other_user: &UserId, + recipient_devices: Vec, methods: Option>, ) -> Self { let inner = Mutex::new(InnerRequest::Created(RequestState::new( @@ -131,6 +133,7 @@ impl VerificationRequest { other_user_id: other_user.to_owned().into(), creation_time: Instant::now().into(), we_started: true, + recipient_devices: recipient_devices.into(), } } @@ -138,7 +141,7 @@ impl VerificationRequest { /// verification from the other side. This should be used only for /// self-verifications and it should be sent to the specific device that we /// want to verify. - pub(crate) fn request_to_device(&self) -> RequestToDeviceEventContent { + pub(crate) fn request_to_device(&self) -> ToDeviceRequest { let inner = self.inner.lock().unwrap(); let methods = if let InnerRequest::Created(c) = &*inner { @@ -147,11 +150,18 @@ impl VerificationRequest { SUPPORTED_METHODS.to_vec() }; - RequestToDeviceEventContent::new( + let content = RequestToDeviceEventContent::new( self.account.device_id().into(), self.flow_id().as_str().to_string(), methods, MilliSecondsSinceUnixEpoch::now(), + ); + + ToDeviceRequest::new_for_recipients( + self.other_user(), + self.recipient_devices.to_vec(), + AnyToDeviceEventContent::KeyVerificationRequest(content), + Uuid::new_v4(), ) } @@ -358,6 +368,7 @@ impl VerificationRequest { flow_id: flow_id.into(), we_started: false, creation_time: Instant::now().into(), + recipient_devices: vec![].into(), } } @@ -403,6 +414,10 @@ impl VerificationRequest { fn cancel_with_code(&self, cancel_code: CancelCode) -> Option { let mut inner = self.inner.lock().unwrap(); + + let send_to_everyone = self.we_started() && matches!(&*inner, InnerRequest::Created(_)); + let other_device = inner.other_device_id(); + inner.cancel(true, &cancel_code); let content = if let InnerRequest::Cancelled(c) = &*inner { @@ -413,7 +428,17 @@ impl VerificationRequest { let request = content.map(|c| match c { OutgoingContent::ToDevice(content) => { - ToDeviceRequest::new(&self.other_user(), inner.other_device_id(), content).into() + if send_to_everyone { + ToDeviceRequest::new_for_recipients( + &self.other_user(), + self.recipient_devices.to_vec(), + content, + Uuid::new_v4(), + ) + .into() + } else { + ToDeviceRequest::new(&self.other_user(), other_device, content).into() + } } OutgoingContent::Room(room_id, content) => { RoomMessageRequest { room_id, txn_id: Uuid::new_v4(), content }.into() @@ -444,12 +469,65 @@ impl VerificationRequest { } } + /// Create a key verification cancellation for devices that received the + /// request but either shouldn't continue in the verification or didn't get + /// notified that the other side cancelled. + /// + /// The spec states the following[1]: + /// When Bob accepts or declines the verification on one of his devices + /// (sending either an m.key.verification.ready or m.key.verification.cancel + /// event), Alice will send an m.key.verification.cancel event to Bob’s + /// other devices with a code of m.accepted in the case where Bob accepted + /// the verification, or m.user in the case where Bob rejected the + /// verification. + /// + /// Realistically sending the cancellation to Bob's other devices is only + /// possible if Bob accepted the verification since we don't know the device + /// id of Bob's device that rejected the verification. + /// + /// Thus, we're sending the cancellation to all devices that received the + /// request in the rejection case. + /// + /// [1]: https://spec.matrix.org/unstable/client-server-api/#key-verification-framework + pub(crate) fn cancel_for_other_devices( + &self, + code: CancelCode, + filter_device: Option<&DeviceId>, + ) -> Option { + let cancelled = Cancelled::new(true, code); + let cancel_content = cancelled.as_content(self.flow_id()); + + if let OutgoingContent::ToDevice(c) = cancel_content { + let recipients: Vec = self + .recipient_devices + .to_vec() + .into_iter() + .filter(|d| if let Some(device) = filter_device { &**d != device } else { true }) + .collect(); + + Some(ToDeviceRequest::new_for_recipients( + self.other_user(), + recipients, + c, + Uuid::new_v4(), + )) + } else { + None + } + } + pub(crate) fn receive_ready(&self, sender: &UserId, content: &ReadyContent) { let mut inner = self.inner.lock().unwrap(); match &*inner { InnerRequest::Created(s) => { *inner = InnerRequest::Ready(s.clone().into_ready(sender, content)); + + if let Some(request) = + self.cancel_for_other_devices(CancelCode::Accepted, Some(content.from_device())) + { + self.verification_cache.add_verification_request(request.into()); + } } InnerRequest::Requested(s) => { if sender == self.own_user_id() && content.from_device() != self.account.device_id() @@ -500,6 +578,12 @@ impl VerificationRequest { ); let mut inner = self.inner.lock().unwrap(); inner.cancel(false, content.cancel_code()); + + if let Some(request) = + self.cancel_for_other_devices(content.cancel_code().to_owned(), None) + { + self.verification_cache.add_verification_request(request.into()); + } } } @@ -597,9 +681,7 @@ impl InnerRequest { InnerRequest::Created(s) => s.clone().into_canceled(cancelled_by_us, cancel_code), InnerRequest::Requested(s) => s.clone().into_canceled(cancelled_by_us, cancel_code), InnerRequest::Ready(s) => s.clone().into_canceled(cancelled_by_us, cancel_code), - InnerRequest::Passive(s) => s.clone().into_canceled(cancelled_by_us, cancel_code), - InnerRequest::Done(_) => return, - InnerRequest::Cancelled(_) => return, + InnerRequest::Passive(_) | InnerRequest::Done(_) | InnerRequest::Cancelled(_) => return, }); } @@ -1133,7 +1215,7 @@ mod test { store::{Changes, CryptoStore, MemoryStore}, verification::{ cache::VerificationCache, - event_enums::{OutgoingContent, ReadyContent, StartContent}, + event_enums::{OutgoingContent, ReadyContent, RequestContent, StartContent}, FlowId, }, ReadOnlyDevice, @@ -1180,6 +1262,7 @@ mod test { bob_store.into(), flow_id.clone(), &alice_id(), + vec![], None, ); @@ -1237,6 +1320,7 @@ mod test { bob_store.into(), flow_id.clone(), &alice_id(), + vec![], None, ); @@ -1301,10 +1385,13 @@ mod test { bob_store.into(), flow_id, &alice_id(), + vec![], None, ); - let content = bob_request.request_to_device(); + let request = bob_request.request_to_device(); + let content: OutgoingContent = request.try_into().unwrap(); + let content = RequestContent::try_from(&content).unwrap(); let flow_id = bob_request.flow_id().to_owned(); let alice_request = VerificationRequest::from_request( @@ -1314,7 +1401,7 @@ mod test { alice_store.into(), &bob_id(), flow_id, - &(&content).into(), + &content, ); let content: OutgoingContent = alice_request.accept().unwrap().try_into().unwrap(); From 7433003ffa3b8943df83964714b3d62f4b78e689 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 19 Jul 2021 09:21:28 +0200 Subject: [PATCH 25/35] matrix-sdk: Don't panic when importing invalid key exports --- matrix_sdk/src/client.rs | 12 ++++++++---- matrix_sdk/src/error.rs | 31 ++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index 3d1d2713..f2105751 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -129,6 +129,7 @@ use ruma::{ #[cfg(feature = "encryption")] use crate::{ device::{Device, UserDevices}, + error::RoomKeyImportError, verification::{QrVerification, SasVerification, Verification, VerificationRequest}, }; use crate::{ @@ -2491,8 +2492,12 @@ impl Client { /// ``` #[cfg(all(feature = "encryption", not(target_arch = "wasm32")))] #[cfg_attr(feature = "docs", doc(cfg(all(encryption, not(target_arch = "wasm32")))))] - pub async fn import_keys(&self, path: PathBuf, passphrase: &str) -> Result<(usize, usize)> { - let olm = self.base_client.olm_machine().await.ok_or(Error::AuthenticationRequired)?; + pub async fn import_keys( + &self, + path: PathBuf, + passphrase: &str, + ) -> StdResult<(usize, usize), RoomKeyImportError> { + let olm = self.base_client.olm_machine().await.ok_or(RoomKeyImportError::StoreClosed)?; let passphrase = Zeroizing::new(passphrase.to_owned()); let decrypt = move || { @@ -2501,8 +2506,7 @@ impl Client { }; let task = tokio::task::spawn_blocking(decrypt); - // TODO remove this unwrap. - let import = task.await.expect("Task join error").unwrap(); + let import = task.await.expect("Task join error")?; Ok(olm.import_keys(import, |_, _| {}).await?) } diff --git a/matrix_sdk/src/error.rs b/matrix_sdk/src/error.rs index 968a9609..2fe22129 100644 --- a/matrix_sdk/src/error.rs +++ b/matrix_sdk/src/error.rs @@ -18,7 +18,9 @@ use std::io::Error as IoError; use http::StatusCode; #[cfg(feature = "encryption")] -use matrix_sdk_base::crypto::{CryptoStoreError, DecryptorError, MegolmError, OlmError}; +use matrix_sdk_base::crypto::{ + CryptoStoreError, DecryptorError, KeyExportError, MegolmError, OlmError, +}; use matrix_sdk_base::{Error as SdkBaseError, StoreError}; use reqwest::Error as ReqwestError; use ruma::{ @@ -151,6 +153,33 @@ pub enum Error { Url(#[from] UrlParseError), } +/// Error for the room key importing functionality. +#[cfg(feature = "encryption")] +#[cfg_attr(feature = "docs", doc(cfg(encryption)))] +#[derive(Error, Debug)] +pub enum RoomKeyImportError { + /// An error de/serializing type for the `StateStore` + #[error(transparent)] + SerdeJson(#[from] JsonError), + + /// The cryptostore isn't yet open, logging in is required to open the + /// cryptostore. + #[error("The cryptostore hasn't been yet opened, can't import yet.")] + StoreClosed, + + /// An IO error happened. + #[error(transparent)] + Io(#[from] IoError), + + /// An error occurred in the crypto store. + #[error(transparent)] + CryptoStore(#[from] CryptoStoreError), + + /// An error occurred while importing the key export. + #[error(transparent)] + Export(#[from] KeyExportError), +} + impl Error { /// Try to destructure the error into an universal interactive auth info. /// From 909cd42ac196a57d10a65db4d4477c196e515d32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 19 Jul 2021 09:32:48 +0200 Subject: [PATCH 26/35] crypto: Cancel the verification request for to-device Sas verifications --- matrix_sdk_crypto/src/identities/device.rs | 2 +- matrix_sdk_crypto/src/verification/machine.rs | 2 ++ matrix_sdk_crypto/src/verification/requests.rs | 1 + matrix_sdk_crypto/src/verification/sas/mod.rs | 4 +++- 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/matrix_sdk_crypto/src/identities/device.rs b/matrix_sdk_crypto/src/identities/device.rs index 79884c84..9b432e77 100644 --- a/matrix_sdk_crypto/src/identities/device.rs +++ b/matrix_sdk_crypto/src/identities/device.rs @@ -43,7 +43,7 @@ use crate::{ olm::{InboundGroupSession, PrivateCrossSigningIdentity, Session, Utility}, store::{Changes, CryptoStore, DeviceChanges, Result as StoreResult}, verification::VerificationMachine, - OutgoingVerificationRequest, Sas, ToDeviceRequest, VerificationRequest, + OutgoingVerificationRequest, Sas, ToDeviceRequest, }; #[cfg(test)] use crate::{OlmMachine, ReadOnlyAccount}; diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index 8d0c4858..fbab484f 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -142,6 +142,7 @@ impl VerificationMachine { identity, None, true, + None, ); let request = match content { @@ -565,6 +566,7 @@ mod test { None, None, true, + None, ); machine diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index 450dfeb0..2c78604c 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -1172,6 +1172,7 @@ impl RequestState { other_identity, Some(t.to_owned()), we_started, + Some(request_handle), ); (sas, content) } diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index c0cb4fe4..fd1e5e24 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -195,6 +195,7 @@ impl Sas { other_identity: Option, transaction_id: Option, we_started: bool, + request_handle: Option, ) -> (Sas, OutgoingContent) { let (inner, content) = InnerSas::start( account.clone(), @@ -212,7 +213,7 @@ impl Sas { store, other_identity, we_started, - None, + request_handle, ), content, ) @@ -627,6 +628,7 @@ mod test { None, None, true, + None, ); let flow_id = alice.flow_id().to_owned(); From ff8089912e95295ed2a8c9c94c8431981d18ee22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 19 Jul 2021 09:36:21 +0200 Subject: [PATCH 27/35] crypto: Only send cancellations to other devices if we're the requester --- matrix_sdk_crypto/src/verification/requests.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index 2c78604c..087728b6 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -579,10 +579,12 @@ impl VerificationRequest { let mut inner = self.inner.lock().unwrap(); inner.cancel(false, content.cancel_code()); - if let Some(request) = - self.cancel_for_other_devices(content.cancel_code().to_owned(), None) - { - self.verification_cache.add_verification_request(request.into()); + if self.we_started() { + if let Some(request) = + self.cancel_for_other_devices(content.cancel_code().to_owned(), None) + { + self.verification_cache.add_verification_request(request.into()); + } } } } From cf30c42563ccf3d6ed4ddd3cb3a95243c002b7e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 19 Jul 2021 09:43:35 +0200 Subject: [PATCH 28/35] crypto: Correctly remember our chosen SAS methods --- .../src/verification/sas/inner_sas.rs | 7 +++-- matrix_sdk_crypto/src/verification/sas/mod.rs | 27 +++---------------- .../src/verification/sas/sas_state.rs | 15 ++++++++--- 3 files changed, 20 insertions(+), 29 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/sas/inner_sas.rs b/matrix_sdk_crypto/src/verification/sas/inner_sas.rs index 04267442..161181b6 100644 --- a/matrix_sdk_crypto/src/verification/sas/inner_sas.rs +++ b/matrix_sdk_crypto/src/verification/sas/inner_sas.rs @@ -165,9 +165,12 @@ impl InnerSas { } } - pub fn accept(self) -> Option<(InnerSas, OwnedAcceptContent)> { + pub fn accept( + self, + methods: Vec, + ) -> Option<(InnerSas, OwnedAcceptContent)> { if let InnerSas::Started(s) = self { - let sas = s.into_accepted(); + let sas = s.into_accepted(methods); let content = sas.as_content(); Some((InnerSas::WeAccepted(sas), content)) } else { diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index fd1e5e24..56159d94 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -25,11 +25,7 @@ use matrix_sdk_common::uuid::Uuid; use ruma::{ api::client::r0::keys::upload_signatures::Request as SignatureUploadRequest, events::{ - key::verification::{ - accept::{AcceptEventContent, AcceptMethod, AcceptToDeviceEventContent}, - cancel::CancelCode, - ShortAuthenticationString, - }, + key::verification::{cancel::CancelCode, ShortAuthenticationString}, AnyMessageEventContent, AnyToDeviceEventContent, }, DeviceId, EventId, RoomId, UserId, @@ -327,10 +323,10 @@ impl Sas { ) -> Option { let mut guard = self.inner.lock().unwrap(); let sas: InnerSas = (*guard).clone(); + let methods = settings.allowed_methods; - if let Some((sas, content)) = sas.accept() { + if let Some((sas, content)) = sas.accept(methods) { *guard = sas; - let content = settings.apply(content); Some(match content { OwnedAcceptContent::ToDevice(c) => { @@ -554,23 +550,6 @@ impl AcceptSettings { pub fn with_allowed_methods(methods: Vec) -> Self { Self { allowed_methods: methods } } - - fn apply(self, mut content: OwnedAcceptContent) -> OwnedAcceptContent { - match &mut content { - OwnedAcceptContent::ToDevice(AcceptToDeviceEventContent { - method: AcceptMethod::SasV1(c), - .. - }) - | OwnedAcceptContent::Room( - _, - AcceptEventContent { method: AcceptMethod::SasV1(c), .. }, - ) => { - c.short_authentication_string.retain(|sas| self.allowed_methods.contains(sas)); - content - } - _ => content, - } - } } #[cfg(test)] diff --git a/matrix_sdk_crypto/src/verification/sas/sas_state.rs b/matrix_sdk_crypto/src/verification/sas/sas_state.rs index 06b59a7e..7aa9bc2f 100644 --- a/matrix_sdk_crypto/src/verification/sas/sas_state.rs +++ b/matrix_sdk_crypto/src/verification/sas/sas_state.rs @@ -557,7 +557,15 @@ impl SasState { } } - pub fn into_accepted(self) -> SasState { + pub fn into_accepted(self, methods: Vec) -> SasState { + let mut accepted_protocols = self.state.accepted_protocols.as_ref().to_owned(); + accepted_protocols.short_auth_string = methods; + + // Decimal is required per spec. + if !accepted_protocols.short_auth_string.contains(&ShortAuthenticationString::Decimal) { + accepted_protocols.short_auth_string.push(ShortAuthenticationString::Decimal); + } + SasState { inner: self.inner, ids: self.ids, @@ -567,7 +575,7 @@ impl SasState { started_from_request: self.started_from_request, state: Arc::new(WeAccepted { we_started: false, - accepted_protocols: self.state.accepted_protocols.clone(), + accepted_protocols: accepted_protocols.into(), commitment: self.state.commitment.clone(), }), } @@ -1115,6 +1123,7 @@ mod test { events::key::verification::{ accept::{AcceptMethod, AcceptToDeviceEventContent}, start::{StartMethod, StartToDeviceEventContent}, + ShortAuthenticationString, }, DeviceId, UserId, }; @@ -1162,7 +1171,7 @@ mod test { &start_content.as_start_content(), false, ); - let bob_sas = bob_sas.unwrap().into_accepted(); + let bob_sas = bob_sas.unwrap().into_accepted(vec![ShortAuthenticationString::Emoji]); (alice_sas, bob_sas) } From 55a9e6836df65061b36a5bc950e792a306e5489e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 19 Jul 2021 09:45:47 +0200 Subject: [PATCH 29/35] crypto: Introduce a CancelInfo struct This replaces the separate methods to fetch info about the cancellation. It was a bit annoying to gather all the different info where each method can return None. --- matrix_sdk_crypto/src/lib.rs | 4 ++- matrix_sdk_crypto/src/verification/mod.rs | 32 +++++++++++++++++++ matrix_sdk_crypto/src/verification/qrcode.rs | 18 +++-------- .../src/verification/requests.rs | 14 ++++---- .../src/verification/sas/inner_sas.rs | 16 ---------- matrix_sdk_crypto/src/verification/sas/mod.rs | 18 +++++------ 6 files changed, 57 insertions(+), 45 deletions(-) diff --git a/matrix_sdk_crypto/src/lib.rs b/matrix_sdk_crypto/src/lib.rs index a3d7a244..37251fda 100644 --- a/matrix_sdk_crypto/src/lib.rs +++ b/matrix_sdk_crypto/src/lib.rs @@ -57,4 +57,6 @@ pub use requests::{ OutgoingVerificationRequest, RoomMessageRequest, ToDeviceRequest, }; pub use store::CryptoStoreError; -pub use verification::{AcceptSettings, QrVerification, Sas, Verification, VerificationRequest}; +pub use verification::{ + AcceptSettings, CancelInfo, QrVerification, Sas, Verification, VerificationRequest, +}; diff --git a/matrix_sdk_crypto/src/verification/mod.rs b/matrix_sdk_crypto/src/verification/mod.rs index 49ae8e70..20ed6105 100644 --- a/matrix_sdk_crypto/src/verification/mod.rs +++ b/matrix_sdk_crypto/src/verification/mod.rs @@ -165,6 +165,38 @@ impl Done { } } +/// Information about the cancellation of a verification request or verification +/// flow. +#[derive(Clone, Debug)] +pub struct CancelInfo { + cancelled_by_us: bool, + cancel_code: CancelCode, + reason: &'static str, +} + +impl CancelInfo { + /// Get the human readable reason of the cancellation. + pub fn reason(&self) -> &'static str { + &self.reason + } + + /// Get the `CancelCode` that cancelled this verification. + pub fn cancel_code(&self) -> &CancelCode { + &self.cancel_code + } + + /// Was the verification cancelled by us? + pub fn cancelled_by_us(&self) -> bool { + self.cancelled_by_us + } +} + +impl From for CancelInfo { + fn from(c: Cancelled) -> Self { + Self { cancelled_by_us: c.cancelled_by_us, cancel_code: c.cancel_code, reason: c.reason } + } +} + #[derive(Clone, Debug)] pub struct Cancelled { cancelled_by_us: bool, diff --git a/matrix_sdk_crypto/src/verification/qrcode.rs b/matrix_sdk_crypto/src/verification/qrcode.rs index baff6fab..a645a8e8 100644 --- a/matrix_sdk_crypto/src/verification/qrcode.rs +++ b/matrix_sdk_crypto/src/verification/qrcode.rs @@ -41,7 +41,7 @@ use tracing::trace; use super::{ event_enums::{CancelContent, DoneContent, OutgoingContent, OwnedStartContent, StartContent}, requests::RequestHandle, - Cancelled, Done, FlowId, IdentitiesBeingVerified, VerificationResult, + CancelInfo, Cancelled, Done, FlowId, IdentitiesBeingVerified, VerificationResult, }; use crate::{ olm::{PrivateCrossSigningIdentity, ReadOnlyAccount}, @@ -134,19 +134,11 @@ impl QrVerification { self.we_started } - /// Get the `CancelCode` that cancelled this verification request. - pub fn cancel_code(&self) -> Option { + /// Get info about the cancellation if the verification flow has been + /// cancelled. + pub fn cancel_info(&self) -> Option { if let InnerState::Cancelled(c) = &*self.state.lock().unwrap() { - Some(c.state.cancel_code.to_owned()) - } else { - None - } - } - - /// Has the verification flow been cancelled by us. - pub fn cancelled_by_us(&self) -> Option { - if let InnerState::Cancelled(c) = &*self.state.lock().unwrap() { - Some(c.state.cancelled_by_us) + Some(c.state.clone().into()) } else { None } diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index 087728b6..e12abe10 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -42,7 +42,7 @@ use super::{ CancelContent, DoneContent, OutgoingContent, ReadyContent, RequestContent, StartContent, }, qrcode::{QrVerification, ScanError}, - Cancelled, FlowId, IdentitiesBeingVerified, + CancelInfo, Cancelled, FlowId, IdentitiesBeingVerified, }; use crate::{ olm::{PrivateCrossSigningIdentity, ReadOnlyAccount}, @@ -219,11 +219,13 @@ impl VerificationRequest { } } - /// Get the `CancelCode` that cancelled this verification request. - pub fn cancel_code(&self) -> Option { - match &*self.inner.lock().unwrap() { - InnerRequest::Cancelled(c) => Some(c.state.cancel_code.to_owned()), - _ => None, + /// Get info about the cancellation if the verification request has been + /// cancelled. + pub fn cancel_info(&self) -> Option { + if let InnerRequest::Cancelled(c) = &*self.inner.lock().unwrap() { + Some(c.state.clone().into()) + } else { + None } } diff --git a/matrix_sdk_crypto/src/verification/sas/inner_sas.rs b/matrix_sdk_crypto/src/verification/sas/inner_sas.rs index 161181b6..53677e33 100644 --- a/matrix_sdk_crypto/src/verification/sas/inner_sas.rs +++ b/matrix_sdk_crypto/src/verification/sas/inner_sas.rs @@ -350,22 +350,6 @@ impl InnerSas { matches!(self, InnerSas::Confirmed(_) | InnerSas::WaitingForDone(_) | InnerSas::Done(_)) } - pub fn cancel_code(&self) -> Option { - if let InnerSas::Cancelled(c) = self { - Some(c.state.cancel_code.clone()) - } else { - None - } - } - - pub fn cancelled_by_us(&self) -> Option { - if let InnerSas::Cancelled(c) = self { - Some(c.state.cancelled_by_us) - } else { - None - } - } - pub fn timed_out(&self) -> bool { match self { InnerSas::Created(s) => s.timed_out(), diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index 56159d94..c382205f 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -35,7 +35,7 @@ use tracing::trace; use super::{ event_enums::{AnyVerificationContent, OutgoingContent, OwnedAcceptContent, StartContent}, requests::RequestHandle, - FlowId, IdentitiesBeingVerified, VerificationResult, + CancelInfo, FlowId, IdentitiesBeingVerified, VerificationResult, }; use crate::{ identities::{ReadOnlyDevice, ReadOnlyUserIdentities}, @@ -122,14 +122,14 @@ impl Sas { self.inner.lock().unwrap().has_been_accepted() } - /// Get the cancel code of this SAS verification if it has been cancelled - pub fn cancel_code(&self) -> Option { - self.inner.lock().unwrap().cancel_code() - } - - /// Has the verification flow been cancelled by us. - pub fn cancelled_by_us(&self) -> Option { - self.inner.lock().unwrap().cancelled_by_us() + /// Get info about the cancellation if the verification flow has been + /// cancelled. + pub fn cancel_info(&self) -> Option { + if let InnerSas::Cancelled(c) = &*self.inner.lock().unwrap() { + Some(c.state.as_ref().clone().into()) + } else { + None + } } /// Did we initiate the verification flow. From 0053d2a874f3e506efb1498b694bc9187444c5e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 19 Jul 2021 09:48:22 +0200 Subject: [PATCH 30/35] crypto: Don't send cancellations for passive verification requests --- matrix_sdk_crypto/src/verification/requests.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index e12abe10..41d1081d 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -465,7 +465,13 @@ impl VerificationRequest { if self.is_cancelled() || self.is_done() { None } else if self.timed_out() { - self.cancel_with_code(CancelCode::Timeout) + let request = self.cancel_with_code(CancelCode::Timeout); + + if self.is_passive() { + None + } else { + request + } } else { None } @@ -681,6 +687,12 @@ impl InnerRequest { } fn cancel(&mut self, cancelled_by_us: bool, cancel_code: &CancelCode) { + trace!( + cancelled_by_us = cancelled_by_us, + code = cancel_code.as_str(), + "Verification request going into the cancelled state" + ); + *self = InnerRequest::Cancelled(match self { InnerRequest::Created(s) => s.clone().into_canceled(cancelled_by_us, cancel_code), InnerRequest::Requested(s) => s.clone().into_canceled(cancelled_by_us, cancel_code), From 8f03679935bc636b3cd3396d72075a9e532d95c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 19 Jul 2021 09:59:08 +0200 Subject: [PATCH 31/35] matrix-sdk: Add more getters to the high level verification structs --- matrix_sdk/src/verification/mod.rs | 18 ++++++++++++++++++ matrix_sdk/src/verification/qrcode.rs | 19 ++++++++++++++++++- matrix_sdk/src/verification/requests.rs | 13 ++++++++++++- matrix_sdk/src/verification/sas.rs | 19 ++++++++++++++++++- 4 files changed, 66 insertions(+), 3 deletions(-) diff --git a/matrix_sdk/src/verification/mod.rs b/matrix_sdk/src/verification/mod.rs index a742badf..80a21f2b 100644 --- a/matrix_sdk/src/verification/mod.rs +++ b/matrix_sdk/src/verification/mod.rs @@ -33,6 +33,7 @@ mod qrcode; mod requests; mod sas; +pub use matrix_sdk_base::crypto::{AcceptSettings, CancelInfo}; pub use qrcode::QrVerification; pub use requests::VerificationRequest; pub use sas::SasVerification; @@ -81,6 +82,15 @@ impl Verification { } } + /// Get info about the cancellation if the verification flow has been + /// cancelled. + pub fn cancel_info(&self) -> Option { + match self { + Verification::SasV1(s) => s.cancel_info(), + Verification::QrV1(q) => q.cancel_info(), + } + } + /// Get our own user id. pub fn own_user_id(&self) -> &ruma::UserId { match self { @@ -105,6 +115,14 @@ impl Verification { Verification::QrV1(v) => v.is_self_verification(), } } + + /// Did we initiate the verification flow. + pub fn we_started(&self) -> bool { + match self { + Verification::SasV1(s) => s.we_started(), + Verification::QrV1(q) => q.we_started(), + } + } } impl From for Verification { diff --git a/matrix_sdk/src/verification/qrcode.rs b/matrix_sdk/src/verification/qrcode.rs index f28d11b5..78b80e80 100644 --- a/matrix_sdk/src/verification/qrcode.rs +++ b/matrix_sdk/src/verification/qrcode.rs @@ -14,7 +14,7 @@ use matrix_sdk_base::crypto::{ matrix_qrcode::{qrcode::QrCode, EncodingError}, - QrVerification as BaseQrVerification, + CancelInfo, QrVerification as BaseQrVerification, }; use ruma::UserId; @@ -43,6 +43,23 @@ impl QrVerification { self.inner.is_done() } + /// Did we initiate the verification flow. + pub fn we_started(&self) -> bool { + self.inner.we_started() + } + + /// Get info about the cancellation if the verification flow has been + /// cancelled. + pub fn cancel_info(&self) -> Option { + self.inner.cancel_info() + } + + /// Get the user id of the other user participating in this verification + /// flow. + pub fn other_user_id(&self) -> &UserId { + self.inner.other_user_id() + } + /// Has the verification been cancelled. pub fn is_cancelled(&self) -> bool { self.inner.is_cancelled() diff --git a/matrix_sdk/src/verification/requests.rs b/matrix_sdk/src/verification/requests.rs index 5a94c7b9..b7b39485 100644 --- a/matrix_sdk/src/verification/requests.rs +++ b/matrix_sdk/src/verification/requests.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use matrix_sdk_base::crypto::VerificationRequest as BaseVerificationRequest; +use matrix_sdk_base::crypto::{CancelInfo, VerificationRequest as BaseVerificationRequest}; use ruma::events::key::verification::VerificationMethod; use super::{QrVerification, SasVerification}; @@ -36,6 +36,12 @@ impl VerificationRequest { self.inner.is_cancelled() } + /// Get info about the cancellation if the verification request has been + /// cancelled. + pub fn cancel_info(&self) -> Option { + self.inner.cancel_info() + } + /// Get our own user id. pub fn own_user_id(&self) -> &ruma::UserId { self.inner.own_user_id() @@ -51,6 +57,11 @@ impl VerificationRequest { self.inner.is_ready() } + /// Did we initiate the verification flow. + pub fn we_started(&self) -> bool { + self.inner.we_started() + } + /// Get the user id of the other user participating in this verification /// flow. pub fn other_user_id(&self) -> &ruma::UserId { diff --git a/matrix_sdk/src/verification/sas.rs b/matrix_sdk/src/verification/sas.rs index 76d1df92..3cb46641 100644 --- a/matrix_sdk/src/verification/sas.rs +++ b/matrix_sdk/src/verification/sas.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use matrix_sdk_base::crypto::{AcceptSettings, ReadOnlyDevice, Sas as BaseSas}; +use matrix_sdk_base::crypto::{AcceptSettings, CancelInfo, ReadOnlyDevice, Sas as BaseSas}; use ruma::UserId; use crate::{error::Result, Client}; @@ -121,6 +121,17 @@ impl SasVerification { self.inner.can_be_presented() } + /// Did we initiate the verification flow. + pub fn we_started(&self) -> bool { + self.inner.we_started() + } + + /// Get info about the cancellation if the verification flow has been + /// cancelled. + pub fn cancel_info(&self) -> Option { + self.inner.cancel_info() + } + /// Is the verification process canceled. pub fn is_cancelled(&self) -> bool { self.inner.is_cancelled() @@ -145,4 +156,10 @@ impl SasVerification { pub fn own_user_id(&self) -> &UserId { self.inner.user_id() } + + /// Get the user id of the other user participating in this verification + /// flow. + pub fn other_user_id(&self) -> &UserId { + self.inner.other_user_id() + } } From 5566886f208e9dfd06bc2b4c0236b8ce86b1dffe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 19 Jul 2021 10:26:39 +0200 Subject: [PATCH 32/35] crypto: Add public methods to request verifications with devices --- matrix_sdk_crypto/src/identities/device.rs | 49 ++++++++++++++++++++-- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/matrix_sdk_crypto/src/identities/device.rs b/matrix_sdk_crypto/src/identities/device.rs index 9b432e77..5f782a60 100644 --- a/matrix_sdk_crypto/src/identities/device.rs +++ b/matrix_sdk_crypto/src/identities/device.rs @@ -28,7 +28,8 @@ use ruma::{ encryption::{DeviceKeys, SignedKey}, events::{ forwarded_room_key::ForwardedRoomKeyToDeviceEventContent, - room::encrypted::EncryptedEventContent, AnyToDeviceEventContent, + key::verification::VerificationMethod, room::encrypted::EncryptedEventContent, + AnyToDeviceEventContent, }, DeviceId, DeviceIdBox, DeviceKeyAlgorithm, DeviceKeyId, EventEncryptionAlgorithm, UserId, }; @@ -43,7 +44,7 @@ use crate::{ olm::{InboundGroupSession, PrivateCrossSigningIdentity, Session, Utility}, store::{Changes, CryptoStore, DeviceChanges, Result as StoreResult}, verification::VerificationMachine, - OutgoingVerificationRequest, Sas, ToDeviceRequest, + OutgoingVerificationRequest, Sas, ToDeviceRequest, VerificationRequest, }; #[cfg(test)] use crate::{OlmMachine, ReadOnlyAccount}; @@ -125,7 +126,13 @@ impl Deref for Device { impl Device { /// Start a interactive verification with this `Device` /// - /// Returns a `Sas` object and to-device request that needs to be sent out. + /// Returns a `Sas` object and a to-device request that needs to be sent + /// out. + /// + /// This method has been deprecated in the spec and the + /// [`request_verification()`] method should be used instead. + /// + /// [`request_verification()`]: #method.request_verification pub async fn start_verification(&self) -> StoreResult<(Sas, ToDeviceRequest)> { let (sas, request) = self.verification_machine.start_sas(self.inner.clone()).await?; @@ -136,6 +143,42 @@ impl Device { } } + /// Request an interacitve verification with this `Device` + /// + /// Returns a `VerificationRequest` object and a to-device request that + /// needs to be sent out. + pub async fn request_verification(&self) -> (VerificationRequest, OutgoingVerificationRequest) { + self.request_verification_helper(None).await + } + + /// Request an interacitve verification with this `Device` + /// + /// Returns a `VerificationRequest` object and a to-device request that + /// needs to be sent out. + /// + /// # Arguments + /// + /// * `methods` - The verification methods that we want to support. + pub async fn request_verification_with_methods( + &self, + methods: Vec, + ) -> (VerificationRequest, OutgoingVerificationRequest) { + self.request_verification_helper(Some(methods)).await + } + + async fn request_verification_helper( + &self, + methods: Option>, + ) -> (VerificationRequest, OutgoingVerificationRequest) { + self.verification_machine + .request_to_device_verification( + self.user_id(), + vec![self.device_id().to_owned()], + methods, + ) + .await + } + /// Get the Olm sessions that belong to this device. pub(crate) async fn get_sessions(&self) -> StoreResult>>>> { if let Some(k) = self.get_key(DeviceKeyAlgorithm::Curve25519) { From 24377a45ff6c329416749a8b5c9a016f767c2933 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 19 Jul 2021 10:34:34 +0200 Subject: [PATCH 33/35] matrix-sdk: Add methods to request verification for devices --- matrix_sdk/src/device.rs | 102 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 99 insertions(+), 3 deletions(-) diff --git a/matrix_sdk/src/device.rs b/matrix_sdk/src/device.rs index 92c4fdaa..3d50a460 100644 --- a/matrix_sdk/src/device.rs +++ b/matrix_sdk/src/device.rs @@ -18,9 +18,13 @@ use matrix_sdk_base::crypto::{ store::CryptoStoreError, Device as BaseDevice, LocalTrust, ReadOnlyDevice, UserDevices as BaseUserDevices, }; -use ruma::{DeviceId, DeviceIdBox}; +use ruma::{events::key::verification::VerificationMethod, DeviceId, DeviceIdBox}; -use crate::{error::Result, verification::SasVerification, Client}; +use crate::{ + error::Result, + verification::{SasVerification, VerificationRequest}, + Client, +}; #[derive(Clone, Debug)] /// A device represents a E2EE capable client of an user. @@ -43,7 +47,10 @@ impl Device { /// Returns a `Sas` object that represents the interactive verification /// flow. /// - /// # Example + /// This method has been deprecated in the spec and the + /// [`request_verification()`] method should be used instead. + /// + /// # Examples /// /// ```no_run /// # use std::convert::TryFrom; @@ -62,6 +69,8 @@ impl Device { /// let verification = device.start_verification().await.unwrap(); /// # }); /// ``` + /// + /// [`request_verification()`]: #method.request_verification pub async fn start_verification(&self) -> Result { let (sas, request) = self.inner.start_verification().await?; self.client.send_to_device(&request).await?; @@ -69,6 +78,93 @@ impl Device { Ok(SasVerification { inner: sas, client: self.client.clone() }) } + /// Request an interacitve verification with this `Device` + /// + /// Returns a `VerificationRequest` object and a to-device request that + /// needs to be sent out. + /// + /// The default methods that are supported are `m.sas.v1` and + /// `m.qr_code.show.v1`, if this isn't desireable the + /// [`request_verification_with_methods()`] method can be used to override + /// this. + /// + /// # Examples + /// + /// ```no_run + /// # use std::convert::TryFrom; + /// # use matrix_sdk::{Client, ruma::UserId}; + /// # use url::Url; + /// # use futures::executor::block_on; + /// # let alice = UserId::try_from("@alice:example.org").unwrap(); + /// # let homeserver = Url::parse("http://example.com").unwrap(); + /// # let client = Client::new(homeserver).unwrap(); + /// # block_on(async { + /// let device = client.get_device(&alice, "DEVICEID".into()) + /// .await + /// .unwrap() + /// .unwrap(); + /// + /// let verification = device.request_verification().await.unwrap(); + /// # }); + /// ``` + /// + /// [`request_verification_with_methods()`]: + /// #method.request_verification_with_methods + pub async fn request_verification(&self) -> Result { + let (verification, request) = self.inner.request_verification().await; + self.client.send_verification_request(request).await?; + + Ok(VerificationRequest { inner: verification, client: self.client.clone() }) + } + + /// Request an interacitve verification with this `Device` + /// + /// Returns a `VerificationRequest` object and a to-device request that + /// needs to be sent out. + /// + /// # Arguments + /// + /// * `methods` - The verification methods that we want to support. + /// + /// # Examples + /// + /// ```no_run + /// # use std::convert::TryFrom; + /// # use matrix_sdk::{ + /// # Client, + /// # ruma::{ + /// # UserId, + /// # events::key::verification::VerificationMethod, + /// # } + /// # }; + /// # use url::Url; + /// # use futures::executor::block_on; + /// # let alice = UserId::try_from("@alice:example.org").unwrap(); + /// # let homeserver = Url::parse("http://example.com").unwrap(); + /// # let client = Client::new(homeserver).unwrap(); + /// # block_on(async { + /// let device = client.get_device(&alice, "DEVICEID".into()) + /// .await + /// .unwrap() + /// .unwrap(); + /// + /// // We don't want to support showing a QR code, we only support SAS + /// // verification + /// let methods = vec![VerificationMethod::SasV1]; + /// + /// let verification = device.request_verification_with_methods(methods).await.unwrap(); + /// # }); + /// ``` + pub async fn request_verification_with_methods( + &self, + methods: Vec, + ) -> Result { + let (verification, request) = self.inner.request_verification_with_methods(methods).await; + self.client.send_verification_request(request).await?; + + Ok(VerificationRequest { inner: verification, client: self.client.clone() }) + } + /// Is the device considered to be verified, either by locally trusting it /// or using cross signing. pub fn verified(&self) -> bool { From 26310def0ad90021741bfd4192f097e4768978eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 19 Jul 2021 10:49:22 +0200 Subject: [PATCH 34/35] crypto: Silence a couple of clippy warnings --- matrix_sdk_crypto/src/verification/requests.rs | 1 + matrix_sdk_crypto/src/verification/sas/mod.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/matrix_sdk_crypto/src/verification/requests.rs b/matrix_sdk_crypto/src/verification/requests.rs index 41d1081d..f20d6d15 100644 --- a/matrix_sdk_crypto/src/verification/requests.rs +++ b/matrix_sdk_crypto/src/verification/requests.rs @@ -104,6 +104,7 @@ impl From>> for RequestHandle { } impl VerificationRequest { + #[allow(clippy::too_many_arguments)] pub(crate) fn new( cache: VerificationCache, account: ReadOnlyAccount, diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index c382205f..d4b06949 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -183,6 +183,7 @@ impl Sas { /// /// Returns the new `Sas` object and a `StartEventContent` that needs to be /// sent out through the server to the other device. + #[allow(clippy::too_many_arguments)] pub(crate) fn start( account: ReadOnlyAccount, private_identity: PrivateCrossSigningIdentity, From 3a8ff2f6b43f312b7582146ed712ff245ef9d5aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 19 Jul 2021 11:19:14 +0200 Subject: [PATCH 35/35] matrix-sdk: Allow the key import error to be dead under WASM --- matrix_sdk/src/error.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/matrix_sdk/src/error.rs b/matrix_sdk/src/error.rs index 2fe22129..6ebb9716 100644 --- a/matrix_sdk/src/error.rs +++ b/matrix_sdk/src/error.rs @@ -157,6 +157,8 @@ pub enum Error { #[cfg(feature = "encryption")] #[cfg_attr(feature = "docs", doc(cfg(encryption)))] #[derive(Error, Debug)] +// This is allowed because key importing isn't enabled under wasm. +#[allow(dead_code)] pub enum RoomKeyImportError { /// An error de/serializing type for the `StateStore` #[error(transparent)]