From 762c7a10334c5dd51913560862f62cbefa043d16 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 21 Jun 2023 17:27:35 +0200 Subject: [PATCH 1/8] Store cross signing keys in secret storage --- spec/integ/crypto/cross-signing.spec.ts | 52 ++------------- spec/integ/crypto/crypto.spec.ts | 84 ++++++++++++++++++++++--- spec/test-utils/cross-signing.ts | 62 ++++++++++++++++++ src/client.ts | 3 + src/rust-crypto/rust-crypto.ts | 72 +++++++++++++++------ 5 files changed, 200 insertions(+), 73 deletions(-) create mode 100644 spec/test-utils/cross-signing.ts diff --git a/spec/integ/crypto/cross-signing.spec.ts b/spec/integ/crypto/cross-signing.spec.ts index 16ba6df1d48..ea43ece7ae4 100644 --- a/spec/integ/crypto/cross-signing.spec.ts +++ b/spec/integ/crypto/cross-signing.spec.ts @@ -19,7 +19,8 @@ import "fake-indexeddb/auto"; import { IDBFactory } from "fake-indexeddb"; import { CRYPTO_BACKENDS, InitCrypto } from "../../test-utils/test-utils"; -import { createClient, MatrixClient, IAuthDict, UIAuthCallback } from "../../../src"; +import { createClient, MatrixClient } from "../../../src"; +import { bootstrapCrossSigning, mockSetupCrossSigningRequests } from "../../test-utils/cross-signing"; afterEach(() => { // reset fake-indexeddb after each test, to make sure we don't leak connections @@ -61,56 +62,13 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s fetchMock.mockReset(); }); - /** - * Mock the requests needed to set up cross signing - * - * Return `{}` for `GET _matrix/client/r0/user/:userId/account_data/:type` request - * Return `{}` for `POST _matrix/client/v3/keys/signatures/upload` request (named `upload-sigs` for fetchMock check) - * Return `{}` for `POST /_matrix/client/(unstable|v3)/keys/device_signing/upload` request (named `upload-keys` for fetchMock check) - */ - function mockSetupCrossSigningRequests(): void { - // have account_data requests return an empty object - fetchMock.get("express:/_matrix/client/r0/user/:userId/account_data/:type", {}); - - // we expect a request to upload signatures for our device ... - fetchMock.post({ url: "path:/_matrix/client/v3/keys/signatures/upload", name: "upload-sigs" }, {}); - - // ... and one to upload the cross-signing keys (with UIA) - fetchMock.post( - // legacy crypto uses /unstable/; /v3/ is correct - { - url: new RegExp("/_matrix/client/(unstable|v3)/keys/device_signing/upload"), - name: "upload-keys", - }, - {}, - ); - } - - /** - * Create cross-signing keys, publish the keys - * Mock and bootstrap all the required steps - * - * @param authDict - The parameters to as the `auth` dict in the key upload request. - * @see https://spec.matrix.org/v1.6/client-server-api/#authentication-types - */ - async function bootstrapCrossSigning(authDict: IAuthDict): Promise { - const uiaCallback: UIAuthCallback = async (makeRequest) => { - await makeRequest(authDict); - }; - - // now bootstrap cross signing, and check it resolves successfully - await aliceClient.getCrypto()?.bootstrapCrossSigning({ - authUploadDeviceSigningKeys: uiaCallback, - }); - } - describe("bootstrapCrossSigning (before initialsync completes)", () => { it("publishes keys if none were yet published", async () => { mockSetupCrossSigningRequests(); // provide a UIA callback, so that the cross-signing keys are uploaded const authDict = { type: "test" }; - await bootstrapCrossSigning(authDict); + await bootstrapCrossSigning(aliceClient, authDict); // check the cross-signing keys upload expect(fetchMock.called("upload-keys")).toBeTruthy(); @@ -156,7 +114,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s // provide a UIA callback, so that the cross-signing keys are uploaded const authDict = { type: "test" }; - await bootstrapCrossSigning(authDict); + await bootstrapCrossSigning(aliceClient, authDict); const crossSigningStatus = await aliceClient.getCrypto()!.getCrossSigningStatus(); @@ -180,7 +138,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s it("should return true after bootstrapping cross-signing", async () => { mockSetupCrossSigningRequests(); - await bootstrapCrossSigning({ type: "test" }); + await bootstrapCrossSigning(aliceClient, { type: "test" }); const isCrossSigningReady = await aliceClient.getCrypto()!.isCrossSigningReady(); diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 1618ae304e7..5318fc52890 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -51,7 +51,9 @@ import { escapeRegExp } from "../../../src/utils"; import { downloadDeviceToJsDevice } from "../../../src/rust-crypto/device-converter"; import { flushPromises } from "../../test-utils/flushPromises"; import { mockInitialApiRequests } from "../../test-utils/mockEndpoints"; -import { SECRET_STORAGE_ALGORITHM_V1_AES } from "../../../src/secret-storage"; +import { AddSecretStorageKeyOpts, SECRET_STORAGE_ALGORITHM_V1_AES } from "../../../src/secret-storage"; +import { mockSetupCrossSigningRequests } from "../../test-utils/cross-signing"; +import { CryptoCallbacks } from "../../../src/crypto-api"; const ROOM_ID = "!room:id"; @@ -530,6 +532,27 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }; } + /** + * Create the {@link CryptoCallbacks} + */ + function createCryptoCallbacks(): CryptoCallbacks { + // Store the cached secret storage key and return it when `getSecretStorageKey` is called + let cachedKey: { keyId: string; key: Uint8Array }; + const cacheSecretStorageKey = (keyId: string, keyInfo: AddSecretStorageKeyOpts, key: Uint8Array) => { + cachedKey = { + keyId, + key, + }; + }; + + const getSecretStorageKey = () => Promise.resolve<[string, Uint8Array]>([cachedKey.keyId, cachedKey.key]); + + return { + cacheSecretStorageKey, + getSecretStorageKey, + }; + } + beforeEach(async () => { // anything that we don't have a specific matcher for silently returns a 404 fetchMock.catch(404); @@ -541,6 +564,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, userId: "@alice:localhost", accessToken: "akjgkrgjs", deviceId: "xzcvb", + cryptoCallbacks: createCryptoCallbacks(), }); /* set up listeners for /keys/upload and /sync */ @@ -2183,16 +2207,16 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }); /** - * Create a mock to respond to the PUT request `/_matrix/client/r0/user/:userId/account_data/:type` + * Create a mock to respond to the PUT request `/_matrix/client/r0/user/:userId/account_data/:type(m.secret_storage.*)` * Resolved when a key is uploaded (ie in `body.content.key`) * https://spec.matrix.org/v1.6/client-server-api/#put_matrixclientv3useruseridaccount_datatype */ - function awaitKeyStoredInAccountData(): Promise { + function awaitSecretStorageKeyStoredInAccountData(): Promise { return new Promise((resolve) => { // This url is called multiple times during the secret storage bootstrap process // When we received the newly generated key, we return it fetchMock.put( - "express:/_matrix/client/r0/user/:userId/account_data/:type", + "express:/_matrix/client/r0/user/:userId/account_data/:type(m.secret_storage.*)", (url: string, options: RequestInit) => { const content = JSON.parse(options.body as string); @@ -2207,6 +2231,25 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }); } + /** + * Create a mock to respond to the PUT request `/_matrix/client/r0/user/:userId/account_data/m.cross_signing.master` + * Resolved when the cross signing master key is uploaded + * https://spec.matrix.org/v1.6/client-server-api/#put_matrixclientv3useruseridaccount_datatype + */ + function awaitCrossSigningMasterKeyUpload(): Promise> { + return new Promise((resolve) => { + // Called when the cross signing key master key is uploaded + fetchMock.put( + "express:/_matrix/client/r0/user/:userId/account_data/m.cross_signing.master", + (url: string, options: RequestInit) => { + const content = JSON.parse(options.body as string); + resolve(content.encrypted); + return {}; + }, + ); + }); + } + /** * Send in the sync response the provided `secretStorageKey` into the account_data field * The key is set for the `m.secret_storage.default_key` and `m.secret_storage.key.${secretStorageKey}` events @@ -2258,7 +2301,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, .bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); // Wait for the key to be uploaded in the account data - const secretStorageKey = await awaitKeyStoredInAccountData(); + const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); // Return the newly created key in the sync response sendSyncResponse(secretStorageKey); @@ -2279,7 +2322,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, const bootstrapPromise = aliceClient.getCrypto()!.bootstrapSecretStorage({ createSecretStorageKey }); // Wait for the key to be uploaded in the account data - const secretStorageKey = await awaitKeyStoredInAccountData(); + const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); // Return the newly created key in the sync response sendSyncResponse(secretStorageKey); @@ -2303,7 +2346,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, .bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); // Wait for the key to be uploaded in the account data - let secretStorageKey = await awaitKeyStoredInAccountData(); + let secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); // Return the newly created key in the sync response sendSyncResponse(secretStorageKey); @@ -2317,7 +2360,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, .bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); // Wait for the key to be uploaded in the account data - secretStorageKey = await awaitKeyStoredInAccountData(); + secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); // Return the newly created key in the sync response sendSyncResponse(secretStorageKey); @@ -2329,5 +2372,30 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expect(createSecretStorageKey).toHaveBeenCalledTimes(2); }, ); + + newBackendOnly("should upload cross signing master key", async () => { + mockSetupCrossSigningRequests(); + + await aliceClient.getCrypto()?.bootstrapCrossSigning({}); + + const bootstrapPromise = aliceClient + .getCrypto()! + .bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); + + // Wait for the key to be uploaded in the account data + const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); + + // Return the newly created key in the sync response + sendSyncResponse(secretStorageKey); + + // Wait for the cross signing key to be uploaded + const crossSigningKey = await awaitCrossSigningMasterKeyUpload(); + + // Finally, wait for bootstrapSecretStorage to finished + await bootstrapPromise; + + // Expect the cross signing master key to be uploaded and to be encrypted with `secretStorageKey` + expect(crossSigningKey[secretStorageKey]).toBeDefined(); + }); }); }); diff --git a/spec/test-utils/cross-signing.ts b/spec/test-utils/cross-signing.ts new file mode 100644 index 00000000000..3a598d37194 --- /dev/null +++ b/spec/test-utils/cross-signing.ts @@ -0,0 +1,62 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import fetchMock from "fetch-mock-jest"; + +import { IAuthDict, MatrixClient, UIAuthCallback } from "../../src"; + +/** + * Mock the requests needed to set up cross signing + * + * Return `{}` for `GET _matrix/client/r0/user/:userId/account_data/:type` request + * Return `{}` for `POST _matrix/client/v3/keys/signatures/upload` request (named `upload-sigs` for fetchMock check) + * Return `{}` for `POST /_matrix/client/(unstable|v3)/keys/device_signing/upload` request (named `upload-keys` for fetchMock check) + */ +export function mockSetupCrossSigningRequests(): void { + // have account_data requests return an empty object + fetchMock.get("express:/_matrix/client/r0/user/:userId/account_data/:type", {}); + + // we expect a request to upload signatures for our device ... + fetchMock.post({ url: "path:/_matrix/client/v3/keys/signatures/upload", name: "upload-sigs" }, {}); + + // ... and one to upload the cross-signing keys (with UIA) + fetchMock.post( + // legacy crypto uses /unstable/; /v3/ is correct + { + url: new RegExp("/_matrix/client/(unstable|v3)/keys/device_signing/upload"), + name: "upload-keys", + }, + {}, + ); +} + +/** + * Create cross-signing keys and publish the keys + * + * @param matrixClient - The matrixClient to bootstrap. + * @param authDict - The parameters to as the `auth` dict in the key upload request. + * @see https://spec.matrix.org/v1.6/client-server-api/#authentication-types + */ +export async function bootstrapCrossSigning(matrixClient: MatrixClient, authDict: IAuthDict): Promise { + const uiaCallback: UIAuthCallback = async (makeRequest) => { + await makeRequest(authDict); + }; + + // now bootstrap cross signing, and check it resolves successfully + await matrixClient.getCrypto()?.bootstrapCrossSigning({ + authUploadDeviceSigningKeys: uiaCallback, + }); +} diff --git a/src/client.ts b/src/client.ts index d3e6c4524db..5297eee9d88 100644 --- a/src/client.ts +++ b/src/client.ts @@ -367,6 +367,9 @@ export interface ICreateClientOpts { */ useE2eForGroupCall?: boolean; + /** + * Crypto callbacks provided by the application + */ cryptoCallbacks?: ICryptoCallbacks; /** diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index 033063bc437..8e7fe3668d7 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -387,42 +387,78 @@ export class RustCrypto implements CryptoBackend { createSecretStorageKey, setupNewSecretStorage, }: CreateSecretStorageOpts = {}): Promise { - // If createSecretStorageKey is not set, we stop - if (!createSecretStorageKey) return; - - // See if we already have an AES secret-storage key. - const secretStorageKeyTuple = await this.secretStorage.getKey(); - - if (secretStorageKeyTuple) { - const [, keyInfo] = secretStorageKeyTuple; + // If an AES Key is already stored in the secret storage and setupNewSecretStorage is not set + // we don't want to create a new key + const isNewSecretStorageKeyNeeded = setupNewSecretStorage || !(await this.secretStorageHasAESKey()); + + if (isNewSecretStorageKeyNeeded && createSecretStorageKey) { + // Create a new storage key and add it to secret storage + const recoveryKey = await createSecretStorageKey(); + await this.addSecretStorageKeyToSecretStorage(recoveryKey); + } - // If an AES Key is already stored in the secret storage and setupNewSecretStorage is not set - // we don't want to create a new key - if (keyInfo.algorithm === SECRET_STORAGE_ALGORITHM_V1_AES && !setupNewSecretStorage) { - return; + // If we have cross-signing private keys cached, store them in secret + // storage if they are not there already. + if ( + (await this.isCrossSigningReady()) && + (isNewSecretStorageKeyNeeded || !(await secretStorageContainsCrossSigningKeys(this.secretStorage))) + ) { + const crossSigningPrivateKeys: RustSdkCryptoJs.CrossSigningKeyExport = + await this.olmMachine.exportCrossSigningKeys(); + + if (!crossSigningPrivateKeys.masterKey) { + throw new Error("missing master key in cross signing private keys"); } - } - const recoveryKey = await createSecretStorageKey(); + await this.secretStorage.store("m.cross_signing.master", crossSigningPrivateKeys.masterKey); + } + } + /** + * Add the secretStorage key to the secret storage + * - The secret storage key must have the `keyInfo` field filled + * - The secret storage key is set as the default key of the secret storage + * - Call `cryptoCallbacks.cacheSecretStorageKey` when done + * + * @param secretStorageKey - The secret storage key to add in the secret storage. + */ + private async addSecretStorageKeyToSecretStorage(secretStorageKey: GeneratedSecretStorageKey): Promise { // keyInfo is required to continue - if (!recoveryKey.keyInfo) { - throw new Error("missing keyInfo field in the secret storage key created by createSecretStorageKey"); + if (!secretStorageKey.keyInfo) { + throw new Error("missing keyInfo field in the secret storage key"); } const secretStorageKeyObject = await this.secretStorage.addKey( SECRET_STORAGE_ALGORITHM_V1_AES, - recoveryKey.keyInfo, + secretStorageKey.keyInfo, ); + await this.secretStorage.setDefaultKeyId(secretStorageKeyObject.keyId); this.cryptoCallbacks.cacheSecretStorageKey?.( secretStorageKeyObject.keyId, secretStorageKeyObject.keyInfo, - recoveryKey.privateKey, + secretStorageKey.privateKey, ); } + /** + * Check if a secret storage AES Key is already added in secret storage + * + * @returns True if an AES key is in the secret storage + */ + private async secretStorageHasAESKey(): Promise { + // See if we already have an AES secret-storage key. + const secretStorageKeyTuple = await this.secretStorage.getKey(); + + if (!secretStorageKeyTuple) return false; + + const [, keyInfo] = secretStorageKeyTuple; + + // Check if the key is an AES key + return keyInfo.algorithm === SECRET_STORAGE_ALGORITHM_V1_AES; + } + /** * Implementation of {@link CryptoApi#getCrossSigningStatus} */ From 3b4d6266206bb589c145b17128b0633d8f506d72 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 21 Jun 2023 18:03:39 +0200 Subject: [PATCH 2/8] Update `bootstrapSecretStorage` doc --- src/crypto-api.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/crypto-api.ts b/src/crypto-api.ts index 1a78f11b434..967fc8a61a6 100644 --- a/src/crypto-api.ts +++ b/src/crypto-api.ts @@ -192,12 +192,17 @@ export interface CryptoApi { isSecretStorageReady(): Promise; /** - * Bootstrap the secret storage by creating a new secret storage key and store it in the secret storage. + * Bootstrap the secret storage by creating a new secret storage key, add it in the secret storage and + * store the cross signing keys in the secret storage. * - * - Do nothing if an AES key is already stored in the secret storage and `setupNewKeyBackup` is not set; * - Generate a new key {@link GeneratedSecretStorageKey} with `createSecretStorageKey`. + * Only if `setupNewSecretStorage` is set or if there is no AES key in the secret storage * - Store this key in the secret storage and set it as the default key. * - Call `cryptoCallbacks.cacheSecretStorageKey` if provided. + * - Store the cross signing keys in the secret storage if + * - the cross signing is ready + * - a new key was created during the previous step + * - or the secret storage already contains the cross signing keys * * @param opts - Options object. */ From d7ef249a75b3b1d7879b146d25dcbd7449ee7109 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 21 Jun 2023 18:38:31 +0200 Subject: [PATCH 3/8] Throw error when `createSecretStorageKey` is not set --- spec/integ/crypto/crypto.spec.ts | 14 ++++++++------ src/rust-crypto/rust-crypto.ts | 6 +++++- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 5318fc52890..328dc7d2763 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -2288,12 +2288,14 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, await startClientAndAwaitFirstSync(); }); - newBackendOnly("should do no nothing if createSecretStorageKey is not set", async () => { - await aliceClient.getCrypto()!.bootstrapSecretStorage({ setupNewSecretStorage: true }); - - // No key was created - expect(createSecretStorageKey).toHaveBeenCalledTimes(0); - }); + newBackendOnly( + "should throw an error if we are unable to create a key because createSecretStorageKey is not set", + async () => { + await expect( + aliceClient.getCrypto()!.bootstrapSecretStorage({ setupNewSecretStorage: true }), + ).rejects.toThrow("unable to create a new secret storage key, createSecretStorageKey is not set"); + }, + ); newBackendOnly("should create a new key", async () => { const bootstrapPromise = aliceClient diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index 8e7fe3668d7..5c043f0ed1c 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -391,7 +391,11 @@ export class RustCrypto implements CryptoBackend { // we don't want to create a new key const isNewSecretStorageKeyNeeded = setupNewSecretStorage || !(await this.secretStorageHasAESKey()); - if (isNewSecretStorageKeyNeeded && createSecretStorageKey) { + if (isNewSecretStorageKeyNeeded) { + if (!createSecretStorageKey) { + throw new Error("unable to create a new secret storage key, createSecretStorageKey is not set"); + } + // Create a new storage key and add it to secret storage const recoveryKey = await createSecretStorageKey(); await this.addSecretStorageKeyToSecretStorage(recoveryKey); From 86b8d030a70ea509657c3a0fab74a7df0a935b60 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Fri, 23 Jun 2023 11:21:52 +0200 Subject: [PATCH 4/8] Move mocking functions --- spec/integ/crypto/cross-signing.spec.ts | 24 ++++++++-- spec/integ/crypto/crypto.spec.ts | 3 +- spec/test-utils/cross-signing.ts | 62 ------------------------- spec/test-utils/mockEndpoints.ts | 25 ++++++++++ 4 files changed, 45 insertions(+), 69 deletions(-) delete mode 100644 spec/test-utils/cross-signing.ts diff --git a/spec/integ/crypto/cross-signing.spec.ts b/spec/integ/crypto/cross-signing.spec.ts index ea43ece7ae4..ac7611d65bf 100644 --- a/spec/integ/crypto/cross-signing.spec.ts +++ b/spec/integ/crypto/cross-signing.spec.ts @@ -19,8 +19,8 @@ import "fake-indexeddb/auto"; import { IDBFactory } from "fake-indexeddb"; import { CRYPTO_BACKENDS, InitCrypto } from "../../test-utils/test-utils"; -import { createClient, MatrixClient } from "../../../src"; -import { bootstrapCrossSigning, mockSetupCrossSigningRequests } from "../../test-utils/cross-signing"; +import { createClient, IAuthDict, MatrixClient } from "../../../src"; +import { mockSetupCrossSigningRequests } from "../../test-utils/mockEndpoints"; afterEach(() => { // reset fake-indexeddb after each test, to make sure we don't leak connections @@ -62,13 +62,27 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s fetchMock.mockReset(); }); + /** + * Create cross-signing keys and publish the keys + * + * @param authDict - The parameters to as the `auth` dict in the key upload request. + * @see https://spec.matrix.org/v1.6/client-server-api/#authentication-types + */ + async function bootstrapCrossSigning(authDict: IAuthDict): Promise { + // now bootstrap cross signing, and check it resolves successfully + await aliceClient.getCrypto()?.bootstrapCrossSigning({ + // Expecting to return a promise + authUploadDeviceSigningKeys: (makeRequest) => makeRequest(authDict).then(() => undefined), + }); + } + describe("bootstrapCrossSigning (before initialsync completes)", () => { it("publishes keys if none were yet published", async () => { mockSetupCrossSigningRequests(); // provide a UIA callback, so that the cross-signing keys are uploaded const authDict = { type: "test" }; - await bootstrapCrossSigning(aliceClient, authDict); + await bootstrapCrossSigning(authDict); // check the cross-signing keys upload expect(fetchMock.called("upload-keys")).toBeTruthy(); @@ -114,7 +128,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s // provide a UIA callback, so that the cross-signing keys are uploaded const authDict = { type: "test" }; - await bootstrapCrossSigning(aliceClient, authDict); + await bootstrapCrossSigning(authDict); const crossSigningStatus = await aliceClient.getCrypto()!.getCrossSigningStatus(); @@ -138,7 +152,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s it("should return true after bootstrapping cross-signing", async () => { mockSetupCrossSigningRequests(); - await bootstrapCrossSigning(aliceClient, { type: "test" }); + await bootstrapCrossSigning({ type: "test" }); const isCrossSigningReady = await aliceClient.getCrypto()!.isCrossSigningReady(); diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 328dc7d2763..08c04157624 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -50,9 +50,8 @@ import { ISyncResponder, SyncResponder } from "../../test-utils/SyncResponder"; import { escapeRegExp } from "../../../src/utils"; import { downloadDeviceToJsDevice } from "../../../src/rust-crypto/device-converter"; import { flushPromises } from "../../test-utils/flushPromises"; -import { mockInitialApiRequests } from "../../test-utils/mockEndpoints"; +import { mockInitialApiRequests, mockSetupCrossSigningRequests } from "../../test-utils/mockEndpoints"; import { AddSecretStorageKeyOpts, SECRET_STORAGE_ALGORITHM_V1_AES } from "../../../src/secret-storage"; -import { mockSetupCrossSigningRequests } from "../../test-utils/cross-signing"; import { CryptoCallbacks } from "../../../src/crypto-api"; const ROOM_ID = "!room:id"; diff --git a/spec/test-utils/cross-signing.ts b/spec/test-utils/cross-signing.ts deleted file mode 100644 index 3a598d37194..00000000000 --- a/spec/test-utils/cross-signing.ts +++ /dev/null @@ -1,62 +0,0 @@ -/* -Copyright 2023 The Matrix.org Foundation C.I.C. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -import fetchMock from "fetch-mock-jest"; - -import { IAuthDict, MatrixClient, UIAuthCallback } from "../../src"; - -/** - * Mock the requests needed to set up cross signing - * - * Return `{}` for `GET _matrix/client/r0/user/:userId/account_data/:type` request - * Return `{}` for `POST _matrix/client/v3/keys/signatures/upload` request (named `upload-sigs` for fetchMock check) - * Return `{}` for `POST /_matrix/client/(unstable|v3)/keys/device_signing/upload` request (named `upload-keys` for fetchMock check) - */ -export function mockSetupCrossSigningRequests(): void { - // have account_data requests return an empty object - fetchMock.get("express:/_matrix/client/r0/user/:userId/account_data/:type", {}); - - // we expect a request to upload signatures for our device ... - fetchMock.post({ url: "path:/_matrix/client/v3/keys/signatures/upload", name: "upload-sigs" }, {}); - - // ... and one to upload the cross-signing keys (with UIA) - fetchMock.post( - // legacy crypto uses /unstable/; /v3/ is correct - { - url: new RegExp("/_matrix/client/(unstable|v3)/keys/device_signing/upload"), - name: "upload-keys", - }, - {}, - ); -} - -/** - * Create cross-signing keys and publish the keys - * - * @param matrixClient - The matrixClient to bootstrap. - * @param authDict - The parameters to as the `auth` dict in the key upload request. - * @see https://spec.matrix.org/v1.6/client-server-api/#authentication-types - */ -export async function bootstrapCrossSigning(matrixClient: MatrixClient, authDict: IAuthDict): Promise { - const uiaCallback: UIAuthCallback = async (makeRequest) => { - await makeRequest(authDict); - }; - - // now bootstrap cross signing, and check it resolves successfully - await matrixClient.getCrypto()?.bootstrapCrossSigning({ - authUploadDeviceSigningKeys: uiaCallback, - }); -} diff --git a/spec/test-utils/mockEndpoints.ts b/spec/test-utils/mockEndpoints.ts index a4c162867fe..bd5bb819a42 100644 --- a/spec/test-utils/mockEndpoints.ts +++ b/spec/test-utils/mockEndpoints.ts @@ -28,3 +28,28 @@ export function mockInitialApiRequests(homeserverUrl: string) { filter_id: "fid", }); } + +/** + * Mock the requests needed to set up cross signing + * + * Return `{}` for `GET _matrix/client/r0/user/:userId/account_data/:type` request + * Return `{}` for `POST _matrix/client/v3/keys/signatures/upload` request (named `upload-sigs` for fetchMock check) + * Return `{}` for `POST /_matrix/client/(unstable|v3)/keys/device_signing/upload` request (named `upload-keys` for fetchMock check) + */ +export function mockSetupCrossSigningRequests(): void { + // have account_data requests return an empty object + fetchMock.get("express:/_matrix/client/r0/user/:userId/account_data/:type", {}); + + // we expect a request to upload signatures for our device ... + fetchMock.post({ url: "path:/_matrix/client/v3/keys/signatures/upload", name: "upload-sigs" }, {}); + + // ... and one to upload the cross-signing keys (with UIA) + fetchMock.post( + // legacy crypto uses /unstable/; /v3/ is correct + { + url: new RegExp("/_matrix/client/(unstable|v3)/keys/device_signing/upload"), + name: "upload-keys", + }, + {}, + ); +} From 09a461b8c05c839e8e0899ebf6cf334e89a551bd Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Fri, 23 Jun 2023 12:11:00 +0200 Subject: [PATCH 5/8] Store cross signing keys and user signing keys --- spec/integ/crypto/crypto.spec.ts | 22 +++++++++++++++------- src/rust-crypto/rust-crypto.ts | 16 +++++++++++++++- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 08c04157624..13853da65c5 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -2235,11 +2235,11 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, * Resolved when the cross signing master key is uploaded * https://spec.matrix.org/v1.6/client-server-api/#put_matrixclientv3useruseridaccount_datatype */ - function awaitCrossSigningMasterKeyUpload(): Promise> { + function awaitCrossSigningKeyUpload(key: string): Promise> { return new Promise((resolve) => { - // Called when the cross signing key master key is uploaded + // Called when the cross signing key is uploaded fetchMock.put( - "express:/_matrix/client/r0/user/:userId/account_data/m.cross_signing.master", + `express:/_matrix/client/r0/user/:userId/account_data/m.cross_signing.${key}`, (url: string, options: RequestInit) => { const content = JSON.parse(options.body as string); resolve(content.encrypted); @@ -2374,11 +2374,13 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }, ); - newBackendOnly("should upload cross signing master key", async () => { + newBackendOnly("should upload cross signing keys", async () => { mockSetupCrossSigningRequests(); + // Before setting up secret-storage, bootstrap cross-signing, so that the client has cross-signing keys. await aliceClient.getCrypto()?.bootstrapCrossSigning({}); + // Now, when we bootstrap secret-storage, the cross-signing keys should be uploaded. const bootstrapPromise = aliceClient .getCrypto()! .bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); @@ -2389,14 +2391,20 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, // Return the newly created key in the sync response sendSyncResponse(secretStorageKey); - // Wait for the cross signing key to be uploaded - const crossSigningKey = await awaitCrossSigningMasterKeyUpload(); + // Wait for the cross signing keys to be uploaded + const [masterKey, userSigningKey, selfSigningKey] = await Promise.all([ + awaitCrossSigningKeyUpload("master"), + awaitCrossSigningKeyUpload("user_signing"), + awaitCrossSigningKeyUpload("self_signing"), + ]); // Finally, wait for bootstrapSecretStorage to finished await bootstrapPromise; // Expect the cross signing master key to be uploaded and to be encrypted with `secretStorageKey` - expect(crossSigningKey[secretStorageKey]).toBeDefined(); + expect(masterKey[secretStorageKey]).toBeDefined(); + expect(userSigningKey[secretStorageKey]).toBeDefined(); + expect(selfSigningKey[secretStorageKey]).toBeDefined(); }); }); }); diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index 5c043f0ed1c..1b791f3cafd 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -401,10 +401,14 @@ export class RustCrypto implements CryptoBackend { await this.addSecretStorageKeyToSecretStorage(recoveryKey); } + const crossSigningStatus: RustSdkCryptoJs.CrossSigningStatus = await this.olmMachine.crossSigningStatus(); + const hasPrivateKeys = + crossSigningStatus.hasMaster && crossSigningStatus.hasSelfSigning && crossSigningStatus.hasUserSigning; + // If we have cross-signing private keys cached, store them in secret // storage if they are not there already. if ( - (await this.isCrossSigningReady()) && + hasPrivateKeys && (isNewSecretStorageKeyNeeded || !(await secretStorageContainsCrossSigningKeys(this.secretStorage))) ) { const crossSigningPrivateKeys: RustSdkCryptoJs.CrossSigningKeyExport = @@ -414,7 +418,17 @@ export class RustCrypto implements CryptoBackend { throw new Error("missing master key in cross signing private keys"); } + if (!crossSigningPrivateKeys.userSigningKey) { + throw new Error("missing user signing key in cross signing private keys"); + } + + if (!crossSigningPrivateKeys.self_signing_key) { + throw new Error("missing self signing key in cross signing private keys"); + } + await this.secretStorage.store("m.cross_signing.master", crossSigningPrivateKeys.masterKey); + await this.secretStorage.store("m.cross_signing.user_signing", crossSigningPrivateKeys.userSigningKey); + await this.secretStorage.store("m.cross_signing.self_signing", crossSigningPrivateKeys.self_signing_key); } } From 739947e4a5f7f95b894983d1e01214f6ed53a474 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Fri, 23 Jun 2023 14:23:56 +0200 Subject: [PATCH 6/8] Fix `awaitCrossSigningKeyUpload` documentation --- spec/integ/crypto/crypto.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 13853da65c5..c337472d1d3 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -2231,8 +2231,8 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, } /** - * Create a mock to respond to the PUT request `/_matrix/client/r0/user/:userId/account_data/m.cross_signing.master` - * Resolved when the cross signing master key is uploaded + * Create a mock to respond to the PUT request `/_matrix/client/r0/user/:userId/account_data/m.cross_signing.${key}` + * Resolved when the cross signing key is uploaded * https://spec.matrix.org/v1.6/client-server-api/#put_matrixclientv3useruseridaccount_datatype */ function awaitCrossSigningKeyUpload(key: string): Promise> { From 6ee42d65d35c2c0253f4a5264dd4a116bf2bbfa2 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Fri, 23 Jun 2023 14:37:46 +0200 Subject: [PATCH 7/8] Remove useless comment --- spec/integ/crypto/cross-signing.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/integ/crypto/cross-signing.spec.ts b/spec/integ/crypto/cross-signing.spec.ts index ac7611d65bf..4043379e1e7 100644 --- a/spec/integ/crypto/cross-signing.spec.ts +++ b/spec/integ/crypto/cross-signing.spec.ts @@ -69,9 +69,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s * @see https://spec.matrix.org/v1.6/client-server-api/#authentication-types */ async function bootstrapCrossSigning(authDict: IAuthDict): Promise { - // now bootstrap cross signing, and check it resolves successfully await aliceClient.getCrypto()?.bootstrapCrossSigning({ - // Expecting to return a promise authUploadDeviceSigningKeys: (makeRequest) => makeRequest(authDict).then(() => undefined), }); } From f05961c56c571729610d225b5fc47cc062f3682b Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Fri, 23 Jun 2023 14:54:56 +0200 Subject: [PATCH 8/8] Fix formatting after merge conflict --- spec/integ/crypto/crypto.spec.ts | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index f1231345d26..fd751e57f7a 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -552,19 +552,20 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }; } - beforeEach(async () => { - // anything that we don't have a specific matcher for silently returns a 404 - fetchMock.catch(404); - fetchMock.config.warnOnFallback = false; - - const homeserverUrl = "https://alice-server.com"; - aliceClient = createClient({ - baseUrl: homeserverUrl, - userId: "@alice:localhost", - accessToken: "akjgkrgjs", - deviceId: "xzcvb", - cryptoCallbacks: createCryptoCallbacks(), - }); + beforeEach( + async () => { + // anything that we don't have a specific matcher for silently returns a 404 + fetchMock.catch(404); + fetchMock.config.warnOnFallback = false; + + const homeserverUrl = "https://alice-server.com"; + aliceClient = createClient({ + baseUrl: homeserverUrl, + userId: "@alice:localhost", + accessToken: "akjgkrgjs", + deviceId: "xzcvb", + cryptoCallbacks: createCryptoCallbacks(), + }); /* set up listeners for /keys/upload and /sync */ keyReceiver = new E2EKeyReceiver(homeserverUrl);