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

feat(ssr): add csp nonce to all elements #11826

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

blake-newman
Copy link
Member

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

Other information:

CSPv3 allows simple nonce based policies and directives such as stict-dynamic. Declaring a policy such as:

Content-Security-Policy:
  object-src 'none';
  script-src 'nonce-{random}' 'unsafe-inline' 'unsafe-eval' 'strict-dynamic' https: http:;
  base-uri 'none';
  report-uri https://your-report-collector.example.com/

Would not work with current nonce support with features such as resource hints. This policy creates errors such as

Refused to load the script 'http://localhost:8082/manifest.js' because it violates the following Content Security Policy directive: "script-src 'self' 'nonce-68f9bed4d31fcde221e7b5e871860ff2' 'unsafe-inline' 'unsafe-eval' 'strict-dynamic' http:". 'strict-dynamic' is present, so host-based allowlisting is disabled. Note that 'script-src-elem' was not explicitly set, so 'script-src' is used as a fallback.

This is because not all rendered elements have an associated nonce.

To support stricter policies that only work scripts / resource hinting add nonce attribute to all element that could be affected via a nonce based policy.

@posva posva added the feat:ssr label Dec 16, 2020
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@blake-newman
Copy link
Member Author

@posva is this good to merge - could be useful for many people. I forgot about this PR myself as we forked and patched the change.

@eliasjtg
Copy link

any update?

@blake-newman
Copy link
Member Author

@posva @yyx990803

Is it possible to merge this as part of 2.7 was planning to upgrade to this version and realised this PR was still left open. My bad should have nudged this sooner

@blake-newman blake-newman force-pushed the blake.newman/improve-csp-nonce-support branch from 8e40316 to 8df91b8 Compare November 28, 2022 14:36
add csp nonce to all elements that could potentiall be affected by CSP directives
@blake-newman blake-newman force-pushed the blake.newman/improve-csp-nonce-support branch from 8df91b8 to 5449d9e Compare November 28, 2022 14:48
@blake-newman blake-newman changed the base branch from dev to main November 28, 2022 14:49
@blake-newman
Copy link
Member Author

@yyx990803 i've updated this to the main branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants