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 <jacogr@gmail.com>

Co-authored-by: Jaco Greeff <jacogr@gmail.com>

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",

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<KeyringJson>
+  remove: (key: string) => Promise<void>
+  set: (key: string, value: KeyringJson) => Promise<void>
+}

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
+}

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;

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<unknown> {
   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<unknown> {
 }
 
 const onReady = async () => {
+  registerAccountStoreHandlers();
   await createWindow();
 
   if (features.autoUpdater) {

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(
   <Suspense fallback='...'>
     <ThemeProvider theme={theme}>

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;
+  });
+};

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)
+  }
+});

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');
+    });
+  }
+}

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: {

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'
   }
 );

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