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

Strict CSP-compatibility: replace javascript: URIs with event handlers #2823

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

enricocarraro
Copy link

Description of Changes:

  • Refactor of javascript: URIs generated by buildvolume in src/js/features/volume.js to enable strict CSP-compatibility.

Why:
Content Security Policy is a mechanism designed to make applications more secure against common web vulnerabilities, particularly cross-site scripting. It is enabled by setting the Content-Security-Policy HTTP response header.
An application can add a critical defense-in-depth layer against markup injection attacks by adopting a strict policy that prevents the loading of untrusted scripts or plugins.
To make an application compatible with strict CSP, it is necessary to make changes to HTML templates and client-side code and add the policy header:

  1. Add nonces to <script> elements;
  2. Refactor inline event handlers and javascript: URIs;
  3. Refactor calls to JS APIs incompatible with CSP;
  4. Serve the Content-Security-Policy header.

More on CSP.

This PR is related to a series of efforts to make WordPress strict CSP compatible, more on that here.

@enricocarraro enricocarraro changed the title Replace javascript: URIs with event handlers. Strict CSP-compatibility: replace javascript: URIs with event handlers Oct 8, 2020
@coveralls
Copy link

coveralls commented Oct 8, 2020

Coverage Status

Coverage remained the same at 74.376% when pulling 16a421e on enricocarraro:move_javascript_URIs into d8a667f on mediaelement:master.

src/js/features/volume.js Outdated Show resolved Hide resolved
@jaapmarcus
Copy link
Contributor

This PR is also related to #2729

How ever "there" are some checks need to made if any "scripts" could cause issues..

@enricocarraro
Copy link
Author

enricocarraro commented Oct 9, 2020

@jaapmarcus I didn't find any other incompatibilities that could break default behavior with strict CSP enabled without the 'unsafe-inline' option. The library doesn't generate any HTML that includes inline event handlers, doesn't use JS APIs incompatible with CSP, and doesn't write <script> tags.
Being more practical, a policy like this could work just fine when this PR gets merged:

Content-Security-Policy:
  script-src 'nonce-{random}' 'unsafe-inline' 'unsafe-eval' 'strict-dynamic' https: http:;

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

Successfully merging this pull request may close these issues.

None yet

4 participants