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 21, 2021
1 parent 9a71c53 commit 275fec9
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 27 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
8 changes: 4 additions & 4 deletions src/desktop/DesktopUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {app} from 'electron'
import {defer, delay, noOp, uint8ArrayToHex} from '@tutao/tutanota-utils'
import {log} from "./DesktopLog"
import {DesktopCryptoFacade} from "./DesktopCryptoFacade"
import {fileExists, swapFilename} from "./PathUtils"
import {fileExists, replaceLastComponent} from "./PathUtils"
import url from "url"
import {registerKeys, unregisterKeys} from "./reg-templater"

Expand All @@ -29,7 +29,7 @@ export class DesktopUtils {
}
checkIsPerUserInstall(): Promise<boolean> {
const markerPath = swapFilename(process.execPath, "per_user")
const markerPath = replaceLastComponent(process.execPath, "per_user")
return fileExists(markerPath)
}
Expand Down Expand Up @@ -217,7 +217,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 @@ -234,7 +234,7 @@ export class DesktopUtils {
// * where to put tmp files (also user-dependent)
// all these must be set in the registry
const execPath = process.execPath
const dllPath = swapFilename(execPath, 'mapirs.dll')
const dllPath = replaceLastComponent(execPath, 'mapirs.dll')
// we may be a per-machine installation that's used by multiple users, so the dll will replace %USERPROFILE%
// with the value of the USERPROFILE env var.
const appData = path.join("%USERPROFILE%", 'AppData')
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 275fec9

Please sign in to comment.