Commit d0ae2071 authored by Luke Schoen's avatar Luke Schoen Committed by Amaury Martiny
Browse files

fix: Relates to #124. Security (#451)

* feat: Security aspects for fether-electron. See #124

* feat: Add Source Maps support

* docs: Add Source Maps guide to Readme

* feat: Add webpack-build-notifier add-on with custom Webpack config

* fix: Remove duplicate dependency

* WIP

* WIP

* review-fix: Configure CSP depending on NODE_ENV

* fix: Fix worker-src for the camera in production

* review-fix: Remove unnecessary config of source maps dependency

* Use preload script as buffer between main and renderer processes (#463)

* fix: Remove is-electron since now using preload script

* fix: Remove old preload script

* fix: Do not expose electron, remote, or require to web app

* fix: Add newline

* feat: Single Fether instance lock

* fix: Move preload to static folder so works with binary

* review-fix: Remove fix for webview since not used. Add comment incase used in future. Fix other event handling code

* review-fix: Add optional opt-in to using Webpack notifier plugin by running with NOTIFIER=true yarn start

* review-fix: Use pino.debug instead of console.log

* review-fix: Add worker-src blob to CSP in development for webcam

* review-fix: Update handling of untrusted urls and sessions and certificates

* review-fix: Convert to WSS. Move CSP into array like in Parity-JS Shell. Update CSP

* review-fix: Update CSP to avoid duplication

* review-fix: Remove from new-window event listener that which applies to additional new BrowserWindows since not applicable

* review-fix: Combined pino logs

* review-fix: Change to parsedUrl.href instead of origin. Fix trusted urls for dev

* fix: Remote https 127.0.0.1 in prod

* review-fix: Move WebpackBuildNotifier images so not in binary. Fix ico file

* fix: Remove console.logs

* refactor: Cleanup so can merge. Extract for inclusion in separate PR

* review-fix: Remove debugging notes since better in wiki

* review-fix: Remove other lines due to move to wiki

* fix comment

* review-fix: Remove setPermissionRequestHandler since not know if need. Move to https://hackmd.io/O1FA34BuSNyJoPV1Cu3L0A

* review-fix: Move CSP debugging into onHeadersReceived

* review-fix: Fix isParityRunningStatus

* review-fix: Replace parse-url with Node.js url parser

* review-fix: Remove parse-url from dependencies

* fix: Fix logic in setCertificateVerifyProc

* WIP

* review-fix: Dynamically add WS port from CLI to trusted

* review-fix: Update comments with security warnings

* merge latest from master and fix conflicts

* chore: Remove useless console.log

* misc: See commit details

* Remove --ws-origins from CLI, hard-code instead
* Remove --ws-interface from CLI, hard-code instead
* Ignore --ws-interface and --ws-origins flags in CLI
* Add hard-coded default trusted WS interface to window.bridge
* Add default WS port to window.bridge

* WIP - start implementing isDev. See FIXME for future work required

* review-fix: Use appIsPackaged instead of NODE_ENV

* fix: Add IS_PROD to constants and assign appIsPackaged to it. Expose it to frontend so no longer use NODE_ENV

* feat: Add wiki Fether FAQ to trusted urls since required by PR #482

* fix: Fix untrusted blockscout.com error in setCertificateVerifyProc

* review-fix: fix blocked image hosting and external blockscout urls

* review-fix: trust github token icons

* review-fix: Rename network to fetherNetwork so custom config avoids naming conflict

* review-fix: Remove duplicate pino.debug for CSP

* review-fix: Remove WsSecure until wss and certificates implemented

* review-fix: Update config to show Electron security warnings in all environments

* review-fix: Remove use of wsInterface

* refactor: Refactor tests inside describe blocks

* tests: Add chrome dev tools to tests for trusted urls

* review-fix: Use NODE_ENV and Electron app.isPackaged

* fix: Rebuild yarn.lock

* fix: Fix linting to arg passed to correct script

* review-fix: Remove ws-origins flag and trusted ws origins

* test: Fix failing test

* review-fix: Remove package-lock.json

* fix: Use NODE_ENV consistently instead of process.defaultApp

* fix: Change to hash instead of transactionHash for blockscout
parent 51797f9a
Pipeline #35114 passed with stages
in 13 minutes and 45 seconds
......@@ -44,8 +44,8 @@
"build": "lerna run build",
"preelectron": "yarn build",
"electron": "cd packages/fether-electron && yarn electron",
"lint-files": "./scripts/lint-files.sh",
"lint": "yarn lint-files '**/*.js'",
"lint-files": "./scripts/lint-files.sh '**/*.js'",
"lint": "yarn lint-files",
"prepackage": "yarn build",
"package": "cd packages/fether-electron && yarn package",
"release": "cd packages/fether-electron && yarn release",
......@@ -76,4 +76,4 @@
"ts-node": "^8.0.3",
"typescript": "^3.3.4000"
}
}
\ No newline at end of file
}
// https://webpack.electron.build/add-ons
// https://www.npmjs.com/package/webpack-build-notifier
const path = require('path');
const WebpackBuildNotifierPlugin = require('webpack-build-notifier');
const withWebpackBuildNotifier = process.env.NOTIFIER === 'true';
module.exports = withWebpackBuildNotifier
? {
plugins: [
new WebpackBuildNotifierPlugin({
title: 'Fether Webpack Build',
logo: path.resolve('./build/icons/icon.ico'),
suppressSuccess: false,
compileIcon: path.resolve('./build/icons/webpack/compile.png'),
failureIcon: path.resolve('./build/icons/webpack/failure.png'),
successIcon: path.resolve('./build/icons/webpack/success.png'),
warningIcon: path.resolve('./build/icons/webpack/warning.png')
})
]
}
: {};
{
"main": {
"webpackConfig": "custom.webpack.additions.js"
},
"renderer": {
"sourceDirectory": null
}
},
"title": "Fether"
}
......@@ -30,12 +30,12 @@
"scripts": {
"prebuild": "copyfiles -u 2 \"../fether-react/build/**/*\" static/ && ./scripts/fixElectronBug.sh",
"build": "electron-webpack",
"electron": "cross-env SKIP_PREFLIGHT_CHECK=true electron dist/main/main.js",
"electron": "electron dist/main/main.js",
"prepackage": "./scripts/revertElectronBug.sh",
"package": "electron-builder",
"prerelease": "./scripts/revertElectronBug.sh",
"release": "electron-builder",
"start": "cross-env ELECTRON_START_URL=http://localhost:3000 electron-webpack dev --ws-origins all",
"start": "cross-env ELECTRON_START_URL=http://localhost:3000 electron-webpack dev",
"test": "jest --all --color --coverage"
},
"dependencies": {
......@@ -48,7 +48,8 @@
"fether-react": "^0.3.0",
"pino": "^4.16.1",
"pino-multi-stream": "^3.1.2",
"source-map-support": "^0.5.10"
"source-map-support": "^0.5.10",
"url-pattern": "^1.0.3"
},
"devDependencies": {
"copyfiles": "^2.1.0",
......@@ -56,6 +57,7 @@
"electron": "^4.0.1",
"electron-builder": "^20.38.5",
"electron-webpack": "^2.6.1",
"webpack": "^4.29.1"
"webpack": "^4.29.1",
"webpack-build-notifier": "^0.1.30"
}
}
\ No newline at end of file
}
......@@ -4,7 +4,7 @@
// SPDX-License-Identifier: BSD-3-Clause
import cli from 'commander';
import { DEFAULT_CHAIN, DEFAULT_WS_PORT } from '../constants';
const { productName } = require('../../../../electron-builder.json');
const { version } = require('../../../../package.json');
......@@ -24,22 +24,17 @@ cli
.allowUnknownOption()
.option(
'--chain <chain>',
'The network to connect to, can be one of "foundation", "kovan" or "ropsten". (default: "kovan")',
'kovan'
`The network to connect to, can be one of "foundation", "kovan" or "ropsten". (default: "${DEFAULT_CHAIN}")`,
DEFAULT_CHAIN
)
.option(
'--no-run-parity',
`${productName} will not attempt to run the locally installed parity.`
)
.option(
'--ws-interface <ip>',
`Specify the hostname portion of the WebSockets server ${productName} will connect to. IP should be an interface's IP address. (default: 127.0.0.1)`,
'127.0.0.1'
)
.option(
'--ws-port <port>',
`Specify the port portion of the WebSockets server ${productName} will connect to. (default: 8546)`,
8546
`Specify the port portion of the WebSockets server ${productName} will connect to. (default: ${DEFAULT_WS_PORT})`,
DEFAULT_WS_PORT
)
.parse(
......@@ -47,7 +42,13 @@ cli
// We want to ignore some flags and not pass them down to Parity:
// --inspect: `electron-webpack dev` runs Electron with the `--inspect` flag for HMR
// -psn_*: https://github.com/paritytech/fether/issues/188
.filter(arg => !arg.startsWith('--inspect') && !arg.startsWith('-psn_'))
.filter(
arg =>
!arg.startsWith('--inspect') &&
!arg.startsWith('-psn_') &&
!arg.startsWith('--ws-interface') &&
!arg.startsWith('--ws-origins')
)
);
export default cli;
// Copyright 2015-2019 Parity Technologies (UK) Ltd.
// This file is part of Parity.
//
// SPDX-License-Identifier: BSD-3-Clause
import { IS_PACKAGED } from '../utils/paths';
const IS_PROD = process.env.NODE_ENV === 'production';
/**
* Security. Additional network security is configured after `cli` is available:
* in fether-electron/src/main/app/options/config/index.js
*
* Note: 127.0.0.1 is a trusted loopback and more trustworthy than localhost.
* See https://letsencrypt.org/docs/certificates-for-localhost/
*/
const DEFAULT_CHAIN = 'kovan';
const DEFAULT_WS_PORT = '8546';
const TRUSTED_LOOPBACK = '127.0.0.1';
export {
DEFAULT_CHAIN,
DEFAULT_WS_PORT,
IS_PACKAGED,
IS_PROD,
TRUSTED_LOOPBACK
};
......@@ -4,6 +4,7 @@
// SPDX-License-Identifier: BSD-3-Clause
import electron from 'electron';
import { IS_PROD } from '../../constants';
const { shell } = electron;
......@@ -170,7 +171,7 @@ const getContextTrayMenuTemplate = fetherApp => {
}
];
if (process.env.NODE_ENV !== 'production') {
if (!IS_PROD) {
template.push({
label: 'Reload',
click: () => fetherApp.win.webContents.reload()
......
......@@ -3,11 +3,13 @@
//
// SPDX-License-Identifier: BSD-3-Clause
import { DEFAULT_WS_PORT, TRUSTED_LOOPBACK } from '../constants';
import cli from '../cli';
function setupGlobals () {
// Globals for fether-react parityStore
global.wsInterface = cli.wsInterface;
global.defaultWsInterface = TRUSTED_LOOPBACK;
global.defaultWsPort = DEFAULT_WS_PORT;
global.wsPort = cli.wsPort;
}
......
......@@ -5,7 +5,12 @@
import electron from 'electron';
import { IS_PROD } from '../constants';
import { CSP } from '../utils/csp';
import messages from '../messages';
import Pino from '../utils/pino';
const pino = Pino();
const { ipcMain, session } = electron;
function setupRequestListeners (fetherApp) {
......@@ -30,6 +35,24 @@ function setupRequestListeners (fetherApp) {
callback({ requestHeaders: details.requestHeaders }); // eslint-disable-line
}
);
// Content Security Policy (CSP)
session.defaultSession.webRequest.onHeadersReceived((details, callback) => {
pino.debug(
`Configuring Content-Security-Policy for environment ${
IS_PROD ? 'production' : 'development'
}`
);
/* eslint-disable */
callback({
responseHeaders: {
...details.responseHeaders,
"Content-Security-Policy": [CSP]
}
});
/* eslint-enable */
});
}
export default setupRequestListeners;
......@@ -3,20 +3,71 @@
//
// SPDX-License-Identifier: BSD-3-Clause
import electron from 'electron';
import debounce from 'lodash/debounce';
import { SECURITY_OPTIONS } from '../options/config';
import Pino from '../utils/pino';
const { TRUSTED_HOSTS } = SECURITY_OPTIONS.fetherNetwork;
const trustedHostsAll = Object.values(TRUSTED_HOSTS).flat();
const pino = Pino();
function setupWinListeners (fetherApp) {
const { onWindowClose, processSaveWinPosition, win } = fetherApp;
// Open external links in browser
win.webContents.on('new-window', (event, url) => {
event.preventDefault();
electron.shell.openExternal(url);
/**
* Insecure TLS Validation - verify the application does not explicitly opt-out of TLS validation
*
* References:
* - https://doyensec.com/resources/us-17-Carettoni-Electronegativity-A-Study-Of-Electron-Security-wp.pdf
* - https://electronjs.org/docs/api/session#sessetcertificateverifyprocproc
*/
win.webContents.session.setCertificateVerifyProc((request, callback) => {
const { hostname, certificate, verificationResult, errorCode } = request; // eslint-disable-line
pino.debug(
'Processing server certificate verification request for the session in setCertificateVerifyProc with hostname: ',
hostname
);
if (errorCode) {
pino.error(
'Error processing server certificate verification request for the session in setCertificateVerifyProc: ',
errorCode
);
// Failure accepting certificate due to errorCode
callback(-2); // eslint-disable-line
} else if (!trustedHostsAll.includes(hostname)) {
pino.info(
'Failure accepting server certification due to its hostname being an untrusted host in setCertificateVerifyProc: ',
hostname
);
// Failure accepting server certificate due to its source hostname being untrusted
callback(-2); // eslint-disable-line
} else if (!verificationResult === 'net::OK') {
pino.info(
'Failure accepting server certificate due to it failing Chromium verification: ',
hostname,
verificationResult
);
// Failure accepting server certificate due to it failing Chromium verification
callback(-2); // eslint-disable-line
} else {
pino.info(
'Fallback to using the verification result from Chromium: ',
hostname,
verificationResult
);
// Fallback to using the verification result from Chromium
callback(-3); // eslint-disable-line
// // Success and accept the certifcate, disable Certificate Transparency verification
// callback(0); // eslint-disable-line
}
});
// Windows and Linux (unchecked on others)
......
......@@ -6,10 +6,61 @@
import path from 'path';
import url from 'url';
import Pino from '../../utils/pino';
import { staticPath } from '../../utils/paths';
import cli from '../../cli';
import {
DEFAULT_CHAIN,
DEFAULT_WS_PORT,
IS_PROD,
TRUSTED_LOOPBACK
} from '../../constants';
const pino = Pino();
pino.info(
`Running Fether in ${IS_PROD ? 'production' : 'development'} environment`
);
/**
* Note: If the user provides a custom CLI port to `cli.wsPort` then
* we 'dynamically' trust it in addition to the `DEFAULT_WS_PORT` in
* fether-electron/src/main/index.js, which is where we only
* permit requests from trusted paths.
*
* WARNING: DO NOT add the custom CLI interface `cli.wsInterface` as a
* trusted host. This may avoid Fether being launched with a
* malicious remote `cli.wsInterface` and sending sensitive user information
* (i.e. account password) over RPC.
* See https://github.com/paritytech/fether/pull/451#discussion_r268732256
*
* Note: We also disallows users from using Fether
* with a remote node.
* WARNING: SSH tunnels from an attacker are still possible.
*/
const DEFAULT_HTTP_PORT = '3000';
const CUSTOM_WS_PORT = cli.wsPort;
const TRUSTED_HOSTS = {
github: ['api.github.com', 'github.com', 'raw.githubusercontent.com'],
blockscout: ['blockscout.com']
};
const TRUSTED_WS_PORTS = [DEFAULT_WS_PORT, CUSTOM_WS_PORT];
const DEFAULT_HTTP_TRUSTED_LOOPBACK = `http://${TRUSTED_LOOPBACK}:${DEFAULT_HTTP_PORT}`;
const TRUSTED_URLS = [
DEFAULT_HTTP_TRUSTED_LOOPBACK,
`ws://${TRUSTED_LOOPBACK}:${DEFAULT_WS_PORT}`,
`ws://${TRUSTED_LOOPBACK}:${CUSTOM_WS_PORT}`,
'https://parity.io',
'https://wiki.parity.io/Fether-FAQ',
'https://github.com/paritytech/fether/issues/new',
'https://api.github.com/repos/paritytech/fether/releases/latest'
];
// https://electronjs.org/docs/tutorial/security#electron-security-warnings
process.env.ELECTRON_ENABLE_SECURITY_WARNINGS = true;
const INDEX_HTML_PATH =
process.env.ELECTRON_START_URL ||
(!IS_PROD && process.env.ELECTRON_START_URL) ||
url.format({
pathname: path.join(staticPath, 'build', 'index.html'),
protocol: 'file:',
......@@ -60,10 +111,6 @@ const DEFAULT_OPTIONS = {
show: false, // Run showWindow later
showDockIcon: true, // macOS usage only
tabbingIdentifier: 'parity',
webPreferences: {
devTools: true,
enableRemoteModule: true // Remote is required in fether-react parityStore.js
},
width: 360,
windowPosition: windowPosition, // Required
withTaskbar: false
......@@ -79,4 +126,87 @@ const TASKBAR_OPTIONS = {
withTaskbar: true
};
export { DEFAULT_OPTIONS, TASKBAR_OPTIONS };
const SECURITY_OPTIONS = {
/**
* Note: The keys used in this options object are passed as an argument
* to the `BrowserWindow` as defined in the Electron API documents
* https://electronjs.org/docs/api/browser-window#browserwindow.
* However, `fetherNetwork` is a custom property that is not part of
* of the API and has been added just to keep the configuration together.
* It has been given a unique name to prevent naming conflicts.
*/
fetherNetwork: {
DEFAULT_CHAIN,
DEFAULT_WS_PORT,
TRUSTED_HOSTS,
TRUSTED_LOOPBACK,
TRUSTED_URLS,
TRUSTED_WS_PORTS
},
webPreferences: {
/**
* Potential security risk options set explicitly even when default is favourable.
* Reference: https://electronjs.org/docs/tutorial/security
*/
devTools: true,
/**
* `nodeIntegration` when enabled allows the software to use Electron's APIs
* and gain access to Node.js. It must be disabled to restricting access to
* Node.js global symbols like `require` from global scope and requires the
* user to sanitise user inputs to reduce the possible XSS attack surface.
*/
nodeIntegration: false, // Must be disabled
nodeIntegrationInWorker: false, // Must be disabled
/**
* Electron security recommends us to set this to `true`. However, we need
* some communication between the main process and the renderer process
* (via ipcMain and ipcRenderer), so we need to disabled contextIsolation.
* https://stackoverflow.com/questions/55164360/with-contextisolation-true-is-it-possible-to-use-ipcrenderer
* Currently experimental and may change or be removed in future Electron releases.
*/
contextIsolation: false, // Should be enabled
/**
* Isolate access to Electron/Node.js from the Fether web app, by creating
* a bridge which plays the role of an API between main and renderer
* processes.
* https://github.com/electron/electron/issues/9920#issuecomment-336757899
*/
preload: path.join(staticPath, 'preload.js'),
/**
* Sandbox the BrowserWindow renderer associated with the window to mitigate
* against the risk of malicious preload scripts, whilst still allowing access to
* all underlying Electron/Node.js primitives using `remote` or internal IPC
* Reference: https://doyensec.com/resources/us-17-Carettoni-Electronegativity-A-Study-Of-Electron-Security-wp.pdf
*/
sandbox: true, // Do not set to false. Run electron with `electron --enable-sandbox` to sandbox all BrowserWindow instances
enableRemoteModule: true, // Remote is required in fether-react parityStore.js
// Enables same origin policy to prevent execution of insecure code. Do not set to false
webSecurity: true,
allowRunningInsecureContent: false, // Do not set to true
plugins: false,
experimentalFeatures: false, // Do not set to true
enableBlinkFeatures: '', // Do not enable any of them
nativeWindowOpen: true,
/**
* `webviewTag` when enabled allows content to be embedded into the
* Electron app and to be run as a separate process when Electron handles
* new browser windows. It is important to reduce privileges
* to try and prevent attackers from controlling the new browser windows
* with the `window.open` command and passing a WebView tag
* (see `webView`) to enable `nodeIntegration`.
*
* If any webview's https://electronjs.org/docs/api/webview-tag are implemented
* then it is important to check if it is necessary to update security by
* implementing the `''will-attach-webview'` listener
* https://electronjs.org/blog/webview-fix to intercept and prevent
* a new WebView (that may be used by an attacker to gain access to the
* file system) in addition to setting `webviewTag: false`.
*/
webviewTag: false, // Associated with `will-attach-webview`
safeDialogs: true,
safeDialogsMessage: 'Electron consecutive dialog protection was triggered',
navigateOnDragDrop: false
}
};
export { DEFAULT_OPTIONS, SECURITY_OPTIONS, TASKBAR_OPTIONS };
......@@ -3,9 +3,15 @@
//
// SPDX-License-Identifier: BSD-3-Clause
import { DEFAULT_OPTIONS, TASKBAR_OPTIONS } from './config';
import { DEFAULT_OPTIONS, SECURITY_OPTIONS, TASKBAR_OPTIONS } from './config';
export default (withTaskbar, customOptions) =>
withTaskbar
? Object.assign({}, DEFAULT_OPTIONS, TASKBAR_OPTIONS, customOptions || {})
: Object.assign({}, DEFAULT_OPTIONS, customOptions || {});
? Object.assign(
{},
DEFAULT_OPTIONS,
TASKBAR_OPTIONS,
customOptions || {},
SECURITY_OPTIONS
)
: Object.assign({}, DEFAULT_OPTIONS, customOptions || {}, SECURITY_OPTIONS);
......@@ -61,7 +61,6 @@ class ParityEthereum {
isRunning = async () => {
return isParityRunning({
wsInterface: cli.wsInterface,
wsPort: cli.wsPort
});
};
......@@ -75,8 +74,6 @@ class ParityEthereum {
'--light',
'--chain',
cli.chain,
'--ws-interface',
cli.wsInterface,
'--ws-port',
cli.wsPort
],
......
// Copyright 2015-2019 Parity Technologies (UK) Ltd.
// This file is part of Parity.
//
// SPDX-License-Identifier: BSD-3-Clause
import { IS_PROD } from '../constants';
/* eslint-disable */
// References: https://github.com/parity-js/shell
const CSP_CONFIG = {
// Disallow mixed content
blockAllMixedContent: "block-all-mixed-content;",
// Disallow framing and web workers.
childSrc: "child-src 'none';",
// FIXME - Only allow connecting to WSS and HTTPS servers.
connectSrc: "connect-src http: ws:;",
// Fallback for missing directives.
// Reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/default-src
//
// Disallow everything as fallback by default for all CSP fetch directives.
defaultSrc: "default-src 'none';",
// Disallow fonts.
fontSrc: "font-src 'none';", // Additionally used in Parity-JS Shell `'self' data: https:`
// Disallow submitting any forms
formAction: "form-action 'none';",
// Disallow framing.
frameSrc: "frame-src 'none';",
imgSrc: !IS_PROD
? // Only allow HTTPS for images. Token provider logos must be https://
// Allow `data:` `blob:`.
"img-src 'self' 'unsafe-inline' file: data: blob: https:;"
: // Only allow HTTPS for images. Token provider logos must be https://
// Allow `data:` `blob:`.
"img-src 'unsafe-inline' file: data: blob: https:;", // Additionally used in Parity-JS Shell `'self'`
// Disallow manifests.
manifestSrc: "manifest-src 'none';",
// Disallow media.
mediaSrc: "media-src 'none';",
// Disallow fonts and `<webview>` objects
objectSrc: "object-src 'none';",
// Disallow prefetching.
prefetchSrc: "prefetch-src 'none';",
scriptSrc: !IS_PROD
? // Only allow `http:` and `unsafe-eval` in dev mode (required by create-react-app)
"script-src 'self' file: http: blob: 'unsafe-inline' 'unsafe-eval';"
: "script-src file: 'unsafe-inline';",
styleSrc: !IS_PROD
? "style-src 'self' 'unsafe-inline' file: blob:;" // Additionally used in Parity-JS Shell `data: https:`
: "style-src unsafe-inline' file: blob:;", // Additionally used in Parity-JS Shell `data: https:`
// Allow `blob:` for camera access (worker)
workerSrc: "worker-src blob:;" // Additionally used in Parity-JS Shell `'self' https:`
};
/* eslint-enable */
const CSP = Object.values(CSP_CONFIG).join(' ');
export { CSP };
// Copyright 2015-2019 Parity Technologies (UK) Ltd.
// This file is part of Parity.