From 6e0922c5b2959ac1b48500ac508d8fc5a97286f9 Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Mon, 14 Mar 2022 14:19:09 +0100 Subject: [PATCH] [Admin] Security - Add handler to enable Content Security Policy (#11447) * [Admin] Security - Add handler to enable Content Security Policy * [Admin] Security - Add handler to enable Content Security Policy * [Admin] Security - Add handler to enable Content Security Policy - fix document preview * [Admin] Security - Add handler to enable Content Security Policy - add docs * [Admin] Security - Add handler to enable Content Security Policy - review changes * [Admin] Security - Add handler to enable Content Security Policy - review changes * [Admin] Security - Add handler to enable Content Security Policy - review changes * [Admin] Security - Add handler to enable Content Security Policy - review changes * added missing config node + micro optimizations Co-authored-by: Bernhard Rusch --- .../ContentSecurityPolicyUrlsPass.php | 45 +++++ .../DependencyInjection/Configuration.php | 37 +++++ .../EventListener/AdminSecurityListener.php | 83 ++++++++++ bundles/AdminBundle/PimcoreAdminBundle.php | 2 + .../Resources/config/event_listeners.yaml | 1 + .../Resources/config/security_services.yaml | 7 + .../js/pimcore/document/pages/preview.js | 13 +- .../views/Admin/Index/index.html.twig | 20 +-- .../Security/ContentSecurityPolicyHandler.php | 155 ++++++++++++++++++ .../Resources/config/pimcore/default.yaml | 1 + .../Resources/views/back-office.html.twig | 2 +- .../26_Best_Practice/75_Security_Concept.md | 36 ++++ lib/Twig/Extension/Templating/HeadScript.php | 4 + 13 files changed, 391 insertions(+), 15 deletions(-) create mode 100644 bundles/AdminBundle/DependencyInjection/Compiler/ContentSecurityPolicyUrlsPass.php create mode 100644 bundles/AdminBundle/EventListener/AdminSecurityListener.php create mode 100644 bundles/AdminBundle/Security/ContentSecurityPolicyHandler.php diff --git a/bundles/AdminBundle/DependencyInjection/Compiler/ContentSecurityPolicyUrlsPass.php b/bundles/AdminBundle/DependencyInjection/Compiler/ContentSecurityPolicyUrlsPass.php new file mode 100644 index 00000000000..c8812eadf45 --- /dev/null +++ b/bundles/AdminBundle/DependencyInjection/Compiler/ContentSecurityPolicyUrlsPass.php @@ -0,0 +1,45 @@ +getDefinition(ContentSecurityPolicyHandler::class); + + + $config = $container->getParameter('pimcore_admin.config'); + + if (count($config['admin_csp_header']['additional_urls'])) { + foreach ($config['admin_csp_header']['additional_urls'] as $additionalUrlsKey => $additionalUrlsArr) { + $definition->addMethodCall('addAllowedUrls', [$additionalUrlsKey, $additionalUrlsArr]); + } + } + } +} diff --git a/bundles/AdminBundle/DependencyInjection/Configuration.php b/bundles/AdminBundle/DependencyInjection/Configuration.php index 2addf96d78d..121c6c6eb22 100644 --- a/bundles/AdminBundle/DependencyInjection/Configuration.php +++ b/bundles/AdminBundle/DependencyInjection/Configuration.php @@ -15,6 +15,7 @@ namespace Pimcore\Bundle\AdminBundle\DependencyInjection; +use Pimcore\Bundle\AdminBundle\Security\ContentSecurityPolicyHandler; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; @@ -54,6 +55,42 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->end() ->end() + ->arrayNode('admin_csp_header') + ->canBeEnabled() + ->info('Can be used to enable or disable the Content Security Policy headers.') + ->children() + ->arrayNode('additional_urls') + ->addDefaultsIfNotSet() + ->normalizeKeys(false) + ->children() + ->arrayNode(ContentSecurityPolicyHandler::DEFAULT_OPT) + ->scalarPrototype()->end() + ->end() + ->arrayNode(ContentSecurityPolicyHandler::IMG_OPT) + ->scalarPrototype()->end() + ->end() + ->arrayNode(ContentSecurityPolicyHandler::SCRIPT_OPT) + ->scalarPrototype()->end() + ->end() + ->arrayNode(ContentSecurityPolicyHandler::STYLE_OPT) + ->scalarPrototype()->end() + ->end() + ->arrayNode(ContentSecurityPolicyHandler::CONNECT_OPT) + ->scalarPrototype()->end() + ->end() + ->arrayNode(ContentSecurityPolicyHandler::FONT_OPT) + ->scalarPrototype()->end() + ->end() + ->arrayNode(ContentSecurityPolicyHandler::MEDIA_OPT) + ->scalarPrototype()->end() + ->end() + ->arrayNode(ContentSecurityPolicyHandler::FRAME_OPT) + ->scalarPrototype()->end() + ->end() + ->end() + ->end() + ->end() + ->end() ->scalarNode('custom_admin_path_identifier') ->defaultNull() ->validate() diff --git a/bundles/AdminBundle/EventListener/AdminSecurityListener.php b/bundles/AdminBundle/EventListener/AdminSecurityListener.php new file mode 100644 index 00000000000..2c9b6297240 --- /dev/null +++ b/bundles/AdminBundle/EventListener/AdminSecurityListener.php @@ -0,0 +1,83 @@ + 'onKernelResponse', + ]; + } + + public function onKernelResponse(ResponseEvent $event) + { + if (!$this->config['admin_csp_header']['enabled']) { + return; + } + + $request = $event->getRequest(); + + if (!$event->isMainRequest()) { + return; + } + + if (!$this->matchesPimcoreContext($request, PimcoreContextResolver::CONTEXT_ADMIN)) { + return; + } + + if ($this->requestHelper->isFrontendRequestByAdmin($request)) { + return; + } + + $response = $event->getResponse(); + + // set CSP header with random nonce string to the response + $response->headers->set("Content-Security-Policy", $this->contentSecurityPolicyHandler->getCspHeader()); + } + +} + + diff --git a/bundles/AdminBundle/PimcoreAdminBundle.php b/bundles/AdminBundle/PimcoreAdminBundle.php index 14be87f9263..45e7230f455 100644 --- a/bundles/AdminBundle/PimcoreAdminBundle.php +++ b/bundles/AdminBundle/PimcoreAdminBundle.php @@ -15,6 +15,7 @@ namespace Pimcore\Bundle\AdminBundle; +use Pimcore\Bundle\AdminBundle\DependencyInjection\Compiler\ContentSecurityPolicyUrlsPass; use Pimcore\Bundle\AdminBundle\DependencyInjection\Compiler\GDPRDataProviderPass; use Pimcore\Bundle\AdminBundle\DependencyInjection\Compiler\ImportExportLocatorsPass; use Pimcore\Bundle\AdminBundle\DependencyInjection\Compiler\SerializerPass; @@ -44,6 +45,7 @@ public function build(ContainerBuilder $container) $container->addCompilerPass(new GDPRDataProviderPass()); $container->addCompilerPass(new ImportExportLocatorsPass()); $container->addCompilerPass(new TranslationServicesPass()); + $container->addCompilerPass(new ContentSecurityPolicyUrlsPass()); /** @var SecurityExtension $extension */ $extension = $container->getExtension('security'); diff --git a/bundles/AdminBundle/Resources/config/event_listeners.yaml b/bundles/AdminBundle/Resources/config/event_listeners.yaml index 5127ac41a0e..21694a62f9a 100644 --- a/bundles/AdminBundle/Resources/config/event_listeners.yaml +++ b/bundles/AdminBundle/Resources/config/event_listeners.yaml @@ -8,6 +8,7 @@ services: # SECURITY # + Pimcore\Bundle\AdminBundle\EventListener\AdminSecurityListener: ~ Pimcore\Bundle\AdminBundle\EventListener\BruteforceProtectionListener: ~ Pimcore\Bundle\AdminBundle\EventListener\AdminAuthenticationDoubleCheckListener: diff --git a/bundles/AdminBundle/Resources/config/security_services.yaml b/bundles/AdminBundle/Resources/config/security_services.yaml index b9018e5cdeb..3047722e17d 100644 --- a/bundles/AdminBundle/Resources/config/security_services.yaml +++ b/bundles/AdminBundle/Resources/config/security_services.yaml @@ -65,6 +65,13 @@ services: tags: - { name: monolog.logger, channel: security } + Pimcore\Bundle\AdminBundle\Security\ContentSecurityPolicyHandler: + public: true + calls: + - [ setLogger, [ '@logger' ] ] + tags: + - { name: monolog.logger, channel: security } + # user checker checking admin users for validity Pimcore\Bundle\AdminBundle\Security\User\UserChecker: ~ diff --git a/bundles/AdminBundle/Resources/public/js/pimcore/document/pages/preview.js b/bundles/AdminBundle/Resources/public/js/pimcore/document/pages/preview.js index 2bdfbeea5b2..9734ced0c04 100644 --- a/bundles/AdminBundle/Resources/public/js/pimcore/document/pages/preview.js +++ b/bundles/AdminBundle/Resources/public/js/pimcore/document/pages/preview.js @@ -31,8 +31,6 @@ pimcore.document.pages.preview = Class.create({ if (this.layout == null) { - var iframeOnLoad = "pimcore.globalmanager.get('document_" + this.page.id + "').preview.iFrameLoaded()"; - // preview switcher only for pages not for emails var tbar = []; if(this.page.getType() == "page") { @@ -124,9 +122,16 @@ pimcore.document.pages.preview = Class.create({ scrollable: false, bodyStyle: "background:#323232;", bodyCls: "pimcore_overflow_scrolling", - html: '' + 'name="' + this.iframeName + '">', + listeners: { + afterrender: function () { + Ext.get(this.getIframe()).on('load', function () { + this.iFrameLoaded(); + }.bind(this)); + }.bind(this) + } }); this.timeSlider = Ext.create('Ext.slider.Single', { diff --git a/bundles/AdminBundle/Resources/views/Admin/Index/index.html.twig b/bundles/AdminBundle/Resources/views/Admin/Index/index.html.twig index 7b51eefd1ff..589466d617a 100644 --- a/bundles/AdminBundle/Resources/views/Admin/Index/index.html.twig +++ b/bundles/AdminBundle/Resources/views/Admin/Index/index.html.twig @@ -88,7 +88,7 @@ {{ settings.hostname }} :: Pimcore - - - + + @@ -698,17 +698,17 @@ {# pimcore constants #} - - - - + + + {% for scriptUrl in scriptLibs %} - + {% endfor %} @@ -734,7 +734,7 @@ {% if settings.disableMinifyJs %} {% for pluginJsPath in pluginJsPaths %} - + {% endfor %} {% else %} {{ pimcore_minimize_scripts(pluginJsPaths)|raw }} @@ -745,6 +745,6 @@ {% endfor %} {# MUST BE THE LAST LINE #} - + diff --git a/bundles/AdminBundle/Security/ContentSecurityPolicyHandler.php b/bundles/AdminBundle/Security/ContentSecurityPolicyHandler.php new file mode 100644 index 00000000000..bc6b1451105 --- /dev/null +++ b/bundles/AdminBundle/Security/ContentSecurityPolicyHandler.php @@ -0,0 +1,155 @@ + [ + 'https://liveupdate.pimcore.org/update-check', //AdminBundle statistics & update-check service + 'https://nominatim.openstreetmap.org/' //CoreBundle geocoding_url_template + ], + ]; + + public function __construct(protected Config $config, protected array $cspHeaderOptions = []) + { + $resolver = new OptionsResolver(); + $this->configureOptions($resolver); + + $this->cspHeaderOptions = $resolver->resolve($cspHeaderOptions); + } + + public function configureOptions(OptionsResolver $resolver) + { + $resolver->setDefaults([ + self::DEFAULT_OPT => "'self'", + self::IMG_OPT => "* data: blob:", + self::MEDIA_OPT => "'self' data:", + self::SCRIPT_OPT => "'self' 'nonce-" . $this->getNonce() . "' 'unsafe-inline' 'unsafe-eval'", + self::STYLE_OPT => "'self' 'unsafe-inline'", + self::FRAME_OPT => "'self'", + self::CONNECT_OPT => "'self' blob:", + self::FONT_OPT => "'self'", + ]); + } + + /** + * @return string + */ + public function getCspHeader(): string + { + $cspHeaderOptions = array_map(function ($k, $v) { + return "$k $v " . $this->getAllowedUrls($k); + }, array_keys($this->cspHeaderOptions), array_values($this->cspHeaderOptions)); + + return implode(';' ,$cspHeaderOptions); + } + + /** + * @param string $key + * @param bool $flatten + * + * @return array|string + */ + private function getAllowedUrls(string $key, bool $flatten = true): array|string + { + if (!$flatten) { + return $this->allowedUrls[$key] ?? []; + } + + return isset($this->allowedUrls[$key]) && is_array($this->allowedUrls[$key]) ? implode(' ', $this->allowedUrls[$key]) : ''; + } + + /** + * @param string $key + * @param array $value + * + * @return $this + */ + public function addAllowedUrls(string $key, array $value): self + { + if(!isset($this->allowedUrls[$key])) { + $this->allowedUrls[$key] = []; + } + + foreach ($value as $val) { + $this->allowedUrls[$key][] = $val; + } + + return $this; + } + + /** + * @param string $key + * @param string $value + * + * @return $this + */ + public function setCspHeader(string $key, string $value): self + { + $this->cspHeaderOptions[$key] = $value; + + return $this; + } + + /** + * + * @return string + */ + public function getNonceHtmlAttribute(): string + { + return $this->config['admin_csp_header']['enabled'] ? ' nonce="' . $this->getNonce() . '"' : ''; + } + + /** + * Generates a random nonce parameter. + * + * @return string + */ + private function getNonce(): string + { + if (!$this->nonce) { + $this->nonce = generateRandomSymfonySecret(); + } + + return $this->nonce; + } +} diff --git a/bundles/CoreBundle/Resources/config/pimcore/default.yaml b/bundles/CoreBundle/Resources/config/pimcore/default.yaml index f8a7a0f42d4..2b4cc7ce678 100644 --- a/bundles/CoreBundle/Resources/config/pimcore/default.yaml +++ b/bundles/CoreBundle/Resources/config/pimcore/default.yaml @@ -59,6 +59,7 @@ twig: # this is only here for compatibility/dev reasons and may be removed later container: '@service_container' pimcore_csrf: '@Pimcore\Bundle\AdminBundle\Security\CsrfProtectionHandler' + pimcore_csp: '@Pimcore\Bundle\AdminBundle\Security\ContentSecurityPolicyHandler' paths: '%kernel.project_dir%/templates': App diff --git a/bundles/EcommerceFrameworkBundle/Resources/views/back-office.html.twig b/bundles/EcommerceFrameworkBundle/Resources/views/back-office.html.twig index dbdcf9f77dc..0599b614989 100644 --- a/bundles/EcommerceFrameworkBundle/Resources/views/back-office.html.twig +++ b/bundles/EcommerceFrameworkBundle/Resources/views/back-office.html.twig @@ -76,7 +76,7 @@ {{ pimcore_head_script() }} {% endblock %} -