From dec4650d3d11449788ff7ff0789628a6e4089e59 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Tue, 29 Aug 2023 13:27:28 +0200 Subject: [PATCH] ElementR: Update `CryptoApi.userHasCrossSigningKeys` (#3646) * WIP `CryptoApi.getStoredCrossSigningForUser` * Fix QRCode * Add docs and rename * Add tests for `RustCrossSigningInfo.ts` * Do `/keys/query` instead of using `UserIdentity` * Review changes * Get rid of `CrossSigningInfo` * Merge `hasCrossSigningKeysForUser` into `userHasCrossSigningKeys` * Apply suggestions from code review * More review comments --------- Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Co-authored-by: Richard van der Hoff --- spec/integ/crypto/crypto.spec.ts | 55 ++++++++++++++++++++++- spec/unit/rust-crypto/rust-crypto.spec.ts | 17 +++++++ src/client.ts | 1 + src/common-crypto/CryptoBackend.ts | 1 + src/crypto-api.ts | 26 ++++++----- src/crypto/index.ts | 6 +-- src/rust-crypto/rust-crypto.ts | 55 +++++++++++++++++------ 7 files changed, 132 insertions(+), 29 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index fd578084f..c6009c18e 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -25,7 +25,16 @@ import Olm from "@matrix-org/olm"; import type { IDeviceKeys } from "../../../src/@types/crypto"; import * as testUtils from "../../test-utils/test-utils"; import { CRYPTO_BACKENDS, getSyncResponse, InitCrypto, syncPromise } from "../../test-utils/test-utils"; -import { TEST_ROOM_ID, TEST_ROOM_ID as ROOM_ID, TEST_USER_ID } from "../../test-utils/test-data"; +import { + BOB_SIGNED_CROSS_SIGNING_KEYS_DATA, + BOB_SIGNED_TEST_DEVICE_DATA, + BOB_TEST_USER_ID, + SIGNED_CROSS_SIGNING_KEYS_DATA, + SIGNED_TEST_DEVICE_DATA, + TEST_ROOM_ID, + TEST_ROOM_ID as ROOM_ID, + TEST_USER_ID, +} from "../../test-utils/test-data"; import { TestClient } from "../../TestClient"; import { logger } from "../../../src/logger"; import { @@ -36,6 +45,7 @@ import { IDownloadKeyResult, IEvent, IndexedDBCryptoStore, + IRoomEvent, IStartClientOpts, MatrixClient, MatrixEvent, @@ -44,7 +54,6 @@ import { Room, RoomMember, RoomStateEvent, - IRoomEvent, } from "../../../src/matrix"; import { DeviceInfo } from "../../../src/crypto/deviceinfo"; import { E2EKeyReceiver, IE2EKeyReceiver } from "../../test-utils/E2EKeyReceiver"; @@ -2566,4 +2575,46 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }, ); }); + + describe("Check if the cross signing keys are available for a user", () => { + beforeEach(async () => { + // anything that we don't have a specific matcher for silently returns a 404 + fetchMock.catch(404); + + keyResponder = new E2EKeyResponder(aliceClient.getHomeserverUrl()); + keyResponder.addCrossSigningData(SIGNED_CROSS_SIGNING_KEYS_DATA); + keyResponder.addDeviceKeys(SIGNED_TEST_DEVICE_DATA); + keyResponder.addKeyReceiver(BOB_TEST_USER_ID, keyReceiver); + keyResponder.addCrossSigningData(BOB_SIGNED_CROSS_SIGNING_KEYS_DATA); + keyResponder.addDeviceKeys(BOB_SIGNED_TEST_DEVICE_DATA); + + expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); + await startClientAndAwaitFirstSync(); + }); + + it("Cross signing keys are available for an untracked user with cross signing keys on the homeserver", async () => { + // Needed for old crypto, download and cache locally the cross signing keys of Bob + await aliceClient.getCrypto()?.getUserDeviceInfo([BOB_TEST_USER_ID], true); + + const hasCrossSigningKeysForUser = await aliceClient + .getCrypto()! + .userHasCrossSigningKeys(BOB_TEST_USER_ID, true); + expect(hasCrossSigningKeysForUser).toBe(true); + }); + + it("Cross signing keys are available for a tracked user", async () => { + // Process Alice keys, old crypto has a sleep(5ms) during the process + await jest.advanceTimersByTimeAsync(5); + await flushPromises(); + + // Alice is the local user and should be tracked ! + const hasCrossSigningKeysForUser = await aliceClient.getCrypto()!.userHasCrossSigningKeys(TEST_USER_ID); + expect(hasCrossSigningKeysForUser).toBe(true); + }); + + it("Cross signing keys are not available for an unknown user", async () => { + const hasCrossSigningKeysForUser = await aliceClient.getCrypto()!.userHasCrossSigningKeys("@unknown:xyz"); + expect(hasCrossSigningKeysForUser).toBe(false); + }); + }); }); diff --git a/spec/unit/rust-crypto/rust-crypto.spec.ts b/spec/unit/rust-crypto/rust-crypto.spec.ts index e5fc8a265..447f8ce5f 100644 --- a/spec/unit/rust-crypto/rust-crypto.spec.ts +++ b/spec/unit/rust-crypto/rust-crypto.spec.ts @@ -460,6 +460,23 @@ describe("RustCrypto", () => { await expect(rustCrypto.userHasCrossSigningKeys()).resolves.toBe(true); }); + + it("returns true if the user is untracked, downloadUncached is set at true and the cross-signing keys are available", async () => { + fetchMock.post("path:/_matrix/client/v3/keys/query", { + device_keys: { + [testData.BOB_TEST_USER_ID]: { + [testData.BOB_TEST_DEVICE_ID]: testData.BOB_SIGNED_TEST_DEVICE_DATA, + }, + }, + ...testData.BOB_SIGNED_CROSS_SIGNING_KEYS_DATA, + }); + + await expect(rustCrypto.userHasCrossSigningKeys(testData.BOB_TEST_USER_ID, true)).resolves.toBe(true); + }); + + it("returns false if the user is unknown", async () => { + await expect(rustCrypto.userHasCrossSigningKeys(testData.BOB_TEST_USER_ID)).resolves.toBe(false); + }); }); describe("createRecoveryKeyFromPassphrase", () => { diff --git a/src/client.ts b/src/client.ts index 824399401..696ff8140 100644 --- a/src/client.ts +++ b/src/client.ts @@ -2628,6 +2628,7 @@ export class MatrixClient extends TypedEventEmitter; - /** * Perform any background tasks that can be done before a message is ready to * send, in order to speed up sending of the message. @@ -88,6 +78,22 @@ export interface CryptoApi { */ importRoomKeys(keys: IMegolmSessionData[], opts?: ImportRoomKeysOpts): Promise; + /** + * Check if the given user has published cross-signing keys. + * + * - If the user is tracked, a `/keys/query` request is made to update locally the cross signing keys. + * - If the user is not tracked locally and downloadUncached is set to true, + * a `/keys/query` request is made to the server to retrieve the cross signing keys. + * - Otherwise, return false + * + * @param userId - the user ID to check. Defaults to the local user. + * @param downloadUncached - If true, download the device list for users whose device list we are not + * currently tracking. Defaults to false, in which case `false` will be returned for such users. + * + * @returns true if the cross signing keys are available. + */ + userHasCrossSigningKeys(userId?: string, downloadUncached?: boolean): Promise; + /** * Get the device information for the given list of users. * diff --git a/src/crypto/index.ts b/src/crypto/index.ts index d59fbe8d3..8e6c339f6 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -700,9 +700,9 @@ export class Crypto extends TypedEventEmitter { - await this.downloadKeys([this.userId]); - return this.deviceList.getStoredCrossSigningForUser(this.userId) !== null; + public async userHasCrossSigningKeys(userId = this.userId): Promise { + await this.downloadKeys([userId]); + return this.deviceList.getStoredCrossSigningForUser(userId) !== null; } /** diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index e00cc454a..9a90c9db1 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -244,20 +244,6 @@ export class RustCrypto extends TypedEventEmitter { - const userId = new RustSdkCryptoJs.UserId(this.userId); - /* make sure we have an *up-to-date* idea of the user's cross-signing keys. This is important, because if we - * return "false" here, we will end up generating new cross-signing keys and replacing the existing ones. - */ - const request = this.olmMachine.queryKeysForUsers([userId]); - await this.outgoingRequestProcessor.makeOutgoingRequest(request); - const userIdentity = await this.olmMachine.getIdentity(userId); - return userIdentity !== undefined; - } - public prepareToEncrypt(room: Room): void { const encryptor = this.roomEncryptors[room.roomId]; @@ -289,6 +275,47 @@ export class RustCrypto extends TypedEventEmitter { + // TODO: could probably do with a more efficient way of doing this than returning the whole set and searching + const rustTrackedUsers: Set = await this.olmMachine.trackedUsers(); + let rustTrackedUser: RustSdkCryptoJs.UserId | undefined; + for (const u of rustTrackedUsers) { + if (userId === u.toString()) { + rustTrackedUser = u; + break; + } + } + + if (rustTrackedUser !== undefined) { + if (userId === this.userId) { + /* make sure we have an *up-to-date* idea of the user's cross-signing keys. This is important, because if we + * return "false" here, we will end up generating new cross-signing keys and replacing the existing ones. + */ + const request = this.olmMachine.queryKeysForUsers([rustTrackedUser]); + await this.outgoingRequestProcessor.makeOutgoingRequest(request); + } + const userIdentity = await this.olmMachine.getIdentity(rustTrackedUser); + return userIdentity !== undefined; + } else if (downloadUncached) { + // Download the cross signing keys and check if the master key is available + const keyResult = await this.downloadDeviceList(new Set([userId])); + const keys = keyResult.master_keys?.[userId]; + + // No master key + if (!keys) return false; + + // `keys` is an object with { [`ed25519:${pubKey}`]: pubKey } + // We assume only a single key, and we want the bare form without type + // prefix, so we select the values. + return Boolean(Object.values(keys.keys)[0]); + } else { + return false; + } + } + /** * Get the device information for the given list of users. *