Rust crypto: ensure we persist the key backup version (#3770)

Fixes element-hq/element-web#26259
matrix-org · Oct 4, 2023 · 95baccf · 95baccf
1 parent 10b6c24
commit 95baccf
Show file tree

Hide file tree

Showing 7 changed files with 83 additions and 11 deletions.
diff --git a/spec/integ/crypto/megolm-backup.spec.ts b/spec/integ/crypto/megolm-backup.spec.ts
@@ -163,15 +163,31 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe
             },
         };
 
-        fetchMock.get(
-            "express:/_matrix/client/v3/room_keys/keys/:room_id/:session_id",
-            testData.CURVE25519_KEY_BACKUP_DATA,
-        );
+        fetchMock.get("express:/_matrix/client/v3/room_keys/keys/:room_id/:session_id", (url, request) => {
+            // check that the version is correct
+            const version = new URLSearchParams(new URL(url).search).get("version");
+            if (version == "1") {
+                return testData.CURVE25519_KEY_BACKUP_DATA;
+            } else {
+                return {
+                    status: 403,
+                    body: {
+                        current_version: "1",
+                        errcode: "M_WRONG_ROOM_KEYS_VERSION",
+                        error: "Wrong backup version.",
+                    },
+                };
+            }
+        });
+
         fetchMock.get("path:/_matrix/client/v3/room_keys/version", testData.SIGNED_BACKUP_DATA);
 
         aliceClient = await initTestClient();
         const aliceCrypto = aliceClient.getCrypto()!;
-        await aliceCrypto.storeSessionBackupPrivateKey(Buffer.from(testData.BACKUP_DECRYPTION_KEY_BASE64, "base64"));
+        await aliceCrypto.storeSessionBackupPrivateKey(
+            Buffer.from(testData.BACKUP_DECRYPTION_KEY_BASE64, "base64"),
+            testData.SIGNED_BACKUP_DATA.version!,
+        );
 
         // start after saving the private key
         await aliceClient.startClient();
@@ -645,6 +661,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe
             await aliceClient.startClient();
             await aliceCrypto.storeSessionBackupPrivateKey(
                 Buffer.from(testData.BACKUP_DECRYPTION_KEY_BASE64, "base64"),
+                testData.SIGNED_BACKUP_DATA.version!,
             );
 
             const result = await aliceCrypto.isKeyBackupTrusted(testData.SIGNED_BACKUP_DATA);
@@ -658,6 +675,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe
             await aliceClient.startClient();
             await aliceCrypto.storeSessionBackupPrivateKey(
                 Buffer.from(testData.BACKUP_DECRYPTION_KEY_BASE64, "base64"),
+                testData.SIGNED_BACKUP_DATA.version!,
             );
 
             const backup: KeyBackupInfo = JSON.parse(JSON.stringify(testData.SIGNED_BACKUP_DATA));

diff --git a/spec/unit/crypto/backup.spec.ts b/spec/unit/crypto/backup.spec.ts
@@ -229,6 +229,22 @@ describe("MegolmBackup", function () {
             );
         });
 
+        test("fail if given backup has no version", async () => {
+            const client = makeTestClient(cryptoStore);
+            await client.initCrypto();
+            const data = {
+                algorithm: olmlib.MEGOLM_BACKUP_ALGORITHM,
+                auth_data: {
+                    public_key: "hSDwCYkwp1R0i33ctD73Wg2/Og0mOBr066SpjqqbTmo",
+                },
+            };
+            const key = Uint8Array.from([1, 2, 3, 4, 5, 6, 7, 8]);
+            await client.getCrypto()!.storeSessionBackupPrivateKey(key, "1");
+            await expect(client.restoreKeyBackupWithCache(undefined, undefined, data)).rejects.toThrow(
+                "Backup version must be defined",
+            );
+        });
+
         it("automatically calls the key back up", function () {
             const groupSession = new Olm.OutboundGroupSession();
             groupSession.create();

diff --git a/spec/unit/rust-crypto/rust-crypto.spec.ts b/spec/unit/rust-crypto/rust-crypto.spec.ts
@@ -783,10 +783,23 @@ describe("RustCrypto", () => {
         it("can save and restore a key", async () => {
             const key = "testtesttesttesttesttesttesttest";
             const rustCrypto = await makeTestRustCrypto();
-            await rustCrypto.storeSessionBackupPrivateKey(new TextEncoder().encode(key));
+            await rustCrypto.storeSessionBackupPrivateKey(
+                new TextEncoder().encode(key),
+                testData.SIGNED_BACKUP_DATA.version!,
+            );
             const fetched = await rustCrypto.getSessionBackupPrivateKey();
             expect(new TextDecoder().decode(fetched!)).toEqual(key);
         });
+
+        it("fails to save a key if version not provided", async () => {
+            const key = "testtesttesttesttesttesttesttest";
+            const rustCrypto = await makeTestRustCrypto();
+            await expect(() => rustCrypto.storeSessionBackupPrivateKey(new TextEncoder().encode(key))).rejects.toThrow(
+                "storeSessionBackupPrivateKey: version is required",
+            );
+            const fetched = await rustCrypto.getSessionBackupPrivateKey();
+            expect(fetched).toBeNull();
+        });
     });
 
     describe("getActiveSessionBackupVersion", () => {

diff --git a/src/client.ts b/src/client.ts
@@ -3864,6 +3864,10 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
             throw new Error("End-to-end encryption disabled");
         }
 
+        if (!backupInfo.version) {
+            throw new Error("Backup version must be defined");
+        }
+
         let totalKeyCount = 0;
         let keys: IMegolmSessionData[] = [];
 
@@ -3881,7 +3885,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
             // Cache the key, if possible.
             // This is async.
             this.cryptoBackend
-                .storeSessionBackupPrivateKey(privKey)
+                .storeSessionBackupPrivateKey(privKey, backupInfo.version)
                 .catch((e) => {
                     logger.warn("Error caching session backup key:", e);
                 })

diff --git a/src/crypto-api.ts b/src/crypto-api.ts
@@ -372,11 +372,25 @@ export interface CryptoApi {
      * Store the backup decryption key.
      *
      * This should be called if the client has received the key from another device via secret sharing (gossiping).
+     * It is the responsability of the caller to check that the decryption key is valid for the current backup version.
      *
      * @param key - the backup decryption key
+     *
+     * @deprecated prefer the variant with a `version` parameter.
      */
     storeSessionBackupPrivateKey(key: Uint8Array): Promise<void>;
 
+    /**
+     * Store the backup decryption key.
+     *
+     * This should be called if the client has received the key from another device via secret sharing (gossiping).
+     * It is the responsability of the caller to check that the decryption key is valid for the given backup version.
+     *
+     * @param key - the backup decryption key
+     * @param version - the backup version corresponding to this decryption key
+     */
+    storeSessionBackupPrivateKey(key: Uint8Array, version: string): Promise<void>;
+
     /**
      * Get the current status of key backup.
      *

diff --git a/src/crypto/index.ts b/src/crypto/index.ts
@@ -1317,7 +1317,7 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
      * @param key - the private key
      * @returns a promise so you can catch failures
      */
-    public async storeSessionBackupPrivateKey(key: ArrayLike<number>): Promise<void> {
+    public async storeSessionBackupPrivateKey(key: ArrayLike<number>, version?: string): Promise<void> {
         if (!(key instanceof Uint8Array)) {
             // eslint-disable-next-line @typescript-eslint/no-base-to-string
             throw new Error(`storeSessionBackupPrivateKey expects Uint8Array, got ${key}`);

diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts
@@ -1005,12 +1005,19 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
      * Implementation of {@link CryptoApi#storeSessionBackupPrivateKey}.
      *
      * @param key - the backup decryption key
+     * @param version - the backup version for this key.
      */
-    public async storeSessionBackupPrivateKey(key: Uint8Array): Promise<void> {
+    public async storeSessionBackupPrivateKey(key: Uint8Array, version?: string): Promise<void> {
         const base64Key = encodeBase64(key);
 
-        // TODO get version from backupManager
-        await this.olmMachine.saveBackupDecryptionKey(RustSdkCryptoJs.BackupDecryptionKey.fromBase64(base64Key), "");
+        if (!version) {
+            throw new Error("storeSessionBackupPrivateKey: version is required");
+        }
+
+        await this.olmMachine.saveBackupDecryptionKey(
+            RustSdkCryptoJs.BackupDecryptionKey.fromBase64(base64Key),
+            version,
+        );
     }
 
     /**