Implement Content Security Policy (CSP) #1190

Nutomic · 2018-10-03T22:52:15Z

This would improve the security, and prevent XSS attacks. Here's a good overview:

https://www.html5rocks.com/en/tutorials/security/content-security-policy/

Also check out these links:
https://infosec.mozilla.org/guidelines/web_security#content-security-policy
https://observatory.mozilla.org/analyze.html?host=peertube.social

rigelk · 2018-10-04T11:57:23Z

I already had a try at it some time ago, but it introduced errors at that time so we rolled it back: 4bdd947 😓

rigelk · 2018-10-04T17:11:37Z

I tried https://addons.mozilla.org/en-US/firefox/addon/laboratory-by-mozilla/ and it is really interesting to see how the CSP evolves as you record a session on a PeerTube instance. Kudos to that neat extension.

Unsurprisingly, script-src and style-src are bound to use 'self' 'unsafe-inline' which is far from ideal, but which we can't do without.

What is interesting is the connect-src. Since PeerTube's client loads the video from the origin instance, the CSP should hold all intances's URLs the instance knows about (even generating it per watch page sounds overly complex). That's unrealistic of course, so for now we should use 'connect-src': *.

What do you think about the following?

contentSecurityPolicy: {
  directives: {
    defaultSrc: ['*'],
    mediaSrc: ["'self'"],
    fontSrc: ["'self' data:"],
    imgSrc: ["'self' data:"],
    scriptSrc: ["'self' 'unsafe-inline'"],
    styleSrc: ["'self' 'unsafe-inline'"]
  }
}

Nutomic · 2018-10-04T18:16:08Z

To be honest I have no idea how CSP works, other than the article I linked above. But it's probably better to have a basic policy than having nothing. And you should also be able to limit child-src and some others.

rigelk · 2018-10-04T23:54:49Z

// Security middleware
app.use(helmet({
  frameguard: {
    action: 'deny' // we only allow it for /videos/embed, see server/controllers/client.ts
  },
  hsts: false,
  contentSecurityPolicy: {
    directives: {
      defaultSrc: ['*'], // by default, not specifying default-src = '*'
      mediaSrc: ["'self'"],
      fontSrc: ["'self' data:"],
      imgSrc: ["'self' data:"],
      scriptSrc: ["'self' 'unsafe-inline'"],
      styleSrc: ["'self' 'unsafe-inline'"],
      objectSrc: ["'none'"],
      pluginTypes: ["'none'"],
      manifestSrc: ["'self'"],
      frameSrc: ["'none'"], // instead of deprecated child-src
      workerSrc: ["'self'"], // instead of deprecated child-src
      upgradeInsecureRequests: true
    },
    browserSniff: false // assumes a modern browser, but allows CDN in front
  },
  referrerPolicy: {
    policy: 'strict-origin-when-cross-origin'
  }
}))

That should be a correct configuration of the CSP (and bonus referrerPolicy to test too) - but would need to be tested on a live instance first.

Nutomic · 2018-10-05T00:25:46Z

How about merging this for the next release with Content-Security-Policy-Report-Only, and logging the violations somewhere?

rigelk · 2018-10-05T00:30:15Z

I don't think testing through releases is a good process… and we've frozen merges (except bugfixes) until the next release anyway.

Nutomic · 2018-10-05T00:48:09Z

I could test it on my instance, but I'm not sure how to compile Peertube for development (without breaking anything). And it would still need some way to log the violations on the server.

rigelk · 2018-10-05T00:56:06Z

Please don't test it on your instance, that's too hazardous, especially with an instance that big.

I'll check how to create a reporting endpoint.

Nutomic · 2018-10-05T01:08:41Z

It should be fine to test with Content-Security-Policy-Report-Only, as that only reports violations, but doesn't enforce the policy (it's in the first link if you haven't read it).

rigelk · 2018-10-05T01:17:59Z

You could use https://gist.github.com/rigelk/54b798bf8098fde431330b3868db133f with https://github.com/mozilla/csp-logger in standalone mode. Since we're only modifying ./server.ts, rebuilding the server can be done with npm run build:server.

ghost · 2018-10-05T11:22:43Z

@rigelk you don't need a reporting endpoint.
A browser respecting CSP will log the error in the developer console.

using chrome/chromium, you also will get nice error messages.
The messages in firefox aren't that helpful.

Nutomic · 2018-10-07T01:15:53Z

@rigelk is that code ignored in debug mode? I'm trying to extract the plain header and set it via Nginx on the server, because that's easier (and imo less risky) than recompiling.

@BO41 that means users would actually have to report the errors.

rigelk · 2018-10-07T10:18:24Z

@Nutomic by debug mode, your mean developing it locally on your computer with npm run dev? The code is not ignored in that case, it is just only applied to the API endpoint: localhost:9000. (it's because we serve the client through angular directly, since it's able to do hot reload and more)

Nutomic · 2018-10-07T20:17:34Z

@rigelk ah okay, I was only checking localhost:3000. I have it configured now, and it already reported an error:

[2018-10-07 22:15:27.309] [WARN] csp - violation of [script-src] from [default-src *; media-src 'self'; font-src 'self' data:; img-src 'self' data:; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; object-src 'none'; plugin-types 'none'; manifest-src 'self'; frame-src 'none'; worker-src 'self'; upgrade-insecure-requests; report-uri /csp-reporting;]
  on https://peertube.social/video-channels/1a799cd3-7a4a-4b63-83e3-aad1029daa34/videos by eval line:1
  agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36
  referrer: https://peertube.social/videos/watch/b757a40a-d1b3-4d8a-b6c8-abd03a3a60f2

rigelk · 2018-10-07T20:38:30Z

Ah right, Angular uses eval, so script-src must be set to 'unsafe-eval'. It kind of defeats the purposes of CSPs for scripts, but that's the best we can do for now.

Nutomic · 2018-10-07T20:51:49Z

Okay that seems to fix it, no more errors so far after clicking around a bit. I hope there's some way to get rid of the eval usage. Here's a relevant issue, apparently the eval is solved already in Angular:

angular/angular#19142
angular/angular-cli#3430
angular/angular-cli#6872

rigelk · 2018-10-07T21:16:18Z

As said in the last issue you linked, using AOT and removing reflect-metadata allows to get rid of unsafe-eval. Maybe we could remove it in production builds, like suggested in angular/angular-cli#6325 ? ping @Chocobozzz

rigelk · 2018-10-10T22:21:09Z

Meanwhile, I have tested live the CSP configuration I previously suggested and completed it to achieve the following:

  contentSecurityPolicy: {
    directives: {
      defaultSrc: ["'none'"], // by default, not specifying default-src = '*'
      connectSrc: ['*'],
      mediaSrc: ["'self'"],
      fontSrc: ["'self' data:"],
      imgSrc: ["'self' data:"],
      scriptSrc: ["'self' 'unsafe-inline' 'unsafe-eval'"],
      styleSrc: ["'self' 'unsafe-inline'"],
      objectSrc: ["'none'"],
      formAction: ["'self'"],
      frameAncestors: ["'*'"],
      baseUri: ["'none'"],
      pluginTypes: ["'none'"],
      manifestSrc: ["'self'"],
      frameSrc: ["'self'"], // instead of deprecated child-src / self because of test-embed
      workerSrc: ["'self'"], // instead of deprecated child-src
      upgradeInsecureRequests: true
    },
    browserSniff: false // assumes a modern browser, but allows CDN in front
  },

Hopefully we're close to a good first implementation :)

Nutomic · 2018-10-10T22:46:29Z

@rigelk Can you paste the raw header please?

rigelk · 2018-10-11T08:29:53Z

@Nutomic Content-Security-Policy: default-src 'none'; connect-src *; media-src 'self'; font-src 'self' data:; img-src 'self' data:; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; object-src 'none'; form-action 'self'; frame-ancestors '*'; base-uri 'none'; plugin-types 'none'; manifest-src 'self'; frame-src 'self'; worker-src 'self'; upgrade-insecure-requests

Nutomic · 2018-10-11T18:23:48Z

should be frame-ancestors * instead of frame-ancestors '*' (without quotes)
should be base-uri 'self' instead of base-uri 'none'
Chromium shows the error Invalid plugin type in 'plugin-types' Content Security Policy directive: ''none''. Did you mean to set the object-src directive to 'none'? Apparently this is because object-src should only be set if some plugins are allowed, according to this thread.

Edit: I guess we also need to set media-src *:

[2018-10-11 21:59:07.631] [WARN] csp - violation of [media-src] from [default-src 'none'; connect-src *; media-src 'self'; font-src 'self' data:; img-src 'self' data:; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; object-src 'none'; form-action 'self'; frame-ancestors *; base-uri 'self'; ; manifest-src 'self'; frame-src 'self'; worker-src 'self'; upgrade-insecure-requests; report-uri /csp-reporting;]
  on https://peertube.social/videos/watch/7ae45cdb-a2fa-4544-8dfc-77a0b299b58b by blob
  agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.102 Safari/537.36 Vivaldi/2.0.1309.37

Edit2: Weird, it's still reporting the same error with media-src *, but apparently only in Apple browsers:

[2018-10-11 22:33:57.447] [WARN] csp - violation of [media-src] from [default-src 'none'; connect-src *; media-src *; font-src 'self' data:; img-src 'self' data:; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; object-src 'none'; form-action 'self'; frame-ancestors *; base-uri 'self'; ; manifest-src 'self'; frame-src 'self'; worker-src 'self'; upgrade-insecure-requests; report-uri /csp-reporting;]
  on https://peertube.social/videos/watch/7ae45cdb-a2fa-4544-8dfc-77a0b299b58b by blob
  agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.102 Safari/537.36 Vivaldi/2.0.1309.37

And another violation for embeds:

[2018-10-11 22:27:53.615] [WARN] csp - violation of [img-src] from [default-src 'none'; connect-src *; media-src *; font-src 'self' data:; img-src 'self' data:; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; object-src 'none'; form-action 'self'; frame-ancestors *; base-uri 'self'; ; manifest-src 'self'; frame-src 'self'; worker-src 'self'; upgrade-insecure-requests; report-uri /csp-reporting;]
  on https://peertube.social/videos/embed/ab8cc45f-8fcf-414e-9492-e5ebfd8e592d by android-webview-video-poster line:1
  agent: Mozilla/5.0 (Linux; Android 8.1.0; Nokia 7 plus Build/OPR1.170623.026; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/69.0.3497.100 Mobile Safari/537.36

rigelk · 2018-10-11T22:22:28Z

Great! Thanks for the feedback @Nutomic - the plugin-src was confusing as it doesn't yield any error in Chromium :/

Let me try to improve that tomorrow.

rigelk · 2018-10-12T10:33:12Z

With the fixes suggested by @Nutomic, I realised we're using frame-ancestors: frame-ancestors: * and it overrides X-Frame-Options: DENY on browsers that understand this CSP 2 rule.

Right now we're using X-Frame-Options: DENY on all routes except the embed route. We should replicate the behaviour with the CSP.

EDIT: it's a bit complicated, so I suggest we make a first version without frame-ancestors.

rigelk · 2018-10-12T11:50:32Z

Finally I managed to replicate it 🙂

rigelk · 2018-10-13T19:10:14Z

I also managed to remove unsafe-inline as I said in #1190 (comment)

Chocobozzz added Type: Feature Request ✨ Component: Security labels Oct 4, 2018

rigelk mentioned this issue Oct 12, 2018

add Content Security Policy #1252

Merged

eranshmil mentioned this issue Nov 9, 2018

Content Security Policy - EvalError due to new Function() call ngxs/store#516

Closed

Chocobozzz closed this as completed in #1252 Dec 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Content Security Policy (CSP) #1190

Implement Content Security Policy (CSP) #1190

Nutomic commented Oct 3, 2018

rigelk commented Oct 4, 2018

rigelk commented Oct 4, 2018

Nutomic commented Oct 4, 2018

rigelk commented Oct 4, 2018

Nutomic commented Oct 5, 2018 •

edited

rigelk commented Oct 5, 2018 •

edited

Nutomic commented Oct 5, 2018

rigelk commented Oct 5, 2018

Nutomic commented Oct 5, 2018

rigelk commented Oct 5, 2018 •

edited

ghost commented Oct 5, 2018

Nutomic commented Oct 7, 2018

rigelk commented Oct 7, 2018 •

edited

Nutomic commented Oct 7, 2018

rigelk commented Oct 7, 2018

Nutomic commented Oct 7, 2018

rigelk commented Oct 7, 2018 •

edited

rigelk commented Oct 10, 2018

Nutomic commented Oct 10, 2018

rigelk commented Oct 11, 2018

Nutomic commented Oct 11, 2018 •

edited

rigelk commented Oct 11, 2018

rigelk commented Oct 12, 2018 •

edited

rigelk commented Oct 12, 2018

rigelk commented Oct 13, 2018

Implement Content Security Policy (CSP) #1190

Implement Content Security Policy (CSP) #1190

Comments

Nutomic commented Oct 3, 2018

rigelk commented Oct 4, 2018

rigelk commented Oct 4, 2018

Nutomic commented Oct 4, 2018

rigelk commented Oct 4, 2018

Nutomic commented Oct 5, 2018 • edited

rigelk commented Oct 5, 2018 • edited

Nutomic commented Oct 5, 2018

rigelk commented Oct 5, 2018

Nutomic commented Oct 5, 2018

rigelk commented Oct 5, 2018 • edited

ghost commented Oct 5, 2018

Nutomic commented Oct 7, 2018

rigelk commented Oct 7, 2018 • edited

Nutomic commented Oct 7, 2018

rigelk commented Oct 7, 2018

Nutomic commented Oct 7, 2018

rigelk commented Oct 7, 2018 • edited

rigelk commented Oct 10, 2018

Nutomic commented Oct 10, 2018

rigelk commented Oct 11, 2018

Nutomic commented Oct 11, 2018 • edited

rigelk commented Oct 11, 2018

rigelk commented Oct 12, 2018 • edited

rigelk commented Oct 12, 2018

rigelk commented Oct 13, 2018

Nutomic commented Oct 5, 2018 •

edited

rigelk commented Oct 5, 2018 •

edited

rigelk commented Oct 5, 2018 •

edited

rigelk commented Oct 7, 2018 •

edited

rigelk commented Oct 7, 2018 •

edited

Nutomic commented Oct 11, 2018 •

edited

rigelk commented Oct 12, 2018 •

edited