From a8a9242fd9b2331fac2b916ad13865fa24b9b394 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Fri, 24 Nov 2023 10:47:48 +0000 Subject: [PATCH 1/8] Ensure we cache a room when it's not encrypted. --- src/e2ee/RoomTracker.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/e2ee/RoomTracker.ts b/src/e2ee/RoomTracker.ts index 98ed5bda..352f4bca 100644 --- a/src/e2ee/RoomTracker.ts +++ b/src/e2ee/RoomTracker.ts @@ -1,4 +1,5 @@ import { MatrixClient } from "../MatrixClient"; +import { MatrixError } from "../models/MatrixError"; import { EncryptionEventContent } from "../models/events/EncryptionEvent"; import { ICryptoRoomInformation } from "./ICryptoRoomInformation"; @@ -61,7 +62,13 @@ export class RoomTracker { encEvent = await this.client.getRoomStateEvent(roomId, "m.room.encryption", ""); encEvent.algorithm = encEvent.algorithm ?? 'UNKNOWN'; } catch (e) { - return; // failure == no encryption + if (e instanceof MatrixError && e.errcode === "M_NOT_FOUND") { + // No encryption + encEvent = {}; + } else { + // Unexpected failure, do not store. + return; + } } // Pick out the history visibility setting too From 2fdc594fef55c3996edef0597af5593356b4beaf Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Fri, 24 Nov 2023 10:56:59 +0000 Subject: [PATCH 2/8] Cache rooms that are not encrypted. --- src/e2ee/RoomTracker.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/e2ee/RoomTracker.ts b/src/e2ee/RoomTracker.ts index 352f4bca..b4e635ac 100644 --- a/src/e2ee/RoomTracker.ts +++ b/src/e2ee/RoomTracker.ts @@ -63,12 +63,9 @@ export class RoomTracker { encEvent.algorithm = encEvent.algorithm ?? 'UNKNOWN'; } catch (e) { if (e instanceof MatrixError && e.errcode === "M_NOT_FOUND") { - // No encryption encEvent = {}; - } else { - // Unexpected failure, do not store. - return; } + return; // Other failures should not be cached. } // Pick out the history visibility setting too From 0ee885efb4c2229700fe12d4dbb8a69dccd568c7 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Fri, 24 Nov 2023 10:57:12 +0000 Subject: [PATCH 3/8] Recheck encrypted status on each m.room.encryption --- src/e2ee/CryptoClient.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/e2ee/CryptoClient.ts b/src/e2ee/CryptoClient.ts index be9d1ecf..5acec0d5 100644 --- a/src/e2ee/CryptoClient.ts +++ b/src/e2ee/CryptoClient.ts @@ -117,6 +117,8 @@ export class CryptoClient { if (membership.effectiveMembership !== 'join' && membership.effectiveMembership !== 'invite') return; await this.engine.addTrackedUsers([membership.membershipFor]); } else if (event['type'] === 'm.room.encryption') { + // Update encryption status of room. + await this.roomTracker.queueRoomCheck(roomId); return this.client.getRoomMembers(roomId, null, ['join', 'invite']).then( members => this.engine.addTrackedUsers(members.map(e => e.membershipFor)), e => void LogService.warn("CryptoClient", `Unable to get members of room ${roomId}`), From b924ef0bc70ce223ab9f9768fc8cb3abaf621f93 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Fri, 24 Nov 2023 14:52:19 +0000 Subject: [PATCH 4/8] Don't check all joined rooms on startup --- src/MatrixClient.ts | 2 +- src/appservice/Intent.ts | 2 +- src/e2ee/CryptoClient.ts | 4 +--- src/e2ee/RoomTracker.ts | 10 ---------- 4 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/MatrixClient.ts b/src/MatrixClient.ts index 5370a411..ddba1827 100644 --- a/src/MatrixClient.ts +++ b/src/MatrixClient.ts @@ -656,7 +656,7 @@ export class MatrixClient extends EventEmitter { if (this.crypto) { LogService.debug("MatrixClientLite", "Preparing end-to-end encryption"); - await this.crypto.prepare(this.lastJoinedRoomIds); + await this.crypto.prepare(); LogService.info("MatrixClientLite", "End-to-end encryption enabled"); } diff --git a/src/appservice/Intent.ts b/src/appservice/Intent.ts index eb3f3787..61a9917e 100644 --- a/src/appservice/Intent.ts +++ b/src/appservice/Intent.ts @@ -168,7 +168,7 @@ export class Intent { } // Now set up crypto - await this.client.crypto.prepare(await this.getJoinedRooms()); + await this.client.crypto.prepare(); this.appservice.on("room.event", (roomId, event) => { this.client.crypto.onRoomEvent(roomId, event); diff --git a/src/e2ee/CryptoClient.ts b/src/e2ee/CryptoClient.ts index 5acec0d5..789cce29 100644 --- a/src/e2ee/CryptoClient.ts +++ b/src/e2ee/CryptoClient.ts @@ -68,9 +68,7 @@ export class CryptoClient { * Prepares the crypto client for usage. * @param {string[]} roomIds The room IDs the MatrixClient is joined to. */ - public async prepare(roomIds: string[]) { - await this.roomTracker.prepare(roomIds); - + public async prepare() { if (this.ready) return; // stop re-preparing here const storedDeviceId = await this.client.cryptoStore.getDeviceId(); diff --git a/src/e2ee/RoomTracker.ts b/src/e2ee/RoomTracker.ts index b4e635ac..5d6dfbb4 100644 --- a/src/e2ee/RoomTracker.ts +++ b/src/e2ee/RoomTracker.ts @@ -34,16 +34,6 @@ export class RoomTracker { } } - /** - * Prepares the room tracker to track the given rooms. - * @param {string[]} roomIds The room IDs to track. This should be the joined rooms set. - */ - public async prepare(roomIds: string[]) { - for (const roomId of roomIds) { - await this.queueRoomCheck(roomId); - } - } - /** * Queues a room check for the tracker. If the room needs an update to the store, an * update will be made. From 5c059a1f5d1d7dcc035d879367b10dc528b80458 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Fri, 24 Nov 2023 16:37:32 +0000 Subject: [PATCH 5/8] Fix test --- examples/encryption_bot.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/encryption_bot.ts b/examples/encryption_bot.ts index 4dda4c56..98bae196 100644 --- a/examples/encryption_bot.ts +++ b/examples/encryption_bot.ts @@ -35,9 +35,9 @@ const worksImage = fs.readFileSync("./examples/static/it-works.png"); const client = new MatrixClient(homeserverUrl, accessToken, storage, crypto); (async function() { - let encryptedRoomId: string; + let encryptedRoomId: string|undefined = undefined; const joinedRooms = await client.getJoinedRooms(); - await client.crypto.prepare(joinedRooms); // init crypto because we're doing things before the client is started + await client.crypto.prepare(); // init crypto because we're doing things before the client is started for (const roomId of joinedRooms) { if (await client.crypto.isRoomEncrypted(roomId)) { encryptedRoomId = roomId; From bf1159e88833024c5dba0a5350a984d39f5913d9 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 27 Nov 2023 17:20:45 +0000 Subject: [PATCH 6/8] Fix tests --- src/e2ee/CryptoClient.ts | 2 -- test/encryption/CryptoClientTest.ts | 43 ++++++++++++----------------- test/encryption/KeyBackupTest.ts | 2 +- test/encryption/RoomTrackerTest.ts | 22 ++------------- 4 files changed, 21 insertions(+), 48 deletions(-) diff --git a/src/e2ee/CryptoClient.ts b/src/e2ee/CryptoClient.ts index 789cce29..110fa090 100644 --- a/src/e2ee/CryptoClient.ts +++ b/src/e2ee/CryptoClient.ts @@ -115,8 +115,6 @@ export class CryptoClient { if (membership.effectiveMembership !== 'join' && membership.effectiveMembership !== 'invite') return; await this.engine.addTrackedUsers([membership.membershipFor]); } else if (event['type'] === 'm.room.encryption') { - // Update encryption status of room. - await this.roomTracker.queueRoomCheck(roomId); return this.client.getRoomMembers(roomId, null, ['join', 'invite']).then( members => this.engine.addTrackedUsers(members.map(e => e.membershipFor)), e => void LogService.warn("CryptoClient", `Unable to get members of room ${roomId}`), diff --git a/test/encryption/CryptoClientTest.ts b/test/encryption/CryptoClientTest.ts index 60d7fe84..bd055fc1 100644 --- a/test/encryption/CryptoClientTest.ts +++ b/test/encryption/CryptoClientTest.ts @@ -17,7 +17,7 @@ describe('CryptoClient', () => { bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); @@ -28,24 +28,17 @@ describe('CryptoClient', () => { describe('prepare', () => { it('should prepare the room tracker', () => testCryptoStores(async (cryptoStoreType) => { const userId = "@alice:example.org"; - const roomIds = ["!a:example.org", "!b:example.org"]; const { client, http } = createTestClient(null, userId, cryptoStoreType); client.getWhoAmI = () => Promise.resolve({ user_id: userId, device_id: TEST_DEVICE_ID }); - const prepareSpy = simple.stub().callFn((rids: string[]) => { - expect(rids).toBe(roomIds); - return Promise.resolve(); - }); - - (client.crypto).roomTracker.prepare = prepareSpy; // private member access - bindNullEngine(http); - await Promise.all([ - client.crypto.prepare(roomIds), + // Prepare first + await Promise.all([, + client.crypto.prepare(), http.flushAllExpected(), ]); - expect(prepareSpy.callCount).toEqual(1); + expect(client.crypto.isReady).toBe(true); })); it('should use a stored device ID', () => testCryptoStores(async (cryptoStoreType) => { @@ -59,7 +52,7 @@ describe('CryptoClient', () => { bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); expect(whoamiSpy.callCount).toEqual(0); @@ -118,7 +111,7 @@ describe('CryptoClient', () => { bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); @@ -138,7 +131,7 @@ describe('CryptoClient', () => { const { client } = createTestClient(null, userId, cryptoStoreType); await client.cryptoStore.setDeviceId(TEST_DEVICE_ID); - // await client.crypto.prepare([]); // deliberately commented + // await client.crypto.prepare(); // deliberately commented try { await client.crypto.isRoomEncrypted("!new:example.org"); @@ -159,7 +152,7 @@ describe('CryptoClient', () => { bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); @@ -176,7 +169,7 @@ describe('CryptoClient', () => { bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); @@ -193,7 +186,7 @@ describe('CryptoClient', () => { bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); @@ -210,7 +203,7 @@ describe('CryptoClient', () => { bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); @@ -248,7 +241,7 @@ describe('CryptoClient', () => { it('should sign the object while retaining signatures without mutation', async () => { bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); @@ -305,7 +298,7 @@ describe('CryptoClient', () => { it('should fail in unencrypted rooms', async () => { bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); @@ -381,7 +374,7 @@ describe('CryptoClient', () => { it('should encrypt media', async () => { bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); @@ -467,7 +460,7 @@ describe('CryptoClient', () => { it('should be symmetrical', async () => { bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); @@ -492,7 +485,7 @@ describe('CryptoClient', () => { it('should decrypt', async () => { bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); @@ -522,7 +515,7 @@ describe('CryptoClient', () => { await client.cryptoStore.setDeviceId(TEST_DEVICE_ID); bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); })); diff --git a/test/encryption/KeyBackupTest.ts b/test/encryption/KeyBackupTest.ts index 186a530f..100a8d8e 100644 --- a/test/encryption/KeyBackupTest.ts +++ b/test/encryption/KeyBackupTest.ts @@ -21,7 +21,7 @@ describe('KeyBackups', () => { const prepareCrypto = async () => { bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); }; diff --git a/test/encryption/RoomTrackerTest.ts b/test/encryption/RoomTrackerTest.ts index 3c4c16a1..0968544f 100644 --- a/test/encryption/RoomTrackerTest.ts +++ b/test/encryption/RoomTrackerTest.ts @@ -44,7 +44,7 @@ describe('RoomTracker', () => { await client.cryptoStore.setDeviceId(TEST_DEVICE_ID); bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); (client.crypto as any).engine.addTrackedUsers = () => Promise.resolve(); @@ -72,7 +72,7 @@ describe('RoomTracker', () => { await client.cryptoStore.setDeviceId(TEST_DEVICE_ID); bindNullEngine(http); await Promise.all([ - client.crypto.prepare([]), + client.crypto.prepare(), http.flushAllExpected(), ]); @@ -103,24 +103,6 @@ describe('RoomTracker', () => { expect(queueSpy.callCount).toEqual(1); })); - describe('prepare', () => { - it('should queue updates for rooms', async () => { - const roomIds = ["!a:example.org", "!b:example.org"]; - - const { client } = createTestClient(); - - const queueSpy = simple.stub().callFn((rid: string) => { - expect(rid).toEqual(roomIds[queueSpy.callCount - 1]); - return Promise.resolve(); - }); - - const tracker = new RoomTracker(client); - tracker.queueRoomCheck = queueSpy; - await tracker.prepare(roomIds); - expect(queueSpy.callCount).toEqual(2); - }); - }); - describe('queueRoomCheck', () => { it('should store unknown rooms', () => testCryptoStores(async (cryptoStoreType) => { const roomId = "!b:example.org"; From be459b06a94d0eea9a1409f324c53d1c448fbfc3 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 27 Nov 2023 17:24:11 +0000 Subject: [PATCH 7/8] Comma --- test/encryption/CryptoClientTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/encryption/CryptoClientTest.ts b/test/encryption/CryptoClientTest.ts index bd055fc1..f097be90 100644 --- a/test/encryption/CryptoClientTest.ts +++ b/test/encryption/CryptoClientTest.ts @@ -34,7 +34,7 @@ describe('CryptoClient', () => { bindNullEngine(http); // Prepare first - await Promise.all([, + await Promise.all([ client.crypto.prepare(), http.flushAllExpected(), ]); From 7e050c7336a62cbad51353d82b1d75dac7828c29 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Wed, 29 Nov 2023 16:07:40 +0000 Subject: [PATCH 8/8] Fix branch --- src/e2ee/RoomTracker.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/e2ee/RoomTracker.ts b/src/e2ee/RoomTracker.ts index 5d6dfbb4..9b3c0e82 100644 --- a/src/e2ee/RoomTracker.ts +++ b/src/e2ee/RoomTracker.ts @@ -54,8 +54,9 @@ export class RoomTracker { } catch (e) { if (e instanceof MatrixError && e.errcode === "M_NOT_FOUND") { encEvent = {}; + } else { + return; // Other failures should not be cached. } - return; // Other failures should not be cached. } // Pick out the history visibility setting too