From 2c2376083b5b020974a40a8e12a176e8888cb2e4 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Fri, 8 Jun 2018 13:37:48 +0200 Subject: [PATCH 1/2] Electron security --- package.json | 2 +- src/Dapp/dapp.css | 1 - src/Dapp/dapp.js | 29 +++++++----------- src/index.electron.js | 35 ++++++++++++++++++++++ src/index.parity.ejs | 70 ++++++++++++++++++++++++------------------- src/inject.js | 22 +++++++++++--- src/preload.js | 38 +++++++++++++++++++++++ src/util/csp.js | 41 +++++++++++++++++++++++++ webpack/app.js | 4 ++- webpack/inject.js | 8 ++--- webpack/preload.js | 61 +++++++++++++++++++++++++++++++++++++ webpack/shared.js | 8 ----- 12 files changed, 251 insertions(+), 68 deletions(-) create mode 100644 src/preload.js create mode 100644 src/util/csp.js create mode 100644 webpack/preload.js diff --git a/package.json b/package.json index e3ed1b6e8..b603af2cd 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "build": "npm run build:inject && npm run build:app && npm run build:electron && npm run build:embed", "build:app": "webpack --config webpack/app", "build:electron": "webpack --config webpack/electron", - "build:inject": "webpack --config webpack/inject", + "build:inject": "webpack --config webpack/inject && webpack --config webpack/preload", "build:embed": "cross-env EMBED=1 node webpack/embed", "build:i18n": "npm run clean && npm run build && babel-node ./scripts/build-i18n.js", "ci:build": "cross-env NODE_ENV=production npm run build", diff --git a/src/Dapp/dapp.css b/src/Dapp/dapp.css index a458aa7a4..3cd0684d4 100644 --- a/src/Dapp/dapp.css +++ b/src/Dapp/dapp.css @@ -23,7 +23,6 @@ flex-grow: 1; margin: 0; padding: 0; - opacity: 0; width: 100%; z-index: 1; } diff --git a/src/Dapp/dapp.js b/src/Dapp/dapp.js index d45fda58f..262d16a6d 100644 --- a/src/Dapp/dapp.js +++ b/src/Dapp/dapp.js @@ -83,17 +83,16 @@ export default class Dapp extends Component { // Reput eventListeners when webview has finished loading dapp webview.addEventListener('did-finish-load', () => { this.setState({ loading: false }); - // Listen to IPC messages from this webview - webview.addEventListener('ipc-message', event => - this.requestsStore.receiveMessage({ - ...event.args[0], - source: event.target - })); - // Send ping message to tell dapp we're ready to listen to its ipc messages - webview.send('ping'); }); - this.onDappLoad(); + // Listen to IPC messages from this webview. More particularly, to IPC + // messages coming from the preload.js injected in this webview. + webview.addEventListener('ipc-message', event => { + this.requestsStore.receiveMessage({ + ...event.args[0], + source: event.target + }); + }); }; loadApp (id) { @@ -115,7 +114,6 @@ export default class Dapp extends Component { frameBorder={ 0 } id='dappFrame' name={ name } - onLoad={ this.onDappLoad } ref={ this.handleIframe } sandbox='allow-forms allow-popups allow-same-origin allow-scripts allow-top-navigation' scrolling='auto' @@ -135,16 +133,17 @@ export default class Dapp extends Component { posixDirName, '..', '.build', - 'inject.js' + 'preload.js' )}`; + // https://electronjs.org/docs/tutorial/security#3-enable-context-isolation-for-remote-content return ; } @@ -210,10 +209,4 @@ export default class Dapp extends Component { ? this.renderWebview(src, hash) : this.renderIframe(src, hash); } - - onDappLoad = () => { - const frame = document.getElementById('dappFrame'); - - frame.style.opacity = 1; - } } diff --git a/src/index.electron.js b/src/index.electron.js index 7c9db7ff3..d6cf2f6cc 100644 --- a/src/index.electron.js +++ b/src/index.electron.js @@ -91,6 +91,41 @@ function createWindow () { } }); + // Do not accept all kind of web permissions (camera, location...) + // https://electronjs.org/docs/tutorial/security#4-handle-session-permission-requests-from-remote-content + session.defaultSession + .setPermissionRequestHandler((webContents, permission, callback) => { + if (!webContents.getURL().startsWith('file:')) { + // Denies the permissions request for all non-file://. Currently all + // network dapps are loaded on http://127.0.0.1:8545, so they won't + // have any permissions. + return callback(false); + } + + // All others loaded on file:// (shell, builtin, local) can have those + // permissions. + return callback(true); + }); + + // Verify WebView Options Before Creation + // https://electronjs.org/docs/tutorial/security#12-verify-webview-options-before-creation + mainWindow.webContents.on('will-attach-webview', (event, webPreferences, params) => { + // Strip away inline preload scripts, ours is at preloadURL + delete webPreferences.preload; + // Verify the location of our prelaod script is legitimate (unless uiDev has been passed) + if (webPreferences.preloadURL !== encodeURI(url.format({ + pathname: path.join(__dirname, 'preload.js'), + protocol: 'file:', + slashes: true + }))) { + throw new Error(`Unknown preload.js is being injected, quitting for security reasons. ${webPreferences.preloadURL}`); + } + + // Disable Node.js integration + webPreferences.nodeIntegration = false; + webPreferences.contextIsolation = true; + }); + // Create the Application's main menu // https://github.com/electron/electron/blob/master/docs/api/menu.md#examples const template = [ diff --git a/src/index.parity.ejs b/src/index.parity.ejs index 7009ce0ee..f774c825a 100644 --- a/src/index.parity.ejs +++ b/src/index.parity.ejs @@ -1,35 +1,43 @@ - - - - - <%= htmlWebpackPlugin.options.title %> - - - -
-
Loading
-
- - + html, + body, + #container { + width: 100%; + height: 100%; + margin: 0; + padding: 0; + font-family: Sans-Serif; + } + + .loading { + text-align: center; + padding-top: 5em; + font-size: 2em; + color: #999; + } + + + + +
+
Loading
+
+ + + \ No newline at end of file diff --git a/src/inject.js b/src/inject.js index 96bc31e2b..bf38ca224 100644 --- a/src/inject.js +++ b/src/inject.js @@ -15,7 +15,6 @@ // along with Parity. If not, see . import Api from '@parity/api'; -import isElectron from 'is-electron'; import qs from 'query-string'; console.log('This inject.js has been injected by the shell.'); @@ -26,9 +25,9 @@ function initProvider () { let appId = match ? match[0] : query.appId; - const ethereum = isElectron() - ? new Api.Provider.Ipc(appId) - : new Api.Provider.PostMessage(appId); + // The dapp will use the PostMessage provider, send postMessages to + // preload.js, and preload.js will relay those messages to the shell. + const ethereum = new Api.Provider.PostMessage(appId); console.log(`Requesting API communications token for ${appId}`); @@ -69,4 +68,19 @@ if (typeof window !== 'undefined' && !window.isParity) { initParity(ethereum); console.warn('Deprecation: Dapps should only used the exposed EthereumProvider on `window.ethereum`, the use of `window.parity` and `window.web3` will be removed in future versions of this injector'); + + // Disable eval() for dapps + // https://electronjs.org/docs/tutorial/security#7-override-and-disable-eval + // + // TODO Currently Web3 Console dapp needs eval(), and is the only builtin + // that needs it, so we cannot blindly disable it as per the recommendation. + // One idea is to check here in inject.js if allowJsEval is set to true, but + // this requires more work (future PR). + // For now we simply allow eval(). All builtin dapps are trusted and can use + // eval(), and all network dapps are served on 127.0.0.1:8545, which have CSP + // that disallow eval(). So security-wise it should be enough. + // + // window.eval = global.eval = function () { // eslint-disable-line + // throw new Error(`Sorry, this app does not support window.eval().`); + // }; } diff --git a/src/preload.js b/src/preload.js new file mode 100644 index 000000000..359707fb9 --- /dev/null +++ b/src/preload.js @@ -0,0 +1,38 @@ +// Copyright 2015-2017 Parity Technologies (UK) Ltd. +// This file is part of Parity. + +// Parity is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity. If not, see . + +import fs from 'fs'; +import { ipcRenderer, remote, webFrame } from 'electron'; +import path from 'path'; + +// The following two lines is just a proxy between the webview and the renderer process. +// If we receive an IPC message from the shell, we relay it to the webview as +// postMessage. +ipcRenderer.on('PARITY_SHELL_IPC_CHANNEL', (_, data) => window.postMessage(data, '*')); +// If we receive a postMessage from the webview, we transfer it to the renderer +// as an IPC message. +window.addEventListener('message', (event) => { + ipcRenderer.sendToHost('PARITY_SHELL_IPC_CHANNEL', { data: event.data }); +}); + +// Load inject.js as a string, and inject it into the webview with executeJavaScript +fs.readFile(path.join(remote.getGlobal('dirName'), '..', '.build', 'inject.js'), 'utf8', (err, injectFile) => { + if (err) { + console.error(err); + return; + } + webFrame.executeJavaScript(injectFile); +}); diff --git a/src/util/csp.js b/src/util/csp.js new file mode 100644 index 000000000..c85f962d2 --- /dev/null +++ b/src/util/csp.js @@ -0,0 +1,41 @@ +const shared = [ + // Restrict everything to the same origin by default. + "default-src 'self';", + // Allow connecting to WS servers and HTTP(S) servers. + // We could be more restrictive and allow only RPC server URL. + 'connect-src http: https: ws: wss:;', + // Allow framing any content from HTTP(S). + // Again we could only allow embedding from RPC server URL. + // (deprecated) + "frame-src 'none';", + // Allow framing and web workers from HTTP(S). + "child-src 'self' http: https:;", + // We allow data: blob: and HTTP(s) images. + // We could get rid of wildcarding HTTP and only allow RPC server URL. + // (http required for local dapps icons) + "img-src 'self' 'unsafe-inline' file: data: blob: http: https:;", + // Allow style from data: blob: and HTTPS. + "style-src 'self' 'unsafe-inline' data: blob: https:;", + // Allow fonts from data: and HTTPS. + "font-src 'self' data: https:;", + // Same restrictions as script-src with additional + // blob: that is required for camera access (worker) + "worker-src 'self' https: blob:;", + // Disallow submitting forms from any dapps + "form-action 'none';", + // Never allow mixed content + 'block-all-mixed-content;' +]; + +// These are the CSP for the renderer process (aka the shell) +const rendererCsp = [ + ...shared, + // Allow which are objects + "object-src 'self';", + // Allow scripts + "script-src 'self';" +]; + +module.exports = { + rendererCsp +}; diff --git a/webpack/app.js b/webpack/app.js index 0d00ef2ff..97145b913 100644 --- a/webpack/app.js +++ b/webpack/app.js @@ -26,6 +26,7 @@ const CopyWebpackPlugin = require('copy-webpack-plugin'); const HtmlWebpackPlugin = require('html-webpack-plugin'); const ServiceWorkerWebpackPlugin = require('serviceworker-webpack-plugin'); +const { rendererCsp } = require('../src/util/csp'); const rulesEs6 = require('./rules/es6'); const rulesParity = require('./rules/parity'); const Shared = require('./shared'); @@ -54,7 +55,7 @@ module.exports = { cache: !isProd, devtool: isProd ? false - : isEmbed ? '#source-map' : '#eval', + : '#source-map', context: path.join(__dirname, '../src'), entry, output: { @@ -175,6 +176,7 @@ module.exports = { plugins, new HtmlWebpackPlugin({ + csp: rendererCsp.join(' '), title: 'Parity UI', filename: 'index.html', template: './index.parity.ejs', diff --git a/webpack/inject.js b/webpack/inject.js index 8d55ea576..817fcfe81 100644 --- a/webpack/inject.js +++ b/webpack/inject.js @@ -52,12 +52,12 @@ module.exports = { { test: /\.js$/, exclude: /node_modules/, - use: [ { + use: [{ loader: 'happypack/loader', options: { id: 'babel' } - } ] + }] }, { test: /\.json$/, @@ -65,12 +65,12 @@ module.exports = { }, { test: /\.html$/, - use: [ { + use: [{ loader: 'file-loader', options: { name: '[name].[ext]' } - } ] + }] } ] }, diff --git a/webpack/preload.js b/webpack/preload.js new file mode 100644 index 000000000..dbb2faf02 --- /dev/null +++ b/webpack/preload.js @@ -0,0 +1,61 @@ +// Copyright 2015-2017 Parity Technologies (UK) Ltd. +// This file is part of Parity. + +// Parity is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity. If not, see . + +const path = require('path'); + +const rulesEs6 = require('./rules/es6'); +const rulesParity = require('./rules/parity'); +const Shared = require('./shared'); + +const DEST = process.env.BUILD_DEST || '.build'; + +module.exports = { + context: path.join(__dirname, '../src'), + devtool: false, + entry: './preload.js', + output: { + path: path.join(__dirname, '../', DEST), + filename: 'preload.js' + }, + + target: 'electron-renderer', + + resolve: { + alias: {} + }, + + node: { + fs: 'empty' + }, + + module: { + rules: [ + rulesParity, + rulesEs6, + { + test: /\.js$/, + exclude: /node_modules/, + use: [{ + loader: 'happypack/loader', + options: { + id: 'babel' + } + }] + } + ] + }, + plugins: Shared.getPlugins() +}; diff --git a/webpack/shared.js b/webpack/shared.js index 11b2a38e7..d7857db54 100644 --- a/webpack/shared.js +++ b/webpack/shared.js @@ -90,14 +90,6 @@ function addProxies (app) { } })); - app.use('/parity-utils', proxy({ - target: 'http://127.0.0.1:3000', - changeOrigin: true, - pathRewrite: { - '^/parity-utils': '' - } - })); - app.use('/rpc', proxy({ target: 'http://127.0.0.1:8545', changeOrigin: true From d7116e40fb55e3be8d6da4754bfe718292231ae0 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Mon, 11 Jun 2018 11:01:09 +0200 Subject: [PATCH 2/2] Bump to 0.1.3 --- package-lock.json | 4 ++-- package.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 89615602d..d4b6e15c2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "parity-ui", - "version": "0.1.2", + "version": "0.1.3", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -20163,4 +20163,4 @@ "integrity": "sha1-KOwXzwl0PtyrBW3dixsGJizHPDA=" } } -} +} \ No newline at end of file diff --git a/package.json b/package.json index b603af2cd..f06115e34 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "parity-ui", - "version": "0.1.2", + "version": "0.1.3", "description": "The Electron app for Parity UI", "main": ".build/electron.js", "jsnext:main": ".build/electron.js", @@ -186,4 +186,4 @@ "solc": "ngotchac/solc-js", "store": "1.3.20" } -} +} \ No newline at end of file