Add content-security-policy

[zealousidealroll]

Related to #167

[bluetech]

1. Isn't this missing style-src 'self' cdnjs.cloudflare.com? Looking at files header.hbs and rustdoc.hbs.

2. It would be more secure to drop cdnjs.cloudflare.com from the CSP, and instead use hashes (static) or nonces (dynamic) as described here - note, it's not supported in CSP1 (IE11). This way it is only allowed to use the specified files and not every file in the CDN. An alternative of course is to host the files directly, not through CDN.

[zealousidealroll]

- Isn't this missing style-src 'self' cdnjs.cloudflare.com? Looking at files header.hbs and rustdoc.hbs.

Yeah. Thanks for the reminder.

- It would be more secure to drop cdnjs.cloudflare.com from the CSP, and instead use hashes (static) or nonces (dynamic) as described [here](https://www.w3.org/TR/CSP2/#source-list-syntax) - note, it's not supported in CSP1 (IE11). This way it is only allowed to use the specified files and not every file in the CDN. An alternative of course is to host the files directly, not through CDN.

Yeah, but I'd rather do that as a separate pull request. This is already an improvement.

[Xaeroxe]

If merged this should close #167 as it would completely disable inline scripts for all modern browser users, even including IE.

[zealousidealroll]

Okay, I added the style-src.

[mozfreddyb]

Is there anything else that needs to be done before this can be merged?

[tesuji]

r? @QuietMisdreavus

[QuietMisdreavus]

I just did the barest reading of CSP formatting, so forgive me if i'm misunderstanding. Should this include an img-src so that we can load GitHub avatars from avatars.githubusercontent.com on crate details pages? In fact, we should probably allow images from any source, since certain crates have badges in their readme, or custom logos and favicons in their docs. (For example, the images in curve25519-dalek, the favicon in Rocket, the badges in egg-mode's readme.) However, if people consider this too much of a risk, i would be willing to limit it to just 'self' avatars.githubusercontent.com and break all these images.

This will neuter the pwnies crate and the dalek crates which had a working KaTeX import prior to our server migration, since they were loading scripts from other hosts. Since this was the entire point of the discussion, i'm willing to do this, but i'd like to tag in @hdevalence and @isislovecruft all the same to make sure they're aware of this.

[hdevalence]

Ideally I would like either rustdoc or docs.rs to have its own support for KaTeX, so that any crate could just use it directly by some first-class mechanism. (I realize this is somewhat orthogonal to the issue at hand, just restating it for clarity).

The dalek crates are all hosted with KaTeX on doc.dalek.rs, so while it would be unfortunate if the math on docs.rs broke, it's not the end of the world. To me, it seems more important that docs.rs isn't XSSable.

[QuietMisdreavus]

I'd also like to add the ability to link in some libraries as a first-class mechanism (#302). If i can get that in, then we can get these libraries to work properly on docs.rs without having to resort to XSS. If everyone's okay with fixing the XSS first and adding the features later, then i can work on the libraries feature in due time.

[QuietMisdreavus]

Ah, it looks like our version of Iron has possibly updated since this PR was originally posted, or something else happened that made these imports not work:

error[E0405]: cannot find trait `AfterMiddleware` in this scope
  --> src/web/mod.rs:72:6
   |
72 | impl AfterMiddleware for ContentSecurityPolicy {
   |      ^^^^^^^^^^^^^^^ not found in this scope
help: possible candidates are found in other modules, you can import them into scope
   |
48 | use iron::AfterMiddleware;
   |
48 | use iron::middleware::AfterMiddleware;
   |

error[E0425]: cannot find value `CONTENT_SECURITY_POLICY` in module `iron::headers`
  --> src/web/mod.rs:74:44
   |
74 |         if resp.headers.get(iron::headers::CONTENT_SECURITY_POLICY) == None {
   |                                            ^^^^^^^^^^^^^^^^^^^^^^^ not found in `iron::headers`
help: possible candidate is found in another module, you can import it into scope
   |
48 | use reqwest::header::CONTENT_SECURITY_POLICY;
   |

error[E0425]: cannot find value `CONTENT_SECURITY_POLICY` in module `iron::headers`
  --> src/web/mod.rs:76:32
   |
76 |                 iron::headers::CONTENT_SECURITY_POLICY,
   |                                ^^^^^^^^^^^^^^^^^^^^^^^ not found in `iron::headers`
help: possible candidate is found in another module, you can import it into scope
   |
48 | use reqwest::header::CONTENT_SECURITY_POLICY;
   |

[pietroalbini]

We should also add upgrade-insecure-requests to the CSP to reduce mixed content warnings on the browsers that supports it.

[QuietMisdreavus]

I took the branch and started updating it to properly compile. It turns out that the version of Hyper that Iron used doesn't have a header struct for CSP, so i had to write a basic one myself. (I didn't try to properly parse out a typed CSP, only store the text used for it. There's a content-security-policy crate which has structs that describe a CSP, but it currently does not support re-serializing the data back to a header format, so it is not sufficient for our use.)

However, when i tried to test it, i ran into problems with all the inline scripts used in the docs.rs template files. Since we don't have separate JavaScript files for the little scripts used on docs.rs's own pages, we'll need to either split those into separate files, or start including nonces/hashes in our CSP. I'll work on this.

In addition, i have doubts that including upgrade-insecure-requests will fix the favicon problem. The way our server is set up, the actual docs.rs web server doesn't handle HTTPS - AWS CloudFront is supplying that while providing our CDN. To be honest, i'm not sure what's even happening that's causing the favicon to be loaded over HTTP, since we're redirecting it exactly the same way we redirect anything else. The underlying URL may be served over HTTP, but when loading anything but a favicon, CloudFront will do the HTTPS upgrade for us. I'm worried that adding upgrade-insecure-requests will cause CloudFront itself to request HTTPS from the docs.rs server, which is not set up to handle it at the moment.

In addition, including upgrade-insecure-requests will make testing a development server harder since it's not guaranteed that those will be running over HTTPS, especially if it's running on localhost. If we do include it, it will likely need to be configurable, so that the production site can use it but a development site can skip it.

[pietroalbini]

I'm worried that adding upgrade-insecure-requests will cause CloudFront itself to request HTTPS from the docs.rs server, which is not set up to handle it at the moment.

Cloudfront doesn't care about the header (it's only used by browser), and its configuration explicitly says to only use HTTP when contacting the server.

I'll try to find time to investigate what's happening with the favicon.

[QuietMisdreavus]

I've hit another hurdle: Rustdoc itself adds some inline <script> and <style> tags:

• One inline script to set global JS variables related to creating relative URLs for the search bar.
• One inline script to set a global JS variable related to loading the right sidebar information.
• One inline style to set the image for the crate-search filter drop-down. (I'm actually not sure what this is doing here? @GuillaumeGomez, do you remember why this had to be here and not in rustdoc.css? We should also not emit this in the first place for docs.rs since we don't even emit the crate search filter for it any more.)

I was able to put nonces on all of docs.rs's own inline scripts, but since these are controlled by upstream rustdoc, it's going to be more difficult to properly allow these without just setting script-src 'unsafe-inline'.

[QuietMisdreavus]

Looking at it again, it looks like the reason for the inline style was because the SVG being linked in has the resource suffix applied, so from rustdoc's perspective it's going to be different per-crate.

I have an idea for how to allow these, but it will probably slow down our web server: we can dynamically insert nonces into rustdoc output, but skip docblocks, which are always sourced from user data. This way, the inline scripts and styles that rustdoc itself adds can run, but anything a user adds will get blocked. However, this also means that we need to deeply inspect and modify the HTML that rustdoc emits when serving it. Ideally, this would occur once, when documentation is initially built, but that won't help existing docs.

I have a branch available at https://github.com/QuietMisdreavus/docs.rs/tree/csp - i will push it over this PR when i believe that it's ready to be live. It's almost there, pending whether there's another way around the inline script issue.

[pietroalbini]

I have an idea for how to allow these, but it will probably slow down our web server: we can dynamically insert nonces into rustdoc output, but skip docblocks, which are always sourced from user data. This way, the inline scripts and styles that rustdoc itself adds can run, but anything a user adds will get blocked. However, this also means that we need to deeply inspect and modify the HTML that rustdoc emits when serving it. Ideally, this would occur once, when documentation is initially built, but that won't help existing docs.

I don't think that's going to work, an attacker can find ways to inject custom code outside of docblocks.

[pietroalbini]

Closing this PR, as we'll need to take a different approach to fix the problem. The team is working on it, and I hope I'll have public information to share soon.