diff --git a/src/e2ee/CryptoClient.ts b/src/e2ee/CryptoClient.ts index 43455db8..34d56b8b 100644 --- a/src/e2ee/CryptoClient.ts +++ b/src/e2ee/CryptoClient.ts @@ -301,13 +301,17 @@ export class CryptoClient { * Enable backing up of room keys. * @param {IKeyBackupInfoRetrieved} info The configuration for key backup behaviour, * as returned by {@link MatrixClient#getKeyBackupVersion}. - * @returns {Promise} Resolves when complete. + * @returns {Promise} Resolves once backups have been enabled. */ @requiresReady() public async enableKeyBackup(info: IKeyBackupInfoRetrieved): Promise { - this.client.on("to_device.decrypted", this.onToDeviceMessage); + if (!this.engine.isBackupEnabled()) { + // Only add the listener if we didn't add it already + this.client.on("to_device.decrypted", this.onToDeviceMessage); + } await this.engine.enableKeyBackup(info); - this.engine.backupRoomKeys(); + // Back up any pending keys now, but asynchronously + void this.engine.backupRoomKeys(); } /** diff --git a/src/e2ee/RustEngine.ts b/src/e2ee/RustEngine.ts index ff64bef5..8239b79c 100644 --- a/src/e2ee/RustEngine.ts +++ b/src/e2ee/RustEngine.ts @@ -35,7 +35,11 @@ export class RustEngine { private keyBackupVersion: KeyBackupVersion|undefined; private keyBackupWaiter = Promise.resolve(); - private isBackupEnabled = false; + + private backupEnabled = false; + public isBackupEnabled() { + return this.backupEnabled; + } public constructor(public readonly machine: OlmMachine, private client: MatrixClient) { } @@ -154,7 +158,7 @@ export class RustEngine { public enableKeyBackup(info: IKeyBackupInfoRetrieved): Promise { this.keyBackupWaiter = this.keyBackupWaiter.then(async () => { - if (this.isBackupEnabled) { + if (this.backupEnabled) { // Finish any pending backups before changing the backup version/pubkey await this.actuallyDisableKeyBackup(); } @@ -168,7 +172,7 @@ export class RustEngine { } await this.machine.enableBackupV1(publicKey, info.version); this.keyBackupVersion = info.version; - this.isBackupEnabled = true; + this.backupEnabled = true; }); return this.keyBackupWaiter; } @@ -183,12 +187,12 @@ export class RustEngine { private async actuallyDisableKeyBackup(): Promise { await this.machine.disableBackup(); this.keyBackupVersion = undefined; - this.isBackupEnabled = false; + this.backupEnabled = false; } public backupRoomKeys(): Promise { this.keyBackupWaiter = this.keyBackupWaiter.then(async () => { - if (!this.isBackupEnabled) { + if (!this.backupEnabled) { throw new Error("Key backup error: attempted to create a backup before having enabled backups"); } await this.actuallyBackupRoomKeys(); @@ -202,7 +206,7 @@ export class RustEngine { private backupRoomKeysIfEnabled(): Promise { this.keyBackupWaiter = this.keyBackupWaiter.then(async () => { - if (this.isBackupEnabled) { + if (this.backupEnabled) { await this.actuallyBackupRoomKeys(); } }); diff --git a/test/encryption/KeyBackupTest.ts b/test/encryption/KeyBackupTest.ts index a7f2f076..186a530f 100644 --- a/test/encryption/KeyBackupTest.ts +++ b/test/encryption/KeyBackupTest.ts @@ -1,3 +1,4 @@ +import * as simple from "simple-mock"; import HttpBackend from 'matrix-mock-request'; import { @@ -8,7 +9,7 @@ import { IKeyBackupUpdateResponse, KeyBackupEncryptionAlgorithm, } from "../../src/models/KeyBackup"; -import { RoomEncryptionAlgorithm, ICryptoRoomInformation, MatrixClient, MembershipEvent, RoomTracker } from "../../src"; +import { ICryptoRoomInformation, IToDeviceMessage, MatrixClient, MembershipEvent, RoomEncryptionAlgorithm, RoomTracker } from "../../src"; import { bindNullEngine, createTestClient, testCryptoStores, TEST_DEVICE_ID, generateCurve25519PublicKey, bindNullQuery } from "../TestUtils"; const USER_ID = "@alice:example.org"; @@ -208,19 +209,22 @@ describe('KeyBackups', () => { }; const knownSessions: Set = new Set(); + let expectedSessions = 0; let etagCount = 0; + const onBackupRequest = (path, obj: Record): IKeyBackupUpdateResponse => { + const sessions = obj?.rooms[roomId]?.sessions; + expect(sessions).toBeDefined(); + + Object.keys(sessions).forEach(session => { knownSessions.add(session); }); + return { + count: knownSessions.size, + etag: `etag${++etagCount}`, + }; + }; + const expectToPutRoomKey = () => { - http.when("PUT", "/room_keys/keys").respond(200, (path, obj: Record): IKeyBackupUpdateResponse => { - const sessions = obj?.rooms[roomId]?.sessions; - expect(sessions).toBeDefined(); - - Object.keys(sessions).forEach(session => { knownSessions.add(session); }); - return { - count: knownSessions.size, - etag: `etag${++etagCount}`, - }; - }); + http.when("PUT", "/room_keys/keys").respond(200, onBackupRequest); }; expectToPutRoomKey(); @@ -228,16 +232,52 @@ describe('KeyBackups', () => { client.enableKeyBackup(keyBackupInfo), http.flushAllExpected(), ]); - expect(knownSessions.size).toStrictEqual(1); + expect(knownSessions.size).toBe(++expectedSessions); + + // --- Test that it's safe to re-enable backups + + // Re-enabling backups replays all existing keys, so expect another request to be made + expectToPutRoomKey(); + await Promise.all([ + client.enableKeyBackup(keyBackupInfo), + http.flushAllExpected(), + ]); + // No new session expected this time + expect(knownSessions.size).toBe(expectedSessions); // --- Back up a new room key by generating one while backups are enabled expectToPutRoomKey(); await encryptRoomEvent(); - expect(knownSessions.size).toStrictEqual(2); + expect(knownSessions.size).toBe(++expectedSessions); // --- Back up a room key received via a to-device message - // TODO: use updateSyncData to send an *encrypted* "m.room_key" event. + + const onRoomKeySpy = simple.mock((client.crypto as any).engine, "backupRoomKeys"); + + // TODO: Encrypt this so that it will actually be included in the backup. + // Until then, no backup request or new session are expected. + const toDeviceMessage: IToDeviceMessage = { + type: "m.room_key", + sender: USER_ID, + content: { + algorithm: RoomEncryptionAlgorithm.MegolmV1AesSha2, + room_id: roomId, + session_id: "abc", + session_key: "def", + }, + }; + + bindNullEngine(http); + await Promise.all([ + client.crypto.updateSyncData( + [toDeviceMessage], + {}, [], [], [], + ), + http.flushAllExpected(), + ]); + expect(knownSessions.size).toBe(expectedSessions); + expect(onRoomKeySpy.callCount).toBe(1); // --- Export a room key // TODO: consider moving this to a test dedicated to key exports