-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Handle cross-signing keys missing locally and/or from secret storage #31367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 15 commits
e1ca135
d0ed546
85f8b9c
48743ee
303417e
c44916c
fb2dbe7
d7a2ac9
8983cef
f1f1f85
bb3a130
ff53fc1
bccb180
6f31fed
da86841
46915f7
3f52a6f
b09f67c
edc4d81
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ import { | |
| RoomStateEvent, | ||
| type SyncState, | ||
| ClientStoppedError, | ||
| TypedEventEmitter, | ||
| } from "matrix-js-sdk/src/matrix"; | ||
| import { logger as baseLogger, type BaseLogger, LogSpan } from "matrix-js-sdk/src/logger"; | ||
| import { CryptoEvent, type KeyBackupInfo } from "matrix-js-sdk/src/crypto-api"; | ||
|
|
@@ -29,7 +30,6 @@ import { | |
| } from "./toasts/BulkUnverifiedSessionsToast"; | ||
| import { | ||
| hideToast as hideSetupEncryptionToast, | ||
| Kind as SetupKind, | ||
| showToast as showSetupEncryptionToast, | ||
| } from "./toasts/SetupEncryptionToast"; | ||
| import { | ||
|
|
@@ -65,7 +65,48 @@ export const RECOVERY_ACCOUNT_DATA_KEY = "io.element.recovery"; | |
|
|
||
| const logger = baseLogger.getChild("DeviceListener:"); | ||
|
|
||
| export default class DeviceListener { | ||
| /** | ||
| * The state of the device and the user's account. | ||
| */ | ||
| export enum DeviceState { | ||
| /** | ||
| * The device is in a good state. | ||
| */ | ||
| OK = "ok", | ||
| /** | ||
| * The user needs to set up recovery. | ||
| */ | ||
| SET_UP_RECOVERY = "set_up_recovery", | ||
| /** | ||
| * The device is not verified. | ||
| */ | ||
| VERIFY_THIS_SESSION = "verify_this_session", | ||
| /** | ||
| * Key storage is out of sync (keys are missing locally, from recovery, or both). | ||
| */ | ||
| KEY_STORAGE_OUT_OF_SYNC = "key_storage_out_of_sync", | ||
| /** | ||
| * Key storage is not enabled, and has not been marked as purposely disabled. | ||
| */ | ||
| TURN_ON_KEY_STORAGE = "turn_on_key_storage", | ||
| /** | ||
| * The user's identity needs resetting, due to missing keys. | ||
| */ | ||
| IDENTITY_NEEDS_RESET = "identity_needs_reset", | ||
| } | ||
|
Comment on lines
+68
to
+96
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use string union? It allows more flexibility |
||
|
|
||
| /** | ||
| * The events emitted by {@link DeviceListener} | ||
| */ | ||
| export enum DeviceListenerEvents { | ||
| DeviceState = "device_state", | ||
| } | ||
|
|
||
| type EventHandlerMap = { | ||
| [DeviceListenerEvents.DeviceState]: (state: DeviceState) => void; | ||
| }; | ||
|
|
||
| export default class DeviceListener extends TypedEventEmitter<DeviceListenerEvents, EventHandlerMap> { | ||
| private dispatcherRef?: string; | ||
| // device IDs for which the user has dismissed the verify toast ('Later') | ||
| private dismissed = new Set<string>(); | ||
|
|
@@ -87,6 +128,7 @@ export default class DeviceListener { | |
| private shouldRecordClientInformation = false; | ||
| private enableBulkUnverifiedSessionsReminder = true; | ||
| private deviceClientInformationSettingWatcherRef: string | undefined; | ||
| private deviceState: DeviceState = DeviceState.OK; | ||
|
|
||
| // Remember the current analytics state to avoid sending the same event multiple times. | ||
| private analyticsVerificationState?: string; | ||
|
|
@@ -198,8 +240,8 @@ export default class DeviceListener { | |
| } | ||
|
|
||
| /** | ||
| * If a `Kind.KEY_STORAGE_OUT_OF_SYNC` condition from {@link doRecheck} | ||
| * requires a reset of cross-signing keys. | ||
| * If the device is in a `DeviceState.KEY_STORAGE_OUT_OF_SYNC` state, check if | ||
| * it 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. | ||
|
|
@@ -227,16 +269,15 @@ export default class DeviceListener { | |
| } | ||
|
|
||
| /** | ||
| * If a `Kind.KEY_STORAGE_OUT_OF_SYNC` condition from {@link doRecheck} | ||
| * requires a reset of key backup. | ||
| * If the device is in a `DeviceState.KEY_STORAGE_OUT_OF_SYNC` state, check if | ||
| * it 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.) | ||
| * (The user should already have a key backup created at this point, the | ||
| * device state would be `DeviceState.TURN_ON_KEY_STORAGE`.) | ||
| * | ||
| * If the user has forgotten their recovery key, we need to reset backup if: | ||
| * - the user hasn't disabled backup, and | ||
|
|
@@ -425,11 +466,9 @@ export default class DeviceListener { | |
|
|
||
| const recoveryIsOk = secretStorageStatus.ready || recoveryDisabled; | ||
|
|
||
| const isCurrentDeviceTrusted = | ||
| crossSigningReady && | ||
| Boolean( | ||
| (await crypto.getDeviceVerificationStatus(cli.getSafeUserId(), cli.deviceId!))?.crossSigningVerified, | ||
| ); | ||
| const isCurrentDeviceTrusted = Boolean( | ||
| (await crypto.getDeviceVerificationStatus(cli.getSafeUserId(), cli.deviceId!))?.crossSigningVerified, | ||
| ); | ||
|
|
||
| const keyBackupUploadActive = await this.isKeyBackupUploadActive(logSpan); | ||
| const backupDisabled = await this.recheckBackupDisabled(cli); | ||
|
|
@@ -445,65 +484,70 @@ export default class DeviceListener { | |
|
|
||
| await this.reportCryptoSessionStateToAnalytics(cli); | ||
|
|
||
| if (this.dismissedThisDeviceToast || allSystemsReady) { | ||
| if (allSystemsReady) { | ||
| logSpan.info("No toast needed"); | ||
| hideSetupEncryptionToast(); | ||
| await this.setDeviceState(DeviceState.OK, logSpan); | ||
|
|
||
| this.checkKeyBackupStatus(); | ||
| } else if (await this.shouldShowSetupEncryptionToast()) { | ||
| } else { | ||
| // make sure our keys are finished downloading | ||
| await crypto.getUserDeviceInfo([cli.getSafeUserId()]); | ||
|
|
||
| if (!isCurrentDeviceTrusted) { | ||
| // the current device is not trusted: prompt the user to verify | ||
| logSpan.info("Current device not verified: showing VERIFY_THIS_SESSION toast"); | ||
| showSetupEncryptionToast(SetupKind.VERIFY_THIS_SESSION); | ||
| logSpan.info("Current device not verified: setting state to VERIFY_THIS_SESSION"); | ||
| await this.setDeviceState(DeviceState.VERIFY_THIS_SESSION, logSpan); | ||
| } else if (!allCrossSigningSecretsCached) { | ||
| // cross signing ready & device trusted, but we are missing secrets from our local cache. | ||
| // prompt the user to enter their recovery key. | ||
| logSpan.info( | ||
| "Some secrets not cached: showing KEY_STORAGE_OUT_OF_SYNC toast", | ||
| "Some secrets not cached: setting state to KEY_STORAGE_OUT_OF_SYNC", | ||
| crossSigningStatus.privateKeysCachedLocally, | ||
| crossSigningStatus.privateKeysInSecretStorage, | ||
| ); | ||
| await this.setDeviceState( | ||
| crossSigningStatus.privateKeysInSecretStorage | ||
| ? DeviceState.KEY_STORAGE_OUT_OF_SYNC | ||
| : DeviceState.IDENTITY_NEEDS_RESET, | ||
| logSpan, | ||
| ); | ||
| showSetupEncryptionToast(SetupKind.KEY_STORAGE_OUT_OF_SYNC); | ||
| } else if (!keyBackupIsOk) { | ||
| logSpan.info("Key backup upload is unexpectedly turned off: showing TURN_ON_KEY_STORAGE toast"); | ||
| showSetupEncryptionToast(SetupKind.TURN_ON_KEY_STORAGE); | ||
| logSpan.info("Key backup upload is unexpectedly turned off: setting state to TURN_ON_KEY_STORAGE"); | ||
| await this.setDeviceState(DeviceState.TURN_ON_KEY_STORAGE, logSpan); | ||
| } else if (secretStorageStatus.defaultKeyId === null) { | ||
| // The user just hasn't set up 4S yet: if they have key | ||
| // backup, prompt them to turn on recovery too. (If not, they | ||
| // have explicitly opted out, so don't hassle them.) | ||
| if (recoveryDisabled) { | ||
| logSpan.info("Recovery disabled: no toast needed"); | ||
| hideSetupEncryptionToast(); | ||
| await this.setDeviceState(DeviceState.OK, logSpan); | ||
| } else if (keyBackupUploadActive) { | ||
| logSpan.info("No default 4S key: showing SET_UP_RECOVERY toast"); | ||
| showSetupEncryptionToast(SetupKind.SET_UP_RECOVERY); | ||
| logSpan.info("No default 4S key: setting state to SET_UP_RECOVERY"); | ||
| await this.setDeviceState(DeviceState.SET_UP_RECOVERY, logSpan); | ||
| } else { | ||
| logSpan.info("No default 4S key but backup disabled: no toast needed"); | ||
| hideSetupEncryptionToast(); | ||
| await this.setDeviceState(DeviceState.OK, logSpan); | ||
| } | ||
| } else { | ||
| // If we get here, then we are verified, have key backup, and | ||
| // 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. | ||
| // key cached locally. If any of the cross-signing keys are | ||
| // missing locally, that is handled by the | ||
| // `!allCrossSigningSecretsCached` branch above. | ||
| logSpan.warn("4S is missing secrets or backup key not cached", { | ||
| crossSigningReady, | ||
| secretStorageStatus, | ||
| allCrossSigningSecretsCached, | ||
| isCurrentDeviceTrusted, | ||
| backupKeyCached, | ||
| }); | ||
| // 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); | ||
| await this.setDeviceState(DeviceState.KEY_STORAGE_OUT_OF_SYNC, logSpan); | ||
| } | ||
| if (this.dismissedThisDeviceToast) { | ||
| this.checkKeyBackupStatus(); | ||
| } | ||
| } else { | ||
| logSpan.info("Not yet ready, but shouldShowSetupEncryptionToast==false"); | ||
| } | ||
|
|
||
| // This needs to be done after awaiting on getUserDeviceInfo() above, so | ||
|
|
@@ -595,6 +639,22 @@ export default class DeviceListener { | |
| return recoveryStatus?.enabled === false; | ||
| } | ||
|
|
||
| public getDeviceState(): DeviceState { | ||
| return this.deviceState; | ||
| } | ||
|
|
||
| private async setDeviceState(newState: DeviceState, logSpan: LogSpan): Promise<void> { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing tsdoc |
||
| this.deviceState = newState; | ||
| this.emit(DeviceListenerEvents.DeviceState, newState); | ||
| if (newState === DeviceState.OK || this.dismissedThisDeviceToast) { | ||
| hideSetupEncryptionToast(); | ||
| } else if (await this.shouldShowSetupEncryptionToast()) { | ||
| showSetupEncryptionToast(newState); | ||
| } else { | ||
| logSpan.info("Not yet ready, but shouldShowSetupEncryptionToast==false"); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Reports current recovery state to analytics. | ||
| * Checks if the session is verified and if the recovery is correctly set up (i.e all secrets known locally and in 4S). | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,9 @@ import { SettingsSection } from "../shared/SettingsSection"; | |
| import { _t } from "../../../../languageHandler"; | ||
| import { SettingsSubheader } from "../SettingsSubheader"; | ||
| import { accessSecretStorage } from "../../../../SecurityManager"; | ||
| import DeviceListener from "../../../../DeviceListener"; | ||
| import { useMatrixClientContext } from "../../../../contexts/MatrixClientContext"; | ||
| import { resetKeyBackupAndWait } from "../../../../utils/crypto/resetKeyBackup"; | ||
|
|
||
| interface RecoveryPanelOutOfSyncProps { | ||
| /** | ||
|
|
@@ -33,6 +36,8 @@ interface RecoveryPanelOutOfSyncProps { | |
| * the client. | ||
| */ | ||
| export function RecoveryPanelOutOfSync({ onForgotRecoveryKey, onFinish }: RecoveryPanelOutOfSyncProps): JSX.Element { | ||
| const matrixClient = useMatrixClientContext(); | ||
|
|
||
| return ( | ||
| <SettingsSection | ||
| legacy={false} | ||
|
|
@@ -55,7 +60,29 @@ export function RecoveryPanelOutOfSync({ onForgotRecoveryKey, onFinish }: Recove | |
| kind="primary" | ||
| Icon={KeyIcon} | ||
| onClick={async () => { | ||
| await accessSecretStorage(); | ||
| const crypto = matrixClient.getCrypto()!; | ||
|
|
||
| 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(); | ||
| } | ||
| }); | ||
| }); | ||
|
Comment on lines
+76
to
+85
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is happening if this promises are rejected? |
||
| onFinish(); | ||
| }} | ||
| > | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.