Remove inline event handlers and JavaScript URIs for Strict CSP-compatibility

[enricocarraro]

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.
A basic policy (nonce + strict-dynamic + unsafe-eval) would block more than ​40% of the XSS sinks.

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.

Since these PRs are quite significant, I separated the one regarding script nonces (#39941) and the one on inline event handlers and JavaScript URIs to facilitate code reviews.
This patch builds on top of ​this one (use ​this link to compare the two branches easily); I introduced some new JavaScript files where I moved inline event handlers and JavaScript URIs to, in particular:

• src/js/_enqueues/admin/link-manager.js: handles link deletion confirmation;
• src/js/_enqueues/admin/media-events.js: events related to the media library and media details;
• src/js/_enqueues/admin/setup-config.js: event handler for try again button;
• src/js/_enqueues/admin/themes-list.js: theme deletion confirm event handler;
• src/js/_enqueues/lib/metabox-events.js: event handlers for various meta boxes.

I also moved various event handlers to already existing JavaScript files.
In some cases, I couldn't move the inline event handlers to a file because 'user-defined' ajax calls could load the generated HTML, so I moved them to an inline script.

Trac ticket: https://core.trac.wordpress.org/ticket/51407

[enricocarraro]

As suggested by @adamsilverstein, here's a list of page changes:

1. Invalid Comment ID 'Go Back' button and MultiSite 'Back' button
   • Originally in src/wp-admin/comment.php] moved to src/js/_enqueues/admin/common.js
   • ​commit
2. Custom Image Header: 'Blog Name' anchor
   • Originally in src/wp-admin/includes/class-custom-image-header.php moved to src/js/_enqueues/admin/custom-header.js
   • ​commit
3. Link Manager: 'Delete' button
   • Originally in src/wp-admin/includes/class-wp-links-list-table.php moved to src/js/_enqueues/admin/link-manager.js
   • ​commit
4. Link details: 'Delete' button
   • Originally in src/wp-admin/includes/meta-boxes.php moved to src/js/_enqueues/admin/link.js
   • ​commit
5. Media : 'Delete' button, 'Edit Image' button, 'Cancel' delete button, Use Featured Image, 'Cancel' button, 'Insert gallery', 'Update gallery settings'
   • Originally in src/wp-admin/includes/media.php moved to src/js/_enqueues/admin/media-events.js
   • ​commit
6. Media Insert URL Form: src 4 radio inputs for alignment, "Go", "Link to image" and "None" buttons
   • Originally in src/wp-admin/includes/media.php moved to src/js/_enqueues/admin/media-events.js
   • ​same as above
7. Media Library List View: 'Delete' and 'Attach' button
   • Originally in src/wp-admin/includes/class-wp-media-list-table.php moved to src/js/_enqueues/admin/media.js
   • ​commit
8. Advanced Edit form: "Get shortlink"
   • Originally in src/wp-admin/edit-form-advanced.php moved to src/js/_enqueues/admin/post.js
   • ​commit
9. Setup Config: "Go back" button on config failure
   • Originally in src/wp-admin/setup-config.php moved to src/js/_enqueues/admin/setup-config.js
   • ​commit
10. Theme List: Delete button
    • Originally in src/wp-admin/includes/class-wp-themes-list-table.php moved to src/js/_enqueues/admin/themes-list.js
    • ​commit
11. Image Edit: help toggles, text fields for crop and scale and buttons for Preview, Crop, Rotate, Flip, Undo, Redo, Cancel, Submit and Scale
    • Originally in src/wp-admin/includes/image-edit.php moved to src/js/_enqueues/lib/image-edit.js
    • ​commit
12. Block Editor: metabox location form; Media Details : "Delete" button, "Add new comment" button, "Reply" button, "Show comments" button
    • Originally in src/wp-admin/includes/post.php and src/js/_enqueues/lib/metabox-events.js moved to src/js/_enqueues/lib/metabox-events.js
    • ​commit
13. Dashboard: reply to comment button
    • Originally in src/wp-admin/includes/dashboard.php and src/js/_enqueues/admin/edit-comments.js
    • ​commit
14. Customize Menu view: "widget areas" and "Widgets panel" button
    • Originally in src/wp-includes/class-wp-customize-nav-menus.php moved to src/js/_enqueues/wp/customize/nav-menus.js
    • ​commit
15. Customize Widgets: Nav Menu widget "Create some" button when no menus exist
    • Originally in src/wp-includes/widgets/class-wp-nav-menu-widget.php moved to src/js/_enqueues/wp/customize/widgets.js
    • ​commit
16. Theme and Plugin Editor: "Docs Lookup" button
    • Originally in src/wp-admin/plugin-editor.php and src/wp-admin/theme-editor.php
    • ​commit
17. Async Upload: "Dismiss" button
    • Target src/wp-admin/async-upload.php
    • ​commit
18. Theme List Install: "Try Again" Button
    • Target: src/wp-admin/includes/class-wp-theme-install-list-table.php
    • ​commit
19. Template Meta Form: "Enter" new
    • Target: src/wp-admin/includes/template.php
    • ​commit
20. Comment Template: "Reply" button
    • Target: src/wp-includes/comment-template.php
    • ​commit
21. Default wp_die handler: "Go back" button
    • Target: src/wp-includes/functions.php
    • ​commit
22. ThickBox: iframe loading
    • Target: src/js/_enqueues/vendor/thickbox/thickbox.js
    • ​commit

List of efforts on WordPress JavaScript dependencies:

• TinyMCE compat3x plugin: ​PR
• MediaElement volume: ​PR

[enricocarraro]

@whyisjake Do you know anybody that could help in the review process? I would appreciate some more eyes on it since there isn't much time until the 5.6 beta phase.
I would also like to stress how important strict CSP would be in protecting WordPress users.

[hellofromTonya]

This ticket does not seem to be directly caused by Version 5.6 (ie trunk). It does reference being built upon #39941, which has a Version of 4.8. Changing the version to match that ticket.

However, if the version is different, please advise or update.

[jornfranke]

I think a better way would be not to use nonces (too complex to configure on the web server side etc.). I recommend to make all Javascript strictly as external files. Then, I dont need to configure the nounces in the headers.

Othrwise I strongly support strict CSP. The new block editor is a security disaster:
unsafe-inline, unsafe-eval, external references to google fonts...

A good out of the box WordPress installation must work with the following CSP for all areas (especially the admin area):

Content-Security-Policy: default-src 'none'; base-uri 'self'; script-src 'self'; style-src 'self'; img-src 'self'; connect-src 'self'; font-src 'self'; object-src 'none'; media-src 'none'; child-src 'self'; form-action 'self'; frame-ancestors 'none'; navigate-to 'self'; block-all-mixed-content

[hellofromTonya]

Notes from last scrub:

@whyisjake raised his hand to review for 2nd opinion.

@adamsilverstein thought there's still time for this ticket to land in 5.7 Beta 1.

[hellofromTonya]

5.7 Beta 1 is happening shortly. Ran out of time to get this ticket into the release. Punting to 5.8.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

[poena]

Punting to 5.9.

[adamsilverstein]

Marking as future release for now and will look for opportunities to circle back to this work.

[jornfranke]

I agree and strongly support that all Javascript as well as CSS should be strictly external for higher security with CSP.

[wildturtles]

agree with @jornfranke not making it lead bad dev/sys admin to do ugly stuff like remove the inline source code.

Any news about that achievment ?

Any things about good practices in dev advices (core, plugins)

[jornfranke]

Hi,

Gutenberg did some fix related to CSP:
​https://github.com/WordPress/gutenberg/issues/42513

Do you have an idea when this could be included?

Thank you.

Best regards

[westonruter]

In 62723:

Script Loader: Enable attribute filters for concatenated scripts.

Replace the manually constructed <script> markup in _print_scripts() with wp_print_inline_script_tag() and wp_print_script_tag(). The merged inline script holding the concatenated wp_localize_script() data, and the load-scripts.php tag itself, are now passed through the wp_inline_script_attributes and wp_script_attributes filters, as every other script tag in core already is. Themes and plugins can therefore add attributes to them, such as the nonce needed to serve the admin under a Content-Security-Policy that disallows unsafe-inline.

The emitted markup is otherwise unchanged, aside from the attribute quoting and entity encoding applied by WP_HTML_Tag_Processor.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/11702.
Follow-up to r56687, r60719.

Props sukhendu2002, galaxor, wildworks, westonruter.
See #51407, #57548, #58664, #59446.
Fixes #64683.