Validate backup private key before migrating it (#4114)

* Migrate own identity trust to rust crypto * Fix gendoc not happy if msk of IDownloadKeyResult has a signature * add missing mock * code review * Code review * Review gh suggestion Co-authored-by: Richard van der Hoff <[email protected]> * Review gh suggestion Co-authored-by: Richard van der Hoff <[email protected]> * Review gh suggestion Co-authored-by: Richard van der Hoff <[email protected]> * Review gh suggestion Co-authored-by: Richard van der Hoff <[email protected]> * review move function down in file * Review gh suggestion Co-authored-by: Richard van der Hoff <[email protected]> * Review gh suggestion Co-authored-by: Richard van der Hoff <[email protected]> * Review: Cleaning tests, renaming * Review: better comment Co-authored-by: Richard van der Hoff <[email protected]> * Comment paragraphs * retry until initial key query is successfull * Validate backup private key before migrating it * post merge fix * Fix test, missing mock * Use crypto wasm instead of lib olm to check backup key * typo * code review * quick lint --------- Co-authored-by: Richard van der Hoff <[email protected]>
matrix-org · Apr 15, 2024 · 8438533 · 8438533
1 parent 475f82c
commit 8438533
Show file tree

Hide file tree

Showing 4 changed files with 164 additions and 6 deletions.
diff --git a/spec/integ/crypto/rust-crypto.spec.ts b/spec/integ/crypto/rust-crypto.spec.ts
@@ -162,6 +162,110 @@ describe("MatrixClient.initRustCrypto", () => {
             expect(progressListener).toHaveBeenLastCalledWith(-1, -1);
         }, 60000);
 
+        describe("Private key backup migration", () => {
+            it("should not migrate the backup private key if backup has changed", async () => {
+                // Here we have a new backup server side, and the migrated account has the previous backup key.
+                fetchMock.get("path:/_matrix/client/v3/room_keys/version", MSK_NOT_CACHED_DATASET.newBackupResponse);
+
+                fetchMock.post("path:/_matrix/client/v3/keys/query", MSK_NOT_CACHED_DATASET.keyQueryResponse);
+
+                await populateStore("test-store", MSK_NOT_CACHED_DATASET.dumpPath);
+                const cryptoStore = new IndexedDBCryptoStore(indexedDB, "test-store");
+
+                const matrixClient = createClient({
+                    baseUrl: "http://test.server",
+                    userId: MSK_NOT_CACHED_DATASET.userId,
+                    deviceId: MSK_NOT_CACHED_DATASET.deviceId,
+                    cryptoStore,
+                    pickleKey: MSK_NOT_CACHED_DATASET.pickleKey,
+                });
+
+                await matrixClient.initRustCrypto();
+
+                const privateBackupKey = await matrixClient.getCrypto()?.getSessionBackupPrivateKey();
+                expect(privateBackupKey).toBeNull();
+            });
+
+            it("should not migrate the backup private key if backup has unknown algorithm", async () => {
+                // Here we have a new backup server side, and the migrated account has the previous backup key.
+                const backupResponse = {
+                    ...MSK_NOT_CACHED_DATASET.backupResponse,
+                    algorithm: "m.megolm_backup.v8",
+                };
+                fetchMock.get("path:/_matrix/client/v3/room_keys/version", backupResponse);
+
+                fetchMock.post("path:/_matrix/client/v3/keys/query", MSK_NOT_CACHED_DATASET.keyQueryResponse);
+
+                await populateStore("test-store", MSK_NOT_CACHED_DATASET.dumpPath);
+                const cryptoStore = new IndexedDBCryptoStore(indexedDB, "test-store");
+
+                const matrixClient = createClient({
+                    baseUrl: "http://test.server",
+                    userId: MSK_NOT_CACHED_DATASET.userId,
+                    deviceId: MSK_NOT_CACHED_DATASET.deviceId,
+                    cryptoStore,
+                    pickleKey: MSK_NOT_CACHED_DATASET.pickleKey,
+                });
+
+                await matrixClient.initRustCrypto();
+
+                const privateBackupKey = await matrixClient.getCrypto()?.getSessionBackupPrivateKey();
+                expect(privateBackupKey).toBeNull();
+            });
+
+            it("should not migrate the backup private key if the backup has been deleted", async () => {
+                // The old backup has been deleted server side.
+                fetchMock.get("path:/_matrix/client/v3/room_keys/version", {
+                    status: 404,
+                    body: {
+                        errcode: "M_NOT_FOUND",
+                        error: "No backup found",
+                    },
+                });
+
+                fetchMock.post("path:/_matrix/client/v3/keys/query", MSK_NOT_CACHED_DATASET.keyQueryResponse);
+
+                await populateStore("test-store", MSK_NOT_CACHED_DATASET.dumpPath);
+                const cryptoStore = new IndexedDBCryptoStore(indexedDB, "test-store");
+
+                const matrixClient = createClient({
+                    baseUrl: "http://test.server",
+                    userId: MSK_NOT_CACHED_DATASET.userId,
+                    deviceId: MSK_NOT_CACHED_DATASET.deviceId,
+                    cryptoStore,
+                    pickleKey: MSK_NOT_CACHED_DATASET.pickleKey,
+                });
+
+                await matrixClient.initRustCrypto();
+
+                const privateBackupKey = await matrixClient.getCrypto()?.getSessionBackupPrivateKey();
+                expect(privateBackupKey).toBeNull();
+            });
+
+            it("should migrate the backup private key if the backup matches", async () => {
+                // The old backup has been deleted server side.
+                fetchMock.get("path:/_matrix/client/v3/room_keys/version", MSK_NOT_CACHED_DATASET.backupResponse);
+
+                fetchMock.post("path:/_matrix/client/v3/keys/query", MSK_NOT_CACHED_DATASET.keyQueryResponse);
+
+                await populateStore("test-store", MSK_NOT_CACHED_DATASET.dumpPath);
+                const cryptoStore = new IndexedDBCryptoStore(indexedDB, "test-store");
+
+                const matrixClient = createClient({
+                    baseUrl: "http://test.server",
+                    userId: MSK_NOT_CACHED_DATASET.userId,
+                    deviceId: MSK_NOT_CACHED_DATASET.deviceId,
+                    cryptoStore,
+                    pickleKey: MSK_NOT_CACHED_DATASET.pickleKey,
+                });
+
+                await matrixClient.initRustCrypto();
+
+                const privateBackupKey = await matrixClient.getCrypto()?.getSessionBackupPrivateKey();
+                expect(privateBackupKey).toBeDefined();
+            });
+        });
+
         describe("Legacy trust migration", () => {
             async function populateAndStartLegacyCryptoStore(dumpPath: string): Promise<IndexedDBCryptoStore> {
                 const testStoreName = "test-store";

diff --git a/spec/test-utils/test_indexeddb_cryptostore_dump/no_cached_msk_dump/index.ts b/spec/test-utils/test_indexeddb_cryptostore_dump/no_cached_msk_dump/index.ts
@@ -245,6 +245,28 @@ const BACKUP_RESPONSE: KeyBackupInfo = {
     count: 0,
 };
 
+/**
+ * This was generated by doing a backup reset on the account.
+ * This is a new valid backup for this account.
+ */
+const NEW_BACKUP_RESPONSE: KeyBackupInfo = {
+    auth_data: {
+        public_key: "CkDxWALi3lcChgjEZFEM6clYq5x768XBwsL++eaOzTI",
+        signatures: {
+            "@migration:localhost": {
+                "ed25519:YVEGEYPYWX":
+                    "ZSYuQDdwgB9WKXQ+z5aWWfqSolBCGRw53kur1Vy956gFefgzCBkMbw5M0I2UgfU2Cukri7jZ4ig201zmLNmaAA",
+                "ed25519:rXCrBin/+xyh+yW//vWte+2UV0et1ZHTWfalp/Ekack":
+                    "+UQ8EA507LoIqgK9rPsqPoGrj+iRBJeY2Oz0mMtXmVf8c1y8G0KWJNUWqvOysnOhsoJf1bt8ey48CxjjtSQ2AA",
+            },
+        },
+    },
+    version: "3",
+    algorithm: "m.megolm_backup.v1.curve25519-aes-sha2",
+    etag: "0",
+    count: 0,
+};
+
 /**
  * A dataset containing the information for the tested user.
  * To be used during tests.
@@ -256,5 +278,6 @@ export const MSK_NOT_CACHED_DATASET: DumpDataSetInfo = {
     keyQueryResponse: KEY_QUERY_RESPONSE,
     rotatedKeyQueryResponse: ROTATED_KEY_QUERY_RESPONSE,
     backupResponse: BACKUP_RESPONSE,
+    newBackupResponse: NEW_BACKUP_RESPONSE,
     dumpPath: "spec/test-utils/test_indexeddb_cryptostore_dump/no_cached_msk_dump/dump.json",
 };
diff --git a/spec/unit/rust-crypto/rust-crypto.spec.ts b/spec/unit/rust-crypto/rust-crypto.spec.ts
@@ -205,7 +205,20 @@ describe("initRustCrypto", () => {
             createMegolmSessions(legacyStore, nDevices, nSessionsPerDevice);
             await legacyStore.markSessionsNeedingBackup([{ senderKey: pad43("device5"), sessionId: "session5" }]);
 
-            fetchMock.get("path:/_matrix/client/v3/room_keys/version", { version: "45" });
+            fetchMock.get("path:/_matrix/client/v3/room_keys/version", {
+                auth_data: {
+                    public_key: "backup_key_public",
+                },
+                version: "45",
+                algorithm: "m.megolm_backup.v1.curve25519-aes-sha2",
+            });
+            // The cached key should be valid for the backup
+            const mockBackupDecryptionKey: any = {
+                megolmV1PublicKey: {
+                    publicKeyBase64: "backup_key_public",
+                },
+            };
+            jest.spyOn(RustSdkCryptoJs.BackupDecryptionKey, "fromBase64").mockReturnValue(mockBackupDecryptionKey);
 
             function legacyMigrationProgressListener(progress: number, total: number): void {
                 logger.log(`migrated ${progress} of ${total}`);

diff --git a/src/rust-crypto/libolm_migration.ts b/src/rust-crypto/libolm_migration.ts
@@ -23,7 +23,7 @@ import { decryptAES, IEncryptedPayload } from "../crypto/aes";
 import { IHttpOpts, MatrixHttpApi } from "../http-api";
 import { requestKeyBackupVersion } from "./backup";
 import { IRoomEncryption } from "../crypto/RoomList";
-import { CrossSigningKeyInfo } from "../crypto-api";
+import { CrossSigningKeyInfo, Curve25519AuthData } from "../crypto-api";
 import { RustCrypto } from "./rust-crypto";
 import { KeyBackupInfo } from "../crypto-api/keybackup";
 import { sleep } from "../utils";
@@ -161,7 +161,8 @@ async function migrateBaseData(
     const recoveryKey = await getAndDecryptCachedSecretKey(legacyStore, pickleKey, "m.megolm_backup.v1");
 
     // If we have a backup recovery key, we need to try to figure out which backup version it is for.
-    // All we can really do is ask the server for the most recent version.
+    // All we can really do is ask the server for the most recent version and check if the cached key we have matches.
+    // It is possible that the backup has changed since last time his session was opened.
     if (recoveryKey) {
         let backupCallDone = false;
         let backupInfo: KeyBackupInfo | null = null;
@@ -175,9 +176,26 @@ async function migrateBaseData(
                 await sleep(2000);
             }
         }
-        if (backupInfo) {
-            migrationData.backupVersion = backupInfo.version;
-            migrationData.backupRecoveryKey = recoveryKey;
+        if (backupInfo && backupInfo.algorithm == "m.megolm_backup.v1.curve25519-aes-sha2") {
+            // check if the recovery key matches, as the active backup version may have changed since the key was cached
+            // and the migration started.
+            try {
+                const decryptionKey = RustSdkCryptoJs.BackupDecryptionKey.fromBase64(recoveryKey);
+                const publicKey = (backupInfo.auth_data as Curve25519AuthData)?.public_key;
+                const isValid = decryptionKey.megolmV1PublicKey.publicKeyBase64 == publicKey;
+                if (isValid) {
+                    migrationData.backupVersion = backupInfo.version;
+                    migrationData.backupRecoveryKey = recoveryKey;
+                } else {
+                    logger.debug(
+                        "The backup key to migrate does not match the active backup version",
+                        `Cached pub key: ${decryptionKey.megolmV1PublicKey.publicKeyBase64}`,
+                        `Active pub key: ${publicKey}`,
+                    );
+                }
+            } catch (e) {
+                logger.warn("Failed to check if the backup key to migrate matches the active backup version", e);
+            }
         }
     }