Skip to content

Commit

Permalink
Implement an active cookie signature algorithm upgrade path
Browse files Browse the repository at this point in the history
  • Loading branch information
martijnc committed Jul 3, 2024
1 parent 15ce1d6 commit 953d667
Show file tree
Hide file tree
Showing 14 changed files with 517 additions and 10 deletions.
28 changes: 21 additions & 7 deletions src/EventListener/SignedCookieListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

namespace Nelmio\SecurityBundle\EventListener;

use Nelmio\SecurityBundle\SignedCookie\LegacySignatureCookieTracker;
use Nelmio\SecurityBundle\SignedCookie\LegacySignatureCookieTrackerInterface;
use Nelmio\SecurityBundle\SignedCookie\SignatureUpgradeCheckerInterface;
use Nelmio\SecurityBundle\Signer\SignerInterface;
use Symfony\Component\HttpFoundation\Cookie;
use Symfony\Component\HttpKernel\Event\RequestEvent;
Expand All @@ -27,12 +30,15 @@ final class SignedCookieListener
*/
private $signedCookieNames;

private LegacySignatureCookieTrackerInterface $legacySignatureCookieTracker;

/**
* @param list<string> $signedCookieNames
*/
public function __construct(SignerInterface $signer, array $signedCookieNames)
public function __construct(SignerInterface $signer, array $signedCookieNames, ?LegacySignatureCookieTrackerInterface $legacySignatureCookieTracker = null)
{
$this->signer = $signer;
$this->legacySignatureCookieTracker = $legacySignatureCookieTracker ?? new LegacySignatureCookieTracker();
if (\in_array('*', $signedCookieNames, true)) {
$this->signedCookieNames = true;
} else {
Expand All @@ -46,17 +52,25 @@ public function onKernelRequest(RequestEvent $e): void
return;
}

$this->legacySignatureCookieTracker->clear();

$request = $e->getRequest();

$names = true === $this->signedCookieNames ? $request->cookies->keys() : $this->signedCookieNames;
foreach ($names as $name) {
if ($request->cookies->has($name)) {
$cookie = $request->cookies->get($name);
if ($this->signer->verifySignedValue($cookie)) {
$request->cookies->set($name, $this->signer->getVerifiedRawValue($cookie));
} else {
$request->cookies->remove($name);
if (!$request->cookies->has($name)) {
continue;
}

$cookie = $request->cookies->get($name);
if ($this->signer->verifySignedValue($cookie)) {
$request->cookies->set($name, $this->signer->getVerifiedRawValue($cookie));

if ($this->signer instanceof SignatureUpgradeCheckerInterface && $this->signer->needsUpgrade($cookie)) {
$this->legacySignatureCookieTracker->flagForUpgrade($name);
}
} else {
$request->cookies->remove($name);
}
}
}
Expand Down
69 changes: 69 additions & 0 deletions src/EventListener/SignedCookieUpgradeListener.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<?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\EventListener;

use Nelmio\SecurityBundle\SignedCookie\LegacySignatureCookieTrackerInterface;
use Nelmio\SecurityBundle\SignedCookie\UpgradedCookieBuilderInterface;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\ResponseEvent;

class SignedCookieUpgradeListener
{
private LegacySignatureCookieTrackerInterface $legacySignatureCookieTracker;

private UpgradedCookieBuilderInterface $upgradedCookieBuilder;

public function __construct(LegacySignatureCookieTrackerInterface $legacySignatureCookieTracker, UpgradedCookieBuilderInterface $upgradedCookieBuilder)
{
$this->legacySignatureCookieTracker = $legacySignatureCookieTracker;
$this->upgradedCookieBuilder = $upgradedCookieBuilder;
}

public function onKernelResponse(ResponseEvent $event): void
{
if (!$event->isMainRequest()) {
return;
}

$response = $event->getResponse();
$request = $event->getRequest();

$currentResponseCookies = $this->extractCookieNames($response);
foreach ($this->legacySignatureCookieTracker->getCookiesForUpgrade() as $name) {
if (\in_array($name, $currentResponseCookies, true)) {
continue;
}
$cookie = $this->upgradedCookieBuilder->build($name, $request->cookies->get($name));
if (null === $cookie) {
continue;
}

$response->headers->setCookie($cookie);
}
}

/**
* @return string[]
*/
private function extractCookieNames(Response $response): array
{
$names = [];
$cookies = $response->headers->getCookies();
foreach ($cookies as $cookie) {
$names[] = $cookie->getName();
}

return $names;
}
}
35 changes: 33 additions & 2 deletions src/Resources/config/signed_cookie.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,18 @@
*/

use Nelmio\SecurityBundle\EventListener\SignedCookieListener;
use Nelmio\SecurityBundle\EventListener\SignedCookieUpgradeListener;
use Nelmio\SecurityBundle\SignedCookie\LegacySignatureCookieTracker;
use Nelmio\SecurityBundle\SignedCookie\LegacySignatureCookieTrackerInterface;
use Nelmio\SecurityBundle\SignedCookie\UpgradedCookieBuilderInterface;
use Nelmio\SecurityBundle\SignedCookie\UpgradedCookieBuilderRegistry;
use Nelmio\SecurityBundle\Signer;
use Nelmio\SecurityBundle\Signer\SignerInterface;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
use Symfony\Component\DependencyInjection\Loader\Configurator\ReferenceConfigurator;
use Symfony\Component\HttpKernel\KernelEvents;

use function Symfony\Component\DependencyInjection\Loader\Configurator\tagged_iterator;

return static function (ContainerConfigurator $containerConfigurator): void {
$containerConfigurator->services()
Expand All @@ -24,14 +32,15 @@
->args([
new ReferenceConfigurator('nelmio_security.signer'),
'%nelmio_security.signed_cookie.names%',
new ReferenceConfigurator('nelmio_security.legacy_signature_cookie_tracker'),
])
->tag('kernel.event_listener', [
'event' => 'kernel.request',
'event' => KernelEvents::REQUEST,
'method' => 'onKernelRequest',
'priority' => 10000,
])
->tag('kernel.event_listener', [
'event' => 'kernel.response',
'event' => KernelEvents::RESPONSE,
'method' => 'onKernelResponse',
'priority' => -10000,
])
Expand All @@ -45,5 +54,27 @@
])

->alias(SignerInterface::class, 'nelmio_security.signer')

->set('nelmio_security.signed_cookie_upgrade_listener', SignedCookieUpgradeListener::class)
->args([
new ReferenceConfigurator('nelmio_security.legacy_signature_cookie_tracker'),
new ReferenceConfigurator('nelmio_security.upgraded_cookie_builder_registry'),
])
->tag('kernel.event_listener', [
'event' => KernelEvents::RESPONSE,
'method' => 'onKernelResponse',
'priority' => -9990,
])

->set('nelmio_security.legacy_signature_cookie_tracker', LegacySignatureCookieTracker::class)

->alias(LegacySignatureCookieTrackerInterface::class, 'nelmio_security.legacy_signature_cookie_tracker')

->set('nelmio_security.upgraded_cookie_builder_registry', UpgradedCookieBuilderRegistry::class)
->args([
tagged_iterator('nelmio_security.upgraded_cookie_builder'),
])

->alias(UpgradedCookieBuilderInterface::class, 'nelmio_security.upgraded_cookie_builder_registry')
;
};
6 changes: 6 additions & 0 deletions src/Resources/doc/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,12 @@ to `sha256` and `hash_algo` to `sha3-256`.
The `legacy_hash_algo` option can expose your application to downgrade attacks and should only be used temporarily
for backward compatibility.

To shorten the timeframe with `legacy_hash_algo` enabled, the bundle can actively upgrade existing cookies with a legacy
signature. To do this, your app needs to register a service that implements `UpgradedCookieBuilderInterface` and tag it
with `nelmio_security.upgraded_cookie_builder` in the container. The service is responsible for creating the `Cookie`
from the provided name and value, so your application can set the appropriate cookie properties, such as expiration and
path.

Clickjacking Protection
-----------------------

Expand Down
40 changes: 40 additions & 0 deletions src/SignedCookie/LegacySignatureCookieTracker.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?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\SignedCookie;

class LegacySignatureCookieTracker implements LegacySignatureCookieTrackerInterface
{
/**
* @var string[]
*/
private array $names = [];

public function flagForUpgrade(string $cookieName): void
{
if (\in_array($cookieName, $this->names, true)) {
return;
}
$this->names[] = $cookieName;
}

public function getCookiesForUpgrade(): array
{
return $this->names;
}

public function clear(): void
{
$this->names = [];
}
}
26 changes: 26 additions & 0 deletions src/SignedCookie/LegacySignatureCookieTrackerInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?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\SignedCookie;

interface LegacySignatureCookieTrackerInterface
{
public function flagForUpgrade(string $cookieName): void;

/**
* @return string[]
*/
public function getCookiesForUpgrade(): array;

public function clear(): void;
}
19 changes: 19 additions & 0 deletions src/SignedCookie/SignatureUpgradeCheckerInterface.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\SignedCookie;

interface SignatureUpgradeCheckerInterface
{
public function needsUpgrade(string $signedValue): bool;
}
21 changes: 21 additions & 0 deletions src/SignedCookie/UpgradedCookieBuilderInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?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\SignedCookie;

use Symfony\Component\HttpFoundation\Cookie;

interface UpgradedCookieBuilderInterface
{
public function build(string $name, ?string $value): ?Cookie;
}
44 changes: 44 additions & 0 deletions src/SignedCookie/UpgradedCookieBuilderRegistry.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?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\SignedCookie;

use Symfony\Component\HttpFoundation\Cookie;

class UpgradedCookieBuilderRegistry implements UpgradedCookieBuilderInterface
{
/**
* @var iterable<UpgradedCookieBuilderInterface>
*/
private iterable $builders;

/**
* @param iterable<UpgradedCookieBuilderInterface> $builders
*/
public function __construct(iterable $builders)
{
$this->builders = $builders;
}

public function build(string $name, ?string $value): ?Cookie
{
foreach ($this->builders as $builder) {
$cookie = $builder->build($name, $value);
if (null !== $cookie) {
return $cookie;
}
}

return null;
}
}
18 changes: 17 additions & 1 deletion src/Signer.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@

namespace Nelmio\SecurityBundle;

use Nelmio\SecurityBundle\SignedCookie\SignatureUpgradeCheckerInterface;
use Nelmio\SecurityBundle\Signer\SignerInterface;

final class Signer implements SignerInterface
final class Signer implements SignerInterface, SignatureUpgradeCheckerInterface
{
private string $secret;
private string $algo;
Expand Down Expand Up @@ -89,6 +90,21 @@ public function getVerifiedRawValue(string $signedValue): string
return $valueSignatureTuple[0];
}

public function needsUpgrade(string $signedValue): bool
{
if (null === $this->legacyAlgo) {
return false;
}
[$value, $signature] = $this->splitSignatureFromSignedValue($signedValue);
if (null === $signature) {
return false;
}

$expectedLegacySignature = $this->generateSignature($value, $this->legacyAlgo);

return hash_equals($expectedLegacySignature, $signature);
}

private function generateSignature(string $value, string $algo): string
{
return hash_hmac($algo, $value, $this->secret);
Expand Down
Loading

0 comments on commit 953d667

Please sign in to comment.