Add code to migrate from keytar to safeStorage

Translated to TypeScript from PR #3742

src/desktop/DesktopMain.ts

@@ -18,7 +18,7 @@ import { DesktopTray } from "./tray/DesktopTray"
 import { log } from "./DesktopLog"
 import { UpdaterWrapper } from "./UpdaterWrapper"
 import { ElectronNotificationFactory } from "./NotificatonFactory"
-import { KeytarSecretStorage } from "./sse/SecretStorage"
+import { KeytarSecretStorage, SafeStorageSecretStorage } from "./sse/SecretStorage"
 import fs from "node:fs"
 import { DesktopIntegrator, getDesktopIntegratorForPlatform } from "./integration/DesktopIntegrator"
 import net from "node:net"
@@ -130,8 +130,10 @@ if (opts.registerAsMailHandler && opts.unregisterAsMailHandler) {
 async function createComponents(): Promise<Components> {
 	const en = (await import("../translations/en.js")).default
 	lang.init(en)
-	const secretStorage = new KeytarSecretStorage()
-	const keyStoreFacade = new KeyStoreFacadeImpl(secretStorage, desktopCrypto)
+	const keytar = await import("keytar")
+	const secretStorage = new KeytarSecretStorage(keytar)
+	const safeStorageSecretStorage = new SafeStorageSecretStorage(electron, fs, path, secretStorage)
+	const keyStoreFacade = new KeyStoreFacadeImpl(safeStorageSecretStorage, desktopCrypto)
 	const configMigrator = new DesktopConfigMigrator(desktopCrypto, keyStoreFacade, electron)
 	const conf = new DesktopConfig(configMigrator, keyStoreFacade, desktopCrypto)
 	// Fire config loading, dont wait for it

src/desktop/sse/SecretStorage.ts

@@ -1,8 +1,9 @@
-import { default as keytar } from "keytar"
 import { CancelledError } from "../../api/common/error/CancelledError"
 import { noOp } from "@tutao/tutanota-utils"
-
-const { CANCELLED, getPassword, setPassword } = keytar
+import * as PathModule from "node:path"
+import * as FsModule from "node:fs"
+import { DeviceStorageUnavailableError } from "../../api/common/error/DeviceStorageUnavailableError.js"
+import type { default as Keytar } from "keytar"
 
 export interface SecretStorage {
 	getPassword(service: string, account: string): Promise<string | null>
@@ -11,17 +12,28 @@ export interface SecretStorage {
 }
 
 export class KeytarSecretStorage implements SecretStorage {
+	private readonly CANCELLED: string
+	private readonly _getPassword: (service: string, account: string) => Promise<string | null>
+	private readonly _setPassword: (service: string, account: string, password: string) => Promise<void>
+
 	/**
 	 * keytar can't handle concurrent accesses to the keychain, so we need to sequence
 	 * calls to getPassword and setPassword.
 	 * this promise chain stores pending operations.
 	 */
+
+	constructor(keytar: typeof Keytar) {
+		this.CANCELLED = keytar.CANCELLED
+		this._getPassword = keytar.getPassword
+		this._setPassword = keytar.setPassword
+	}
+
 	private lastOp: Promise<unknown> = Promise.resolve()
 
 	getPassword(service: string, account: string): Promise<string | null> {
 		const newOp = this.lastOp.catch(noOp).then(() =>
-			getPassword(service, account).catch((e) => {
-				if (e.message === CANCELLED) {
+			this._getPassword(service, account).catch((e) => {
+				if (e.message === this.CANCELLED) {
 					throw new CancelledError("user cancelled keychain unlock")
 				}
 				throw e
@@ -33,8 +45,100 @@ export class KeytarSecretStorage implements SecretStorage {
 	}
 
 	setPassword(service: string, account: string, password: string): Promise<void> {
-		const newOp = this.lastOp.catch(noOp).then(() => setPassword(service, account, password))
+		const newOp = this.lastOp.catch(noOp).then(() => this._setPassword(service, account, password))
 		this.lastOp = newOp
 		return newOp
 	}
 }
+
+/**
+ * Secret Storage impl using the electron 15+ SafeStorage API
+ *
+ * Note: the main thread will be blocked while the keychain is being unlocked,
+ * potentially for as long as the user takes to enter a password.
+ * We're asking for access before any windows are created, which should prevent
+ * any weirdness arising from that.
+ */
+export class SafeStorageSecretStorage implements SecretStorage {
+	private initialized = false
+
+	constructor(
+		private readonly electron: typeof Electron.CrossProcessExports,
+		private readonly fs: typeof FsModule,
+		private readonly path: typeof PathModule,
+		private readonly keytarSecretStorage: KeytarSecretStorage,
+	) {}
+
+	async getPassword(service: string, account: string): Promise<string | null> {
+		await this.assertAvailable()
+		const keyPath = this.getKeyPath(service, account)
+		try {
+			const encPwBuffer = await this.fs.promises.readFile(keyPath)
+			return this.electron.safeStorage.decryptString(encPwBuffer)
+		} catch (e) {
+			if (e.code === "ENOENT") {
+				// we might not have the key in safeStorage yet, but we might have it in the keytar storage.
+				return await this.migrateKeytarPassword(service, account)
+			}
+			throw e
+		}
+	}
+
+	async setPassword(service: string, account: string, password: string): Promise<void> {
+		await this.assertAvailable()
+		const keyPath = this.getKeyPath(service, account)
+		const cypherBuffer = this.electron.safeStorage.encryptString(password)
+		return this.fs.promises.writeFile(keyPath, cypherBuffer)
+	}
+
+	private getKeyPath(service: string, account: string): string {
+		const fname = service.concat("-", account)
+		const safeStoragePath = this.getSafeStoragePath()
+		return this.path.join(safeStoragePath, fname)
+	}
+
+	/**
+	 * this should always be a path inside the user's home directory (or equivalent)
+	 * @private
+	 */
+	private getSafeStoragePath(): string {
+		return this.path.join(this.electron.app.getPath("userData"), "safe_storage")
+	}
+
+	/**
+	 * ensures that the safe_storage directory exists and that we can use the
+	 * safeStorage API
+	 * @private
+	 */
+	private async assertAvailable(): Promise<void> {
+		await this.fs.promises.mkdir(this.getSafeStoragePath(), { recursive: true })
+		if (this.electron.safeStorage.isEncryptionAvailable()) {
+			if (!this.initialized) {
+				this.initialized = true
+				console.log("using safeStorage with backend", process.platform, this.electron.safeStorage.getSelectedStorageBackend())
+			}
+			return
+		}
+		throw new DeviceStorageUnavailableError("safeStorage API is not available", null)
+	}
+
+	/**
+	 * most devices will have stored a deviceKey with keytar, which we can move
+	 * to the safeStorage impl.
+	 *
+	 * @private
+	 */
+	private async migrateKeytarPassword(service: string, account: string): Promise<string | null> {
+		let keytarPw: string | null = null
+		try {
+			keytarPw = await this.keytarSecretStorage.getPassword(service, account)
+		} catch (e) {
+			console.log("keytar failed, assuming there's no pw stored")
+		}
+		if (keytarPw != null) {
+			await this.setPassword(service, account, keytarPw)
+		}
+
+		return keytarPw
+	}
+}

test/tests/Suite.ts

@@ -150,6 +150,7 @@ async function setupSuite({ integration }: { integration?: boolean }) {
 		await import("./desktop/DesktopNotifierTest.js")
 		await import("./desktop/ApplicationWindowTest.js")
 		await import("./desktop/sse/DesktopSseClientTest.js")
+		await import("./desktop/sse/SecretStorageTest.js")
 		await import("./desktop/sse/DesktopAlarmStorageTest.js")
 		await import("./desktop/sse/DesktopAlarmSchedulerTest.js")
 		await import("./desktop/files/DesktopFileFacadeTest.js")

test/tests/desktop/sse/SecretStorageTest.ts

@@ -0,0 +1,62 @@
+import o from "@tutao/otest"
+import { KeytarSecretStorage, SafeStorageSecretStorage } from "../../../../src/desktop/sse/SecretStorage.js"
+import type { ElectronExports, FsExports } from "../../../../src/desktop/ElectronExportTypes.js"
+import path from "node:path"
+import { matchers, object, verify, when } from "testdouble"
+import { assertThrows } from "@tutao/tutanota-test-utils"
+import { DeviceStorageUnavailableError } from "../../../../src/api/common/error/DeviceStorageUnavailableError.js"
+
+o.spec("SecretStorage", function () {
+	o.spec("SafeStorageSecretStorage", function () {
+		let electron: ElectronExports
+		let fs: FsExports
+		let keytarSecretStorage: KeytarSecretStorage
+		let subject: SafeStorageSecretStorage
+		o.beforeEach(() => {
+			electron = object()
+			fs = object()
+			keytarSecretStorage = object()
+			subject = new SafeStorageSecretStorage(electron, fs, path, keytarSecretStorage)
+		})
+
+		o("will not ask keytar if there is a password file", async function () {
+			when(fs.promises.readFile(matchers.anything())).thenResolve(Buffer.from([1, 2, 3, 4, 5]))
+			when(electron.safeStorage.isEncryptionAvailable()).thenReturn(true)
+			when(electron.app.getPath(matchers.anything())).thenReturn("/any/path")
+			when(fs.promises.mkdir(matchers.anything(), matchers.anything())).thenResolve("some/path")
+			await subject.getPassword("service", "account")
+			verify(keytarSecretStorage.getPassword(matchers.anything(), matchers.anything()), { times: 0 })
+			verify(fs.promises.writeFile(matchers.anything(), matchers.anything()), { times: 0 })
+		})
+
+		o("will ask keytar if there is no password file", async function () {
+			when(fs.promises.readFile(matchers.anything())).thenReject({ code: "ENOENT" })
+
+			when(electron.safeStorage.isEncryptionAvailable()).thenReturn(true)
+			when(electron.app.getPath(matchers.anything())).thenReturn("/any/path")
+			when(fs.promises.mkdir(matchers.anything(), matchers.anything())).thenResolve("some/path")
+			when(keytarSecretStorage.getPassword(matchers.anything(), matchers.anything())).thenResolve(null)
+			const pw = await subject.getPassword("service", "account")
+			o(pw).equals(null)
+			verify(fs.promises.writeFile(matchers.anything(), matchers.anything()), { times: 0 })
+		})
+
+		o("will write the keytar password if found", async function () {
+			when(fs.promises.readFile(matchers.anything())).thenReject({ code: "ENOENT" })
+
+			when(electron.safeStorage.isEncryptionAvailable()).thenReturn(true)
+			when(electron.app.getPath(matchers.anything())).thenReturn("/any/path")
+			when(fs.promises.mkdir(matchers.anything(), matchers.anything())).thenResolve("some/path")
+			when(keytarSecretStorage.getPassword(matchers.anything(), matchers.anything())).thenResolve("hellopassword")
+			await subject.getPassword("service", "account")
+			verify(fs.promises.writeFile(matchers.anything(), matchers.anything()), { times: 1 })
+		})
+
+		o("will throw an error if there is no safeStorage available", async function () {
+			when(electron.safeStorage.isEncryptionAvailable()).thenReturn(false)
+			when(electron.app.getPath(matchers.anything())).thenReturn("/any/path")
+			await assertThrows(DeviceStorageUnavailableError, () => subject.getPassword("service", "account"))
+			await assertThrows(DeviceStorageUnavailableError, () => subject.setPassword("service", "account", "password"))
+		})
+	})
+})