From 01af303d6391531b2aa71498376b52c9e490810c Mon Sep 17 00:00:00 2001
From: Hubert Chathi <hubert@uhoreg.ca>
Date: Thu, 4 Apr 2019 14:08:30 -0400
Subject: [PATCH] fix the selection of the verification methods, and test more
 things

---
 spec/unit/crypto/verification/sas.spec.js | 27 +++++++++++++++++++++++
 src/crypto/verification/SAS.js            | 12 +++++++---
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/spec/unit/crypto/verification/sas.spec.js b/spec/unit/crypto/verification/sas.spec.js
index 902d48083..660cfa666 100644
--- a/spec/unit/crypto/verification/sas.spec.js
+++ b/spec/unit/crypto/verification/sas.spec.js
@@ -160,10 +160,25 @@ describe("SAS verification", function() {
         });
 
         it("should verify a key", async function() {
+            let macMethod;
+            const origSendToDevice = alice.sendToDevice;
+            bob.sendToDevice = function(type, map) {
+                if (type === "m.key.verification.accept") {
+                    macMethod = map[alice.getUserId()][alice.deviceId]
+                        .message_authentication_code;
+                }
+                return origSendToDevice.call(this, type, map);
+            };
+
             await Promise.all([
                 aliceVerifier.verify(),
                 bobPromise.then((verifier) => verifier.verify()),
             ]);
+
+            // make sure that it uses the preferred method
+            expect(macMethod).toBe("hkdf-hmac-sha256");
+
+            // make sure Alice and Bob verified each other
             expect(alice.setDeviceVerified)
                 .toHaveBeenCalledWith(bob.getUserId(), bob.deviceId);
             expect(bob.setDeviceVerified)
@@ -173,6 +188,7 @@ describe("SAS verification", function() {
         it("should be able to verify using the old MAC", async function() {
             // pretend that Alice can only understand the old (incorrect) MAC,
             // and make sure that she can still verify with Bob
+            let macMethod;
             const origSendToDevice = alice.sendToDevice;
             alice.sendToDevice = function(type, map) {
                 if (type === "m.key.verification.start") {
@@ -186,10 +202,21 @@ describe("SAS verification", function() {
                 }
                 return origSendToDevice.call(this, type, map);
             };
+            bob.sendToDevice = function(type, map) {
+                if (type === "m.key.verification.accept") {
+                    macMethod = map[alice.getUserId()][alice.deviceId]
+                        .message_authentication_code;
+                }
+                return origSendToDevice.call(this, type, map);
+            };
+
             await Promise.all([
                 aliceVerifier.verify(),
                 bobPromise.then((verifier) => verifier.verify()),
             ]);
+
+            expect(macMethod).toBe("hmac-sha256");
+
             expect(alice.setDeviceVerified)
                 .toHaveBeenCalledWith(bob.getUserId(), bob.deviceId);
             expect(bob.setDeviceVerified)
diff --git a/src/crypto/verification/SAS.js b/src/crypto/verification/SAS.js
index 8d8858ca6..5889c56be 100644
--- a/src/crypto/verification/SAS.js
+++ b/src/crypto/verification/SAS.js
@@ -281,12 +281,18 @@ export default class SAS extends Base {
 
     async _doRespondVerification() {
         let content = this.startEvent.getContent();
+        // Note: we intersect using our pre-made lists, rather than the sets,
+        // so that the result will be in our order of preference.  Then
+        // fetching the first element from the array will give our preferred
+        // method out of the ones offered by the other party.
         const keyAgreement
-              = intersection(content.key_agreement_protocols, KEY_AGREEMENT_SET)[0];
+              = intersection(
+                  KEY_AGREEMENT_LIST, new Set(content.key_agreement_protocols),
+              )[0];
         const hashMethod
-              = intersection(content.hashes, HASHES_SET)[0];
+              = intersection(HASHES_LIST, new Set(content.hashes))[0];
         const macMethod
-              = intersection(content.message_authentication_codes, MAC_SET)[0];
+              = intersection(MAC_LIST, new Set(content.message_authentication_codes))[0];
         // FIXME: allow app to specify what SAS methods can be used
         const sasMethods
               = intersection(content.short_authentication_string, SAS_SET);