Skip to content

Explore removing inline module script for Blazor Wasm #36805

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.

Sign up for GitHub

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

Jump to bottom
Closed
TanayParikh opened this issue Sep 21, 2021 · 11 comments · Fixed by #38721
Closed

Explore removing inline module script for Blazor Wasm #36805

TanayParikh opened this issue Sep 21, 2021 · 11 comments · Fixed by #38721
Assignees
@pavelsavara
Labels
area-blazor Includes: Blazor, Razor Components cost: S Will take up to 2 days to complete feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly task
Milestone
7.0-preview1

Comments

@TanayParikh
Copy link
Contributor

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

Lines 201 to 215 in 5750125

// Due to a strange behavior in macOS Catalina, we have to delay loading the WebAssembly files
// until after it finishes evaluating a <script> element that assigns a value to window.Module.
// This may be fixed in a later version of macOS/iOS, or even if not it may be possible to reduce
// this to a smaller workaround.
function addGlobalModuleScriptTagsToDocument(callback: () => void) {
const scriptElem = document.createElement('script');
// This pollutes global but is needed so it can be called from the script.
// The callback is put in the global scope so that it can be run after the script is loaded.
// onload cannot be used in this case for non-file scripts.
window['__wasmmodulecallback__'] = callback;
scriptElem.text = 'var Module; window.__wasmmodulecallback__(); delete window.__wasmmodulecallback__;';
document.body.appendChild(scriptElem);
}

@TanayParikh TanayParikh added investigate area-blazor Includes: Blazor, Razor Components feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly labels Sep 21, 2021
@TanayParikh TanayParikh modified the milestone: .NET 7 Planning Sep 21, 2021
This was referenced Sep 21, 2021
Merged
Closed
@mkArtakMSFT mkArtakMSFT added this to the .NET 7 Planning milestone Sep 21, 2021
TanayParikh added a commit to dotnet/AspNetCore.Docs that referenced this issue Sep 21, 2021
@TanayParikh
Update Blazor CSP Guidance
5a80e8e 
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 TanayParikh mentioned this issue Sep 21, 2021
Merged
@TanayParikh
Copy link
Contributor Author

Note, if / when this is done, we must update the associated CSP guidance to stop requiring the script hash as part of the CSP.

@mkArtakMSFT mkArtakMSFT added cost: S Will take up to 2 days to complete triaged labels Oct 28, 2021
@guardrex
Copy link
Contributor

Side-note for docs tracking: I'm going to let this PU issue track the work and not keep a docs repo issue on this given that it's an IF/WHEN scenario at this time.

@guardrex guardrex mentioned this issue Nov 12, 2021
Closed
@TanayParikh TanayParikh mentioned this issue Jan 3, 2022
Merged
@TanayParikh
Copy link
Contributor Author

Assigning to @pavelsavara as #38721 resolves this issue.

@pavelsavara
Copy link
Member

pavelsavara commented Jan 10, 2022 •
edited
Loading

We will use dynamic import instead of script tag, but we will not remove it completely.
We will replace it with <link rel="modulepreload" ... to implement integrity check for chromium.
See

@TanayParikh
Copy link
Contributor Author

Hmm, would that still fix this issue though as there's no inline script?

https://github.com/dotnet/aspnetcore/pull/38721/files#diff-a5167572a221c4bbe4fa36e4e110b39011b6e8f547ce0ab8bdd204f55201f1c0L211-R229

Essentially, does the app run without errors with the following tag within the <head> section of the wwwroot/index.html host page?

<meta http-equiv="Content-Security-Policy" 
      content="base-uri 'self';
               block-all-mixed-content;
               default-src 'self';
               img-src data: https:;
               object-src 'none';
               script-src 'self' 
                          'unsafe-eval';
               style-src 'self';
               upgrade-insecure-requests;">

@pavelsavara
Copy link
Member

Seems fine
image

@pavelsavara
Copy link
Member

Maybe I don't fully follow what is the issue/goal here. Could you please elaborate @TanayParikh ?

@ghost ghost closed this as completed in #38721 Jan 11, 2022
@guardrex
Copy link
Contributor

guardrex commented Jan 11, 2022 •
edited
Loading

@TanayParikh ... There's no repo issue here for the 7.0 Roadmap (yet), and it's not clear if this will make that roadmap when that issue is created. Don't we need a tracking issue on the docs side so that we don't forget to document this for 7.0? Note that we don't have the ability to pre-document 7.0 yet ... the ability to version content in docs for 7.0 will happen mid-year ... but we can have a docs issue on HOLD on the Blazor.Docs project so that we don't miss this when the time comes.

@TanayParikh TanayParikh mentioned this issue Jan 11, 2022
Closed
@TanayParikh
Copy link
Contributor Author

what is the issue/goal here. Could you please elaborate @TanayParikh ?

Discussed with @pavelsavara offline. The issue here was we had an inline JS which is not permitted with the default CSP policy:

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

Line 212 in 5750125

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

To allow this script without triggering CSP errors we added the script's hash to the CSP:

https://github.com/dotnet/AspNetCore.Docs/pull/23365/files#diff-af9b6f8b5de38b13f29e909fd265e200641be05e82021bb432cff343b7dacd76R91

With @pavelsavara's changes here:
https://github.com/dotnet/aspnetcore/pull/38721/files#diff-a5167572a221c4bbe4fa36e4e110b39011b6e8f547ce0ab8bdd204f55201f1c0L211-R223

we no longer have the inline JS script, hence we also no longer need the script hash we added to the CSP.

Thanks for pointing that out @guardrex, I've created dotnet/AspNetCore.Docs#24570 to ensure we update the docs accordingly for .NET 7

@damienbod
Copy link
Contributor

damienbod commented Jan 12, 2022 •
edited
Loading

@TanayParikh This is cool, when will this be released?

Is there a way to add a CSP nonce to all the Blazor js scripts after this change? It is easy to add this, if the Blazor js script is added in the index/host file, but if the script is created dynamically then I don't see how this can be added.

Greetings Damien

@TanayParikh
Copy link
Contributor Author

This is cool, when will this be released?

This'll be a part of .NET 7.

Is there a way to add a CSP nonce to all the Blazor js scripts after this change?

The purpose of this change is specifically to facilitate removing the inline JS script hash from the Blazor WASM CSP.

I think you may be interested in #6001 for your use case. Feel free to leave a thumbs up and/or a comment in that issue with your details 😄

@ghost ghost locked as resolved and limited conversation to collaborators Feb 11, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@pavelsavara pavelsavara

Labels
area-blazor Includes: Blazor, Razor Components cost: S Will take up to 2 days to complete feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly task
Projects
None yet
Milestone
7.0-preview1
Development

Successfully merging a pull request may close this issue.

[wasm] dotnet.js modularization pavelsavara/aspnetcore
6 participants
@pavelsavara @SteveSandersonMS @guardrex @damienbod @TanayParikh @mkArtakMSFT