Key storage out of sync: reset key backup when needed (#31279)

* add function to pause device listener

* add function to check if key backup key missing both locally and in 4s

* reset backup if backup key missing both locally and in 4s

* fixup! add function to check if key backup key missing both locally and in 4s

* Drop KEY_STORAGE_OUT_OF_SYNC_STORE in favour of checking cross-signing

Check if cross-signing needs resetting, because that seems to be what
KEY_STORAGE_OUT_OF_SYNC_STORE is actually trying to do.

* add a function for resetting key backup and waiting until it's ready

* trigger key storage out of sync toast when missing backup key locally

and fetch it when user enters their recovery key

* reset backup when needed if user forgets recovery key

* rename function as suggested in code review
This commit is contained in:
Hubert Chathi
2025-11-20 15:25:31 -05:00
committed by GitHub
parent 1285b73be6
commit aee24be1b4
9 changed files with 512 additions and 120 deletions

View File

@@ -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 @@ export default class DeviceListener {
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 @@ export default class DeviceListener {
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 @@ export default class DeviceListener {
// 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 @@ export default class DeviceListener {
}
} 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");

View File

@@ -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");
}
// 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("No existing key backup versions are present, creating new backup");
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 });
}
// 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();
}
// 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],

View File

@@ -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 { initialiseDehydrationIfEnabled } from "../../../../utils/device/dehydra
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 @@ export function ChangeRecoveryKey({
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.

View File

@@ -1,4 +1,5 @@
/*
Copyright 2025 Element Creations Ltd.
Copyright 2024 New Vector Ltd.
Copyright 2020 The Matrix.org Foundation C.I.C.
@@ -26,6 +27,8 @@ import { Action } from "../dispatcher/actions";
import { UserTab } from "../components/views/dialogs/UserTab";
import defaultDispatcher from "../dispatcher/dispatcher";
import ConfirmKeyStorageOffDialog from "../components/views/dialogs/ConfirmKeyStorageOffDialog";
import { MatrixClientPeg } from "../MatrixClientPeg";
import { resetKeyBackupAndWait } from "../utils/crypto/resetKeyBackup";
import { PosthogAnalytics } from "../PosthogAnalytics";
const TOAST_KEY = "setupencryption";
@@ -37,7 +40,6 @@ const getTitle = (kind: Kind): string => {
case Kind.VERIFY_THIS_SESSION:
return _t("encryption|verify_toast_title");
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE:
return _t("encryption|key_storage_out_of_sync");
case Kind.TURN_ON_KEY_STORAGE:
return _t("encryption|turn_on_key_storage");
@@ -50,7 +52,6 @@ const getIcon = (kind: Kind): string | undefined => {
return undefined;
case Kind.VERIFY_THIS_SESSION:
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE:
return "verification_warning";
case Kind.TURN_ON_KEY_STORAGE:
return "key_storage";
@@ -64,7 +65,6 @@ const getSetupCaption = (kind: Kind): string => {
case Kind.VERIFY_THIS_SESSION:
return _t("action|verify");
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE:
return _t("encryption|enter_recovery_key");
case Kind.TURN_ON_KEY_STORAGE:
return _t("action|continue");
@@ -78,7 +78,6 @@ const getSetupCaption = (kind: Kind): string => {
const getPrimaryButtonIcon = (kind: Kind): ComponentType<React.SVGAttributes<SVGElement>> | undefined => {
switch (kind) {
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE:
return KeyIcon;
default:
return;
@@ -92,7 +91,6 @@ const getSecondaryButtonLabel = (kind: Kind): string => {
case Kind.VERIFY_THIS_SESSION:
return _t("encryption|verification|unverified_sessions_toast_reject");
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE:
return _t("encryption|forgot_recovery_key");
case Kind.TURN_ON_KEY_STORAGE:
return _t("action|dismiss");
@@ -106,7 +104,6 @@ const getDescription = (kind: Kind): string => {
case Kind.VERIFY_THIS_SESSION:
return _t("encryption|verify_toast_description");
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE:
return _t("encryption|key_storage_out_of_sync_description");
case Kind.TURN_ON_KEY_STORAGE:
return _t("encryption|turn_on_key_storage_description");
@@ -126,13 +123,9 @@ export enum Kind {
*/
VERIFY_THIS_SESSION = "verify_this_session",
/**
* Prompt the user to enter their recovery key, to retrieve secrets
* Prompt the user to enter their recovery key
*/
KEY_STORAGE_OUT_OF_SYNC = "key_storage_out_of_sync",
/**
* Prompt the user to enter their recovery key, to store secrets
*/
KEY_STORAGE_OUT_OF_SYNC_STORE = "key_storage_out_of_sync_store",
/**
* Prompt the user to turn on key storage
*/
@@ -174,8 +167,7 @@ export const showToast = (kind: Kind): void => {
case Kind.VERIFY_THIS_SESSION:
Modal.createDialog(SetupEncryptionDialog, {}, undefined, /* priority = */ false, /* static = */ true);
break;
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE: {
case Kind.KEY_STORAGE_OUT_OF_SYNC: {
const modal = Modal.createDialog(
Spinner,
undefined,
@@ -183,10 +175,34 @@ export const showToast = (kind: Kind): void => {
/* priority */ false,
/* static */ true,
);
const matrixClient = MatrixClientPeg.safeGet();
const crypto = matrixClient.getCrypto()!;
try {
await accessSecretStorage();
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(false);
// pause the device listener because we could be making lots
// of changes, and don't want toasts to pop up and disappear
// while we're doing it
await deviceListener.whilePaused(async () => {
await accessSecretStorage(async () => {
// Reset backup if needed.
if (needsBackupReset) {
await resetKeyBackupAndWait(crypto);
} else if (await matrixClient.isKeyBackupKeyStored()) {
await crypto.loadSessionBackupPrivateKeyFromSecretStorage();
}
});
});
} catch (error) {
onAccessSecretStorageFailed(kind, error as Error);
await onAccessSecretStorageFailed(error as Error);
} finally {
modal.close();
}
@@ -209,13 +225,18 @@ export const showToast = (kind: Kind): void => {
deviceListener.dismissEncryptionSetup();
break;
}
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE: {
// Open the user settings dialog to the encryption tab and start the flow to reset encryption
case Kind.KEY_STORAGE_OUT_OF_SYNC: {
// Open the user settings dialog to the encryption tab and start the flow to reset encryption or change the recovery key
const deviceListener = DeviceListener.sharedInstance();
const needsCrossSigningReset = await deviceListener.keyStorageOutOfSyncNeedsCrossSigningReset(true);
const payload: OpenToTabPayload = {
action: Action.ViewUserSettings,
initialTabId: UserTab.Encryption,
props: { initialEncryptionState: "reset_identity_forgot" },
props: {
initialEncryptionState: needsCrossSigningReset
? "reset_identity_forgot"
: "change_recovery_key",
},
};
defaultDispatcher.dispatch(payload);
break;
@@ -250,25 +271,23 @@ export const showToast = (kind: Kind): void => {
* recovery key, but this failed. If the user just gave up, that is fine,
* but if not, that means downloading encryption info from 4S did not fix
* the problem we identified. Presumably, something is wrong with what they
* have in 4S. If we were trying to fetch secrets from 4S, we tell them to
* reset their identity, to reset everything. If we were trying to store
* secrets in 4S, or set up recovery, we tell them to change their recovery
* key, to create a new 4S that we can store the secrets in.
* have in 4S.
*/
const onAccessSecretStorageFailed = (
kind: Kind.KEY_STORAGE_OUT_OF_SYNC | Kind.KEY_STORAGE_OUT_OF_SYNC_STORE,
error: Error,
): void => {
const onAccessSecretStorageFailed = async (error: Error): Promise<void> => {
if (error instanceof AccessCancelledError) {
// The user cancelled the dialog - just allow it to close
} else {
// A real error happened - jump to the reset identity tab
// A real error happened - jump to the reset identity or change
// recovery tab
const needsCrossSigningReset =
await DeviceListener.sharedInstance().keyStorageOutOfSyncNeedsCrossSigningReset(true);
const payload: OpenToTabPayload = {
action: Action.ViewUserSettings,
initialTabId: UserTab.Encryption,
props: {
initialEncryptionState:
kind === Kind.KEY_STORAGE_OUT_OF_SYNC ? "reset_identity_sync_failed" : "change_recovery_key",
initialEncryptionState: needsCrossSigningReset
? "reset_identity_sync_failed"
: "change_recovery_key",
},
};
defaultDispatcher.dispatch(payload);

View File

@@ -0,0 +1,20 @@
/*
Copyright 2025 Element Creations Ltd.
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
Please see LICENSE files in the repository root for full details.
*/
import { type CryptoApi } from "matrix-js-sdk/src/crypto-api";
/**
* Creates a new key backup version, and wait until it is enabled.
*
* This is typically used within a {@link DeviceListener.pause()} call, to
* ensure that the device listener doesn't check the backup status until after the
* key backup is active.
*/
export async function resetKeyBackupAndWait(crypto: CryptoApi): Promise<void> {
await crypto.resetKeyBackup();
await crypto.checkKeyBackupAndEnable();
}