From 30f66b3add7eb32a7fdd51c4ceb218caa389b91f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 28 Jun 2023 14:43:39 +0100 Subject: [PATCH 1/4] Pass `supportedVerificationMethods` into `VerificationRequest` ... so that the application can later call `accept()` and we know what to send. --- spec/unit/rust-crypto/verification.spec.ts | 2 +- src/rust-crypto/rust-crypto.ts | 15 +++++++++++---- src/rust-crypto/verification.ts | 8 ++++++++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/spec/unit/rust-crypto/verification.spec.ts b/spec/unit/rust-crypto/verification.spec.ts index cb92fd5778e..b76a672d216 100644 --- a/spec/unit/rust-crypto/verification.spec.ts +++ b/spec/unit/rust-crypto/verification.spec.ts @@ -32,7 +32,7 @@ describe("VerificationRequest", () => { startSas: jest.fn(), } as unknown as Mocked; mockedOutgoingRequestProcessor = {} as Mocked; - request = new RustVerificationRequest(mockedInner, mockedOutgoingRequestProcessor); + request = new RustVerificationRequest(mockedInner, mockedOutgoingRequestProcessor, undefined); }); it("does not permit methods other than SAS", async () => { diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index d5592ec418f..0146512ed85 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -553,7 +553,14 @@ export class RustCrypto extends TypedEventEmitter request.roomId === undefined) - .map((request) => new RustVerificationRequest(request, this.outgoingRequestProcessor)); + .map( + (request) => + new RustVerificationRequest( + request, + this.outgoingRequestProcessor, + this.supportedVerificationMethods, + ), + ); } /** @@ -600,7 +607,7 @@ export class RustCrypto extends TypedEventEmitter Date: Wed, 28 Jun 2023 14:38:12 +0100 Subject: [PATCH 2/4] Implement `VerificationRequest.accept` --- spec/integ/crypto/verification.spec.ts | 8 ++++--- src/rust-crypto/verification.ts | 33 ++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/spec/integ/crypto/verification.spec.ts b/spec/integ/crypto/verification.spec.ts index e9b9edf2991..be47ef6fb16 100644 --- a/spec/integ/crypto/verification.spec.ts +++ b/spec/integ/crypto/verification.spec.ts @@ -541,12 +541,14 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st }); describe("Incoming verification from another device", () => { - beforeEach(() => { + beforeEach(async () => { e2eKeyResponder.addDeviceKeys(TEST_USER_ID, TEST_DEVICE_ID, SIGNED_TEST_DEVICE_DATA); - }); - oldBackendOnly("Incoming verification: can accept", async () => { aliceClient = await startTestClient(); + await waitForDeviceList(); + }); + + it("Incoming verification: can accept", async () => { const TRANSACTION_ID = "abcd"; // Initiate the request by sending a to-device message diff --git a/src/rust-crypto/verification.ts b/src/rust-crypto/verification.ts index a4ec876fd0a..67512db9b56 100644 --- a/src/rust-crypto/verification.ts +++ b/src/rust-crypto/verification.ts @@ -38,6 +38,9 @@ export class RustVerificationRequest extends TypedEventEmitter implements VerificationRequest { + /** Are we in the process of sending an `m.key.verification.ready` event? */ + private _accepting = false; + private _verifier: Verifier | undefined; /** @@ -121,7 +124,9 @@ export class RustVerificationRequest case RustSdkCryptoJs.VerificationRequestPhase.Requested: return VerificationPhase.Requested; case RustSdkCryptoJs.VerificationRequestPhase.Ready: - return VerificationPhase.Ready; + // if we're still sending the `m.key.verification.ready`, that counts as "Requested" in the js-sdk's + // parlance. + return this._accepting ? VerificationPhase.Requested : VerificationPhase.Ready; case RustSdkCryptoJs.VerificationRequestPhase.Transitioned: return VerificationPhase.Started; case RustSdkCryptoJs.VerificationRequestPhase.Done: @@ -145,7 +150,7 @@ export class RustVerificationRequest * the remote echo which causes a transition to {@link VerificationPhase.Ready}. */ public get accepting(): boolean { - throw new Error("not implemented"); + return this._accepting; } /** @@ -206,8 +211,28 @@ export class RustVerificationRequest * * @returns Promise which resolves when the event has been sent. */ - public accept(): Promise { - throw new Error("not implemented"); + public async accept(): Promise { + if (this.inner.phase() != RustSdkCryptoJs.VerificationRequestPhase.Requested || this._accepting) { + throw new Error(`Cannot accept a verification request in phase ${this.phase}`); + } + + this._accepting = true; + try { + const req: undefined | OutgoingRequest = + this.supportedVerificationMethods === undefined + ? this.inner.accept() + : this.inner.acceptWithMethods( + this.supportedVerificationMethods.map(verificationMethodIdentifierToMethod), + ); + if (req) { + await this.outgoingRequestProcessor.makeOutgoingRequest(req); + } + } finally { + this._accepting = false; + } + + // phase may have changed, so emit a 'change' event + this.emit(VerificationRequestEvent.Change); } /** From 1b8c1caaa81762dd9e60eb6118b02cc24c625c90 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 28 Jun 2023 14:41:11 +0100 Subject: [PATCH 3/4] Implement `VerificationRequest.declining` --- spec/integ/crypto/verification.spec.ts | 48 +++++++++++++++++++++----- src/rust-crypto/verification.ts | 21 ++++++++--- yarn.lock | 6 ++-- 3 files changed, 59 insertions(+), 16 deletions(-) diff --git a/spec/integ/crypto/verification.spec.ts b/spec/integ/crypto/verification.spec.ts index be47ef6fb16..00247ec33a7 100644 --- a/spec/integ/crypto/verification.spec.ts +++ b/spec/integ/crypto/verification.spec.ts @@ -552,15 +552,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st const TRANSACTION_ID = "abcd"; // Initiate the request by sending a to-device message - returnToDeviceMessageFromSync({ - type: "m.key.verification.request", - content: { - from_device: TEST_DEVICE_ID, - methods: ["m.sas.v1"], - transaction_id: TRANSACTION_ID, - timestamp: Date.now() - 1000, - }, - }); + returnToDeviceMessageFromSync(buildRequestMessage(TRANSACTION_ID)); const request: VerificationRequest = await emitPromise( aliceClient, CryptoEvent.VerificationRequestReceived, @@ -587,6 +579,31 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st expect(toDeviceMessage.from_device).toEqual(aliceClient.deviceId); expect(toDeviceMessage.transaction_id).toEqual(TRANSACTION_ID); }); + + it("Incoming verification: can refuse", async () => { + const TRANSACTION_ID = "abcd"; + + // Initiate the request by sending a to-device message + returnToDeviceMessageFromSync(buildRequestMessage(TRANSACTION_ID)); + const request: VerificationRequest = await emitPromise( + aliceClient, + CryptoEvent.VerificationRequestReceived, + ); + expect(request.transactionId).toEqual(TRANSACTION_ID); + + // Alice declines, by sending a cancellation + const sendToDevicePromise = expectSendToDeviceMessage("m.key.verification.cancel"); + const cancelPromise = request.cancel(); + expect(canAcceptVerificationRequest(request)).toBe(false); + expect(request.accepting).toBe(false); + expect(request.declining).toBe(true); + await cancelPromise; + const requestBody = await sendToDevicePromise; + expect(request.phase).toEqual(VerificationPhase.Cancelled); + + const toDeviceMessage = requestBody.messages[TEST_USER_ID][TEST_DEVICE_ID]; + expect(toDeviceMessage.transaction_id).toEqual(TRANSACTION_ID); + }); }); async function startTestClient(opts: Partial = {}): Promise { @@ -670,6 +687,19 @@ function encodeUnpaddedBase64(uint8Array: ArrayBuffer | Uint8Array): string { return Buffer.from(uint8Array).toString("base64").replace(/=+$/g, ""); } +/** build an m.key.verification.request to-device message originating from the dummy device */ +function buildRequestMessage(transactionId: string): { type: string; content: object } { + return { + type: "m.key.verification.request", + content: { + from_device: TEST_DEVICE_ID, + methods: ["m.sas.v1"], + transaction_id: transactionId, + timestamp: Date.now() - 1000, + }, + }; +} + /** build an m.key.verification.ready to-device message originating from the dummy device */ function buildReadyMessage(transactionId: string, methods: string[]): { type: string; content: object } { return { diff --git a/src/rust-crypto/verification.ts b/src/rust-crypto/verification.ts index 67512db9b56..a94539507b6 100644 --- a/src/rust-crypto/verification.ts +++ b/src/rust-crypto/verification.ts @@ -41,6 +41,9 @@ export class RustVerificationRequest /** Are we in the process of sending an `m.key.verification.ready` event? */ private _accepting = false; + /** Are we in the process of sending an `m.key.verification.cancellation` event? */ + private _cancelling = false; + private _verifier: Verifier | undefined; /** @@ -158,7 +161,7 @@ export class RustVerificationRequest * the remote echo which causes a transition to {@link VerificationPhase.Cancelled}). */ public get declining(): boolean { - throw new Error("not implemented"); + return this._cancelling; } /** @@ -244,9 +247,19 @@ export class RustVerificationRequest * @returns Promise which resolves when the event has been sent. */ public async cancel(params?: { reason?: string; code?: string }): Promise { - const req: undefined | OutgoingRequest = this.inner.cancel(); - if (req) { - await this.outgoingRequestProcessor.makeOutgoingRequest(req); + if (this._cancelling) { + // already cancelling; do nothing + return; + } + + this._cancelling = true; + try { + const req: undefined | OutgoingRequest = this.inner.cancel(); + if (req) { + await this.outgoingRequestProcessor.makeOutgoingRequest(req); + } + } finally { + this._cancelling = false; } } diff --git a/yarn.lock b/yarn.lock index 43ad61ac39a..d6a608cfdf1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1427,9 +1427,9 @@ lodash "^4.17.21" "@matrix-org/matrix-sdk-crypto-js@^0.1.0-alpha.11": - version "0.1.0-alpha.11" - resolved "https://registry.yarnpkg.com/@matrix-org/matrix-sdk-crypto-js/-/matrix-sdk-crypto-js-0.1.0-alpha.11.tgz#24d705318c3159ef7dbe43bca464ac2bdd11e45d" - integrity sha512-HD3rskPkqrUUSaKzGLg97k/bN+OZrkcX7ODB/pNBs/jqq+/A0wDKqsszJotzFwsQcDPpWn78BmMyvBo4tLxKjw== + version "0.1.0" + resolved "https://registry.yarnpkg.com/@matrix-org/matrix-sdk-crypto-js/-/matrix-sdk-crypto-js-0.1.0.tgz#766580036d4df12120ded223e13b5640e77db136" + integrity sha512-ra/bcFdleC1iRNms2I96UXA0NvQYWpMsHrV5EfJRS7qV1PtnQNvgsvMfjMbkx8QT2ErEmIhsvB5fPCpfp8BSuw== "@matrix-org/olm@https://gitlab.matrix.org/api/v4/projects/27/packages/npm/@matrix-org/olm/-/@matrix-org/olm-3.2.14.tgz": version "3.2.14" From 1b3846f55b4fc33ce14e2a6b6bccc686af454689 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 30 Jun 2023 13:30:53 +0100 Subject: [PATCH 4/4] Update src/rust-crypto/verification.ts --- src/rust-crypto/verification.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rust-crypto/verification.ts b/src/rust-crypto/verification.ts index a94539507b6..5382a64636a 100644 --- a/src/rust-crypto/verification.ts +++ b/src/rust-crypto/verification.ts @@ -215,7 +215,7 @@ export class RustVerificationRequest * @returns Promise which resolves when the event has been sent. */ public async accept(): Promise { - if (this.inner.phase() != RustSdkCryptoJs.VerificationRequestPhase.Requested || this._accepting) { + if (this.inner.phase() !== RustSdkCryptoJs.VerificationRequestPhase.Requested || this._accepting) { throw new Error(`Cannot accept a verification request in phase ${this.phase}`); }