diff --git a/src/ContentSecurityPolicy/ConfigurationDirectiveSetBuilder.php b/src/ContentSecurityPolicy/ConfigurationDirectiveSetBuilder.php new file mode 100644 index 0000000..aaa8a29 --- /dev/null +++ b/src/ContentSecurityPolicy/ConfigurationDirectiveSetBuilder.php @@ -0,0 +1,63 @@ + + * + * 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, + * report?: array, + * } $config + */ + private array $config; + + /** + * @phpstan-var 'enforce'|'report' $kind + */ + private string $kind; + + /** + * @phpstan-param array{ + * enforce?: array, + * report?: array, + * } $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, + * report?: array, + * } $config + * @phpstan-param 'enforce'|'report' $kind + */ + public static function create(PolicyManager $policyManager, array $config, string $kind): self + { + return new self($policyManager, $config, $kind); + } +} diff --git a/src/ContentSecurityPolicy/DirectiveSetBuilderInterface.php b/src/ContentSecurityPolicy/DirectiveSetBuilderInterface.php new file mode 100644 index 0000000..9dc2d66 --- /dev/null +++ b/src/ContentSecurityPolicy/DirectiveSetBuilderInterface.php @@ -0,0 +1,19 @@ + + * + * 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; +} diff --git a/src/ContentSecurityPolicy/LegacyDirectiveSetBuilder.php b/src/ContentSecurityPolicy/LegacyDirectiveSetBuilder.php new file mode 100644 index 0000000..bbdf239 --- /dev/null +++ b/src/ContentSecurityPolicy/LegacyDirectiveSetBuilder.php @@ -0,0 +1,32 @@ + + * + * 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; + } +} diff --git a/src/DependencyInjection/NelmioSecurityExtension.php b/src/DependencyInjection/NelmioSecurityExtension.php index 176526b..45c757c 100644 --- a/src/DependencyInjection/NelmioSecurityExtension.php +++ b/src/DependencyInjection/NelmioSecurityExtension.php @@ -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; @@ -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'])) { @@ -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'); @@ -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; } } diff --git a/src/EventListener/ContentSecurityPolicyListener.php b/src/EventListener/ContentSecurityPolicyListener.php index 35a6ccf..84a7da5 100644 --- a/src/EventListener/ContentSecurityPolicyListener.php +++ b/src/EventListener/ContentSecurityPolicyListener.php @@ -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; @@ -24,8 +26,8 @@ final class ContentSecurityPolicyListener extends AbstractContentTypeRestrictableListener { - private DirectiveSet $report; - private DirectiveSet $enforce; + private DirectiveSetBuilderInterface $reportDirectiveSetBuilder; + private DirectiveSetBuilderInterface $enforceDirectiveSetBuilder; private bool $compatHeaders; /** @@ -45,12 +47,14 @@ final class ContentSecurityPolicyListener extends AbstractContentTypeRestrictabl private ?RequestMatcherInterface $requestMatcher; /** - * @param list $hosts - * @param list $contentTypes + * @param DirectiveSetBuilderInterface|DirectiveSet $reportDirectiveSetBuilder + * @param DirectiveSetBuilderInterface|DirectiveSet $enforceDirectiveSetBuilder + * @param list $hosts + * @param list $contentTypes */ public function __construct( - DirectiveSet $report, - DirectiveSet $enforce, + $reportDirectiveSetBuilder, + $enforceDirectiveSetBuilder, NonceGeneratorInterface $nonceGenerator, ShaComputerInterface $shaComputer, bool $compatHeaders = true, @@ -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; @@ -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 @@ -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)); } } @@ -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.3', + 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)); + } } diff --git a/tests/Listener/ContentSecurityPolicyListenerTest.php b/tests/Listener/ContentSecurityPolicyListenerTest.php index 5257e77..f424b0d 100644 --- a/tests/Listener/ContentSecurityPolicyListenerTest.php +++ b/tests/Listener/ContentSecurityPolicyListenerTest.php @@ -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; @@ -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 $directives * @param list $contentTypes @@ -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; } /** diff --git a/tests/Twig/IntegrationTest.php b/tests/Twig/IntegrationTest.php index 8e90cec..35096c9 100644 --- a/tests/Twig/IntegrationTest.php +++ b/tests/Twig/IntegrationTest.php @@ -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; @@ -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 ); @@ -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 ); @@ -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; + } }