From e9be23f8535dfce3f40f56ca9eb3c0e6be274ab9 Mon Sep 17 00:00:00 2001 From: Jan Veen Date: Thu, 11 Mar 2021 21:10:26 +0100 Subject: [PATCH 1/3] crypto: Add settings to customize SAS accepting Offer specifying settings to SAS accept() requests to limit the allowed verification methods. --- matrix_sdk/src/sas.rs | 13 ++- matrix_sdk_crypto/src/lib.rs | 2 +- matrix_sdk_crypto/src/verification/mod.rs | 2 +- matrix_sdk_crypto/src/verification/sas/mod.rs | 98 ++++++++++++++++--- 4 files changed, 97 insertions(+), 18 deletions(-) diff --git a/matrix_sdk/src/sas.rs b/matrix_sdk/src/sas.rs index 389633f7..2baa10c0 100644 --- a/matrix_sdk/src/sas.rs +++ b/matrix_sdk/src/sas.rs @@ -12,7 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -use matrix_sdk_base::crypto::{OutgoingVerificationRequest, ReadOnlyDevice, Sas as BaseSas}; +use matrix_sdk_base::crypto::{ + AcceptSettings, OutgoingVerificationRequest, ReadOnlyDevice, Sas as BaseSas, +}; use crate::{error::Result, Client}; @@ -26,7 +28,14 @@ pub struct Sas { impl Sas { /// Accept the interactive verification flow. pub async fn accept(&self) -> Result<()> { - if let Some(req) = self.inner.accept() { + self.accept_with_settings(Default::default()).await + } + + /// Accept the interactive verification flow with specific settings. + /// + /// Pass `settings` to customizes the accept-request before sending it. + pub async fn accept_with_settings(&self, settings: AcceptSettings) -> Result<()> { + if let Some(req) = self.inner.accept_with_settings(settings) { match req { OutgoingVerificationRequest::ToDevice(r) => { self.client.send_to_device(&r).await?; diff --git a/matrix_sdk_crypto/src/lib.rs b/matrix_sdk_crypto/src/lib.rs index e5fcfad3..96dde71a 100644 --- a/matrix_sdk_crypto/src/lib.rs +++ b/matrix_sdk_crypto/src/lib.rs @@ -55,4 +55,4 @@ pub use requests::{ OutgoingVerificationRequest, RoomMessageRequest, ToDeviceRequest, }; pub use store::CryptoStoreError; -pub use verification::{Sas, VerificationRequest}; +pub use verification::{AcceptSettings, Sas, VerificationRequest}; diff --git a/matrix_sdk_crypto/src/verification/mod.rs b/matrix_sdk_crypto/src/verification/mod.rs index eeed8de5..c642dd21 100644 --- a/matrix_sdk_crypto/src/verification/mod.rs +++ b/matrix_sdk_crypto/src/verification/mod.rs @@ -18,7 +18,7 @@ mod sas; pub use machine::VerificationMachine; pub use requests::VerificationRequest; -pub use sas::{Sas, VerificationResult}; +pub use sas::{AcceptSettings, Sas, VerificationResult}; #[cfg(test)] pub(crate) mod test { diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index 22445fa0..62210d5e 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -27,8 +27,12 @@ use tracing::{error, info, trace, warn}; use matrix_sdk_common::{ api::r0::keys::upload_signatures::Request as SignatureUploadRequest, events::{ - key::verification::cancel::CancelCode, AnyMessageEvent, AnyMessageEventContent, - AnyToDeviceEvent, AnyToDeviceEventContent, + key::verification::{ + accept::{AcceptEventContent, AcceptMethod, AcceptToDeviceEventContent}, + cancel::CancelCode, + ShortAuthenticationString, + }, + AnyMessageEvent, AnyMessageEventContent, AnyToDeviceEvent, AnyToDeviceEventContent, }, identifiers::{DeviceId, EventId, RoomId, UserId}, uuid::Uuid, @@ -253,18 +257,35 @@ impl Sas { /// This does nothing if the verification was already accepted, otherwise it /// returns an `AcceptEventContent` that needs to be sent out. pub fn accept(&self) -> Option { - self.inner.lock().unwrap().accept().map(|c| match c { - AcceptContent::ToDevice(c) => { - let content = AnyToDeviceEventContent::KeyVerificationAccept(c); - self.content_to_request(content).into() - } - AcceptContent::Room(room_id, content) => RoomMessageRequest { - room_id, - txn_id: Uuid::new_v4(), - content: AnyMessageEventContent::KeyVerificationAccept(content), - } - .into(), - }) + self.accept_with_settings(Default::default()) + } + + /// Accept the SAS verification customizing the accept method. + /// + /// This does nothing if the verification was already accepted, otherwise it + /// returns an `AcceptEventContent` that needs to be sent out. + /// + /// Specify a function modifying the attributes of the accept request. + pub fn accept_with_settings( + &self, + settings: AcceptSettings, + ) -> Option { + self.inner + .lock() + .unwrap() + .accept() + .map(|c| match settings.apply(c) { + AcceptContent::ToDevice(c) => { + let content = AnyToDeviceEventContent::KeyVerificationAccept(c); + self.content_to_request(content).into() + } + AcceptContent::Room(room_id, content) => RoomMessageRequest { + room_id, + txn_id: Uuid::new_v4(), + content: AnyMessageEventContent::KeyVerificationAccept(content), + } + .into(), + }) } /// Confirm the Sas verification. @@ -654,6 +675,55 @@ impl Sas { } } +/// Customize the accept-reply for a verification process +#[derive(Debug)] +pub struct AcceptSettings { + allowed_methods: Vec, +} + +impl Default for AcceptSettings { + fn default() -> Self { + Self { + allowed_methods: vec![ + ShortAuthenticationString::Decimal, + ShortAuthenticationString::Emoji, + ], + } + } +} + +impl AcceptSettings { + /// Create settings with exactly the specified SAS methods + pub fn with_allowed_methods(methods: Vec) -> Self { + Self { + allowed_methods: methods, + } + } + + fn apply(self, mut content: AcceptContent) -> AcceptContent { + match &mut content { + AcceptContent::ToDevice(AcceptToDeviceEventContent { + method: AcceptMethod::MSasV1(c), + .. + }) => { + c.short_authentication_string = self.allowed_methods; + content + } + AcceptContent::Room( + _, + AcceptEventContent { + method: AcceptMethod::MSasV1(c), + .. + }, + ) => { + c.short_authentication_string = self.allowed_methods; + content + } + _ => content, + } + } +} + #[cfg(test)] mod test { use std::{convert::TryFrom, sync::Arc}; From 587c09e700a913c276f5b5c9aba790247475346d Mon Sep 17 00:00:00 2001 From: Jan Veen Date: Fri, 12 Mar 2021 14:43:59 +0100 Subject: [PATCH 2/3] crypto: Prohibit extending verification methods Intersect the allowed methods passed from the user with the methods supported by the other party. If the user added new methods to the request, the remote party would cancel the verification. --- matrix_sdk_crypto/src/verification/sas/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index 62210d5e..4c0f3abf 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -705,18 +705,16 @@ impl AcceptSettings { AcceptContent::ToDevice(AcceptToDeviceEventContent { method: AcceptMethod::MSasV1(c), .. - }) => { - c.short_authentication_string = self.allowed_methods; - content - } - AcceptContent::Room( + }) + | AcceptContent::Room( _, AcceptEventContent { method: AcceptMethod::MSasV1(c), .. }, ) => { - c.short_authentication_string = self.allowed_methods; + c.short_authentication_string + .retain(|sas| self.allowed_methods.contains(sas)); content } _ => content, From 42c8c421502b12a9af8e058c7cd9cca8cbcb0eb3 Mon Sep 17 00:00:00 2001 From: Jan Veen Date: Fri, 12 Mar 2021 15:45:58 +0100 Subject: [PATCH 3/3] crypto: Improve doc of SAS accept settings Document arguments explicitly. Adapt to changed implementation. Provide example call. --- matrix_sdk/src/sas.rs | 17 ++++++++++++++++- matrix_sdk_crypto/src/verification/sas/mod.rs | 7 ++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/matrix_sdk/src/sas.rs b/matrix_sdk/src/sas.rs index 2baa10c0..01d548fe 100644 --- a/matrix_sdk/src/sas.rs +++ b/matrix_sdk/src/sas.rs @@ -33,7 +33,22 @@ impl Sas { /// Accept the interactive verification flow with specific settings. /// - /// Pass `settings` to customizes the accept-request before sending it. + /// # Arguments + /// + /// * `settings` - specific customizations to the verification flow. + /// + /// # Examples + /// + /// ```ignore + /// use matrix_sdk::Sas; + /// use matrix_sdk_base::crypto::AcceptSettings; + /// use matrix_sdk::events::key::verification::ShortAuthenticationString; + /// + /// let only_decimal = AcceptSettings::with_allowed_methods( + /// vec![ShortAuthenticationString::Decimal] + /// ); + /// sas.accept_with_settings(only_decimal); + /// ``` pub async fn accept_with_settings(&self, settings: AcceptSettings) -> Result<()> { if let Some(req) = self.inner.accept_with_settings(settings) { match req { diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index 4c0f3abf..12b5f161 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -682,6 +682,7 @@ pub struct AcceptSettings { } impl Default for AcceptSettings { + /// All methods are allowed fn default() -> Self { Self { allowed_methods: vec![ @@ -693,7 +694,11 @@ impl Default for AcceptSettings { } impl AcceptSettings { - /// Create settings with exactly the specified SAS methods + /// Create settings restricting the allowed SAS methods + /// + /// # Arguments + /// + /// * `methods` - The methods this client allows at most pub fn with_allowed_methods(methods: Vec) -> Self { Self { allowed_methods: methods,