Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce DirectiveSetBuilderInterface to allow runtime modification #348

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions src/ContentSecurityPolicy/ConfigurationDirectiveSetBuilder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php

declare(strict_types=1);

/*
* This file is part of the Nelmio SecurityBundle.
*
* (c) Nelmio <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Nelmio\SecurityBundle\ContentSecurityPolicy;

class ConfigurationDirectiveSetBuilder implements DirectiveSetBuilderInterface
{
private PolicyManager $policyManager;

/**
* @phpstan-var array{
* enforce?: array<string, mixed>,
* report?: array<string, mixed>,
* } $config
*/
private array $config;

/**
* @phpstan-var 'enforce'|'report' $kind
*/
private string $kind;

/**
* @phpstan-param array{
* enforce?: array<string, mixed>,
* report?: array<string, mixed>,
* } $config
* @phpstan-param 'enforce'|'report' $kind
*/
public function __construct(PolicyManager $policyManager, array $config, string $kind)
{
$this->policyManager = $policyManager;
$this->config = $config;
$this->kind = $kind;
}

public function buildDirectiveSet(): DirectiveSet
{
return DirectiveSet::fromConfig($this->policyManager, $this->config, $this->kind);
}

/**
* @phpstan-param array{
* enforce?: array<string, mixed>,
* report?: array<string, mixed>,
* } $config
* @phpstan-param 'enforce'|'report' $kind
*/
public static function create(PolicyManager $policyManager, array $config, string $kind): self
{
return new self($policyManager, $config, $kind);
}
}
19 changes: 19 additions & 0 deletions src/ContentSecurityPolicy/DirectiveSetBuilderInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

declare(strict_types=1);

/*
* This file is part of the Nelmio SecurityBundle.
*
* (c) Nelmio <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Nelmio\SecurityBundle\ContentSecurityPolicy;

interface DirectiveSetBuilderInterface
{
public function buildDirectiveSet(): DirectiveSet;
}
32 changes: 32 additions & 0 deletions src/ContentSecurityPolicy/LegacyDirectiveSetBuilder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

declare(strict_types=1);

/*
* This file is part of the Nelmio SecurityBundle.
*
* (c) Nelmio <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Nelmio\SecurityBundle\ContentSecurityPolicy;

/**
* @internal
*/
class LegacyDirectiveSetBuilder implements DirectiveSetBuilderInterface
{
private DirectiveSet $directiveSet;

public function __construct(DirectiveSet $directiveSet)
{
$this->directiveSet = $directiveSet;
}

public function buildDirectiveSet(): DirectiveSet
{
return clone $this->directiveSet;
}
}
25 changes: 16 additions & 9 deletions src/DependencyInjection/NelmioSecurityExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@

namespace Nelmio\SecurityBundle\DependencyInjection;

use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet;
use Nelmio\SecurityBundle\ContentSecurityPolicy\ConfigurationDirectiveSetBuilder;
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSetBuilderInterface;
use Symfony\Component\Config\Definition\Processor;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\ContainerBuilder;
Expand Down Expand Up @@ -50,11 +51,17 @@ public function load(array $configs, ContainerBuilder $container): void

$cspConfig = $config['csp'];

$enforceDefinition = $this->buildDirectiveSetDefinition($container, $cspConfig, 'enforce');
$reportDefinition = $this->buildDirectiveSetDefinition($container, $cspConfig, 'report');
$enforceDefinition = $this->createDirectiveSetBuilder($container, $cspConfig, 'enforce');
$reportDefinition = $this->createDirectiveSetBuilder($container, $cspConfig, 'report');

$container->addDefinitions([
'nelmio_security.directive_set_builder.report' => $reportDefinition,
'nelmio_security.directive_set_builder.enforce' => $enforceDefinition,
]);

$cspListenerDefinition = $container->getDefinition('nelmio_security.csp_listener');
$cspListenerDefinition->setArguments([$reportDefinition, $enforceDefinition, new Reference('nelmio_security.nonce_generator'), new Reference('nelmio_security.sha_computer'), (bool) $cspConfig['compat_headers'], $cspConfig['hosts'], $cspConfig['content_types']]);
$cspListenerDefinition->setArguments([new Reference('nelmio_security.directive_set_builder.report'), new Reference('nelmio_security.directive_set_builder.enforce'), new Reference('nelmio_security.nonce_generator'), new Reference('nelmio_security.sha_computer'), (bool) $cspConfig['compat_headers'], $cspConfig['hosts'], $cspConfig['content_types']]);

$container->setParameter('nelmio_security.csp.hash_algorithm', $cspConfig['hash']['algorithm']);

if (isset($cspConfig['request_matcher'])) {
Expand Down Expand Up @@ -179,11 +186,11 @@ public function load(array $configs, ContainerBuilder $container): void
* } $config
* @phpstan-param 'enforce'|'report' $type
*/
private function buildDirectiveSetDefinition(ContainerBuilder $container, array $config, string $type): Definition
private function createDirectiveSetBuilder(ContainerBuilder $container, array $config, string $type): Definition
{
$directiveDefinition = new Definition(DirectiveSet::class);
$builderDefinition = new Definition(DirectiveSetBuilderInterface::class);

$directiveDefinition->setFactory([DirectiveSet::class, 'fromConfig']);
$builderDefinition->setFactory([ConfigurationDirectiveSetBuilder::class, 'create']);

$pmDefinition = $container->getDefinition('nelmio_security.policy_manager');

Expand All @@ -198,8 +205,8 @@ private function buildDirectiveSetDefinition(ContainerBuilder $container, array
$pmDefinition->setArguments([new Reference('nelmio_security.ua_parser')]);
}

$directiveDefinition->setArguments([$pmDefinition, $config, $type]);
$builderDefinition->setArguments([$pmDefinition, $config, $type]);

return $directiveDefinition;
return $builderDefinition;
}
}
61 changes: 49 additions & 12 deletions src/EventListener/ContentSecurityPolicyListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
namespace Nelmio\SecurityBundle\EventListener;

use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet;
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSetBuilderInterface;
use Nelmio\SecurityBundle\ContentSecurityPolicy\LegacyDirectiveSetBuilder;
use Nelmio\SecurityBundle\ContentSecurityPolicy\NonceGeneratorInterface;
use Nelmio\SecurityBundle\ContentSecurityPolicy\ShaComputerInterface;
use Symfony\Component\HttpFoundation\Request;
Expand All @@ -24,8 +26,8 @@

final class ContentSecurityPolicyListener extends AbstractContentTypeRestrictableListener
{
private DirectiveSet $report;
private DirectiveSet $enforce;
private DirectiveSetBuilderInterface $reportDirectiveSetBuilder;
private DirectiveSetBuilderInterface $enforceDirectiveSetBuilder;
private bool $compatHeaders;

/**
Expand All @@ -45,12 +47,14 @@ final class ContentSecurityPolicyListener extends AbstractContentTypeRestrictabl
private ?RequestMatcherInterface $requestMatcher;

/**
* @param list<string> $hosts
* @param list<string> $contentTypes
* @param DirectiveSetBuilderInterface|DirectiveSet $reportDirectiveSetBuilder
* @param DirectiveSetBuilderInterface|DirectiveSet $enforceDirectiveSetBuilder
* @param list<string> $hosts
* @param list<string> $contentTypes
*/
public function __construct(
DirectiveSet $report,
DirectiveSet $enforce,
$reportDirectiveSetBuilder,
$enforceDirectiveSetBuilder,
NonceGeneratorInterface $nonceGenerator,
ShaComputerInterface $shaComputer,
bool $compatHeaders = true,
Expand All @@ -59,8 +63,8 @@ public function __construct(
?RequestMatcherInterface $requestMatcher = null
) {
parent::__construct($contentTypes);
$this->report = $report;
$this->enforce = $enforce;
$this->reportDirectiveSetBuilder = $this->ensureDirectiveSetBuilder($reportDirectiveSetBuilder);
$this->enforceDirectiveSetBuilder = $this->ensureDirectiveSetBuilder($enforceDirectiveSetBuilder);
$this->compatHeaders = $compatHeaders;
$this->hosts = $hosts;
$this->nonceGenerator = $nonceGenerator;
Expand Down Expand Up @@ -110,14 +114,20 @@ public function addStyle(string $html): void
$this->sha['style-src'][] = $this->shaComputer->computeForStyle($html);
}

/**
* @deprecated Use `nelmio_security.directive_set_builder.report` instead.
*/
public function getReport(): DirectiveSet
{
return $this->report;
return $this->reportDirectiveSetBuilder->buildDirectiveSet();
}

/**
* @deprecated Use `nelmio_security.directive_set_builder.enforce` instead.
*/
public function getEnforcement(): DirectiveSet
{
return $this->enforce;
return $this->enforceDirectiveSetBuilder->buildDirectiveSet();
}

public function getNonce(string $usage): string
Expand Down Expand Up @@ -169,10 +179,10 @@ public function onKernelResponse(ResponseEvent $e): void
}

if (!$response->headers->has('Content-Security-Policy-Report-Only')) {
$response->headers->add($this->buildHeaders($request, $this->report, true, $this->compatHeaders, $signatures));
$response->headers->add($this->buildHeaders($request, $this->reportDirectiveSetBuilder->buildDirectiveSet(), true, $this->compatHeaders, $signatures));
}
if (!$response->headers->has('Content-Security-Policy')) {
$response->headers->add($this->buildHeaders($request, $this->enforce, false, $this->compatHeaders, $signatures));
$response->headers->add($this->buildHeaders($request, $this->enforceDirectiveSetBuilder->buildDirectiveSet(), false, $this->compatHeaders, $signatures));
}
}

Expand Down Expand Up @@ -233,4 +243,31 @@ private function buildHeaders(

return $headers;
}

/**
* @param DirectiveSetBuilderInterface|DirectiveSet $builderOrDirectiveSet
*/
private function ensureDirectiveSetBuilder($builderOrDirectiveSet): DirectiveSetBuilderInterface
{
if ($builderOrDirectiveSet instanceof DirectiveSetBuilderInterface) {
return $builderOrDirectiveSet;
}

if ($builderOrDirectiveSet instanceof DirectiveSet) {
trigger_deprecation(
'nelmio/security-bundle',
'3.5',
sprintf(
'Passing %s directly to the %s constructor is deprecated and will be removed in 4.0. Pass a %s instead.',
DirectiveSet::class,
self::class,
DirectiveSetBuilderInterface::class
)
);

return new LegacyDirectiveSetBuilder($builderOrDirectiveSet);
}

throw new \InvalidArgumentException(sprintf('The %s constructor %s expects a or %s.', self::class, DirectiveSetBuilderInterface::class, DirectiveSet::class));
}
}
32 changes: 30 additions & 2 deletions tests/Listener/ContentSecurityPolicyListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
namespace Nelmio\SecurityBundle\Tests\Listener;

use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet;
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSetBuilderInterface;
use Nelmio\SecurityBundle\ContentSecurityPolicy\NonceGeneratorInterface;
use Nelmio\SecurityBundle\ContentSecurityPolicy\PolicyManager;
use Nelmio\SecurityBundle\ContentSecurityPolicy\ShaComputerInterface;
Expand Down Expand Up @@ -464,6 +465,22 @@ public function testHeadersAreNotOverwrittenIfPresent(): void
);
}

public function testLegacyConstructorCreatesDirectiveSetBuilder(): void
{
$reportDirectiveSet = new DirectiveSet(new PolicyManager());
$reportDirectiveSet->setDirective('script-src', 'https://report.deprecation-test.example.com');

$enforceDirectiveSet = new DirectiveSet(new PolicyManager());
$enforceDirectiveSet->setDirective('script-src', 'https://enforce.deprecation-test.example.com');

$listener = new ContentSecurityPolicyListener($reportDirectiveSet, $enforceDirectiveSet, $this->nonceGenerator, $this->shaComputer, true, []);

$response = $this->callListener($listener, '/', true);

$this->assertSame('script-src https://report.deprecation-test.example.com', $response->headers->get('Content-Security-Policy-Report-Only'));
$this->assertSame('script-src https://enforce.deprecation-test.example.com', $response->headers->get('Content-Security-Policy'));
}

/**
* @param array<string, string|true> $directives
* @param list<string> $contentTypes
Expand All @@ -473,11 +490,22 @@ private function buildSimpleListener(array $directives, bool $reportOnly = false
$directiveSet = new DirectiveSet(new PolicyManager());
$directiveSet->setDirectives($directives);

$builder = $this->createDirectiveSetBuilderMock($directiveSet);
$dummyBuilder = $this->createDirectiveSetBuilderMock(new DirectiveSet(new PolicyManager()));

if ($reportOnly) {
return new ContentSecurityPolicyListener($directiveSet, new DirectiveSet(new PolicyManager()), $this->nonceGenerator, $this->shaComputer, $compatHeaders, $contentTypes);
return new ContentSecurityPolicyListener($builder, $dummyBuilder, $this->nonceGenerator, $this->shaComputer, $compatHeaders, $contentTypes);
}

return new ContentSecurityPolicyListener(new DirectiveSet(new PolicyManager()), $directiveSet, $this->nonceGenerator, $this->shaComputer, $compatHeaders, $contentTypes);
return new ContentSecurityPolicyListener($dummyBuilder, $builder, $this->nonceGenerator, $this->shaComputer, $compatHeaders, $contentTypes);
}

private function createDirectiveSetBuilderMock(DirectiveSet $directiveSet): DirectiveSetBuilderInterface
{
$mock = $this->createMock(DirectiveSetBuilderInterface::class);
$mock->method('buildDirectiveSet')->willReturn($directiveSet);

return $mock;
}

/**
Expand Down
17 changes: 13 additions & 4 deletions tests/Twig/IntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
namespace Nelmio\SecurityBundle\Tests\Twig;

use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet;
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSetBuilderInterface;
use Nelmio\SecurityBundle\ContentSecurityPolicy\NonceGeneratorInterface;
use Nelmio\SecurityBundle\ContentSecurityPolicy\PolicyManager;
use Nelmio\SecurityBundle\ContentSecurityPolicy\ShaComputerInterface;
Expand Down Expand Up @@ -45,8 +46,8 @@ public function testItWorksDynamically(): void
$policyManager = new PolicyManager();

$listener = new ContentSecurityPolicyListener(
new DirectiveSet($policyManager),
new DirectiveSet($policyManager),
$this->createDirectiveSetBuilderMock(new DirectiveSet($policyManager)),
$this->createDirectiveSetBuilderMock(new DirectiveSet($policyManager)),
$this->createStub(NonceGeneratorInterface::class),
$shaComputer
);
Expand Down Expand Up @@ -94,8 +95,8 @@ public function testItWorksStatically(): void
$policyManager = new PolicyManager();

$listener = new ContentSecurityPolicyListener(
new DirectiveSet($policyManager),
new DirectiveSet($policyManager),
$this->createDirectiveSetBuilderMock(new DirectiveSet($policyManager)),
$this->createDirectiveSetBuilderMock(new DirectiveSet($policyManager)),
$this->createStub(NonceGeneratorInterface::class),
$shaComputer
);
Expand Down Expand Up @@ -127,4 +128,12 @@ public function testItWorksStatically(): void

$this->assertSame(['script-src' => ['sha-script'], 'style-src' => ['sha-style']], $getSha($listener));
}

private function createDirectiveSetBuilderMock(DirectiveSet $directiveSet): DirectiveSetBuilderInterface
{
$mock = $this->createMock(DirectiveSetBuilderInterface::class);
$mock->method('buildDirectiveSet')->willReturn($directiveSet);

return $mock;
}
}
Loading