Skip to content

Commit

Permalink
[desktop] use electron safeStorage to store device keys
Browse files Browse the repository at this point in the history
* renames PathUtils.swapFilename to replaceLastComponent
* provides a new impl for SecretStorage
* uses electron.safeStorage to encrypt the device key that then gets
stored in a file in app.getPath('userData')/safe_storage/
* from now on, per-user data is encrypted with a per-user key,
even for per-machine installs.

#3733
close #3676
  • Loading branch information
ganthern committed Dec 16, 2021
1 parent e7dfff5 commit 456fc5b
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 26 deletions.
2 changes: 1 addition & 1 deletion src/api/common/error/DeviceStorageUnavailableError.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import {TutanotaError} from "./TutanotaError"

export class DeviceStorageUnavailableError extends TutanotaError {
constructor(msg: string, error: Error) {
constructor(msg: string, error: ?Error) {
super("DeviceStorageUnavailableError", error ? (msg + "> " + (error.stack ? error.stack : error.message)) : msg)
}
}
5 changes: 3 additions & 2 deletions src/desktop/DesktopMain.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ import {DesktopTray} from "./tray/DesktopTray"
import {log} from "./DesktopLog";
import {UpdaterWrapperImpl} from "./UpdaterWrapper"
import {ElectronNotificationFactory} from "./NotificatonFactory"
import {KeytarSecretStorage} from "./sse/SecretStorage"
import {SafeStorageSecretStorage} from "./sse/SecretStorage"
import fs from "fs"
import path from "path"
import {DesktopIntegrator, getDesktopIntegratorForPlatform} from "./integration/DesktopIntegrator"
import net from "net"
import child_process from "child_process"
Expand Down Expand Up @@ -96,7 +97,7 @@ if (opts.registerAsMailHandler && opts.unregisterAsMailHandler) {

async function createComponents(): Promise<Components> {
lang.init(en)
const secretStorage = new KeytarSecretStorage()
const secretStorage = new SafeStorageSecretStorage(electron, fs, path)
const deviceKeyProvider = new DeviceKeyProviderImpl(secretStorage, desktopCrypto)
const configMigrator = new DesktopConfigMigrator(desktopCrypto, deviceKeyProvider, electron)
const conf = new DesktopConfig(app, configMigrator, deviceKeyProvider, desktopCrypto)
Expand Down
6 changes: 3 additions & 3 deletions src/desktop/DesktopUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {log} from "./DesktopLog"
import {uint8ArrayToHex} from "@tutao/tutanota-utils"
import {delay} from "@tutao/tutanota-utils"
import {DesktopCryptoFacade} from "./DesktopCryptoFacade"
import {swapFilename} from "./PathUtils"
import {replaceLastComponent} from "./PathUtils"
import url from "url"

export class DesktopUtils {
Expand Down Expand Up @@ -214,7 +214,7 @@ export class DesktopUtils {
*/
_writeToDisk(contents: string): string {
const filename = uint8ArrayToHex(this._desktopCrypto.randomBytes(12))
const filePath = swapFilename(process.execPath, filename)
const filePath = replaceLastComponent(process.execPath, filename)
this._fs.writeFileSync(filePath, contents, {encoding: 'utf-8', mode: 0o400})
return filePath
}
Expand All @@ -225,7 +225,7 @@ export class DesktopUtils {
async _registerOnWin(): Promise<void> {
const execPath = process.execPath
const dllPath = swapFilename(execPath, "mapirs.dll")
const dllPath = replaceLastComponent(execPath, "mapirs.dll")
const logPath = path.join(app.getPath('userData'), 'logs')
const tmpPath = this.getTutanotaTempPath('attach')
const tmpRegScript = (await import('./reg-templater.js')).registerKeys(
Expand Down
8 changes: 4 additions & 4 deletions src/desktop/PathUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ export function urlIsPrefix(prefix: URL, url: URL): boolean {
}

/**
* replace the last component in a file path with another
* replace the last component in a path with another
* @param p path to a file/folder
* @param file the file name to put in the last path component
* @param newLast the name to put in the last path component
* @param pathModule path module to use for cross platform testing
*/
export function swapFilename(p: string, file: string, pathModule: $Exports<"path"> = path): string {
export function replaceLastComponent(p: string, newLast: string, pathModule: $Exports<"path"> = path): string {
const dir = pathModule.dirname(p)
return pathModule.join(dir, file)
return pathModule.join(dir, newLast)
}
113 changes: 110 additions & 3 deletions src/desktop/sse/SecretStorage.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,126 @@
//@flow

import {getPassword, setPassword} from "keytar"
import {deletePassword, getPassword, setPassword} from "keytar"
import type {App, SafeStorage} from "electron"
import {log} from "../DesktopLog"
import {DeviceStorageUnavailableError} from "../../api/common/error/DeviceStorageUnavailableError"

export interface SecretStorage {
getPassword(service: string, account: string): Promise<?string>;

setPassword(service: string, account: string, password: string): Promise<void>;
}

export class KeytarSecretStorage implements SecretStorage {
/**
* @deprecated: will be replaced by SafeStorageSecretStorage
*/
class KeytarSecretStorage implements SecretStorage {

getPassword(service: string, account: string): Promise<?string> {
return getPassword(service, account)
}

setPassword(service: string, account: string, password: string): Promise<void> {
return setPassword(service, account, password);
return setPassword(service, account, password)
}

deletePassword(service: string, account: string): Promise<boolean> {
return deletePassword(service, account)
}
}

/**
* 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 {
_safeStorage: SafeStorage
_app: App
_fs: $Exports<"fs">
_path: $Exports<"path">
_keytarSecretStorage: KeytarSecretStorage

constructor({safeStorage, app}: $Exports<"electron">, fs: $Exports<"fs">, path: $Exports<"path">) {
this._safeStorage = safeStorage
this._app = app
this._fs = fs
this._path = path
this._keytarSecretStorage = new KeytarSecretStorage()
}
async getPassword(service: string, account: string): Promise<?string> {
await this._assertAvailable()
await this._migrateKeytarPassword(service, account)
const keyPath = this._getKeyPath(service, account)
try {
const encPwBuffer = await this._fs.promises.readFile(keyPath)
const plainPw = this._safeStorage.decryptString(encPwBuffer)
return Promise.resolve(plainPw)
} catch (e) {
if (e.code === "ENOENT") return null
throw e
}
}
async setPassword(service: string, account: string, password: string): Promise<void> {
await this._assertAvailable()
const keyPath = this._getKeyPath(service, account)
const cypherBuffer = this._safeStorage.encryptString(password)
return this._fs.promises.writeFile(keyPath, cypherBuffer)
}
_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
*/
_getSafeStoragePath(): string {
return this._path.join(this._app.getPath('userData'), "safe_storage")
}
/**
* ensures that the safe_storage directory exists and that we can use the
* safeStorage API
* @private
*/
async _assertAvailable(): Promise<void> {
await this._fs.promises.mkdir(this._getSafeStoragePath(), {recursive: true})
// see https://github.com/electron/electron/issues/32206
// the rest of the safeStorage API should be throwing errors
// we can catch until this works.
if (process.platform === 'linux') return
if (this._safeStorage.isEncryptionAvailable()) 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
*/
async _migrateKeytarPassword(service: string, account: string): Promise<void> {
let keytarPw = null
try {
keytarPw = await this._keytarSecretStorage.getPassword(service, account)
} catch (e) {
log.debug("keytar failed, assuming there's no pw stored")
}
if (keytarPw) {
await this.setPassword(service, account, keytarPw)
// do not do this until later. there may be multiple installs using
// the deviceKey if for some reason keytar used a system keychain
// to store it.
// await this._keytarSecretStorage.deletePassword(service, account)
}
}
}
26 changes: 13 additions & 13 deletions test/client/desktop/PathUtilsTest.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@flow
import o from "ospec"
import {nonClobberingFilename, swapFilename} from "../../../src/desktop/PathUtils"
import {nonClobberingFilename, replaceLastComponent} from "../../../src/desktop/PathUtils"
import path from "path"

o.spec("PathUtils", function () {
Expand Down Expand Up @@ -206,21 +206,21 @@ o.spec("PathUtils", function () {

o.spec("swapFileName Test", function () {
o("replace file with file, posix", function () {
o(swapFilename("/a/b/c.txt", "d.txt")).equals("/a/b/d.txt")
o(swapFilename("a/b/c.txt", "d.txt")).equals("a/b/d.txt")
o(swapFilename("/a/b/c.txt", "d")).equals("/a/b/d")
o(swapFilename("/a/b/c", "d.txt")).equals("/a/b/d.txt")
o(swapFilename("/a/b/c", "d.txt")).equals("/a/b/d.txt")
o(swapFilename("/", "bla.txt")).equals("/bla.txt")
o(replaceLastComponent("/a/b/c.txt", "d.txt")).equals("/a/b/d.txt")
o(replaceLastComponent("a/b/c.txt", "d.txt")).equals("a/b/d.txt")
o(replaceLastComponent("/a/b/c.txt", "d")).equals("/a/b/d")
o(replaceLastComponent("/a/b/c", "d.txt")).equals("/a/b/d.txt")
o(replaceLastComponent("/a/b/c", "d.txt")).equals("/a/b/d.txt")
o(replaceLastComponent("/", "bla.txt")).equals("/bla.txt")
})

o("replace file with file, windows", function () {
o(swapFilename("C:\\tmp\\file.html", "text.txt", path.win32)).equals("C:\\tmp\\text.txt")
o(swapFilename("C:\\tmp\\file.html", "text", path.win32)).equals("C:\\tmp\\text")
o(swapFilename("C:\\tmp\\folder\\", "text", path.win32)).equals("C:\\tmp\\text")
o(swapFilename("tmp\\file.html", "text.txt", path.win32)).equals("tmp\\text.txt")
o(swapFilename("tmp", "text.txt", path.win32)).equals("text.txt")
o(swapFilename("C:\\", "text.txt", path.win32)).equals("C:\\text.txt")
o(replaceLastComponent("C:\\tmp\\file.html", "text.txt", path.win32)).equals("C:\\tmp\\text.txt")
o(replaceLastComponent("C:\\tmp\\file.html", "text", path.win32)).equals("C:\\tmp\\text")
o(replaceLastComponent("C:\\tmp\\folder\\", "text", path.win32)).equals("C:\\tmp\\text")
o(replaceLastComponent("tmp\\file.html", "text.txt", path.win32)).equals("tmp\\text.txt")
o(replaceLastComponent("tmp", "text.txt", path.win32)).equals("text.txt")
o(replaceLastComponent("C:\\", "text.txt", path.win32)).equals("C:\\text.txt")
})
})
})

0 comments on commit 456fc5b

Please sign in to comment.