Merge pull request #1513 from hedgedoc/fix/csp

docs/content/configuration.md

@@ -91,6 +91,8 @@ these are rarely used for various reasons.
 | `csp.addGoogleAnalytics`      | `CMD_CSP_ADD_GOOGLE_ANALYTICS` | **`false`** or `true`                                                                     | Enable to allow users to add Google Analytics to their notes. We don't recommend enabling this option, as it increases the attack surface of XSS attacks.                 |
 | `csp.upgradeInsecureRequests` |                                | **`auto`** or `true` or `false`                                                           | By default (`auto`), insecure (HTTP) requests are upgraded to HTTPS via CSP if `useSSL` is on. To change this behaviour, set to either `true` or `false`.                 |
 | `csp.reportUri`               | `CMD_CSP_REPORTURI`            | **`undefined`**, `https://<someid>.report-uri.com/r/d/csp/enforce`                        | Allows to add a URL for CSP reports in case of violations.                                                                                                                |
+| `csp.allowFraming`            | `CMD_CSP_ALLOW_FRAMING`        | **`true`** or `false`                                                                     | Disable to disallow embedding of the instance via iframe. We **strongly recommend disabling** this option, as it increases the attack surface of XSS attacks.             |
+| `csp.allowPDFEmbed`           | `CMD_CSP_ALLOW_PDF_EMBED`      | **`true`** or `false`                                                                     | Disable to disallow embedding PDFs. We recommend disabling this option, as it increases the attack surface of XSS attacks.                                                |
 | `cookiePolicy`                | `CMD_COOKIE_POLICY`            | **`lax`**, `strict` or `none`                                                             | Set a SameSite policy whether cookies are send from cross-origin. Be careful: setting a SameSite value of none without https breaks the editor.                           | 
 
 ## Privacy and External Requests

lib/config/default.js

@@ -25,7 +25,9 @@ module.exports = {
     addDisqus: false,
     addGoogleAnalytics: false,
     upgradeInsecureRequests: 'auto',
-    reportURI: undefined
+    reportURI: undefined,
+    allowFraming: true,
+    allowPDFEmbed: true
   },
   cookiePolicy: 'lax',
   protocolUseSSL: false,

lib/config/environment.js

@@ -22,7 +22,9 @@ module.exports = {
     enable: toBooleanConfig(process.env.CMD_CSP_ENABLE),
     reportURI: process.env.CMD_CSP_REPORTURI,
     addDisqus: toBooleanConfig(process.env.CMD_CSP_ADD_DISQUS),
-    addGoogleAnalytics: toBooleanConfig(process.env.CMD_CSP_ADD_GOOGLE_ANALYTICS)
+    addGoogleAnalytics: toBooleanConfig(process.env.CMD_CSP_ADD_GOOGLE_ANALYTICS),
+    allowFraming: toBooleanConfig(process.env.CMD_CSP_ALLOW_FRAMING),
+    allowPDFEmbed: toBooleanConfig(process.env.CMD_CSP_ALLOW_PDF_EMBED)
   },
   cookiePolicy: process.env.CMD_COOKIE_POLICY,
   protocolUseSSL: toBooleanConfig(process.env.CMD_PROTOCOL_USESSL),

lib/csp.js

@@ -4,15 +4,26 @@ const { v4: uuidv4 } = require('uuid')
 const CspStrategy = {}
 
 const defaultDirectives = {
-  defaultSrc: ['\'self\''],
-  scriptSrc: ['\'self\'', 'vimeo.com', 'https://gist.github.com', 'www.slideshare.net'],
-  imgSrc: ['*'],
-  styleSrc: ['\'self\'', '\'unsafe-inline\'', 'https://github.githubassets.com'], // unsafe-inline is required for some libs, plus used in views
-  fontSrc: ['\'self\'', 'data:', 'https://public.slidesharecdn.com'],
+  defaultSrc: ['\'none\''],
+  baseUri: ['\'self\''],
+  connectSrc: ['\'self\''],
+  fontSrc: ['\'self\''],
+  manifestSrc: ['\'self\''],
+  frameSrc: ['\'self\'', 'https://player.vimeo.com', 'https://www.slideshare.net/slideshow/embed_code/key/', 'https://www.youtube.com'],
+  imgSrc: ['*'], // we allow using arbitrary images
+  scriptSrc: [
+    config.serverURL + '/build/',
+    config.serverURL + '/js/',
+    config.serverURL + '/config',
+    'https://gist.github.com/',
+    'https://vimeo.com/api/oembed.json',
+    'https://www.slideshare.net/api/oembed/2',
+    '\'unsafe-inline\'' // this is ignored by browsers supporting nonces/hashes
+  ],
+  styleSrc: [config.serverURL + '/build/', config.serverURL + '/css/', '\'unsafe-inline\'', 'https://github.githubassets.com'], // unsafe-inline is required for some libs, plus used in views
   objectSrc: ['*'], // Chrome PDF viewer treats PDFs as objects :/
-  mediaSrc: ['*'],
-  childSrc: ['*'],
-  connectSrc: ['*']
+  formAction: ['\'self\''],
+  mediaSrc: ['*']
 }
 
 const cdnDirectives = {
@@ -35,6 +46,15 @@ const dropboxDirectives = {
   scriptSrc: ['https://www.dropbox.com', '\'unsafe-inline\'']
 }
 
+const disallowFramingDirectives = {
+  frameAncestors: ['\'self\'']
+}
+
+const allowPDFEmbedDirectives = {
+  objectSrc: ['*'], // Chrome and Firefox treat PDFs as objects
+  frameSrc: ['*'] // Chrome also checks PDFs against frame-src
+}
+
 CspStrategy.computeDirectives = function () {
   const directives = {}
   mergeDirectives(directives, config.csp.directives)
@@ -43,9 +63,9 @@ CspStrategy.computeDirectives = function () {
   mergeDirectivesIf(config.csp.addDisqus, directives, disqusDirectives)
   mergeDirectivesIf(config.csp.addGoogleAnalytics, directives, googleAnalyticsDirectives)
   mergeDirectivesIf(config.dropbox.appKey, directives, dropboxDirectives)
-  if (!areAllInlineScriptsAllowed(directives)) {
-    addInlineScriptExceptions(directives)
-  }
+  mergeDirectivesIf(!config.csp.allowFraming, directives, disallowFramingDirectives)
+  mergeDirectivesIf(config.csp.allowPDFEmbed, directives, allowPDFEmbedDirectives)
+  addInlineScriptExceptions(directives)
   addUpgradeUnsafeRequestsOptionTo(directives)
   addReportURI(directives)
   return directives
@@ -67,10 +87,6 @@ function mergeDirectivesIf (condition, 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
@@ -79,11 +95,11 @@ function addInlineScriptExceptions (directives) {
 }
 
 function getCspNonce (req, res) {
-  return "'nonce-" + res.locals.nonce + "'"
+  return '\'nonce-' + res.locals.nonce + '\''
 }
 
 function addUpgradeUnsafeRequestsOptionTo (directives) {
-  if (config.csp.upgradeInsecureRequests === 'auto' && config.useSSL) {
+  if (config.csp.upgradeInsecureRequests === 'auto' && (config.useSSL || config.protocolUseSSL)) {
     directives.upgradeInsecureRequests = []
   } else if (config.csp.upgradeInsecureRequests === true) {
     directives.upgradeInsecureRequests = []

public/docs/release-notes.md

@@ -8,6 +8,11 @@
 
 ### Features
 - HedgeDoc now automatically retries connecting to the database up to 30 times on startup.
+- This release introduces the `csp.allowFraming` config option, which controls whether embedding a HedgeDoc instance
+  in other webpages is allowed. We **strongly recommend disabling** this option to reduce the risk of XSS attacks.
+- This release introduces the `csp.allowPDFEmbed` config option, which controls whether embedding PDFs inside HedgeDoc
+  notes is allowed. We recommend disabling this option if you don't use the feature, to reduce the attack surface of
+  XSS attacks.
 
 ### Bugfixes
 - Fix crash when trying to read the current Git commit on startup 

public/js/extra.js

@@ -284,12 +284,12 @@ export function finishView (view) {
   // youtube
   view.find('div.youtube.raw').removeClass('raw')
     .click(function () {
-      imgPlayiframe(this, '//www.youtube.com/embed/')
+      imgPlayiframe(this, 'https://www.youtube.com/embed/')
     })
     // vimeo
   view.find('div.vimeo.raw').removeClass('raw')
     .click(function () {
-      imgPlayiframe(this, '//player.vimeo.com/video/')
+      imgPlayiframe(this, 'https://player.vimeo.com/video/')
     })
     .each((key, value) => {
       const vimeoLink = `https://vimeo.com/${$(value).attr('data-videoid')}`
@@ -453,7 +453,7 @@ export function finishView (view) {
     .each((key, value) => {
       $.ajax({
         type: 'GET',
-        url: `//www.slideshare.net/api/oembed/2?url=http://www.slideshare.net/${$(value).attr('data-slideshareid')}&format=json`,
+        url: `https://www.slideshare.net/api/oembed/2?url=https://www.slideshare.net/${$(value).attr('data-slideshareid')}&format=json`,
         jsonp: 'callback',
         dataType: 'jsonp',
         success (data) {
@@ -1118,7 +1118,7 @@ const youtubePlugin = new Plugin(
     if (!videoid) return
     const div = $('<div class="youtube raw"></div>')
     div.attr('data-videoid', videoid)
-    const thumbnailSrc = `//img.youtube.com/vi/${videoid}/hqdefault.jpg`
+    const thumbnailSrc = `https://img.youtube.com/vi/${videoid}/hqdefault.jpg`
     const image = `<img src="${thumbnailSrc}" />`
     div.append(image)
     const icon = '<i class="icon fa fa-youtube-play fa-5x"></i>'

test/csp.js

@@ -144,7 +144,7 @@ describe('Content security policies', function () {
     const variations = ['default', 'script', 'img', 'style', 'font', 'object', 'media', 'child', 'connect']
 
     for (let i = 0; i < variations.length; i++) {
-      assert.strictEqual(csp.computeDirectives()[variations[i] + 'Src'].toString(), ['https://' + variations[i] + '.example.com'].concat(unextendedCSP[variations[i] + 'Src']).toString())
+      assert.strictEqual(csp.computeDirectives()[variations[i] + 'Src'].toString(), ['https://' + variations[i] + '.example.com'].concat(unextendedCSP[variations[i] + 'Src']).filter(x => x != null).toString())
     }
   })