Skip to content

[Security/Http] Check tokens before loading users from providers #49078

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 24, 2023
Merged

[Security/Http] Check tokens before loading users from providers #49078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 38 additions & 6 deletions src/Symfony/Component/Security/Core/Signature/SignatureHasher.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,24 +45,48 @@ public function __construct(PropertyAccessorInterface $propertyAccessor, array $
}

/**
* Verifies the hash using the provided user and expire time.
* Verifies the hash using the provided user identifier and expire time.
*
* This method must be called before the user object is loaded from a provider.
*
* @param int $expires The expiry time as a unix timestamp
* @param string $hash The plaintext hash provided by the request
*
* @throws InvalidSignatureException If the signature does not match the provided parameters
* @throws ExpiredSignatureException If the signature is no longer valid
*/
public function verifySignatureHash(UserInterface $user, int $expires, string $hash): void
public function acceptSignatureHash(string $userIdentifier, int $expires, string $hash): void
{
if (!hash_equals($hash, $this->computeSignatureHash($user, $expires))) {
if ($expires < time()) {
throw new ExpiredSignatureException('Signature has expired.');
}
$hmac = substr($hash, 0, 44);
$payload = substr($hash, 44).':'.$expires.':'.$userIdentifier;

if (!hash_equals($hmac, $this->generateHash($payload))) {
throw new InvalidSignatureException('Invalid or expired signature.');
}
}

/**
* Verifies the hash using the provided user and expire time.
*
* @param int $expires The expiry time as a unix timestamp
* @param string $hash The plaintext hash provided by the request
*
* @throws InvalidSignatureException If the signature does not match the provided parameters
* @throws ExpiredSignatureException If the signature is no longer valid
*/
public function verifySignatureHash(UserInterface $user, int $expires, string $hash): void
{
if ($expires < time()) {
throw new ExpiredSignatureException('Signature has expired.');
}

if (!hash_equals($hash, $this->computeSignatureHash($user, $expires))) {
throw new InvalidSignatureException('Invalid or expired signature.');
}

if ($this->expiredSignaturesStorage && $this->maxUses) {
if ($this->expiredSignaturesStorage->countUsages($hash) >= $this->maxUses) {
throw new ExpiredSignatureException(sprintf('Signature can only be used "%d" times.', $this->maxUses));
Expand All @@ -79,7 +103,8 @@ public function verifySignatureHash(UserInterface $user, int $expires, string $h
*/
public function computeSignatureHash(UserInterface $user, int $expires): string
{
$signatureFields = [base64_encode(method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername()), $expires];
$userIdentifier = method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername();
$fieldsHash = hash_init('sha256');

foreach ($this->signatureProperties as $property) {
$value = $this->propertyAccessor->getValue($user, $property) ?? '';
Expand All @@ -90,9 +115,16 @@ public function computeSignatureHash(UserInterface $user, int $expires): string
if (!\is_scalar($value) && !(\is_object($value) && method_exists($value, '__toString'))) {
throw new \InvalidArgumentException(sprintf('The property path "%s" on the user object "%s" must return a value that can be cast to a string, but "%s" was returned.', $property, \get_class($user), get_debug_type($value)));
}
$signatureFields[] = base64_encode($value);
hash_update($fieldsHash, ':'.base64_encode($value));
}

return base64_encode(hash_hmac('sha256', implode(':', $signatureFields), $this->secret));
$fieldsHash = strtr(base64_encode(hash_final($fieldsHash, true)), '+/=', '-_~');

return $this->generateHash($fieldsHash.':'.$expires.':'.$userIdentifier).$fieldsHash;
}

private function generateHash(string $tokenValue): string
{
return strtr(base64_encode(hash_hmac('sha256', $tokenValue, $this->secret, true)), '+/=', '-_~');
}
}
22 changes: 11 additions & 11 deletions src/Symfony/Component/Security/Http/LoginLink/LoginLinkHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,16 @@ public function consumeLoginLink(Request $request): UserInterface
{
$userIdentifier = $request->get('user');

if (!$hash = $request->get('hash')) {
throw new InvalidLoginLinkException('Missing "hash" parameter.');
}
if (!$expires = $request->get('expires')) {
throw new InvalidLoginLinkException('Missing "expires" parameter.');
}

try {
$this->signatureHasher->acceptSignatureHash($userIdentifier, $expires, $hash);

// @deprecated since Symfony 5.3, change to $this->userProvider->loadUserByIdentifier() in 6.0
if (method_exists($this->userProvider, 'loadUserByIdentifier')) {
$user = $this->userProvider->loadUserByIdentifier($userIdentifier);
Expand All @@ -93,19 +102,10 @@ public function consumeLoginLink(Request $request): UserInterface

$user = $this->userProvider->loadUserByUsername($userIdentifier);
}
} catch (UserNotFoundException $exception) {
throw new InvalidLoginLinkException('User not found.', 0, $exception);
}

if (!$hash = $request->get('hash')) {
throw new InvalidLoginLinkException('Missing "hash" parameter.');
}
if (!$expires = $request->get('expires')) {
throw new InvalidLoginLinkException('Missing "expires" parameter.');
}

try {
$this->signatureHasher->verifySignatureHash($user, $expires, $hash);
} catch (UserNotFoundException $e) {
throw new InvalidLoginLinkException('User not found.', 0, $e);
} catch (ExpiredSignatureException $e) {
throw new ExpiredLoginLinkException(ucfirst(str_ireplace('signature', 'login link', $e->getMessage())), 0, $e);
} catch (InvalidSignatureException $e) {
Expand Down
45 changes: 24 additions & 21 deletions src/Symfony/Component/Security/Http/RememberMe/PersistentRememberMeHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,16 @@ public function __construct(TokenProviderInterface $tokenProvider, string $secre
*/
public function createRememberMeCookie(UserInterface $user): void
{
$series = base64_encode(random_bytes(64));
$tokenValue = $this->generateHash(base64_encode(random_bytes(64)));
$series = random_bytes(66);
$tokenValue = strtr(base64_encode(substr($series, 33)), '+/=', '-_~');
$series = strtr(base64_encode(substr($series, 0, 33)), '+/=', '-_~');
$token = new PersistentToken(\get_class($user), method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername(), $series, $tokenValue, new \DateTime());

$this->tokenProvider->createNewToken($token);
$this->createCookie(RememberMeDetails::fromPersistentToken($token, time() + $this->options['lifetime']));
}

/**
* {@inheritdoc}
*/
public function processRememberMe(RememberMeDetails $rememberMeDetails, UserInterface $user): void
public function consumeRememberMeCookie(RememberMeDetails $rememberMeDetails): UserInterface
{
if (!str_contains($rememberMeDetails->getValue(), ':')) {
throw new AuthenticationException('The cookie is incorrectly formatted.');
Expand All @@ -86,18 +84,28 @@ public function processRememberMe(RememberMeDetails $rememberMeDetails, UserInte
throw new AuthenticationException('The cookie has expired.');
}

return parent::consumeRememberMeCookie($rememberMeDetails->withValue($persistentToken->getLastUsed()->getTimestamp().':'.$rememberMeDetails->getValue().':'.$persistentToken->getClass()));
}

public function processRememberMe(RememberMeDetails $rememberMeDetails, UserInterface $user): void
{
[$lastUsed, $series, $tokenValue, $class] = explode(':', $rememberMeDetails->getValue(), 4);
$persistentToken = new PersistentToken($class, $rememberMeDetails->getUserIdentifier(), $series, $tokenValue, new \DateTime('@'.$lastUsed));

// if a token was regenerated less than a minute ago, there is no need to regenerate it
// if multiple concurrent requests reauthenticate a user we do not want to update the token several times
if ($persistentToken->getLastUsed()->getTimestamp() + 60 < time()) {
$tokenValue = $this->generateHash(base64_encode(random_bytes(64)));
$tokenLastUsed = new \DateTime();
if ($this->tokenVerifier) {
$this->tokenVerifier->updateExistingToken($persistentToken, $tokenValue, $tokenLastUsed);
}
$this->tokenProvider->updateToken($series, $tokenValue, $tokenLastUsed);

$this->createCookie($rememberMeDetails->withValue($series.':'.$tokenValue));
if ($persistentToken->getLastUsed()->getTimestamp() + 60 >= time()) {
return;
}

$tokenValue = strtr(base64_encode(random_bytes(33)), '+/=', '-_~');
$tokenLastUsed = new \DateTime();
if ($this->tokenVerifier) {
$this->tokenVerifier->updateExistingToken($persistentToken, $tokenValue, $tokenLastUsed);
}
$this->tokenProvider->updateToken($series, $tokenValue, $tokenLastUsed);

$this->createCookie($rememberMeDetails->withValue($series.':'.$tokenValue));
}

/**
Expand All @@ -113,7 +121,7 @@ public function clearRememberMeCookie(): void
}

$rememberMeDetails = RememberMeDetails::fromRawCookie($cookie);
[$series, ] = explode(':', $rememberMeDetails->getValue());
[$series] = explode(':', $rememberMeDetails->getValue());
$this->tokenProvider->deleteTokenBySeries($series);
}

Expand All @@ -124,9 +132,4 @@ public function getTokenProvider(): TokenProviderInterface
{
return $this->tokenProvider;
}

private function generateHash(string $tokenValue): string
{
return hash_hmac('sha256', $tokenValue, $this->secret);
}
}
7 changes: 4 additions & 3 deletions src/Symfony/Component/Security/Http/RememberMe/RememberMeDetails.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ public function __construct(string $userFqcn, string $userIdentifier, int $expir

public static function fromRawCookie(string $rawCookie): self
{
$cookieParts = explode(self::COOKIE_DELIMITER, base64_decode($rawCookie), 4);
$cookieParts = explode(self::COOKIE_DELIMITER, $rawCookie, 4);
if (4 !== \count($cookieParts)) {
throw new AuthenticationException('The cookie contains invalid data.');
}
if (false === $cookieParts[1] = base64_decode($cookieParts[1], true)) {
if (false === $cookieParts[1] = base64_decode(strtr($cookieParts[1], '-_~', '+/='), true)) {
throw new AuthenticationException('The user identifier contains a character from outside the base64 alphabet.');
}
$cookieParts[0] = strtr($cookieParts[0], '.', '\\');

return new static(...$cookieParts);
}
Expand Down Expand Up @@ -83,6 +84,6 @@ public function getValue(): string
public function toString(): string
{
// $userIdentifier is encoded because it might contain COOKIE_DELIMITER, we assume other values don't
return base64_encode(implode(self::COOKIE_DELIMITER, [$this->userFqcn, base64_encode($this->userIdentifier), $this->expires, $this->value]));
return implode(self::COOKIE_DELIMITER, [strtr($this->userFqcn, '\\', '.'), strtr(base64_encode($this->userIdentifier), '+/=', '-_~'), $this->expires, $this->value]);
}
}
16 changes: 13 additions & 3 deletions src/Symfony/Component/Security/Http/RememberMe/SignatureRememberMeHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,19 @@ public function createRememberMeCookie(UserInterface $user): void
$this->createCookie($details);
}

/**
* {@inheritdoc}
*/
public function consumeRememberMeCookie(RememberMeDetails $rememberMeDetails): UserInterface
{
try {
$this->signatureHasher->acceptSignatureHash($rememberMeDetails->getUserIdentifier(), $rememberMeDetails->getExpires(), $rememberMeDetails->getValue());
} catch (InvalidSignatureException $e) {
throw new AuthenticationException('The cookie\'s hash is invalid.', 0, $e);
} catch (ExpiredSignatureException $e) {
throw new AuthenticationException('The cookie has expired.', 0, $e);
}

return parent::consumeRememberMeCookie($rememberMeDetails);
}

public function processRememberMe(RememberMeDetails $rememberMeDetails, UserInterface $user): void
{
try {
Expand Down
38 changes: 15 additions & 23 deletions src/Symfony/Component/Security/Http/Tests/LoginLink/LoginLinkHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ protected function setUp(): void

/**
* @group time-sensitive
*
* @dataProvider provideCreateLoginLinkData
*/
public function testCreateLoginLink($user, array $extraProperties, Request $request = null)
Expand All @@ -68,7 +69,7 @@ public function testCreateLoginLink($user, array $extraProperties, Request $requ
// allow a small expiration offset to avoid time-sensitivity
&& abs(time() + 600 - $parameters['expires']) <= 1
// make sure hash is what we expect
&& $parameters['hash'] === $this->createSignatureHash('weaverryan', $parameters['expires'], array_values($extraProperties));
&& $parameters['hash'] === $this->createSignatureHash('weaverryan', $parameters['expires'], $extraProperties);
}),
UrlGeneratorInterface::ABSOLUTE_URL
)
Expand Down Expand Up @@ -115,7 +116,7 @@ public function provideCreateLoginLinkData()
public function testConsumeLoginLink()
{
$expires = time() + 500;
$signature = $this->createSignatureHash('weaverryan', $expires, ['ryan@symfonycasts.com', 'pwhash']);
$signature = $this->createSignatureHash('weaverryan', $expires);
$request = Request::create(sprintf('/login/verify?user=weaverryan&hash=%s&expires=%d', $signature, $expires));

$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
Expand All @@ -131,44 +132,37 @@ public function testConsumeLoginLink()

public function testConsumeLoginLinkWithExpired()
{
$this->expectException(ExpiredLoginLinkException::class);
$expires = time() - 500;
$signature = $this->createSignatureHash('weaverryan', $expires, ['ryan@symfonycasts.com', 'pwhash']);
$signature = $this->createSignatureHash('weaverryan', $expires);
$request = Request::create(sprintf('/login/verify?user=weaverryan&hash=%s&expires=%d', $signature, $expires));

$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
$this->userProvider->createUser($user);

$linker = $this->createLinker(['max_uses' => 3]);
$this->expectException(ExpiredLoginLinkException::class);
$linker->consumeLoginLink($request);
}

public function testConsumeLoginLinkWithUserNotFound()
{
$this->expectException(InvalidLoginLinkException::class);
$request = Request::create('/login/verify?user=weaverryan&hash=thehash&expires=10000');
$request = Request::create('/login/verify?user=weaverryan&hash=thehash&expires='.(time() + 500));

$linker = $this->createLinker();
$this->expectException(InvalidLoginLinkException::class);
$linker->consumeLoginLink($request);
}

public function testConsumeLoginLinkWithDifferentSignature()
{
$this->expectException(InvalidLoginLinkException::class);
$request = Request::create(sprintf('/login/verify?user=weaverryan&hash=fake_hash&expires=%d', time() + 500));

$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
$this->userProvider->createUser($user);

$linker = $this->createLinker();
$this->expectException(InvalidLoginLinkException::class);
$linker->consumeLoginLink($request);
}

public function testConsumeLoginLinkExceedsMaxUsage()
{
$this->expectException(ExpiredLoginLinkException::class);
$expires = time() + 500;
$signature = $this->createSignatureHash('weaverryan', $expires, ['ryan@symfonycasts.com', 'pwhash']);
$signature = $this->createSignatureHash('weaverryan', $expires);
$request = Request::create(sprintf('/login/verify?user=weaverryan&hash=%s&expires=%d', $signature, $expires));

$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
Expand All @@ -179,6 +173,7 @@ public function testConsumeLoginLinkExceedsMaxUsage()
$this->expiredLinkCache->save($item);

$linker = $this->createLinker(['max_uses' => 3]);
$this->expectException(ExpiredLoginLinkException::class);
$linker->consumeLoginLink($request);
}

Expand Down Expand Up @@ -206,15 +201,12 @@ public function testConsumeLoginLinkWithMissingExpiration()
$linker->consumeLoginLink($request);
}

private function createSignatureHash(string $username, int $expires, array $extraFields): string
private function createSignatureHash(string $username, int $expires, array $extraFields = ['emailProperty' => 'ryan@symfonycasts.com', 'passwordProperty' => 'pwhash']): string
{
$fields = [base64_encode($username), $expires];
foreach ($extraFields as $extraField) {
$fields[] = base64_encode($extraField);
}
$hasher = new SignatureHasher($this->propertyAccessor, array_keys($extraFields), 's3cret');
$user = new TestLoginLinkHandlerUser($username, $extraFields['emailProperty'] ?? '', $extraFields['passwordProperty'] ?? '', $extraFields['lastAuthenticatedAt'] ?? null);

// matches hash logic in the class
return base64_encode(hash_hmac('sha256', implode(':', $fields), 's3cret'));
return $hasher->computeSignatureHash($user, $expires);
}

private function createLinker(array $options = [], array $extraProperties = ['emailProperty', 'passwordProperty']): LoginLinkHandler
Expand Down Expand Up @@ -298,7 +290,7 @@ public function loadUserByIdentifier(string $userIdentifier): TestLoginLinkHandl

public function refreshUser(UserInterface $user): TestLoginLinkHandlerUser
{
return $this->users[$username];
return $this->users[$user->getUserIdentifier()];
}

public function supportsClass(string $class): bool
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Security/Http/Tests/RememberMe/PersistentRememberMeHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ public function testConsumeRememberMeCookieValid()

/** @var Cookie $cookie */
$cookie = $this->request->attributes->get(ResponseListener::COOKIE_ATTR_NAME);
$rememberParts = explode(':', base64_decode($rememberMeDetails->toString()), 4);
$cookieParts = explode(':', base64_decode($cookie->getValue()), 4);
$rememberParts = explode(':', $rememberMeDetails->toString(), 4);
$cookieParts = explode(':', $cookie->getValue(), 4);

$this->assertSame($rememberParts[0], $cookieParts[0]); // class
$this->assertSame($rememberParts[1], $cookieParts[1]); // identifier
Expand Down
Loading