From 3bf67d50b77c847440d0742eff719c8f7cd5f329 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 23 Sep 2024 17:18:23 +0100 Subject: [PATCH 1/5] RTCSession cleanup: deprecate getKeysForParticipant() and getEncryption(); add emitEncryptionKeys() --- spec/unit/matrixrtc/MatrixRTCSession.spec.ts | 107 +++++++++++++------ src/matrixrtc/MatrixRTCSession.ts | 26 ++++- 2 files changed, 102 insertions(+), 31 deletions(-) diff --git a/spec/unit/matrixrtc/MatrixRTCSession.spec.ts b/spec/unit/matrixrtc/MatrixRTCSession.spec.ts index 31614010396..d29491418a2 100644 --- a/spec/unit/matrixrtc/MatrixRTCSession.spec.ts +++ b/spec/unit/matrixrtc/MatrixRTCSession.spec.ts @@ -585,12 +585,15 @@ describe("MatrixRTCSession", () => { it("creates a key when joining", () => { sess!.joinRoomSession([mockFocus], mockFocus, { manageMediaKeys: true }); - const keys = sess?.getKeysForParticipant("@alice:example.org", "AAAAAAA"); - expect(keys).toHaveLength(1); - - const allKeys = sess!.getEncryptionKeys(); - expect(allKeys).toBeTruthy(); - expect(Array.from(allKeys)).toHaveLength(1); + const encryptionKeyChangedListener = jest.fn(); + sess!.on(MatrixRTCSessionEvent.EncryptionKeyChanged, encryptionKeyChangedListener); + sess?.emitEncryptionKeys(); + expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1); + expect(encryptionKeyChangedListener).toHaveBeenCalledWith( + expect.any(Uint8Array), + 0, + "@alice:example.org:AAAAAAA", + ); }); it("sends keys when joining", async () => { @@ -1197,9 +1200,16 @@ describe("MatrixRTCSession", () => { getTs: jest.fn().mockReturnValue(Date.now()), } as unknown as MatrixEvent); - const bobKeys = sess.getKeysForParticipant("@bob:example.org", "bobsphone")!; - expect(bobKeys).toHaveLength(1); - expect(bobKeys[0]).toEqual(Buffer.from("this is the key", "utf-8")); + const encryptionKeyChangedListener = jest.fn(); + sess!.on(MatrixRTCSessionEvent.EncryptionKeyChanged, encryptionKeyChangedListener); + sess!.emitEncryptionKeys(); + expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1); + expect(encryptionKeyChangedListener).toHaveBeenCalledWith( + Buffer.from("this is the key", "utf-8"), + 0, + "@bob:example.org:bobsphone", + ); + expect(sess!.statistics.counters.roomEventEncryptionKeysReceived).toEqual(1); }); @@ -1222,13 +1232,16 @@ describe("MatrixRTCSession", () => { getTs: jest.fn().mockReturnValue(Date.now()), } as unknown as MatrixEvent); - const bobKeys = sess.getKeysForParticipant("@bob:example.org", "bobsphone")!; - expect(bobKeys).toHaveLength(5); - expect(bobKeys[0]).toBeFalsy(); - expect(bobKeys[1]).toBeFalsy(); - expect(bobKeys[2]).toBeFalsy(); - expect(bobKeys[3]).toBeFalsy(); - expect(bobKeys[4]).toEqual(Buffer.from("this is the key", "utf-8")); + const encryptionKeyChangedListener = jest.fn(); + sess!.on(MatrixRTCSessionEvent.EncryptionKeyChanged, encryptionKeyChangedListener); + sess!.emitEncryptionKeys(); + expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1); + expect(encryptionKeyChangedListener).toHaveBeenCalledWith( + Buffer.from("this is the key", "utf-8"), + 4, + "@bob:example.org:bobsphone", + ); + expect(sess!.statistics.counters.roomEventEncryptionKeysReceived).toEqual(1); }); @@ -1251,9 +1264,16 @@ describe("MatrixRTCSession", () => { getTs: jest.fn().mockReturnValue(Date.now()), } as unknown as MatrixEvent); - let bobKeys = sess.getKeysForParticipant("@bob:example.org", "bobsphone")!; - expect(bobKeys).toHaveLength(1); - expect(bobKeys[0]).toEqual(Buffer.from("this is the key", "utf-8")); + const encryptionKeyChangedListener = jest.fn(); + sess!.on(MatrixRTCSessionEvent.EncryptionKeyChanged, encryptionKeyChangedListener); + sess!.emitEncryptionKeys(); + expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1); + expect(encryptionKeyChangedListener).toHaveBeenCalledWith( + Buffer.from("this is the key", "utf-8"), + 0, + "@bob:example.org:bobsphone", + ); + expect(sess!.statistics.counters.roomEventEncryptionKeysReceived).toEqual(1); sess.onCallEncryption({ @@ -1272,9 +1292,20 @@ describe("MatrixRTCSession", () => { getTs: jest.fn().mockReturnValue(Date.now()), } as unknown as MatrixEvent); - bobKeys = sess.getKeysForParticipant("@bob:example.org", "bobsphone")!; - expect(bobKeys).toHaveLength(5); - expect(bobKeys[4]).toEqual(Buffer.from("this is the key", "utf-8")); + encryptionKeyChangedListener.mockClear(); + sess!.emitEncryptionKeys(); + expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(2); + expect(encryptionKeyChangedListener).toHaveBeenCalledWith( + Buffer.from("this is the key", "utf-8"), + 0, + "@bob:example.org:bobsphone", + ); + expect(encryptionKeyChangedListener).toHaveBeenCalledWith( + Buffer.from("this is the key", "utf-8"), + 4, + "@bob:example.org:bobsphone", + ); + expect(sess!.statistics.counters.roomEventEncryptionKeysReceived).toEqual(2); }); @@ -1313,9 +1344,16 @@ describe("MatrixRTCSession", () => { getTs: jest.fn().mockReturnValue(1000), // earlier timestamp than the newer key } as unknown as MatrixEvent); - const bobKeys = sess.getKeysForParticipant("@bob:example.org", "bobsphone")!; - expect(bobKeys).toHaveLength(1); - expect(bobKeys[0]).toEqual(Buffer.from("newer key", "utf-8")); + const encryptionKeyChangedListener = jest.fn(); + sess!.on(MatrixRTCSessionEvent.EncryptionKeyChanged, encryptionKeyChangedListener); + sess!.emitEncryptionKeys(); + expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1); + expect(encryptionKeyChangedListener).toHaveBeenCalledWith( + Buffer.from("newer key", "utf-8"), + 0, + "@bob:example.org:bobsphone", + ); + expect(sess!.statistics.counters.roomEventEncryptionKeysReceived).toEqual(2); }); @@ -1354,9 +1392,15 @@ describe("MatrixRTCSession", () => { getTs: jest.fn().mockReturnValue(1000), // same timestamp as the first key } as unknown as MatrixEvent); - const bobKeys = sess.getKeysForParticipant("@bob:example.org", "bobsphone")!; - expect(bobKeys).toHaveLength(1); - expect(bobKeys[0]).toEqual(Buffer.from("second key", "utf-8")); + const encryptionKeyChangedListener = jest.fn(); + sess!.on(MatrixRTCSessionEvent.EncryptionKeyChanged, encryptionKeyChangedListener); + sess!.emitEncryptionKeys(); + expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1); + expect(encryptionKeyChangedListener).toHaveBeenCalledWith( + Buffer.from("second key", "utf-8"), + 0, + "@bob:example.org:bobsphone", + ); }); it("ignores keys event for the local participant", () => { @@ -1378,8 +1422,11 @@ describe("MatrixRTCSession", () => { getTs: jest.fn().mockReturnValue(Date.now()), } as unknown as MatrixEvent); - const myKeys = sess.getKeysForParticipant(client.getUserId()!, client.getDeviceId()!)!; - expect(myKeys).toBeFalsy(); + const encryptionKeyChangedListener = jest.fn(); + sess!.on(MatrixRTCSessionEvent.EncryptionKeyChanged, encryptionKeyChangedListener); + sess!.emitEncryptionKeys(); + expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(0); + expect(sess!.statistics.counters.roomEventEncryptionKeysReceived).toEqual(0); }); diff --git a/src/matrixrtc/MatrixRTCSession.ts b/src/matrixrtc/MatrixRTCSession.ts index 1cf486062f7..ea1473e933d 100644 --- a/src/matrixrtc/MatrixRTCSession.ts +++ b/src/matrixrtc/MatrixRTCSession.ts @@ -405,22 +405,46 @@ export class MatrixRTCSession extends TypedEventEmitter { + keys.forEach((key, index) => { + this.emit(MatrixRTCSessionEvent.EncryptionKeyChanged, key.key, index, participantId); + }); + }); + } + /** * Get the known encryption keys for a given participant device. * * @param userId the user ID of the participant * @param deviceId the device ID of the participant * @returns The encryption keys for the given participant, or undefined if they are not known. + * + * @deprecated This will be made private in a future release. */ public getKeysForParticipant(userId: string, deviceId: string): Array | undefined { + return this.getKeysForParticipantInternal(userId, deviceId); + } + + private getKeysForParticipantInternal(userId: string, deviceId: string): Array | undefined { return this.encryptionKeys.get(getParticipantId(userId, deviceId))?.map((entry) => entry.key); } /** * A map of keys used to encrypt and decrypt (we are using a symmetric * cipher) given participant's media. This also includes our own key + * + * @deprecated This will be made private in a future release. */ public getEncryptionKeys(): IterableIterator<[string, Array]> { + // the returned array doesn't contain the timestamps + return this.getEncryptionKeysInternal(); + } + + private getEncryptionKeysInternal(): IterableIterator<[string, Array]> { // the returned array doesn't contain the timestamps return Array.from(this.encryptionKeys.entries()) .map(([participantId, keys]): [string, Uint8Array[]] => [participantId, keys.map((k) => k.key)]) @@ -434,7 +458,7 @@ export class MatrixRTCSession extends TypedEventEmitter Date: Mon, 23 Sep 2024 17:20:25 +0100 Subject: [PATCH 2/5] Clarify comment --- 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 ea1473e933d..498eb1750b6 100644 --- a/src/matrixrtc/MatrixRTCSession.ts +++ b/src/matrixrtc/MatrixRTCSession.ts @@ -406,7 +406,7 @@ export class MatrixRTCSession extends TypedEventEmitter { From e73c660066b7437aace4feef913a0dcbcb891d79 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 27 Sep 2024 14:31:13 +0100 Subject: [PATCH 3/5] Feedback from code review --- src/matrixrtc/MatrixRTCSession.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/matrixrtc/MatrixRTCSession.ts b/src/matrixrtc/MatrixRTCSession.ts index 498eb1750b6..4f4b218f248 100644 --- a/src/matrixrtc/MatrixRTCSession.ts +++ b/src/matrixrtc/MatrixRTCSession.ts @@ -406,9 +406,10 @@ export class MatrixRTCSession extends TypedEventEmitter { keys.forEach((key, index) => { this.emit(MatrixRTCSessionEvent.EncryptionKeyChanged, key.key, index, participantId); @@ -440,11 +441,6 @@ export class MatrixRTCSession extends TypedEventEmitter]> { - // the returned array doesn't contain the timestamps - return this.getEncryptionKeysInternal(); - } - - private getEncryptionKeysInternal(): IterableIterator<[string, Array]> { // the returned array doesn't contain the timestamps return Array.from(this.encryptionKeys.entries()) .map(([participantId, keys]): [string, Uint8Array[]] => [participantId, keys.map((k) => k.key)]) From 7f62654938f2aa1907f1f50460480cf5ae397063 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 27 Sep 2024 15:50:25 +0100 Subject: [PATCH 4/5] Update src/matrixrtc/MatrixRTCSession.ts Co-authored-by: Andrew Ferrazzutti --- 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 4f4b218f248..eab4b919770 100644 --- a/src/matrixrtc/MatrixRTCSession.ts +++ b/src/matrixrtc/MatrixRTCSession.ts @@ -406,7 +406,7 @@ export class MatrixRTCSession extends TypedEventEmitter Date: Fri, 27 Sep 2024 15:53:19 +0100 Subject: [PATCH 5/5] Fix test --- spec/unit/matrixrtc/MatrixRTCSession.spec.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/unit/matrixrtc/MatrixRTCSession.spec.ts b/spec/unit/matrixrtc/MatrixRTCSession.spec.ts index d29491418a2..4599066c748 100644 --- a/spec/unit/matrixrtc/MatrixRTCSession.spec.ts +++ b/spec/unit/matrixrtc/MatrixRTCSession.spec.ts @@ -587,7 +587,7 @@ describe("MatrixRTCSession", () => { sess!.joinRoomSession([mockFocus], mockFocus, { manageMediaKeys: true }); const encryptionKeyChangedListener = jest.fn(); sess!.on(MatrixRTCSessionEvent.EncryptionKeyChanged, encryptionKeyChangedListener); - sess?.emitEncryptionKeys(); + sess?.reemitEncryptionKeys(); expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1); expect(encryptionKeyChangedListener).toHaveBeenCalledWith( expect.any(Uint8Array), @@ -1202,7 +1202,7 @@ describe("MatrixRTCSession", () => { const encryptionKeyChangedListener = jest.fn(); sess!.on(MatrixRTCSessionEvent.EncryptionKeyChanged, encryptionKeyChangedListener); - sess!.emitEncryptionKeys(); + sess!.reemitEncryptionKeys(); expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1); expect(encryptionKeyChangedListener).toHaveBeenCalledWith( Buffer.from("this is the key", "utf-8"), @@ -1234,7 +1234,7 @@ describe("MatrixRTCSession", () => { const encryptionKeyChangedListener = jest.fn(); sess!.on(MatrixRTCSessionEvent.EncryptionKeyChanged, encryptionKeyChangedListener); - sess!.emitEncryptionKeys(); + sess!.reemitEncryptionKeys(); expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1); expect(encryptionKeyChangedListener).toHaveBeenCalledWith( Buffer.from("this is the key", "utf-8"), @@ -1266,7 +1266,7 @@ describe("MatrixRTCSession", () => { const encryptionKeyChangedListener = jest.fn(); sess!.on(MatrixRTCSessionEvent.EncryptionKeyChanged, encryptionKeyChangedListener); - sess!.emitEncryptionKeys(); + sess!.reemitEncryptionKeys(); expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1); expect(encryptionKeyChangedListener).toHaveBeenCalledWith( Buffer.from("this is the key", "utf-8"), @@ -1293,7 +1293,7 @@ describe("MatrixRTCSession", () => { } as unknown as MatrixEvent); encryptionKeyChangedListener.mockClear(); - sess!.emitEncryptionKeys(); + sess!.reemitEncryptionKeys(); expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(2); expect(encryptionKeyChangedListener).toHaveBeenCalledWith( Buffer.from("this is the key", "utf-8"), @@ -1346,7 +1346,7 @@ describe("MatrixRTCSession", () => { const encryptionKeyChangedListener = jest.fn(); sess!.on(MatrixRTCSessionEvent.EncryptionKeyChanged, encryptionKeyChangedListener); - sess!.emitEncryptionKeys(); + sess!.reemitEncryptionKeys(); expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1); expect(encryptionKeyChangedListener).toHaveBeenCalledWith( Buffer.from("newer key", "utf-8"), @@ -1394,7 +1394,7 @@ describe("MatrixRTCSession", () => { const encryptionKeyChangedListener = jest.fn(); sess!.on(MatrixRTCSessionEvent.EncryptionKeyChanged, encryptionKeyChangedListener); - sess!.emitEncryptionKeys(); + sess!.reemitEncryptionKeys(); expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1); expect(encryptionKeyChangedListener).toHaveBeenCalledWith( Buffer.from("second key", "utf-8"), @@ -1424,7 +1424,7 @@ describe("MatrixRTCSession", () => { const encryptionKeyChangedListener = jest.fn(); sess!.on(MatrixRTCSessionEvent.EncryptionKeyChanged, encryptionKeyChangedListener); - sess!.emitEncryptionKeys(); + sess!.reemitEncryptionKeys(); expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(0); expect(sess!.statistics.counters.roomEventEncryptionKeysReceived).toEqual(0);