From 081c1c232f94bb3771a1a46b814377acd3e9ee97 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 29 Sep 2023 16:48:07 +0100 Subject: [PATCH 01/10] Make MSC3906 implementation compatible with Rust Crypto --- spec/unit/rendezvous/rendezvous.spec.ts | 202 +++++++++++++++--------- src/rendezvous/MSC3906Rendezvous.ts | 49 +++--- 2 files changed, 154 insertions(+), 97 deletions(-) diff --git a/spec/unit/rendezvous/rendezvous.spec.ts b/spec/unit/rendezvous/rendezvous.spec.ts index eb8a72c26ae..04a504b3968 100644 --- a/spec/unit/rendezvous/rendezvous.spec.ts +++ b/spec/unit/rendezvous/rendezvous.spec.ts @@ -23,7 +23,7 @@ import { MSC3903ECDHPayload, MSC3903ECDHv2RendezvousChannel as MSC3903ECDHRendezvousChannel, } from "../../../src/rendezvous/channels"; -import { MatrixClient } from "../../../src"; +import { Device, MatrixClient } from "../../../src"; import { MSC3886SimpleHttpRendezvousTransport, MSC3886SimpleHttpRendezvousTransportDetails, @@ -31,16 +31,59 @@ import { import { DummyTransport } from "./DummyTransport"; import { decodeBase64 } from "../../../src/base64"; import { logger } from "../../../src/logger"; -import { DeviceInfo } from "../../../src/crypto/deviceinfo"; +import { CrossSigningKey } from "../../../src/crypto-api"; + +type UserID = string; +type DeviceID = string; +type Fingerprint = string; +type PartialUserDevices = Map>; +type PartialDeviceMap = Map; +type SimpleDeviceMap = Record>; + +function mockDevice(userId: UserID, deviceId: DeviceID, fingerprint: Fingerprint): Partial { + return { + deviceId, + userId, + getFingerprint: () => fingerprint, + }; +} + +function mockDeviceMap( + userId: UserID, + deviceId: DeviceID, + deviceKey?: Fingerprint, + otherDevices: SimpleDeviceMap = {}, +): PartialDeviceMap { + const deviceMap: PartialDeviceMap = new Map(); + + const myDevices: PartialUserDevices = new Map(); + if (deviceKey) { + myDevices.set(deviceId, mockDevice(userId, deviceId, deviceKey)); + } + deviceMap.set(userId, myDevices); + + for (const u in otherDevices) { + let userDevices = deviceMap.get(u); + if (!userDevices) { + userDevices = new Map(); + deviceMap.set(u, userDevices); + } + for (const d in otherDevices[u]) { + userDevices.set(d, mockDevice(u, d, otherDevices[u][d])); + } + } + + return deviceMap; +} function makeMockClient(opts: { - userId: string; - deviceId: string; - deviceKey?: string; + userId: UserID; + deviceId: DeviceID; + deviceKey?: Fingerprint; getLoginTokenEnabled: boolean; msc3882r0Only: boolean; msc3886Enabled: boolean; - devices?: Record>; + devices?: SimpleDeviceMap; verificationFunction?: ( userId: string, deviceId: string, @@ -48,50 +91,56 @@ function makeMockClient(opts: { blocked: boolean, known: boolean, ) => void; - crossSigningIds?: Record; -}): MatrixClient { - return { - getVersions() { - return { - unstable_features: { - "org.matrix.msc3882": opts.getLoginTokenEnabled, - "org.matrix.msc3886": opts.msc3886Enabled, - }, - }; - }, - getCapabilities() { - return opts.msc3882r0Only - ? {} - : { - capabilities: { - "m.get_login_token": { - enabled: opts.getLoginTokenEnabled, + crossSigningIds?: Partial>; +}): [MatrixClient, PartialDeviceMap] { + const deviceMap = mockDeviceMap(opts.userId, opts.deviceId, opts.deviceKey, opts.devices); + return [ + { + getVersions() { + return { + unstable_features: { + "org.matrix.msc3882": opts.getLoginTokenEnabled, + "org.matrix.msc3886": opts.msc3886Enabled, + }, + }; + }, + getCapabilities() { + return opts.msc3882r0Only + ? {} + : { + capabilities: { + "m.get_login_token": { + enabled: opts.getLoginTokenEnabled, + }, }, - }, - }; - }, - getUserId() { - return opts.userId; - }, - getDeviceId() { - return opts.deviceId; - }, - getDeviceEd25519Key() { - return opts.deviceKey; - }, - baseUrl: "https://example.com", - crypto: { - getStoredDevice(userId: string, deviceId: string) { - return opts.devices?.[deviceId] ?? null; + }; }, - setDeviceVerification: opts.verificationFunction, - crossSigningInfo: { - getId(key: string) { - return opts.crossSigningIds?.[key]; - }, + getUserId() { + return opts.userId; + }, + getDeviceId() { + return opts.deviceId; + }, + getDeviceEd25519Key() { + return opts.deviceKey; }, - }, - } as unknown as MatrixClient; + baseUrl: "https://example.com", + getCrypto() { + return { + getUserDeviceInfo([userId]: string[], deviceId: string): Promise { + return Promise.resolve(deviceMap); + }, + getCrossSigningKeyId(key: CrossSigningKey): string | null { + return opts.crossSigningIds?.[key] ?? null; + }, + }; + }, + crypto: { + setDeviceVerification: opts.verificationFunction, + }, + } as unknown as MatrixClient, + deviceMap, + ]; } function makeTransport(name: string, uri = "https://test.rz/123456") { @@ -106,6 +155,7 @@ describe("Rendezvous", function () { let httpBackend: MockHttpBackend; let fetchFn: typeof global.fetch; let transports: DummyTransport[]; + const userId: UserID = "@user:example.com"; beforeEach(function () { httpBackend = new MockHttpBackend(); @@ -118,9 +168,9 @@ describe("Rendezvous", function () { }); it("generate and cancel", async function () { - const alice = makeMockClient({ - userId: "@alice:example.com", - deviceId: "DEVICEID", + const [alice] = makeMockClient({ + userId, + deviceId: "ALICE", msc3886Enabled: false, getLoginTokenEnabled: true, msc3882r0Only: true, @@ -194,8 +244,8 @@ describe("Rendezvous", function () { // alice is already signs in and generates a code const aliceOnFailure = jest.fn(); - const alice = makeMockClient({ - userId: "alice", + const [alice] = makeMockClient({ + userId, deviceId: "ALICE", msc3886Enabled: false, getLoginTokenEnabled, @@ -257,8 +307,8 @@ describe("Rendezvous", function () { // alice is already signs in and generates a code const aliceOnFailure = jest.fn(); - const alice = makeMockClient({ - userId: "alice", + const [alice] = makeMockClient({ + userId, deviceId: "ALICE", getLoginTokenEnabled: true, msc3882r0Only: false, @@ -316,8 +366,8 @@ describe("Rendezvous", function () { // alice is already signs in and generates a code const aliceOnFailure = jest.fn(); - const alice = makeMockClient({ - userId: "alice", + const [alice] = makeMockClient({ + userId, deviceId: "ALICE", getLoginTokenEnabled: true, msc3882r0Only: false, @@ -375,7 +425,7 @@ describe("Rendezvous", function () { // alice is already signs in and generates a code const aliceOnFailure = jest.fn(); - const alice = makeMockClient({ + const [alice] = makeMockClient({ userId: "alice", deviceId: "ALICE", getLoginTokenEnabled: true, @@ -436,7 +486,7 @@ describe("Rendezvous", function () { // alice is already signs in and generates a code const aliceOnFailure = jest.fn(); - const alice = makeMockClient({ + const [alice] = makeMockClient({ userId: "alice", deviceId: "ALICE", getLoginTokenEnabled: true, @@ -495,7 +545,7 @@ describe("Rendezvous", function () { await bobCompleteProm; }); - async function completeLogin(devices: Record>) { + async function completeLogin(devices: SimpleDeviceMap) { const aliceTransport = makeTransport("Alice", "https://test.rz/123456"); const bobTransport = makeTransport("Bob", "https://test.rz/999999"); transports.push(aliceTransport, bobTransport); @@ -505,8 +555,8 @@ describe("Rendezvous", function () { // alice is already signs in and generates a code const aliceOnFailure = jest.fn(); const aliceVerification = jest.fn(); - const alice = makeMockClient({ - userId: "alice", + const [alice, deviceMap] = makeMockClient({ + userId, deviceId: "ALICE", getLoginTokenEnabled: true, msc3882r0Only: false, @@ -575,13 +625,15 @@ describe("Rendezvous", function () { aliceRz, bobTransport, bobEcdh, + devices, + deviceMap, }; } it("approve on existing device + verification", async function () { const { bobEcdh, aliceRz } = await completeLogin({ - BOB: { - getFingerprint: () => "bbbb", + [userId]: { + BOB: "bbbb", }, }); const verifyProm = aliceRz.verifyNewDeviceOnExistingDevice(); @@ -607,33 +659,29 @@ describe("Rendezvous", function () { }); it("device appears online within timeout", async function () { - const devices: Record> = {}; - const { aliceRz } = await completeLogin(devices); - // device appears after 1 second + const devices: SimpleDeviceMap = {}; + const { aliceRz, deviceMap } = await completeLogin(devices); + // device appears before the timeout setTimeout(() => { - devices.BOB = { - getFingerprint: () => "bbbb", - }; + deviceMap.get(userId)?.set("BOB", mockDevice(userId, "BOB", "bbbb")); }, 1000); await aliceRz.verifyNewDeviceOnExistingDevice(2000); }); it("device appears online after timeout", async function () { - const devices: Record> = {}; - const { aliceRz } = await completeLogin(devices); - // device appears after 1 second + const devices: SimpleDeviceMap = {}; + const { aliceRz, deviceMap } = await completeLogin(devices); + // device appears after the timeout setTimeout(() => { - devices.BOB = { - getFingerprint: () => "bbbb", - }; + deviceMap.get(userId)?.set("BOB", mockDevice(userId, "BOB", "bbbb")); }, 1500); await expect(aliceRz.verifyNewDeviceOnExistingDevice(1000)).rejects.toThrow(); }); it("mismatched device key", async function () { const { aliceRz } = await completeLogin({ - BOB: { - getFingerprint: () => "XXXX", + [userId]: { + BOB: "XXXX", }, }); await expect(aliceRz.verifyNewDeviceOnExistingDevice(1000)).rejects.toThrow(/different key/); diff --git a/src/rendezvous/MSC3906Rendezvous.ts b/src/rendezvous/MSC3906Rendezvous.ts index 4a92bbd7797..21331272cff 100644 --- a/src/rendezvous/MSC3906Rendezvous.ts +++ b/src/rendezvous/MSC3906Rendezvous.ts @@ -17,12 +17,12 @@ limitations under the License. import { UnstableValue } from "matrix-events-sdk"; import { RendezvousChannel, RendezvousFailureListener, RendezvousFailureReason, RendezvousIntent } from "."; -import { ICrossSigningKey, IGetLoginTokenCapability, MatrixClient, GET_LOGIN_TOKEN_CAPABILITY } from "../client"; -import { CrossSigningInfo } from "../crypto/CrossSigning"; -import { DeviceInfo } from "../crypto/deviceinfo"; +import { IGetLoginTokenCapability, MatrixClient, GET_LOGIN_TOKEN_CAPABILITY } from "../client"; import { buildFeatureSupportMap, Feature, ServerSupport } from "../feature"; import { logger } from "../logger"; import { sleep } from "../utils"; +import { CrossSigningKey } from "../crypto-api"; +import { Device } from "../matrix"; enum PayloadType { Start = "m.login.start", @@ -178,10 +178,9 @@ export class MSC3906Rendezvous { return deviceId; } - private async verifyAndCrossSignDevice( - deviceInfo: DeviceInfo, - ): Promise { - if (!this.client.crypto) { + private async verifyAndCrossSignDevice(deviceInfo: Device): Promise { + const crypto = this.client.getCrypto(); + if (!crypto) { throw new Error("Crypto not available on client"); } @@ -203,19 +202,21 @@ export class MSC3906Rendezvous { } // mark the device as verified locally + cross sign logger.info(`Marking device ${this.newDeviceId} as verified`); - const info = await this.client.crypto.setDeviceVerification(userId, this.newDeviceId, true, false, true); + // TODO: this function isn't available with rust crypto backend + await this.client.crypto!.setDeviceVerification(userId, this.newDeviceId, true, false, true); - const masterPublicKey = this.client.crypto.crossSigningInfo.getId("master")!; + const masterPublicKey = (await crypto.getCrossSigningKeyId(CrossSigningKey.Master)) ?? undefined; + + const ourDeviceId = this.client.getDeviceId(); + const ourDevice = ourDeviceId ? await this.getOwnDevice(ourDeviceId) : undefined; await this.send({ type: PayloadType.Finish, outcome: Outcome.Verified, - verifying_device_id: this.client.getDeviceId()!, - verifying_device_key: this.client.getDeviceEd25519Key()!, + verifying_device_id: ourDevice?.deviceId, + verifying_device_key: ourDevice?.getFingerprint(), master_key: masterPublicKey, }); - - return info; } /** @@ -223,9 +224,7 @@ export class MSC3906Rendezvous { * @param timeout - time in milliseconds to wait for device to come online * @returns the new device info if the device was verified */ - public async verifyNewDeviceOnExistingDevice( - timeout = 10 * 1000, - ): Promise { + public async verifyNewDeviceOnExistingDevice(timeout = 10 * 1000): Promise { if (!this.newDeviceId) { throw new Error("No new device to sign"); } @@ -235,7 +234,8 @@ export class MSC3906Rendezvous { return undefined; } - if (!this.client.crypto) { + const crypto = this.client.getCrypto(); + if (!crypto) { throw new Error("Crypto not available on client"); } @@ -245,21 +245,30 @@ export class MSC3906Rendezvous { throw new Error("No user ID set"); } - let deviceInfo = this.client.crypto.getStoredDevice(userId, this.newDeviceId); + let deviceInfo = await this.getOwnDevice(this.newDeviceId); if (!deviceInfo) { logger.info("Going to wait for new device to be online"); await sleep(timeout); - deviceInfo = this.client.crypto.getStoredDevice(userId, this.newDeviceId); + deviceInfo = await this.getOwnDevice(this.newDeviceId); } if (deviceInfo) { - return await this.verifyAndCrossSignDevice(deviceInfo); + await this.verifyAndCrossSignDevice(deviceInfo); + return; } throw new Error("Device not online within timeout"); } + private async getOwnDevice(deviceId: string): Promise { + const userId = this.client.getUserId(); + if (!userId) { + return undefined; + } + return (await this.client.getCrypto()?.getUserDeviceInfo([userId], true))?.get(userId)?.get(deviceId); + } + public async cancel(reason: RendezvousFailureReason): Promise { this.onFailure?.(reason); await this.channel.cancel(reason); From c8790c339abcdbab66fee3f2b4f51c27c238ebb8 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Tue, 31 Oct 2023 12:31:41 +0000 Subject: [PATCH 02/10] Verify using CryptoApi but no cross-signing (yet) --- spec/unit/rendezvous/rendezvous.spec.ts | 6 +++--- src/rendezvous/MSC3906Rendezvous.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/unit/rendezvous/rendezvous.spec.ts b/spec/unit/rendezvous/rendezvous.spec.ts index 04a504b3968..6e72a5b2329 100644 --- a/spec/unit/rendezvous/rendezvous.spec.ts +++ b/spec/unit/rendezvous/rendezvous.spec.ts @@ -133,11 +133,11 @@ function makeMockClient(opts: { getCrossSigningKeyId(key: CrossSigningKey): string | null { return opts.crossSigningIds?.[key] ?? null; }, + setDeviceVerified(userId: string, deviceId: string, verified: boolean): Promise { + return Promise.resolve(); + }, }; }, - crypto: { - setDeviceVerification: opts.verificationFunction, - }, } as unknown as MatrixClient, deviceMap, ]; diff --git a/src/rendezvous/MSC3906Rendezvous.ts b/src/rendezvous/MSC3906Rendezvous.ts index 21331272cff..aae5f126e8b 100644 --- a/src/rendezvous/MSC3906Rendezvous.ts +++ b/src/rendezvous/MSC3906Rendezvous.ts @@ -116,7 +116,7 @@ export class MSC3906Rendezvous { await this.send({ type: PayloadType.Progress, protocols: [LOGIN_TOKEN_PROTOCOL.name] }); - logger.info("Waiting for other device to chose protocol"); + logger.info("Waiting for other device to choose protocol"); const { type, protocol, outcome } = await this.receive(); if (type === PayloadType.Finish) { @@ -202,8 +202,8 @@ export class MSC3906Rendezvous { } // mark the device as verified locally + cross sign logger.info(`Marking device ${this.newDeviceId} as verified`); - // TODO: this function isn't available with rust crypto backend - await this.client.crypto!.setDeviceVerification(userId, this.newDeviceId, true, false, true); + await crypto.setDeviceVerified(userId, this.newDeviceId, true); + // TODO: cross sign the device const masterPublicKey = (await crypto.getCrossSigningKeyId(CrossSigningKey.Master)) ?? undefined; From ed4f49bee06cb7de60e3a10e2ca769dd59db4d1a Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 29 Nov 2023 16:34:05 +0000 Subject: [PATCH 03/10] Use new crossSignDevice() function --- src/rendezvous/MSC3906Rendezvous.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rendezvous/MSC3906Rendezvous.ts b/src/rendezvous/MSC3906Rendezvous.ts index aae5f126e8b..49ab621b361 100644 --- a/src/rendezvous/MSC3906Rendezvous.ts +++ b/src/rendezvous/MSC3906Rendezvous.ts @@ -203,7 +203,7 @@ export class MSC3906Rendezvous { // mark the device as verified locally + cross sign logger.info(`Marking device ${this.newDeviceId} as verified`); await crypto.setDeviceVerified(userId, this.newDeviceId, true); - // TODO: cross sign the device + await crypto.crossSignDevice(this.newDeviceId); const masterPublicKey = (await crypto.getCrossSigningKeyId(CrossSigningKey.Master)) ?? undefined; From 5cc344f4c2f2fcaef29d845a63c46c8ae5d03d46 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 29 Nov 2023 17:57:51 +0000 Subject: [PATCH 04/10] Mock crossSignDevice() function --- spec/unit/rendezvous/rendezvous.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/unit/rendezvous/rendezvous.spec.ts b/spec/unit/rendezvous/rendezvous.spec.ts index 6e72a5b2329..52687518143 100644 --- a/spec/unit/rendezvous/rendezvous.spec.ts +++ b/spec/unit/rendezvous/rendezvous.spec.ts @@ -136,6 +136,9 @@ function makeMockClient(opts: { setDeviceVerified(userId: string, deviceId: string, verified: boolean): Promise { return Promise.resolve(); }, + crossSignDevice(deviceId: string): Promise { + return Promise.resolve(); + }, }; }, } as unknown as MatrixClient, From 87322a6d512266dbbc7431d34be3e49bce92486e Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 12 Jan 2024 09:00:25 +0000 Subject: [PATCH 05/10] Fix type of parameter in mock --- spec/unit/rendezvous/rendezvous.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/rendezvous/rendezvous.spec.ts b/spec/unit/rendezvous/rendezvous.spec.ts index 52687518143..6f0895ddabf 100644 --- a/spec/unit/rendezvous/rendezvous.spec.ts +++ b/spec/unit/rendezvous/rendezvous.spec.ts @@ -127,7 +127,7 @@ function makeMockClient(opts: { baseUrl: "https://example.com", getCrypto() { return { - getUserDeviceInfo([userId]: string[], deviceId: string): Promise { + getUserDeviceInfo([userId]: string[], downloadUncached?: boolean): Promise { return Promise.resolve(deviceMap); }, getCrossSigningKeyId(key: CrossSigningKey): string | null { From bce3112a6225591f32497fb996469cda1d0a5ebd Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 24 Jan 2024 09:47:34 +0100 Subject: [PATCH 06/10] review: cleaning --- spec/unit/rendezvous/rendezvous.spec.ts | 25 ++++++++++++++++++++----- src/rendezvous/MSC3906Rendezvous.ts | 21 ++++++++------------- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/spec/unit/rendezvous/rendezvous.spec.ts b/spec/unit/rendezvous/rendezvous.spec.ts index 6f0895ddabf..d09360e4e82 100644 --- a/spec/unit/rendezvous/rendezvous.spec.ts +++ b/spec/unit/rendezvous/rendezvous.spec.ts @@ -31,7 +31,7 @@ import { import { DummyTransport } from "./DummyTransport"; import { decodeBase64 } from "../../../src/base64"; import { logger } from "../../../src/logger"; -import { CrossSigningKey } from "../../../src/crypto-api"; +import { CrossSigningKey, OwnDeviceKeys } from "../../../src/crypto-api"; type UserID = string; type DeviceID = string; @@ -40,12 +40,12 @@ type PartialUserDevices = Map>; type PartialDeviceMap = Map; type SimpleDeviceMap = Record>; -function mockDevice(userId: UserID, deviceId: DeviceID, fingerprint: Fingerprint): Partial { +function mockDevice(userId: UserID, deviceId: DeviceID, fingerprint: Fingerprint): Device { return { deviceId, userId, getFingerprint: () => fingerprint, - }; + } as unknown as Device; } function mockDeviceMap( @@ -96,6 +96,15 @@ function makeMockClient(opts: { const deviceMap = mockDeviceMap(opts.userId, opts.deviceId, opts.deviceKey, opts.devices); return [ { + doesServerSupportUnstableFeature: jest.fn().mockImplementation((feature) => { + if (feature === "org.matrix.msc3886") { + return opts.msc3886Enabled; + } else if (feature === "org.matrix.msc3882") { + return opts.getLoginTokenEnabled; + } else { + return false; + } + }), getVersions() { return { unstable_features: { @@ -139,6 +148,12 @@ function makeMockClient(opts: { crossSignDevice(deviceId: string): Promise { return Promise.resolve(); }, + getOwnDeviceKeys(): Promise { + return Promise.resolve({ + ed25519: opts.deviceKey!, + curve25519: "aaaa", + }); + }, }; }, } as unknown as MatrixClient, @@ -666,7 +681,7 @@ describe("Rendezvous", function () { const { aliceRz, deviceMap } = await completeLogin(devices); // device appears before the timeout setTimeout(() => { - deviceMap.get(userId)?.set("BOB", mockDevice(userId, "BOB", "bbbb")); + deviceMap.get(userId)!.set("BOB", mockDevice(userId, "BOB", "bbbb")); }, 1000); await aliceRz.verifyNewDeviceOnExistingDevice(2000); }); @@ -676,7 +691,7 @@ describe("Rendezvous", function () { const { aliceRz, deviceMap } = await completeLogin(devices); // device appears after the timeout setTimeout(() => { - deviceMap.get(userId)?.set("BOB", mockDevice(userId, "BOB", "bbbb")); + deviceMap.get(userId)!.set("BOB", mockDevice(userId, "BOB", "bbbb")); }, 1500); await expect(aliceRz.verifyNewDeviceOnExistingDevice(1000)).rejects.toThrow(); }); diff --git a/src/rendezvous/MSC3906Rendezvous.ts b/src/rendezvous/MSC3906Rendezvous.ts index 49ab621b361..ce2000ebd70 100644 --- a/src/rendezvous/MSC3906Rendezvous.ts +++ b/src/rendezvous/MSC3906Rendezvous.ts @@ -179,10 +179,7 @@ export class MSC3906Rendezvous { } private async verifyAndCrossSignDevice(deviceInfo: Device): Promise { - const crypto = this.client.getCrypto(); - if (!crypto) { - throw new Error("Crypto not available on client"); - } + const crypto = this.client.getCrypto()!; if (!this.newDeviceId) { throw new Error("No new device ID set"); @@ -195,11 +192,8 @@ export class MSC3906Rendezvous { ); } - const userId = this.client.getUserId(); + const userId = this.client.getUserId()!; - if (!userId) { - throw new Error("No user ID set"); - } // mark the device as verified locally + cross sign logger.info(`Marking device ${this.newDeviceId} as verified`); await crypto.setDeviceVerified(userId, this.newDeviceId, true); @@ -207,14 +201,14 @@ export class MSC3906Rendezvous { const masterPublicKey = (await crypto.getCrossSigningKeyId(CrossSigningKey.Master)) ?? undefined; - const ourDeviceId = this.client.getDeviceId(); - const ourDevice = ourDeviceId ? await this.getOwnDevice(ourDeviceId) : undefined; + const ourDeviceId = this.client.getDeviceId()!; + const ourDeviceKey = (await this.client.getCrypto()!.getOwnDeviceKeys()).ed25519; await this.send({ type: PayloadType.Finish, outcome: Outcome.Verified, - verifying_device_id: ourDevice?.deviceId, - verifying_device_key: ourDevice?.getFingerprint(), + verifying_device_id: ourDeviceId, + verifying_device_key: ourDeviceKey, master_key: masterPublicKey, }); } @@ -266,7 +260,8 @@ export class MSC3906Rendezvous { if (!userId) { return undefined; } - return (await this.client.getCrypto()?.getUserDeviceInfo([userId], true))?.get(userId)?.get(deviceId); + const ownDeviceInfo = await this.client.getCrypto()!.getUserDeviceInfo([userId]); + return ownDeviceInfo.get(userId)?.get(deviceId); } public async cancel(reason: RendezvousFailureReason): Promise { From 2ee3f9ec1c6c115c2d60e751ee897250f4dcd47f Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 24 Jan 2024 10:38:17 +0100 Subject: [PATCH 07/10] review: Remove unneeded defensive coding --- src/rendezvous/MSC3906Rendezvous.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/rendezvous/MSC3906Rendezvous.ts b/src/rendezvous/MSC3906Rendezvous.ts index ce2000ebd70..fc706f4a885 100644 --- a/src/rendezvous/MSC3906Rendezvous.ts +++ b/src/rendezvous/MSC3906Rendezvous.ts @@ -233,12 +233,6 @@ export class MSC3906Rendezvous { throw new Error("Crypto not available on client"); } - const userId = this.client.getUserId(); - - if (!userId) { - throw new Error("No user ID set"); - } - let deviceInfo = await this.getOwnDevice(this.newDeviceId); if (!deviceInfo) { @@ -256,10 +250,7 @@ export class MSC3906Rendezvous { } private async getOwnDevice(deviceId: string): Promise { - const userId = this.client.getUserId(); - if (!userId) { - return undefined; - } + const userId = this.client.getUserId()!; const ownDeviceInfo = await this.client.getCrypto()!.getUserDeviceInfo([userId]); return ownDeviceInfo.get(userId)?.get(deviceId); } From 5eeab19c0b9f139b6b4bed02234639cb3dc55804 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 24 Jan 2024 11:05:51 +0100 Subject: [PATCH 08/10] review: fix outdated documentation --- src/rendezvous/MSC3906Rendezvous.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/rendezvous/MSC3906Rendezvous.ts b/src/rendezvous/MSC3906Rendezvous.ts index fc706f4a885..185eedfe280 100644 --- a/src/rendezvous/MSC3906Rendezvous.ts +++ b/src/rendezvous/MSC3906Rendezvous.ts @@ -216,7 +216,6 @@ export class MSC3906Rendezvous { /** * Verify the device and cross-sign it. * @param timeout - time in milliseconds to wait for device to come online - * @returns the new device info if the device was verified */ public async verifyNewDeviceOnExistingDevice(timeout = 10 * 1000): Promise { if (!this.newDeviceId) { From 0e9d3df63e261ec22bc359281b85f41d7196dbe2 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 30 Jan 2024 10:04:19 +0100 Subject: [PATCH 09/10] QR login: review, cleaning --- spec/unit/rendezvous/rendezvous.spec.ts | 22 +++++++++++----------- src/rendezvous/MSC3906Rendezvous.ts | 4 ++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/spec/unit/rendezvous/rendezvous.spec.ts b/spec/unit/rendezvous/rendezvous.spec.ts index d09360e4e82..c7a31b8a1ff 100644 --- a/spec/unit/rendezvous/rendezvous.spec.ts +++ b/spec/unit/rendezvous/rendezvous.spec.ts @@ -36,8 +36,6 @@ import { CrossSigningKey, OwnDeviceKeys } from "../../../src/crypto-api"; type UserID = string; type DeviceID = string; type Fingerprint = string; -type PartialUserDevices = Map>; -type PartialDeviceMap = Map; type SimpleDeviceMap = Record>; function mockDevice(userId: UserID, deviceId: DeviceID, fingerprint: Fingerprint): Device { @@ -53,10 +51,10 @@ function mockDeviceMap( deviceId: DeviceID, deviceKey?: Fingerprint, otherDevices: SimpleDeviceMap = {}, -): PartialDeviceMap { - const deviceMap: PartialDeviceMap = new Map(); +): Map> { + const deviceMap: Map> = new Map(); - const myDevices: PartialUserDevices = new Map(); + const myDevices: Map = new Map(); if (deviceKey) { myDevices.set(deviceId, mockDevice(userId, deviceId, deviceKey)); } @@ -92,7 +90,7 @@ function makeMockClient(opts: { known: boolean, ) => void; crossSigningIds?: Partial>; -}): [MatrixClient, PartialDeviceMap] { +}): [MatrixClient, Map>] { const deviceMap = mockDeviceMap(opts.userId, opts.deviceId, opts.deviceKey, opts.devices); return [ { @@ -127,16 +125,19 @@ function makeMockClient(opts: { getUserId() { return opts.userId; }, + getSafeUserId() { + return opts.userId; + }, getDeviceId() { return opts.deviceId; }, - getDeviceEd25519Key() { - return opts.deviceKey; - }, baseUrl: "https://example.com", getCrypto() { return { - getUserDeviceInfo([userId]: string[], downloadUncached?: boolean): Promise { + getUserDeviceInfo( + [userId]: string[], + downloadUncached?: boolean, + ): Promise>> { return Promise.resolve(deviceMap); }, getCrossSigningKeyId(key: CrossSigningKey): string | null { @@ -643,7 +644,6 @@ describe("Rendezvous", function () { aliceRz, bobTransport, bobEcdh, - devices, deviceMap, }; } diff --git a/src/rendezvous/MSC3906Rendezvous.ts b/src/rendezvous/MSC3906Rendezvous.ts index 185eedfe280..b7574f84bed 100644 --- a/src/rendezvous/MSC3906Rendezvous.ts +++ b/src/rendezvous/MSC3906Rendezvous.ts @@ -192,7 +192,7 @@ export class MSC3906Rendezvous { ); } - const userId = this.client.getUserId()!; + const userId = this.client.getSafeUserId(); // mark the device as verified locally + cross sign logger.info(`Marking device ${this.newDeviceId} as verified`); @@ -202,7 +202,7 @@ export class MSC3906Rendezvous { const masterPublicKey = (await crypto.getCrossSigningKeyId(CrossSigningKey.Master)) ?? undefined; const ourDeviceId = this.client.getDeviceId()!; - const ourDeviceKey = (await this.client.getCrypto()!.getOwnDeviceKeys()).ed25519; + const ourDeviceKey = (await crypto.getOwnDeviceKeys()).ed25519; await this.send({ type: PayloadType.Finish, From 1f8bce0a9b71cfe3ac1d08c056dc98b13c26416c Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 30 Jan 2024 10:06:43 +0100 Subject: [PATCH 10/10] QR login | review: use getSafeUserId --- src/rendezvous/MSC3906Rendezvous.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rendezvous/MSC3906Rendezvous.ts b/src/rendezvous/MSC3906Rendezvous.ts index b7574f84bed..e558c224abe 100644 --- a/src/rendezvous/MSC3906Rendezvous.ts +++ b/src/rendezvous/MSC3906Rendezvous.ts @@ -249,7 +249,7 @@ export class MSC3906Rendezvous { } private async getOwnDevice(deviceId: string): Promise { - const userId = this.client.getUserId()!; + const userId = this.client.getSafeUserId(); const ownDeviceInfo = await this.client.getCrypto()!.getUserDeviceInfo([userId]); return ownDeviceInfo.get(userId)?.get(deviceId); }