From 9660c4b2be900b3e481f25cdc1866ecc880f2170 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 23 May 2025 14:33:17 +0100 Subject: [PATCH] Refactor store and fix bugs with it (#2348) --- src/i18n/strings/en_EN.json | 8 +- src/store.ts | 272 +++++++++++++++++++----------------- 2 files changed, 147 insertions(+), 133 deletions(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 60ec188..27690be 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -59,13 +59,11 @@ "backend_no_encryption": "Your system has a supported keyring but encryption is not available.", "backend_no_encryption_detail": "Electron has detected that encryption is not available on your keyring %(backend)s. Please ensure that you have the keyring installed. If you do have the keyring installed, please reboot and try again. Optionally, you can allow %(brand)s to use a weaker form of encryption.", "backend_no_encryption_title": "No encryption support", - "unknown_backend_override": "Your system has an unsupported keyring meaning the database cannot be opened.", - "unknown_backend_override_details": "Please check the logs for more details.", - "unknown_backend_override_title": "Failed to load database", "unsupported_keyring": "Your system has an unsupported keyring meaning the database cannot be opened.", - "unsupported_keyring_cta": "Use weaker encryption", "unsupported_keyring_detail": "Electron's keyring detection did not find a supported backend. You can attempt to manually configure the backend by starting %(brand)s with a command-line argument, a one-time operation. See %(link)s.", - "unsupported_keyring_title": "System unsupported" + "unsupported_keyring_title": "System unsupported", + "unsupported_keyring_use_basic_text": "Use weaker encryption", + "unsupported_keyring_use_plaintext": "Use no encryption" } }, "view_menu": { diff --git a/src/store.ts b/src/store.ts index 69945e2..357e479 100644 --- a/src/store.ts +++ b/src/store.ts @@ -41,6 +41,10 @@ const LEGACY_KEYTAR_SERVICE = "riot.im"; * + All other backends are linux-specific and are encrypted using the keychain */ type SafeStorageBackend = ReturnType | "system" | "plaintext"; +/** + * The "unknown" backend is not a valid backend, so we exclude it from the type. + */ +type SaneSafeStorageBackend = Exclude; /** * Map of safeStorage backends to their command line arguments. @@ -85,7 +89,7 @@ interface StoreData { /** * Fallback storage writer for secrets, mainly used for automated tests and systems without any safeStorage support. */ -class PlaintextStorageWriter { +class StorageWriter { public constructor(protected readonly store: ElectronStore) {} public getKey(key: string): `safeStorage.${string}` { @@ -108,7 +112,7 @@ class PlaintextStorageWriter { /** * Storage writer for secrets using safeStorage. */ -class SafeStorageWriter extends PlaintextStorageWriter { +class SafeStorageWriter extends StorageWriter { public set(key: string, secret: string): void { this.store.set(this.getKey(key), safeStorage.encryptString(secret).toString("base64")); } @@ -157,7 +161,10 @@ class Store extends ElectronStore { const store = new Store(mode ?? Mode.Encrypted); Store.internalInstance = store; - if (process.platform === "linux" && store.get("safeStorageBackendOverride")) { + if ( + process.platform === "linux" && + (store.get("safeStorageBackendOverride") || store.get("safeStorageBackendMigrate")) + ) { const backend = store.get("safeStorageBackend")!; if (backend in safeStorageBackendMap) { // If the safeStorage backend which was used to write the data is one we can specify via the commandLine @@ -174,7 +181,7 @@ class Store extends ElectronStore { // Provides "raw" access to the underlying secrets storage, // should be avoided in favour of the getSecret/setSecret/deleteSecret methods. - private secrets?: PlaintextStorageWriter | SafeStorageWriter; + private secrets?: StorageWriter; private constructor(private mode: Mode) { super({ @@ -228,6 +235,37 @@ class Store extends ElectronStore { await this.safeStorageReadyPromise; } + /** + * Normalise the backend to a sane value (exclude `unknown`), respect forcePlaintext mode, + * and ensure that if an encrypted backend is picked that encryption is available, falling back to plaintext if not. + * @param forcePlaintext - whether to force plaintext mode + * @private + */ + private chooseBackend(forcePlaintext: boolean): SaneSafeStorageBackend { + if (forcePlaintext) { + return "plaintext"; + } + + if (process.platform === "linux") { + // The following enables plain text encryption if the backend used is basic_text. + // It has no significance for any other backend. + // We do this early so that in case we end up using the basic_text backend (either because that's the only one available + // or as a fallback when the configured backend lacks encryption support), encryption is already turned on. + safeStorage.setUsePlainTextEncryption(true); + + // Linux safeStorage support is hellish, the support varies on the Desktop Environment used rather than the store itself. + // https://github.com/electron/electron/issues/39789 https://github.com/microsoft/vscode/issues/185212 + const selectedBackend = safeStorage.getSelectedStorageBackend(); + + if (selectedBackend === "unknown" || !safeStorage.isEncryptionAvailable()) { + return "plaintext"; + } + return selectedBackend; + } + + return safeStorage.isEncryptionAvailable() ? "system" : "plaintext"; + } + /** * Prepare the safeStorage backend for use. * We don't eagerly import from keytar as that would bring in data for all Element profiles and not just the current one, @@ -236,134 +274,110 @@ class Store extends ElectronStore { private async prepareSafeStorage(): Promise { await app.whenReady(); - let safeStorageBackend = this.get("safeStorageBackend"); - if (process.platform === "linux") { - // Linux safeStorage support is hellish, the support varies on the Desktop Environment used rather than the store itself. - // https://github.com/electron/electron/issues/39789 https://github.com/microsoft/vscode/issues/185212 - const selectedSafeStorageBackend = safeStorage.getSelectedStorageBackend(); + // The backend the existing data is written with if any + let existingSafeStorageBackend = this.get("safeStorageBackend"); + // The backend and encryption status of the currently loaded backend + const backend = this.chooseBackend(this.mode === Mode.ForcePlaintext); - // The following enables plain text encryption if the backend used is basic_text. - // It has no significance for any other backend. - // We do this early so that in case we end up using the basic_text backend (either because that's the only one available - // or as a fallback when the configured backend lacks encryption support), encryption is already turned on. - safeStorage.setUsePlainTextEncryption(true); - - const isEncryptionAvailable = safeStorage.isEncryptionAvailable(); - console.info( - `safeStorage backend '${selectedSafeStorageBackend}' selected, '${safeStorageBackend}' in config, isEncryptionAvailable = ${isEncryptionAvailable}.`, - ); - - if (selectedSafeStorageBackend === "unknown") { - // This should never happen but good to be safe - await dialog.showMessageBox({ - title: _t("store|error|unknown_backend_override_title"), - message: _t("store|error|unknown_backend_override"), - detail: _t("store|error|unknown_backend_override_details"), - type: "error", - }); - throw new Error("safeStorage backend unknown"); + // Handle migrations + if (existingSafeStorageBackend) { + if (existingSafeStorageBackend === "basic_text" && backend !== "plaintext" && backend !== "basic_text") { + return this.prepareMigrateBasicTextToPlaintext(); } - if (this.get("safeStorageBackendMigrate")) { - return this.upgradeLinuxBackend2(); + if (this.get("safeStorageBackendMigrate") && backend === "basic_text") { + return this.migrateBasicTextToPlaintext(); } - // Whether we were using basic_text as a fallback before - const usingFallback = this.get("safeStorageBackendOverride") && safeStorageBackend === "basic_text"; + if (existingSafeStorageBackend === "plaintext" && backend !== "plaintext") { + this.migratePlaintextToEncrypted(); + // Ensure we update existingSafeStorageBackend so we don't fall into the "backend changed" clause below + existingSafeStorageBackend = this.get("safeStorageBackend"); + } + } - if (this.mode === Mode.Encrypted && !isEncryptionAvailable && !usingFallback) { - // Sometimes we may have a working backend that for some reason does not support encryption at the moment. - // This may be because electron reported an incorrect backend or because of some known issues with the keyring itself. - // In any case, when this happens, we give the user an option to use a weaker form of encryption. - const { response } = await dialog.showMessageBox({ - title: _t("store|error|backend_no_encryption_title"), - message: _t("store|error|backend_no_encryption"), - detail: _t("store|error|backend_no_encryption_detail", { - backend: safeStorageBackend, - brand: global.vectorConfig.brand || "Element", - }), - type: "error", - buttons: [_t("action|cancel"), _t("store|error|unsupported_keyring_cta")], - defaultId: 0, - cancelId: 0, - }); - if (response === 0) { - throw new Error( - `Encryption support not available on backend ${safeStorageBackend} and user prohibits using weaker encryption.`, - ); - } - this.recordSafeStorageBackend("basic_text"); + if (!existingSafeStorageBackend) { + // First launch of the app or first launch since the update + if (this.mode === Mode.Encrypted && (backend === "plaintext" || backend === "basic_text")) { + // Ask the user for consent to use a degraded mode + await this.consultUserConsentDegradedMode(backend); + } + // Store the backend used for the safeStorage data so we can detect if it changes, and we know how the data is encoded + this.recordSafeStorageBackend(backend); + } else if (existingSafeStorageBackend !== backend) { + console.warn(`safeStorage backend changed from ${existingSafeStorageBackend} to ${backend}`); + + if (existingSafeStorageBackend in safeStorageBackendMap) { this.set("safeStorageBackendOverride", true); - relaunchApp(); - } else if (usingFallback) { - // On the next run, don't use the fallback. - // This is so that we can check if the problems with the keyring fixed itself. - this.set("safeStorageBackendOverride", false); - } else if (!safeStorageBackend) { - if (selectedSafeStorageBackend === "basic_text" && this.mode === Mode.Encrypted) { - const { response } = await dialog.showMessageBox({ - title: _t("store|error|unsupported_keyring_title"), - message: _t("store|error|unsupported_keyring"), - detail: _t("store|error|unsupported_keyring_detail", { - brand: global.vectorConfig.brand || "Element", - link: "https://www.electronjs.org/docs/latest/api/safe-storage#safestoragegetselectedstoragebackend-linux", - }), - type: "error", - buttons: [_t("action|cancel"), _t("store|error|unsupported_keyring_cta")], - defaultId: 0, - cancelId: 0, - }); - if (response === 0) { - throw new Error("safeStorage backend basic_text and user rejected it"); - } - this.mode = Mode.AllowPlaintext; - } - - // Store the backend used for the safeStorage data so we can detect if it changes - this.recordSafeStorageBackend(selectedSafeStorageBackend); - safeStorageBackend = selectedSafeStorageBackend; - } else if (safeStorageBackend !== selectedSafeStorageBackend) { - console.warn(`safeStorage backend changed from ${safeStorageBackend} to ${selectedSafeStorageBackend}`); - - if (safeStorageBackend === "basic_text") { - return this.upgradeLinuxBackend1(); - } else if (safeStorageBackend === "plaintext") { - this.upgradeLinuxBackend3(); - } else if (safeStorageBackend in safeStorageBackendMap) { - this.set("safeStorageBackendOverride", true); - relaunchApp(); - return; - } else { - // Warn the user that the backend has changed and tell them that we cannot migrate - const { response } = await dialog.showMessageBox({ - title: _t("store|error|backend_changed_title"), - message: _t("store|error|backend_changed"), - detail: _t("store|error|backend_changed_detail"), - type: "question", - buttons: [_t("common|no"), _t("common|yes")], - defaultId: 0, - cancelId: 0, - }); - if (response === 0) { - throw new Error("safeStorage backend changed and cannot migrate"); - } - await clearDataAndRelaunch(); - } + return relaunchApp(); + } else { + await this.consultUserBackendChangedUnableToMigrate(); } - } else if (!safeStorageBackend) { - safeStorageBackend = this.mode === Mode.Encrypted ? "system" : "plaintext"; - this.recordSafeStorageBackend(safeStorageBackend); } - if (this.mode !== Mode.ForcePlaintext && safeStorage.isEncryptionAvailable()) { + console.info(`Using storage mode '${this.mode}' with backend '${backend}'`); + if (backend !== "plaintext") { this.secrets = new SafeStorageWriter(this); - } else if (this.mode !== Mode.Encrypted) { - this.secrets = new PlaintextStorageWriter(this); } else { - throw new Error(`safeStorage is not available`); + this.secrets = new StorageWriter(this); } + } - console.info(`Using storage mode '${this.mode}' with backend '${safeStorageBackend}'`); + private async consultUserBackendChangedUnableToMigrate(): Promise { + const { response } = await dialog.showMessageBox({ + title: _t("store|error|backend_changed_title"), + message: _t("store|error|backend_changed"), + detail: _t("store|error|backend_changed_detail"), + type: "question", + buttons: [_t("common|no"), _t("common|yes")], + defaultId: 0, + cancelId: 0, + }); + if (response === 0) { + throw new Error("safeStorage backend changed and cannot migrate"); + } + return clearDataAndRelaunch(); + } + + private async consultUserConsentDegradedMode(backend: "plaintext" | "basic_text"): Promise { + if (backend === "plaintext") { + // Sometimes we may have a working backend that for some reason does not support encryption at the moment. + // This may be because electron reported an incorrect backend or because of some known issues with the keyring itself. + // Or the environment specified `--storage-mode=force-plaintext`. + // In any case, when this happens, we give the user an option to use a weaker form of encryption. + const { response } = await dialog.showMessageBox({ + title: _t("store|error|backend_no_encryption_title"), + message: _t("store|error|backend_no_encryption"), + detail: _t("store|error|backend_no_encryption_detail", { + backend: safeStorage.getSelectedStorageBackend(), + brand: global.vectorConfig.brand || "Element", + }), + type: "error", + buttons: [_t("action|cancel"), _t("store|error|unsupported_keyring_use_plaintext")], + defaultId: 0, + cancelId: 0, + }); + if (response === 0) { + throw new Error("isEncryptionAvailable=false and user rejected plaintext"); + } + } else { + // Electron did not identify a compatible encrypted backend, ask user for consent to degraded mode + const { response } = await dialog.showMessageBox({ + title: _t("store|error|unsupported_keyring_title"), + message: _t("store|error|unsupported_keyring"), + detail: _t("store|error|unsupported_keyring_detail", { + brand: global.vectorConfig.brand || "Element", + link: "https://www.electronjs.org/docs/latest/api/safe-storage#safestoragegetselectedstoragebackend-linux", + }), + type: "error", + buttons: [_t("action|cancel"), _t("store|error|unsupported_keyring_use_basic_text")], + defaultId: 0, + cancelId: 0, + }); + if (response === 0) { + throw new Error("safeStorage backend basic_text and user rejected it"); + } + } } private recordSafeStorageBackend(backend: SafeStorageBackend): void { @@ -373,35 +387,37 @@ class Store extends ElectronStore { /** * Linux support for upgrading the backend from basic_text to one of the encrypted backends, * this is quite a tricky process as the backend is not known until the app is ready & cannot be changed once it is. - * First we restart the app in basic_text backend mode, then decrypt the data & restart back in default backend mode, - * and re-encrypt the data. + * 1. We restart the app in safeStorageBackendMigrate mode + * 2. Now that we are in the mode which our data is written in we decrypt the data, write it back in plaintext + * & restart back in default backend mode, + * 3. Finally, we load the plaintext data & encrypt it. */ - private upgradeLinuxBackend1(): void { + private prepareMigrateBasicTextToPlaintext(): void { console.info(`Starting safeStorage migration to ${safeStorage.getSelectedStorageBackend()}`); this.set("safeStorageBackendMigrate", true); relaunchApp(); } - private upgradeLinuxBackend2(): void { - this.secrets = new PlaintextStorageWriter(this); + private migrateBasicTextToPlaintext(): void { + const secrets = new SafeStorageWriter(this); console.info("Performing safeStorage migration"); const data = this.get("safeStorage"); if (data) { for (const key in data) { - this.set(this.secrets.getKey(key), this.secrets!.get(key)); + this.set(secrets.getKey(key), secrets.get(key)); } this.recordSafeStorageBackend("plaintext"); } - this.set("safeStorageBackendMigrate", false); + this.delete("safeStorageBackendMigrate"); relaunchApp(); } - private upgradeLinuxBackend3(): void { - this.secrets = new PlaintextStorageWriter(this); + private migratePlaintextToEncrypted(): void { + const secrets = new SafeStorageWriter(this); const selectedSafeStorageBackend = safeStorage.getSelectedStorageBackend(); console.info(`Finishing safeStorage migration to ${selectedSafeStorageBackend}`); const data = this.get("safeStorage"); if (data) { for (const key in data) { - this.secrets.set(key, data[key]); + secrets.set(key, data[key]); } } this.recordSafeStorageBackend(selectedSafeStorageBackend);