Key backup: clean up SecureKeyBackup.prepareKeyBackupVersion

spec/integ/crypto/megolm-backup.spec.ts

@@ -225,6 +225,53 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe
         expect(backupStatus).toStrictEqual(testData.SIGNED_BACKUP_DATA.version);
     });
 
+    oldBackendOnly("test backup version creation", async function () {
+        // 404 means that there is no active backup
+        fetchMock.get("/_matrix/client/v3/room_keys/version", 404);
+
+        aliceClient = await initTestClient();
+        await aliceClient.startClient();
+
+        const preparedBackup = await aliceClient.prepareKeyBackupVersion();
+
+        // The prepared backup should be signed
+        // Only device signature as cross signing is not bootstraped
+        // TODO improvement bootstrap cross signing to check for the added signature
+        expect(preparedBackup?.auth_data.signatures?.[TEST_USER_ID]).toBeDefined();
+        expect(preparedBackup?.auth_data.signatures?.[TEST_USER_ID]?.[`ed25519:${TEST_DEVICE_ID}`]).toBeDefined();
+
+        // mock backup creation API
+        const backupVersion = "1";
+        const expectedBackupResponse = {
+            algorithm: preparedBackup.algorithm,
+            auth_data: preparedBackup.auth_data,
+            version: backupVersion,
+        };
+        fetchMock.post("express:/_matrix/client/v3/room_keys/version", expectedBackupResponse);
+        fetchMock.get("express:/_matrix/client/v3/room_keys/version", expectedBackupResponse, {
+            overwriteRoutes: true,
+        });
+
+        const backupEnabled = new Promise<void>((resolve, reject) => {
+            aliceClient.on(CryptoEvent.KeyBackupStatus, (enabled) => {
+                if (enabled) {
+                    resolve();
+                }
+            });
+        });
+
+        await aliceClient.createKeyBackupVersion({
+            algorithm: preparedBackup.algorithm,
+            auth_data: preparedBackup.auth_data,
+        });
+
+        await backupEnabled;
+
+        const backupStatus = aliceClient.getCrypto()!.getActiveSessionBackupVersion();
+        expect(backupStatus).toBeDefined();
+        expect(backupStatus).toStrictEqual(backupVersion);
+    });
+
     /** make sure that the client knows about the dummy device */
     async function waitForDeviceList(): Promise<void> {
         // Completing the initial sync will make the device list download outdated device lists (of which our own

src/client.ts

@@ -3354,13 +3354,14 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         password?: string | Uint8Array | null,
         opts: IKeyBackupPrepareOpts = { secureSecretStorage: false },
     ): Promise<Pick<IPreparedKeyBackupVersion, "algorithm" | "auth_data" | "recovery_key">> {
-        if (!this.crypto) {
+        if (!this.getCrypto()) {
             throw new Error("End-to-end encryption disabled");
         }
 
         // eslint-disable-next-line camelcase
-        const { algorithm, auth_data, recovery_key, privateKey } =
-            await this.crypto.backupManager.prepareKeyBackupVersion(password);
+        const { algorithm, auth_data, recovery_key, privateKey } = await this.getCrypto()!.prepareKeyBackupVersion(
+            password,
+        );
 
         if (opts.secureSecretStorage) {
             await this.secretStorage.store("m.megolm_backup.v1", encodeBase64(privateKey));
@@ -3394,36 +3395,17 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
      * @returns Object with 'version' param indicating the version created
      */
     public async createKeyBackupVersion(info: IKeyBackupInfo): Promise<IKeyBackupInfo> {
-        if (!this.crypto) {
+        if (!this.getCrypto()) {
             throw new Error("End-to-end encryption disabled");
         }
 
-        await this.crypto.backupManager.createKeyBackupVersion(info);
+        await this.getCrypto()!.getBackupManager().createKeyBackupVersion(info);
 
         const data = {
             algorithm: info.algorithm,
             auth_data: info.auth_data,
         };
 
-        // Sign the backup auth data with the device key for backwards compat with
-        // older devices with cross-signing. This can probably go away very soon in
-        // favour of just signing with the cross-singing master key.
-        // XXX: Private member access
-        await this.crypto.signObject(data.auth_data);
-
-        if (
-            this.cryptoCallbacks.getCrossSigningKey &&
-            // XXX: Private member access
-            this.crypto.crossSigningInfo.getId()
-        ) {
-            // now also sign the auth data with the cross-signing master key
-            // we check for the callback explicitly here because we still want to be able
-            // to create an un-cross-signed key backup if there is a cross-signing key but
-            // no callback supplied.
-            // XXX: Private member access
-            await this.crypto.crossSigningInfo.signObject(data.auth_data, "master");
-        }
-
         const res = await this.http.authedRequest<IKeyBackupInfo>(Method.Post, "/room_keys/version", undefined, data, {
             prefix: ClientPrefix.V3,
         });

src/common-crypto/SecureKeyBackup.ts

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { KeyBackupInfo } from "../crypto-api/keybackup";
+import { IPreparedKeyBackupVersion, KeyBackupInfo } from "../crypto-api/keybackup";
 
 /**
  * Interface to server-side key backup
@@ -42,6 +42,24 @@ export interface SecureKeyBackup {
      *    (or lack thereof).
      */
     checkAndStart(): Promise<KeyBackupCheck | null>;
+
+    /**
+     * Set up the data required to create a new backup version.  The backup version
+     * will not be created and enabled until createKeyBackupVersion is called.
+     *
+     * @param password - Passphrase string that can be entered by the user
+     *     when restoring the backup as an alternative to entering the recovery key.
+     *     Optional. If null a random recovery key will be created
+     *
+     * @returns Object that can be passed to createKeyBackupVersion and
+     *     additionally has a 'recovery_key' member with the user-facing recovery key string. The backup data is not yet signed, the cryptoBackend will do it.
+     */
+    prepareUnsignedKeyBackupVersion(
+        key?: string | Uint8Array | null,
+        algorithm?: string | undefined,
+    ): Promise<IPreparedKeyBackupVersion>;
+
+    createKeyBackupVersion(info: KeyBackupInfo): Promise<void>;
 }
 
 /**

src/crypto-api.ts

@@ -21,6 +21,7 @@ import { UIAuthCallback } from "./interactive-auth";
 import { AddSecretStorageKeyOpts, SecretStorageCallbacks, SecretStorageKeyDescription } from "./secret-storage";
 import { VerificationRequest } from "./crypto-api/verification";
 import { KeyBackupInfo } from "./crypto-api/keybackup";
+import { IPreparedKeyBackupVersion } from "./crypto/backup";
 
 /**
  * Public interface to the cryptography parts of the js-sdk
@@ -331,6 +332,15 @@ export interface CryptoApi {
      * @returns If automatic key backups are enabled, the `version` of the active backup. Otherwise, `null`.
      */
     getActiveSessionBackupVersion(): Promise<string | null>;
+
+    /**
+     * Prepare a backup version, signed with current device and identity (if available).
+     * Can be used to upload a new backup.
+     */
+    prepareKeyBackupVersion(
+        key?: string | Uint8Array | null,
+        algorithm?: string | undefined,
+    ): Promise<IPreparedKeyBackupVersion>;
 }
 
 /**

src/crypto-api/keybackup.ts

@@ -43,3 +43,18 @@ export interface KeyBackupInfo {
     etag?: string;
     version?: string; // number contained within
 }
+
+export type AuthData = KeyBackupInfo["auth_data"];
+
+/**
+ * Prepared backup data.
+ * Contains the data needed to create a new version.
+ */
+/* eslint-disable camelcase */
+export interface IPreparedKeyBackupVersion {
+    algorithm: string;
+    auth_data: AuthData;
+    recovery_key: string;
+    privateKey: Uint8Array;
+}
+/* eslint-enable camelcase */

src/crypto/backup.ts

@@ -36,17 +36,19 @@ import {
     IKeyBackupInfo,
     IKeyBackupSession,
 } from "./keybackup";
+import { AuthData, IPreparedKeyBackupVersion } from "../crypto-api";
 import { UnstableValue } from "../NamespacedValue";
 import { CryptoEvent } from "./index";
 import { crypto } from "./crypto";
 import { HTTPError, MatrixError } from "../http-api";
 import { SecureKeyBackup } from "../common-crypto/SecureKeyBackup";
 
+// re-export for backward compatibility
+export type { IPreparedKeyBackupVersion } from "../crypto-api/keybackup";
+
 const KEY_BACKUP_KEYS_PER_REQUEST = 200;
 const KEY_BACKUP_CHECK_RATE_LIMIT = 5000; // ms
 
-type AuthData = IKeyBackupInfo["auth_data"];
-
 type SigInfo = {
     deviceId: string;
     valid?: boolean | null; // true: valid, false: invalid, null: cannot attempt validation
@@ -69,15 +71,6 @@ export interface IKeyBackupCheck {
     trustInfo: TrustInfo;
 }
 
-/* eslint-disable camelcase */
-export interface IPreparedKeyBackupVersion {
-    algorithm: string;
-    auth_data: AuthData;
-    recovery_key: string;
-    privateKey: Uint8Array;
-}
-/* eslint-enable camelcase */
-
 /** A function used to get the secret key for a backup.
  */
 type GetKey = () => Promise<ArrayLike<number>>;
@@ -202,9 +195,17 @@ export class BackupManager implements SecureKeyBackup {
         return Boolean(this.algorithm);
     }
 
+    /** @deprecated Do not use directly, use CryptoApi.prepareKeyBackupVersion()  */
     public async prepareKeyBackupVersion(
         key?: string | Uint8Array | null,
         algorithm?: string | undefined,
+    ): Promise<IPreparedKeyBackupVersion> {
+        return this.prepareUnsignedKeyBackupVersion(key, algorithm);
+    }
+
+    public async prepareUnsignedKeyBackupVersion(
+        key?: string | Uint8Array | null,
+        algorithm?: string | undefined,
     ): Promise<IPreparedKeyBackupVersion> {
         const Algorithm = algorithm ? algorithmsByName[algorithm] : DefaultAlgorithm;
         if (!Algorithm) {

src/crypto/index.ts

@@ -92,6 +92,7 @@ import {
     DeviceVerificationStatus,
     ImportRoomKeysOpts,
     VerificationRequest as CryptoApiVerificationRequest,
+    IPreparedKeyBackupVersion,
 } from "../crypto-api";
 import { Device, DeviceMap } from "../models/device";
 import { deviceInfoToDevice } from "./device-converter";
@@ -539,6 +540,29 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
         }
     }
 
+    public async prepareKeyBackupVersion(
+        key?: string | Uint8Array | null | undefined,
+        algorithm?: string | undefined,
+    ): Promise<IPreparedKeyBackupVersion> {
+        const preparedVersion = await this.backupManager.prepareUnsignedKeyBackupVersion(key, algorithm);
+
+        // sign the auth_data with existing device and cross signing keys if available
+        await this.signObject(preparedVersion.auth_data);
+        if (this.crossSigningInfo.getId() && (await this.crossSigningInfo.isStoredInKeyCache("master"))) {
+            try {
+                logger.log("Adding cross-signing signature to key backup");
+                await this.crossSigningInfo.signObject(preparedVersion.auth_data, "master");
+            } catch (e) {
+                // This step is not critical (just helpful), so we catch here
+                // and continue if it fails.
+                logger.error("Signing key backup with cross-signing keys failed", e);
+            }
+        }
+
+        // return the now signed version
+        return preparedVersion;
+    }
+
     /**
      * Initialise the crypto module so that it is ready for use
      *
@@ -977,21 +1001,6 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
             }
         };
 
-        const signKeyBackupWithCrossSigning = async (keyBackupAuthData: IKeyBackupInfo["auth_data"]): Promise<void> => {
-            if (this.crossSigningInfo.getId() && (await this.crossSigningInfo.isStoredInKeyCache("master"))) {
-                try {
-                    logger.log("Adding cross-signing signature to key backup");
-                    await this.crossSigningInfo.signObject(keyBackupAuthData, "master");
-                } catch (e) {
-                    // This step is not critical (just helpful), so we catch here
-                    // and continue if it fails.
-                    logger.error("Signing key backup with cross-signing keys failed", e);
-                }
-            } else {
-                logger.warn("Cross-signing keys not available, skipping signature on key backup");
-            }
-        };
-
         const oldSSSSKey = await this.secretStorage.getKey();
         const [oldKeyId, oldKeyInfo] = oldSSSSKey || [null, null];
         const storageExists =
@@ -1045,11 +1054,6 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
             // store the backup key in secret storage
             await secretStorage.store("m.megolm_backup.v1", olmlib.encodeBase64(backupKey!), [newKeyId]);
 
-            // The backup is trusted because the user provided the private key.
-            // Sign the backup with the cross-signing key so the key backup can
-            // be trusted via cross-signing.
-            await signKeyBackupWithCrossSigning(keyBackupInfo.auth_data);
-
             builder.addSessionBackup(keyBackupInfo);
         } else {
             // 4S is already set up
@@ -1095,12 +1099,6 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
                 auth_data: info.auth_data,
             };
 
-            // Sign with cross-signing master key
-            await signKeyBackupWithCrossSigning(data.auth_data);
-
-            // sign with the device fingerprint
-            await this.signObject(data.auth_data);
-
             builder.addSessionBackup(data);
         }
 

src/rust-crypto/backup.ts

@@ -15,6 +15,7 @@ limitations under the License.
 */
 
 import { KeyBackupCheck, SecureKeyBackup } from "../common-crypto/SecureKeyBackup";
+import { IPreparedKeyBackupVersion, KeyBackupInfo } from "../crypto-api/keybackup";
 
 export class RustBackupManager implements SecureKeyBackup {
     public async checkAndStart(): Promise<KeyBackupCheck | null> {
@@ -28,4 +29,16 @@ export class RustBackupManager implements SecureKeyBackup {
         // TODO stub
         return null;
     }
+
+    public async prepareUnsignedKeyBackupVersion(
+        key?: string | Uint8Array | null | undefined,
+        algorithm?: string | undefined,
+    ): Promise<IPreparedKeyBackupVersion> {
+        throw new Error("Method not implemented.");
+    }
+
+    public async createKeyBackupVersion(info: KeyBackupInfo): Promise<void> {
+        //TODO stub
+        return;
+    }
 }

src/rust-crypto/rust-crypto.ts

@@ -38,6 +38,7 @@ import {
     CryptoCallbacks,
     DeviceVerificationStatus,
     GeneratedSecretStorageKey,
+    IPreparedKeyBackupVersion,
     ImportRoomKeyProgressData,
     ImportRoomKeysOpts,
     VerificationRequest,
@@ -755,6 +756,19 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
         return await this.backupManager.getActiveBackupVersion();
     }
 
+    public async prepareKeyBackupVersion(
+        key?: string | Uint8Array | null | undefined,
+        algorithm?: string | undefined,
+    ): Promise<IPreparedKeyBackupVersion> {
+        const preparedVersion = await this.backupManager.prepareUnsignedKeyBackupVersion(key, algorithm);
+
+        // sign the auth_data with existing device and cross signing keys if available
+        await this.signObject(preparedVersion.auth_data);
+
+        // return the now signed version
+        return preparedVersion;
+    }
+
     ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////
     //
     // SyncCryptoCallbacks implementation