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] 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();