From 725bcf5b2ffcf7ad5638ea54cd713a9039531df8 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 24 May 2024 13:25:08 +0100 Subject: [PATCH] Don't run migration for Rust crypto if the legacy store is empty Fixes https://github.com/element-hq/element-web/issues/27447 --- spec/integ/crypto/rust-crypto.spec.ts | 31 +++++++++++++++++++ .../empty_account/README.md | 10 ++++++ .../empty_account/dump.json | 14 +++++++++ .../empty_account/index.ts | 31 +++++++++++++++++++ src/rust-crypto/libolm_migration.ts | 13 ++++++++ 5 files changed, 99 insertions(+) create mode 100644 spec/test-utils/test_indexeddb_cryptostore_dump/empty_account/README.md create mode 100644 spec/test-utils/test_indexeddb_cryptostore_dump/empty_account/dump.json create mode 100644 spec/test-utils/test_indexeddb_cryptostore_dump/empty_account/index.ts diff --git a/spec/integ/crypto/rust-crypto.spec.ts b/spec/integ/crypto/rust-crypto.spec.ts index c6b990fed0f..a8c94e7a5be 100644 --- 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,36 @@ 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", EMPTY_ACCOUNT_DATASET.backupResponse); + + 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 { 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 new file mode 100644 index 00000000000..df3ab87e55c --- /dev/null +++ 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 new file mode 100644 index 00000000000..10227c8bc43 --- /dev/null +++ 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 new file mode 100644 index 00000000000..c789c65b4e1 --- /dev/null +++ b/spec/test-utils/test_indexeddb_cryptostore_dump/empty_account/index.ts @@ -0,0 +1,31 @@ +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: any = { + device_keys: {}, + master_keys: {}, + self_signing_keys: {}, + user_signing_keys: {}, +}; + +/** + * A `/room_keys/version` response containing the current server-side backup info. + * To be used during tests with fetchmock. + */ +const BACKUP_RESPONSE: any = {}; + +/** + * 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", + backupResponse: BACKUP_RESPONSE, + keyQueryResponse: KEYS_QUERY_RESPONSE, + dumpPath: "spec/test-utils/test_indexeddb_cryptostore_dump/empty_account/dump.json", +}; diff --git a/src/rust-crypto/libolm_migration.ts b/src/rust-crypto/libolm_migration.ts index 2738f88f6c8..61c132ce5c7 100644 --- 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 account; + await legacyStore.doTxn("readonly", [IndexedDBCryptoStore.STORE_ACCOUNT], (txn) => { + legacyStore.getAccount(txn, (acct) => { + account = acct; + }); + }); + if (!account) { + // This store is not properly set up. Nothing to migrate. + logger.warn("Legacy crypto store is not set up (no account found). Not migrating."); + return; + } + let migrationState = await legacyStore.getMigrationState(); if (migrationState >= MigrationState.MEGOLM_SESSIONS_MIGRATED) {