From cf30c42563ccf3d6ed4ddd3cb3a95243c002b7e5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Damir=20Jeli=C4=87?= <poljar@termina.org.uk>
Date: Mon, 19 Jul 2021 09:43:35 +0200
Subject: [PATCH] crypto: Correctly remember our chosen SAS methods

---
 .../src/verification/sas/inner_sas.rs         |  7 +++--
 matrix_sdk_crypto/src/verification/sas/mod.rs | 27 +++----------------
 .../src/verification/sas/sas_state.rs         | 15 ++++++++---
 3 files changed, 20 insertions(+), 29 deletions(-)

diff --git a/matrix_sdk_crypto/src/verification/sas/inner_sas.rs b/matrix_sdk_crypto/src/verification/sas/inner_sas.rs
index 04267442..161181b6 100644
--- a/matrix_sdk_crypto/src/verification/sas/inner_sas.rs
+++ b/matrix_sdk_crypto/src/verification/sas/inner_sas.rs
@@ -165,9 +165,12 @@ impl InnerSas {
         }
     }
 
-    pub fn accept(self) -> Option<(InnerSas, OwnedAcceptContent)> {
+    pub fn accept(
+        self,
+        methods: Vec<ShortAuthenticationString>,
+    ) -> Option<(InnerSas, OwnedAcceptContent)> {
         if let InnerSas::Started(s) = self {
-            let sas = s.into_accepted();
+            let sas = s.into_accepted(methods);
             let content = sas.as_content();
             Some((InnerSas::WeAccepted(sas), content))
         } else {
diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs
index fd1e5e24..56159d94 100644
--- a/matrix_sdk_crypto/src/verification/sas/mod.rs
+++ b/matrix_sdk_crypto/src/verification/sas/mod.rs
@@ -25,11 +25,7 @@ use matrix_sdk_common::uuid::Uuid;
 use ruma::{
     api::client::r0::keys::upload_signatures::Request as SignatureUploadRequest,
     events::{
-        key::verification::{
-            accept::{AcceptEventContent, AcceptMethod, AcceptToDeviceEventContent},
-            cancel::CancelCode,
-            ShortAuthenticationString,
-        },
+        key::verification::{cancel::CancelCode, ShortAuthenticationString},
         AnyMessageEventContent, AnyToDeviceEventContent,
     },
     DeviceId, EventId, RoomId, UserId,
@@ -327,10 +323,10 @@ impl Sas {
     ) -> Option<OutgoingVerificationRequest> {
         let mut guard = self.inner.lock().unwrap();
         let sas: InnerSas = (*guard).clone();
+        let methods = settings.allowed_methods;
 
-        if let Some((sas, content)) = sas.accept() {
+        if let Some((sas, content)) = sas.accept(methods) {
             *guard = sas;
-            let content = settings.apply(content);
 
             Some(match content {
                 OwnedAcceptContent::ToDevice(c) => {
@@ -554,23 +550,6 @@ impl AcceptSettings {
     pub fn with_allowed_methods(methods: Vec<ShortAuthenticationString>) -> Self {
         Self { allowed_methods: methods }
     }
-
-    fn apply(self, mut content: OwnedAcceptContent) -> OwnedAcceptContent {
-        match &mut content {
-            OwnedAcceptContent::ToDevice(AcceptToDeviceEventContent {
-                method: AcceptMethod::SasV1(c),
-                ..
-            })
-            | OwnedAcceptContent::Room(
-                _,
-                AcceptEventContent { method: AcceptMethod::SasV1(c), .. },
-            ) => {
-                c.short_authentication_string.retain(|sas| self.allowed_methods.contains(sas));
-                content
-            }
-            _ => content,
-        }
-    }
 }
 
 #[cfg(test)]
diff --git a/matrix_sdk_crypto/src/verification/sas/sas_state.rs b/matrix_sdk_crypto/src/verification/sas/sas_state.rs
index 06b59a7e..7aa9bc2f 100644
--- a/matrix_sdk_crypto/src/verification/sas/sas_state.rs
+++ b/matrix_sdk_crypto/src/verification/sas/sas_state.rs
@@ -557,7 +557,15 @@ impl SasState<Started> {
         }
     }
 
-    pub fn into_accepted(self) -> SasState<WeAccepted> {
+    pub fn into_accepted(self, methods: Vec<ShortAuthenticationString>) -> SasState<WeAccepted> {
+        let mut accepted_protocols = self.state.accepted_protocols.as_ref().to_owned();
+        accepted_protocols.short_auth_string = methods;
+
+        // Decimal is required per spec.
+        if !accepted_protocols.short_auth_string.contains(&ShortAuthenticationString::Decimal) {
+            accepted_protocols.short_auth_string.push(ShortAuthenticationString::Decimal);
+        }
+
         SasState {
             inner: self.inner,
             ids: self.ids,
@@ -567,7 +575,7 @@ impl SasState<Started> {
             started_from_request: self.started_from_request,
             state: Arc::new(WeAccepted {
                 we_started: false,
-                accepted_protocols: self.state.accepted_protocols.clone(),
+                accepted_protocols: accepted_protocols.into(),
                 commitment: self.state.commitment.clone(),
             }),
         }
@@ -1115,6 +1123,7 @@ mod test {
         events::key::verification::{
             accept::{AcceptMethod, AcceptToDeviceEventContent},
             start::{StartMethod, StartToDeviceEventContent},
+            ShortAuthenticationString,
         },
         DeviceId, UserId,
     };
@@ -1162,7 +1171,7 @@ mod test {
             &start_content.as_start_content(),
             false,
         );
-        let bob_sas = bob_sas.unwrap().into_accepted();
+        let bob_sas = bob_sas.unwrap().into_accepted(vec![ShortAuthenticationString::Emoji]);
 
         (alice_sas, bob_sas)
     }