Skip to content
This repository has been archived by the owner on Feb 26, 2020. It is now read-only.

Implement Electron security guidelines #129

Merged
merged 13 commits into from
Jun 8, 2018
35 changes: 35 additions & 0 deletions electron/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,41 @@ function createWindow () {
callback({ requestHeaders: details.requestHeaders });
});

// 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
})) && !cli.uiDev) {
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;
});

mainWindow.on('closed', () => {
mainWindow = null;
});
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,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",
Expand Down
1 change: 0 additions & 1 deletion src/Dapp/dapp.css
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
flex-grow: 1;
margin: 0;
padding: 0;
opacity: 0;
width: 100%;
z-index: 1;
}
Expand Down
29 changes: 11 additions & 18 deletions src/Dapp/dapp.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,17 +93,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) {
Expand All @@ -125,7 +124,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'
Expand All @@ -136,16 +134,17 @@ export default class Dapp extends Component {
renderWebview = (src, hash) => {
const preload = `file://${path.join(
getBuildPath(),
'inject.js'
'preload.js'
)}`;

// https://electronjs.org/docs/tutorial/security#3-enable-context-isolation-for-remote-content
return <webview
className={ styles.frame }
id='dappFrame'
nodeintegration='false'
preload={ preload }
ref={ this.handleWebview }
src={ `${src}${hash}` }
webpreferences='contextIsolation'
/>;
}

Expand Down Expand Up @@ -211,10 +210,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;
}
}
70 changes: 39 additions & 31 deletions src/index.parity.ejs
Original file line number Diff line number Diff line change
@@ -1,35 +1,43 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<title><%= htmlWebpackPlugin.options.title %></title>
<style>
html {
background: white;
background-repeat: round;
}

html, body, #container {
width: 100%;
height: 100%;
margin: 0;
padding: 0;
font-family: Sans-Serif;
}
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta http-equiv="Content-Security-Policy" content="<%= htmlWebpackPlugin.options.csp %>">
<title>
<%= htmlWebpackPlugin.options.title %>
</title>
<style>
html {
background: white;
background-repeat: round;
}

.loading {
text-align: center;
padding-top: 5em;
font-size: 2em;
color: #999;
}
</style>
</head>
<body>
<div id="container">
<div class="loading">Loading</div>
</div>
</body>
</html>
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;
}
</style>
</head>

<body>
<div id="container">
<div class="loading">Loading</div>
</div>
</body>

</html>
24 changes: 18 additions & 6 deletions src/inject.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,8 @@
// along with Parity. If not, see <http://www.gnu.org/licenses/>.

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

function getAppId () {
// Dapps built into the shell; URL: file://path-to-shell/.build/dapps/0x0587.../index.html
// Dapps installed from the registry and served by Parity; URL: http://127.0.0.1:8545/ff19...
Expand All @@ -43,9 +40,9 @@ function getAppId () {
function initProvider () {
const appId = getAppId();

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

Expand Down Expand Up @@ -86,4 +83,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().`);
// };
}
38 changes: 38 additions & 0 deletions src/preload.js
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/>.

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) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use getBuildPath() from src/util/host.js here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately not, I was getting a window.require is not a function.

Reason is: in preload.js, isElectron() is true, however it's not a renderer process so you would do require('electron') instead of window.require('electron')

if (err) {
console.error(err);
return;
}
webFrame.executeJavaScript(injectFile);
});
41 changes: 41 additions & 0 deletions src/util/csp.js
Original file line number Diff line number Diff line change
@@ -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 <webview> which are objects
"object-src 'self';",
// Allow scripts
"script-src 'self';"
];

module.exports = {
rendererCsp
};
4 changes: 3 additions & 1 deletion webpack/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -52,7 +53,7 @@ module.exports = {
cache: !isProd,
devtool: isProd
? false
: isEmbed ? '#source-map' : '#eval',
: '#source-map',
context: path.join(__dirname, '../src'),
entry,
output: {
Expand Down Expand Up @@ -173,6 +174,7 @@ module.exports = {
plugins,

new HtmlWebpackPlugin({
csp: rendererCsp.join(' '),
title: 'Parity UI',
filename: 'index.html',
template: './index.parity.ejs',
Expand Down
8 changes: 4 additions & 4 deletions webpack/inject.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,25 +52,25 @@ module.exports = {
{
test: /\.js$/,
exclude: /node_modules/,
use: [ {
use: [{
loader: 'happypack/loader',
options: {
id: 'babel'
}
} ]
}]
},
{
test: /\.json$/,
use: ['json-loader']
},
{
test: /\.html$/,
use: [ {
use: [{
loader: 'file-loader',
options: {
name: '[name].[ext]'
}
} ]
}]
}
]
},
Expand Down
Loading