Deprecate MatrixClient.{prepare,create}KeyBackupVersion in favour of new CryptoApi.resetKeyBackup API (#3689)

* new resetKeyBackup API

* add delete backup version test

* code review

* code review

spec/integ/crypto/crypto.spec.ts

@@ -40,6 +40,7 @@ import { logger } from "../../../src/logger";
 import {
     Category,
     createClient,
+    CryptoEvent,
     IClaimOTKsResult,
     IContent,
     IDownloadKeyResult,
@@ -61,9 +62,13 @@ 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, mockSetupCrossSigningRequests } from "../../test-utils/mockEndpoints";
+import {
+    mockInitialApiRequests,
+    mockSetupCrossSigningRequests,
+    mockSetupMegolmBackupRequests,
+} from "../../test-utils/mockEndpoints";
 import { AddSecretStorageKeyOpts, SECRET_STORAGE_ALGORITHM_V1_AES } from "../../../src/secret-storage";
-import { CryptoCallbacks } from "../../../src/crypto-api";
+import { CryptoCallbacks, KeyBackupInfo } from "../../../src/crypto-api";
 import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder";
 
 afterEach(() => {
@@ -2197,11 +2202,9 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
                     "express:/_matrix/client/v3/user/:userId/account_data/:type(m.secret_storage.*)",
                     (url: string, options: RequestInit) => {
                         const content = JSON.parse(options.body as string);
-
                         if (content.key) {
                             resolve(content.key);
                         }
-
                         return {};
                     },
                     { overwriteRoutes: true },
@@ -2228,6 +2231,74 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
             });
         }
 
+        function awaitMegolmBackupKeyUpload(): Promise<Record<string, {}>> {
+            return new Promise((resolve) => {
+                // Called when the megolm backup key is uploaded
+                fetchMock.put(
+                    `express:/_matrix/client/v3/user/:userId/account_data/m.megolm_backup.v1`,
+                    (url: string, options: RequestInit) => {
+                        const content = JSON.parse(options.body as string);
+                        resolve(content.encrypted);
+                        return {};
+                    },
+                    { overwriteRoutes: true },
+                );
+            });
+        }
+
+        /**
+         * Add all mocks needed to set up cross-signing, key backup, 4S and then
+         * configure the account to have recovery.
+         *
+         * @param backupVersion - The version of the created backup
+         */
+        async function bootstrapSecurity(backupVersion: string): Promise<void> {
+            mockSetupCrossSigningRequests();
+            mockSetupMegolmBackupRequests(backupVersion);
+
+            // promise which will resolve when a `KeyBackupStatus` event is emitted with `enabled: true`
+            const backupStatusUpdate = new Promise<void>((resolve) => {
+                aliceClient.on(CryptoEvent.KeyBackupStatus, (enabled) => {
+                    if (enabled) {
+                        resolve();
+                    }
+                });
+            });
+
+            const setupPromises = [
+                awaitCrossSigningKeyUpload("master"),
+                awaitCrossSigningKeyUpload("user_signing"),
+                awaitCrossSigningKeyUpload("self_signing"),
+                awaitMegolmBackupKeyUpload(),
+            ];
+
+            // 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,
+                setupNewKeyBackup: true,
+            });
+
+            // 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
+            await Promise.all(setupPromises);
+
+            // wait for bootstrapSecretStorage to finished
+            await bootstrapPromise;
+            // Finally ensure backup is working
+            await aliceClient.getCrypto()!.checkKeyBackupAndEnable();
+
+            await backupStatusUpdate;
+        }
+
         /**
          * 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
@@ -2385,6 +2456,95 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
             expect(userSigningKey[secretStorageKey]).toBeDefined();
             expect(selfSigningKey[secretStorageKey]).toBeDefined();
         });
+
+        oldBackendOnly("should create a new megolm backup", async () => {
+            const backupVersion = "abc";
+            await bootstrapSecurity(backupVersion);
+
+            // Expect a backup to be available and used
+            const activeBackup = await aliceClient.getCrypto()!.getActiveSessionBackupVersion();
+            expect(activeBackup).toStrictEqual(backupVersion);
+        });
+
+        oldBackendOnly("Reset key backup should create a new backup and update 4S", async () => {
+            // First set up 4S and key backup
+            const backupVersion = "1";
+            await bootstrapSecurity(backupVersion);
+
+            const currentVersion = await aliceClient.getCrypto()!.getActiveSessionBackupVersion();
+            const currentBackupKey = await aliceClient.getCrypto()!.getSessionBackupPrivateKey();
+
+            // we will call reset backup, it should delete the existing one, then setup a new one
+            // Let's mock for that
+
+            // Mock delete and replace the GET to return 404 as soon as called
+            const awaitDeleteCalled = new Promise<void>((resolve) => {
+                fetchMock.delete(
+                    "express:/_matrix/client/v3/room_keys/version/:version",
+                    (url: string, options: RequestInit) => {
+                        fetchMock.get(
+                            "path:/_matrix/client/v3/room_keys/version",
+                            {
+                                status: 404,
+                                body: { errcode: "M_NOT_FOUND", error: "Account data not found." },
+                            },
+                            { overwriteRoutes: true },
+                        );
+                        resolve();
+                        return {};
+                    },
+                    { overwriteRoutes: true },
+                );
+            });
+
+            const newVersion = "2";
+            fetchMock.post(
+                "path:/_matrix/client/v3/room_keys/version",
+                (url, request) => {
+                    const backupData: KeyBackupInfo = JSON.parse(request.body?.toString() ?? "{}");
+                    backupData.version = newVersion;
+                    backupData.count = 0;
+                    backupData.etag = "zer";
+
+                    // update get call with new version
+                    fetchMock.get("path:/_matrix/client/v3/room_keys/version", backupData, {
+                        overwriteRoutes: true,
+                    });
+                    return {
+                        version: backupVersion,
+                    };
+                },
+                { overwriteRoutes: true },
+            );
+
+            const newBackupStatusUpdate = new Promise<void>((resolve) => {
+                aliceClient.on(CryptoEvent.KeyBackupStatus, (enabled) => {
+                    if (enabled) {
+                        resolve();
+                    }
+                });
+            });
+
+            const newBackupUploadPromise = awaitMegolmBackupKeyUpload();
+
+            await aliceClient.getCrypto()!.resetKeyBackup();
+            await awaitDeleteCalled;
+            await newBackupStatusUpdate;
+            await newBackupUploadPromise;
+
+            const nextVersion = await aliceClient.getCrypto()!.getActiveSessionBackupVersion();
+            const nextKey = await aliceClient.getCrypto()!.getSessionBackupPrivateKey();
+
+            expect(nextVersion).toBeDefined();
+            expect(nextVersion).not.toEqual(currentVersion);
+            expect(nextKey).not.toEqual(currentBackupKey);
+
+            // Test deletion of the backup
+            await aliceClient.getCrypto()!.deleteKeyBackupVersion(nextVersion!);
+            await aliceClient.getCrypto()!.checkKeyBackupAndEnable();
+            // XXX Legacy crypto does not update 4S when deleting backup; should ensure that rust implem does it.
+            expect(await aliceClient.getCrypto()!.getActiveSessionBackupVersion()).toBeNull();
+        });
     });
 
     describe("Incoming verification in a DM", () => {

spec/test-utils/mockEndpoints.ts

@@ -16,6 +16,8 @@ limitations under the License.
 
 import fetchMock from "fetch-mock-jest";
 
+import { KeyBackupInfo } from "../../src/crypto-api";
+
 /**
  * Mock out the endpoints that the js-sdk calls when we call `MatrixClient.start()`.
  *
@@ -56,3 +58,35 @@ export function mockSetupCrossSigningRequests(): void {
         {},
     );
 }
+
+/**
+ * Mock out requests to `/room_keys/version`.
+ *
+ * Returns `404 M_NOT_FOUND` for GET requests until `POST room_keys/version` is called.
+ * Once the POST is done, `GET /room_keys/version` will return the posted backup
+ * instead of 404.
+ *
+ * @param backupVersion - The backup version that will be returned by `POST room_keys/version`.
+ */
+export function mockSetupMegolmBackupRequests(backupVersion: string): void {
+    fetchMock.get("path:/_matrix/client/v3/room_keys/version", {
+        status: 404,
+        body: {
+            errcode: "M_NOT_FOUND",
+            error: "No current backup version",
+        },
+    });
+
+    fetchMock.post("path:/_matrix/client/v3/room_keys/version", (url, request) => {
+        const backupData: KeyBackupInfo = JSON.parse(request.body?.toString() ?? "{}");
+        backupData.version = backupVersion;
+        backupData.count = 0;
+        backupData.etag = "zer";
+        fetchMock.get("path:/_matrix/client/v3/room_keys/version", backupData, {
+            overwriteRoutes: true,
+        });
+        return {
+            version: backupVersion,
+        };
+    });
+}

src/client.ts

@@ -3270,6 +3270,8 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
     /**
      * Get information about the current key backup.
      * @returns Information object from API or null
+     *
+     * @deprecated Prefer {@link CryptoApi.checkKeyBackupAndEnable}.
      */
     public async getKeyBackupVersion(): Promise<IKeyBackupInfo | null> {
         let res: IKeyBackupInfo;
@@ -3341,6 +3343,8 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
 
     /**
      * Disable backing up of keys.
+     *
+     * @deprecated It should be unnecessary to disable key backup.
      */
     public disableKeyBackup(): void {
         if (!this.crypto) {
@@ -3360,6 +3364,8 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
      *
      * @returns Object that can be passed to createKeyBackupVersion and
      *     additionally has a 'recovery_key' member with the user-facing recovery key string.
+     *
+     * @deprecated Use {@link Crypto.CryptoApi.resetKeyBackup | `CryptoApi.resetKeyBackup`}.
      */
     public async prepareKeyBackupVersion(
         password?: string | Uint8Array | null,
@@ -3403,6 +3409,8 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
      *
      * @param info - Info object from prepareKeyBackupVersion
      * @returns Object with 'version' param indicating the version created
+     *
+     * @deprecated Use {@link Crypto.CryptoApi.resetKeyBackup | `CryptoApi.resetKeyBackup`}.
      */
     public async createKeyBackupVersion(info: IKeyBackupInfo): Promise<IKeyBackupInfo> {
         if (!this.crypto) {
@@ -3448,24 +3456,15 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         return res;
     }
 
+    /**
+     * @deprecated Use {@link Crypto.CryptoApi.deleteKeyBackupVersion | `CryptoApi.deleteKeyBackupVersion`}.
+     */
     public async deleteKeyBackupVersion(version: string): Promise<void> {
-        if (!this.crypto) {
+        if (!this.cryptoBackend) {
             throw new Error("End-to-end encryption disabled");
         }
 
-        // If we're currently backing up to this backup... stop.
-        // (We start using it automatically in createKeyBackupVersion
-        // so this is symmetrical).
-        // TODO: convert this to use crypto.getActiveSessionBackupVersion. And actually check the version.
-        if (this.crypto.backupManager.version) {
-            this.crypto.backupManager.disableKeyBackup();
-        }
-
-        const path = utils.encodeUri("/room_keys/version/$version", {
-            $version: version,
-        });
-
-        await this.http.authedRequest(Method.Delete, path, undefined, undefined, { prefix: ClientPrefix.V3 });
+        await this.cryptoBackend.deleteKeyBackupVersion(version);
     }
 
     private makeKeyBackupPath(roomId: undefined, sessionId: undefined, version?: string): IKeyBackupPath;

src/crypto-api.ts

@@ -382,6 +382,24 @@ export interface CryptoApi {
      *   and trust information (as returned by {@link isKeyBackupTrusted}).
      */
     checkKeyBackupAndEnable(): Promise<KeyBackupCheck | null>;
+
+    /**
+     * Creates a new key backup version.
+     *
+     * If there are existing backups they will be replaced.
+     *
+     * The decryption key will be saved in Secret Storage (the {@link SecretStorageCallbacks.getSecretStorageKey} Crypto
+     * callback will be called)
+     * and the backup engine will be started.
+     */
+    resetKeyBackup(): Promise<void>;
+
+    /**
+     * Deletes the given key backup.
+     *
+     * @param version - The backup version to delete.
+     */
+    deleteKeyBackupVersion(version: string): Promise<void>;
 }
 
 /**

src/crypto/backup.ts

@@ -25,7 +25,7 @@ import { MEGOLM_ALGORITHM, verifySignature } from "./olmlib";
 import { DeviceInfo } from "./deviceinfo";
 import { DeviceTrustLevel } from "./CrossSigning";
 import { keyFromPassphrase } from "./key_passphrase";
-import { safeSet, sleep } from "../utils";
+import { encodeUri, safeSet, sleep } from "../utils";
 import { IndexedDBCryptoStore } from "./store/indexeddb-crypto-store";
 import { encodeRecoveryKey } from "./recoverykey";
 import { calculateKeyCheck, decryptAES, encryptAES, IEncryptedPayload } from "./aes";
@@ -39,7 +39,7 @@ import {
 import { UnstableValue } from "../NamespacedValue";
 import { CryptoEvent } from "./index";
 import { crypto } from "./crypto";
-import { HTTPError, MatrixError } from "../http-api";
+import { ClientPrefix, HTTPError, MatrixError, Method } from "../http-api";
 import { BackupTrustInfo } from "../crypto-api/keybackup";
 
 const KEY_BACKUP_KEYS_PER_REQUEST = 200;
@@ -224,6 +224,33 @@ export class BackupManager {
         this.algorithm = await BackupManager.makeAlgorithm(info, this.getKey);
     }
 
+    /**
+     * Deletes all key backups.
+     *
+     * Will call the API to delete active backup until there is no more present.
+     */
+    public async deleteAllKeyBackupVersions(): Promise<void> {
+        // there could be several backup versions, delete all to be safe.
+        let current = (await this.baseApis.getKeyBackupVersion())?.version ?? null;
+        while (current != null) {
+            await this.deleteKeyBackupVersion(current);
+            this.disableKeyBackup();
+            current = (await this.baseApis.getKeyBackupVersion())?.version ?? null;
+        }
+    }
+
+    /**
+     * Deletes the given key backup.
+     *
+     * @param version - The backup version to delete.
+     */
+    public async deleteKeyBackupVersion(version: string): Promise<void> {
+        const path = encodeUri("/room_keys/version/$version", { $version: version });
+        await this.baseApis.http.authedRequest<void>(Method.Delete, path, undefined, undefined, {
+            prefix: ClientPrefix.V3,
+        });
+    }
+
     /**
      * Check the server for an active key backup and
      * if one is present and has a valid signature from
@@ -333,7 +360,7 @@ export class BackupManager {
         };
 
         if (!backupInfo || !backupInfo.algorithm || !backupInfo.auth_data || !backupInfo.auth_data.signatures) {
-            logger.info("Key backup is absent or missing required data");
+            logger.info(`Key backup is absent or missing required data: ${JSON.stringify(backupInfo)}`);
             return ret;
         }
 

src/crypto/index.ts

@@ -98,6 +98,7 @@ import {
 } from "../crypto-api";
 import { Device, DeviceMap } from "../models/device";
 import { deviceInfoToDevice } from "./device-converter";
+import { ClientPrefix, Method } from "../http-api";
 
 /* re-exports for backwards compatibility */
 export type {
@@ -1088,7 +1089,7 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
                 // and want to write to the local secretStorage object
                 { secureSecretStorage: false },
             );
-            // write the key ourselves to 4S
+            // write the key to 4S
             const privateKey = decodeRecoveryKey(info.recovery_key);
             await secretStorage.store("m.megolm_backup.v1", olmlib.encodeBase64(privateKey));
 
@@ -1145,6 +1146,48 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
         logger.log("Secure Secret Storage ready");
     }
 
+    /**
+     * Implementation of {@link CryptoApi#resetKeyBackup}.
+     */
+    public async resetKeyBackup(): Promise<void> {
+        // Delete existing ones
+        // There is no use case for having several key backup version live server side.
+        // Even if not deleted it would be lost as the key to restore is lost.
+        // There should be only one backup at a time.
+        await this.backupManager.deleteAllKeyBackupVersions();
+
+        const info = await this.backupManager.prepareKeyBackupVersion();
+
+        await this.signObject(info.auth_data);
+
+        // add new key backup
+        const { version } = await this.baseApis.http.authedRequest<{ version: string }>(
+            Method.Post,
+            "/room_keys/version",
+            undefined,
+            info,
+            {
+                prefix: ClientPrefix.V3,
+            },
+        );
+
+        logger.log(`Created backup version ${version}`);
+
+        // write the key to 4S
+        const privateKey = info.privateKey;
+        await this.secretStorage.store("m.megolm_backup.v1", olmlib.encodeBase64(privateKey));
+        await this.storeSessionBackupPrivateKey(privateKey);
+
+        await this.backupManager.checkAndStart();
+    }
+
+    /**
+     * Implementation of {@link CryptoApi#deleteKeyBackupVersion}.
+     */
+    public async deleteKeyBackupVersion(version: string): Promise<void> {
+        await this.backupManager.deleteKeyBackupVersion(version);
+    }
+
     /**
      * @deprecated Use {@link MatrixClient#secretStorage} and {@link SecretStorage.ServerSideSecretStorage#addKey}.
      */

src/rust-crypto/rust-crypto.ts

@@ -938,6 +938,20 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
         return await this.backupManager.checkKeyBackupAndEnable(true);
     }
 
+    /**
+     * Implementation of {@link CryptoApi#resetKeyBackup}.
+     */
+    public async resetKeyBackup(): Promise<void> {
+        // stub
+    }
+
+    /**
+     * Implementation of {@link CryptoApi#deleteKeyBackupVersion}.
+     */
+    public async deleteKeyBackupVersion(version: string): Promise<void> {
+        // stub
+    }
+
     ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////
     //
     // SyncCryptoCallbacks implementation