Merge pull request #1314 from matrix-org/bwindels/catchverifysenderrors

Fix: catch send errors in SAS verifier
matrix-org · Apr 9, 2020 · a686231 · a686231
2 parents d1c9030 + 97cf4bf
commit a686231
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 12 deletions.
diff --git a/spec/unit/crypto/verification/sas.spec.js b/spec/unit/crypto/verification/sas.spec.js
@@ -44,7 +44,16 @@ describe("SAS verification", function() {
     });
 
     it("should error on an unexpected event", async function() {
-        const sas = new SAS({}, "@alice:example.com", "ABCDEFG");
+        //channel, baseApis, userId, deviceId, startEvent, request
+        const request = {
+            onVerifierCancelled: function() {},
+        };
+        const channel = {
+            send: function() {
+                return Promise.resolve();
+            },
+        };
+        const sas = new SAS(channel, {}, "@alice:example.com", "ABCDEFG", null, request);
         sas.handleEvent(new MatrixEvent({
             sender: "@alice:example.com",
             type: "es.inquisition",

diff --git a/src/crypto/verification/Base.js b/src/crypto/verification/Base.js
@@ -287,6 +287,7 @@ export class VerificationBase extends EventEmitter {
         this._endTimer(); // always kill the activity timer
         if (!this._done) {
             this.cancelled = true;
+            this.request.onVerifierCancelled();
             if (this.userId && this.deviceId) {
                 // send a cancellation to the other user (if it wasn't
                 // cancelled by the other user)

diff --git a/src/crypto/verification/SAS.js b/src/crypto/verification/SAS.js
@@ -295,7 +295,7 @@ export class SAS extends Base {
         const hashCommitment = content.commitment;
         const olmSAS = new global.Olm.SAS();
         try {
-            this._send("m.key.verification.key", {
+            await this._send("m.key.verification.key", {
                 key: olmSAS.get_pubkey(),
             });
 
@@ -318,9 +318,13 @@ export class SAS extends Base {
             const verifySAS = new Promise((resolve, reject) => {
                 this.sasEvent = {
                     sas: generateSas(sasBytes, sasMethods),
-                    confirm: () => {
-                        this._sendMAC(olmSAS, macMethod);
-                        resolve();
+                    confirm: async () => {
+                        try {
+                            await this._sendMAC(olmSAS, macMethod);
+                            resolve();
+                        } catch (err) {
+                            reject(err);
+                        }
                     },
                     cancel: () => reject(newUserCancelledError()),
                     mismatch: () => reject(newMismatchedSASError()),
@@ -377,7 +381,7 @@ export class SAS extends Base {
         const olmSAS = new global.Olm.SAS();
         try {
             const commitmentStr = olmSAS.get_pubkey() + anotherjson.stringify(content);
-            this._send("m.key.verification.accept", {
+            await this._send("m.key.verification.accept", {
                 key_agreement_protocol: keyAgreement,
                 hash: hashMethod,
                 message_authentication_code: macMethod,
@@ -391,7 +395,7 @@ export class SAS extends Base {
             // FIXME: make sure event is properly formed
             content = e.getContent();
             olmSAS.set_their_key(content.key);
-            this._send("m.key.verification.key", {
+            await this._send("m.key.verification.key", {
                 key: olmSAS.get_pubkey(),
             });
 
@@ -403,9 +407,13 @@ export class SAS extends Base {
             const verifySAS = new Promise((resolve, reject) => {
                 this.sasEvent = {
                     sas: generateSas(sasBytes, sasMethods),
-                    confirm: () => {
-                        this._sendMAC(olmSAS, macMethod);
-                        resolve();
+                    confirm: async () => {
+                        try {
+                            await this._sendMAC(olmSAS, macMethod);
+                            resolve();
+                        } catch(err) {
+                            reject(err);
+                        }
                     },
                     cancel: () => reject(newUserCancelledError()),
                     mismatch: () => reject(newMismatchedSASError()),
@@ -461,7 +469,7 @@ export class SAS extends Base {
             keyList.sort().join(","),
             baseInfo + "KEY_IDS",
         );
-        this._send("m.key.verification.mac", { mac, keys });
+        return this._send("m.key.verification.mac", { mac, keys });
     }
 
     async _checkMAC(olmSAS, content, method) {

diff --git a/src/crypto/verification/request/VerificationRequest.js b/src/crypto/verification/request/VerificationRequest.js
@@ -73,6 +73,7 @@ export class VerificationRequest extends EventEmitter {
         this._accepting = false;
         this._declining = false;
         this._verifierHasFinished = false;
+        this._cancelled = false;
         this._chosenMethod = null;
         // we keep a copy of the QR Code data (including other user master key) around
         // for QR reciprocate verification, to protect against
@@ -525,7 +526,7 @@ export class VerificationRequest extends EventEmitter {
         }
 
         const cancelEvent = this._getEventByEither(CANCEL_TYPE);
-        if (cancelEvent && phase() !== PHASE_DONE) {
+        if ((this._cancelled || cancelEvent) && phase() !== PHASE_DONE) {
             transitions.push({phase: PHASE_CANCELLED, event: cancelEvent});
             return transitions;
         }
@@ -858,6 +859,15 @@ export class VerificationRequest extends EventEmitter {
         return true;
     }
 
+    onVerifierCancelled() {
+        this._cancelled = true;
+        // move to cancelled phase
+        const newTransitions = this._applyPhaseTransitions();
+        if (newTransitions.length) {
+            this._setPhase(newTransitions[newTransitions.length - 1].phase);
+        }
+    }
+
     onVerifierFinished() {
         this.channel.send("m.key.verification.done", {});
         this._verifierHasFinished = true;