From e8258ce6ce08c0f67be4df7b09f4d1fa2734e4fc Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Tue, 17 Oct 2023 17:20:50 +0200 Subject: [PATCH 1/6] Wire up history visibility in `RoomEncryptor.ts` --- spec/integ/crypto/crypto.spec.ts | 81 +++++++++++++++++++++++++++++++- src/rust-crypto/RoomEncryptor.ts | 31 +++++++++++- 2 files changed, 108 insertions(+), 4 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 55058bf0789..fd54093fdf2 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -23,7 +23,7 @@ import { MockResponse, MockResponseFunction } from "fetch-mock"; import Olm from "@matrix-org/olm"; import * as testUtils from "../../test-utils/test-utils"; -import { CRYPTO_BACKENDS, getSyncResponse, InitCrypto, syncPromise } from "../../test-utils/test-utils"; +import { CRYPTO_BACKENDS, getSyncResponse, InitCrypto, mkEventCustom, syncPromise } from "../../test-utils/test-utils"; import * as testData from "../../test-utils/test-data"; import { BOB_SIGNED_CROSS_SIGNING_KEYS_DATA, @@ -31,12 +31,14 @@ import { BOB_TEST_USER_ID, SIGNED_CROSS_SIGNING_KEYS_DATA, SIGNED_TEST_DEVICE_DATA, + 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, ClientEvent, createClient, CryptoEvent, @@ -71,8 +73,8 @@ import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder"; import { DecryptionError } from "../../../src/crypto/algorithms"; import { IKeyBackup } from "../../../src/crypto/backup"; import { - createOlmSession, createOlmAccount, + createOlmSession, encryptGroupSessionKey, encryptMegolmEvent, encryptMegolmEventRawPlainText, @@ -80,6 +82,7 @@ import { establishOlmSession, getTestOlmAccountKeys, } from "./olm-utils"; +import { HistoryVisibility } from "../../../src/@types/partials"; afterEach(() => { // reset fake-indexeddb after each test, to make sure we don't leak connections @@ -924,6 +927,80 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }); }); + newBackendOnly("should rotate the session when the history visibility changes", async () => { + expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); + await startClientAndAwaitFirstSync(); + const p2pSession = await establishOlmSession(aliceClient, keyReceiver, syncResponder, testOlmAccount); + + // Tell alice we share a room with bob + syncResponder.sendOrQueueSyncResponse(getSyncResponse(["@bob:xyz"])); + await syncPromise(aliceClient); + + // Force alice to download bob keys + expectAliceKeyQuery(getTestKeysQueryResponse("@bob:xyz")); + + /** + * Return the event received on rooms/{roomId}/send/ endpoint. + * See https://spec.matrix.org/latest/client-server-api/#put_matrixclientv3roomsroomidsendeventtypetxnid + * @returns the content of event (no decryption) + */ + function expectSendMessage() { + return new Promise((resolve) => { + fetchMock.putOnce( + new RegExp("/send/"), + (url, request) => { + const content = JSON.parse(request.body as string); + resolve(content); + return { event_id: "$event_id" }; + }, + { overwriteRoutes: true }, + ); + }); + } + + // Send a message to bob and get the current session id + let [, , encryptedMessage] = await Promise.all([ + aliceClient.sendTextMessage(TEST_ROOM_ID, "test"), + expectSendRoomKey("@bob:xyz", testOlmAccount, p2pSession), + expectSendMessage(), + ]); + + // Check that the session id exists + const sessionId = encryptedMessage.session_id; + expect(sessionId).toBeDefined(); + + // Change history visibility in sync response + const syncResponse = getSyncResponse([]); + syncResponse.rooms[Category.Join][TEST_ROOM_ID].timeline.events.push( + mkEventCustom({ + sender: TEST_USER_ID, + type: "m.room.history_visibility", + state_key: "", + content: { + history_visibility: HistoryVisibility.Invited, + }, + }), + ); + + // send the new visibility + syncResponder.sendOrQueueSyncResponse(syncResponse); + await syncPromise(aliceClient); + + // Resend a message to bob and get the new session id + [, , encryptedMessage] = await Promise.all([ + aliceClient.sendTextMessage(TEST_ROOM_ID, "test"), + expectSendRoomKey("@bob:xyz", testOlmAccount, p2pSession), + expectSendMessage(), + ]); + + // Check that the new session id exists + const newSessionId = encryptedMessage.session_id; + expect(newSessionId).toBeDefined(); + + // Check that the session id has changed + expect(sessionId).not.toEqual(newSessionId); + }); + oldBackendOnly("We should start a new megolm session when a device is blocked", async () => { expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); await startClientAndAwaitFirstSync(); diff --git a/src/rust-crypto/RoomEncryptor.ts b/src/rust-crypto/RoomEncryptor.ts index 5f1af2610ca..a70dccbb630 100644 --- a/src/rust-crypto/RoomEncryptor.ts +++ b/src/rust-crypto/RoomEncryptor.ts @@ -14,7 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { EncryptionSettings, OlmMachine, RoomId, UserId } from "@matrix-org/matrix-sdk-crypto-wasm"; +import { + EncryptionSettings, + OlmMachine, + RoomId, + UserId, + HistoryVisibility as RustHistoryVisibility, +} from "@matrix-org/matrix-sdk-crypto-wasm"; import { EventType } from "../@types/event"; import { IContent, MatrixEvent } from "../models/event"; @@ -23,6 +29,7 @@ import { Logger, logger } from "../logger"; import { KeyClaimManager } from "./KeyClaimManager"; import { RoomMember } from "../models/room-member"; import { OutgoingRequestProcessor } from "./OutgoingRequestProcessor"; +import { HistoryVisibility } from "../@types/partials"; /** * RoomEncryptor: responsible for encrypting messages to a given room @@ -103,7 +110,9 @@ export class RoomEncryptor { this.prefixedLogger.debug("Sessions for users are ready; now sharing room key"); const rustEncryptionSettings = new EncryptionSettings(); - /* FIXME historyVisibility, rotation, etc */ + /* FIXME rotation, etc */ + + rustEncryptionSettings.historyVisibility = this.toRustHistoryVisibility(this.room.getHistoryVisibility()); const shareMessages = await this.olmMachine.shareRoomKey( new RoomId(this.room.roomId), @@ -117,6 +126,24 @@ export class RoomEncryptor { } } + /** + * Convert a HistoryVisibility to a RustHistoryVisibility + * @param visibility - HistoryVisibility enum + $ @returns a RustHistoryVisibility enum + */ + private toRustHistoryVisibility(visibility: HistoryVisibility): RustHistoryVisibility { + switch (visibility) { + case HistoryVisibility.Invited: + return RustHistoryVisibility.Invited; + case HistoryVisibility.Joined: + return RustHistoryVisibility.Joined; + case HistoryVisibility.Shared: + return RustHistoryVisibility.Shared; + case HistoryVisibility.WorldReadable: + return RustHistoryVisibility.WorldReadable; + } + } + /** * Discard any existing group session for this room */ From 539a2190d8c04fef3ea44e1f18ca069a28ac6392 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Tue, 17 Oct 2023 17:47:29 +0200 Subject: [PATCH 2/6] Add more tests to history visibility conversion --- spec/unit/rust-crypto/RoomEncryptor.spec.ts | 31 +++++++++++++++++ src/rust-crypto/RoomEncryptor.ts | 38 ++++++++++----------- 2 files changed, 50 insertions(+), 19 deletions(-) create mode 100644 spec/unit/rust-crypto/RoomEncryptor.spec.ts diff --git a/spec/unit/rust-crypto/RoomEncryptor.spec.ts b/spec/unit/rust-crypto/RoomEncryptor.spec.ts new file mode 100644 index 00000000000..66d21f9d5da --- /dev/null +++ b/spec/unit/rust-crypto/RoomEncryptor.spec.ts @@ -0,0 +1,31 @@ +/* + * + * Copyright 2023 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * / + */ + +import { HistoryVisibility as RustHistoryVisibility } from "@matrix-org/matrix-sdk-crypto-wasm"; + +import { HistoryVisibility } from "../../../src"; +import { toRustHistoryVisibility } from "../../../src/rust-crypto/RoomEncryptor"; + +it.each([ + [HistoryVisibility.Invited, RustHistoryVisibility.Invited], + [HistoryVisibility.Joined, RustHistoryVisibility.Joined], + [HistoryVisibility.Shared, RustHistoryVisibility.Shared], + [HistoryVisibility.WorldReadable, RustHistoryVisibility.WorldReadable], +])("JS HistoryVisibility to Rust HistoryVisibility: converts %s to %s", (historyVisibility, expected) => { + expect(toRustHistoryVisibility(historyVisibility)).toBe(expected); +}); diff --git a/src/rust-crypto/RoomEncryptor.ts b/src/rust-crypto/RoomEncryptor.ts index a70dccbb630..a867c5bec42 100644 --- a/src/rust-crypto/RoomEncryptor.ts +++ b/src/rust-crypto/RoomEncryptor.ts @@ -112,7 +112,7 @@ export class RoomEncryptor { const rustEncryptionSettings = new EncryptionSettings(); /* FIXME rotation, etc */ - rustEncryptionSettings.historyVisibility = this.toRustHistoryVisibility(this.room.getHistoryVisibility()); + rustEncryptionSettings.historyVisibility = toRustHistoryVisibility(this.room.getHistoryVisibility()); const shareMessages = await this.olmMachine.shareRoomKey( new RoomId(this.room.roomId), @@ -126,24 +126,6 @@ export class RoomEncryptor { } } - /** - * Convert a HistoryVisibility to a RustHistoryVisibility - * @param visibility - HistoryVisibility enum - $ @returns a RustHistoryVisibility enum - */ - private toRustHistoryVisibility(visibility: HistoryVisibility): RustHistoryVisibility { - switch (visibility) { - case HistoryVisibility.Invited: - return RustHistoryVisibility.Invited; - case HistoryVisibility.Joined: - return RustHistoryVisibility.Joined; - case HistoryVisibility.Shared: - return RustHistoryVisibility.Shared; - case HistoryVisibility.WorldReadable: - return RustHistoryVisibility.WorldReadable; - } - } - /** * Discard any existing group session for this room */ @@ -179,3 +161,21 @@ export class RoomEncryptor { ); } } + +/** + * Convert a HistoryVisibility to a RustHistoryVisibility + * @param visibility - HistoryVisibility enum + $ @returns a RustHistoryVisibility enum + */ +export function toRustHistoryVisibility(visibility: HistoryVisibility): RustHistoryVisibility { + switch (visibility) { + case HistoryVisibility.Invited: + return RustHistoryVisibility.Invited; + case HistoryVisibility.Joined: + return RustHistoryVisibility.Joined; + case HistoryVisibility.Shared: + return RustHistoryVisibility.Shared; + case HistoryVisibility.WorldReadable: + return RustHistoryVisibility.WorldReadable; + } +} From 1a0d2ef41f9d958951c17b1efcd2c6392e9c08d6 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 18 Oct 2023 16:10:21 +0200 Subject: [PATCH 3/6] Factorize `expectSendMessage` and `expectSendMegolmMessage` --- spec/integ/crypto/crypto.spec.ts | 61 +++++++++++++------------------- 1 file changed, 24 insertions(+), 37 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index fd54093fdf2..744a67b200c 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -149,6 +149,27 @@ async function expectSendRoomKey( }); } +/** + * Return the event received on rooms/{roomId}/send/m.room.encrypted endpoint. + * See https://spec.matrix.org/latest/client-server-api/#put_matrixclientv3roomsroomidsendeventtypetxnid + * @returns the content of event (no decryption) + */ +function expectEncryptedSendMessage() { + return new Promise((resolve) => { + fetchMock.putOnce( + new RegExp("/send/m.room.encrypted/"), + (url, request) => { + const content = JSON.parse(request.body as string); + resolve(content); + return { event_id: "$event_id" }; + }, + // append to the list of intercepts on this path (since we have some tests that call + // this function multiple times) + { overwriteRoutes: true }, + ); + }); +} + /** * Expect that the client sends an encrypted event * @@ -162,22 +183,7 @@ async function expectSendRoomKey( async function expectSendMegolmMessage( inboundGroupSessionPromise: Promise, ): Promise> { - const encryptedMessageContent = await new Promise((resolve) => { - fetchMock.putOnce( - new RegExp("/send/m.room.encrypted/"), - (url: string, opts: RequestInit): MockResponse => { - resolve(JSON.parse(opts.body as string)); - return { - event_id: "$event_id", - }; - }, - { - // append to the list of intercepts on this path (since we have some tests that call - // this function multiple times) - overwriteRoutes: false, - }, - ); - }); + const encryptedMessageContent = await expectEncryptedSendMessage(); // In some of the tests, the room key is sent *after* the actual event, so we may need to wait for it now. const inboundGroupSession = await inboundGroupSessionPromise; @@ -939,30 +945,11 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, // Force alice to download bob keys expectAliceKeyQuery(getTestKeysQueryResponse("@bob:xyz")); - /** - * Return the event received on rooms/{roomId}/send/ endpoint. - * See https://spec.matrix.org/latest/client-server-api/#put_matrixclientv3roomsroomidsendeventtypetxnid - * @returns the content of event (no decryption) - */ - function expectSendMessage() { - return new Promise((resolve) => { - fetchMock.putOnce( - new RegExp("/send/"), - (url, request) => { - const content = JSON.parse(request.body as string); - resolve(content); - return { event_id: "$event_id" }; - }, - { overwriteRoutes: true }, - ); - }); - } - // Send a message to bob and get the current session id let [, , encryptedMessage] = await Promise.all([ aliceClient.sendTextMessage(TEST_ROOM_ID, "test"), expectSendRoomKey("@bob:xyz", testOlmAccount, p2pSession), - expectSendMessage(), + expectEncryptedSendMessage(), ]); // Check that the session id exists @@ -990,7 +977,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, [, , encryptedMessage] = await Promise.all([ aliceClient.sendTextMessage(TEST_ROOM_ID, "test"), expectSendRoomKey("@bob:xyz", testOlmAccount, p2pSession), - expectSendMessage(), + expectEncryptedSendMessage(), ]); // Check that the new session id exists From 5583a999f14817ff849d105a5668718e7d76214f Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Thu, 19 Oct 2023 10:50:04 +0200 Subject: [PATCH 4/6] Use correct import --- spec/integ/crypto/crypto.spec.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 744a67b200c..0a3c9eb38b5 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -31,7 +31,6 @@ import { BOB_TEST_USER_ID, SIGNED_CROSS_SIGNING_KEYS_DATA, SIGNED_TEST_DEVICE_DATA, - TEST_ROOM_ID, TEST_ROOM_ID as ROOM_ID, TEST_USER_ID, } from "../../test-utils/test-data"; @@ -55,6 +54,7 @@ import { Room, RoomMember, RoomStateEvent, + HistoryVisibility, } from "../../../src/matrix"; import { DeviceInfo } from "../../../src/crypto/deviceinfo"; import { E2EKeyReceiver } from "../../test-utils/E2EKeyReceiver"; @@ -82,7 +82,6 @@ import { establishOlmSession, getTestOlmAccountKeys, } from "./olm-utils"; -import { HistoryVisibility } from "../../../src/@types/partials"; afterEach(() => { // reset fake-indexeddb after each test, to make sure we don't leak connections @@ -947,7 +946,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, // Send a message to bob and get the current session id let [, , encryptedMessage] = await Promise.all([ - aliceClient.sendTextMessage(TEST_ROOM_ID, "test"), + aliceClient.sendTextMessage(ROOM_ID, "test"), expectSendRoomKey("@bob:xyz", testOlmAccount, p2pSession), expectEncryptedSendMessage(), ]); @@ -958,7 +957,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, // Change history visibility in sync response const syncResponse = getSyncResponse([]); - syncResponse.rooms[Category.Join][TEST_ROOM_ID].timeline.events.push( + syncResponse.rooms[Category.Join][ROOM_ID].timeline.events.push( mkEventCustom({ sender: TEST_USER_ID, type: "m.room.history_visibility", @@ -975,7 +974,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, // Resend a message to bob and get the new session id [, , encryptedMessage] = await Promise.all([ - aliceClient.sendTextMessage(TEST_ROOM_ID, "test"), + aliceClient.sendTextMessage(ROOM_ID, "test"), expectSendRoomKey("@bob:xyz", testOlmAccount, p2pSession), expectEncryptedSendMessage(), ]); From f888f2a1a246826d2060005d8eafc4cdf344f296 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Thu, 19 Oct 2023 10:52:40 +0200 Subject: [PATCH 5/6] Fix overwriteRoutes --- spec/integ/crypto/crypto.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 0a3c9eb38b5..14f81dc411d 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -164,7 +164,7 @@ function expectEncryptedSendMessage() { }, // append to the list of intercepts on this path (since we have some tests that call // this function multiple times) - { overwriteRoutes: true }, + { overwriteRoutes: false }, ); }); } From 4e31975f8936701b1b0206747a2cf9e5f24ec6d8 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Mon, 23 Oct 2023 16:05:38 +0200 Subject: [PATCH 6/6] Update comments --- spec/integ/crypto/crypto.spec.ts | 2 +- src/rust-crypto/RoomEncryptor.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 14f81dc411d..fe113f9b15d 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -968,7 +968,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }), ); - // send the new visibility + // Update the new visibility syncResponder.sendOrQueueSyncResponse(syncResponse); await syncPromise(aliceClient); diff --git a/src/rust-crypto/RoomEncryptor.ts b/src/rust-crypto/RoomEncryptor.ts index a867c5bec42..fc10c6a1338 100644 --- a/src/rust-crypto/RoomEncryptor.ts +++ b/src/rust-crypto/RoomEncryptor.ts @@ -165,7 +165,7 @@ export class RoomEncryptor { /** * Convert a HistoryVisibility to a RustHistoryVisibility * @param visibility - HistoryVisibility enum - $ @returns a RustHistoryVisibility enum + * @returns a RustHistoryVisibility enum */ export function toRustHistoryVisibility(visibility: HistoryVisibility): RustHistoryVisibility { switch (visibility) {