Skip to content

Commit

Permalink
Ignore newlines in CSP directive in app.yml
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@sbreker
sbreker committed Aug 12, 2024
1 parent 0b481b6 commit d00c51b
Show file tree
Hide file tree
Showing 2 changed files with 175 additions and 26 deletions.
70 changes: 44 additions & 26 deletions lib/filter/QubitCSPFilter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));
Expand Down
131 changes: 131 additions & 0 deletions test/phpunit/csvImportValidator/lib/filter/QubitCspFilterTest.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
<?php

use org\bovigo\vfs\vfsStream;
use PHPUnit\Framework\TestCase;

/**
* @internal
*
* @covers \QubitApcUniversalClassLoader
* @covers \QubitCSP
*/
class QubitCspFilterTest extends TestCase
{
public function setUp(): void
{
$app_yml = <<<'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;

$app_yml_multiline_greaterthan = <<<'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;

$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.');
}
}

0 comments on commit d00c51b

Please sign in to comment.