Don't run migration for Rust crypto if the legacy store is empty (#4218)

* Don't run migration for Rust crypto if the legacy store is empty Fixes element-hq/element-web#27447 * Add copyright for the TypeScript files in legacy DB dumps * Provide a type for the accountPickle we check for before migration * Remove redundant backup response This is unused * Simplify keys response * Downgrade log message. --------- Co-authored-by: Richard van der Hoff <[email protected]>
matrix-org · May 28, 2024 · 909caab · 909caab
1 parent 7c87625
commit 909caab
Show file tree

Hide file tree

Showing 8 changed files with 153 additions and 0 deletions.
diff --git a/spec/integ/crypto/rust-crypto.spec.ts b/spec/integ/crypto/rust-crypto.spec.ts
@@ -23,6 +23,7 @@ import { populateStore } from "../../test-utils/test_indexeddb_cryptostore_dump"
 import { MSK_NOT_CACHED_DATASET } from "../../test-utils/test_indexeddb_cryptostore_dump/no_cached_msk_dump";
 import { IDENTITY_NOT_TRUSTED_DATASET } from "../../test-utils/test_indexeddb_cryptostore_dump/unverified";
 import { FULL_ACCOUNT_DATASET } from "../../test-utils/test_indexeddb_cryptostore_dump/full_account";
+import { EMPTY_ACCOUNT_DATASET } from "../../test-utils/test_indexeddb_cryptostore_dump/empty_account";
 
 jest.setTimeout(15000);
 
@@ -304,6 +305,38 @@ describe("MatrixClient.initRustCrypto", () => {
             });
         });
 
+        it("should not migrate if account data is missing", async () => {
+            // See https://github.com/element-hq/element-web/issues/27447
+
+            // Given we have an almost-empty legacy account in the database
+            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", EMPTY_ACCOUNT_DATASET.keyQueryResponse);
+
+            const testStoreName = "test-store";
+            await populateStore(testStoreName, EMPTY_ACCOUNT_DATASET.dumpPath);
+            const cryptoStore = new IndexedDBCryptoStore(indexedDB, testStoreName);
+
+            const matrixClient = createClient({
+                baseUrl: "http://test.server",
+                userId: EMPTY_ACCOUNT_DATASET.userId,
+                deviceId: EMPTY_ACCOUNT_DATASET.deviceId,
+                cryptoStore,
+                pickleKey: EMPTY_ACCOUNT_DATASET.pickleKey,
+            });
+
+            // When we start Rust crypto, potentially triggering an upgrade
+            const progressListener = jest.fn();
+            matrixClient.addListener(CryptoEvent.LegacyCryptoStoreMigrationProgress, progressListener);
+
+            await matrixClient.initRustCrypto();
+
+            // Then no error occurs, and no upgrade happens
+            expect(progressListener.mock.calls.length).toBe(0);
+        }, 60000);
+
         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/empty_account/README.md b/spec/test-utils/test_indexeddb_cryptostore_dump/empty_account/README.md
@@ -0,0 +1,10 @@
+## Dump of an empty libolm indexeddb cryptostore to test skipping migration
+
+A dump of an account which is almost completely empty, and totally unsuitable
+for use as a real account.
+
+This dump was manually created by copying and editing full_account.
+
+Created to test
+["Unable to restore session" error due due to half-initialised legacy indexeddb crypto store #27447](https://github.com/element-hq/element-web/issues/27447).
+We should not launch the Rust migration code when we find a DB in this state.
diff --git a/spec/test-utils/test_indexeddb_cryptostore_dump/empty_account/dump.json b/spec/test-utils/test_indexeddb_cryptostore_dump/empty_account/dump.json
@@ -0,0 +1,14 @@
+{
+    "account": [],
+    "device_data": [],
+    "inbound_group_sessions": [],
+    "inbound_group_sessions_withheld": [],
+    "notified_error_devices": [],
+    "outgoingRoomKeyRequests": [],
+    "parked_shared_history": [],
+    "rooms": [],
+    "session_problems": [],
+    "sessions": [],
+    "sessions_needing_backup": [],
+    "shared_history_inbound_group_sessions": []
+}
diff --git a/spec/test-utils/test_indexeddb_cryptostore_dump/empty_account/index.ts b/spec/test-utils/test_indexeddb_cryptostore_dump/empty_account/index.ts
@@ -0,0 +1,35 @@
+/*
+Copyright 2024 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { DumpDataSetInfo } from "../index";
+
+/**
+ * A key query response containing the current keys of the tested user.
+ * To be used during tests with fetchmock.
+ */
+const KEYS_QUERY_RESPONSE = { device_keys: { "@emptyuser:example.com": {} } };
+
+/**
+ * A dataset containing the information for the tested user.
+ * To be used during tests.
+ */
+export const EMPTY_ACCOUNT_DATASET: DumpDataSetInfo = {
+    userId: "@emptyuser:example.com",
+    deviceId: "EMPTYDEVIC",
+    pickleKey: "+/bcdefghijklmnopqrstu1/zyxvutsrqponmlkjih2",
+    keyQueryResponse: KEYS_QUERY_RESPONSE,
+    dumpPath: "spec/test-utils/test_indexeddb_cryptostore_dump/empty_account/dump.json",
+};
diff --git a/spec/test-utils/test_indexeddb_cryptostore_dump/full_account/index.ts b/spec/test-utils/test_indexeddb_cryptostore_dump/full_account/index.ts
@@ -1,3 +1,19 @@
+/*
+Copyright 2024 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 import { DumpDataSetInfo } from "../index";
 
 /**

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
@@ -1,3 +1,19 @@
+/*
+Copyright 2024 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 import { KeyBackupInfo } from "../../../../src/crypto-api/keybackup";
 import { DumpDataSetInfo } from "../index";
 

diff --git a/spec/test-utils/test_indexeddb_cryptostore_dump/unverified/index.ts b/spec/test-utils/test_indexeddb_cryptostore_dump/unverified/index.ts
@@ -1,3 +1,19 @@
+/*
+Copyright 2024 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 import { DumpDataSetInfo } from "../index";
 
 /**

diff --git a/src/rust-crypto/libolm_migration.ts b/src/rust-crypto/libolm_migration.ts
@@ -83,6 +83,19 @@ export async function migrateFromLegacyCrypto(args: {
     }
 
     await legacyStore.startup();
+
+    let accountPickle: string | null = null;
+    await legacyStore.doTxn("readonly", [IndexedDBCryptoStore.STORE_ACCOUNT], (txn) => {
+        legacyStore.getAccount(txn, (acctPickle) => {
+            accountPickle = acctPickle;
+        });
+    });
+    if (!accountPickle) {
+        // This store is not properly set up. Nothing to migrate.
+        logger.debug("Legacy crypto store is not set up (no account found). Not migrating.");
+        return;
+    }
+
     let migrationState = await legacyStore.getMigrationState();
 
     if (migrationState >= MigrationState.MEGOLM_SESSIONS_MIGRATED) {