Follow-up to #2510104: Convert drupalSettings from JavaScript to JSON, to allow for CSP in the future

Problem/Motivation

Content security policy is a browser feature available that helps prevent XSS attacks based on headers sent by the site.

For CSP spec see: http://www.w3.org/TR/CSP/
https://www.owasp.org/index.php/Content_Security_Policy
https://www.owasp.org/index.php/Clickjacking_Defense_Cheat_Sheet

Inline JS is not compatible with enabling a reasonable secure content security policy, so this issue is postponed until the Drupal settings are fixed in the related issue

Proposed resolution

Implement a basic and reasonably secure CSP header for Drupal core, such as

Content-Security-Policy: default-src 'self'; frame-ancestors 'self';

Possibly (or as a follow-up or in contrib): Add a callback to receive and log CSP violation reports to watchdog. e.g. with CSP report-uri directive like:

Content-Security-Policy: default-src 'self'; frame-ancestors 'self'; report-uri /system/csp-report-logger;

Likely this reporting should be supported only as a something that can be temporarily enabled for debugging. It has obvious potential for abuse (DoS attacks, bogus data, etc) such as outlined at https://www.virtuesecurity.com/blog/abusing-csp-violation-reporting/

Remaining tasks

Implement

User interface changes

Possibly an admin page to configure some aspects of the CSP (optional for 8.0.x)

API changes

API addition to allow modules to alter or add to the CSP header for each page/or response event.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because it is security hardening.
Issue priority Major because CSP is an important security practice.
Prioritized changes The main goal of this issue are security improvements.
Disruption Likely not disruptive for contributed modules - unless they embed iframe or load 3rd party JS.
CommentFileSizeAuthor
#37 2513356-nr-bot.txt1.63 KBneeds-review-queue-bot

Issue fork drupal-2513356

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pwolanin’s picture

Title: Add a default CSP and minimal API to core » Add a default CSP and clickjacking defence and minimal API for CSP to core
Issue summary: View changes
pwolanin’s picture

Issue summary: View changes
pwolanin’s picture

Issue summary: View changes
Fabianx’s picture

Nice!

Some questions:

- Can we via our libraries mechanism automatically emit headers for external JS added via that way?
- Is it possible to also restrict directories? Given we know all directories due to aggregation could we restrict those directories?

pwolanin’s picture

From discussion with webchick, having a CSP in core that restricts JS execution should be 8.1.x material.

Not sure about X-Frame-Options: SAMEORIGIN however

Fabianx’s picture

Even if we make it an optional hardening?

pwolanin’s picture

Version: 8.0.x-dev » 8.1.x-dev
Issue summary: View changes

yes, optional == contrib. I think for 8.0.x

Fabianx’s picture

Version: 8.1.x-dev » 8.0.x-dev
Status: Postponed » Active

Well, core needs to support the API from day 0 - even if then only the contrib module enables it.

Adding at 8.1 will potentially break contrib modules and it is one of the most important security hardenings we can do.

It is also just 'ready' now. And the web is evolving ...

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Wim Leers’s picture

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

gapple’s picture

Core currently still requires 'unsafe-inline' when CSS aggregation is disabled due to #2897408: Remove IE9 support from CssCollectionRenderer and provide it in contrib instead.

--

I've started working on a contrib module to provide automated CSP headers at https://www.drupal.org/project/csp
Currently it parses defined libraries to add a Content-Security-Policy-Report-Only header with the necessary domains for script-src and style-src. It doesn't actually report anywhere yet other than the browser console, but a built-in report endpoint is also in the works.
#2895245: API for modules to alter policy considers adding an additional attribute to library definitions to support additional policies such as connect-src.

--

@Fabianx: "Is it possible to also restrict directories? Given we know all directories due to aggregation could we restrict those directories?"

Policies can only specify domains, not directories, but I have another experiment where I used Apache rewrite rules to establish separate subdomains restricted to their responsibilities, altered URIs to their respective subdomains, and applied the policy:

default-src 'self'; script-src assets.example.com; style-src assets.example.com; img-src assets.example.com files.example.com; connect-src 'self';
gapple’s picture

Version: 8.4.x-dev » 8.5.x-dev
Anybody’s picture

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

gapple’s picture

I reopened #2789139: [upstream] CSP requires 'unsafe-inline' because of CKEditor 4, since CKEditor still requires 'unsafe-inline'. The improvements in CKEditor 4.7 only removed the need for 'unsafe-eval'.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

gapple’s picture

Priority: Major » Normal

Any documentation or articles I've seen so far only uses domains, and I haven't tested the state of browser support, but the spec does allow restricting by directory (CSP Level 3 - 6.6.2.10. path-part matching).

So this would theoretically be possible:

  default-src 'self'; 
  script-src example.com/core/ example.com/modules/ example.com/themes/ example.com/profiles/ example.com/sites/default/files/js/; 
  style-src example.com/core/ example.com/modules/ example.com/themes/ example.com/profiles/ example.com/sites/default/files/css/; 
gapple’s picture

CSP module has seen significant improvements, and has a stable release now.

I've proposed using an additional property in library definitions to enable support for additional directives: #2895245: API for modules to alter policy

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mxr576’s picture

Issue tags: -JavaScript +JavaScript

CKEditor has been removed from core, CKEditor 4 is removed from Drupal Core in 10.0.0

#2789139: [upstream] CSP requires 'unsafe-inline' because of CKEditor 4 comment #30

Is that unblock this issue?

gapple’s picture

A remaining issue is that the core/drupal.ajax library still requires style-src 'unsafe-inline': #3110517: Improve Drupal\Core\Ajax\AddCssCommand to accept an array of CSS assets

----

The CSP module's default enforced policy is currently quite lenient (frame-ancestors 'self'; object-src 'none'), because it's reasonably likely that something like default-src 'self' will block expected functionality and has the potential to DDOS your own site with the default of logging violation reports to the site log (if not properly configured in a dev environment before deploying to an active site).
The default report-only policy is stricter (adding style-src and script-src directives, and will include any relevant domains from library definitions), but also only reports to the browser console in anticipation of the potential initial violations.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

bkosborne’s picture

I guess the X-Frame-Options header that core adds via FinishResponseSubscriber should also be removed when this is done. The "frame-ancestors" directive in the CSP header is well supported and obsoletes X-Frame-Options.

gapple’s picture

With IE no longer being supported, core could replace its default X-Frame-Options: SAMEORIGIN with Content-Security-Policy: frame-ancestors 'self'. Starting to always add a default CSP header could cause problems for a site that sets X-Frame-Options: DENY though. (It looks like IE was the only browser to support ALLOW-FROM).

A possible transition is:
[11.1] Add a late-subscriber that translates x-frame-options to a CSP frame-ancestors if the response does not have a CSP header (assume if a CSP header is set but does not include frame-ancestors, then it was intended to allow any frame parent). Issue a deprecation warning if the response has a value other than X-Frame-Options: SAMEORIGIN to notify users to use CSP instead.
[12.0] Remove the late subscriber, and replace X-Frame-Options with a static CSP policy that includes frame-ancestors 'self'. A site that uses the Content-Security-Policy module, SecKit, or a custom listener to set the CSP header will override this default value.

My suggestion for a minimal static policy that core can add by default:

  Content-Security-Policy: script-src * 'unsafe-inline'; object-src 'none'; frame-ancestors 'self'

The simplest way to allow modifying the default CSP (or adding a CSP-Report-Only), would be to have a container parameter. Users wanting a more flexible implementation can use the CSP module.

gapple’s picture

Status: Active » Needs review

A slightly rough MR:
- Introduce a new service parameter for setting static CSP values
- A late acting response subscriber will translate X-Frame-Options to Content-Secutity-Policy: frame-ancestors
- If a CSP policy is set (via service parameter, or another module like CSP or seckit), then that value is not changed.
- The X-Frame-Options header is always removed - either it's being replaced by core with an equivalent CSP policy, or we assume that the CSP policy set by the user has their desired frame-ancestors value (including the option of being omitted).
- If X-Frame-Options is not set to SAMEORIGIN, then a deprecation warning is issued to use CSP to set the value. (If the value is still the default SAMEORIGIN, then a future version of Drupal changing to frame-ancestors 'self' will not change browser behaviour).
- ALLOW-FROM is ignored by modern browsers (and equivalent to not sending the X-Frame-Options header), but is translated to a CSP value if used.

In a future version of Drupal:
- The service parameter can be set to a default enforced CSP value
- The late acting event subscriber can be completely removed
- Core can stop setting X-Frame-Options
- Modules (such as CSP or seckit) will still override core's default (now static) value.

Why the CSP policy value:
- script-src * 'unsafe-inline' will only block eval(). Not including 'unsafe-inline' would have the potential to break existing sites (and hopefully its presence leads people to explore making sure it's not required...)
- object-src 'none' is recommended to block legacy HTML elements
- frame-ancestors replaces X-Frame-Options
- I don't think there's other values that can safely be added as a default enforced policy directive without a reasonable risk of negatively affecting some sites. (maybe base-uri 'self'?)

needs-review-queue-bot’s picture

Status: Needs review » Needs work
FileSize
1.63 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.