From 7fe2d7993e2a9ae19cdeb9ba85615c1a53b70637 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 21 Jun 2023 17:27:35 +0200 Subject: [PATCH] Store cross signing keys in secret storage --- spec/integ/crypto/cross-signing.spec.ts | 52 ++------------- spec/integ/crypto/crypto.spec.ts | 85 ++++++++++++++++++++++--- spec/test-utils/cross-signing.ts | 62 ++++++++++++++++++ src/client.ts | 3 + src/rust-crypto/rust-crypto.ts | 72 +++++++++++++++------ 5 files changed, 200 insertions(+), 74 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..a7130cd6661 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); @@ -2202,7 +2226,25 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, return {}; }, - { overwriteRoutes: true }, + ); + }); + } + + /** + * 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 {}; + }, ); }); } @@ -2258,7 +2300,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 +2321,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 +2345,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 +2359,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 +2371,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} */