Skip to content
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

Feature Request : Add Support for Strict Content Security Policy #21711

Closed
naveedahmed1 opened this issue Sep 6, 2021 · 7 comments
Closed

Feature Request : Add Support for Strict Content Security Policy #21711

naveedahmed1 opened this issue Sep 6, 2021 · 7 comments
Labels
area: devkit/build-angular feature Issue that requests a new feature
Milestone

Comments

@naveedahmed1
Copy link

naveedahmed1 commented Sep 6, 2021

🚀 Feature request

Command (mark with an x)

  • [x ] build

Description

Lighthouse now throws error if No CSP is found in enforcement mode.

Ensure CSP is effective against XSS attacks

No CSP found in enforcement mode with High Severity

Describe the solution you'd like

My knowledge about CSP is limited and I was going through these guides on CSP:

http://web.dev
https://web.dev/csp-xss/
https://web.dev/strict-csp/

Apparently Nonce-based CSP seems easy to implement for SSR, for example for scripts, after Angular returns the final html, we can use string replace function on server side, to replace "<script" with "<script nonce="RANDOM_NONCE"" add RANDOM_NONCE in CSP header and I believe it should work.

But this string replace work will have to be done for each request.

On the other hand, in case of Hash-based strict CSP if Angular CLI calculates hashes and include these hashes in script tags at build time, this could save us the string replace work that has to be done for each request in case of nonce. But there's a tradeoff, we will have to sync these hashes with our Server Side App so that appropriate headers could be added for generated response. Alternatively CSP could be generated by Angular CLI and added as Content-Security-Policy meta tag.

Ref: https://web.dev/strict-csp/#step-1:-decide-if-you-need-a-nonce-or-hash-based-csp

There are two types of strict CSPs, nonce- and hash-based. Here's how they work:

Nonce-based CSP: You generate a random number at runtime, include it in your CSP, and associate it with every script tag in your page. An attacker can't include and run a malicious script in your page, because they would need to guess the correct random number for that script. This only works if the number is not guessable and newly generated at runtime for every response.

Hash-based CSP: The hash of every inline script tag is added to the CSP. Note that each script has a different hash. An attacker can't include and run a malicious script in your page, because the hash of that script would need to be present in your CSP.

Criteria for choosing a strict CSP approach:

Nonce-based CSP
For HTML pages rendered on the server where you can create a new random token (nonce) for every response

Hash-based CSP
For HTML pages served statically or those that need to be cached. For example, single-page web applications built with frameworks such as Angular, React or others, that are statically served without server-side rendering

@naveedahmed1
Copy link
Author

I think this repo from Google can provide some directions https://github.com/google/strict-csp/tree/main/strict-csp-html-webpack-plugin

@alan-agius4 alan-agius4 added area: devkit/build-angular feature Issue that requests a new feature labels Sep 7, 2021
@ngbot ngbot bot added this to the Backlog milestone Sep 7, 2021
@dgp1130
Copy link
Collaborator

dgp1130 commented Sep 9, 2021

Hi @naveedahmed1, I don't think it should be necessary do anything very fancy for CSP. We only generate scripts and styles for the same domain, so default-src self; should be enough without any need for hashes or nonces. Is there some reason this is inadequate?

The one exception is styles, where we require style-src unsafe-inline;, see angular/angular#6361.

We can't really generate a CSP for users as there are various attributes that apply beyond what Angular provides. Users can control their index.html page and add third-party scripts, or control fetch data, iframe URLs, etc. We can't take full responsibility for generating this string, so it's ultimately up to users and including default-src self; style-src unsafe-inline; should be all that users need to do for Angular.

I can see the argument that SSR could generate a CSP policy to some extent. That still runs into challenges with user-controlled fields that can't be generated and has an implicit risk that attacker code in the request could poison files before hashes/nonces are generated and invalidate much of the security CSP provides. Regardless, that would be handled by Universal rather than the CLI. We can follow up there if you have a specific thought or request for SSR.

We could probably do a better job documenting this, as our current CSP section is pretty limited.

@ghost
Copy link

ghost commented Sep 17, 2021

default-src self; style-src unsafe-inline; works great for most scenarios.

However, we're running into a scenario where it would be beneficial to use strict-dynamic, which disables self in browsers which support it. The result is that the bundled Angular files (polyfills.js, runtime.js, etc...) are blocked when using strict-dynamic.

It sounds like the direction that @naveedahmed1 was trying to go down might solve this issue for us. If there was a way to use hashing or nonces for the Angular bundle files, then that would probably allow us to use strict-dynamic. In the meantime it seems that Angular can't be used alongside strict-dynamic at this point in time because I can't find any solution that would work.

@clydin
Copy link
Member

clydin commented Sep 17, 2021

Could you expand on the scenario where it would be beneficial to use strict-dynamic? We are always interested in new use cases.

With the current internal bundler usage of Webpack, dynamic import expressions are not used for lazy loading of JavaScript but rather Webpack uses its own runtime that injects script elements into the DOM.
Also, sub-resource integrity (SRI) checks are fully supported with the CLI which will generate the appropriate integrity attributes for both initial and lazy scripts (given the above Webpack injection method) when the option is enabled. (SRI: https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity)
In the future, we are currently exploring the ability to generate a bundled ES module application output which may include the use of dynamic imports. In that scenario, SRI is not supported with dynamic imports and the use of strict-dynamic then provides a replacement of such functionality.

Irrespective of the above, hashes for each script within the application can potentially be calculated as part of the deployment step for the application. This is a useful time to calculate them as the CSP configuration would need to be updated for each deployment to include those hashes. The server configuration containing the new CSP configuration can then be uploaded during deployment or forwarded to the project's infrastructure team for inclusion, if applicable.

Nonces are recommended to change per request and would best be handled by Angular Universal and SSR (as mentioned in a previous comment) as Universal generates the HTML for each request and can then inject nonces as needed.

@ghost
Copy link

ghost commented Sep 20, 2021

Our use-case for strict-dynamic is as follows:
We're loading a handful of external scripts for integrations with third party services. These scripts then load other scripts, which load even more scripts. This quickly results in a very long script-src whitelist which is undesirable for two important reasons:

  • It's difficult to keep track of from a maintenance perspective
  • After reading this paper I'm learning that long http whitelists are inherently insecure for a variety of reasons.

What we really want is the "trust propagation" aspect of strict-dynamic so that we can whitelist only the 2 or 3 top-level entry point scripts and implicitly trust all of the downstream scripts that they load. Ideally we could use hashes for these, but even a domain-based whitelist with trust propagation would be a step forward from where we are.

The unfortunate (for us) side effect of browsers' current implementation is that, by design, they ignore self when strict-dynamic is specified, which breaks Angular because the go-to way to tell the browser to load [polyfill.js, runtime.js, etc...] is to use self. In a perfect world we could tell the browser to use strict-dynamic alongside self, but that doesn't seem to be possible from my research.

If my current understanding is correct, then SRI is a great tool to solve a different problem, thus it wouldn't help us to achieve the "trust propagation" that we're looking for. It seems that SRI adds the hash to the script tag that's inserted into the index, whereas what we need is to add the hash to script-src in our CSP.

Your suggestion of calculating the hashes of the generated scripts and writing them to the CSP sounds like it would probably work. However, there are two extra things to keep in mind there:

  • We'd have to take the time to write and maintain our own script to run as an extra stage in our build pipeline
  • At that point we'd have to maintain two separate index.htmls, two separate CSPs, or some variation thereof, because per my understanding this would only be possible for the deployed application and not for local development with ng serve.

Per my understanding I think the above are absolutely possible, but it seems to be enough engineering effort to tip the cost/benefit ratio in favor of living with long whitelists at least for the short-term.

Please feel free to poke holes in the above if relevant. I might have gotten something wrong because I'm still learning the nitty-gritty details about modern CSP stuff. And thank you for your help!

@dgp1130
Copy link
Collaborator

dgp1130 commented Sep 23, 2021

I think the original issue was about CSP in general, and default-src self; style-src unsafe-inline; should be enough there for Angular.

Regarding @pbrowning-tt's concern, it's unfortunate that strict-dynamic doesn't work with self, which I think is the real solution here. I'm not sure if there's any motivation to make that work in the CSP spec or if there is some technical reason why that can't or shouldn't be supported.

While Angular could generate a CSP with explicit hashes, we don't fully own the user's CSP string, as dynamic behavior in the app can require other changes to the CSP which we couldn't possibly generate (such as image-src or frame-src). Allowing the high-degree of customization that would be required makes this a much more complex feature for Angular than just hashing a bunch of files. For users, there is nothing Angular-specific here (aside from the style-src unsafe-inline;) and since strict-dynamic happens to require this much complexity, that is the effort that is required in its solution.

Our current approach has been to let users deal with the CSP strings and require minimal exceptions for Angular to allow the policy to be as strict as possible. Future CSP features might allow Angular to be more intelligent than default-src self; gives, and then we might consider doing something more complex to improve application security. As it stands today however, we can't do much better than default-src self; and generating a policy compatible with strict-dynamic is unnecessary complexity that can be just as easily handled by the user.

We should definitely update our docs about this to be more clear about style-src unsafe-inline; which I think is the main action item for this issue.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: devkit/build-angular feature Issue that requests a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants