index : matrix-js-sdk

My fork of matrix-js-sdk

diff options
context:
space:
mode:
authorMichael Telatynski <[email protected]>2020-06-03 10:41:15 +0100
committerGitHub <[email protected]>2020-06-03 10:41:15 +0100
commita987a31667ae6aa14389c4e69eb5aafdde645822 (patch)
tree7746e3b5c6137bc595d7aab5b0ccb753d54eb42f
parent29f10bcd447783057e4da7523f41daa82bc1896b (diff)
parentb83aa546617b6428dc1c4f66857e6b367903fd93 (diff)
downloadmatrix-js-sdk-a987a31667ae6aa14389c4e69eb5aafdde645822.tar.gz
Merge pull request #1388 from matrix-org/dbkr/timeouts
Fix verification request timeouts to match spec
-rw-r--r--spec/unit/crypto/verification/verification_request.spec.js36
-rw-r--r--src/crypto/verification/request/VerificationRequest.js61
2 files changed, 78 insertions, 19 deletions
diff --git a/spec/unit/crypto/verification/verification_request.spec.js b/spec/unit/crypto/verification/verification_request.spec.js
index ac9b0f8d..a90d0482 100644
--- a/spec/unit/crypto/verification/verification_request.spec.js
+++ b/spec/unit/crypto/verification/verification_request.spec.js
@@ -119,6 +119,8 @@ async function distributeEvent(ownRequest, theirRequest, event) {
await theirRequest.channel.handleEvent(event, theirRequest, true);
}
+jest.useFakeTimers();
+
describe("verification request unit tests", function() {
beforeAll(function() {
setupWebcrypto();
@@ -246,4 +248,38 @@ describe("verification request unit tests", function() {
expect(bob1Request.done).toBe(true);
expect(bob2Request.done).toBe(true);
});
+
+ it("request times out after 10 minutes", async function() {
+ const alice = makeMockClient("@alice:matrix.tld", "device1");
+ const bob = makeMockClient("@bob:matrix.tld", "device1");
+ const aliceRequest = new VerificationRequest(
+ new InRoomChannel(alice, "!room", bob.getUserId()), new Map(), alice);
+ await aliceRequest.sendRequest();
+ const [requestEvent] = alice.popEvents();
+ await aliceRequest.channel.handleEvent(requestEvent, aliceRequest, true,
+ true, true);
+
+ expect(aliceRequest.cancelled).toBe(false);
+ expect(aliceRequest._cancellingUserId).toBe(undefined);
+ jest.advanceTimersByTime(10 * 60 * 1000);
+ expect(aliceRequest._cancellingUserId).toBe(alice.getUserId());
+ });
+
+ it("request times out 2 minutes after receipt", async function() {
+ const alice = makeMockClient("@alice:matrix.tld", "device1");
+ const bob = makeMockClient("@bob:matrix.tld", "device1");
+ const aliceRequest = new VerificationRequest(
+ new InRoomChannel(alice, "!room", bob.getUserId()), new Map(), alice);
+ await aliceRequest.sendRequest();
+ const [requestEvent] = alice.popEvents();
+ const bobRequest = new VerificationRequest(
+ new InRoomChannel(bob, "!room"), new Map(), bob);
+
+ await bobRequest.channel.handleEvent(requestEvent, bobRequest, true);
+
+ expect(bobRequest.cancelled).toBe(false);
+ expect(bobRequest._cancellingUserId).toBe(undefined);
+ jest.advanceTimersByTime(2 * 60 * 1000);
+ expect(bobRequest._cancellingUserId).toBe(bob.getUserId());
+ });
});
diff --git a/src/crypto/verification/request/VerificationRequest.js b/src/crypto/verification/request/VerificationRequest.js
index a0d0998d..a77b8c96 100644
--- a/src/crypto/verification/request/VerificationRequest.js
+++ b/src/crypto/verification/request/VerificationRequest.js
@@ -25,15 +25,17 @@ import {
} from "../Error";
import {QRCodeData, SCAN_QR_CODE_METHOD} from "../QRCode";
-// the recommended amount of time before a verification request
-// should be (automatically) cancelled without user interaction
-// and ignored.
-const VERIFICATION_REQUEST_TIMEOUT = 10 * 60 * 1000; //10m
+// How long after the event's timestamp that the request times out
+const TIMEOUT_FROM_EVENT_TS = 10 * 60 * 1000; // 10 minutes
+
+// How long after we receive the event that the request times out
+const TIMEOUT_FROM_EVENT_RECEIPT = 2 * 60 * 1000; // 2 minutes
+
// to avoid almost expired verification notifications
// from showing a notification and almost immediately
// disappearing, also ignore verification requests that
// are this amount of time away from expiring.
-const VERIFICATION_REQUEST_MARGIN = 3 * 1000; //3s
+const VERIFICATION_REQUEST_MARGIN = 3 * 1000; // 3 seconds
export const EVENT_PREFIX = "m.key.verification.";
@@ -80,6 +82,9 @@ export class VerificationRequest extends EventEmitter {
// cross-signing identity reset between the .ready and .start event
// and signing the wrong key after .start
this._qrCodeData = null;
+
+ // The timestamp when we received the request event from the other side
+ this._requestReceivedAt = null;
}
/**
@@ -165,12 +170,26 @@ export class VerificationRequest extends EventEmitter {
return this._chosenMethod;
}
+ calculateEventTimeout(event) {
+ let effectiveExpiresAt = this.channel.getTimestamp(event)
+ + TIMEOUT_FROM_EVENT_TS;
+
+ if (this._requestReceivedAt && !this.initiatedByMe &&
+ this.phase <= PHASE_REQUESTED
+ ) {
+ const expiresAtByReceipt = this._requestReceivedAt
+ + TIMEOUT_FROM_EVENT_RECEIPT;
+ effectiveExpiresAt = Math.min(effectiveExpiresAt, expiresAtByReceipt);
+ }
+
+ return Math.max(0, effectiveExpiresAt - Date.now());
+ }
+
/** The current remaining amount of ms before the request should be automatically cancelled */
get timeout() {
const requestEvent = this._getEventByEither(REQUEST_TYPE);
if (requestEvent) {
- const elapsed = Date.now() - this.channel.getTimestamp(requestEvent);
- return Math.max(0, VERIFICATION_REQUEST_TIMEOUT - elapsed);
+ return this.calculateEventTimeout(requestEvent);
}
return 0;
}
@@ -735,7 +754,7 @@ export class VerificationRequest extends EventEmitter {
_setupTimeout(phase) {
const shouldTimeout = !this._timeoutTimer && !this.observeOnly &&
- phase === PHASE_REQUESTED && this.initiatedByMe;
+ phase === PHASE_REQUESTED;
if (shouldTimeout) {
this._timeoutTimer = setTimeout(this._cancelOnTimeout, this.timeout);
@@ -754,7 +773,17 @@ export class VerificationRequest extends EventEmitter {
_cancelOnTimeout = () => {
try {
- this.cancel({reason: "Other party didn't accept in time", code: "m.timeout"});
+ if (this.initiatedByMe) {
+ this.cancel({
+ reason: "Other party didn't accept in time",
+ code: "m.timeout",
+ });
+ } else {
+ this.cancel({
+ reason: "User didn't accept in time",
+ code: "m.timeout",
+ });
+ }
} catch (err) {
logger.error("Error while cancelling verification request", err);
}
@@ -791,16 +820,8 @@ export class VerificationRequest extends EventEmitter {
if (!isLiveEvent) {
this._observeOnly = true;
}
- // a timestamp is not provided on all to_device events
- const timestamp = this.channel.getTimestamp(event);
- if (Number.isFinite(timestamp)) {
- const elapsed = Date.now() - timestamp;
- // don't allow interaction on old requests
- if (elapsed > (VERIFICATION_REQUEST_TIMEOUT - VERIFICATION_REQUEST_MARGIN) ||
- elapsed < -(VERIFICATION_REQUEST_TIMEOUT / 2)
- ) {
- this._observeOnly = true;
- }
+ if (this.calculateEventTimeout(event) < VERIFICATION_REQUEST_MARGIN) {
+ this._observeOnly = true;
}
}
@@ -819,6 +840,8 @@ export class VerificationRequest extends EventEmitter {
this._eventsByThem.delete(type);
}
}
+ // also remember when we received the request event
+ this._requestReceivedAt = Date.now();
}
}