-
Notifications
You must be signed in to change notification settings - Fork 22.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update CSP practical guide for consistency #37227
Update CSP practical guide for consistency #37227
Conversation
Preview URLs
External URLs (1)URL:
(comment last updated: 2024-12-20 10:45:28) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this, Chris! I had a few comments. I also wonder though: why do we need this document, in this form, as well as https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP#strict_csp ? It seems like they cover essentially the same ground, except https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP#strict_csp adds some more details, that someone would have to know anyway if they want to implement strict CSP?
Should we consider making this doc much shorter, and just say "implement strict CSP, here's the link"?
files/en-us/web/security/practical_implementation_guides/csp/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/csp/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/csp/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/csp/index.md
Outdated
Show resolved
Hide resolved
@wbamberg I intended that this document be a short, snappy, "quickfire" guide to what you need to, do, which links back to your guide for a much more in-depth treatment of the topic. It looks from your comments that I perhaps haven't gone far enough. I'll revisit it now, considering your comments. I'll also look at the Observatory scoring criteria, to make sure I don't miss anything out that really needs to be in here. |
Thanks @wbamberg! I also chopped down some of the sections a bit, and removed all the extra examples at the bottom, and they didn't seem that useful really, and diluted the message of the guide. WDYT? |
I'm not going to harp on about this endlessly, but for example, we just landed a guide about XSS, which describes strict CSP as one of the main defenses. The relevant bit is just this: https://developer.mozilla.org/en-US/docs/Web/Security/Attacks/XSS#deploying_a_csp . It doesn't quote a strict CSP, or talk about steps to deploying it, or strict-dynamic, or any other considerations. It really just says very quickly what a strict CSP is, and points to the guide page, which ought to give people all this detail and more. CSP is complicated, and duplicating a lot of these details introduces a lot of scope for errors. |
files/en-us/web/security/practical_implementation_guides/csp/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/csp/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/csp/index.md
Outdated
Show resolved
Hide resolved
This pull request has merge conflicts that must be resolved before it can be merged. |
files/en-us/web/security/practical_implementation_guides/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/index.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates, Chris! I like the shape of this document a lot better now. I have a few more comments but they are all little.
files/en-us/web/security/practical_implementation_guides/csp/index.md
Outdated
Show resolved
Hide resolved
|
||
## Problem | ||
|
||
The main problem this article focuses on is Cross-site scripting ({{Glossary("Cross-site_scripting", "XSS")}}) attacks. These are generally due to a lack of control and awareness of the sources from which site resources are loaded. This problem gets more difficult to manage as sites become larger and more complex and increasingly rely on third-party resources such as JavaScript libraries. | ||
|
||
> [!NOTE] | ||
> CSP is one part of a complete strategy for protecting against XSS attacks. There are other factors involved, such as [output encoding](/en-US/docs/Web/Security/Attacks/XSS#output_encoding) and [sanitization](/en-US/docs/Web/Security/Attacks/XSS#sanitization), which are also important. | ||
|
||
CSP can also help to fix other problems, which are covered in other articles: | ||
|
||
- [Preventing clickjacking](/en-US/docs/Web/Security/Practical_implementation_guides/Clickjacking) by stopping your site being embedded into {{htmlelement("iframe")}} elements. This is done using the CSP [`frame-ancestors`](/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors) directive. | ||
- Preventing [manipulator-in-the-middle](/en-US/docs/Glossary/MitM) (MiTM) attacks by upgrading any HTTP connections to HTTPS. This is helped by the CSP [`upgrade-insecure-requests`](/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors) directive. See [HSTS implementation](/en-US/docs/Web/Security/Practical_implementation_guides/TLS#http_strict_transport_security_implementation) for more details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Preventing [manipulator-in-the-middle](/en-US/docs/Glossary/MitM) (MiTM) attacks by upgrading any HTTP connections to HTTPS. This is helped by the CSP [`upgrade-insecure-requests`](/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors) directive. See [HSTS implementation](/en-US/docs/Web/Security/Practical_implementation_guides/TLS#http_strict_transport_security_implementation) for more details. | |
- Preventing [manipulator-in-the-middle](/en-US/docs/Glossary/MitM) (MiTM) attacks by upgrading any HTTP connections to HTTPS. This is helped by the CSP [`upgrade-insecure-requests`](/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors) directive. See [Upgrading insecure requests](/en-US/docs/Web/HTTP/CSP#upgrading_insecure_requests). |
(a better link I think, I am biased though of course!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated ;-)
- [Web workers](/en-US/docs/Web/API/Web_Workers_API) (via [document directives](/en-US/docs/Glossary/Document_directive)). | ||
- Embedded (i.e., {{htmlelement("iframe")}}) content. | ||
- Navigation / form submission destinations (via [navigation directives](/en-US/docs/Glossary/Navigation_directive)). | ||
Strict CSPs are preferred over [location-based](/en-US/docs/Web/HTTP/CSP#location-based_policies) policies, also called allowlist policies, where you specify which domains scripts can be run from. This is because allowlist policies often end up allowing unsafe domains, which defeats the entire point of having a CSP, and they can get very large and unwieldy, especially if you are trying to permit services that require many third party scripts to function. | ||
|
||
### Steps for implementing CSP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wonder if it's worth calling this "Steps for implementing a strict CSP"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree; it is not just that — strict CSP is the main thrust of it, but it also covers other things, like alternatives if strict CSP isn't for you, and reporting. I'm going to leave this as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, fair enough.
1. Begin by trying out a strict CSP, as outlined above, then start to pinpoint resources that are failing to load as a result of the policy. | ||
2. Make sure that external and internal scripts (included via {{htmlelement("script")}} elements) that you want to run have the correct nonce inserted into the [`nonce`](/en-US/docs/Web/HTML/Element/script#nonce) attributes by the server. If you are instead using hashes, external scripts should have the correct hash inserted into [`integrity`](/en-US/docs/Web/HTML/Element/script#integrity) attributes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This 1-2 reads a little odd to me. I don't think realistically you will start by sending the header and then add nonce/hash to all the scripts that break (I mean for one thing the hashes are directly tied to individual resources). I think it's more likely that you will update your website to generate nonces or hashes and insert them into the documents you serve, in one step.
It might also be worth having a step about, decide whether to use nonces or hashes, which depends on whether you can dynamically generate content to serve (nonces) or need to serve static content (hashes).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I have updated the structure, merging the two bullets into one and adding an extra bullet about choosing nonces or hashes.
``` | ||
1. Begin by trying out a strict CSP, as outlined above, then start to pinpoint resources that are failing to load as a result of the policy. | ||
2. Make sure that external and internal scripts (included via {{htmlelement("script")}} elements) that you want to run have the correct nonce inserted into the [`nonce`](/en-US/docs/Web/HTML/Element/script#nonce) attributes by the server. If you are instead using hashes, external scripts should have the correct hash inserted into [`integrity`](/en-US/docs/Web/HTML/Element/script#integrity) attributes. | ||
3. If an allowed script goes on to load other, third-party scripts, those scripts will fail to load because they won't have the required nonce or hash. Mitigate this problem by adding the [`strict-dynamic`](/en-US/docs/Web/HTTP/CSP#the_strict-dynamic_keyword) directive, which gives scripts loaded by the first script the same level of trust without being explicitly given a nonce or hash. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3. If an allowed script goes on to load other, third-party scripts, those scripts will fail to load because they won't have the required nonce or hash. Mitigate this problem by adding the [`strict-dynamic`](/en-US/docs/Web/HTTP/CSP#the_strict-dynamic_keyword) directive, which gives scripts loaded by the first script the same level of trust without being explicitly given a nonce or hash. | |
3. If an allowed script goes on to load third-party scripts, those scripts will fail to load because they won't have the required nonce or hash. Mitigate this problem by adding the [`strict-dynamic`](/en-US/docs/Web/HTTP/CSP#the_strict-dynamic_keyword) directive, which gives scripts loaded by the first script the same level of trust without being explicitly given a nonce or hash. |
I think the first comma in "load other, third-party scripts, those scripts will" was ungrammatical, it technically wants to be "load other, third-party, scripts, those scripts will" (i.e. "other scripts that are third-party scripts") but that's horrible of course.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, your suggestion is much better ;-)
Updated.
6. If you are still having trouble with certain items, you can consider an alternative, more permissive policy. For example: | ||
- Add specific sources as highlighted during testing; for example, `style-src 'self' https://example.com/`. | ||
- `default-src https:` still provides some protection, disabling unsafe inline/`eval()` and only allowing loading of resources (images, fonts, scripts, etc.) over HTTPS. | ||
- `unsafe-hashes` allows you to use hashes to enable usage of inline event handlers and `style` attribute values. | ||
- `unsafe-eval` allows dynamic evaluation of strings as JavaScript, but beware: it defeats much of the purpose of having a CSP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6. If you are still having trouble with certain items, you can consider an alternative, more permissive policy. For example: | |
- Add specific sources as highlighted during testing; for example, `style-src 'self' https://example.com/`. | |
- `default-src https:` still provides some protection, disabling unsafe inline/`eval()` and only allowing loading of resources (images, fonts, scripts, etc.) over HTTPS. | |
- `unsafe-hashes` allows you to use hashes to enable usage of inline event handlers and `style` attribute values. | |
- `unsafe-eval` allows dynamic evaluation of strings as JavaScript, but beware: it defeats much of the purpose of having a CSP. | |
6. If you are unable to remove usages of `eval()`, you can add [`unsafe-eval`](/en-US/docs/Web/HTTP/Headers/Content-Security-Policy#unsafe-eval) keyword to your strict CSP: although this makes it significantly weaker. | |
7. If you are unable to remove event handler attributes, you can add the [`unsafe-hashes`](/en-US/docs/Web/HTTP/Headers/Content-Security-Policy#unsafe-hashes) to your strict CSP: this is somewhat unsafe, but much safer than allowing all inline JavaScript. | |
8. If you are unable to get a strict CSP to work, an allowlist-based CSP is much better than none, and a CSP like `default-src https:` still provides some protection, disabling unsafe inline/`eval()` and only allowing loading of resources (images, fonts, scripts, etc.) over HTTPS. |
So this is just a suggestion but what I'm trying to get at it that your point 6 here seems to me to cover two cases of thing: relaxations which you can make in the context of a strict CSP (unsafe-eval, unsafe-hashes) and departures from a strict CSP. So I'm trying to distinguish these and make it clearer that the things in point 8 are in a place where you've given up on strict CSP.
In particular I thought the previous text could imply that if you just add default-src https
to a strict CSP, then it will allow more permissive scripts ("only allowing loading of ... scripts ... over HTTPS.") but it won't of course because strict CSP sets script-src
which will be used instead. I know you do say "an alternative, more permissive policy" but I think it's not that clear that this means "abandoning strict CSP".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could even pull step 8 out of the list into a subsequent paragraph, to emphasise that this is a departure from the main approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This suggestion is really good. I've included it, but rewritten it a bit.
And yes, I've made 8. a paragraph below the list rather than another bullet. It is a departure rather than a subsequent step on the main approach.
Content-Security-Policy: script-src 'strict-dynamic' 'nonce-2726c7f26c' | ||
``` | ||
> [!WARNING] | ||
> If at all possible, avoid including unsafe sources inside your CSP. Examples include `unsafe-inline`, `data:` URIs inside `script-src`, `object-src`, or `default-src`, and overly broad sources or form submission targets. Similarly, the use of `script-src 'self'` can be unsafe for sites with JSONP endpoints. These sites should use a `script-src` that includes the path to their JavaScript source folder(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
> If at all possible, avoid including unsafe sources inside your CSP. Examples include `unsafe-inline`, `data:` URIs inside `script-src`, `object-src`, or `default-src`, and overly broad sources or form submission targets. Similarly, the use of `script-src 'self'` can be unsafe for sites with JSONP endpoints. These sites should use a `script-src` that includes the path to their JavaScript source folder(s). | |
> If at all possible, avoid including unsafe sources inside your CSP. Examples include: | |
> - `unsafe-inline` | |
> - `data:` URIs inside `script-src`, `object-src`, or `default-src` | |
> - overly broad sources or form submission targets | |
> | |
> Similarly, the use of `script-src 'self'` can be unsafe for sites with JSONP endpoints. These sites should use a `script-src` that includes the path to their JavaScript source folder(s). |
Is the JSONP thing only relevant to self
? I thought it was an issue for any kind of location-based policy. I don't really understand this very well though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion applied. I don't know about the JSONP thing either.
|
||
```http-nolint | ||
Reporting-Endpoints: csp-endpoint="https://example.com/csp-reports" | ||
## Report-only CSPs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have made this an H3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree with this. It's not really a standalone topic. Updated.
```http | ||
Content-Security-Policy: default-src 'none'; frame-ancestors 'none' | ||
``` | ||
> [!NOTE] > `report-to` is preferred over the deprecated `report-uri`; however, both are still needed because `report-to` does not yet have full cross-browser support. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the >
at the start of this is a workaround for the new note format not working when the content starts with a backtick? In this situation Josh uses the old format. You could also do:
> [!NOTE] > `report-to` is preferred over the deprecated `report-uri`; however, both are still needed because `report-to` does not yet have full cross-browser support. | |
> [!NOTE] The `report-to` directive is preferred over the deprecated `report-uri` directive; however, both are still needed because `report-to` does not yet have full cross-browser support. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that's just what prettier does to it as a result of the error. I usually fix such instances in the manner you've suggested, but I missed this one. Fixed now!
…ndex.md Co-authored-by: wbamberg <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the updates, Chris! I'm not merging in case you want feedback from the people tagged in your description, but otherwise I'm happy for this to go.
Thank YOU, @wbamberg! I think this is much better now, and it doesn't really lack any information that it had before. I am therefore electing to merge it rather than leave it lying around. If anyone else has concerns about the content update, I can always create a new PR to handle it. |
Description
After the main MDN CSP guide was updated to follow accepted advice from elsewhere (such as OWASP and Google), it became clear that our practical CSP guide (which is referenced by the HTTP Observatory tool) also needing updating to be consistent with this advice.
This PR attempts to do that. We need to be careful before merging that this is not only correct but also gels with the reports returned by Observatory and Mozilla's sec team.
Also alerting @mozfreddyb and @argl to this PR.
Motivation
Additional details
Related issues and pull requests