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