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

Decouple UA analyzer and parser to allow caching #109

Merged
merged 1 commit into from
Aug 24, 2016
Merged
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
20 changes: 20 additions & 0 deletions DependencyInjection/Compiler/UAParserCompilerPass.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

namespace Nelmio\SecurityBundle\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;

class UAParserCompilerPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container)
{
if (!$container->hasParameter('nelmio_browser_adaptive_parser')) {
return;
}

$container
->getDefinition('nelmio_security.ua_parser')
->setArguments(array($container->getDefinition($container->getParameter('nelmio_browser_adaptive_parser'))));
}
}
24 changes: 22 additions & 2 deletions DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about logging a warning here, BC should be kept until version 3

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed

@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) {
Expand Down
11 changes: 8 additions & 3 deletions DependencyInjection/NelmioSecurityExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 2 additions & 0 deletions NelmioSecurityBundle.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -22,5 +23,6 @@ public function build(ContainerBuilder $container)
parent::build($container);

$container->addCompilerPass(new CSPTwigCompilerPass());
$container->addCompilerPass(new UAParserCompilerPass());
}
}
49 changes: 46 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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' ]
Expand All @@ -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'
Expand Down Expand Up @@ -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
<service id="my_own_parser" class="Nelmio\SecurityBundle\UserAgent\UAFamilyParser\DoctrineCacheUAFamilyParser">
<argument type="service" id="doctrine_cache.providers.redis_cache"/>
<argument type="service" id="nelmio_security.ua_parser.ua_php"/>
<argument>604800</argument>
</service>
```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
Expand Down
9 changes: 6 additions & 3 deletions Resources/config/csp.yml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
9 changes: 6 additions & 3 deletions Resources/config/csp_legacy.yml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
5 changes: 3 additions & 2 deletions Tests/ContentSecurityPolicy/DirectiveSetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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()
Expand Down
24 changes: 24 additions & 0 deletions Tests/DependencyInjection/ConfigurationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
45 changes: 45 additions & 0 deletions UserAgent/UAFamilyParser/DoctrineCacheUAFamilyParser.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

/*
* This file is part of the Nelmio SecurityBundle.
*
* (c) Nelmio <hello@nelm.io>
*
* 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;
}
}
48 changes: 48 additions & 0 deletions UserAgent/UAFamilyParser/PsrCacheUAFamilyParser.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

/*
* This file is part of the Nelmio SecurityBundle.
*
* (c) Nelmio <hello@nelm.io>
*
* 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-')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not type-hint the interface instead?

{
$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;
}
}
29 changes: 29 additions & 0 deletions UserAgent/UAFamilyParser/UAFamilyParser.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

/*
* This file is part of the Nelmio SecurityBundle.
*
* (c) Nelmio <hello@nelm.io>
*
* 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);
}
}
Loading