From 57e586bd06a3d5e249d3ebffbff0552cd6123812 Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Tue, 12 Jul 2016 09:18:18 +0200 Subject: [PATCH] Add doctrine cache ua family parser --- CHANGELOG.md | 2 + .../Compiler/UAParserCompilerPass.php | 20 ++++++++ DependencyInjection/Configuration.php | 24 ++++++++- .../NelmioSecurityExtension.php | 11 +++-- NelmioSecurityBundle.php | 2 + README.md | 49 +++++++++++++++++-- Resources/config/csp.yml | 9 ++-- Resources/config/csp_legacy.yml | 9 ++-- .../DirectiveSetTest.php | 5 +- .../DependencyInjection/ConfigurationTest.php | 24 +++++++++ .../DoctrineCacheUAFamilyParser.php | 45 +++++++++++++++++ .../UAFamilyParser/PsrCacheUAFamilyParser.php | 48 ++++++++++++++++++ UserAgent/UAFamilyParser/UAFamilyParser.php | 29 +++++++++++ .../UAFamilyParserInterface.php | 17 +++++++ ...serAgentParser.php => UserAgentParser.php} | 8 +-- composer.json | 2 + 16 files changed, 284 insertions(+), 20 deletions(-) create mode 100644 DependencyInjection/Compiler/UAParserCompilerPass.php create mode 100644 UserAgent/UAFamilyParser/DoctrineCacheUAFamilyParser.php create mode 100644 UserAgent/UAFamilyParser/PsrCacheUAFamilyParser.php create mode 100644 UserAgent/UAFamilyParser/UAFamilyParser.php create mode 100644 UserAgent/UAFamilyParser/UAFamilyParserInterface.php rename UserAgent/{UAParserUserAgentParser.php => UserAgentParser.php} (82%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ef92b59..cfa29e13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ### 2.0.2 (2016-xx-xx) * Fix typo in the ALLOW-FROM implementation + * Update browser_adaptive configuration. Allow custom adapters + * Add Doctrine Cache and Psr Cache adapters for caching UA family parser ### 2.0.1 (2016-06-04) * Fix CookieSessionHandler::open that should return true unless there's an error diff --git a/DependencyInjection/Compiler/UAParserCompilerPass.php b/DependencyInjection/Compiler/UAParserCompilerPass.php new file mode 100644 index 00000000..341cd900 --- /dev/null +++ b/DependencyInjection/Compiler/UAParserCompilerPass.php @@ -0,0 +1,20 @@ +hasParameter('nelmio_browser_adaptive_parser')) { + return; + } + + $container + ->getDefinition('nelmio_security.ua_parser') + ->setArguments(array($container->getDefinition($container->getParameter('nelmio_browser_adaptive_parser')))); + } +} diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 72a56778..917fa5fc 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -210,9 +210,29 @@ private function addReportOrEnforceNode($reportOrEnforce) ->end(); $children - ->booleanNode('browser_adaptive') + ->arrayNode('browser_adaptive') + ->canBeEnabled() ->info('Do not send directives that browser do not support') - ->defaultValue(false) + ->addDefaultsIfNotSet() + ->children() + ->scalarNode('parser') + ->defaultValue('nelmio_security.ua_parser.ua_php') + ->end() + ->end() + ->beforeNormalization() + ->always(function ($v) { + if (!is_array($v)) { + @trigger_error("browser_adaptive configuration is now an array. Using boolean is deprecated and will not be supported anymore in version 3", E_USER_DEPRECATED); + + return array( + 'enabled' => $v, + 'parser' => 'nelmio_security.ua_parser.ua_php', + ); + } + + return $v; + }) + ->end() ->end(); foreach (DirectiveSet::getNames() as $name => $type) { diff --git a/DependencyInjection/NelmioSecurityExtension.php b/DependencyInjection/NelmioSecurityExtension.php index 81f85ff6..54968457 100644 --- a/DependencyInjection/NelmioSecurityExtension.php +++ b/DependencyInjection/NelmioSecurityExtension.php @@ -157,14 +157,19 @@ private function buildDirectiveSetDefinition(ContainerBuilder $container, $confi $pmDefinition = $container->getDefinition('nelmio_security.policy_manager'); - if (isset($config[$type]) && $config[$type]['browser_adaptive']) { - $service = $container->getParameter('nelmio_security.ua_parser.service'); + if (isset($config[$type]) && $config[$type]['browser_adaptive']['enabled']) { + $service = $config[$type]['browser_adaptive']['parser']; if ($service === 'nelmio_security.ua_parser.ua_php' && !class_exists('UAParser\Parser')) { throw new \RuntimeException('You must require "ua-parser/uap-php" as a dependency to use the browser_adaptive feature or configure your own "nelmio_security.ua_parser.service"'); } - $pmDefinition->setArguments(array($container->getDefinition($service))); + $container->setParameter('nelmio_browser_adaptive_parser', $service); + + $uaParser = $container->getDefinition('nelmio_security.ua_parser'); + $uaParser->setArguments(array($container->getDefinition('nelmio_security.ua_parser.ua_php'))); + + $pmDefinition->setArguments(array($uaParser)); } $directiveDefinition->setArguments(array($pmDefinition, $config, $type)); diff --git a/NelmioSecurityBundle.php b/NelmioSecurityBundle.php index 6f09f73b..e578c09b 100644 --- a/NelmioSecurityBundle.php +++ b/NelmioSecurityBundle.php @@ -11,6 +11,7 @@ namespace Nelmio\SecurityBundle; +use Nelmio\SecurityBundle\DependencyInjection\Compiler\UAParserCompilerPass; use Symfony\Component\HttpKernel\Bundle\Bundle; use Symfony\Component\DependencyInjection\ContainerBuilder; use Nelmio\SecurityBundle\DependencyInjection\Compiler\CSPTwigCompilerPass; @@ -22,5 +23,6 @@ public function build(ContainerBuilder $container) parent::build($container); $container->addCompilerPass(new CSPTwigCompilerPass()); + $container->addCompilerPass(new UAParserCompilerPass()); } } diff --git a/README.md b/README.md index f2441d9e..d5b2f14b 100644 --- a/README.md +++ b/README.md @@ -93,7 +93,8 @@ nelmio_security: content_types: [] enforce: level1_fallback: false - browser_adaptive: false + browser_adaptive: + enabled: false report-uri: %router.request_context.base_url%/nelmio/csp/report default-src: - 'none' @@ -168,7 +169,8 @@ nelmio_security: level1_fallback: true # Only send directives supported by the browser, defaults to false # This is a port of https://github.com/twitter/secureheaders/blob/83a564a235c8be1a8a3901373dbc769da32f6ed7/lib/secure_headers/headers/policy_management.rb#L97 - browser_adaptive: false + browser_adaptive: + enabled: false report-uri: %router.request_context.base_url%/nelmio/csp/report default-src: [ 'self' ] frame-src: [ 'https://www.youtube.com' ] @@ -186,7 +188,8 @@ nelmio_security: level1_fallback: true # Only send directives supported by the browser, defaults to false # This is a port of https://github.com/twitter/secureheaders/blob/83a564a235c8be1a8a3901373dbc769da32f6ed7/lib/secure_headers/headers/policy_management.rb#L97 - browser_adaptive: true + browser_adaptive: + enabled: true report-uri: %router.request_context.base_url%/nelmio/csp/report script-src: - 'self' @@ -231,6 +234,46 @@ nelmio_security: compat_headers: false ``` +#### Using browser adaptive directives + +Nelmio can only send directives that can be understood by the browser. This reduces noise provided via the report URI. +This is a direct port of what has been done in [Twitter SecureHeaders library](https://github.com/twitter/secureheaders). + +Use the `enabled` key to enable it. + +```yaml +nelmio_security: + csp: + enforce: + browser_adaptive: + enabled: true +``` + +**WARNING** This will parse the user agent and can consume some CPU usage. You can specify a cached parser to +avoid consumong to much CPU usage: + +```yaml +nelmio_security: + csp: + enforce: + browser_adaptive: + enabled: true + parser: my_own_parser +``` + +And declare service `my_ow_parser` based on one of the cached parser NelmioSecurityBundle provides or your own one. +For instance, using the `DoctrineCacheUAFamilyParser`: + +```xml + + + + 604800 + +```xml + +Have a look in the `Nelmio\SecurityBundle\UserAgent\UAFamilyParser` for these parsers. + #### Message digest for inline script handling If you want to disable `'unsafe-inline'` on `script-src` or `style-src` (recommended), Nelmio Security Bundle diff --git a/Resources/config/csp.yml b/Resources/config/csp.yml index 2510fcb0..6f0f9173 100644 --- a/Resources/config/csp.yml +++ b/Resources/config/csp.yml @@ -1,11 +1,14 @@ parameters: nelmio_security.nonce_generator.number_of_bytes: 16 - nelmio_security.ua_parser.service: nelmio_security.ua_parser.ua_php services: - nelmio_security.ua_parser.ua_php: - class: Nelmio\SecurityBundle\UserAgent\UAParserUserAgentParser + nelmio_security.ua_parser: + class: Nelmio\SecurityBundle\UserAgent\UserAgentParser public: false + + nelmio_security.ua_parser.ua_php: + class: Nelmio\SecurityBundle\UserAgent\UAFamilyParser\UAFamilyParser + public: true arguments: ['@nelmio_security.ua_parser.ua_php.provider'] nelmio_security.ua_parser.ua_php.provider: diff --git a/Resources/config/csp_legacy.yml b/Resources/config/csp_legacy.yml index a25be22e..20a1dde9 100644 --- a/Resources/config/csp_legacy.yml +++ b/Resources/config/csp_legacy.yml @@ -1,11 +1,14 @@ parameters: nelmio_security.nonce_generator.number_of_bytes: 16 - nelmio_security.ua_parser.service: nelmio_security.ua_parser.ua_php services: - nelmio_security.ua_parser.ua_php: - class: Nelmio\SecurityBundle\UserAgent\UAParserUserAgentParser + nelmio_security.ua_parser: + class: Nelmio\SecurityBundle\UserAgent\UserAgentParser public: false + + nelmio_security.ua_parser.ua_php: + class: Nelmio\SecurityBundle\UserAgent\UAFamilyParser\UAFamilyParser + public: true arguments: ['@nelmio_security.ua_parser.ua_php.provider'] nelmio_security.ua_parser.ua_php.provider: diff --git a/Tests/ContentSecurityPolicy/DirectiveSetTest.php b/Tests/ContentSecurityPolicy/DirectiveSetTest.php index d2fbb96e..2cac72e8 100644 --- a/Tests/ContentSecurityPolicy/DirectiveSetTest.php +++ b/Tests/ContentSecurityPolicy/DirectiveSetTest.php @@ -4,7 +4,8 @@ use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet; use Nelmio\SecurityBundle\ContentSecurityPolicy\PolicyManager; -use Nelmio\SecurityBundle\UserAgent\UAParserUserAgentParser; +use Nelmio\SecurityBundle\UserAgent\UAFamilyParser\UAFamilyParser; +use Nelmio\SecurityBundle\UserAgent\UserAgentParser; use Symfony\Component\HttpFoundation\Request; use UAParser\Parser; @@ -30,7 +31,7 @@ public function testFromConfig($expected, $ua, array $directives) private function createPolicyManager() { - return new PolicyManager(new UAParserUserAgentParser(Parser::create())); + return new PolicyManager(new UserAgentParser(new UAFamilyParser(Parser::create()))); } public function provideVariousConfig() diff --git a/Tests/DependencyInjection/ConfigurationTest.php b/Tests/DependencyInjection/ConfigurationTest.php index 494eeee7..dafba69b 100644 --- a/Tests/DependencyInjection/ConfigurationTest.php +++ b/Tests/DependencyInjection/ConfigurationTest.php @@ -63,6 +63,30 @@ public function testCspWithLevel2() ); } + public function testBrowserAdaptiveBoolean() + { + $this->processYamlConfiguration( + "csp:\n". + " report:\n". + " script-src:\n". + " - 'self'\n". + " browser_adaptive: true\n" + ); + } + + public function testBrowserAdaptiveArray() + { + $this->processYamlConfiguration( + "csp:\n". + " report:\n". + " script-src:\n". + " - 'self'\n". + " browser_adaptive:\n". + " enabled: true\n". + " parser: service_name" + ); + } + private function processYamlConfiguration($config) { $parser = new Parser(); diff --git a/UserAgent/UAFamilyParser/DoctrineCacheUAFamilyParser.php b/UserAgent/UAFamilyParser/DoctrineCacheUAFamilyParser.php new file mode 100644 index 00000000..7ac5db22 --- /dev/null +++ b/UserAgent/UAFamilyParser/DoctrineCacheUAFamilyParser.php @@ -0,0 +1,45 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Nelmio\SecurityBundle\UserAgent\UAFamilyParser; + +use Doctrine\Common\Cache\Cache; + +class DoctrineCacheUAFamilyParser implements UAFamilyParserInterface +{ + private $cache; + private $parser; + private $lifetime; + private $prefix; + + public function __construct(Cache $cache, UAFamilyParser $parser, $lifetime = 0, $prefix = 'nelmio-ua-parser-') + { + $this->parser = $parser; + $this->cache = $cache; + $this->lifetime = $lifetime; + $this->prefix = $prefix; + } + + public function getUaFamily($userAgent) + { + $id = $this->prefix.md5($userAgent); + + if (false !== $name = $this->cache->fetch($id)) { + return $name; + } + + $name = $this->parser->getUaFamily($userAgent); + + $this->cache->save($id, $name, $this->lifetime); + + return $name; + } +} diff --git a/UserAgent/UAFamilyParser/PsrCacheUAFamilyParser.php b/UserAgent/UAFamilyParser/PsrCacheUAFamilyParser.php new file mode 100644 index 00000000..eadadee7 --- /dev/null +++ b/UserAgent/UAFamilyParser/PsrCacheUAFamilyParser.php @@ -0,0 +1,48 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Nelmio\SecurityBundle\UserAgent\UAFamilyParser; + +use Psr\Cache\CacheItemPoolInterface; +use Psr\Cache\CacheException; + +class PsrCacheUAFamilyParser implements UAFamilyParserInterface +{ + private $cache; + private $parser; + private $lifetime; + private $prefix; + + public function __construct(CacheItemPoolInterface $cache, UAFamilyParser $parser, $lifetime = 0, $prefix = 'nelmio-ua-parser-') + { + $this->parser = $parser; + $this->cache = $cache; + $this->lifetime = $lifetime; + $this->prefix = $prefix; + } + + public function getUaFamily($userAgent) + { + $id = $this->prefix.md5($userAgent); + + $item = $this->cache->getItem($id); + + if ($item->isHit()) { + return $item->get(); + } + + $name = $this->parser->getUaFamily($userAgent); + + $this->cache->save($item->set($name)->expiresAfter($this->lifetime)); + + return $name; + } +} diff --git a/UserAgent/UAFamilyParser/UAFamilyParser.php b/UserAgent/UAFamilyParser/UAFamilyParser.php new file mode 100644 index 00000000..03fa3fcc --- /dev/null +++ b/UserAgent/UAFamilyParser/UAFamilyParser.php @@ -0,0 +1,29 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Nelmio\SecurityBundle\UserAgent\UAFamilyParser; + +use UAParser\Parser; + +class UAFamilyParser implements UAFamilyParserInterface +{ + private $parser; + + public function __construct(Parser $parser) + { + $this->parser = $parser; + } + + public function getUaFamily($userAgent) + { + return strtolower($this->parser->parse($userAgent)->ua->family); + } +} diff --git a/UserAgent/UAFamilyParser/UAFamilyParserInterface.php b/UserAgent/UAFamilyParser/UAFamilyParserInterface.php new file mode 100644 index 00000000..ccbc7f57 --- /dev/null +++ b/UserAgent/UAFamilyParser/UAFamilyParserInterface.php @@ -0,0 +1,17 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Nelmio\SecurityBundle\UserAgent\UAFamilyParser; + +interface UAFamilyParserInterface +{ + public function getUaFamily($userAgent); +} diff --git a/UserAgent/UAParserUserAgentParser.php b/UserAgent/UserAgentParser.php similarity index 82% rename from UserAgent/UAParserUserAgentParser.php rename to UserAgent/UserAgentParser.php index e7213287..9dc3cff8 100644 --- a/UserAgent/UAParserUserAgentParser.php +++ b/UserAgent/UserAgentParser.php @@ -11,13 +11,13 @@ namespace Nelmio\SecurityBundle\UserAgent; -use UAParser\Parser; +use Nelmio\SecurityBundle\UserAgent\UAFamilyParser\UAFamilyParserInterface; -class UAParserUserAgentParser implements UserAgentParserInterface +class UserAgentParser implements UserAgentParserInterface { private $parser; - public function __construct(Parser $parser) + public function __construct(UAFamilyParserInterface $parser) { $this->parser = $parser; } @@ -27,7 +27,7 @@ public function __construct(Parser $parser) */ public function getBrowser($userAgent) { - $name = strtolower($this->parser->parse($userAgent)->ua->family); + $name = $this->parser->getUaFamily($userAgent); switch (true) { case 'chrome' === $name: diff --git a/composer.json b/composer.json index ef11ac5b..47f28776 100644 --- a/composer.json +++ b/composer.json @@ -20,7 +20,9 @@ "paragonie/random_compat": "~1.0|~2.0" }, "require-dev": { + "doctrine/cache": "^1.0", "phpunit/phpunit": "^4.5", + "psr/cache": "^1.0", "twig/twig": "^1.24", "ua-parser/uap-php": "^3.4" },