Key storage out of sync: reset key backup when needed

src/DeviceListener.ts

@@ -1,4 +1,5 @@
 /*
+Copyright 2025 Element Creations Ltd.
 Copyright 2024 New Vector Ltd.
 Copyright 2020 The Matrix.org Foundation C.I.C.
 
@@ -144,6 +145,25 @@
         this.client = undefined;
     }
 
+    /**
+     * Pause the device listener while a function runs.
+     *
+     * This can be done if the function makes several changes that would trigger
+     * multiple events, to suppress warning toasts until the process is
+     * finished.
+     */
+    public async whilePaused(fn: () => Promise<void>): Promise<void> {
+        const client = this.client;
+        try {
+            this.stop();
+            await fn();
+        } finally {
+            if (client) {
+                this.start(client);
+            }
+        }
+    }
+
     /**
      * Dismiss notifications about our own unverified devices
      *
@@ -177,6 +197,67 @@
         await this.client?.setAccountData(RECOVERY_ACCOUNT_DATA_KEY, { enabled: false });
     }
 
+    /**
+     * If a `Kind.KEY_STORAGE_OUT_OF_SYNC` condition from {@link doRecheck}
+     * requires a reset of cross-signing keys.
+     *
+     * We will reset cross-signing keys if both our local cache and 4S don't
+     * have all cross-signing keys.
+     *
+     * In theory, if the set of keys in our cache and in 4S are different, and
+     * we have a complete set between the two, we could be OK, but that
+     * should be exceptionally rare, and is more complicated to detect.
+     */
+    public async keyStorageOutOfSyncNeedsCrossSigningReset(forgotRecovery: boolean): Promise<boolean> {
+        const crypto = this.client?.getCrypto();
+        if (!crypto) {
+            return false;
+        }
+        const crossSigningStatus = await crypto.getCrossSigningStatus();
+        const allCrossSigningSecretsCached =
+            crossSigningStatus.privateKeysCachedLocally.masterKey &&
+            crossSigningStatus.privateKeysCachedLocally.selfSigningKey &&
+            crossSigningStatus.privateKeysCachedLocally.userSigningKey;
+
+        if (forgotRecovery) {
+            return !allCrossSigningSecretsCached;
+        } else {
+            return !allCrossSigningSecretsCached && !crossSigningStatus.privateKeysInSecretStorage;
+        }
+    }
+
+    /**
+     * If a `Kind.KEY_STORAGE_OUT_OF_SYNC` condition from {@link doRecheck}
+     * requires a reset of key backup.
+     *
+     * If the user has their recovery key, we need to reset backup if:
+     * - the user hasn't disabled backup,
+     * - we don't have the backup key cached locally, *and*
+     * - we don't have the backup key stored in 4S.
+     * (The user should already have a key backup created at this point,
+     * otherwise `doRecheck` would have triggered a `Kind.TURN_ON_KEY_STORAGE`
+     * condition.)
+     *
+     * If the user has forgotten their recovery key, we need to reset backup if:
+     * - the user hasn't disabled backup, and
+     * - we don't have the backup key locally.
+     */
+    public async keyStorageOutOfSyncNeedsBackupReset(forgotRecovery: boolean): Promise<boolean> {
+        const crypto = this.client?.getCrypto();
+        if (!crypto) {
+            return false;
+        }
+        const shouldHaveBackup = !(await this.recheckBackupDisabled(this.client!));
+        const backupKeyCached = (await crypto.getSessionBackupPrivateKey()) !== null;
+        const backupKeyStored = await this.client!.isKeyBackupKeyStored();
+
+        if (forgotRecovery) {
+            return shouldHaveBackup && !backupKeyCached;
+        } else {
+            return shouldHaveBackup && !backupKeyCached && !backupKeyStored;
+        }
+    }
+
     private async ensureDeviceIdsAtStartPopulated(): Promise<void> {
         if (this.ourDeviceIdsAtStart === null) {
             this.ourDeviceIdsAtStart = await this.getDeviceIds();
@@ -357,7 +438,10 @@
         // said we are OK with that.
         const keyBackupIsOk = keyBackupUploadActive || backupDisabled;
 
-        const allSystemsReady = isCurrentDeviceTrusted && allCrossSigningSecretsCached && keyBackupIsOk && recoveryIsOk;
+        const backupKeyCached = (await crypto.getSessionBackupPrivateKey()) !== null;
+
+        const allSystemsReady =
+            isCurrentDeviceTrusted && allCrossSigningSecretsCached && keyBackupIsOk && recoveryIsOk && backupKeyCached;
 
         await this.reportCryptoSessionStateToAnalytics(cli);
 
@@ -401,15 +485,22 @@
                 }
             } else {
                 // If we get here, then we are verified, have key backup, and
-                // 4S, but crypto.isSecretStorageReady returned false, which
-                // means that 4S doesn't have all the secrets.
-                logSpan.warn("4S is missing secrets", {
+                // 4S, but allSystemsReady is false, which means that either
+                // secretStorageStatus.ready is false (which means that 4S
+                // doesn't have all the secrets), or we don't have the backup
+                // key cached locally.
+                logSpan.warn("4S is missing secrets or backup key not cached", {
                     crossSigningReady,
                     secretStorageStatus,
                     allCrossSigningSecretsCached,
                     isCurrentDeviceTrusted,
+                    backupKeyCached,
                 });
-                showSetupEncryptionToast(SetupKind.KEY_STORAGE_OUT_OF_SYNC_STORE);
+                // We use the right toast variant based on whether the backup
+                // key is missing locally.  If any of the cross-signing keys are
+                // missing locally, that is handled by the
+                // `!allCrossSigningSecretsCached` branch above.
+                showSetupEncryptionToast(SetupKind.KEY_STORAGE_OUT_OF_SYNC);
             }
         } else {
             logSpan.info("Not yet ready, but shouldShowSetupEncryptionToast==false");

src/components/viewmodels/settings/encryption/KeyStoragePanelViewModel.ts

@@ -12,6 +12,7 @@ import { logger } from "matrix-js-sdk/src/logger";
 import { useMatrixClientContext } from "../../../../contexts/MatrixClientContext";
 import DeviceListener, { BACKUP_DISABLED_ACCOUNT_DATA_KEY } from "../../../../DeviceListener";
 import { useEventEmitterAsyncState } from "../../../../hooks/useEventEmitter";
+import { resetKeyBackupAndWait } from "../../../../utils/crypto/resetKeyBackup";
 
 interface KeyStoragePanelState {
     /**
@@ -75,63 +76,58 @@ export function useKeyStoragePanelViewModel(): KeyStoragePanelState {
         async (enable: boolean) => {
             setPendingValue(enable);
             try {
-                // stop the device listener since enabling or (especially) disabling key storage must be
+                // pause the device listener since enabling or (especially) disabling key storage must be
                 // done with a sequence of API calls that will put the account in a slightly different
-                // state each time, so suppress any warning toasts until the process is finished (when
-                // we'll turn it back on again.)
-                DeviceListener.sharedInstance().stop();
-
-                const crypto = matrixClient.getCrypto();
-                if (!crypto) {
-                    logger.error("Can't change key backup status: no crypto module available");
-                    return;
-                }
-                if (enable) {
-                    const childLogger = logger.getChild("[enable key storage]");
-                    childLogger.info("User requested enabling key storage");
-                    let currentKeyBackup = await crypto.checkKeyBackupAndEnable();
-                    if (currentKeyBackup) {
-                        logger.info(
-                            `Existing key backup is present. version: ${currentKeyBackup.backupInfo.version}`,
-                            currentKeyBackup.trustInfo,
-                        );
-                        // Check if the current key backup can be used. Either of these properties causes the key backup to be used.
-                        if (currentKeyBackup.trustInfo.trusted || currentKeyBackup.trustInfo.matchesDecryptionKey) {
-                            logger.info("Existing key backup can be used");
+                // state each time, so suppress any warning toasts until the process is finished
+                await DeviceListener.sharedInstance().whilePaused(async () => {
+                    const crypto = matrixClient.getCrypto();
+                    if (!crypto) {
+                        logger.error("Can't change key backup status: no crypto module available");
+                        return;
+                    }
+                    if (enable) {
+                        const childLogger = logger.getChild("[enable key storage]");
+                        childLogger.info("User requested enabling key storage");
+                        let currentKeyBackup = await crypto.checkKeyBackupAndEnable();
+                        if (currentKeyBackup) {
+                            logger.info(
+                                `Existing key backup is present. version: ${currentKeyBackup.backupInfo.version}`,
+                                currentKeyBackup.trustInfo,
+                            );
+                            // Check if the current key backup can be used. Either of these properties causes the key backup to be used.
+                            if (currentKeyBackup.trustInfo.trusted || currentKeyBackup.trustInfo.matchesDecryptionKey) {
+                                logger.info("Existing key backup can be used");
+                            } else {
+                                logger.warn("Existing key backup cannot be used, creating new backup");
+                                // There aren't any *usable* backups, so we need to create a new one.
+                                currentKeyBackup = null;
+                            }
                         } else {
-                            logger.warn("Existing key backup cannot be used, creating new backup");
-                            // There aren't any *usable* backups, so we need to create a new one.
-                            currentKeyBackup = null;
+                            logger.info("No existing key backup versions are present, creating new backup");
                         }
-                    } else {
-                        logger.info("No existing key backup versions are present, creating new backup");
-                    }
 
-                    // If there is no usable key backup on the server, create one.
-                    // `resetKeyBackup` will delete any existing backup, so we only do this if there is no usable backup.
-                    if (currentKeyBackup === null) {
-                        await crypto.resetKeyBackup();
-                        // resetKeyBackup fires this off in the background without waiting, so we need to do it
-                        // explicitly and wait for it, otherwise it won't be enabled yet when we check again.
-                        await crypto.checkKeyBackupAndEnable();
-                    }
+                        // If there is no usable key backup on the server, create one.
+                        // `resetKeyBackup` will delete any existing backup, so we only do this if there is no usable backup.
+                        if (currentKeyBackup === null) {
+                            await resetKeyBackupAndWait(crypto);
+                        }
 
-                    // Set the flag so that EX no longer thinks the user wants backup disabled
-                    await matrixClient.setAccountData(BACKUP_DISABLED_ACCOUNT_DATA_KEY, { disabled: false });
-                } else {
-                    logger.info("User requested disabling key backup");
-                    // This method will delete the key backup as well as server side recovery keys and other
-                    // server-side crypto data.
-                    await crypto.disableKeyStorage();
-
-                    // Set a flag to say that the user doesn't want key backup.
-                    // Element X uses this to determine whether to set up automatically,
-                    // so this will stop EX turning it back on spontaneously.
-                    await matrixClient.setAccountData(BACKUP_DISABLED_ACCOUNT_DATA_KEY, { disabled: true });
-                }
+                        // Set the flag so that EX no longer thinks the user wants backup disabled
+                        await matrixClient.setAccountData(BACKUP_DISABLED_ACCOUNT_DATA_KEY, { disabled: false });
+                    } else {
+                        logger.info("User requested disabling key backup");
+                        // This method will delete the key backup as well as server side recovery keys and other
+                        // server-side crypto data.
+                        await crypto.disableKeyStorage();
+
+                        // Set a flag to say that the user doesn't want key backup.
+                        // Element X uses this to determine whether to set up automatically,
+                        // so this will stop EX turning it back on spontaneously.
+                        await matrixClient.setAccountData(BACKUP_DISABLED_ACCOUNT_DATA_KEY, { disabled: true });
+                    }
+                });
             } finally {
                 setPendingValue(undefined);
-                DeviceListener.sharedInstance().start(matrixClient);
             }
         },
         [setPendingValue, matrixClient],

src/components/views/settings/encryption/ChangeRecoveryKey.tsx

@@ -1,4 +1,5 @@
 /*
+ * Copyright 2025 Element Creations Ltd.
  * Copyright 2024 New Vector Ltd.
  *
  * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
@@ -29,7 +30,8 @@
 import { withSecretStorageKeyCache } from "../../../../SecurityManager";
 import { EncryptionCardButtons } from "./EncryptionCardButtons";
 import { logErrorAndShowErrorDialog } from "../../../../utils/ErrorUtils.tsx";
-import { RECOVERY_ACCOUNT_DATA_KEY } from "../../../../DeviceListener";
+import DeviceListener, { RECOVERY_ACCOUNT_DATA_KEY } from "../../../../DeviceListener";
+import { resetKeyBackupAndWait } from "../../../../utils/crypto/resetKeyBackup";
 
 /**
  * The possible states of the component.
@@ -123,14 +125,27 @@
                         if (!crypto) return onFinish();
 
                         try {
-                            // We need to enable the cache to avoid to prompt the user to enter the new key
-                            // when we will try to access the secret storage during the bootstrap
-                            await withSecretStorageKeyCache(async () => {
-                                await crypto.bootstrapSecretStorage({
-                                    setupNewSecretStorage: true,
-                                    createSecretStorageKey: async () => recoveryKey,
+                            const deviceListener = DeviceListener.sharedInstance();
+
+                            // we need to call keyStorageOutOfSyncNeedsBackupReset here because
+                            // deviceListener.whilePaused() sets its client to undefined, so
+                            // keyStorageOutOfSyncNeedsBackupReset won't be able to check
+                            // the backup state.
+                            const needsBackupReset = await deviceListener.keyStorageOutOfSyncNeedsBackupReset(true);
+                            await deviceListener.whilePaused(async () => {
+                                // We need to enable the cache to avoid to prompt the user to enter the new key
+                                // when we will try to access the secret storage during the bootstrap
+                                await withSecretStorageKeyCache(async () => {
+                                    await crypto.bootstrapSecretStorage({
+                                        setupNewSecretStorage: true,
+                                        createSecretStorageKey: async () => recoveryKey,
+                                    });
+                                    // Reset the key backup if needed
+                                    if (needsBackupReset) {
+                                        await resetKeyBackupAndWait(crypto);
+                                    }
+                                    await initialiseDehydrationIfEnabled(matrixClient, { createNewKey: true });
                                 });
-                                await initialiseDehydrationIfEnabled(matrixClient, { createNewKey: true });
                             });
 
                             // Record the fact that the user explicitly enabled recovery.