From 0ada9803ab1ce6951aead518a3ba83678fa93356 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Mon, 31 Jul 2023 17:00:15 +0200 Subject: [PATCH] ElementR: Add `CryptoApi.findVerificationRequestDMInProgress` (#3601) * Add `CryptoApi.findVerificationRequestDMInProgress` * Fix linting and missing parameters * Move `ROOM_ID` into `test-data` * Remove verification request from `EventDecryptor` pending list * Fix duplicate timeline event processing * Add extra documentation * Try to fix sonar error * Use `roomId` * Fix typo * Review changes * Review changes * Fix `initRustCrypto` jsdoc * Listen to `ClientEvent.Event` instead of `RoomEvent.Timeline` * Fix missing room id in `generate-test-data.py` * Review changes * Review changes * Handle encrypted event * Fix linting * Comments and run timers * Ignore 404 * Fix test --- spec/integ/crypto/crypto.spec.ts | 270 ++++++++++++++---- .../test-data/generate-test-data.py | 2 + spec/test-utils/test-data/index.ts | 1 + spec/test-utils/test-utils.ts | 67 ++++- spec/unit/rust-crypto/rust-crypto.spec.ts | 7 + src/client.ts | 3 + src/crypto-api.ts | 11 + src/crypto/index.ts | 4 +- .../verification/request/InRoomChannel.ts | 4 +- src/rust-crypto/rust-crypto.ts | 95 +++++- 10 files changed, 395 insertions(+), 69 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index fd751e57f7a..8d2b386c413 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -20,22 +20,23 @@ import fetchMock from "fetch-mock-jest"; import "fake-indexeddb/auto"; import { IDBFactory } from "fake-indexeddb"; import { MockResponse, MockResponseFunction } from "fetch-mock"; +import Olm from "@matrix-org/olm"; import type { IDeviceKeys } from "../../../src/@types/crypto"; import * as testUtils from "../../test-utils/test-utils"; -import { CRYPTO_BACKENDS, InitCrypto, syncPromise } from "../../test-utils/test-utils"; +import { CRYPTO_BACKENDS, getSyncResponse, InitCrypto, syncPromise } from "../../test-utils/test-utils"; +import { TEST_ROOM_ID, TEST_ROOM_ID as ROOM_ID, TEST_USER_ID } from "../../test-utils/test-data"; import { TestClient } from "../../TestClient"; import { logger } from "../../../src/logger"; import { + Category, createClient, IClaimOTKsResult, IContent, IDownloadKeyResult, IEvent, - IJoinedRoom, IndexedDBCryptoStore, IStartClientOpts, - ISyncResponse, MatrixClient, MatrixEvent, MatrixEventEvent, @@ -43,6 +44,7 @@ import { Room, RoomMember, RoomStateEvent, + IRoomEvent, } from "../../../src/matrix"; import { DeviceInfo } from "../../../src/crypto/deviceinfo"; import { E2EKeyReceiver, IE2EKeyReceiver } from "../../test-utils/E2EKeyReceiver"; @@ -53,8 +55,7 @@ import { flushPromises } from "../../test-utils/flushPromises"; import { mockInitialApiRequests, mockSetupCrossSigningRequests } from "../../test-utils/mockEndpoints"; import { AddSecretStorageKeyOpts, SECRET_STORAGE_ALGORITHM_V1_AES } from "../../../src/secret-storage"; import { CryptoCallbacks } from "../../../src/crypto-api"; - -const ROOM_ID = "!room:id"; +import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder"; afterEach(() => { // reset fake-indexeddb after each test, to make sure we don't leak connections @@ -156,18 +157,23 @@ function encryptMegolmEvent(opts: { expect(opts.room_id).toBeTruthy(); plaintext.room_id = opts.room_id; } - return encryptMegolmEventRawPlainText({ senderKey: opts.senderKey, groupSession: opts.groupSession, plaintext }); + return encryptMegolmEventRawPlainText({ + senderKey: opts.senderKey, + groupSession: opts.groupSession, + plaintext, + }); } function encryptMegolmEventRawPlainText(opts: { senderKey: string; groupSession: Olm.OutboundGroupSession; plaintext: Partial; + origin_server_ts?: number; }): IEvent { return { event_id: "$test_megolm_event_" + Math.random(), - sender: "@not_the_real_sender:example.com", - origin_server_ts: 1672944778000, + sender: opts.plaintext.sender ?? "@not_the_real_sender:example.com", + origin_server_ts: opts.plaintext.origin_server_ts ?? 1672944778000, content: { algorithm: "m.megolm.v1.aes-sha2", ciphertext: opts.groupSession.encrypt(JSON.stringify(opts.plaintext)), @@ -213,55 +219,6 @@ function encryptGroupSessionKey(opts: { }); } -// get a /sync response which contains a single room (ROOM_ID), with the members given -function getSyncResponse(roomMembers: string[]): ISyncResponse { - const roomResponse: IJoinedRoom = { - summary: { - "m.heroes": [], - "m.joined_member_count": roomMembers.length, - "m.invited_member_count": roomMembers.length, - }, - state: { - events: [ - testUtils.mkEventCustom({ - sender: roomMembers[0], - type: "m.room.encryption", - state_key: "", - content: { - algorithm: "m.megolm.v1.aes-sha2", - }, - }), - ], - }, - timeline: { - events: [], - prev_batch: "", - }, - ephemeral: { events: [] }, - account_data: { events: [] }, - unread_notifications: {}, - }; - - for (let i = 0; i < roomMembers.length; i++) { - roomResponse.state.events.push( - testUtils.mkMembershipCustom({ - membership: "join", - sender: roomMembers[i], - }), - ); - } - - return { - next_batch: "1", - rooms: { - join: { [ROOM_ID]: roomResponse }, - invite: {}, - leave: {}, - }, - account_data: { events: [] }, - }; -} - /** * Establish an Olm Session with the test user * @@ -415,7 +372,10 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, let aliceClient: MatrixClient; /** an object which intercepts `/keys/upload` requests from {@link #aliceClient} to catch the uploaded keys */ - let keyReceiver: IE2EKeyReceiver; + let keyReceiver: E2EKeyReceiver; + + /** an object which intercepts `/keys/query` requests on the test homeserver */ + let keyResponder: E2EKeyResponder; /** an object which intercepts `/sync` requests from {@link #aliceClient} */ let syncResponder: ISyncResponder; @@ -586,6 +546,10 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, afterEach(async () => { await aliceClient.stopClient(); + + // Allow in-flight things to complete before we tear down the test + await jest.runAllTimersAsync(); + fetchMock.mockReset(); }); @@ -708,6 +672,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, await syncPromise(aliceClient); await testUtils.awaitDecryption(event, { waitOnDecryptionFailure: true }); + expect(event.isDecryptionFailure()).toBeFalsy(); expect(event.getContent().body).toEqual("42"); }); @@ -2411,4 +2376,193 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expect(selfSigningKey[secretStorageKey]).toBeDefined(); }); }); + + describe("Incoming verification in a DM", () => { + beforeEach(async () => { + // anything that we don't have a specific matcher for silently returns a 404 + fetchMock.catch(404); + + keyResponder = new E2EKeyResponder(aliceClient.getHomeserverUrl()); + keyResponder.addKeyReceiver(TEST_USER_ID, keyReceiver); + + expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); + await startClientAndAwaitFirstSync(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + /** + * Return a verification request event from Bob + * @see https://spec.matrix.org/v1.7/client-server-api/#mkeyverificationrequest + */ + function createVerificationRequestEvent(): IRoomEvent { + return { + content: { + body: "Verification request from Bob to Alice", + from_device: "BobDevice", + methods: ["m.sas.v1"], + msgtype: "m.key.verification.request", + to: aliceClient.getUserId()!, + }, + event_id: "$143273582443PhrSn:example.org", + origin_server_ts: Date.now(), + room_id: TEST_ROOM_ID, + sender: "@bob:xyz", + type: "m.room.message", + unsigned: { + age: 1234, + }, + }; + } + + /** + * Create a to-device event + * @param groupSession + * @param p2pSession + */ + function createToDeviceEvent(groupSession: Olm.OutboundGroupSession, p2pSession: Olm.Session): Partial { + return encryptGroupSessionKey({ + recipient: aliceClient.getUserId()!, + recipientCurve25519Key: keyReceiver.getDeviceKey(), + recipientEd25519Key: keyReceiver.getSigningKey(), + olmAccount: testOlmAccount, + p2pSession: p2pSession, + groupSession: groupSession, + room_id: ROOM_ID, + }); + } + + /** + * Create and encrypt a verification request event + * @param groupSession + */ + function createEncryptedMessage(groupSession: Olm.OutboundGroupSession): IEvent { + return encryptMegolmEvent({ + senderKey: testSenderKey, + groupSession: groupSession, + room_id: ROOM_ID, + plaintext: createVerificationRequestEvent(), + }); + } + + newBackendOnly("Verification request from Bob to Alice", async () => { + // Tell alice she is sharing a room with bob + const syncResponse = getSyncResponse(["@bob:xyz"]); + + // Add verification request from Bob to Alice in the DM between them + syncResponse.rooms[Category.Join][TEST_ROOM_ID].timeline.events.push(createVerificationRequestEvent()); + syncResponder.sendOrQueueSyncResponse(syncResponse); + // Wait for the sync response to be processed + await syncPromise(aliceClient); + + const request = aliceClient.getCrypto()!.findVerificationRequestDMInProgress(TEST_ROOM_ID, "@bob:xyz"); + // Expect to find the verification request received during the sync + expect(request?.roomId).toBe(TEST_ROOM_ID); + expect(request?.isSelfVerification).toBe(false); + expect(request?.otherUserId).toBe("@bob:xyz"); + }); + + newBackendOnly("Verification request not found", async () => { + // Tell alice she is sharing a room with bob + syncResponder.sendOrQueueSyncResponse(getSyncResponse(["@bob:xyz"])); + // Wait for the sync response to be processed + await syncPromise(aliceClient); + + // Expect to not find any verification request + const request = aliceClient.getCrypto()!.findVerificationRequestDMInProgress(TEST_ROOM_ID, "@bob:xyz"); + expect(request).not.toBeDefined(); + }); + + newBackendOnly("Process encrypted verification request", async () => { + const p2pSession = await createOlmSession(testOlmAccount, keyReceiver); + const groupSession = new Olm.OutboundGroupSession(); + groupSession.create(); + + // make the room_key event, but don't send it yet + const toDeviceEvent = createToDeviceEvent(groupSession, p2pSession); + + // Add verification request from Bob to Alice in the DM between them + syncResponder.sendOrQueueSyncResponse({ + next_batch: 1, + rooms: { join: { [ROOM_ID]: { timeline: { events: [createEncryptedMessage(groupSession)] } } } }, + }); + // Wait for the sync response to be processed + await syncPromise(aliceClient); + + const room = aliceClient.getRoom(ROOM_ID)!; + const matrixEvent = room.getLiveTimeline().getEvents()[0]; + + // wait for a first attempt at decryption: should fail + await testUtils.awaitDecryption(matrixEvent); + expect(matrixEvent.getContent().msgtype).toEqual("m.bad.encrypted"); + + // Send the Bob's keys + syncResponder.sendOrQueueSyncResponse({ + next_batch: 2, + to_device: { + events: [toDeviceEvent], + }, + }); + await syncPromise(aliceClient); + + // Wait for the message to be decrypted + await testUtils.awaitDecryption(matrixEvent, { waitOnDecryptionFailure: true }); + + const request = aliceClient.getCrypto()!.findVerificationRequestDMInProgress(TEST_ROOM_ID, "@bob:xyz"); + // Expect to find the verification request received during the sync + expect(request?.roomId).toBe(TEST_ROOM_ID); + expect(request?.isSelfVerification).toBe(false); + expect(request?.otherUserId).toBe("@bob:xyz"); + }); + + newBackendOnly( + "If Bob keys are not received in the 5mins after the verification request, the request is ignored", + async () => { + const p2pSession = await createOlmSession(testOlmAccount, keyReceiver); + const groupSession = new Olm.OutboundGroupSession(); + groupSession.create(); + + // make the room_key event, but don't send it yet + const toDeviceEvent = createToDeviceEvent(groupSession, p2pSession); + + jest.useFakeTimers(); + + // Add verification request from Bob to Alice in the DM between them + syncResponder.sendOrQueueSyncResponse({ + next_batch: 1, + rooms: { join: { [ROOM_ID]: { timeline: { events: [createEncryptedMessage(groupSession)] } } } }, + }); + // Wait for the sync response to be processed + await syncPromise(aliceClient); + + const room = aliceClient.getRoom(ROOM_ID)!; + const matrixEvent = room.getLiveTimeline().getEvents()[0]; + + // wait for a first attempt at decryption: should fail + await testUtils.awaitDecryption(matrixEvent); + expect(matrixEvent.getContent().msgtype).toEqual("m.bad.encrypted"); + + // Advance time by 5mins, the verification request should be ignored after that + jest.advanceTimersByTime(5 * 60 * 1000); + + // Send the Bob's keys + syncResponder.sendOrQueueSyncResponse({ + next_batch: 2, + to_device: { + events: [toDeviceEvent], + }, + }); + await syncPromise(aliceClient); + + // Wait for the message to be decrypted + await testUtils.awaitDecryption(matrixEvent, { waitOnDecryptionFailure: true }); + + const request = aliceClient.getCrypto()!.findVerificationRequestDMInProgress(TEST_ROOM_ID, "@bob:xyz"); + // the request should not be present + expect(request).not.toBeDefined(); + }, + ); + }); }); diff --git a/spec/test-utils/test-data/generate-test-data.py b/spec/test-utils/test-data/generate-test-data.py index a5cc8bd8fe7..9bc63dbd0ea 100755 --- a/spec/test-utils/test-data/generate-test-data.py +++ b/spec/test-utils/test-data/generate-test-data.py @@ -34,6 +34,7 @@ # input data TEST_USER_ID = "@alice:localhost" TEST_DEVICE_ID = "test_device" +TEST_ROOM_ID = "!room:id" # any 32-byte string can be an ed25519 private key. TEST_DEVICE_PRIVATE_KEY_BYTES = b"deadbeefdeadbeefdeadbeefdeadbeef" @@ -130,6 +131,7 @@ def main() -> None: export const TEST_USER_ID = "{TEST_USER_ID}"; export const TEST_DEVICE_ID = "{TEST_DEVICE_ID}"; +export const TEST_ROOM_ID = "{TEST_ROOM_ID}"; /** The base64-encoded public ed25519 key for this device */ export const TEST_DEVICE_PUBLIC_ED25519_KEY_BASE64 = "{b64_public_key}"; diff --git a/spec/test-utils/test-data/index.ts b/spec/test-utils/test-data/index.ts index 6b904e62d8a..abba2da09ef 100644 --- a/spec/test-utils/test-data/index.ts +++ b/spec/test-utils/test-data/index.ts @@ -11,6 +11,7 @@ import { KeyBackupInfo } from "../../../src/crypto-api"; export const TEST_USER_ID = "@alice:localhost"; export const TEST_DEVICE_ID = "test_device"; +export const TEST_ROOM_ID = "!room:id"; /** The base64-encoded public ed25519 key for this device */ export const TEST_DEVICE_PUBLIC_ED25519_KEY_BASE64 = "YI/7vbGVLpGdYtuceQR8MSsKB/QjgfMXM1xqnn+0NWU"; diff --git a/spec/test-utils/test-utils.ts b/spec/test-utils/test-utils.ts index dab7dfc9b85..9fe48528aeb 100644 --- a/spec/test-utils/test-utils.ts +++ b/spec/test-utils/test-utils.ts @@ -6,9 +6,19 @@ import "../olm-loader"; import { logger } from "../../src/logger"; import { IContent, IEvent, IEventRelation, IUnsigned, MatrixEvent, MatrixEventEvent } from "../../src/models/event"; -import { ClientEvent, EventType, IPusher, MatrixClient, MsgType, RelationType } from "../../src"; +import { + ClientEvent, + EventType, + IJoinedRoom, + IPusher, + ISyncResponse, + MatrixClient, + MsgType, + RelationType, +} from "../../src"; import { SyncState } from "../../src/sync"; import { eventMapperFor } from "../../src/event-mapper"; +import { TEST_ROOM_ID } from "./test-data"; /** * Return a promise that is resolved when the client next emits a @@ -39,6 +49,61 @@ export function syncPromise(client: MatrixClient, count = 1): Promise { }); } +/** + * Return a sync response which contains a single room (by default TEST_ROOM_ID), with the members given + * @param roomMembers + * @param roomId + * + * @returns the sync response + */ +export function getSyncResponse(roomMembers: string[], roomId = TEST_ROOM_ID): ISyncResponse { + const roomResponse: IJoinedRoom = { + summary: { + "m.heroes": [], + "m.joined_member_count": roomMembers.length, + "m.invited_member_count": roomMembers.length, + }, + state: { + events: [ + mkEventCustom({ + sender: roomMembers[0], + type: "m.room.encryption", + state_key: "", + content: { + algorithm: "m.megolm.v1.aes-sha2", + }, + }), + ], + }, + timeline: { + events: [], + prev_batch: "", + }, + ephemeral: { events: [] }, + account_data: { events: [] }, + unread_notifications: {}, + }; + + for (let i = 0; i < roomMembers.length; i++) { + roomResponse.state.events.push( + mkMembershipCustom({ + membership: "join", + sender: roomMembers[i], + }), + ); + } + + return { + next_batch: "1", + rooms: { + join: { [roomId]: roomResponse }, + invite: {}, + leave: {}, + }, + account_data: { events: [] }, + }; +} + /** * Create a spy for an object and automatically spy its methods. * @param constr - The class constructor (used with 'new') diff --git a/spec/unit/rust-crypto/rust-crypto.spec.ts b/spec/unit/rust-crypto/rust-crypto.spec.ts index 608aed32336..c21e34531f2 100644 --- a/spec/unit/rust-crypto/rust-crypto.spec.ts +++ b/spec/unit/rust-crypto/rust-crypto.spec.ts @@ -576,6 +576,13 @@ describe("RustCrypto", () => { expect(await rustCrypto.getActiveSessionBackupVersion()).toBeNull(); }); }); + + describe("findVerificationRequestDMInProgress", () => { + it("returns undefined if the userId is not provided", async () => { + const rustCrypto = await makeTestRustCrypto(); + expect(rustCrypto.findVerificationRequestDMInProgress(testData.TEST_ROOM_ID)).not.toBeDefined(); + }); + }); }); /** build a basic RustCrypto instance for testing diff --git a/src/client.ts b/src/client.ts index f50a1c74058..ea17dfb8395 100644 --- a/src/client.ts +++ b/src/client.ts @@ -2243,6 +2243,9 @@ export class MatrixClient extends TypedEventEmitter { + rustCrypto.onLiveEventFromSync(event); + }); // re-emit the events emitted by the crypto impl this.reEmitter.reEmit(rustCrypto, [ diff --git a/src/crypto-api.ts b/src/crypto-api.ts index 40c45c83ddf..78eb0c9fd05 100644 --- a/src/crypto-api.ts +++ b/src/crypto-api.ts @@ -267,9 +267,20 @@ export interface CryptoApi { * @param roomId - the room to use for verification * * @returns the VerificationRequest that is in progress, if any + * @deprecated prefer `userId` parameter variant. */ findVerificationRequestDMInProgress(roomId: string): VerificationRequest | undefined; + /** + * Finds a DM verification request that is already in progress for the given room and user. + * + * @param roomId - the room to use for verification. + * @param userId - search for a verification request for the given user. + * + * @returns the VerificationRequest that is in progress, if any. + */ + findVerificationRequestDMInProgress(roomId: string, userId?: string): VerificationRequest | undefined; + /** * Send a verification request to our other devices. * diff --git a/src/crypto/index.ts b/src/crypto/index.ts index 1716e03f562..9578496c601 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -2380,8 +2380,8 @@ export class Crypto extends TypedEventEmitter request.roomId?.toString() === roomId); + + if (request) { + return new RustVerificationRequest( + request, + this.outgoingRequestProcessor, + this._supportedVerificationMethods, + ); + } } /** @@ -1018,6 +1034,73 @@ export class RustCrypto extends TypedEventEmitter { + // Ignore state event + if (event.isState()) return; + + const processEvent = async (evt: MatrixEvent): Promise => { + // Process only key validation request + if ( + evt.getType() === EventType.RoomMessage && + evt.getContent().msgtype === MsgType.KeyVerificationRequest + ) { + await this.onKeyVerificationRequest(evt); + } + }; + + // If the event is encrypted of in failure, we wait for decryption + if (event.isDecryptionFailure() || event.isEncrypted()) { + // 5 mins + const TIMEOUT_DELAY = 5 * 60 * 1000; + + const timeoutId = setTimeout(() => event.off(MatrixEventEvent.Decrypted, onDecrypted), TIMEOUT_DELAY); + + const onDecrypted = (decryptedEvent: MatrixEvent, error?: Error): void => { + if (error) return; + + clearTimeout(timeoutId); + event.off(MatrixEventEvent.Decrypted, onDecrypted); + processEvent(decryptedEvent); + }; + // After 5mins, we are not expecting the event to be decrypted + + event.on(MatrixEventEvent.Decrypted, onDecrypted); + } else { + await processEvent(event); + } + } + + /** + * Handle key verification request. + * + * @param event - a key validation request event. + */ + private async onKeyVerificationRequest(event: MatrixEvent): Promise { + const roomId = event.getRoomId(); + + if (!roomId) { + throw new Error("missing roomId in the event"); + } + + await this.olmMachine.receiveVerificationEvent( + JSON.stringify({ + event_id: event.getId(), + type: event.getType(), + sender: event.getSender(), + state_key: event.getStateKey(), + content: event.getContent(), + origin_server_ts: event.getTs(), + }), + new RustSdkCryptoJs.RoomId(roomId), + ); + } + /////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // // Outgoing requests