From d746f314ba424ff6af29bd370d78eb306220f3e8 Mon Sep 17 00:00:00 2001 From: Oliver Hoff Date: Mon, 28 Mar 2016 04:37:10 +0200 Subject: [PATCH 01/10] extend csrf token storage infrastructure - cookie token storage managing csrf tokens stored inside cookies (prerequisite for "double submit cookies" csrf prevention strategy) - kernel response event listener add the cookie headers of the cookie token storage to request responses - token storage factory interface to create token storages based on requests - session token storage factory using the requests session to create a session token storage - cookie token storage factory using the requests cookies to create a cookie token storage - request stack token storage managing and using token storages inside master request's attributes --- .../AbstractTokenStorageProxy.php | 62 +++++++ .../Csrf/TokenStorage/CookieTokenStorage.php | 165 ++++++++++++++++++ .../CookieTokenStorageFactory.php | 30 ++++ .../CookieTokenStorageListener.php | 58 ++++++ .../TokenStorage/RequestStackTokenStorage.php | 85 +++++++++ .../SessionTokenStorageFactory.php | 48 +++++ .../TokenStorageFactoryInterface.php | 30 ++++ 7 files changed, 478 insertions(+) create mode 100644 src/Symfony/Component/Security/Csrf/TokenStorage/AbstractTokenStorageProxy.php create mode 100644 src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php create mode 100644 src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php create mode 100644 src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php create mode 100644 src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php create mode 100644 src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php create mode 100644 src/Symfony/Component/Security/Csrf/TokenStorage/TokenStorageFactoryInterface.php diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/AbstractTokenStorageProxy.php b/src/Symfony/Component/Security/Csrf/TokenStorage/AbstractTokenStorageProxy.php new file mode 100644 index 0000000000000..4a58d61639145 --- /dev/null +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/AbstractTokenStorageProxy.php @@ -0,0 +1,62 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Csrf\TokenStorage; + +/** + * Forwards calls to another TokenStorageInterface + * + * @author Oliver Hoff + */ +abstract class AbstractTokenStorageProxy implements TokenStorageInterface +{ + /** + * {@inheritDoc} + * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::getToken() + */ + public function getToken($tokenId) + { + return $this->getProxiedTokenStorage()->getToken($tokenId); + } + + /** + * {@inheritDoc} + * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::setToken() + */ + public function setToken($tokenId, $token) + { + // TODO interface declares return void, use return stmt or not? + $this->getProxiedTokenStorage()->setToken($tokenId, $token); + } + + /** + * {@inheritDoc} + * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::removeToken() + */ + public function removeToken($tokenId) + { + return $this->getProxiedTokenStorage()->removeToken($tokenId); + } + + /** + * {@inheritDoc} + * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::hasToken() + */ + public function hasToken($tokenId) + { + return $this->getProxiedTokenStorage()->hasToken($tokenId); + } + + /** + * @return TokenStorageInterface + */ + abstract protected function getProxiedTokenStorage(); +} diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php new file mode 100644 index 0000000000000..b7a82020c6205 --- /dev/null +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php @@ -0,0 +1,165 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Csrf\TokenStorage; + +use Symfony\Component\HttpFoundation\Cookie; +use Symfony\Component\HttpFoundation\ParameterBag; +use Symfony\Component\Security\Csrf\Exception\TokenNotFoundException; + +/** + * Accesses tokens in a set of cookies. A changeset records edits made to + * tokens. The changeset can be retrieved as a list of cookies to be used in a + * response's headers to "persist" the changes. + * + * @author Oliver Hoff + */ +class CookieTokenStorage implements TokenStorageInterface +{ + /** + * @var array + */ + private $transientTokens; + + /** + * @var ParameterBag + */ + private $cookies; + + /** + * @var boolean + */ + private $secure; + + /** + * @param ParameterBag $cookies + * @param boolean $secure + */ + public function __construct(ParameterBag $cookies, $secure) + { + $this->transientTokens = []; + $this->cookies = $cookies; + $this->secure = (bool) $secure; + } + + /** + * {@inheritDoc} + * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::getToken() + */ + public function getToken($tokenId) + { + $token = $this->resolveToken($tokenId); + + if (!strlen($token)) { + throw new TokenNotFoundException; + } + + return $token; + } + + /** + * {@inheritDoc} + * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::hasToken() + */ + public function hasToken($tokenId) + { + return strlen($this->resolveToken($tokenId)) > 0; + } + + /** + * {@inheritDoc} + * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::setToken() + */ + public function setToken($tokenId, $token) + { + if (!strlen($token)) { + throw new \InvalidArgumentException('Empty tokens are not allowed'); + } + + $this->updateToken($tokenId, $token); + } + + /** + * {@inheritDoc} + * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::removeToken() + */ + public function removeToken($tokenId) + { + $token = $this->resolveToken($tokenId); + + $this->updateToken($tokenId, ''); + + return strlen($token) ? $token : null; + } + + /** + * @return array + */ + public function createCookies() + { + $cookies = []; + + foreach ($this->transientTokens as $tokenId => $token) { + // FIXME empty tokens are handled by the http foundations cookie class + // and are recognized as a "delete" cookie + // the problem is the that the value of deleted cookies get set to + // the string "deleted" and not the empty string + + // TODO http only? + $name = $this->generateCookieName($tokenId); + $cookies[] = new Cookie($name, $token, 0, null, null, $this->secure, true); + } + + return $cookies; + } + + /** + * @param string $tokenId + * @return string + */ + protected function resolveToken($tokenId) + { + if (isset($this->transientTokens[$tokenId])) { + return $this->transientTokens[$tokenId]; + } + + $name = $this->generateCookieName($tokenId); + + return $this->cookies->get($name, ''); + } + + /** + * @param string $tokenId + * @param string $token + * @return void + */ + protected function updateToken($tokenId, $token) + { + $token = (string) $token; + $name = $this->generateCookieName($tokenId); + + if ($token === $this->cookies->get($name, '')) { + unset($this->transientTokens[$tokenId]); + } else { + $this->transientTokens[$tokenId] = $token; + } + } + + /** + * + * @param string $tokenId + * @return string + */ + protected function generateCookieName($tokenId) + { + return sprintf('_csrf/%s/%s', $this->secure ? 'insecure' : 'secure', $tokenId); + } +} diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php new file mode 100644 index 0000000000000..7ed0b60c9d2b1 --- /dev/null +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php @@ -0,0 +1,30 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Csrf\TokenStorage; + +use Symfony\Component\HttpFoundation\Request; + +/** + * Creates CSRF token storages based on the requests cookies. + * + * @author Oliver Hoff + */ +class CookieTokenStorageFactory implements TokenStorageFactoryInterface +{ + /** + * {@inheritDoc} + * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageFactoryInterface::createTokenStorage() + */ + public function createTokenStorage(Request $request) { + return new CookieTokenStorage($request->cookies, $request->isSecure()); + } +} diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php new file mode 100644 index 0000000000000..5485ecd96f0cc --- /dev/null +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php @@ -0,0 +1,58 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Csrf\TokenStorage; + +use Symfony\Component\HttpKernel\Event\FilterResponseEvent; + +/** + * Checks the request's attributes for a CookieTokenStorage instance. If one is + * found, the cookies representing the storage's changeset are appended to the + * response headers. + * + * TODO where to put this class? + * + * @author Oliver Hoff + */ +class CookieTokenStorageListener +{ + /** + * @var string + */ + private $tokenStorageKey; + + /** + * @param string|null $tokenStorageKey + */ + public function __construct($tokenStorageKey = null) + { + // TODO should this class get its own DEFAULT_TOKEN_STORAGE_KEY constant? + // if no, where should the sole constant be put? + $this->tokenStorageKey = $tokenStorageKey === null ? RequestStackTokenStorage::DEFAULT_TOKEN_STORAGE_KEY : $tokenStorageKey; + } + + /** + * @param FilterResponseEvent $event + * @return void + */ + public function onKernelResponse(FilterResponseEvent $event) + { + $storage = $event->getRequest()->attributes->get($this->tokenStorageKey); + if (!$storage instanceof CookieTokenStorage) { + return; + } + + $headers = $event->getResponse()->headers; + foreach ($storage->createCookies() as $cookie) { + $headers->setCookie($cookie); + } + } +} diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php new file mode 100644 index 0000000000000..e6bfba1e57548 --- /dev/null +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php @@ -0,0 +1,85 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Csrf\TokenStorage; + +use Symfony\Component\HttpFoundation\RequestStack; + +/** + * Forwards token storage calls to a token storage stored in the master + * request's attributes. If the attributes don't hold a token storage yet, one + * is created and set into the attributes. + * + * @author Oliver Hoff + */ +class RequestStackTokenStorage extends AbstractTokenStorageProxy +{ + /** + * @var string + */ + const DEFAULT_TOKEN_STORAGE_KEY = '_csrf_token_storage'; + + /** + * @var RequestStack + */ + private $requestStack; + + /** + * @var TokenStorageFactoryInterface + */ + private $factory; + + /** + * @var string + */ + private $tokenStorageKey; + + /** + * @param RequestStack $requestStack + * @param TokenStorageFactoryInterface $factory + * @param string|null $tokenStorageKey + */ + public function __construct( + RequestStack $requestStack, + TokenStorageFactoryInterface $factory, + $tokenStorageKey = null + ) { + $this->requestStack = $requestStack; + $this->factory = $factory; + $this->tokenStorageKey = $tokenStorageKey === null ? self::DEFAULT_TOKEN_STORAGE_KEY : $tokenStorageKey; + } + + /** + * {@inheritDoc} + * @see \Symfony\Component\Security\Csrf\TokenStorage\AbstractTokenStorageProxy::getProxiedTokenStorage() + */ + public function getProxiedTokenStorage() + { + // TODO use master or current request? + $request = $this->requestStack->getMasterRequest(); + + if (!$request) { + throw new \RuntimeException('Not in a request context'); + } + + $storage = $request->attributes->get($this->tokenStorageKey); + + // TODO what if storage is set and not an instance of TokenStorageInterface? + // error out? overwrite? + if (!$storage instanceof TokenStorageInterface) { + $storage = $this->factory->createTokenStorage($request); + $request->attributes->set($this->tokenStorageKey, $storage); + } + + return $storage; + } + +} diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php b/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php new file mode 100644 index 0000000000000..dc9695d0b4b91 --- /dev/null +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.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 Symfony\Component\Security\Csrf\TokenStorage; + +use Symfony\Component\HttpFoundation\Request; + +/** + * Creates CSRF token storages based on the requests session. + * + * @author Oliver Hoff + */ +class SessionTokenStorageFactory implements TokenStorageFactoryInterface +{ + /** + * @var string + */ + private $namespace; + + /** + * @param string $namespace The namespace under which the token is stored in the session + */ + public function __construct($namespace = null) + { + $this->namespace = $namespace === null ? SessionTokenStorage::SESSION_NAMESPACE : $namespace; + } + + /** + * {@inheritDoc} + * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageFactoryInterface::createTokenStorage() + */ + public function createTokenStorage(Request $request) { + $session = $request->getSession(); + if (!$session) { + throw new \RuntimeException('Request has no session'); + } + + return new SessionTokenStorage($session, $this->namespace); + } +} diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/TokenStorageFactoryInterface.php b/src/Symfony/Component/Security/Csrf/TokenStorage/TokenStorageFactoryInterface.php new file mode 100644 index 0000000000000..f2062791fede1 --- /dev/null +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/TokenStorageFactoryInterface.php @@ -0,0 +1,30 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Csrf\TokenStorage; + +use Symfony\Component\HttpFoundation\Request; + +/** + * Creates CSRF token storages. + * + * @author Oliver Hoff + */ +interface TokenStorageFactoryInterface +{ + /** + * Creates a new token storage for the given request. + * + * @param Request $request + * @return TokenStorageInterface + */ + public function createTokenStorage(Request $request); +} From e9573af5591e6da6e69c5bde586d512a089bae89 Mon Sep 17 00:00:00 2001 From: Oliver Hoff Date: Wed, 21 Sep 2016 03:38:01 +0200 Subject: [PATCH 02/10] code style and feedback --- .../Exception/UnexpectedValueException.php | 21 +++++++++++++++++++ .../Csrf/TokenStorage/CookieTokenStorage.php | 20 +++++++++--------- .../TokenStorage/RequestStackTokenStorage.php | 21 +++++++++++++------ .../SessionTokenStorageFactory.php | 5 +++-- 4 files changed, 49 insertions(+), 18 deletions(-) create mode 100644 src/Symfony/Component/Security/Core/Exception/UnexpectedValueException.php diff --git a/src/Symfony/Component/Security/Core/Exception/UnexpectedValueException.php b/src/Symfony/Component/Security/Core/Exception/UnexpectedValueException.php new file mode 100644 index 0000000000000..165735eb37f6f --- /dev/null +++ b/src/Symfony/Component/Security/Core/Exception/UnexpectedValueException.php @@ -0,0 +1,21 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Exception; + +/** + * Base UnexpectedValueException for the Security component. + * + * @author Oliver Hoff + */ +class UnexpectedValueException extends \UnexpectedValueException implements ExceptionInterface +{ +} diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php index b7a82020c6205..29d620e3389c7 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php @@ -13,6 +13,7 @@ use Symfony\Component\HttpFoundation\Cookie; use Symfony\Component\HttpFoundation\ParameterBag; +use Symfony\Component\Security\Core\Exception\InvalidArgumentException; use Symfony\Component\Security\Csrf\Exception\TokenNotFoundException; /** @@ -27,7 +28,7 @@ class CookieTokenStorage implements TokenStorageInterface /** * @var array */ - private $transientTokens; + private $transientTokens = array(); /** * @var ParameterBag @@ -45,7 +46,6 @@ class CookieTokenStorage implements TokenStorageInterface */ public function __construct(ParameterBag $cookies, $secure) { - $this->transientTokens = []; $this->cookies = $cookies; $this->secure = (bool) $secure; } @@ -58,8 +58,8 @@ public function getToken($tokenId) { $token = $this->resolveToken($tokenId); - if (!strlen($token)) { - throw new TokenNotFoundException; + if ('' === $token) { + throw new TokenNotFoundException(); } return $token; @@ -71,7 +71,7 @@ public function getToken($tokenId) */ public function hasToken($tokenId) { - return strlen($this->resolveToken($tokenId)) > 0; + return '' !== $this->resolveToken($tokenId); } /** @@ -80,8 +80,10 @@ public function hasToken($tokenId) */ public function setToken($tokenId, $token) { - if (!strlen($token)) { - throw new \InvalidArgumentException('Empty tokens are not allowed'); + $token = (string) $token; + + if ('' === $token) { + throw new InvalidArgumentException('Empty tokens are not allowed'); } $this->updateToken($tokenId, $token); @@ -97,7 +99,7 @@ public function removeToken($tokenId) $this->updateToken($tokenId, ''); - return strlen($token) ? $token : null; + return '' === $token ? null : $token; } /** @@ -113,7 +115,6 @@ public function createCookies() // the problem is the that the value of deleted cookies get set to // the string "deleted" and not the empty string - // TODO http only? $name = $this->generateCookieName($tokenId); $cookies[] = new Cookie($name, $token, 0, null, null, $this->secure, true); } @@ -143,7 +144,6 @@ protected function resolveToken($tokenId) */ protected function updateToken($tokenId, $token) { - $token = (string) $token; $name = $this->generateCookieName($tokenId); if ($token === $this->cookies->get($name, '')) { diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php index e6bfba1e57548..cf2ea3495a118 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php @@ -12,6 +12,8 @@ namespace Symfony\Component\Security\Csrf\TokenStorage; use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\Security\Core\Exception\RuntimeException; +use Symfony\Component\Security\Core\Exception\UnexpectedValueException; /** * Forwards token storage calls to a token storage stored in the master @@ -67,18 +69,25 @@ public function getProxiedTokenStorage() $request = $this->requestStack->getMasterRequest(); if (!$request) { - throw new \RuntimeException('Not in a request context'); + throw new RuntimeException('Not in a request context'); } $storage = $request->attributes->get($this->tokenStorageKey); - // TODO what if storage is set and not an instance of TokenStorageInterface? - // error out? overwrite? - if (!$storage instanceof TokenStorageInterface) { - $storage = $this->factory->createTokenStorage($request); - $request->attributes->set($this->tokenStorageKey, $storage); + if ($storage instanceof TokenStorageInterface) { + return $storage; } + if ($storage !== null) { + throw new UnexpectedValueException(sprintf( + 'Expected null or an instance of "Symfony\\Component\\Security\\Csrf\\TokenStorage\\TokenStorageInterface", got "%s"', + is_object($storage) ? get_class($storage) : gettype($storage) + )); + } + + $storage = $this->factory->createTokenStorage($request); + $request->attributes->set($this->tokenStorageKey, $storage); + return $storage; } diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php b/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php index dc9695d0b4b91..e5d69b492be70 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Csrf\TokenStorage; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Security\Core\Exception\RuntimeException; /** * Creates CSRF token storages based on the requests session. @@ -30,7 +31,7 @@ class SessionTokenStorageFactory implements TokenStorageFactoryInterface */ public function __construct($namespace = null) { - $this->namespace = $namespace === null ? SessionTokenStorage::SESSION_NAMESPACE : $namespace; + $this->namespace = $namespace === null ? SessionTokenStorage::SESSION_NAMESPACE : (string) $namespace; } /** @@ -40,7 +41,7 @@ public function __construct($namespace = null) public function createTokenStorage(Request $request) { $session = $request->getSession(); if (!$session) { - throw new \RuntimeException('Request has no session'); + throw new RuntimeException('Request has no session'); } return new SessionTokenStorage($session, $this->namespace); From beed130207db2ee1188e5b0e16a54f1c983fde22 Mon Sep 17 00:00:00 2001 From: Oliver Hoff Date: Wed, 21 Sep 2016 04:24:26 +0200 Subject: [PATCH 03/10] more codestyle fixes --- .../AbstractTokenStorageProxy.php | 14 +++++++----- .../Csrf/TokenStorage/CookieTokenStorage.php | 22 +++++++++++-------- .../CookieTokenStorageFactory.php | 6 +++-- .../CookieTokenStorageListener.php | 1 - .../TokenStorage/RequestStackTokenStorage.php | 8 +++---- .../SessionTokenStorageFactory.php | 6 +++-- .../TokenStorageFactoryInterface.php | 1 + 7 files changed, 35 insertions(+), 23 deletions(-) diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/AbstractTokenStorageProxy.php b/src/Symfony/Component/Security/Csrf/TokenStorage/AbstractTokenStorageProxy.php index 4a58d61639145..eeb78e33f17c1 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/AbstractTokenStorageProxy.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/AbstractTokenStorageProxy.php @@ -12,14 +12,15 @@ namespace Symfony\Component\Security\Csrf\TokenStorage; /** - * Forwards calls to another TokenStorageInterface + * Forwards calls to another TokenStorageInterface. * * @author Oliver Hoff */ abstract class AbstractTokenStorageProxy implements TokenStorageInterface { /** - * {@inheritDoc} + * {@inheritdoc} + * * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::getToken() */ public function getToken($tokenId) @@ -28,7 +29,8 @@ public function getToken($tokenId) } /** - * {@inheritDoc} + * {@inheritdoc} + * * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::setToken() */ public function setToken($tokenId, $token) @@ -38,7 +40,8 @@ public function setToken($tokenId, $token) } /** - * {@inheritDoc} + * {@inheritdoc} + * * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::removeToken() */ public function removeToken($tokenId) @@ -47,7 +50,8 @@ public function removeToken($tokenId) } /** - * {@inheritDoc} + * {@inheritdoc} + * * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::hasToken() */ public function hasToken($tokenId) diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php index 29d620e3389c7..d434e0c0a64cb 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php @@ -36,13 +36,13 @@ class CookieTokenStorage implements TokenStorageInterface private $cookies; /** - * @var boolean + * @var bool */ private $secure; /** * @param ParameterBag $cookies - * @param boolean $secure + * @param bool $secure */ public function __construct(ParameterBag $cookies, $secure) { @@ -51,7 +51,8 @@ public function __construct(ParameterBag $cookies, $secure) } /** - * {@inheritDoc} + * {@inheritdoc} + * * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::getToken() */ public function getToken($tokenId) @@ -66,7 +67,8 @@ public function getToken($tokenId) } /** - * {@inheritDoc} + * {@inheritdoc} + * * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::hasToken() */ public function hasToken($tokenId) @@ -75,7 +77,8 @@ public function hasToken($tokenId) } /** - * {@inheritDoc} + * {@inheritdoc} + * * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::setToken() */ public function setToken($tokenId, $token) @@ -90,7 +93,8 @@ public function setToken($tokenId, $token) } /** - * {@inheritDoc} + * {@inheritdoc} + * * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::removeToken() */ public function removeToken($tokenId) @@ -107,7 +111,7 @@ public function removeToken($tokenId) */ public function createCookies() { - $cookies = []; + $cookies = array(); foreach ($this->transientTokens as $tokenId => $token) { // FIXME empty tokens are handled by the http foundations cookie class @@ -124,6 +128,7 @@ public function createCookies() /** * @param string $tokenId + * * @return string */ protected function resolveToken($tokenId) @@ -140,7 +145,6 @@ protected function resolveToken($tokenId) /** * @param string $tokenId * @param string $token - * @return void */ protected function updateToken($tokenId, $token) { @@ -154,8 +158,8 @@ protected function updateToken($tokenId, $token) } /** - * * @param string $tokenId + * * @return string */ protected function generateCookieName($tokenId) diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php index 7ed0b60c9d2b1..58bf839c774d4 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php @@ -21,10 +21,12 @@ class CookieTokenStorageFactory implements TokenStorageFactoryInterface { /** - * {@inheritDoc} + * {@inheritdoc} + * * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageFactoryInterface::createTokenStorage() */ - public function createTokenStorage(Request $request) { + public function createTokenStorage(Request $request) + { return new CookieTokenStorage($request->cookies, $request->isSecure()); } } diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php index 5485ecd96f0cc..5f40d2a1ae62e 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php @@ -41,7 +41,6 @@ public function __construct($tokenStorageKey = null) /** * @param FilterResponseEvent $event - * @return void */ public function onKernelResponse(FilterResponseEvent $event) { diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php index cf2ea3495a118..d1f5b93676f8e 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php @@ -45,9 +45,9 @@ class RequestStackTokenStorage extends AbstractTokenStorageProxy private $tokenStorageKey; /** - * @param RequestStack $requestStack + * @param RequestStack $requestStack * @param TokenStorageFactoryInterface $factory - * @param string|null $tokenStorageKey + * @param string|null $tokenStorageKey */ public function __construct( RequestStack $requestStack, @@ -60,7 +60,8 @@ public function __construct( } /** - * {@inheritDoc} + * {@inheritdoc} + * * @see \Symfony\Component\Security\Csrf\TokenStorage\AbstractTokenStorageProxy::getProxiedTokenStorage() */ public function getProxiedTokenStorage() @@ -90,5 +91,4 @@ public function getProxiedTokenStorage() return $storage; } - } diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php b/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php index e5d69b492be70..cc7d674d58df2 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php @@ -35,10 +35,12 @@ public function __construct($namespace = null) } /** - * {@inheritDoc} + * {@inheritdoc} + * * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageFactoryInterface::createTokenStorage() */ - public function createTokenStorage(Request $request) { + public function createTokenStorage(Request $request) + { $session = $request->getSession(); if (!$session) { throw new RuntimeException('Request has no session'); diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/TokenStorageFactoryInterface.php b/src/Symfony/Component/Security/Csrf/TokenStorage/TokenStorageFactoryInterface.php index f2062791fede1..ce3cd639048dc 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/TokenStorageFactoryInterface.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/TokenStorageFactoryInterface.php @@ -24,6 +24,7 @@ interface TokenStorageFactoryInterface * Creates a new token storage for the given request. * * @param Request $request + * * @return TokenStorageInterface */ public function createTokenStorage(Request $request); From 1cea0d26ac3cbee7d5fdd870def961d477b192a9 Mon Sep 17 00:00:00 2001 From: Oliver Hoff Date: Wed, 21 Sep 2016 05:52:32 +0200 Subject: [PATCH 04/10] remove @see --- .../Csrf/TokenStorage/AbstractTokenStorageProxy.php | 8 -------- .../Security/Csrf/TokenStorage/CookieTokenStorage.php | 8 -------- .../Csrf/TokenStorage/CookieTokenStorageFactory.php | 2 -- .../Csrf/TokenStorage/RequestStackTokenStorage.php | 2 -- .../Csrf/TokenStorage/SessionTokenStorageFactory.php | 2 -- 5 files changed, 22 deletions(-) diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/AbstractTokenStorageProxy.php b/src/Symfony/Component/Security/Csrf/TokenStorage/AbstractTokenStorageProxy.php index eeb78e33f17c1..326088865ecfb 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/AbstractTokenStorageProxy.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/AbstractTokenStorageProxy.php @@ -20,8 +20,6 @@ abstract class AbstractTokenStorageProxy implements TokenStorageInterface { /** * {@inheritdoc} - * - * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::getToken() */ public function getToken($tokenId) { @@ -30,8 +28,6 @@ public function getToken($tokenId) /** * {@inheritdoc} - * - * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::setToken() */ public function setToken($tokenId, $token) { @@ -41,8 +37,6 @@ public function setToken($tokenId, $token) /** * {@inheritdoc} - * - * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::removeToken() */ public function removeToken($tokenId) { @@ -51,8 +45,6 @@ public function removeToken($tokenId) /** * {@inheritdoc} - * - * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::hasToken() */ public function hasToken($tokenId) { diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php index d434e0c0a64cb..492314301c480 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php @@ -52,8 +52,6 @@ public function __construct(ParameterBag $cookies, $secure) /** * {@inheritdoc} - * - * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::getToken() */ public function getToken($tokenId) { @@ -68,8 +66,6 @@ public function getToken($tokenId) /** * {@inheritdoc} - * - * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::hasToken() */ public function hasToken($tokenId) { @@ -78,8 +74,6 @@ public function hasToken($tokenId) /** * {@inheritdoc} - * - * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::setToken() */ public function setToken($tokenId, $token) { @@ -94,8 +88,6 @@ public function setToken($tokenId, $token) /** * {@inheritdoc} - * - * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface::removeToken() */ public function removeToken($tokenId) { diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php index 58bf839c774d4..e62a4b5a76110 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php @@ -22,8 +22,6 @@ class CookieTokenStorageFactory implements TokenStorageFactoryInterface { /** * {@inheritdoc} - * - * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageFactoryInterface::createTokenStorage() */ public function createTokenStorage(Request $request) { diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php index d1f5b93676f8e..23708e77e03e4 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php @@ -61,8 +61,6 @@ public function __construct( /** * {@inheritdoc} - * - * @see \Symfony\Component\Security\Csrf\TokenStorage\AbstractTokenStorageProxy::getProxiedTokenStorage() */ public function getProxiedTokenStorage() { diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php b/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php index cc7d674d58df2..3718018a67b48 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php @@ -36,8 +36,6 @@ public function __construct($namespace = null) /** * {@inheritdoc} - * - * @see \Symfony\Component\Security\Csrf\TokenStorage\TokenStorageFactoryInterface::createTokenStorage() */ public function createTokenStorage(Request $request) { From a298be61e2dcfae3f1384d100d1c0fbcce1b34f0 Mon Sep 17 00:00:00 2001 From: Oliver Hoff Date: Wed, 21 Sep 2016 05:53:26 +0200 Subject: [PATCH 05/10] update suggestion notice in composer.json --- src/Symfony/Component/Security/Csrf/composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Security/Csrf/composer.json b/src/Symfony/Component/Security/Csrf/composer.json index 4047fd5435a87..ef80422361989 100644 --- a/src/Symfony/Component/Security/Csrf/composer.json +++ b/src/Symfony/Component/Security/Csrf/composer.json @@ -25,7 +25,7 @@ "symfony/http-foundation": "~2.8|~3.0" }, "suggest": { - "symfony/http-foundation": "For using the class SessionTokenStorage." + "symfony/http-foundation": "For using SessionTokenStorage, CookieTokenStorage or RequestStackTokenStorage." }, "autoload": { "psr-4": { "Symfony\\Component\\Security\\Csrf\\": "" }, From a533b0e45e0a7e0573f4d72978adb692be7d448d Mon Sep 17 00:00:00 2001 From: Oliver Hoff Date: Wed, 21 Sep 2016 05:57:03 +0200 Subject: [PATCH 06/10] add a secureNamespace to SessionTokenStorageFactory for secure requests --- .../TokenStorage/SessionTokenStorageFactory.php | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php b/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php index 3718018a67b48..eab816a99d039 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorageFactory.php @@ -27,11 +27,18 @@ class SessionTokenStorageFactory implements TokenStorageFactoryInterface private $namespace; /** - * @param string $namespace The namespace under which the token is stored in the session + * @var string */ - public function __construct($namespace = null) + private $secureNamespace; + + /** + * @param string $namespace The namespace under which tokens are stored in the session + * @param string $secureNamespace The namespace under which tokens are stored in the session for secure connections + */ + public function __construct($namespace = null, $secureNamespace = null) { $this->namespace = $namespace === null ? SessionTokenStorage::SESSION_NAMESPACE : (string) $namespace; + $this->secureNamespace = $secureNamespace === null ? $this->namespace : (string) $secureNamespace; } /** @@ -44,6 +51,8 @@ public function createTokenStorage(Request $request) throw new RuntimeException('Request has no session'); } - return new SessionTokenStorage($session, $this->namespace); + $namespace = $request->isSecure() ? $this->secureNamespace : $this->namespace; + + return new SessionTokenStorage($session, $namespace); } } From 893fa0c1a57c68860234bd3bcda1abddbf4eb964 Mon Sep 17 00:00:00 2001 From: Oliver Hoff Date: Sat, 24 Sep 2016 18:21:22 +0200 Subject: [PATCH 07/10] implement EventSubscriberInterface into CookieTokenStorageListener --- .../TokenStorage/CookieTokenStorageListener.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php index 5f40d2a1ae62e..96abd1dac959a 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php @@ -12,6 +12,8 @@ namespace Symfony\Component\Security\Csrf\TokenStorage; use Symfony\Component\HttpKernel\Event\FilterResponseEvent; +use Symfony\Component\HttpKernel\KernelEvents; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; /** * Checks the request's attributes for a CookieTokenStorage instance. If one is @@ -22,7 +24,7 @@ * * @author Oliver Hoff */ -class CookieTokenStorageListener +class CookieTokenStorageListener implements EventSubscriberInterface { /** * @var string @@ -54,4 +56,14 @@ public function onKernelResponse(FilterResponseEvent $event) $headers->setCookie($cookie); } } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents() + { + return array( + KernelEvents::RESPONSE => array(array('onKernelResponse', 0)), + ); + } } From 61b5483ff5c807a14b0dc37b63f2f06ed2f0b337 Mon Sep 17 00:00:00 2001 From: Oliver Hoff Date: Sun, 25 Sep 2016 08:49:52 +0200 Subject: [PATCH 08/10] add verification logic to CookieTokenStorage --- .../Csrf/TokenStorage/CookieTokenStorage.php | 188 ++++++++++++++++-- .../CookieTokenStorageFactory.php | 31 ++- 2 files changed, 206 insertions(+), 13 deletions(-) diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php index 492314301c480..c444f84717af6 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php @@ -25,11 +25,26 @@ */ class CookieTokenStorage implements TokenStorageInterface { + /** + * @var string + */ + const COOKIE_DELIMITER = '_'; + /** * @var array */ private $transientTokens = array(); + /** + * @var array + */ + private $resolvedTokens = array(); + + /** + * @var array + */ + private $refreshTokens = array(); + /** * @var ParameterBag */ @@ -40,14 +55,36 @@ class CookieTokenStorage implements TokenStorageInterface */ private $secure; + /** + * @var string + */ + private $secret; + + /** + * @var int + */ + private $ttl; + /** * @param ParameterBag $cookies * @param bool $secure + * @param string $secret + * @param int $ttl */ - public function __construct(ParameterBag $cookies, $secure) + public function __construct(ParameterBag $cookies, $secure, $secret, $ttl = null) { $this->cookies = $cookies; $this->secure = (bool) $secure; + $this->secret = (string) $secret; + $this->ttl = $ttl === null ? 60 * 60 : (int) $ttl; + + if ('' === $this->secret) { + throw new InvalidArgumentException('Secret must be a non-empty string'); + } + + if ($this->ttl < 60) { + throw new InvalidArgumentException('TTL must be an integer greater than or equal to 60'); + } } /** @@ -110,9 +147,16 @@ public function createCookies() // and are recognized as a "delete" cookie // the problem is the that the value of deleted cookies get set to // the string "deleted" and not the empty string + $cookies[] = $this->createTokenCookie($tokenId, $token); + $cookies[] = $this->createVerificationCookie($tokenId, $token); + } + + foreach ($this->refreshTokens as $tokenId => $token) { + if (isset($this->transientTokens[$tokenId])) { + continue; + } - $name = $this->generateCookieName($tokenId); - $cookies[] = new Cookie($name, $token, 0, null, null, $this->secure, true); + $cookies[] = $this->createVerificationCookie($tokenId, $token); } return $cookies; @@ -120,18 +164,47 @@ public function createCookies() /** * @param string $tokenId + * @param bool $excludeTransient * * @return string */ - protected function resolveToken($tokenId) + protected function resolveToken($tokenId, $excludeTransient = false) { - if (isset($this->transientTokens[$tokenId])) { + if (!$excludeTransient && isset($this->transientTokens[$tokenId])) { return $this->transientTokens[$tokenId]; } - $name = $this->generateCookieName($tokenId); + if (isset($this->resolvedTokens[$tokenId])) { + return $this->resolvedTokens[$tokenId]; + } + + $this->resolvedTokens[$tokenId] = ''; - return $this->cookies->get($name, ''); + $token = $this->getTokenCookieValue($tokenId); + if ('' === $token) { + return ''; + } + + $parts = explode(self::COOKIE_DELIMITER, $this->getVerificationCookieValue($tokenId), 2); + if (count($parts) != 2) { + return ''; + } + + list($expires, $hash) = $parts; + $time = time(); + if (!ctype_digit($expires) || $expires < $time) { + return ''; + } + if (!hash_equals($this->generateVerificationHash($tokenId, $token, $expires), $hash)) { + return ''; + } + + $time += $this->ttl / 2; + if ($expires < $time) { + $this->refreshTokens[$tokenId] = $token; + } + + return $this->resolvedTokens[$tokenId] = $token; } /** @@ -140,9 +213,7 @@ protected function resolveToken($tokenId) */ protected function updateToken($tokenId, $token) { - $name = $this->generateCookieName($tokenId); - - if ($token === $this->cookies->get($name, '')) { + if ($token === $this->resolveToken($tokenId, true)) { unset($this->transientTokens[$tokenId]); } else { $this->transientTokens[$tokenId] = $token; @@ -154,8 +225,101 @@ protected function updateToken($tokenId, $token) * * @return string */ - protected function generateCookieName($tokenId) + protected function getTokenCookieValue($tokenId) + { + $name = $this->generateTokenCookieName($tokenId); + + return $this->cookies->get($name, ''); + } + + /** + * @param string $tokenId + * @param string $token + * + * @return Cookie + */ + protected function createTokenCookie($tokenId, $token) + { + $name = $this->generateTokenCookieName($tokenId); + + return new Cookie($name, $token, 0, null, null, $this->secure, false); + } + + /** + * @param string $tokenId + * + * @return string + */ + protected function generateTokenCookieName($tokenId) + { + $encodedTokenId = rtrim(strtr(base64_encode($tokenId), '+/', '-_'), '='); + + return sprintf('_csrf/%s/%s', $this->secure ? 'secure' : 'insecure', $encodedTokenId); + } + + /** + * @param string $tokenId + * + * @return string + */ + protected function getVerificationCookieValue($tokenId) + { + $name = $this->generateVerificationCookieName($tokenId); + + return $this->cookies->get($name, ''); + } + + /** + * @param string $tokenId + * @param string $token + * + * @return Cookie + */ + protected function createVerificationCookie($tokenId, $token) + { + $name = $this->generateVerificationCookieName($tokenId); + $value = $this->generateVerificationCookieValue($tokenId, $token); + + return new Cookie($name, $value, 0, null, null, $this->secure, true); + } + + /** + * @param string $tokenId + * + * @return string + */ + protected function generateVerificationCookieName($tokenId) + { + return $this->generateTokenCookieName($tokenId).'/verify'; + } + + /** + * @param string $tokenId + * @param string $token + * + * @return string + */ + protected function generateVerificationCookieValue($tokenId, $token) + { + if ('' === $token) { + return ''; + } + + $expires = time() + $this->ttl; + $hash = $this->generateVerificationHash($tokenId, $token, $expires); + + return $expires.self::COOKIE_DELIMITER.$hash; + } + + /** + * @param string $tokenId + * @param string $token + * @param int $expires + * + * @return string + */ + protected function generateVerificationHash($tokenId, $token, $expires) { - return sprintf('_csrf/%s/%s', $this->secure ? 'insecure' : 'secure', $tokenId); + return hash_hmac('sha256', $tokenId.$token.$expires, $this->secret); } } diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php index e62a4b5a76110..e2e2c5662d6cd 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Csrf\TokenStorage; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Security\Core\Exception\InvalidArgumentException; /** * Creates CSRF token storages based on the requests cookies. @@ -20,11 +21,39 @@ */ class CookieTokenStorageFactory implements TokenStorageFactoryInterface { + /** + * @var string + */ + private $secret; + + /** + * @var int + */ + private $ttl; + + /** + * @param string $secret + * @param int $ttl + */ + public function __construct($secret, $ttl = null) + { + $this->secret = (string) $secret; + $this->ttl = $ttl === null ? 60 * 60 : (int) $ttl; + + if ('' === $this->secret) { + throw new InvalidArgumentException('Secret must be a non-empty string'); + } + + if ($this->ttl < 60) { + throw new InvalidArgumentException('TTL must be an integer greater than or equal to 60'); + } + } + /** * {@inheritdoc} */ public function createTokenStorage(Request $request) { - return new CookieTokenStorage($request->cookies, $request->isSecure()); + return new CookieTokenStorage($request->cookies, $request->isSecure(), $this->secret, $this->ttl); } } From 6e19ef96a5df17e1703387f1f689266504a26e01 Mon Sep 17 00:00:00 2001 From: Oliver Hoff Date: Mon, 26 Sep 2016 12:10:27 +0200 Subject: [PATCH 09/10] rework CookieTokenStorage --- .../Csrf/TokenStorage/CookieTokenStorage.php | 205 +++++++++--------- .../CookieTokenStorageFactory.php | 2 +- 2 files changed, 108 insertions(+), 99 deletions(-) diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php index c444f84717af6..3d4353976f649 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php @@ -12,7 +12,6 @@ namespace Symfony\Component\Security\Csrf\TokenStorage; use Symfony\Component\HttpFoundation\Cookie; -use Symfony\Component\HttpFoundation\ParameterBag; use Symfony\Component\Security\Core\Exception\InvalidArgumentException; use Symfony\Component\Security\Csrf\Exception\TokenNotFoundException; @@ -31,22 +30,22 @@ class CookieTokenStorage implements TokenStorageInterface const COOKIE_DELIMITER = '_'; /** - * @var array + * @var array A map of tokens to be written in the response */ private $transientTokens = array(); /** - * @var array + * @var array A map of tokens extracted from cookies and verified */ - private $resolvedTokens = array(); + private $extractedTokens = array(); /** * @var array */ - private $refreshTokens = array(); + private $nonces = array(); /** - * @var ParameterBag + * @var array */ private $cookies; @@ -66,14 +65,14 @@ class CookieTokenStorage implements TokenStorageInterface private $ttl; /** - * @param ParameterBag $cookies - * @param bool $secure - * @param string $secret - * @param int $ttl + * @param string $cookies The raw HTTP Cookie header + * @param bool $secure + * @param string $secret + * @param int $ttl */ - public function __construct(ParameterBag $cookies, $secure, $secret, $ttl = null) + public function __construct($cookies, $secure, $secret, $ttl = null) { - $this->cookies = $cookies; + $this->cookies = self::parseCookieHeader($cookies); $this->secure = (bool) $secure; $this->secret = (string) $secret; $this->ttl = $ttl === null ? 60 * 60 : (int) $ttl; @@ -120,7 +119,10 @@ public function setToken($tokenId, $token) throw new InvalidArgumentException('Empty tokens are not allowed'); } - $this->updateToken($tokenId, $token); + // we need to resolve the token first to record the nonces + $this->resolveToken($tokenId); + + $this->transientTokens[$tokenId] = $token; } /** @@ -130,33 +132,28 @@ public function removeToken($tokenId) { $token = $this->resolveToken($tokenId); - $this->updateToken($tokenId, ''); + $this->transientTokens[$tokenId] = ''; return '' === $token ? null : $token; } /** - * @return array + * @return Cookie[] */ public function createCookies() { $cookies = array(); foreach ($this->transientTokens as $tokenId => $token) { - // FIXME empty tokens are handled by the http foundations cookie class - // and are recognized as a "delete" cookie - // the problem is the that the value of deleted cookies get set to - // the string "deleted" and not the empty string - $cookies[] = $this->createTokenCookie($tokenId, $token); - $cookies[] = $this->createVerificationCookie($tokenId, $token); - } - - foreach ($this->refreshTokens as $tokenId => $token) { - if (isset($this->transientTokens[$tokenId])) { - continue; + if (isset($this->nonces[$tokenId])) { + foreach (array_keys($this->nonces[$tokenId]) as $nonce) { + $cookies[] = $this->createDeleteCookie($tokenId, $nonce); + } } - $cookies[] = $this->createVerificationCookie($tokenId, $token); + if ($token !== '') { + $cookies[] = $this->createCookie($tokenId, $token); + } } return $cookies; @@ -164,72 +161,90 @@ public function createCookies() /** * @param string $tokenId - * @param bool $excludeTransient * * @return string */ - protected function resolveToken($tokenId, $excludeTransient = false) + protected function resolveToken($tokenId) { - if (!$excludeTransient && isset($this->transientTokens[$tokenId])) { + if (isset($this->transientTokens[$tokenId])) { return $this->transientTokens[$tokenId]; } - if (isset($this->resolvedTokens[$tokenId])) { - return $this->resolvedTokens[$tokenId]; + if (isset($this->extractedTokens[$tokenId])) { + return $this->extractedTokens[$tokenId]; } - $this->resolvedTokens[$tokenId] = ''; + $this->extractedTokens[$tokenId] = ''; - $token = $this->getTokenCookieValue($tokenId); - if ('' === $token) { + $prefix = $this->generateCookieName($tokenId, ''); + $prefixLength = strlen($prefix); + $cookies = $this->findCookiesByPrefix($prefix); + + // record the nonces used, so we can delete all obsolete cookies of this + // token id, if necessary + foreach ($cookies as $cookie) { + $this->nonces[$tokenId][substr($cookie[0], $prefixLength)] = true; + } + + // if there is more than one cookie for the prefix, we get cookie tossed maybe + if (count($cookies) != 1) { return ''; } - $parts = explode(self::COOKIE_DELIMITER, $this->getVerificationCookieValue($tokenId), 2); - if (count($parts) != 2) { + $parts = explode(self::COOKIE_DELIMITER, $cookies[0][1], 3); + if (count($parts) != 3) { return ''; } + list($expires, $signature, $token) = $parts; - list($expires, $hash) = $parts; + // expired token $time = time(); if (!ctype_digit($expires) || $expires < $time) { return ''; } - if (!hash_equals($this->generateVerificationHash($tokenId, $token, $expires), $hash)) { + + // invalid signature + $nonce = substr($cookies[0][0], $prefixLength); + if (!hash_equals($this->generateSignature($tokenId, $token, $expires, $nonce), $signature)) { return ''; } $time += $this->ttl / 2; if ($expires < $time) { - $this->refreshTokens[$tokenId] = $token; + $this->transientTokens[$tokenId] = $token; } - return $this->resolvedTokens[$tokenId] = $token; + return $this->extractedTokens[$tokenId] = $token; } /** - * @param string $tokenId - * @param string $token + * @param string $prefix + * + * @return array */ - protected function updateToken($tokenId, $token) + protected function findCookiesByPrefix($prefix) { - if ($token === $this->resolveToken($tokenId, true)) { - unset($this->transientTokens[$tokenId]); - } else { - $this->transientTokens[$tokenId] = $token; + $cookies = array(); + foreach ($this->cookies as $cookie) { + if (0 === strpos($cookie[0], $prefix)) { + $cookies[] = $cookie; + } } + + return $cookies; } /** * @param string $tokenId + * @param string $nonce * - * @return string + * @return Cookie */ - protected function getTokenCookieValue($tokenId) + protected function createDeleteCookie($tokenId, $nonce) { - $name = $this->generateTokenCookieName($tokenId); + $name = $this->generateCookieName($tokenId, $nonce); - return $this->cookies->get($name, ''); + return new Cookie($name, '', 0, null, null, $this->secure, true); } /** @@ -238,88 +253,82 @@ protected function getTokenCookieValue($tokenId) * * @return Cookie */ - protected function createTokenCookie($tokenId, $token) + protected function createCookie($tokenId, $token) { - $name = $this->generateTokenCookieName($tokenId); + $expires = time() + $this->ttl; + $nonce = self::encodeBase64Url(random_bytes(6)); + $signature = $this->generateSignature($tokenId, $token, $expires, $nonce); - return new Cookie($name, $token, 0, null, null, $this->secure, false); - } + $this->nonces[$tokenId][$nonce] = true; - /** - * @param string $tokenId - * - * @return string - */ - protected function generateTokenCookieName($tokenId) - { - $encodedTokenId = rtrim(strtr(base64_encode($tokenId), '+/', '-_'), '='); + $name = $this->generateCookieName($tokenId, $nonce); + $value = $expires.self::COOKIE_DELIMITER.$signature.self::COOKIE_DELIMITER.$token; - return sprintf('_csrf/%s/%s', $this->secure ? 'secure' : 'insecure', $encodedTokenId); + return new Cookie($name, $value, 0, null, null, $this->secure, true); } /** * @param string $tokenId + * @param string $nonce * * @return string */ - protected function getVerificationCookieValue($tokenId) + protected function generateCookieName($tokenId, $nonce) { - $name = $this->generateVerificationCookieName($tokenId); - - return $this->cookies->get($name, ''); + return sprintf( + '_csrf_%s_%s_%s', + (int) $this->secure, + self::encodeBase64Url($tokenId), + $nonce + ); } /** * @param string $tokenId * @param string $token - * - * @return Cookie - */ - protected function createVerificationCookie($tokenId, $token) - { - $name = $this->generateVerificationCookieName($tokenId); - $value = $this->generateVerificationCookieValue($tokenId, $token); - - return new Cookie($name, $value, 0, null, null, $this->secure, true); - } - - /** - * @param string $tokenId + * @param int $expires + * @param string $nonce * * @return string */ - protected function generateVerificationCookieName($tokenId) + protected function generateSignature($tokenId, $token, $expires, $nonce) { - return $this->generateTokenCookieName($tokenId).'/verify'; + return hash_hmac('sha256', $tokenId.$token.$expires.$nonce.$this->secure, $this->secret); } /** - * @param string $tokenId - * @param string $token + * @param string $header * - * @return string + * @return array */ - protected function generateVerificationCookieValue($tokenId, $token) + public static function parseCookieHeader($header) { - if ('' === $token) { - return ''; + $header = trim((string) $header); + if ('' === $header) { + return array(); } - $expires = time() + $this->ttl; - $hash = $this->generateVerificationHash($tokenId, $token, $expires); + $cookies = array(); + foreach (explode(';', $header) as $cookie) { + if (false === strpos($cookie, '=')) { + continue; + } + + $cookies[] = array_map(function ($item) { + return urldecode(trim($item, ' "')); + }, explode('=', $cookie, 2)); + } - return $expires.self::COOKIE_DELIMITER.$hash; + return $cookies; } /** - * @param string $tokenId - * @param string $token - * @param int $expires + * @param string $data * * @return string */ - protected function generateVerificationHash($tokenId, $token, $expires) + public static function encodeBase64Url($data) { - return hash_hmac('sha256', $tokenId.$token.$expires, $this->secret); + return rtrim(strtr(base64_encode($data), '+/', '-_'), '='); } } diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php index e2e2c5662d6cd..930a011ef826d 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php @@ -54,6 +54,6 @@ public function __construct($secret, $ttl = null) */ public function createTokenStorage(Request $request) { - return new CookieTokenStorage($request->cookies, $request->isSecure(), $this->secret, $this->ttl); + return new CookieTokenStorage($request->headers->get('Cookie'), $request->isSecure(), $this->secret, $this->ttl); } } From 99dde3b93f8c8106ef09558217c84381f9eb0ae6 Mon Sep 17 00:00:00 2001 From: Oliver Hoff Date: Wed, 28 Sep 2016 13:33:30 +0200 Subject: [PATCH 10/10] code style --- .../Csrf/TokenStorage/CookieTokenStorage.php | 4 ++-- .../TokenStorage/CookieTokenStorageFactory.php | 2 +- .../TokenStorage/CookieTokenStorageListener.php | 9 ++++++--- .../TokenStorage/RequestStackTokenStorage.php | 15 ++++----------- 4 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php index 3d4353976f649..f306742e39ce4 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorage.php @@ -75,7 +75,7 @@ public function __construct($cookies, $secure, $secret, $ttl = null) $this->cookies = self::parseCookieHeader($cookies); $this->secure = (bool) $secure; $this->secret = (string) $secret; - $this->ttl = $ttl === null ? 60 * 60 : (int) $ttl; + $this->ttl = null === $ttl ? 60 * 60 : (int) $ttl; if ('' === $this->secret) { throw new InvalidArgumentException('Secret must be a non-empty string'); @@ -151,7 +151,7 @@ public function createCookies() } } - if ($token !== '') { + if ('' !== $token) { $cookies[] = $this->createCookie($tokenId, $token); } } diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php index 930a011ef826d..2cd04c6b58125 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageFactory.php @@ -38,7 +38,7 @@ class CookieTokenStorageFactory implements TokenStorageFactoryInterface public function __construct($secret, $ttl = null) { $this->secret = (string) $secret; - $this->ttl = $ttl === null ? 60 * 60 : (int) $ttl; + $this->ttl = null === $ttl ? 60 * 60 : (int) $ttl; if ('' === $this->secret) { throw new InvalidArgumentException('Secret must be a non-empty string'); diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php index 96abd1dac959a..bb81df6086a69 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/CookieTokenStorageListener.php @@ -26,6 +26,11 @@ */ class CookieTokenStorageListener implements EventSubscriberInterface { + /** + * @var string + */ + const DEFAULT_TOKEN_STORAGE_KEY = '_csrf_token_storage'; + /** * @var string */ @@ -36,9 +41,7 @@ class CookieTokenStorageListener implements EventSubscriberInterface */ public function __construct($tokenStorageKey = null) { - // TODO should this class get its own DEFAULT_TOKEN_STORAGE_KEY constant? - // if no, where should the sole constant be put? - $this->tokenStorageKey = $tokenStorageKey === null ? RequestStackTokenStorage::DEFAULT_TOKEN_STORAGE_KEY : $tokenStorageKey; + $this->tokenStorageKey = null === $tokenStorageKey ? self::DEFAULT_TOKEN_STORAGE_KEY : $tokenStorageKey; } /** diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php index 23708e77e03e4..dae342e9adce0 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/RequestStackTokenStorage.php @@ -49,11 +49,8 @@ class RequestStackTokenStorage extends AbstractTokenStorageProxy * @param TokenStorageFactoryInterface $factory * @param string|null $tokenStorageKey */ - public function __construct( - RequestStack $requestStack, - TokenStorageFactoryInterface $factory, - $tokenStorageKey = null - ) { + public function __construct(RequestStack $requestStack, TokenStorageFactoryInterface $factory, $tokenStorageKey = null) + { $this->requestStack = $requestStack; $this->factory = $factory; $this->tokenStorageKey = $tokenStorageKey === null ? self::DEFAULT_TOKEN_STORAGE_KEY : $tokenStorageKey; @@ -64,7 +61,6 @@ public function __construct( */ public function getProxiedTokenStorage() { - // TODO use master or current request? $request = $this->requestStack->getMasterRequest(); if (!$request) { @@ -77,11 +73,8 @@ public function getProxiedTokenStorage() return $storage; } - if ($storage !== null) { - throw new UnexpectedValueException(sprintf( - 'Expected null or an instance of "Symfony\\Component\\Security\\Csrf\\TokenStorage\\TokenStorageInterface", got "%s"', - is_object($storage) ? get_class($storage) : gettype($storage) - )); + if (null !== $storage) { + throw new UnexpectedValueException(sprintf('Expected null or an implementation of "%s", got "%s"', TokenStorageInterface::class, is_object($storage) ? get_class($storage) : gettype($storage))); } $storage = $this->factory->createTokenStorage($request);