From ba183ce6543f102ae635502a0da0ac7c923cc97a Mon Sep 17 00:00:00 2001 From: Literallie Date: Wed, 18 Oct 2017 17:10:23 +0200 Subject: [PATCH 01/14] Add basic CSP support --- app.js | 25 +++++++++++++++++++++++++ lib/config/default.js | 10 ++++++++++ 2 files changed, 35 insertions(+) diff --git a/app.js b/app.js index 62e6627..54ec6cf 100644 --- a/app.js +++ b/app.js @@ -108,6 +108,31 @@ if (config.hsts.enable) { logger.info('https://en.wikipedia.org/wiki/HTTP_Strict_Transport_Security') } +// use Content-Security-Policy to limit XSS, dangerous plugins, etc. +// https://helmetjs.github.io/docs/csp/ +if (config.csp.enable) { + var cdnDirectives = { + scriptSrc: ["https://cdnjs.cloudflare.com"], + styleSrc: ["https://cdnjs.cloudflare.com", "https://fonts.googleapis.com"], + fontSrc: ["https://cdnjs.cloudflare.com", "https://fonts.gstatic.com"] + } + var directives = {} + for (var propertyName in config.csp.directives) { + if(config.csp.directives.hasOwnProperty(propertyName)) { + var directive = config.csp.directives[propertyName] + if (config.usecdn && !!cdnDirectives[propertyName]) { + directive = directive.concat(cdnDirectives[propertyName]) + } + directives[propertyName] = directive; + } + } + app.use(helmet.contentSecurityPolicy({ + directives: directives + })) +} else { + logger.info('Content-Security-Policy is disabled. This may be a security risk.'); +} + i18n.configure({ locales: ['en', 'zh', 'fr', 'de', 'ja', 'es', 'ca', 'el', 'pt', 'it', 'tr', 'ru', 'nl', 'hr', 'pl', 'uk', 'hi', 'sv', 'eo', 'da'], cookie: 'locale', diff --git a/lib/config/default.js b/lib/config/default.js index f4c45e3..e207dfc 100644 --- a/lib/config/default.js +++ b/lib/config/default.js @@ -13,6 +13,16 @@ module.exports = { includeSubdomains: true, preload: true }, + csp: { + enable: true, + reportUri: '', + directives: { + defaultSrc: ["'self'"], + scriptSrc: ["'self'"], + styleSrc: ["'self'", "'unsafe-inline'"], + fontSrc: ["'self'"] + } + }, protocolusessl: false, usecdn: true, allowanonymous: true, From 5d2d3ec875310de07fe79ae605dfbc0f1df585c5 Mon Sep 17 00:00:00 2001 From: Literallie Date: Wed, 18 Oct 2017 17:45:57 +0200 Subject: [PATCH 02/14] CSP: Upgrade insecure requests if possible Config option; default is to only upgrade if usessl --- app.js | 5 +++++ lib/config/default.js | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app.js b/app.js index 54ec6cf..8af029e 100644 --- a/app.js +++ b/app.js @@ -126,6 +126,11 @@ if (config.csp.enable) { directives[propertyName] = directive; } } + if(config.csp.upgradeInsecureRequests === 'auto') { + directives.upgradeInsecureRequests = config.usessl === 'true' + } else { + directives.upgradeInsecureRequests = config.csp.upgradeInsecureRequests === 'true' + } app.use(helmet.contentSecurityPolicy({ directives: directives })) diff --git a/lib/config/default.js b/lib/config/default.js index e207dfc..217d11d 100644 --- a/lib/config/default.js +++ b/lib/config/default.js @@ -20,8 +20,9 @@ module.exports = { defaultSrc: ["'self'"], scriptSrc: ["'self'"], styleSrc: ["'self'", "'unsafe-inline'"], - fontSrc: ["'self'"] - } + fontSrc: ["'self'"], + }, + upgradeInsecureRequests: 'auto' }, protocolusessl: false, usecdn: true, From 080436aebb4c4681f85cc8bf5d8563832ff8dbdd Mon Sep 17 00:00:00 2001 From: Literallie Date: Wed, 18 Oct 2017 17:48:53 +0200 Subject: [PATCH 03/14] CSP: Add nonce to slide view inline JS --- app.js | 7 +++++++ lib/response.js | 3 ++- package.json | 1 + public/views/slide.ejs | 2 +- 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/app.js b/app.js index 8af029e..b78f94e 100644 --- a/app.js +++ b/app.js @@ -12,6 +12,7 @@ var session = require('express-session') var SequelizeStore = require('connect-session-sequelize')(session.Store) var fs = require('fs') var path = require('path') +var uuid = require('uuid') var morgan = require('morgan') var passportSocketIo = require('passport.socketio') @@ -108,6 +109,11 @@ if (config.hsts.enable) { logger.info('https://en.wikipedia.org/wiki/HTTP_Strict_Transport_Security') } +app.use((req, res, next) => { + res.locals.nonce = uuid.v4() + next() +}) + // use Content-Security-Policy to limit XSS, dangerous plugins, etc. // https://helmetjs.github.io/docs/csp/ if (config.csp.enable) { @@ -126,6 +132,7 @@ if (config.csp.enable) { directives[propertyName] = directive; } } + directives.scriptSrc.push(function (req, res) { return "'nonce-" + res.locals.nonce + "'" }) if(config.csp.upgradeInsecureRequests === 'auto') { directives.upgradeInsecureRequests = config.usessl === 'true' } else { diff --git a/lib/response.js b/lib/response.js index a22d1e7..287d53e 100755 --- a/lib/response.js +++ b/lib/response.js @@ -584,7 +584,8 @@ function showPublishSlide (req, res, next) { lastchangeuserprofile: note.lastchangeuser ? models.User.getProfile(note.lastchangeuser) : null, robots: meta.robots || false, // default allow robots GA: meta.GA, - disqus: meta.disqus + disqus: meta.disqus, + cspNonce: res.locals.nonce } return renderPublishSlide(data, res) }).catch(function (err) { diff --git a/package.json b/package.json index 4c8dc56..35fe4f9 100644 --- a/package.json +++ b/package.json @@ -114,6 +114,7 @@ "tedious": "^1.14.0", "to-markdown": "^3.0.3", "toobusy-js": "^0.5.1", + "uuid": "^3.1.0", "uws": "~0.14.1", "validator": "^6.2.0", "velocity-animate": "^1.4.0", diff --git a/public/views/slide.ejs b/public/views/slide.ejs index 7ff5016..c7dd989 100644 --- a/public/views/slide.ejs +++ b/public/views/slide.ejs @@ -41,7 +41,7 @@ - + <% if(useCDN) { %> diff --git a/public/views/pretty.ejs b/public/views/pretty.ejs index 80d2505..b2988e3 100644 --- a/public/views/pretty.ejs +++ b/public/views/pretty.ejs @@ -72,9 +72,7 @@ - + <% if(useCDN) { %> diff --git a/public/views/slide.ejs b/public/views/slide.ejs index c7dd989..269ce04 100644 --- a/public/views/slide.ejs +++ b/public/views/slide.ejs @@ -89,9 +89,7 @@ - + <% if(useCDN) { %> From 0cbdc852cb29bfcadf1229899938c757b03f5ed6 Mon Sep 17 00:00:00 2001 From: Literallie Date: Wed, 18 Oct 2017 22:44:16 +0200 Subject: [PATCH 05/14] CSP: Allow more content types --- lib/config/default.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/config/default.js b/lib/config/default.js index 217d11d..0b6ca26 100644 --- a/lib/config/default.js +++ b/lib/config/default.js @@ -18,9 +18,13 @@ module.exports = { reportUri: '', directives: { defaultSrc: ["'self'"], - scriptSrc: ["'self'"], - styleSrc: ["'self'", "'unsafe-inline'"], - fontSrc: ["'self'"], + scriptSrc: ["'self'", "'unsafe-eval'", "vimeo.com", "https://gist.github.com", "www.slideshare.net", "https://query.yahooapis.com", "https://*.disqus.com"], + imgSrc: ["*"], + styleSrc: ["'self'", "'unsafe-inline'", "https://assets-cdn.github.com"], + fontSrc: ["'self'", "https://public.slidesharecdn.com"], + objectSrc: ["*"], + childSrc: ["*"], + connectSrc: ["'self'", "https://links.services.disqus.com", "wss://realtime.services.disqus.com"] }, upgradeInsecureRequests: 'auto' }, From 996cb379912d5ee7b6e26f3c688ce447b4762bc4 Mon Sep 17 00:00:00 2001 From: Literallie Date: Wed, 18 Oct 2017 22:45:17 +0200 Subject: [PATCH 06/14] CSP: Workaround for ws:// protocol The spec allows wss:// for 'self', but not ws:// :( --- app.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/app.js b/app.js index 8873585..15c9e61 100644 --- a/app.js +++ b/app.js @@ -116,6 +116,15 @@ app.use((req, res, next) => { // use Content-Security-Policy to limit XSS, dangerous plugins, etc. // https://helmetjs.github.io/docs/csp/ +function getCspNonce (req, res) { + return "'nonce-" + res.locals.nonce + "'" +} + +function getCspWebSocketUrl (req, res) { + // wss: is included in 'self', but 'ws:' is not + return (req.protocol === 'http' ? 'ws:' : 'wss:') + config.serverurl.replace(/https?:/, "") +} + if (config.csp.enable) { var cdnDirectives = { scriptSrc: ['https://cdnjs.cloudflare.com', 'https://cdn.mathjax.org'], @@ -125,14 +134,15 @@ if (config.csp.enable) { var directives = {} for (var propertyName in config.csp.directives) { if (config.csp.directives.hasOwnProperty(propertyName)) { - var directive = config.csp.directives[propertyName] + var directive = [].concat(config.csp.directives[propertyName]) if (config.usecdn && !!cdnDirectives[propertyName]) { directive = directive.concat(cdnDirectives[propertyName]) } directives[propertyName] = directive } } - directives.scriptSrc.push(function (req, res) { return "'nonce-" + res.locals.nonce + "'" }) + directives.scriptSrc.push(getCspNonce) + directives.connectSrc.push(getCspWebSocketUrl) if (config.csp.upgradeInsecureRequests === 'auto') { directives.upgradeInsecureRequests = config.usessl === 'true' } else { From 5b83deb043296c23ff912a2472703c1f7faddb4b Mon Sep 17 00:00:00 2001 From: Literallie Date: Thu, 19 Oct 2017 22:48:13 +0200 Subject: [PATCH 07/14] Load js-url lib using legacy-loader Doesn't use eval, plus no window object access --- package.json | 1 + public/js/history.js | 6 ++++-- public/js/index.js | 7 +++++-- webpackBaseConfig.js | 10 +++++++--- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 35fe4f9..0d9f501 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ "js-yaml": "^3.7.0", "jsdom-nogyp": "^0.8.3", "keymaster": "^1.6.2", + "legacy-loader": "0.0.2", "list.js": "^1.5.0", "lodash": "^4.17.4", "lz-string": "1.4.4", diff --git a/public/js/history.js b/public/js/history.js index e14b80d..da82fd0 100644 --- a/public/js/history.js +++ b/public/js/history.js @@ -12,14 +12,16 @@ import { urlpath } from './lib/config' +var jsUrl = require('js-url') + window.migrateHistoryFromTempCallback = null migrateHistoryFromTemp() function migrateHistoryFromTemp () { - if (window.url('#tempid')) { + if (jsUrl('#tempid')) { $.get(`${serverurl}/temp`, { - tempid: window.url('#tempid') + tempid: jsUrl('#tempid') }) .done(data => { if (data && data.temp) { diff --git a/public/js/index.js b/public/js/index.js index b336af9..25bd1c3 100644 --- a/public/js/index.js +++ b/public/js/index.js @@ -3,6 +3,7 @@ modeType, Idle, serverurl, key, gapi, Dropbox, FilePicker ot, MediaUploader, hex2rgb, num_loaded, Visibility */ + require('../vendor/showup/showup') require('../css/index.css') @@ -21,6 +22,8 @@ import _ from 'lodash' import List from 'list.js' +var jsUrl = require('js-url') + import { checkLoginStateChanged, setloginStateChangeEvent @@ -1474,12 +1477,12 @@ $('#gistImportModalConfirm').click(function () { if (!isValidURL(gisturl)) { showMessageModal(' Import from Gist', 'Not a valid URL :(', '', '', false) } else { - var hostname = window.url('hostname', gisturl) + var hostname = jsUrl('hostname', gisturl) if (hostname !== 'gist.github.com') { showMessageModal(' Import from Gist', 'Not a valid Gist URL :(', '', '', false) } else { ui.spinner.show() - $.get('https://api.github.com/gists/' + window.url('-1', gisturl)) + $.get('https://api.github.com/gists/' + jsUrl('-1', gisturl)) .done(function (data) { if (data.files) { var contents = '' diff --git a/webpackBaseConfig.js b/webpackBaseConfig.js index 41a63e7..71a8f2e 100644 --- a/webpackBaseConfig.js +++ b/webpackBaseConfig.js @@ -190,7 +190,7 @@ module.exports = { index: [ 'babel-polyfill', 'script!jquery-ui-resizable', - 'script!js-url', + 'js-url', 'expose?filterXSS!xss', 'script!Idle.Js', 'expose?LZString!lz-string', @@ -241,7 +241,7 @@ module.exports = { 'expose?jsyaml!js-yaml', 'script!mermaid', 'expose?moment!moment', - 'script!js-url', + 'js-url', 'script!handlebars', 'expose?hljs!highlight.js', 'expose?emojify!emojify.js', @@ -374,7 +374,8 @@ module.exports = { 'bootstrap-tooltip': path.join(__dirname, 'public/vendor/bootstrap/tooltip.min.js'), 'headjs': path.join(__dirname, 'node_modules/reveal.js/lib/js/head.min.js'), 'reveal-markdown': path.join(__dirname, 'public/js/reveal-markdown.js'), - abcjs: path.join(__dirname, 'public/vendor/abcjs_basic_3.1.1-min.js') + abcjs: path.join(__dirname, 'public/vendor/abcjs_basic_3.1.1-min.js'), + 'js-url': path.join(__dirname, 'node_modules/js-url/url.js') } }, @@ -429,6 +430,9 @@ module.exports = { }, { test: /\.gif(\?v=\d+\.\d+\.\d+)?$/, loader: 'url?limit=10000&mimetype=image/gif' + }, { + test: /\/node_modules\/js-url\/url.js/, + loader: 'legacy' }] }, node: { From 91101c856c3efac53e8a4db4cc537b77370aa7df Mon Sep 17 00:00:00 2001 From: Literallie Date: Fri, 20 Oct 2017 12:31:16 +0200 Subject: [PATCH 08/14] Change CSP config format to be more intuitive --- README.md | 2 ++ app.js | 40 ++++++++++++++++++++++++++++++++++----- config.json.example | 7 +++++++ lib/config/default.js | 10 +--------- lib/config/environment.js | 3 +++ 5 files changed, 48 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 6e2e710..d71eb1c 100644 --- a/README.md +++ b/README.md @@ -158,6 +158,7 @@ Environment variables (will overwrite other server configs) | HMD_HSTS_INCLUDE_SUBDOMAINS | `true` | set to include subdomains in HSTS (default is `true`) | | HMD_HSTS_MAX_AGE | `31536000` | max duration in seconds to tell clients to keep HSTS status (default is a year) | | HMD_HSTS_PRELOAD | `true` | whether to allow preloading of the site's HSTS status (e.g. into browsers) | +| HMD_CSP_ENABLE | `true` | whether to enable Content Security Policy (directives cannot be configured with environment variables) | Application settings `config.json` --- @@ -171,6 +172,7 @@ Application settings `config.json` | alloworigin | `['localhost']` | domain name whitelist | | usessl | `true` or `false` | set to use ssl server (if true will auto turn on `protocolusessl`) | | hsts | `{"enable": "true", "maxAgeSeconds": "31536000", "includeSubdomains": "true", "preload": "true"}` | [HSTS](https://en.wikipedia.org/wiki/HTTP_Strict_Transport_Security) options to use with HTTPS (default is the example value, max age is a year) | +| csp | `{"enable": "true", "directives": {"scriptSrc": "trustwodthy-scripts.example.com"}, "upgradeInsecureRequests": "auto", "addDefaults": "true"}` | Configures [Content Security Policy](https://helmetjs.github.io/docs/csp/). Directives are directly passed to Helmet, so [their format](https://helmetjs.github.io/docs/csp/) applies. Further, some defaults are added so that the application doesn't break. To disable adding these defaults, set `addDefaults` to `false`. If `usecdn` is on, default CDN locations are allowed too. By default (`auto`), insecure (HTTP) requests are upgraded to HTTPS via CSP if `usessl` is on. To change this behaviour, set `upgradeInsecureRequests` to either `true` or `false`. | | protocolusessl | `true` or `false` | set to use ssl protocol for resources path (only applied when domain is set) | | urladdport | `true` or `false` | set to add port on callback url (port 80 or 443 won't applied) (only applied when domain is set) | | usecdn | `true` or `false` | set to use CDN resources or not (default is `true`) | diff --git a/app.js b/app.js index 15c9e61..01ecc84 100644 --- a/app.js +++ b/app.js @@ -125,7 +125,28 @@ function getCspWebSocketUrl (req, res) { return (req.protocol === 'http' ? 'ws:' : 'wss:') + config.serverurl.replace(/https?:/, "") } +function mergeWithDefaults(configured, defaultDirective, cdnDirective) { + var directive = [].concat(configured) + if (config.csp.addDefaults && defaultDirective) { + directive = directive.concat(defaultDirective) + } + if (config.usecdn && cdnDirective) { + directive = directive.concat(cdnDirective) + } + return directive +} + if (config.csp.enable) { + var defaultDirectives = { + defaultSrc: ['\'self\''], + scriptSrc: ['\'self\'', 'vimeo.com', 'https://gist.github.com', 'www.slideshare.net', 'https://query.yahooapis.com', 'https://*.disqus.com', '\'unsafe-eval\''], // TODO: Remove unsafe-eval - webpack script-loader issues + imgSrc: ['*'], + styleSrc: ['\'self\'', '\'unsafe-inline\'', 'https://assets-cdn.github.com'], // unsafe-inline is required for some libs, plus used in views + fontSrc: ['\'self\'', 'https://public.slidesharecdn.com'], + objectSrc: ['*'], // Chrome PDF viewer treats PDFs as objects :/ + childSrc: ['*'], + connectSrc: ['\'self\'', 'https://links.services.disqus.com', 'wss://realtime.services.disqus.com'] + }; var cdnDirectives = { scriptSrc: ['https://cdnjs.cloudflare.com', 'https://cdn.mathjax.org'], styleSrc: ['https://cdnjs.cloudflare.com', 'https://fonts.googleapis.com'], @@ -134,11 +155,20 @@ if (config.csp.enable) { var directives = {} for (var propertyName in config.csp.directives) { if (config.csp.directives.hasOwnProperty(propertyName)) { - var directive = [].concat(config.csp.directives[propertyName]) - if (config.usecdn && !!cdnDirectives[propertyName]) { - directive = directive.concat(cdnDirectives[propertyName]) - } - directives[propertyName] = directive + directives[propertyName] = mergeWithDefaults( + config.csp.directives[propertyName], + defaultDirectives[propertyName], + cdnDirectives[propertyName] + ) + } + } + for (var propertyName in defaultDirectives) { + if (!directives[propertyName]) { + directives[propertyName] = mergeWithDefaults( + [], + defaultDirectives[propertyName], + cdnDirectives[propertyName] + ) } } directives.scriptSrc.push(getCspNonce) diff --git a/config.json.example b/config.json.example index e2d774c..7e4394b 100644 --- a/config.json.example +++ b/config.json.example @@ -22,6 +22,13 @@ "includeSubdomains": "true", "preload": "true" }, + csp: { + "enable": "true", + "directives": { + }, + "upgradeInsecureRequests": "auto" + "addDefaults": "true" + }, "db": { "username": "", "password": "", diff --git a/lib/config/default.js b/lib/config/default.js index 0b6ca26..cfde9ae 100644 --- a/lib/config/default.js +++ b/lib/config/default.js @@ -15,17 +15,9 @@ module.exports = { }, csp: { enable: true, - reportUri: '', directives: { - defaultSrc: ["'self'"], - scriptSrc: ["'self'", "'unsafe-eval'", "vimeo.com", "https://gist.github.com", "www.slideshare.net", "https://query.yahooapis.com", "https://*.disqus.com"], - imgSrc: ["*"], - styleSrc: ["'self'", "'unsafe-inline'", "https://assets-cdn.github.com"], - fontSrc: ["'self'", "https://public.slidesharecdn.com"], - objectSrc: ["*"], - childSrc: ["*"], - connectSrc: ["'self'", "https://links.services.disqus.com", "wss://realtime.services.disqus.com"] }, + addDefaults: true, upgradeInsecureRequests: 'auto' }, protocolusessl: false, diff --git a/lib/config/environment.js b/lib/config/environment.js index 40b7e09..fa9698f 100644 --- a/lib/config/environment.js +++ b/lib/config/environment.js @@ -14,6 +14,9 @@ module.exports = { includeSubdomains: toBooleanConfig(process.env.HMD_HSTS_INCLUDE_SUBDOMAINS), preload: toBooleanConfig(process.env.HMD_HSTS_PRELOAD) }, + csp: { + enable: toBooleanConfig(process.env.HMD_CSP_ENABLE) + }, protocolusessl: toBooleanConfig(process.env.HMD_PROTOCOL_USESSL), alloworigin: process.env.HMD_ALLOW_ORIGIN ? process.env.HMD_ALLOW_ORIGIN.split(',') : undefined, usecdn: toBooleanConfig(process.env.HMD_USECDN), From d51da8c12c2446d081eaa7f32406941b09142c1c Mon Sep 17 00:00:00 2001 From: Literallie Date: Sat, 21 Oct 2017 00:46:53 +0200 Subject: [PATCH 09/14] Don't add nonce to CSP if unsafe-inline is on Browsers ignore unsafe-inline if a nonce is sent --- app.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app.js b/app.js index 01ecc84..8664707 100644 --- a/app.js +++ b/app.js @@ -171,7 +171,9 @@ if (config.csp.enable) { ) } } - directives.scriptSrc.push(getCspNonce) + if (directives.scriptSrc.indexOf('\'unsafe-inline\'') === -1) { + directives.scriptSrc.push(getCspNonce) + } directives.connectSrc.push(getCspWebSocketUrl) if (config.csp.upgradeInsecureRequests === 'auto') { directives.upgradeInsecureRequests = config.usessl === 'true' From 2b2b8d6d1daa0cd8a90459a9592d2c0dd753f8b2 Mon Sep 17 00:00:00 2001 From: Literallie Date: Sat, 21 Oct 2017 00:48:48 +0200 Subject: [PATCH 10/14] Allow any connect-src in CSP Managing these for all the integrations seems like a lot of effort --- app.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app.js b/app.js index 8664707..c382cc3 100644 --- a/app.js +++ b/app.js @@ -145,7 +145,7 @@ if (config.csp.enable) { fontSrc: ['\'self\'', 'https://public.slidesharecdn.com'], objectSrc: ['*'], // Chrome PDF viewer treats PDFs as objects :/ childSrc: ['*'], - connectSrc: ['\'self\'', 'https://links.services.disqus.com', 'wss://realtime.services.disqus.com'] + connectSrc: ['*'] }; var cdnDirectives = { scriptSrc: ['https://cdnjs.cloudflare.com', 'https://cdn.mathjax.org'], From e5f03fe1356dca679b6e19c0dc02136f8f78936a Mon Sep 17 00:00:00 2001 From: Literallie Date: Sat, 21 Oct 2017 01:25:30 +0200 Subject: [PATCH 11/14] Add dirty workaround for speakers view inline script --- app.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app.js b/app.js index c382cc3..cdabc7d 100644 --- a/app.js +++ b/app.js @@ -173,6 +173,10 @@ if (config.csp.enable) { } if (directives.scriptSrc.indexOf('\'unsafe-inline\'') === -1) { directives.scriptSrc.push(getCspNonce) + // TODO: This is the SHA-256 hash of the inline script in + // build/reveal.js/plugins/notes/notes.html . Any cleaner + // solution appreciated. + directives.scriptSrc.push('\'sha256-EtvSSxRwce5cLeFBZbvZvDrTiRoyoXbWWwvEVciM5Ag=\'') } directives.connectSrc.push(getCspWebSocketUrl) if (config.csp.upgradeInsecureRequests === 'auto') { From 04f5e3a3414abbb76841df8375598fb690323f11 Mon Sep 17 00:00:00 2001 From: Literallie Date: Sun, 22 Oct 2017 01:22:48 +0200 Subject: [PATCH 12/14] Move CSP logic to new file, Fix boolean config examples Not sure why I was quoting these in the first place --- README.md | 4 +-- app.js | 77 +++---------------------------------------- config.json.example | 10 +++--- lib/csp.js | 80 +++++++++++++++++++++++++++++++++++++++++++++ public/js/index.js | 1 - 5 files changed, 91 insertions(+), 81 deletions(-) create mode 100644 lib/csp.js diff --git a/README.md b/README.md index d71eb1c..2633b3b 100644 --- a/README.md +++ b/README.md @@ -171,8 +171,8 @@ Application settings `config.json` | port | `80` | web app port | | alloworigin | `['localhost']` | domain name whitelist | | usessl | `true` or `false` | set to use ssl server (if true will auto turn on `protocolusessl`) | -| hsts | `{"enable": "true", "maxAgeSeconds": "31536000", "includeSubdomains": "true", "preload": "true"}` | [HSTS](https://en.wikipedia.org/wiki/HTTP_Strict_Transport_Security) options to use with HTTPS (default is the example value, max age is a year) | -| csp | `{"enable": "true", "directives": {"scriptSrc": "trustwodthy-scripts.example.com"}, "upgradeInsecureRequests": "auto", "addDefaults": "true"}` | Configures [Content Security Policy](https://helmetjs.github.io/docs/csp/). Directives are directly passed to Helmet, so [their format](https://helmetjs.github.io/docs/csp/) applies. Further, some defaults are added so that the application doesn't break. To disable adding these defaults, set `addDefaults` to `false`. If `usecdn` is on, default CDN locations are allowed too. By default (`auto`), insecure (HTTP) requests are upgraded to HTTPS via CSP if `usessl` is on. To change this behaviour, set `upgradeInsecureRequests` to either `true` or `false`. | +| hsts | `{"enable": true, "maxAgeSeconds": 31536000, "includeSubdomains": true, "preload": true}` | [HSTS](https://en.wikipedia.org/wiki/HTTP_Strict_Transport_Security) options to use with HTTPS (default is the example value, max age is a year) | +| csp | `{"enable": true, "directives": {"scriptSrc": "trustworthy-scripts.example.com"}, "upgradeInsecureRequests": "auto", "addDefaults": true}` | Configures [Content Security Policy](https://helmetjs.github.io/docs/csp/). Directives are passed to Helmet - see [their documentation](https://helmetjs.github.io/docs/csp/) for more information on the format. Some defaults are added to the configured values so that the application doesn't break. To disable this behaviour, set `addDefaults` to `false`. Further, if `usecdn` is on, some CDN locations are allowed too. By default (`auto`), insecure (HTTP) requests are upgraded to HTTPS via CSP if `usessl` is on. To change this behaviour, set `upgradeInsecureRequests` to either `true` or `false`. | | protocolusessl | `true` or `false` | set to use ssl protocol for resources path (only applied when domain is set) | | urladdport | `true` or `false` | set to add port on callback url (port 80 or 443 won't applied) (only applied when domain is set) | | usecdn | `true` or `false` | set to use CDN resources or not (default is `true`) | diff --git a/app.js b/app.js index cdabc7d..055b8f4 100644 --- a/app.js +++ b/app.js @@ -12,7 +12,6 @@ var session = require('express-session') var SequelizeStore = require('connect-session-sequelize')(session.Store) var fs = require('fs') var path = require('path') -var uuid = require('uuid') var morgan = require('morgan') var passportSocketIo = require('passport.socketio') @@ -25,6 +24,7 @@ var config = require('./lib/config') var logger = require('./lib/logger') var response = require('./lib/response') var models = require('./lib/models') +var csp = require('./lib/csp') // generate front-end constants by template var constpath = path.join(__dirname, './public/js/lib/common/constant.ejs') @@ -109,83 +109,14 @@ if (config.hsts.enable) { logger.info('https://en.wikipedia.org/wiki/HTTP_Strict_Transport_Security') } -app.use((req, res, next) => { - res.locals.nonce = uuid.v4() - next() -}) +// Generate a random nonce per request, for CSP with inline scripts +app.use(csp.addNonceToLocals) // use Content-Security-Policy to limit XSS, dangerous plugins, etc. // https://helmetjs.github.io/docs/csp/ -function getCspNonce (req, res) { - return "'nonce-" + res.locals.nonce + "'" -} - -function getCspWebSocketUrl (req, res) { - // wss: is included in 'self', but 'ws:' is not - return (req.protocol === 'http' ? 'ws:' : 'wss:') + config.serverurl.replace(/https?:/, "") -} - -function mergeWithDefaults(configured, defaultDirective, cdnDirective) { - var directive = [].concat(configured) - if (config.csp.addDefaults && defaultDirective) { - directive = directive.concat(defaultDirective) - } - if (config.usecdn && cdnDirective) { - directive = directive.concat(cdnDirective) - } - return directive -} - if (config.csp.enable) { - var defaultDirectives = { - defaultSrc: ['\'self\''], - scriptSrc: ['\'self\'', 'vimeo.com', 'https://gist.github.com', 'www.slideshare.net', 'https://query.yahooapis.com', 'https://*.disqus.com', '\'unsafe-eval\''], // TODO: Remove unsafe-eval - webpack script-loader issues - imgSrc: ['*'], - styleSrc: ['\'self\'', '\'unsafe-inline\'', 'https://assets-cdn.github.com'], // unsafe-inline is required for some libs, plus used in views - fontSrc: ['\'self\'', 'https://public.slidesharecdn.com'], - objectSrc: ['*'], // Chrome PDF viewer treats PDFs as objects :/ - childSrc: ['*'], - connectSrc: ['*'] - }; - var cdnDirectives = { - scriptSrc: ['https://cdnjs.cloudflare.com', 'https://cdn.mathjax.org'], - styleSrc: ['https://cdnjs.cloudflare.com', 'https://fonts.googleapis.com'], - fontSrc: ['https://cdnjs.cloudflare.com', 'https://fonts.gstatic.com'] - } - var directives = {} - for (var propertyName in config.csp.directives) { - if (config.csp.directives.hasOwnProperty(propertyName)) { - directives[propertyName] = mergeWithDefaults( - config.csp.directives[propertyName], - defaultDirectives[propertyName], - cdnDirectives[propertyName] - ) - } - } - for (var propertyName in defaultDirectives) { - if (!directives[propertyName]) { - directives[propertyName] = mergeWithDefaults( - [], - defaultDirectives[propertyName], - cdnDirectives[propertyName] - ) - } - } - if (directives.scriptSrc.indexOf('\'unsafe-inline\'') === -1) { - directives.scriptSrc.push(getCspNonce) - // TODO: This is the SHA-256 hash of the inline script in - // build/reveal.js/plugins/notes/notes.html . Any cleaner - // solution appreciated. - directives.scriptSrc.push('\'sha256-EtvSSxRwce5cLeFBZbvZvDrTiRoyoXbWWwvEVciM5Ag=\'') - } - directives.connectSrc.push(getCspWebSocketUrl) - if (config.csp.upgradeInsecureRequests === 'auto') { - directives.upgradeInsecureRequests = config.usessl === 'true' - } else { - directives.upgradeInsecureRequests = config.csp.upgradeInsecureRequests === 'true' - } app.use(helmet.contentSecurityPolicy({ - directives: directives + directives: csp.computeDirectives() })) } else { logger.info('Content-Security-Policy is disabled. This may be a security risk.') diff --git a/config.json.example b/config.json.example index 7e4394b..9865877 100644 --- a/config.json.example +++ b/config.json.example @@ -17,17 +17,17 @@ "production": { "domain": "localhost", "hsts": { - "enable": "true", + "enable": true, "maxAgeSeconds": "31536000", - "includeSubdomains": "true", - "preload": "true" + "includeSubdomains": true, + "preload": true }, csp: { - "enable": "true", + "enable": true, "directives": { }, "upgradeInsecureRequests": "auto" - "addDefaults": "true" + "addDefaults": true }, "db": { "username": "", diff --git a/lib/csp.js b/lib/csp.js new file mode 100644 index 0000000..509bc53 --- /dev/null +++ b/lib/csp.js @@ -0,0 +1,80 @@ +var config = require('./config') +var uuid = require('uuid') + +var CspStrategy = {} + +var defaultDirectives = { + defaultSrc: ['\'self\''], + scriptSrc: ['\'self\'', 'vimeo.com', 'https://gist.github.com', 'www.slideshare.net', 'https://query.yahooapis.com', 'https://*.disqus.com', '\'unsafe-eval\''], + // ^ TODO: Remove unsafe-eval - webpack script-loader issues https://github.com/hackmdio/hackmd/issues/594 + imgSrc: ['*'], + styleSrc: ['\'self\'', '\'unsafe-inline\'', 'https://assets-cdn.github.com'], // unsafe-inline is required for some libs, plus used in views + fontSrc: ['\'self\'', 'https://public.slidesharecdn.com'], + objectSrc: ['*'], // Chrome PDF viewer treats PDFs as objects :/ + childSrc: ['*'], + connectSrc: ['*'] +} + +var cdnDirectives = { + scriptSrc: ['https://cdnjs.cloudflare.com', 'https://cdn.mathjax.org'], + styleSrc: ['https://cdnjs.cloudflare.com', 'https://fonts.googleapis.com'], + fontSrc: ['https://cdnjs.cloudflare.com', 'https://fonts.gstatic.com'] +} + +CspStrategy.computeDirectives = function () { + var directives = {} + mergeDirectives(directives, config.csp.directives) + mergeDirectivesIf(config.csp.addDefaults, directives, defaultDirectives) + mergeDirectivesIf(config.usecdn, directives, cdnDirectives) + if (!areAllInlineScriptsAllowed(directives)) { + addInlineScriptExceptions(directives) + } + addUpgradeUnsafeRequestsOptionTo(directives) + return directives +} + +function mergeDirectives (existingDirectives, newDirectives) { + for (var propertyName in newDirectives) { + var newDirective = newDirectives[propertyName] + if (newDirective) { + var existingDirective = existingDirectives[propertyName] || [] + existingDirectives[propertyName] = existingDirective.concat(newDirective) + } + } +} + +function mergeDirectivesIf (condition, existingDirectives, newDirectives) { + if (condition) { + mergeDirectives(existingDirectives, newDirectives) + } +} + +function areAllInlineScriptsAllowed (directives) { + return directives.scriptSrc.indexOf('\'unsafe-inline\'') !== -1 +} + +function addInlineScriptExceptions (directives) { + directives.scriptSrc.push(getCspNonce) + // TODO: This is the SHA-256 hash of the inline script in build/reveal.js/plugins/notes/notes.html + // Any more clean solution appreciated. + directives.scriptSrc.push('\'sha256-EtvSSxRwce5cLeFBZbvZvDrTiRoyoXbWWwvEVciM5Ag=\'') +} + +function getCspNonce (req, res) { + return "'nonce-" + res.locals.nonce + "'" +} + +function addUpgradeUnsafeRequestsOptionTo (directives) { + if (config.csp.upgradeInsecureRequests === 'auto' && config.usessl) { + directives.upgradeInsecureRequests = true + } else if (config.csp.upgradeInsecureRequests === true) { + directives.upgradeInsecureRequests = true + } +} + +CspStrategy.addNonceToLocals = function (req, res, next) { + res.locals.nonce = uuid.v4() + next() +} + +module.exports = CspStrategy diff --git a/public/js/index.js b/public/js/index.js index 25bd1c3..56522e9 100644 --- a/public/js/index.js +++ b/public/js/index.js @@ -3,7 +3,6 @@ modeType, Idle, serverurl, key, gapi, Dropbox, FilePicker ot, MediaUploader, hex2rgb, num_loaded, Visibility */ - require('../vendor/showup/showup') require('../css/index.css') From 567f26f5b9a5ffa0c28fba789ad502c54c4035a7 Mon Sep 17 00:00:00 2001 From: Literallie Date: Sun, 22 Oct 2017 02:48:24 +0200 Subject: [PATCH 13/14] Fix MathJax config not being picked up thanks standard --- public/js/mathjax-config-extra.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/js/mathjax-config-extra.js b/public/js/mathjax-config-extra.js index 54cee79..11ba59c 100644 --- a/public/js/mathjax-config-extra.js +++ b/public/js/mathjax-config-extra.js @@ -1,4 +1,4 @@ -var MathJax = { +window.MathJax = { messageStyle: 'none', skipStartupTypeset: true, tex2jax: { From 3a752fde5117e800d65e26cbe7b15d65eb5b491e Mon Sep 17 00:00:00 2001 From: Literallie Date: Thu, 2 Nov 2017 17:57:44 +0100 Subject: [PATCH 14/14] Revert "Load js-url lib using legacy-loader" Didn't work in Firefox for some reason. `[Script Loader] ReferenceError: module is not defined` This reverts commit 5b83deb043296c23ff912a2472703c1f7faddb4b. --- package.json | 1 - public/js/history.js | 6 ++---- public/js/index.js | 6 ++---- webpackBaseConfig.js | 10 +++------- 4 files changed, 7 insertions(+), 16 deletions(-) diff --git a/package.json b/package.json index 0d9f501..35fe4f9 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,6 @@ "js-yaml": "^3.7.0", "jsdom-nogyp": "^0.8.3", "keymaster": "^1.6.2", - "legacy-loader": "0.0.2", "list.js": "^1.5.0", "lodash": "^4.17.4", "lz-string": "1.4.4", diff --git a/public/js/history.js b/public/js/history.js index da82fd0..e14b80d 100644 --- a/public/js/history.js +++ b/public/js/history.js @@ -12,16 +12,14 @@ import { urlpath } from './lib/config' -var jsUrl = require('js-url') - window.migrateHistoryFromTempCallback = null migrateHistoryFromTemp() function migrateHistoryFromTemp () { - if (jsUrl('#tempid')) { + if (window.url('#tempid')) { $.get(`${serverurl}/temp`, { - tempid: jsUrl('#tempid') + tempid: window.url('#tempid') }) .done(data => { if (data && data.temp) { diff --git a/public/js/index.js b/public/js/index.js index 56522e9..b336af9 100644 --- a/public/js/index.js +++ b/public/js/index.js @@ -21,8 +21,6 @@ import _ from 'lodash' import List from 'list.js' -var jsUrl = require('js-url') - import { checkLoginStateChanged, setloginStateChangeEvent @@ -1476,12 +1474,12 @@ $('#gistImportModalConfirm').click(function () { if (!isValidURL(gisturl)) { showMessageModal(' Import from Gist', 'Not a valid URL :(', '', '', false) } else { - var hostname = jsUrl('hostname', gisturl) + var hostname = window.url('hostname', gisturl) if (hostname !== 'gist.github.com') { showMessageModal(' Import from Gist', 'Not a valid Gist URL :(', '', '', false) } else { ui.spinner.show() - $.get('https://api.github.com/gists/' + jsUrl('-1', gisturl)) + $.get('https://api.github.com/gists/' + window.url('-1', gisturl)) .done(function (data) { if (data.files) { var contents = '' diff --git a/webpackBaseConfig.js b/webpackBaseConfig.js index 71a8f2e..41a63e7 100644 --- a/webpackBaseConfig.js +++ b/webpackBaseConfig.js @@ -190,7 +190,7 @@ module.exports = { index: [ 'babel-polyfill', 'script!jquery-ui-resizable', - 'js-url', + 'script!js-url', 'expose?filterXSS!xss', 'script!Idle.Js', 'expose?LZString!lz-string', @@ -241,7 +241,7 @@ module.exports = { 'expose?jsyaml!js-yaml', 'script!mermaid', 'expose?moment!moment', - 'js-url', + 'script!js-url', 'script!handlebars', 'expose?hljs!highlight.js', 'expose?emojify!emojify.js', @@ -374,8 +374,7 @@ module.exports = { 'bootstrap-tooltip': path.join(__dirname, 'public/vendor/bootstrap/tooltip.min.js'), 'headjs': path.join(__dirname, 'node_modules/reveal.js/lib/js/head.min.js'), 'reveal-markdown': path.join(__dirname, 'public/js/reveal-markdown.js'), - abcjs: path.join(__dirname, 'public/vendor/abcjs_basic_3.1.1-min.js'), - 'js-url': path.join(__dirname, 'node_modules/js-url/url.js') + abcjs: path.join(__dirname, 'public/vendor/abcjs_basic_3.1.1-min.js') } }, @@ -430,9 +429,6 @@ module.exports = { }, { test: /\.gif(\?v=\d+\.\d+\.\d+)?$/, loader: 'url?limit=10000&mimetype=image/gif' - }, { - test: /\/node_modules\/js-url\/url.js/, - loader: 'legacy' }] }, node: {