diff --git a/spec/integ/crypto/verification.spec.ts b/spec/integ/crypto/verification.spec.ts index 0aefe75c9..d9cf48024 100644 --- a/spec/integ/crypto/verification.spec.ts +++ b/spec/integ/crypto/verification.spec.ts @@ -46,8 +46,8 @@ import { type Verifier, VerifierEvent, } from "../../../src/crypto-api/verification"; -import { escapeRegExp } from "../../../src/utils"; -import { awaitDecryption, emitPromise, getSyncResponse, syncPromise } from "../../test-utils/test-utils"; +import { escapeRegExp, sleep } from "../../../src/utils"; +import { awaitDecryption, emitPromise, getSyncResponse, syncPromise, waitFor } from "../../test-utils/test-utils"; import { SyncResponder } from "../../test-utils/SyncResponder"; import { BACKUP_DECRYPTION_KEY_BASE64, @@ -79,11 +79,6 @@ import { import { type KeyBackupInfo, CryptoEvent } from "../../../src/crypto-api"; import { encodeBase64 } from "../../../src/base64"; -// The verification flows use javascript timers to set timeouts. We tell jest to use mock timer implementations -// to ensure that we don't end up with dangling timeouts. -// But the wasm bindings of matrix-sdk-crypto rely on a working `queueMicrotask`. -jest.useFakeTimers({ doNotFake: ["queueMicrotask"] }); - beforeAll(async () => { // we use the libolm primitives in the test, so init the Olm library await Olm.init(); @@ -96,6 +91,13 @@ beforeAll(async () => { await RustSdkCryptoJs.initAsync(); }, 10000); +beforeEach(() => { + // The verification flows use javascript timers to set timeouts. We tell jest to use mock timer implementations + // to ensure that we don't end up with dangling timeouts. + // But the wasm bindings of matrix-sdk-crypto rely on a working `queueMicrotask`. + jest.useFakeTimers({ doNotFake: ["queueMicrotask"] }); +}); + afterEach(() => { // reset fake-indexeddb after each test, to make sure we don't leak connections // cf https://github.com/dumbmatter/fakeIndexedDB#wipingresetting-the-indexeddb-for-a-fresh-state @@ -1080,6 +1082,13 @@ describe("verification", () => { }); it("ignores old verification requests", async () => { + const debug = jest.fn(); + const info = jest.fn(); + const warn = jest.fn(); + + // @ts-ignore overriding RustCrypto's logger + aliceClient.getCrypto()!.logger = { debug, info, warn }; + const eventHandler = jest.fn(); aliceClient.on(CryptoEvent.VerificationRequestReceived, eventHandler); @@ -1094,6 +1103,16 @@ describe("verification", () => { const matrixEvent = room.getLiveTimeline().getEvents()[0]; expect(matrixEvent.getId()).toEqual(verificationRequestEvent.event_id); + // Wait until the request has been processed. We use a real sleep() + // here to make sure any background async tasks are completed. + jest.useRealTimers(); + await waitFor(async () => { + expect(info).toHaveBeenCalledWith( + expect.stringMatching(/^Ignoring just-received verification request/), + ); + sleep(100); + }); + // check that an event has not been raised, and that the request is not found expect(eventHandler).not.toHaveBeenCalled(); expect( diff --git a/spec/unit/rust-crypto/rust-crypto.spec.ts b/spec/unit/rust-crypto/rust-crypto.spec.ts index 952370d46..e9c77ef4e 100644 --- a/spec/unit/rust-crypto/rust-crypto.spec.ts +++ b/spec/unit/rust-crypto/rust-crypto.spec.ts @@ -2419,6 +2419,135 @@ describe("RustCrypto", () => { expect(mockOlmMachine.receiveRoomKeyBundle.mock.calls[0][1]).toEqual(new TextEncoder().encode("asdfghjkl")); }); }); + + describe("Verification requests", () => { + it("fetches device details before room verification requests", async () => { + // Given a RustCrypto + const olmMachine = mockedOlmMachine(); + const outgoingRequestProcessor = mockedOutgoingRequestProcessor(); + const rustCrypto = makeRustCrypto(olmMachine, outgoingRequestProcessor); + + // When we receive a room verification request + const event = mockedEvent("!r:s.co", "@u:s.co", "m.room.message", "m.key.verification.request"); + await rustCrypto.onLiveEventFromSync(event); + + // Then we first fetch device details + expect(outgoingRequestProcessor.makeOutgoingRequest).toHaveBeenCalled(); + + // And we handle the verification event as normal + expect(olmMachine.receiveVerificationEvent).toHaveBeenCalled(); + }); + + it("does not fetch device details before other verification events", async () => { + // Given a RustCrypto + const olmMachine = mockedOlmMachine(); + const outgoingRequestProcessor = mockedOutgoingRequestProcessor(); + const rustCrypto = makeRustCrypto(olmMachine, outgoingRequestProcessor); + + // When we receive some verification event that is not a room request + const event = mockedEvent("!r:s.co", "@u:s.co", "m.key.verification.start"); + await rustCrypto.onLiveEventFromSync(event); + + // Then we do not fetch device details + expect(outgoingRequestProcessor.makeOutgoingRequest).not.toHaveBeenCalled(); + + // And we handle the verification event as normal + expect(olmMachine.receiveVerificationEvent).toHaveBeenCalled(); + }); + + it("throws an error if sender is missing", async () => { + // Given a RustCrypto + const olmMachine = mockedOlmMachine(); + const outgoingRequestProcessor = mockedOutgoingRequestProcessor(); + const rustCrypto = makeRustCrypto(olmMachine, outgoingRequestProcessor); + + // When we receive a verification event without a sender + // Then we throw + const event = mockedEvent("!r:s.co", null, "m.key.verification.start"); + + await expect(async () => await rustCrypto.onLiveEventFromSync(event)).rejects.toThrow( + "missing sender in the event", + ); + + // And we do not fetch device details or handle the event + expect(outgoingRequestProcessor.makeOutgoingRequest).not.toHaveBeenCalled(); + expect(olmMachine.receiveVerificationEvent).not.toHaveBeenCalled(); + }); + + it("throws an error if room is missing", async () => { + // Given a RustCrypto + const olmMachine = mockedOlmMachine(); + const outgoingRequestProcessor = mockedOutgoingRequestProcessor(); + const rustCrypto = makeRustCrypto(olmMachine, outgoingRequestProcessor); + + // When we receive a verification event without a sender + // Then we throw + const event = mockedEvent(null, "@u:s.co", "m.key.verification.start"); + + await expect(async () => await rustCrypto.onLiveEventFromSync(event)).rejects.toThrow( + "missing roomId in the event", + ); + + // And we do not fetch device details or handle the event + expect(outgoingRequestProcessor.makeOutgoingRequest).not.toHaveBeenCalled(); + expect(olmMachine.receiveVerificationEvent).not.toHaveBeenCalled(); + }); + + function mockedOlmMachine(): Mocked { + return { + queryKeysForUsers: jest.fn(), + getVerificationRequest: jest.fn(), + receiveVerificationEvent: jest.fn(), + } as unknown as Mocked; + } + + function makeRustCrypto( + olmMachine: OlmMachine, + outgoingRequestProcessor: OutgoingRequestProcessor, + ): RustCrypto { + const rustCrypto = new RustCrypto( + new DebugLogger(debug("test Verification requests")), + olmMachine, + {} as unknown as MatrixHttpApi, + TEST_USER, + TEST_DEVICE_ID, + {} as ServerSideSecretStorage, + {} as CryptoCallbacks, + ); + + // @ts-ignore mocking outgoingRequestProcessor + rustCrypto.outgoingRequestProcessor = outgoingRequestProcessor; + + return rustCrypto; + } + + function mockedOutgoingRequestProcessor(): OutgoingRequestProcessor { + return { + makeOutgoingRequest: jest.fn(), + } as unknown as Mocked; + } + + function mockedEvent( + roomId: string | null, + senderId: string | null, + eventType: string, + msgtype?: string | undefined, + ): MatrixEvent { + return { + isState: jest.fn().mockReturnValue(false), + getUnsigned: jest.fn().mockReturnValue({}), + isDecryptionFailure: jest.fn(), + isEncrypted: jest.fn(), + getType: jest.fn().mockReturnValue(eventType), + getRoomId: jest.fn().mockReturnValue(roomId), + getSender: jest.fn().mockReturnValue(senderId), + getId: jest.fn(), + getStateKey: jest.fn(), + getContent: jest.fn().mockReturnValue({ msgtype: msgtype }), + getTs: jest.fn(), + } as unknown as MatrixEvent; + } + }); }); /** Build a MatrixHttpApi instance */ diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index 8e8ccb67e..638bf37d1 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -2053,20 +2053,38 @@ export class RustCrypto extends TypedEventEmitter { const roomId = event.getRoomId(); + const senderId = event.getSender(); if (!roomId) { throw new Error("missing roomId in the event"); } + if (!senderId) { + throw new Error("missing sender in the event"); + } + this.logger.debug( `Incoming verification event ${event.getId()} type ${event.getType()} from ${event.getSender()}`, ); - await this.olmMachine.receiveVerificationEvent( + const isRoomVerificationRequest = + event.getType() === EventType.RoomMessage && event.getContent().msgtype === MsgType.KeyVerificationRequest; + + if (isRoomVerificationRequest) { + // Before processing an in-room verification request, we need to + // make sure we have the sender's device information - otherwise we + // will immediately abort verification. So we explicitly fetch it + // from /keys/query and wait for that request to complete before we + // call receiveVerificationEvent. + const req = this.getOlmMachineOrThrow().queryKeysForUsers([new RustSdkCryptoJs.UserId(senderId)]); + await this.outgoingRequestProcessor.makeOutgoingRequest(req); + } + + await this.getOlmMachineOrThrow().receiveVerificationEvent( JSON.stringify({ event_id: event.getId(), type: event.getType(), - sender: event.getSender(), + sender: senderId, state_key: event.getStateKey(), content: event.getContent(), origin_server_ts: event.getTs(), @@ -2074,11 +2092,8 @@ export class RustCrypto extends TypedEventEmitter