From 53a18d12949029f365d7de7d24d12e61333513a1 Mon Sep 17 00:00:00 2001 From: Brian Wolff Date: Mon, 2 Jul 2018 06:52:51 +0000 Subject: [PATCH] CSP: Allow an option of disabling nonces The current rollout plan calls for initial rollout to only disallow external JS, and leave removing unsafe inline stuff to a later date. Thus this adds a useNonces option to the CSP config to allow that. Renamed ContentSecurityPolicy::isEnabled() to isNonceRequired for clarity. The old name has never been in a released version of MediaWiki, so is removed immediately. Change-Id: I756d8e97b77c6f97dbbf040a20c8750fecb157c5 --- includes/ContentSecurityPolicy.php | 30 +++++++++++++++---- includes/DefaultSettings.php | 2 ++ includes/Html.php | 4 +-- includes/OutputPage.php | 2 +- .../includes/ContentSecurityPolicyTest.php | 13 ++++++-- 5 files changed, 40 insertions(+), 11 deletions(-) diff --git a/includes/ContentSecurityPolicy.php b/includes/ContentSecurityPolicy.php index 66a3535ee8..91117f4e36 100644 --- a/includes/ContentSecurityPolicy.php +++ b/includes/ContentSecurityPolicy.php @@ -171,7 +171,6 @@ class ContentSecurityPolicy { $additionalSelfUrls = $this->getAdditionalSelfUrls(); $additionalSelfUrlsScript = $this->getAdditionalSelfUrlsScript(); - $nonceSrc = "'nonce-" . $this->nonce . "'"; // If no default-src is sent at all, it // seems browsers (or at least some), interpret @@ -183,7 +182,11 @@ class ContentSecurityPolicy { $cssSrc = false; $imgSrc = false; $scriptSrc = [ "'unsafe-eval'", "'self'" ]; - if ( $mode !== self::FULL_MODE_RESTRICTED ) { + if ( + $mode !== self::FULL_MODE_RESTRICTED && + ( !isset( $policyConfig['useNonces'] ) || $policyConfig['useNonces'] ) + ) { + $nonceSrc = "'nonce-" . $this->nonce . "'"; $scriptSrc[] = $nonceSrc; } $scriptSrc = array_merge( $scriptSrc, $additionalSelfUrlsScript ); @@ -518,13 +521,28 @@ class ContentSecurityPolicy { } /** - * Is CSP currently enabled (i.e. Should we set nonce attribute) + * Should we set nonce attribute * * @param Config $config Configuration object * @return bool */ - public static function isEnabled( Config $config ) { - return $config->get( 'CSPHeader' ) !== false - || $config->get( 'CSPReportOnlyHeader' ) !== false; + public static function isNonceRequired( Config $config ) { + $configs = [ + $config->get( 'CSPHeader' ), + $config->get( 'CSPReportOnlyHeader' ) + ]; + foreach ( $configs as $headerConfig ) { + if ( + $headerConfig === true || + ( is_array( $headerConfig ) && + !isset( $headerConfig['useNonces'] ) ) || + ( is_array( $headerConfig ) && + isset( $headerConfig['useNonces'] ) && + $headerConfig['useNonces'] ) + ) { + return true; + } + } + return false; } } diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index f3bc9cce0a..2fa3b7281a 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -8759,6 +8759,8 @@ $wgMaxJobDBWriteDuration = false; * $wgCrossSiteAJAXdomains as an allowed load sources. * 'unsafeFallback' Add unsafe-inline as a script source, as a fallback for * browsers that do not understand nonce-sources [default on]. + * 'useNonces' Require nonces on all inline scripts. If disabled and 'unsafeFallback' + * is on, then all inline scripts will be allowed [default true]. * 'script-src' Array of additional places that are allowed to have JS be loaded from. * 'report-uri' true to use MW api [default], false to disable, string for alternate uri * @warning May cause slowness on windows due to slow random number generator. diff --git a/includes/Html.php b/includes/Html.php index 3dd21c113d..ad0130bf8f 100644 --- a/includes/Html.php +++ b/includes/Html.php @@ -565,7 +565,7 @@ class Html { if ( $nonce !== null ) { $attrs['nonce'] = $nonce; } else { - if ( ContentSecurityPolicy::isEnabled( RequestContext::getMain()->getConfig() ) ) { + if ( ContentSecurityPolicy::isNonceRequired( RequestContext::getMain()->getConfig() ) ) { wfWarn( "no nonce set on script. CSP will break it" ); } } @@ -590,7 +590,7 @@ class Html { if ( $nonce !== null ) { $attrs['nonce'] = $nonce; } else { - if ( ContentSecurityPolicy::isEnabled( RequestContext::getMain()->getConfig() ) ) { + if ( ContentSecurityPolicy::isNonceRequired( RequestContext::getMain()->getConfig() ) ) { wfWarn( "no nonce set on script. CSP will break it" ); } } diff --git a/includes/OutputPage.php b/includes/OutputPage.php index 3b4b14a34d..948e1ebcb6 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -4011,7 +4011,7 @@ class OutputPage extends ContextSource { * @since 1.32 */ public function getCSPNonce() { - if ( !ContentSecurityPolicy::isEnabled( $this->getConfig() ) ) { + if ( !ContentSecurityPolicy::isNonceRequired( $this->getConfig() ) ) { return false; } if ( $this->CSPNonce === null ) { diff --git a/tests/phpunit/includes/ContentSecurityPolicyTest.php b/tests/phpunit/includes/ContentSecurityPolicyTest.php index 3f24030dce..c383be6ff2 100644 --- a/tests/phpunit/includes/ContentSecurityPolicyTest.php +++ b/tests/phpunit/includes/ContentSecurityPolicyTest.php @@ -92,6 +92,12 @@ class ContentSecurityPolicyTest extends MediaWikiTestCase { // @codingStandardsIgnoreStart Generic.Files.LineLength return [ [ false, '', '', '' ], + [ + [ 'useNonces' => false ], + "script-src 'unsafe-eval' 'self' 'unsafe-inline' sister-site.somewhere.com *.wikipedia.org; default-src * data: blob:; style-src * data: blob: 'unsafe-inline'; report-uri /w/api.php?action=cspreport&format=json&", + "script-src 'unsafe-eval' 'self' 'unsafe-inline' sister-site.somewhere.com *.wikipedia.org; default-src * data: blob:; style-src * data: blob: 'unsafe-inline'; report-uri /w/api.php?action=cspreport&format=json&reportonly=1&", + "script-src 'unsafe-eval' 'self' sister-site.somewhere.com *.wikipedia.org; default-src * data: blob:; style-src * data: blob: 'unsafe-inline'" + ], [ true, "script-src 'unsafe-eval' 'self' 'nonce-secret' 'unsafe-inline' sister-site.somewhere.com *.wikipedia.org; default-src * data: blob:; style-src * data: blob: 'unsafe-inline'; report-uri /w/api.php?action=cspreport&format=json&", @@ -283,14 +289,14 @@ class ContentSecurityPolicyTest extends MediaWikiTestCase { /** * @dataProvider providerCSPIsEnabled - * @covers ContentSecurityPolicy::isEnabled + * @covers ContentSecurityPolicy::isNonceRequired */ public function testCSPIsEnabled( $main, $reportOnly, $expected ) { global $wgCSPReportOnlyHeader, $wgCSPHeader; global $wgCSPHeader; $oldReport = wfSetVar( $wgCSPReportOnlyHeader, $reportOnly ); $oldMain = wfSetVar( $wgCSPHeader, $main ); - $res = ContentSecurityPolicy::isEnabled( RequestContext::getMain()->getConfig() ); + $res = ContentSecurityPolicy::isNonceRequired( RequestContext::getMain()->getConfig() ); wfSetVar( $wgCSPReportOnlyHeader, $oldReport ); wfSetVar( $wgCSPHeader, $oldMain ); $this->assertEquals( $res, $expected ); @@ -305,6 +311,9 @@ class ContentSecurityPolicyTest extends MediaWikiTestCase { [ false, [], true ], [ [], false, true ], [ [ 'default-src' => [ 'foo.example.com' ] ], false, true ], + [ [ 'useNonces' => false ], [ 'useNonces' => false ], false ], + [ [ 'useNonces' => true ], [ 'useNonces' => false ], true ], + [ [ 'useNonces' => false ], [ 'useNonces' => true ], true ], ]; } } -- 2.20.1