From f8f7eedad362214879fc532a47d6bbdd3a0d61e6 Mon Sep 17 00:00:00 2001 From: Krzysztof Jelski Date: Wed, 10 Jun 2020 18:09:55 +0200 Subject: [PATCH] Improve Electron App security (#2923) * WIP on storing accounts without electron remote modules. * Improve security by isolating main and renderer. * Apply suggestions from code review Co-authored-by: Jaco Greeff Co-authored-by: Jaco Greeff --- package.json | 9 ++-- .../src/api/account-store-api.ts | 12 ++++++ .../src/api/electron-main-api.ts | 9 ++++ .../src/api/global-exported-api.ts | 13 ++++++ packages/apps-electron/src/electron.ts | 8 +++- packages/apps-electron/src/index.tsx | 9 +--- .../apps-electron/src/main/account-store.ts | 39 +++++++++++++++++ packages/apps-electron/src/preload.ts | 15 +++++++ .../src/renderer/remote-electron-store.ts | 35 +++++++++++++++ packages/apps-electron/webpack.main.config.js | 7 ++- .../apps-electron/webpack.renderer.config.js | 2 +- yarn.lock | 43 ++++++++++++++++--- 12 files changed, 180 insertions(+), 21 deletions(-) create mode 100644 packages/apps-electron/src/api/account-store-api.ts create mode 100644 packages/apps-electron/src/api/electron-main-api.ts create mode 100644 packages/apps-electron/src/api/global-exported-api.ts create mode 100644 packages/apps-electron/src/main/account-store.ts create mode 100644 packages/apps-electron/src/preload.ts create mode 100644 packages/apps-electron/src/renderer/remote-electron-store.ts diff --git a/package.json b/package.json index 8a2c501155..9e45569ba0 100644 --- a/package.json +++ b/package.json @@ -53,9 +53,9 @@ "build:www": "rm -rf packages/apps/build && mkdir -p packages/apps/build && yarn run build:i18n && cd packages/apps && NODE_ENV=production webpack --config webpack.config.js", "build:electron": "yarn clean:electronBuild && yarn build:electronMain && yarn build:electronRenderer", "build:devElectronRenderer": "cd packages/apps-electron && NODE_ENV=development webpack --config webpack.renderer.config.js", - "build:electronRenderer": "cd packages/apps-electron && webpack --config webpack.renderer.config.js", + "build:electronRenderer": "cd packages/apps-electron && NODE_ENV=production webpack --config webpack.renderer.config.js", "build:devElectronMain": "cd packages/apps-electron && NODE_ENV=development webpack --config webpack.main.config.js", - "build:electronMain": "cd packages/apps-electron && webpack --config webpack.main.config.js", + "build:electronMain": "cd packages/apps-electron && NODE_ENV=production webpack --config webpack.main.config.js", "packElectron:test": "yarn build:release:electron && electron-builder --dir", "packElectron:mac": "yarn build:release:electron && electron-builder build --mac", "packElectron:win": "yarn build:release:electron && electron-builder build --win", @@ -63,7 +63,7 @@ "packElectron": "yarn build:release:electron && yarn clean:electronRelease && electron-builder build -mwl", "docs": "echo \"skipping docs\"", "clean": "polkadot-dev-clean-build", - "clean:electronBuild": "cd packages/apps-electron polkadot-dev-clean-build", + "clean:electronBuild": "cd packages/apps-electron && polkadot-dev-clean-build", "clean:electronRelease": "cd packages/apps-electron && rm -rf release", "clean:i18n": "rm -rf packages/apps/public/locales/en && mkdir -p packages/apps/public/locales/en", "lint": "polkadot-dev-run-lint", @@ -96,8 +96,9 @@ "@types/styled-components": "^5.1.0", "@types/styled-theming": "^2.2.3", "concurrently": "^5.2.0", + "devtron": "^1.4.0", "dnslink-cloudflare": "^2.0.4", - "electron": "8.3.1", + "electron": "^9.0.2", "electron-builder": "^22.7.0", "electron-builder-notarize": "^1.1.2", "i18next-scanner": "^2.11.0", diff --git a/packages/apps-electron/src/api/account-store-api.ts b/packages/apps-electron/src/api/account-store-api.ts new file mode 100644 index 0000000000..0093a5a331 --- /dev/null +++ b/packages/apps-electron/src/api/account-store-api.ts @@ -0,0 +1,12 @@ +// Copyright 2017-2020 @polkadot/apps authors & contributors +// This software may be modified and distributed under the terms +// of the Apache-2.0 license. See the LICENSE file for details. + +import { KeyringJson } from '@polkadot/ui-keyring/types'; + +export interface AccountStoreApi { + all: () => Promise<{ key: string, value: KeyringJson }[]> + get: (key: string) => Promise + remove: (key: string) => Promise + set: (key: string, value: KeyringJson) => Promise +} diff --git a/packages/apps-electron/src/api/electron-main-api.ts b/packages/apps-electron/src/api/electron-main-api.ts new file mode 100644 index 0000000000..1d315b794c --- /dev/null +++ b/packages/apps-electron/src/api/electron-main-api.ts @@ -0,0 +1,9 @@ +// Copyright 2017-2020 @polkadot/apps authors & contributors +// This software may be modified and distributed under the terms +// of the Apache-2.0 license. See the LICENSE file for details. + +import { AccountStoreApi } from './account-store-api'; + +export interface ElectronMainApi { + accountStore: AccountStoreApi +} diff --git a/packages/apps-electron/src/api/global-exported-api.ts b/packages/apps-electron/src/api/global-exported-api.ts new file mode 100644 index 0000000000..ade5235c82 --- /dev/null +++ b/packages/apps-electron/src/api/global-exported-api.ts @@ -0,0 +1,13 @@ +// Copyright 2017-2020 @polkadot/apps authors & contributors +// This software may be modified and distributed under the terms +// of the Apache-2.0 license. See the LICENSE file for details. + +import { ElectronMainApi } from './electron-main-api'; + +declare global { + interface Window { + ElectronMain: ElectronMainApi + } +} + +export const electronMainApi = window.ElectronMain; diff --git a/packages/apps-electron/src/electron.ts b/packages/apps-electron/src/electron.ts index fe4905db6a..fe90a27d5e 100644 --- a/packages/apps-electron/src/electron.ts +++ b/packages/apps-electron/src/electron.ts @@ -6,6 +6,7 @@ import { app, BrowserWindow, dialog, screen } from 'electron'; import { autoUpdater } from 'electron-updater'; import path from 'path'; import { features } from './featureToggles'; +import { registerAccountStoreHandlers } from './main/account-store'; const ENV = process.env.NODE_ENV || 'production'; const isDev = ENV === 'development'; @@ -16,8 +17,10 @@ function createWindow (): Promise { const win = new BrowserWindow({ height, webPreferences: { - enableRemoteModule: true, - nodeIntegration: true + contextIsolation: true, + enableRemoteModule: false, + nodeIntegration: false, + preload: path.join(__dirname, 'preload.js') }, width }); @@ -34,6 +37,7 @@ function createWindow (): Promise { } const onReady = async () => { + registerAccountStoreHandlers(); await createWindow(); if (features.autoUpdater) { diff --git a/packages/apps-electron/src/index.tsx b/packages/apps-electron/src/index.tsx index cdb879a3c6..e03cfbb37e 100644 --- a/packages/apps-electron/src/index.tsx +++ b/packages/apps-electron/src/index.tsx @@ -7,8 +7,6 @@ import '@polkadot/apps/initSettings'; import 'semantic-ui-css/semantic.min.css'; import '@polkadot/react-components/i18n'; -import electron from 'electron'; -import path from 'path'; import React, { Suspense } from 'react'; import ReactDOM from 'react-dom'; import { HashRouter } from 'react-router-dom'; @@ -20,21 +18,18 @@ import { BlockAuthors, Events } from '@polkadot/react-query'; import AccountSidebar from '@polkadot/app-accounts/Sidebar'; import { Api } from '@polkadot/react-api'; import Apps from '@polkadot/apps/Apps'; -import { FileStore } from '@polkadot/ui-keyring/stores'; +import { RemoteElectronStore } from './renderer/remote-electron-store'; const rootId = 'root'; const rootElement = document.getElementById(rootId); const theme = { theme: settings.uiTheme }; -const defaultStorePath = path.join((electron.app || electron.remote.app).getPath('userData'), 'polkadot'); -const store = new FileStore(defaultStorePath); +const store = new RemoteElectronStore(); if (!rootElement) { throw new Error(`Unable to find element with id '${rootId}'`); } -console.log('Opened in electron app'); - ReactDOM.render( diff --git a/packages/apps-electron/src/main/account-store.ts b/packages/apps-electron/src/main/account-store.ts new file mode 100644 index 0000000000..419fe6d4c0 --- /dev/null +++ b/packages/apps-electron/src/main/account-store.ts @@ -0,0 +1,39 @@ +// Copyright 2017-2020 @polkadot/apps authors & contributors +// This software may be modified and distributed under the terms +// of the Apache-2.0 license. See the LICENSE file for details. + +import { FileStore } from '@polkadot/ui-keyring/stores'; +import { KeyringJson } from '@polkadot/ui-keyring/types'; +import { app, ipcMain } from 'electron'; +import path from 'path'; + +const ACCOUNTS_SUBFOLDER = 'polkadot-accounts'; + +export const registerAccountStoreHandlers = (): void => { + const defaultStorePath = path.join(app.getPath('userData'), ACCOUNTS_SUBFOLDER); + const fileStore = new FileStore(defaultStorePath); + + ipcMain.handle('account-store-set', async (_, key: string, value: KeyringJson) => new Promise((resolve) => + fileStore.set(key, value, resolve) + )); + + ipcMain.handle('account-store-get', async (_, key: string) => new Promise((resolve) => + fileStore.get(key, resolve) + )); + + ipcMain.handle('account-store-remove', async (_, key: string) => new Promise((resolve) => + fileStore.remove(key, resolve) + )); + + ipcMain.handle('account-store-all', () => { + let result: { key: string, value: KeyringJson }[] = []; + + const collect = (key: string, value: KeyringJson) => { + result = [...result, { key, value }]; + }; + + fileStore.all(collect); + + return result; + }); +}; diff --git a/packages/apps-electron/src/preload.ts b/packages/apps-electron/src/preload.ts new file mode 100644 index 0000000000..cfa75dd27c --- /dev/null +++ b/packages/apps-electron/src/preload.ts @@ -0,0 +1,15 @@ +// Copyright 2017-2020 @polkadot/apps authors & contributors +// This software may be modified and distributed under the terms +// of the Apache-2.0 license. See the LICENSE file for details. + +import { KeyringJson } from '@polkadot/ui-keyring/types'; +import { contextBridge, ipcRenderer } from 'electron'; + +contextBridge.exposeInMainWorld('ElectronMain', { + accountStore: { + all: () => ipcRenderer.invoke('account-store-all'), + get: (key: string) => ipcRenderer.invoke('account-store-get', key), + remove: (key: string) => ipcRenderer.invoke('account-store-remove', key), + set: (key: string, value: KeyringJson) => ipcRenderer.invoke('account-store-set', key, value) + } +}); diff --git a/packages/apps-electron/src/renderer/remote-electron-store.ts b/packages/apps-electron/src/renderer/remote-electron-store.ts new file mode 100644 index 0000000000..0f62112427 --- /dev/null +++ b/packages/apps-electron/src/renderer/remote-electron-store.ts @@ -0,0 +1,35 @@ +// Copyright 2017-2020 @polkadot/apps authors & contributors +// This software may be modified and distributed under the terms +// of the Apache-2.0 license. See the LICENSE file for details. + +import { KeyringJson, KeyringStore } from '@polkadot/ui-keyring/types'; +import { electronMainApi } from '../api/global-exported-api'; + +export class RemoteElectronStore implements KeyringStore { + all (cb: (key: string, value: KeyringJson) => void): void { + electronMainApi.accountStore.all() + .then((result: { key: string, value: KeyringJson }[]) => result.forEach(({ key, value }) => cb(key, value))) + .catch(() => { + throw new Error('error getting all accounts'); + }); + } + + get (key: string, cb: (value: KeyringJson) => void): void { + electronMainApi.accountStore.get(key) + .then(cb).catch(() => { + throw new Error('error storing account'); + }); + } + + remove (key: string, cb: (() => void) | undefined): void { + electronMainApi.accountStore.remove(key).then(cb).catch(() => { + throw new Error('error removing account'); + }); + } + + set (key: string, value: KeyringJson, cb: (() => void) | undefined): void { + electronMainApi.accountStore.set(key, value).then(cb).catch(() => { + throw new Error('error saving account'); + }); + } +} diff --git a/packages/apps-electron/webpack.main.config.js b/packages/apps-electron/webpack.main.config.js index 15a74c8d48..9aba91e364 100644 --- a/packages/apps-electron/webpack.main.config.js +++ b/packages/apps-electron/webpack.main.config.js @@ -13,7 +13,10 @@ const isProd = ENV === 'production'; function createWebpack () { return [ { - entry: './src/electron.ts', + entry: { + electron: './src/electron.ts', + preload: './src/preload.ts' + }, mode: ENV, module: { rules: [ @@ -38,7 +41,7 @@ function createWebpack () { minimizer: [new TerserPlugin()] }, output: { - filename: 'electron.js', + filename: '[name].js', path: path.join(__dirname, '/build') }, resolve: { diff --git a/packages/apps-electron/webpack.renderer.config.js b/packages/apps-electron/webpack.renderer.config.js index 242e7edabc..e7c2040159 100644 --- a/packages/apps-electron/webpack.renderer.config.js +++ b/packages/apps-electron/webpack.renderer.config.js @@ -27,6 +27,6 @@ module.exports = merge( template: path.join(context, '../apps/public/index.html') }) ], - target: 'electron-renderer' + target: 'web' } ); diff --git a/yarn.lock b/yarn.lock index 0ef2cf21c5..f029ea0a5b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4973,6 +4973,13 @@ __metadata: languageName: node linkType: hard +"accessibility-developer-tools@npm:^2.11.0": + version: 2.12.0 + resolution: "accessibility-developer-tools@npm:2.12.0" + checksum: 3/edb92620161e1fbf9a81ed02269b2148b3245e2c96b7bbeaf47e65a4259bfa69a015ef662958fe9cdd9def26b105ff3d525be6328f637c3a7820ab90e47d7dea + languageName: node + linkType: hard + "acorn-bigint@npm:^0.4.0": version: 0.4.0 resolution: "acorn-bigint@npm:0.4.0" @@ -9043,6 +9050,17 @@ __metadata: languageName: node linkType: hard +"devtron@npm:^1.4.0": + version: 1.4.0 + resolution: "devtron@npm:1.4.0" + dependencies: + accessibility-developer-tools: ^2.11.0 + highlight.js: ^9.3.0 + humanize-plus: ^1.8.1 + checksum: 3/c481527b34c0c3a90e0ccd88bb2d04b1d9ffb97c4b7a1b2431a12ade59998c3b8aa7755e2392ea06db5ab44d0b82c1f7aedc092c8be8533de65c35de7c764d73 + languageName: node + linkType: hard + "dezalgo@npm:^1.0.0": version: 1.0.3 resolution: "dezalgo@npm:1.0.3" @@ -9530,16 +9548,16 @@ __metadata: languageName: node linkType: hard -"electron@npm:8.3.1": - version: 8.3.1 - resolution: "electron@npm:8.3.1" +"electron@npm:^9.0.2": + version: 9.0.2 + resolution: "electron@npm:9.0.2" dependencies: "@electron/get": ^1.0.1 "@types/node": ^12.0.12 extract-zip: ^1.0.3 bin: electron: cli.js - checksum: 3/309d57ef152be5bf4145387bee47f5e2ff866636745b3b257d5d1b2bd7ddf2e4f7094e2f2d355affccf2872e2072c3aeb5ef43e28473373b5cb77b78fd42ebb2 + checksum: 3/bd73108257b830c24710bb8aae28deebc3e6fefb026f0bd3ae71819791405b63904e62d654cd9611f0f11292d85682117e40e296cd58430de3506e36de4c691c languageName: node linkType: hard @@ -12129,6 +12147,13 @@ __metadata: languageName: node linkType: hard +"highlight.js@npm:^9.3.0": + version: 9.18.1 + resolution: "highlight.js@npm:9.18.1" + checksum: 3/3fe99105e4cda17ae8dac7b49b791065b9b8306c1de40955cd59ce1a2cd2bed2683135a26f9ffd8f872a1d6e9d77935c48ffbb2557c228e647fe940a3bbbc4f2 + languageName: node + linkType: hard + "history@npm:^4.9.0": version: 4.10.1 resolution: "history@npm:4.10.1" @@ -12546,6 +12571,13 @@ __metadata: languageName: node linkType: hard +"humanize-plus@npm:^1.8.1": + version: 1.8.2 + resolution: "humanize-plus@npm:1.8.2" + checksum: 3/5b728bf5ca8a3cfbcec8fc6e381cda889a76e2486f3f7a7b6a867ed26cb63307bbe61a7f205f61c2588ccc46a755a4f6db80c73c3a74a9a6a95b90e2f9412a48 + languageName: node + linkType: hard + "humanize-url@npm:^1.0.0": version: 1.0.1 resolution: "humanize-url@npm:1.0.1" @@ -18030,8 +18062,9 @@ __metadata: "@types/styled-components": ^5.1.0 "@types/styled-theming": ^2.2.3 concurrently: ^5.2.0 + devtron: ^1.4.0 dnslink-cloudflare: ^2.0.4 - electron: 8.3.1 + electron: ^9.0.2 electron-builder: ^22.7.0 electron-builder-notarize: ^1.1.2 i18next-scanner: ^2.11.0