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

CSP for Blazor #34428

Closed
Ponant opened this issue Jul 16, 2021 · 13 comments · Fixed by #36771
Closed

CSP for Blazor #34428

Ponant opened this issue Jul 16, 2021 · 13 comments · Fixed by #36771
Assignees
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed
Milestone

Comments

@Ponant
Copy link
Contributor

Ponant commented Jul 16, 2021

Hello,
I would like to know if there is any plan to make the Blazor Framework more strict when it comes to Content Security Policy, especially in the current context of using tokens in session storage (Azure B2C in our case). I am referring to unsafe-eval and unsafe-inline in the docs,
https://docs.microsoft.com/en-us/aspnet/core/blazor/security/content-security-policy?view=aspnetcore-6.0

Also the idea of using hashes more than allowed lists, see docs above.
That will become important, in my opinion, sooner or later as part of security standards (CSP is widely ignored unfortunately).

@Pilchie Pilchie added the area-blazor Includes: Blazor, Razor Components label Jul 16, 2021
@mkArtakMSFT mkArtakMSFT added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Jul 19, 2021
@mkArtakMSFT mkArtakMSFT added this to the 6.0-rc2 milestone Jul 19, 2021
@damienbod
Copy link
Contributor

I have this problem as well and due to the Javascript created / required by Blazor, the CSP can not be implemented in a good way. The following script is generated:

<script>
var Module; window.__wasmmodulecallback__(); delete window.__wasmmodulecallback__;
</script>

Can the Javascript not be improved? Due to the Blazor Javascript, the CSP for the script-src is defined with

script-src 'self' 'unsafe-inline' 'unsafe-eval'

It would be really cool if this could be improved.

Greetings Damien

@damienbod
Copy link
Contributor

Also have a problem with the aspnetcore-browser-refresh.js script in dev. Would it be possible to add a way to add a CSP nonce to this script? I could disable this for dev, but I think it would be a better solution not the disable part of the CSP for local dev.

@Ponant
Copy link
Contributor Author

Ponant commented Aug 13, 2021

Hi @damienbod , indeed the browser refresh feature is a mess with CSP, it blocks, hangs and so on.
I have made a small library which focuses on performance an directness rather than being fluent and so on. Basic tests have been made and will probably need improvement. If you wish I can drop it on github. I made this library after realizing that existing ones either have perf issues or a wrong approach to CSP (at least from my understanding). But it is made for server side and .Net 6. code style. Cheers

@Ponant
Copy link
Contributor Author

Ponant commented Aug 13, 2021

@damienbod , there is a placeholder issue on CSP on general, #6001 .
So, I believe we should migrate the questions to this issue in this way the .Net people can better track the need for CSP stuff etc and perhaps respond. So I will close this one.

@Ponant Ponant closed this as completed Aug 13, 2021
@damienbod
Copy link
Contributor

damienbod commented Aug 13, 2021

@Ponant I don't think you should close this issue. This is the fix for the Blazor stuff which prevents a good CSP definition, how you implement the CSP is a different topic.

@Ponant
Copy link
Contributor Author

Ponant commented Aug 13, 2021

OK

@Ponant
Copy link
Contributor Author

Ponant commented Sep 13, 2021

@damienbod , hi, here is the code as promised (sorry for the delay I was out on holliday!), #6001 . Your comments are most welcomed.

@mkArtakMSFT mkArtakMSFT added bug This issue describes a behavior which is not expected - a bug. and removed investigate enhancement This issue represents an ask for new feature or an enhancement to an existing one labels Sep 16, 2021
@ghost ghost added the Working label Sep 17, 2021
@TanayParikh
Copy link
Contributor

TanayParikh commented Sep 17, 2021

unsafe-eval

Blazor doesn't appear to use this directly, however we inherit this "dependency" from the mono runtime:
https://github.com/dotnet/runtime/blob/96cb482d2dde661cc394e66c8ee94d5edecd1724/src/mono/wasm/runtime/library_mono.js#L745

@javiercn who'd be a good contact to get eyes on this?

My current operating assumption is blazor-server needs unsafe-inline (for reconnection logic) and blazor-wasm needs unsafe-eval based on the mono runtime. Will spin up some sample apps to confirm this assumption.

@TanayParikh
Copy link
Contributor

TanayParikh commented Sep 17, 2021

cc/ @radical based on the git blame history. What would the feasibility be to remove eval so that we don't have to have unsafe-eval?

@TanayParikh
Copy link
Contributor

Okay having #36771 close out this issue. With that PR, unsafe-inline should no longer be required.

Created #36805 to investigate remove the inline module script in Blazor Wasm, in the interim the sha256-v8v3RKRPmN4odZ1CWM5gw80QKPCCWMcpNeOmimNL2AA= hash can be used in the CSP.

Lastly, created dotnet/runtime#59416 to investigate feasibility of removing eval in mono, such that we no longer require unsafe-eval in the Blazor WASM CSP.

TanayParikh added a commit to dotnet/AspNetCore.Docs that referenced this issue Sep 21, 2021
Reflects changes in dotnet/aspnetcore#36771 / dotnet/aspnetcore#34428.

Created dotnet/aspnetcore#36805 to track removal of the inline script which requires the `sha256-v8v3RKRPmN4odZ1CWM5gw80QKPCCWMcpNeOmimNL2AA=` hash.
@ghost ghost added Done This issue has been fixed and removed Working labels Sep 21, 2021
TanayParikh added a commit that referenced this issue Sep 21, 2021
* Blazor Server Allow Unsafe Inline

For: #34428

* Update MonoPlatform.ts

* Fix DefaultReconnectDisplay.test

* PR Feedback
TanayParikh added a commit that referenced this issue Sep 22, 2021
* Blazor Server Allow Unsafe Inline

For: #34428

* Update MonoPlatform.ts

* Fix DefaultReconnectDisplay.test

* PR Feedback
wtgodbe pushed a commit that referenced this issue Sep 22, 2021
)

* Blazor Stop Requiring `unsafe-inline` in CSP (#36771)

* Blazor Server Allow Unsafe Inline

For: #34428

* Update MonoPlatform.ts

* Fix DefaultReconnectDisplay.test

* PR Feedback

* Update blazor.server.js

* Update blazor.server.js
@indcoder
Copy link

indcoder commented Oct 5, 2021

@TanayParikh IMO, this issue still needs to be open because they are 3 aspects of Blazor WASM which are forcing us to incorporate a lax CSP

@TanayParikh
Copy link
Contributor

TanayParikh commented Oct 5, 2021

Hi @indcoder. The fixes from this issue were mainly targeted towards Blazor Server.

This should be done already. The only thing on "Hold" is the documentation. It will be released inline with the release to avoid confusion.

  • unsafe-eval [open]

This has an external dependency on dotnet/runtime, specifically mono's usage of eval. (dotnet/runtime#59416). This is currently targeted for 7.0. Note, there's also a Chrome bug which may come into play here (WebAssembly/content-security-policy#7).

  • web socket ws(s) for aspnet-browser-refresh in development environment . CSP nonce as mentioned by @damienbod in the thread above.

Using #33068 to track this.

@indcoder
Copy link

indcoder commented Oct 5, 2021

Thank you @TanayParikh for the clarification and by spawning them into their own individual issues for better tracking and triaging.

Going with the below as for now in my Azure SWA. Hunted them by going default-src none

{
"content-security-policy": "default-src 'none'; object-src 'none'; connect-src 'self' ; script-src 'self' 'sha256-v8v3RKRPmN4odZ1CWM5gw80QKPCCWMcpNeOmimNL2AA=' 'unsafe-eval'; font-src 'self'; img-src 'self' data: https:; upgrade-insecure-requests;" 
 },

Only delta from below is font-src
https://github.com/dotnet/AspNetCore.Docs/pull/23365/files#diff-af9b6f8b5de38b13f29e909fd265e200641be05e82021bb432cff343b7dacd76R91

@ghost ghost locked as resolved and limited conversation to collaborators Nov 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants