diff --git a/README.md b/README.md index 396629f..976ae2b 100644 --- a/README.md +++ b/README.md @@ -57,7 +57,11 @@ public function configure(): void ``` _Usually you'll define `private const FRAGMENTS = []` and add them in there so it's clear at the beginning what fragments you're adding._ -To set the **report to**, we usually use an env var named `CSP_REPORT_TO`. You can also call `$this->reportTo()` in your policies configure func if required (perhaps you want the report URI based on the policy applied). +To set the **report to** url, we usually use an env var named `CSP_REPORT_TO`. The expiry time can also be set using `CSP_REPORT_TO_TTL` this tells the browser how long it should remember the url for. + +You can also call `$this->reportTo()` in your policies configure func if required (perhaps you want the report URI based on the policy applied). + +Reporting can be sent to multiple urls if required, `CSP_REPORT_TO` supports CSV, or the directive can be used with an array. To add the policy to the list of applied policies you'll want to add some yaml config: ```yaml diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 741927a..922ea20 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,5 +1,5 @@ - + src/ diff --git a/src/Policies/Policy.php b/src/Policies/Policy.php index 0dd0513..4f9ccb4 100644 --- a/src/Policies/Policy.php +++ b/src/Policies/Policy.php @@ -84,9 +84,28 @@ public function enforce(): self return $this; } + /** + * Add reporting directives to the policy, so that violations can be sent to + * the uri defined as CSP_REPORT_TO in the environment. + * + * @param string $uri - the uri to send the reports to, or empty to remove reporting + * @return self + */ public function reportTo(string $uri): self { + // if the string is empty, we can assume we need to _remove_ reporting + if (empty($uri)) { + unset($this->directives[Directive::REPORT]); + unset($this->directives[Directive::REPORT_TO]); + + return $this; + } + + // Add the report-uri directive - this is deprecated, but still supported by most browsers $this->directives[Directive::REPORT] = [$uri]; + + // Add the report-to directive - this is the new standard, but not yet supported by all browsers + // the syntax for this will be fixed when the header is added $this->directives[Directive::REPORT_TO] = [$uri]; return $this; @@ -108,6 +127,12 @@ public function addNonceForDirective(string $directive): self ); } + /** + * Apply the CSP header to the response + * + * @param HTTPResponse $response + * @return void + */ public function applyTo(HTTPResponse $response) { $this->configure(); @@ -122,10 +147,8 @@ public function applyTo(HTTPResponse $response) return; } - $reportTo = Environment::getEnv('CSP_REPORT_TO'); - if (!array_key_exists(Directive::REPORT, $this->directives) && $reportTo) { - $this->reportTo($reportTo); - } + // optionally add reporting directives + $this->applyReporting($response); $response->addHeader($headerName, (string) $this); $response->addHeader('csp-name', ClassInfo::shortName(static::class)); @@ -143,7 +166,34 @@ public function __toString() : "{$directive} {$valueString}"; } - return implode(';', $directives); + return implode('; ', $directives); + } + + /* + * Takes an array of `Fragment` implementations and adds them to the policy + */ + public function addFragments(array $fragments): self + { + foreach ($fragments as $fragment) { + call_user_func_array([$fragment, 'addTo'], [$this]); + } + + return $this; + } + + /** + * If the given value is not an array and not null, wrap it in one. + * + * @param mixed $value + * @return array + */ + public static function wrap($value): array + { + if (is_null($value)) { + return []; + } + + return is_array($value) ? $value : [$value]; } protected function guardAgainstInvalidDirectives(string $directive) @@ -196,30 +246,112 @@ protected function sanitizeValue(string $value): string return $value; } - /* - * Takes an array of `Fragment` implementations and adds them to the policy + /** + * Add the reporting directives to the policy if the address is set + * as an environment variable. + * + * @param HTTPResponse $response - the response to add the header to + * @return void */ - public function addFragments(array $fragments): self + private function applyReporting(HTTPResponse $response): void { - foreach ($fragments as $fragment) { - call_user_func_array([$fragment, 'addTo'], [$this]); + $reportTo = Environment::getEnv('CSP_REPORT_TO'); + + $hasEnvironmentVariable = !is_null($reportTo) && $reportTo !== false; + + // if we have the environment variable, assume we want both directives + if ($hasEnvironmentVariable) { + $hasMultipleUrls = str_contains($reportTo, ','); + + // if we are handling multiple urls we need to only add a single directive + if ($hasMultipleUrls) { + $reportToArray = explode(',', $reportTo); + $this->directives[Directive::REPORT_TO] = $reportToArray; + $this->applyReportTo($response); + return; + } + + // otherwise add both + $this->reportTo($reportTo); + $this->applyReportTo($response); + return; } - return $this; + // if we don't have the environment variable, + // check if we have the directives manually set + $hasReportDirective = array_key_exists(Directive::REPORT, $this->directives); + $hasReportToDirective = array_key_exists(Directive::REPORT_TO, $this->directives); + + // no directives, no further processing needed + if (!$hasReportDirective && !$hasReportToDirective) { + return; + } + + // if the report-to directive is set, we need to add the header and process the value + if ($hasReportToDirective) { + $this->applyReportTo($response); + return; + } } /** - * If the given value is not an array and not null, wrap it in one. + * Add the necessary extras for the report-to directive * - * @param mixed $value - * @return array + * @param HTTPResponse $response - the response to add the header to + * @return void */ - public static function wrap($value): array + private function applyReportTo(HTTPResponse $response): void { - if (is_null($value)) { - return []; + $hasReportToDirective = array_key_exists(Directive::REPORT_TO, $this->directives); + + // if the environment variable is not set, and the directive is not set, we can't add the header + if (!$hasReportToDirective) { + return; } - return is_array($value) ? $value : [$value]; + // get the directive value + $reportTo = $this->directives[Directive::REPORT_TO]; + + // if the directive is not set, we can't add the header + if (is_null($reportTo) || $reportTo === false || $reportTo === '') { + return; + } + + $endpoints = []; + foreach ($reportTo as $uri) { + // tidy up + $uri = trim($uri); + + // if the value is not a url, we can't add the header + if (!filter_var($uri, FILTER_VALIDATE_URL)) { + continue; + } + + // if the value is a url, we can use it as the endpoint + $endpoints[] = [ + 'url' => $uri, + ]; + } + + // if we don't have any endpoints, we can't add the header + if (count($endpoints) === 0) { + return; + } + + // set a standard group name to use + $groupName = 'csp-endpoint'; + + // add the group name to the directive, replacing the invalid urls + $this->directives[Directive::REPORT_TO] = [$groupName]; + + // set the amount of time the users-browser should store the endpoint + $ttl = Environment::getEnv('CSP_REPORT_TO_TTL') ?: 10886400; // 126 days + + // add the reponse header + $response->addHeader('Report-To', json_encode([ + 'group' => $groupName, + 'max_age' => $ttl, + 'endpoints' => $endpoints, + ], JSON_UNESCAPED_SLASHES)); } } diff --git a/tests/PolicyTest.php b/tests/PolicyTest.php index b8020fc..0e5c59a 100644 --- a/tests/PolicyTest.php +++ b/tests/PolicyTest.php @@ -32,7 +32,7 @@ public function testBasicPolicyAddsCorrectHeaders(): void $policy->applyTo($response); $nonce = NonceGenerator::get(); $expected = <<assertEquals($expected, $response->getHeader('content-security-policy')); $this->assertEquals('Basic', $response->getHeader('csp-name')); @@ -49,23 +49,352 @@ public function testAdminPolicyWillOnlyBeAddedForAdmin(): void $this->assertFalse($policy->shouldBeApplied($request, $response)); } - public function testAReportURICanBeSet(): void + /** + * Check the reporting endpoint can be set from the environment variable + */ + public function testAReportURICanBeSetFromEnvironmentVariable(): void { [$request, $response] = $this->getRequestResponse(); /** @var Policy $policy */ $policy = Injector::inst()->get(CMS::class); - Environment::setEnv('CSP_REPORT_TO', 'https://example.com'); + $reportTo = 'https://example.com'; + $reportTtl = 1234; + Environment::setEnv('CSP_REPORT_TO', $reportTo); Environment::setEnv('CSP_REPORT_ONLY', 'enabled'); + Environment::setEnv('CSP_REPORT_TO_TTL', $reportTtl); + + // apply the policy + $policy->applyTo($response); + + // check the header + $this->assertNull($response->getHeader('Content-Security-Policy')); + $this->assertNotNull($response->getHeader('Content-Security-Policy-Report-Only')); + + // check the report-uri directive + $this->assertStringContainsString( + sprintf('report-uri %s', $reportTo), + $response->getHeader('Content-Security-Policy-Report-Only') + ); + + // check the report-to directive + $this->assertStringContainsString( + 'report-to csp-endpoint', + $response->getHeader('Content-Security-Policy-Report-Only') + ); + + // check the Report-To header + $this->assertNotNull($response->getHeader('Report-To')); + $this->assertStringContainsString( + sprintf( + '{"group":"csp-endpoint","max_age":%d,"endpoints":[{"url":"%s"}]}', + $reportTtl, + $reportTo + ), + $response->getHeader('Report-To') + ); + } + + /** + * Check the reporting endpoint is not output unless set in the environment variable + * or from code + */ + public function testAReportURICanBeUnsetFromEnvironmentVariable(): void + { + [$request, $response] = $this->getRequestResponse(); + /** @var Policy $policy */ + $policy = Injector::inst()->get(CMS::class); + + // apply the policy + $policy->applyTo($response); + + // check the header + $this->assertNotNull($response->getHeader('Content-Security-Policy')); + $this->assertNull($response->getHeader('Content-Security-Policy-Report-Only')); + + // check the report-uri directive + $this->assertStringNotContainsString( + 'report-uri', + $response->getHeader('Content-Security-Policy') + ); + + // check the report-to directive + $this->assertStringNotContainsString( + 'report-to', + $response->getHeader('Content-Security-Policy') + ); + + // check the Report-To header + $this->assertNull($response->getHeader('Report-To')); + } + + /** + * Check the reportTo() function works as expected + */ + public function testAReportURICanBeSetFromCode(): void + { + [$request, $response] = $this->getRequestResponse(); + /** @var Policy $policy */ + $policy = Injector::inst()->get(CMS::class); + Environment::setEnv('CSP_REPORT_ONLY', 'enabled'); + + // change the policy + $reportTo = 'https://silverstripe.com'; + $policy->reportTo($reportTo); + + // apply the policy $policy->applyTo($response); - $this->assertStringContainsString('report-to https://example.com', $response->getHeader('Content-Security-Policy-Report-Only')); - $this->assertStringContainsString('report-uri https://example.com', $response->getHeader('Content-Security-Policy-Report-Only')); - $policy->reportTo('https://silverstripe.com'); - $response->removeHeader('Content-Security-Policy-Report-Only'); + // check the header + $this->assertNull($response->getHeader('Content-Security-Policy')); + $this->assertNotNull($response->getHeader('Content-Security-Policy-Report-Only')); + + // check the basic report-uri directive + $this->assertStringContainsString( + sprintf('report-uri %s', $reportTo), + $response->getHeader('Content-Security-Policy-Report-Only') + ); + + // check the more advanced report-to directive + $this->assertStringContainsString( + 'report-to csp-endpoint', + $response->getHeader('Content-Security-Policy-Report-Only') + ); + + // check the Report-To header + $this->assertNotNull($response->getHeader('Report-To')); + $this->assertStringContainsString( + sprintf( + '{"group":"csp-endpoint","max_age":10886400,"endpoints":[{"url":"%s"}]}', + $reportTo + ), + $response->getHeader('Report-To') + ); + } + + /** + * Check the reportTo() function works as expected + */ + public function testAReportURICanBeUnsetFromCode(): void + { + [$request, $response] = $this->getRequestResponse(); + /** @var Policy $policy */ + $policy = Injector::inst()->get(CMS::class); + + // change the policy to enable reporting + $reportTo = 'https://silverstripe.com'; + $policy->reportTo($reportTo); + + // now disable it + $policy->reportTo(''); + + // apply the policy $policy->applyTo($response); - $this->assertStringContainsString('report-to https://silverstripe.com', $response->getHeader('Content-Security-Policy-Report-Only')); - $this->assertStringContainsString('report-uri https://silverstripe.com', $response->getHeader('Content-Security-Policy-Report-Only')); + + // check the header + $this->assertNotNull($response->getHeader('Content-Security-Policy')); + $this->assertNull($response->getHeader('Content-Security-Policy-Report-Only')); + + // check the basic report-uri directive + $this->assertStringNotContainsString( + 'report-uri', + $response->getHeader('Content-Security-Policy') + ); + + // check the more advanced report-to directive + $this->assertStringNotContainsString( + 'report-to', + $response->getHeader('Content-Security-Policy') + ); + + // check the Report-To header + $this->assertNull($response->getHeader('Report-To')); + } + + /** + * Check we can set the report-uri, without the report-to directive + */ + public function testAReportURICanBeSetWithoutReportTo(): void + { + [$request, $response] = $this->getRequestResponse(); + /** @var Policy $policy */ + $policy = Injector::inst()->get(CMS::class); + Environment::setEnv('CSP_REPORT_ONLY', 'enabled'); + + // change the policy + $reportTo = 'https://silverstripe.com'; + $policy->addDirective(Directive::REPORT, $reportTo); + + // apply the policy + $policy->applyTo($response); + + // check the header + $this->assertNull($response->getHeader('Content-Security-Policy')); + $this->assertNotNull($response->getHeader('Content-Security-Policy-Report-Only')); + + // check the basic report-uri directive + $this->assertStringContainsString( + sprintf('report-uri %s', $reportTo), + $response->getHeader('Content-Security-Policy-Report-Only') + ); + + // check the more advanced report-to directive + $this->assertStringNotContainsString( + 'report-to', + $response->getHeader('Content-Security-Policy-Report-Only') + ); + + // check the Report-To header + $this->assertNull($response->getHeader('Report-To')); + } + + /** + * Check we can set the report-uri, without the report-to directive + */ + public function testAReportToCanBeSetWithoutReportURI(): void + { + [$request, $response] = $this->getRequestResponse(); + /** @var Policy $policy */ + $policy = Injector::inst()->get(CMS::class); + + // change the policy + $reportTo = 'https://silverstripe.com'; + $policy->addDirective(Directive::REPORT_TO, $reportTo); + + // apply the policy + $policy->applyTo($response); + + // check the header + $this->assertNotNull($response->getHeader('Content-Security-Policy')); + $this->assertNull($response->getHeader('Content-Security-Policy-Report-Only')); + + // check the basic report-uri directive + $this->assertStringNotContainsString( + 'report-uri', + $response->getHeader('Content-Security-Policy') + ); + + // check the more advanced report-to directive + $this->assertStringContainsString( + 'report-to csp-endpoint', + $response->getHeader('Content-Security-Policy') + ); + + // check the Report-To header + $this->assertNotNull($response->getHeader('Report-To')); + $this->assertStringContainsString( + sprintf( + '{"group":"csp-endpoint","max_age":10886400,"endpoints":[{"url":"%s"}]}', + $reportTo + ), + $response->getHeader('Report-To') + ); + } + + /** + * Check the reporting endpoint can be set from the environment variable + */ + public function testMultipleReportURICanBeSetFromEnvironmentVariable(): void + { + [$request, $response] = $this->getRequestResponse(); + /** @var Policy $policy */ + $policy = Injector::inst()->get(CMS::class); + + $reportTo = 'https://example.com,http://example.org,https://example.net'; + Environment::setEnv('CSP_REPORT_TO', $reportTo); + Environment::setEnv('CSP_REPORT_ONLY', 'enabled'); + + // apply the policy + $policy->applyTo($response); + + // check the header + $this->assertNull($response->getHeader('Content-Security-Policy')); + $this->assertNotNull($response->getHeader('Content-Security-Policy-Report-Only')); + + // the report-uri directive only supports a single address, + // so we should not expect to see it + $this->assertStringNotContainsString( + 'report-uri', + $response->getHeader('Content-Security-Policy-Report-Only') + ); + + // check the report-to directive + $this->assertStringContainsString( + 'report-to csp-endpoint', + $response->getHeader('Content-Security-Policy-Report-Only') + ); + + // check the Report-To header + $this->assertNotNull($response->getHeader('Report-To')); + + // convert the comma separated list into an array + $urls = explode(',', $reportTo); + $endpoints = []; + foreach ($urls as $url) { + $endpoints[] = ['url' => trim($url)]; + } + + $this->assertStringContainsString( + sprintf( + '{"group":"csp-endpoint","max_age":10886400,"endpoints":%s}', + json_encode($endpoints, JSON_UNESCAPED_SLASHES) + ), + $response->getHeader('Report-To') + ); + } + + /** + * Check the reporting endpoint can be set from the environment variable + */ + public function testMultipleReportURICanBeSetFromCode(): void + { + [$request, $response] = $this->getRequestResponse(); + /** @var Policy $policy */ + $policy = Injector::inst()->get(CMS::class); + + $urls = [ + 'https://example.com', + 'http://example.org', + 'https://example.net', + ]; + $policy->addDirective(Directive::REPORT_TO, $urls); + + // apply the policy + $policy->applyTo($response); + + // check the header + $this->assertNotNull($response->getHeader('Content-Security-Policy')); + $this->assertNull($response->getHeader('Content-Security-Policy-Report-Only')); + + // the report-uri directive only supports a single address, + // so we should not expect to see it + $this->assertStringNotContainsString( + 'report-uri', + $response->getHeader('Content-Security-Policy') + ); + + // check the report-to directive + $this->assertStringContainsString( + 'report-to csp-endpoint', + $response->getHeader('Content-Security-Policy') + ); + + // check the Report-To header + $this->assertNotNull($response->getHeader('Report-To')); + + // convert the comma separated list into an array + $endpoints = []; + foreach ($urls as $url) { + $endpoints[] = ['url' => trim($url)]; + } + + $this->assertStringContainsString( + sprintf( + '{"group":"csp-endpoint","max_age":10886400,"endpoints":%s}', + json_encode($endpoints, JSON_UNESCAPED_SLASHES) + ), + $response->getHeader('Report-To') + ); } public function testIsCanUseMultipleValuesForTheSameDirective(): void @@ -84,7 +413,7 @@ public function configure() [$request, $response] = $this->getRequestResponse(); $policy->applyTo($response); $this->assertEquals( - 'frame-src src-1 src-2;form-action action-1 action-2', + 'frame-src src-1 src-2; form-action action-1 action-2', $response->getHeader('content-security-policy') ); } @@ -104,7 +433,7 @@ public function configure() [$request, $response] = $this->getRequestResponse(); $policy->applyTo($response); $this->assertEquals( - 'connect-src \'none\';frame-src src-1', + 'connect-src \'none\'; frame-src src-1', $response->getHeader('content-security-policy') ); } @@ -124,7 +453,7 @@ public function configure() [$request, $response] = $this->getRequestResponse(); $policy->applyTo($response); $this->assertEquals( - 'connect-src \'self\';frame-src src-1', + 'connect-src \'self\'; frame-src src-1', $response->getHeader('content-security-policy') ); } @@ -252,7 +581,7 @@ public function configure() [$request, $response] = $this->getRequestResponse(); $policy->applyTo($response); $this->assertEquals( - 'upgrade-insecure-requests;block-all-mixed-content', + 'upgrade-insecure-requests; block-all-mixed-content', $response->getHeader('content-security-policy') ); } @@ -272,7 +601,7 @@ public function configure() [$request, $response] = $this->getRequestResponse(); $policy->applyTo($response); $this->assertEquals( - 'img-src *.ytimg.com;script-src www.youtube.com s.ytimg.com player.vimeo.com;frame-src *.youtube.com player.vimeo.com;child-src player.vimeo.com', + 'img-src *.ytimg.com; script-src www.youtube.com s.ytimg.com player.vimeo.com; frame-src *.youtube.com player.vimeo.com; child-src player.vimeo.com', $response->getHeader('content-security-policy') ); }