Element-R: Store cross signing keys in secret storage (#3498)

* Store cross signing keys in secret storage

* Update `bootstrapSecretStorage` doc

* Throw error when `createSecretStorageKey` is not set

* Move mocking functions

* Store cross signing keys and user signing keys

* Fix `awaitCrossSigningKeyUpload` documentation

* Remove useless comment

* Fix formatting after merge conflict

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, 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,45 +63,14 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s
     });
 
     /**
-     * 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
+     * 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<void> {
-        const uiaCallback: UIAuthCallback<void> = async (makeRequest) => {
-            await makeRequest(authDict);
-        };
-
-        // now bootstrap cross signing, and check it resolves successfully
         await aliceClient.getCrypto()?.bootstrapCrossSigning({
-            authUploadDeviceSigningKeys: uiaCallback,
+            authUploadDeviceSigningKeys: (makeRequest) => makeRequest(authDict).then(() => undefined),
         });
     }
 

spec/integ/crypto/crypto.spec.ts

@@ -50,8 +50,9 @@ 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 { SECRET_STORAGE_ALGORITHM_V1_AES } from "../../../src/secret-storage";
+import { mockInitialApiRequests, mockSetupCrossSigningRequests } from "../../test-utils/mockEndpoints";
+import { AddSecretStorageKeyOpts, SECRET_STORAGE_ALGORITHM_V1_AES } from "../../../src/secret-storage";
+import { CryptoCallbacks } from "../../../src/crypto-api";
 
 const ROOM_ID = "!room:id";
 
@@ -530,6 +531,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
@@ -542,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 */
@@ -2187,16 +2210,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<string> {
+        function awaitSecretStorageKeyStoredInAccountData(): Promise<string> {
             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);
 
@@ -2211,6 +2234,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.${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<Record<string, {}>> {
+            return new Promise((resolve) => {
+                // Called when the cross signing key is uploaded
+                fetchMock.put(
+                    `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);
+                        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
@@ -2249,12 +2291,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
@@ -2262,7 +2306,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);
@@ -2283,7 +2327,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);
@@ -2307,7 +2351,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);
@@ -2321,7 +2365,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);
@@ -2333,5 +2377,38 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
                 expect(createSecretStorageKey).toHaveBeenCalledTimes(2);
             },
         );
+
+        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 });
+
+            // 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 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(masterKey[secretStorageKey]).toBeDefined();
+            expect(userSigningKey[secretStorageKey]).toBeDefined();
+            expect(selfSigningKey[secretStorageKey]).toBeDefined();
+        });
     });
 });

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",
+        },
+        {},
+    );
+}

src/client.ts

@@ -367,6 +367,9 @@ export interface ICreateClientOpts {
      */
     useE2eForGroupCall?: boolean;
 
+    /**
+     * Crypto callbacks provided by the application
+     */
     cryptoCallbacks?: ICryptoCallbacks;
 
     /**

src/crypto-api.ts

@@ -192,12 +192,17 @@ export interface CryptoApi {
     isSecretStorageReady(): Promise<boolean>;
 
     /**
-     * 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.
      */

src/rust-crypto/rust-crypto.ts

@@ -387,42 +387,96 @@ export class RustCrypto implements CryptoBackend {
         createSecretStorageKey,
         setupNewSecretStorage,
     }: CreateSecretStorageOpts = {}): Promise<void> {
-        // 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
-            if (keyInfo.algorithm === SECRET_STORAGE_ALGORITHM_V1_AES && !setupNewSecretStorage) {
-                return;
-            }
+        const isNewSecretStorageKeyNeeded = setupNewSecretStorage || !(await this.secretStorageHasAESKey());
+
+        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);
+        }
 
+        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 (
+            hasPrivateKeys &&
+            (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");
+            }
+
+            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);
+        }
+    }
+
+    /**
+     * 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<void> {
         // 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<boolean> {
+        // 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}
      */