Blazor Stop Requiring `unsafe-inline` in CSP by TanayParikh · Pull Request #36771 · dotnet/aspnetcore

TanayParikh · 2021-09-20T23:14:52Z

Blazor Server

The innerHtml was triggering the CSP, thereby requiring unsafe-inline. The reconnection logic has been updated to no longer require innerHtml. This is the only place I could see innerHtml being used, aside from the browser renderer, however that doesn't appear to cause an issue as the nodes don't appear to actually be added to the page.

aspnetcore/src/Components/Web.JS/src/Rendering/BrowserRenderer.ts

Lines 489 to 492 in da06d1b

    
             sharedSvgElemForParsing.innerHTML = markup || ' '; 
        
             return sharedSvgElemForParsing; 
        
           } else { 
        
             sharedTemplateElemForParsing.innerHTML = markup || ' ';

Per the Blazor docs

Specify unsafe-inline to allow the use of inline styles. The inline declaration is required for the UI in Blazor Server apps for reconnecting the client and server after the initial request. In a future release, inline styling might be removed so that unsafe-inline is no longer required.

Note, from what I found unsafe-eval doesn't appear to be an issue for Blazor Server (only WASM due to eval utilization in mono).

Validated using:

<meta http-equiv="Content-Security-Policy" content="default-src 'self'; img-src https://*; child-src 'none';">

Blazor WASM:

Issue was with the global module script:

aspnetcore/src/Components/Web.JS/src/Platform/Mono/MonoPlatform.ts

Line 212 in 5750125

    
           scriptElem.text = 'var Module; window.__wasmmodulecallback__(); delete window.__wasmmodulecallback__;';

unsafe-inline can be removed via adding the script hash to the CSP as per below:

Validated using:

    <meta http-equiv="Content-Security-Policy" 
        content="default-src 'self'; img-src https://*; child-src 'none'; script-src 'sha256-v8v3RKRPmN4odZ1CWM5gw80QKPCCWMcpNeOmimNL2AA=' 'self' 'unsafe-eval'">

where sha256-v8v3RKRPmN4odZ1CWM5gw80QKPCCWMcpNeOmimNL2AA= is the hash of the global module script.

Note: unsafe-eval is still required due to a dependency in mono. Currently the instantiate wasm call is failing without it.

aspnetcore/src/Components/Web.JS/src/Platform/Mono/MonoPlatform.ts

Line 272 in 5750125

compiledInstance = await compileWasmModule(dotnetWasmResource, imports);

Note, unsafe-inline also requires inline js script blocks to either have an associated hash or nonce. This means something like a Blazor.start(...) script would need to be put into a separate file. Not a big deal, just something worth noting.

Fixes: #34428

For: #34428

TanayParikh · 2021-09-21T00:13:05Z

Note: We'll need to update the docs to reflect this change (todo after merge):

https://docs.microsoft.com/en-us/aspnet/core/blazor/security/content-security-policy?view=aspnetcore-6.0

TanayParikh · 2021-09-21T02:25:45Z

In terms of testing, tried out some local scenarios, and didn't encounter any issues. Grep'd through the codebase to find offending functionality and made the fixes in this PR.

I looked into CSP'ifying the E2E tests, but that looks like a much more involved change (refactoring all the inline JS - or providing hashes, both of which may be cumbersome in terms of maintainability of the tests). Additionally, all the inline css would need to be stripped out of the test files as well. I'm open to exploring this path if people think it's worthwhile, personally I'm not totally sure if it's worth the time investment.

javiercn · 2021-09-21T15:28:26Z

@TanayParikh thanks for the thorough investigation.

The fixes for Blazor server look good to me.

Do you think we could get an issue filed on the runtime repo to better understand the need for eval and determine if there are potential alternatives implementations we can take? (BTW, just to be clear, I don't think this is a 6.0 item, but something we want to work towards in 7.0)

As per the unsafe inline, I'm ok if we document this for the time being to make sure folks include the hash in their csp policy.

That said, I think we should consider removing the inline script in 7.0 (not clear how). Can you file a bug to track it?

javiercn

Changes look good.

TanayParikh · 2021-09-21T15:44:54Z

That said, I think we should consider removing the inline script in 7.0 (not clear how). Can you file a bug to track it?

#36805

Do you think we could get an issue filed on the runtime repo to better understand the need for eval and determine if there are potential alternatives implementations we can take? (BTW, just to be clear, I don't think this is a 6.0 item, but something we want to work towards in 7.0)

dotnet/runtime#59416

src/Components/Web.JS/src/Platform/Circuits/DefaultReconnectDisplay.ts

SteveSandersonMS · 2021-09-21T16:13:26Z

@TanayParikh This is a great investigation! It's awesome that we now fully understand the gap between where we are and supporting arbitrary CSPs.

I agree with @javiercn that the Server changes look great (with a possible simplification as per comment above).

About the WebAssembly side:

Our own use of scriptElem.text could very likely be eliminated. If I remember correctly, it was put there as a workaround for an old Safari bug which probably no longer applies.
However, fixing this probably doesn't achieve anything until it becomes possible to call WebAssembly.instantiateStreaming without unsafe-eval. If this truly is an unresolved Chromium limitation, it's out of our direct control to fix.

There's no value in taking the risk of fixing our scriptElem.text code for 6.0 if it doesn't change anything about the option to avoid unsafe-eval. We could do it in main for 7.0 just as a matter of tidiness, but there's no value in bringing that change to release/6.0.

What do you think?

SteveSandersonMS · 2021-09-21T16:16:43Z

Actually I don't even know if Chromium does have a problem with WebAssembly.compileStreaming when unsafe-eval is disallowed. Comments at WebAssembly/content-security-policy#7 (comment) suggest the error reports are inconsistent and might be bogus. We'll have to actually try it ourselves. But it sounds like we'd first need the runtime to remove its direct reference to eval anyway.

TanayParikh · 2021-09-21T16:20:08Z

About the WebAssembly side:

Our own use of scriptElem.text could very likely be eliminated. If I remember correctly, it was put there as a workaround for an old Safari bug which probably no longer applies.

However, fixing this probably doesn't achieve anything until it becomes possible to call WebAssembly.instantiateStreaming without unsafe-eval. If this truly is an unresolved Chromium limitation, it's out of our direct control to fix.

There's no value in taking the risk of fixing our scriptElem.text code for 6.0 if it doesn't change anything about the option to avoid unsafe-eval. We could do it in main for 7.0 just as a matter of tidiness, but there's no value in bringing that change to release/6.0.

What do you think?

I agree! I've created #36805 to track removing scriptElem.text for 7.0, as well as dotnet/runtime#59416 to request mono reevaluate the usage of eval.

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.

TanayParikh · 2021-09-21T16:45:52Z

Documentation updates: dotnet/AspNetCore.Docs#23365

TanayParikh · 2021-09-21T18:51:15Z

/backport to release/6.0

github-actions · 2021-09-21T18:51:32Z

Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1258896840

github-actions · 2021-09-21T18:52:01Z

@TanayParikh an error occurred while backporting to release/6.0, please check the run log for details!

Error: @TanayParikh is not a repo collaborator, backporting is not allowed.

TanayParikh · 2021-09-21T18:55:43Z

@dotnet/aspnet-build has something changed with the backport bot? Using the same command as before.

Error: @TanayParikh is not a repo collaborator, backporting is not allowed.

Perhaps a new requirement? Do I need to be manually added somewhere?

https://docs.github.com/en/organizations/managing-access-to-your-organizations-repositories/adding-outside-collaborators-to-repositories-in-your-organization

github-actions · 2021-09-21T19:12:55Z

Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1258896840

github-actions · 2021-09-21T19:13:23Z

@TanayParikh an error occurred while backporting to release/6.0, please check the run log for details!

Error: @TanayParikh is not a repo collaborator, backporting is not allowed.

TanayParikh · 2021-09-22T16:05:25Z

/backport to release/6.0

github-actions · 2021-09-22T16:05:40Z

Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1262562948

github-actions · 2021-09-22T16:06:04Z

@TanayParikh an error occurred while backporting to release/6.0, please check the run log for details!

Error: @TanayParikh is not a repo collaborator, backporting is not allowed.

* Blazor Server Allow Unsafe Inline For: #34428 * Update MonoPlatform.ts * Fix DefaultReconnectDisplay.test * PR Feedback

) * 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

FelipeCostaGualberto · 2024-12-06T13:24:40Z

Hi, is this completed?

I'm having security complains in a corporate environment because my Blazor WASM App in .NET 8 doesn't work with wasm-unsafe-eval, only with unsafe-eval.

Blazor Server Allow Unsafe Inline

1033e8d

For: #34428

TanayParikh requested a review from a team as a code owner September 20, 2021 23:14

ghost added the area-blazor Includes: Blazor, Razor Components label Sep 20, 2021

Update MonoPlatform.ts

7cf6516

TanayParikh changed the title ~~Blazor Server Allow unsafe-inline~~ Blazor Allow unsafe-inline Sep 21, 2021

TanayParikh added feature-blazor-server feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly labels Sep 21, 2021

TanayParikh changed the title ~~Blazor Allow unsafe-inline~~ Blazor Stop Requiring unsafe-inline in CSP Sep 21, 2021

Fix DefaultReconnectDisplay.test

15d7239

javiercn approved these changes Sep 21, 2021

View reviewed changes

TanayParikh mentioned this pull request Sep 21, 2021

CSP for Blazor #34428

Closed

SteveSandersonMS reviewed Sep 21, 2021

View reviewed changes

src/Components/Web.JS/src/Platform/Circuits/DefaultReconnectDisplay.ts Outdated Show resolved Hide resolved

PR Feedback

3438508

SteveSandersonMS self-requested a review September 21, 2021 16:17

SteveSandersonMS approved these changes Sep 21, 2021

View reviewed changes

TanayParikh mentioned this pull request Sep 21, 2021

Update Blazor CSP Guidance dotnet/AspNetCore.Docs#23365

Merged

TanayParikh enabled auto-merge (squash) September 21, 2021 16:46

TanayParikh merged commit 2b0e81f into main Sep 21, 2021

TanayParikh deleted the taparik/allowUnsafeInline branch September 21, 2021 17:41

ghost added this to the 7.0-preview1 milestone Sep 21, 2021

TanayParikh added a commit that referenced this pull request Sep 22, 2021

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

e7c72b8

* Blazor Server Allow Unsafe Inline For: #34428 * Update MonoPlatform.ts * Fix DefaultReconnectDisplay.test * PR Feedback

TanayParikh mentioned this pull request Sep 22, 2021

Backport Blazor Stop Requiring unsafe-inline in CSP (#36771) #36846

Merged

10 tasks

	sharedSvgElemForParsing.innerHTML = markup \|\| ' ';
	return sharedSvgElemForParsing;
	} else {
	sharedTemplateElemForParsing.innerHTML = markup \|\| ' ';

Conversation

TanayParikh commented Sep 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Blazor Server

Blazor WASM:

Uh oh!

TanayParikh commented Sep 21, 2021

Uh oh!

TanayParikh commented Sep 21, 2021

Uh oh!

javiercn commented Sep 21, 2021

Uh oh!

javiercn left a comment

Choose a reason for hiding this comment

Uh oh!

TanayParikh commented Sep 21, 2021

Uh oh!

Uh oh!

SteveSandersonMS commented Sep 21, 2021

Uh oh!

SteveSandersonMS commented Sep 21, 2021

Uh oh!

TanayParikh commented Sep 21, 2021

Uh oh!

TanayParikh commented Sep 21, 2021

Uh oh!

TanayParikh commented Sep 21, 2021

Uh oh!

github-actions bot commented Sep 21, 2021

Uh oh!

github-actions bot commented Sep 21, 2021

Uh oh!

TanayParikh commented Sep 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 21, 2021

Uh oh!

github-actions bot commented Sep 21, 2021

Uh oh!

TanayParikh commented Sep 22, 2021

Uh oh!

github-actions bot commented Sep 22, 2021

Uh oh!

github-actions bot commented Sep 22, 2021

Uh oh!

FelipeCostaGualberto commented Dec 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

TanayParikh commented Sep 20, 2021 •

edited

Loading

TanayParikh commented Sep 21, 2021 •

edited

Loading