crypto: Time out verification requests as well

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<OutgoingRequest> {
+    pub fn garbage_collect(&self) -> Vec<OutgoingVerificationRequest> {
         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<OutgoingRequest> = v
+                let requests: Vec<OutgoingVerificationRequest> = v
                     .value()
                     .iter()
                     .filter_map(|s| {
                         if let Verification::SasV1(s) = s.value() {
-                            s.cancel_if_timed_out().map(|r| OutgoingRequest {
+                            s.cancel_if_timed_out()
-                                request_id: r.request_id(),
-                                request: Arc::new(r.into()),
-                            })
                         } 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,

matrix_sdk_crypto/src/verification/event_enums.rs

@@ -665,16 +665,15 @@ 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 TryFrom<OutgoingVerificationRequest> for OutgoingContent {
-impl From<OutgoingVerificationRequest> for OutgoingContent {
+    type Error = String;
-    fn from(request: OutgoingVerificationRequest) -> Self {
+
+    fn try_from(request: OutgoingVerificationRequest) -> Result<Self, Self::Error> {
         match request {
-            OutgoingVerificationRequest::ToDevice(r) => Self::try_from(r).unwrap(),
+            OutgoingVerificationRequest::ToDevice(r) => Self::try_from(r),
-            OutgoingVerificationRequest::InRoom(r) => Self::from(r),
+            OutgoingVerificationRequest::InRoom(r) => Ok(Self::from(r)),
         }
     }
 }

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<OutgoingVerificationRequest> = self
+            .requests
+            .iter()
+            .flat_map(|v| {
+                let requests: Vec<OutgoingVerificationRequest> =
+                    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

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)
     }
 

matrix_sdk_crypto/src/verification/qrcode.rs

@@ -207,7 +207,7 @@ impl QrVerification {
 
     /// Cancel the verification flow.
     pub fn cancel(&self) -> Option<OutgoingVerificationRequest> {
-        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<OutgoingContent> {
+    pub fn cancel_with_code(&self, code: CancelCode) -> Option<OutgoingVerificationRequest> {
         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();
 

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<FlowId>,
     other_user_id: Arc<UserId>,
     inner: Arc<Mutex<InnerRequest>>,
+    creation_time: Arc<Instant>,
     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<OutgoingVerificationRequest> {
+        self.cancel_with_code(CancelCode::User)
+    }
+
+    fn cancel_with_code(&self, cancel_code: CancelCode) -> Option<OutgoingVerificationRequest> {
         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::SasV1(s) => s.cancel_with_code(cancel_code),
-                crate::Verification::QrV1(q) => q.cancel(),
+                crate::Verification::QrV1(q) => q.cancel_with_code(cancel_code),
             };
         }
 
         request
     }
 
+    pub(crate) fn cancel_if_timed_out(&self) -> Option<OutgoingVerificationRequest> {
+        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();