From d00c51b834b91244a594723b961f442919a1cf96 Mon Sep 17 00:00:00 2001 From: Steve Breker Date: Sat, 3 Aug 2024 00:35:43 -0700 Subject: [PATCH] Ignore newlines in CSP directive in app.yml This commit adds logic to QubitCspFilter to ignore newlines added when defining the CSP Directive in app.yml. Refactor QubitCspFilter to move config reading logic to separate methods in the QubitCspFilter class. Add tests to ensure that the config vars are read correctly from app.yml when they are formatted in different ways. --- lib/filter/QubitCSPFilter.php | 70 ++++++---- .../lib/filter/QubitCspFilterTest.php | 131 ++++++++++++++++++ 2 files changed, 175 insertions(+), 26 deletions(-) create mode 100644 test/phpunit/csvImportValidator/lib/filter/QubitCspFilterTest.php diff --git a/lib/filter/QubitCSPFilter.php b/lib/filter/QubitCSPFilter.php index f3db06721d..e0794102e4 100644 --- a/lib/filter/QubitCSPFilter.php +++ b/lib/filter/QubitCSPFilter.php @@ -28,36 +28,15 @@ public function execute($filterChain) return; } - $cspResponseHeader = sfConfig::get('app_csp_response_header', ''); - - if (empty($cspResponseHeader)) { + // Get CSP response header from config if available. + if (null === $cspResponseHeader = $this->getCspResponseHeader($this->getContext())) { // CSP is deactivated. $filterChain->execute(); return; } - $context = $this->getContext(); - if (false === array_search($cspResponseHeader, ['Content-Security-Policy-Report-Only', 'Content-Security-Policy'])) { - $context->getLogger()->err( - sprintf( - 'Setting \'app_csp_response_header\' is not set properly. CSP is not being used.' - ) - ); - - $filterChain->execute(); - - return; - } - - $cspDirectives = sfConfig::get('app_csp_directives', ''); - if (empty($cspDirectives)) { - $context->getLogger()->err( - sprintf( - 'Setting \'app_csp_directives\' is not set properly. CSP is not being used.' - ) - ); - + if (null === $cspDirectives = $this->getCspDirectives($this->getContext())) { $filterChain->execute(); return; @@ -69,17 +48,56 @@ public function execute($filterChain) $filterChain->execute(); - if (preg_match('~(text/xml|application/json)~', $context->response->getContentType())) { + if (preg_match('~(text/xml|application/json)~', $this->getContext()->response->getContentType())) { return; } // Set CSP header on response. - $context->response->setHttpHeader( + $this->getContext()->response->setHttpHeader( $cspResponseHeader, $cspDirectives = str_replace('nonce', 'nonce-'.$nonce, $cspDirectives) ); } + public function getCspResponseHeader($context): ?string + { + $cspResponseHeader = sfConfig::get('app_csp_response_header', ''); + + if (empty($cspResponseHeader)) { + // CSP is deactivated. + return null; + } + + if (false === array_search($cspResponseHeader, ['Content-Security-Policy-Report-Only', 'Content-Security-Policy'])) { + $context->getLogger()->err( + sprintf( + 'Setting \'app_csp_response_header\' is not set properly. CSP is not being used.' + ) + ); + + return null; + } + + return $cspResponseHeader; + } + + public function getCspDirectives($context): ?string + { + $cspDirectives = trim(preg_replace('/\s+/', ' ', sfConfig::get('app_csp_directives', ''))); + + if (empty($cspDirectives)) { + $context->getLogger()->err( + sprintf( + 'Setting \'app_csp_directives\' is not set properly. CSP is not being used.' + ) + ); + + return null; + } + + return $cspDirectives; + } + protected function getRandomNonce($length = 16) { return bin2hex(random_bytes($length)); diff --git a/test/phpunit/csvImportValidator/lib/filter/QubitCspFilterTest.php b/test/phpunit/csvImportValidator/lib/filter/QubitCspFilterTest.php new file mode 100644 index 0000000000..817d1d65db --- /dev/null +++ b/test/phpunit/csvImportValidator/lib/filter/QubitCspFilterTest.php @@ -0,0 +1,131 @@ + + default-src 'self'; + font-src 'self'; + img-src 'self' blob:; + script-src 'self' 'nonce'; + style-src 'self' 'nonce'; + worker-src 'self' blob:; + frame-ancestors 'self'; +EOT; + + $app_yml_multiline_pipe = <<<'EOT' +all: + csp: + response_header: Content-Security-Policy-Report-Only + directives: | + default-src 'self'; + font-src 'self'; + img-src 'self' blob:; + script-src 'self' 'nonce'; + style-src 'self' 'nonce'; + worker-src 'self' blob:; + frame-ancestors 'self'; +EOT; + + $directory = [ + 'app.yml' => $app_yml, + 'app_yml_multiline_greaterthan' => $app_yml_multiline_greaterthan, + 'app_yml_multiline_pipe' => $app_yml_multiline_pipe, + ]; + + $this->vfs = vfsStream::setup('root', null, $directory); + } + + public function getCspResponseHeaderProvider() + { + return [ + 'Standard app.yml with single line directive' => [ + 'filename' => '/app.yml', + 'expected' => 'Content-Security-Policy-Report-Only', + ], + ]; + } + + /** + * @dataProvider getCspResponseHeaderProvider + * + * @param mixed $filename + * @param mixed $expected + */ + public function testGetCspResponseHeader($filename, $expected) + { + // Read app.yml contents and populate sfConfig. + $fn = $this->vfs->url().$filename; + $handler = new sfDefineEnvironmentConfigHandler(); + $handler->initialize(['prefix' => 'app_']); + $data = $handler->execute([$fn]); + $data = preg_replace('/^<\?php\s*/', '', $data); + eval($data); + + $qubitCspFilterInstance = new QubitCSP(sfContext::getInstance()); + $settingValue = $qubitCspFilterInstance->getCspResponseHeader(sfContext::getInstance()); + + $this->assertSame($expected, $settingValue, 'Assert CSP response header read correctly.'); + } + + public function getCspDirectivesProvider() + { + return [ + 'Standard app.yml with single line directive' => [ + 'filename' => '/app.yml', + 'expected' => "default-src 'self'; font-src 'self'; img-src 'self' blob:; script-src 'self' 'nonce'; style-src 'self' 'nonce'; worker-src 'self' blob:; frame-ancestors 'self';", + ], + 'app.yml with multiline directive - greaterthan yml string concatenator' => [ + 'filename' => '/app_yml_multiline_greaterthan', + 'expected' => "default-src 'self'; font-src 'self'; img-src 'self' blob:; script-src 'self' 'nonce'; style-src 'self' 'nonce'; worker-src 'self' blob:; frame-ancestors 'self';", + ], + 'app.yml with multiline directive - pipe yml string concatenator' => [ + 'filename' => '/app_yml_multiline_pipe', + 'expected' => "default-src 'self'; font-src 'self'; img-src 'self' blob:; script-src 'self' 'nonce'; style-src 'self' 'nonce'; worker-src 'self' blob:; frame-ancestors 'self';", + ], + ]; + } + + /** + * @dataProvider getCspDirectivesProvider + * + * @param mixed $filename + * @param mixed $expected + */ + public function testGetCspDirectives($filename, $expected) + { + // Read app.yml contents and populate sfConfig. + $fn = $this->vfs->url().$filename; + $handler = new sfDefineEnvironmentConfigHandler(); + $handler->initialize(['prefix' => 'app_']); + $data = $handler->execute([$fn]); + $data = preg_replace('/^<\?php\s*/', '', $data); + eval($data); + + $qubitCspFilterInstance = new QubitCSP(sfContext::getInstance()); + $settingValue = $qubitCspFilterInstance->getCspDirectives(sfContext::getInstance()); + + $this->assertSame($expected, trim($settingValue), 'CSP directive read from config did not match expected value.'); + } +}