From 94fb839140964a967b0f37c1205b037732e0f3d0 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 13 Sep 2024 16:47:33 +0100 Subject: [PATCH 1/3] Don't share full key history for RTC per-participant encryption Also record stats for how many keys have been sent/received and age of those received --- spec/unit/matrixrtc/MatrixRTCSession.spec.ts | 134 +++++++++++++++++-- src/matrixrtc/MatrixRTCSession.ts | 81 ++++++++--- src/matrixrtc/types.ts | 1 + 3 files changed, 186 insertions(+), 30 deletions(-) diff --git a/spec/unit/matrixrtc/MatrixRTCSession.spec.ts b/spec/unit/matrixrtc/MatrixRTCSession.spec.ts index 4cc225f213f..31614010396 100644 --- a/spec/unit/matrixrtc/MatrixRTCSession.spec.ts +++ b/spec/unit/matrixrtc/MatrixRTCSession.spec.ts @@ -594,24 +594,35 @@ describe("MatrixRTCSession", () => { }); it("sends keys when joining", async () => { - const eventSentPromise = new Promise((resolve) => { - sendEventMock.mockImplementation(resolve); - }); + jest.useFakeTimers(); + try { + const eventSentPromise = new Promise((resolve) => { + sendEventMock.mockImplementation(resolve); + }); - sess!.joinRoomSession([mockFocus], mockFocus, { manageMediaKeys: true }); + sess!.joinRoomSession([mockFocus], mockFocus, { manageMediaKeys: true }); - await eventSentPromise; + await eventSentPromise; - expect(sendEventMock).toHaveBeenCalledWith(expect.stringMatching(".*"), "io.element.call.encryption_keys", { - call_id: "", - device_id: "AAAAAAA", - keys: [ + expect(sendEventMock).toHaveBeenCalledWith( + expect.stringMatching(".*"), + "io.element.call.encryption_keys", { - index: 0, - key: expect.stringMatching(".*"), + call_id: "", + device_id: "AAAAAAA", + keys: [ + { + index: 0, + key: expect.stringMatching(".*"), + }, + ], + sent_ts: Date.now(), }, - ], - }); + ); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(1); + } finally { + jest.useRealTimers(); + } }); it("does not send key if join called when already joined", () => { @@ -619,10 +630,12 @@ describe("MatrixRTCSession", () => { expect(client.sendStateEvent).toHaveBeenCalledTimes(1); expect(client.sendEvent).toHaveBeenCalledTimes(1); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(1); sess!.joinRoomSession([mockFocus], mockFocus, { manageMediaKeys: true }); expect(client.sendStateEvent).toHaveBeenCalledTimes(1); expect(client.sendEvent).toHaveBeenCalledTimes(1); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(1); }); it("retries key sends", async () => { @@ -651,6 +664,7 @@ describe("MatrixRTCSession", () => { await eventSentPromise; expect(sendEventMock).toHaveBeenCalledTimes(2); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(2); } finally { jest.useRealTimers(); } @@ -684,6 +698,7 @@ describe("MatrixRTCSession", () => { sess.joinRoomSession([mockFocus], mockFocus, { manageMediaKeys: true }); await keysSentPromise1; + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(1); sendEventMock.mockClear(); jest.advanceTimersByTime(10000); @@ -707,6 +722,7 @@ describe("MatrixRTCSession", () => { await keysSentPromise2; expect(sendEventMock).toHaveBeenCalled(); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(2); } finally { jest.useRealTimers(); } @@ -747,14 +763,17 @@ describe("MatrixRTCSession", () => { key: expect.stringMatching(".*"), }, ], + sent_ts: Date.now(), }, ); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(1); sendEventMock.mockClear(); // these should be a no-op: sess.onMembershipUpdate(); expect(sendEventMock).toHaveBeenCalledTimes(0); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(1); } finally { jest.useRealTimers(); } @@ -796,8 +815,10 @@ describe("MatrixRTCSession", () => { key: expect.stringMatching(".*"), }, ], + sent_ts: Date.now(), }, ); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(1); sendEventMock.mockClear(); @@ -832,8 +853,10 @@ describe("MatrixRTCSession", () => { key: expect.stringMatching(".*"), }, ], + sent_ts: Date.now(), }, ); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(2); } finally { jest.useRealTimers(); } @@ -877,8 +900,10 @@ describe("MatrixRTCSession", () => { key: expect.stringMatching(".*"), }, ], + sent_ts: Date.now(), }, ); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(1); sendEventMock.mockClear(); @@ -913,8 +938,10 @@ describe("MatrixRTCSession", () => { key: expect.stringMatching(".*"), }, ], + sent_ts: Date.now(), }, ); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(2); } finally { jest.useRealTimers(); } @@ -946,6 +973,8 @@ describe("MatrixRTCSession", () => { sess.joinRoomSession([mockFocus], mockFocus, { manageMediaKeys: true }); const firstKeysPayload = await keysSentPromise1; expect(firstKeysPayload.keys).toHaveLength(1); + expect(firstKeysPayload.keys[0].index).toEqual(0); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(1); sendEventMock.mockClear(); @@ -962,8 +991,10 @@ describe("MatrixRTCSession", () => { const secondKeysPayload = await keysSentPromise2; - expect(secondKeysPayload.keys).toHaveLength(2); + expect(secondKeysPayload.keys).toHaveLength(1); + expect(secondKeysPayload.keys[0].index).toEqual(1); expect(onMyEncryptionKeyChanged).toHaveBeenCalledTimes(2); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(2); } finally { jest.useRealTimers(); } @@ -984,6 +1015,7 @@ describe("MatrixRTCSession", () => { await keysSentPromise1; sendEventMock.mockClear(); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(1); const onMembershipsChanged = jest.fn(); sess.on(MatrixRTCSessionEvent.MembershipsChanged, onMembershipsChanged); @@ -1002,6 +1034,7 @@ describe("MatrixRTCSession", () => { }); expect(sendEventMock).not.toHaveBeenCalled(); + expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(1); } finally { jest.useRealTimers(); } @@ -1167,6 +1200,7 @@ describe("MatrixRTCSession", () => { const bobKeys = sess.getKeysForParticipant("@bob:example.org", "bobsphone")!; expect(bobKeys).toHaveLength(1); expect(bobKeys[0]).toEqual(Buffer.from("this is the key", "utf-8")); + expect(sess!.statistics.counters.roomEventEncryptionKeysReceived).toEqual(1); }); it("collects keys at non-zero indices", () => { @@ -1195,6 +1229,7 @@ describe("MatrixRTCSession", () => { expect(bobKeys[2]).toBeFalsy(); expect(bobKeys[3]).toBeFalsy(); expect(bobKeys[4]).toEqual(Buffer.from("this is the key", "utf-8")); + expect(sess!.statistics.counters.roomEventEncryptionKeysReceived).toEqual(1); }); it("collects keys by merging", () => { @@ -1219,6 +1254,7 @@ describe("MatrixRTCSession", () => { let bobKeys = sess.getKeysForParticipant("@bob:example.org", "bobsphone")!; expect(bobKeys).toHaveLength(1); expect(bobKeys[0]).toEqual(Buffer.from("this is the key", "utf-8")); + expect(sess!.statistics.counters.roomEventEncryptionKeysReceived).toEqual(1); sess.onCallEncryption({ getType: jest.fn().mockReturnValue("io.element.call.encryption_keys"), @@ -1239,6 +1275,7 @@ describe("MatrixRTCSession", () => { bobKeys = sess.getKeysForParticipant("@bob:example.org", "bobsphone")!; expect(bobKeys).toHaveLength(5); expect(bobKeys[4]).toEqual(Buffer.from("this is the key", "utf-8")); + expect(sess!.statistics.counters.roomEventEncryptionKeysReceived).toEqual(2); }); it("ignores older keys at same index", () => { @@ -1279,6 +1316,7 @@ describe("MatrixRTCSession", () => { const bobKeys = sess.getKeysForParticipant("@bob:example.org", "bobsphone")!; expect(bobKeys).toHaveLength(1); expect(bobKeys[0]).toEqual(Buffer.from("newer key", "utf-8")); + expect(sess!.statistics.counters.roomEventEncryptionKeysReceived).toEqual(2); }); it("key timestamps are treated as monotonic", () => { @@ -1342,5 +1380,73 @@ describe("MatrixRTCSession", () => { const myKeys = sess.getKeysForParticipant(client.getUserId()!, client.getDeviceId()!)!; expect(myKeys).toBeFalsy(); + expect(sess!.statistics.counters.roomEventEncryptionKeysReceived).toEqual(0); + }); + + it("tracks total age statistics for collected keys", () => { + jest.useFakeTimers(); + try { + const mockRoom = makeMockRoom([membershipTemplate]); + sess = MatrixRTCSession.roomSessionForRoom(client, mockRoom); + + // defaults to getTs() + jest.setSystemTime(1000); + sess.onCallEncryption({ + getType: jest.fn().mockReturnValue("io.element.call.encryption_keys"), + getContent: jest.fn().mockReturnValue({ + device_id: "bobsphone", + call_id: "", + keys: [ + { + index: 0, + key: "dGhpcyBpcyB0aGUga2V5", + }, + ], + }), + getSender: jest.fn().mockReturnValue("@bob:example.org"), + getTs: jest.fn().mockReturnValue(0), + } as unknown as MatrixEvent); + expect(sess!.statistics.totals.roomEventEncryptionKeysReceivedTotalAge).toEqual(1000); + + jest.setSystemTime(2000); + sess.onCallEncryption({ + getType: jest.fn().mockReturnValue("io.element.call.encryption_keys"), + getContent: jest.fn().mockReturnValue({ + device_id: "bobsphone", + call_id: "", + keys: [ + { + index: 0, + key: "dGhpcyBpcyB0aGUga2V5", + }, + ], + sent_ts: 0, + }), + getSender: jest.fn().mockReturnValue("@bob:example.org"), + getTs: jest.fn().mockReturnValue(Date.now()), + } as unknown as MatrixEvent); + expect(sess!.statistics.totals.roomEventEncryptionKeysReceivedTotalAge).toEqual(3000); + + jest.setSystemTime(3000); + sess.onCallEncryption({ + getType: jest.fn().mockReturnValue("io.element.call.encryption_keys"), + getContent: jest.fn().mockReturnValue({ + device_id: "bobsphone", + call_id: "", + keys: [ + { + index: 0, + key: "dGhpcyBpcyB0aGUga2V5", + }, + ], + sent_ts: 1000, + }), + getSender: jest.fn().mockReturnValue("@bob:example.org"), + getTs: jest.fn().mockReturnValue(Date.now()), + } as unknown as MatrixEvent); + expect(sess!.statistics.totals.roomEventEncryptionKeysReceivedTotalAge).toEqual(5000); + } finally { + jest.useRealTimers(); + } }); }); diff --git a/src/matrixrtc/MatrixRTCSession.ts b/src/matrixrtc/MatrixRTCSession.ts index ebd69400a90..2ec2150ebce 100644 --- a/src/matrixrtc/MatrixRTCSession.ts +++ b/src/matrixrtc/MatrixRTCSession.ts @@ -144,6 +144,30 @@ export class MatrixRTCSession extends TypedEventEmitter | undefined; + private currentEncryptionKeyIndex = -1; + + /** + * The statistics for this session. + */ + public statistics = { + counters: { + /** + * The number of times we have sent a room event containing encryption keys. + */ + roomEventEncryptionKeysSent: 0, + /** + * The number of times we have received a room event containing encryption keys. + */ + roomEventEncryptionKeysReceived: 0, + }, + totals: { + /** + * The total age (in milliseconds) of all room events containing encryption keys that we have received. + */ + roomEventEncryptionKeysReceivedTotalAge: 0, + }, + }; + /** * The callId (sessionId) of the call. * @@ -467,10 +491,16 @@ export class MatrixRTCSession extends TypedEventEmitter { this.setNewKeyTimeouts.delete(useKeyTimeout); logger.info(`Delayed-emitting key changed event for ${participantId} idx ${encryptionKeyIndex}`); + if (userId === this.client.getUserId() && deviceId === this.client.getDeviceId()) { + this.currentEncryptionKeyIndex = encryptionKeyIndex; + } this.emit(MatrixRTCSessionEvent.EncryptionKeyChanged, keyBin, encryptionKeyIndex, participantId); }, USE_KEY_DELAY); this.setNewKeyTimeouts.add(useKeyTimeout); } else { + if (userId === this.client.getUserId() && deviceId === this.client.getDeviceId()) { + this.currentEncryptionKeyIndex = encryptionKeyIndex; + } this.emit(MatrixRTCSessionEvent.EncryptionKeyChanged, keyBin, encryptionKeyIndex, participantId); } } @@ -479,8 +509,9 @@ export class MatrixRTCSession extends TypedEventEmitter => { + private sendEncryptionKeysEvent = async (indexToSend?: number): Promise => { if (this.keysEventUpdateTimeout !== undefined) { clearTimeout(this.keysEventUpdateTimeout); this.keysEventUpdateTimeout = undefined; } this.lastEncryptionKeyUpdateRequest = Date.now(); - logger.info("Sending encryption keys event"); - if (!this.isJoined()) return; + logger.info(`Sending encryption keys event. indexToSend=${indexToSend}`); + const userId = this.client.getUserId(); const deviceId = this.client.getDeviceId(); @@ -541,20 +573,33 @@ export class MatrixRTCSession extends TypedEventEmitter { - return { - index, - key: encodeUnpaddedBase64(key), - }; - }), + const content: EncryptionKeysEventContent = { + keys: [ + { + index: keyIndexToSend, + key: encodeUnpaddedBase64(keyToSend), + }, + ], device_id: deviceId, call_id: "", - } as EncryptionKeysEventContent); + sent_ts: Date.now(), + }; + + this.statistics.counters.roomEventEncryptionKeysSent += 1; + + await this.client.sendEvent(this.room.roomId, EventType.CallEncryptionKeysPrefix, content); logger.debug( - `Embedded-E2EE-LOG updateEncryptionKeyEvent participantId=${userId}:${deviceId} numSent=${myKeys.length}`, + `Embedded-E2EE-LOG updateEncryptionKeyEvent participantId=${userId}:${deviceId} numKeys=${myKeys.length} currentKeyIndex=${this.currentEncryptionKeyIndex} keyIndexToSend=${keyIndexToSend}`, this.encryptionKeys, ); } catch (error) { @@ -649,6 +694,10 @@ export class MatrixRTCSession extends TypedEventEmitter Date: Wed, 18 Sep 2024 09:51:05 +0100 Subject: [PATCH 2/3] Update src/matrixrtc/MatrixRTCSession.ts Co-authored-by: Robin --- src/matrixrtc/MatrixRTCSession.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/matrixrtc/MatrixRTCSession.ts b/src/matrixrtc/MatrixRTCSession.ts index 2ec2150ebce..856a4cf55dc 100644 --- a/src/matrixrtc/MatrixRTCSession.ts +++ b/src/matrixrtc/MatrixRTCSession.ts @@ -578,7 +578,7 @@ export class MatrixRTCSession extends TypedEventEmitter Date: Wed, 18 Sep 2024 09:52:32 +0100 Subject: [PATCH 3/3] Add comment about why we track total age of events --- src/matrixrtc/MatrixRTCSession.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/matrixrtc/MatrixRTCSession.ts b/src/matrixrtc/MatrixRTCSession.ts index 856a4cf55dc..1cf486062f7 100644 --- a/src/matrixrtc/MatrixRTCSession.ts +++ b/src/matrixrtc/MatrixRTCSession.ts @@ -163,6 +163,7 @@ export class MatrixRTCSession extends TypedEventEmitter