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] 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,