10000 [Security/Http] Check tokens before loading users from providers · symfony/symfony@f00b9ed · GitHub
[go: up one dir, main page]
Skip to content

Commit f00b9ed

Browse files
nicolas-grekasnicolas-grekas
committed
[Security/Http] Check tokens before loading users from providers
1 parent 01344e3 commit f00b9ed

9 files changed

+117
-104
lines changed

‎src/Symfony/Component/Security/Core/Signature/SignatureHasher.php

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,24 +45,48 @@ public function __construct(PropertyAccessorInterface $propertyAccessor, array $
4545
}
4646

4747
/**
48-
* Verifies the hash using the provided user and expire time.
48+
* Verifies the hash using the provided user identifier and expire time.
49+
*
50+
* This method must be called before the user object is loaded from a provider.
4951
*
5052
* @param int $expires The expiry time as a unix timestamp
5153
* @param string $hash The plaintext hash provided by the request
5254
*
5355
* @throws InvalidSignatureException If the signature does not match the provided parameters
5456
* @throws ExpiredSignatureException If the signature is no longer valid
5557
*/
56-
public function verifySignatureHash(UserInterface $user, int $expires, string $hash): void
58+
public function acceptSignatureHash(string $userIdentifier, int $expires, string $hash): void
5759
{
58-
if (!hash_equals($hash, $this->computeSignatureHash($user, $expires))) {
60+
if ($expires < time()) {
61+
throw new ExpiredSignatureException('Signature has expired.');
62+
}
63+
$hmac = substr($hash, 0, 44);
64+
$payload = substr($hash, 44).':'.$expires.':'.$userIdentifier;
65+
66+
if (!hash_equals($hmac, strtr(base64_encode(hash_hmac('sha256', $payload, $this->secret, true)), '+/=', '-_~'))) {
5967
throw new InvalidSignatureException('Invalid or expired signature.');
6068
}
69+
}
6170

71+
/**
72+
* Verifies the hash using the provided user and expire time.
73+
*
74+
* @param int $expires The expiry time as a unix timestamp
75+
* @param string $hash The plaintext hash provided by the request
76+
*
77+
* @throws InvalidSignatureException If the signature does not match the provided parameters
78+
* @throws ExpiredSignatureException If the signature is no longer valid
79+
*/
80+
public function verifySignatureHash(UserInterface $user, int $expires, string $hash): void
81+
{
6282
if ($expires < time()) {
6383
throw new ExpiredSignatureException('Signature has expired.');
6484
}
6585

86+
if (!hash_equals($hash, $this->computeSignatureHash($user, $expires))) {
87+
throw new InvalidSignatureException('Invalid or expired signature.');
88+
}
89+
6690
if ($this->expiredSignaturesStorage && $this->maxUses) {
6791
if ($this->expiredSignaturesStorage->countUsages($hash) >= $this->maxUses) {
6892
throw new ExpiredSignatureException(sprintf('Signature can only be used "%d" times.', $this->maxUses));
@@ -79,7 +103,8 @@ public function verifySignatureHash(UserInterface $user, int $expires, string $h
79103
*/
80104
public function computeSignatureHash(UserInterface $user, int $expires): string
81105
{
82-
$signatureFields = [base64_encode(method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername()), $expires];
106+
$userIdentifier = method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername();
107+
$fieldsHash = hash_init('sha256');
83108

84109
foreach ($this->signatureProperties as $property) {
85110
$value = $this->propertyAccessor->getValue($user, $property) ?? '';
@@ -90,9 +115,11 @@ public function computeSignatureHash(UserInterface $user, int $expires): string
90115
if (!\is_scalar($value) && !(\is_object($value) && method_exists($value, '__toString'))) {
91116
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)));
92117
}
93-
$signatureFields[] = base64_encode($value);
118+
hash_update($fieldsHash, ':'.base64_encode($value));
94119
}
95120

96-
return base64_encode(hash_hmac('sha256', implode(':', $signatureFields), $this->secret));
121+
$fieldsHash = strtr(base64_encode(hash_final($fieldsHash, true)), '+/=', '-_~');
122+
123+
return strtr(base64_encode(hash_hmac('sha256', $fieldsHash.':'.$expires.':'.$userIdentifier, $this->secret, true)), '+/=', '-_~').$fieldsHash;
97124
}
98125
}

‎src/Symfony/Component/Security/Http/LoginLink/LoginLinkHandler.php

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,16 @@ public function consumeLoginLink(Request $request): UserInterface
8484
{
8585
$userIdentifier = $request->get('user');
8686

87+
if (!$hash = $request->get('hash')) {
88+
throw new InvalidLoginLinkException('Missing "hash" parameter.');
89+
}
90+
if (!$expires = $request->get('expires')) {
91+
throw new InvalidLoginLinkException('Missing "expires" parameter.');
92+
}
93+
8794
try {
95+
$this->signatureHasher->acceptSignatureHash($userIdentifier, $expires, $hash);
96+
8897
// @deprecated since Symfony 5.3, change to $this->userProvider->loadUserByIdentifier() in 6.0
8998
if (method_exists($this->userProvider, 'loadUserByIdentifier')) {
9099
$user = $this->userProvider->loadUserByIdentifier($userIdentifier);
@@ -93,19 +102,10 @@ public function consumeLoginLink(Request $request): UserInterface
93102

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

100-
if (!$hash = $request->get('hash')) {
101-
throw new InvalidLoginLinkException('Missing "hash" parameter.');
102-
}
103-
if (!$expires = $request->get('expires')) {
104-
throw new InvalidLoginLinkException('Missing "expires" parameter.');
105-
}
106-
107-
try {
108106
$this->signatureHasher->verifySignatureHash($user, $expires, $hash);
107+
} catch (UserNotFoundException $e) {
108+
throw new InvalidLoginLinkException('User not found.', 0, $e);
109109
} catch (ExpiredSignatureException $e) {
110110
throw new ExpiredLoginLinkException(ucfirst(str_ireplace('signature', 'login link', $e->getMessage())), 0, $e);
111111
} catch (InvalidSignatureException $e) {

‎src/Symfony/Component/Security/Http/RememberMe/PersistentRememberMeHandler.php

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,16 @@ public function __construct(TokenProviderInterface $tokenProvider, string $secre
5353
*/
5454
public function createRememberMeCookie(UserInterface $user): void
5555
{
56-
$series = base64_encode(random_bytes(64));
57-
$tokenValue = $this->generateHash(base64_encode(random_bytes(64)));
56+
$series = random_bytes(66);
57+
$tokenValue = strtr(base64_encode(substr($series, 33)), '+/=', '-_~');
58+
$series = strtr(base64_encode(substr($series, 0, 33)), '+/=', '-_~');
5859
$token = new PersistentToken(\get_class($user), method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername(), $series, $tokenValue, new \DateTime());
5960

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

64-
/**
65-
* {@inheritdoc}
66-
*/
67-
public function processRememberMe(RememberMeDetails $rememberMeDetails, UserInterface $user): void
65+
public function consumeRememberMeCookie(RememberMeDetails $rememberMeDetails): UserInterface
6866
{
6967
if (!str_contains($rememberMeDetails->getValue(), ':')) {
7068
throw new AuthenticationException('The cookie is incorrectly formatted.');
@@ -86,18 +84,28 @@ public function processRememberMe(RememberMeDetails $rememberMeDetails, UserInte
8684
throw new AuthenticationException('The cookie has expired.');
8785
}
8886

87+
return parent::consumeRememberMeCookie($rememberMeDetails->withValue($persistentToken->getLastUsed()->getTimestamp().':'.$rememberMeDetails->getValue().':'.$persistentToken->getClass()));
88+
}
89+
90+
protected function processRememberMe(RememberMeDetails $rememberMeDetails, UserInterface $user): void
91+
{
92+
[$lastUsed, $series, $tokenValue, $class] = explode(':', $rememberMeDetails->getValue(), 4);
93+
$persistentToken = new PersistentToken($class, $rememberMeDetails->getUserIdentifier(), $series, $tokenValue, new \DateTime('@'.$lastUsed));
94+
8995
// if a token was regenerated less than a minute ago, there is no need to regenerate it
9096
// if multiple concurrent requests reauthenticate a user we do not want to update the token several times
91-
if ($persistentToken->getLastUsed()->getTimestamp() + 60 < time()) {
92-
$tokenValue = $this->generateHash(base64_encode(random_bytes(64)));
93-
$tokenLastUsed = new \DateTime();
94-
if ($this->tokenVerifier) {
95-
$this->tokenVerifier->updateExistingToken($persistentToken, $tokenValue, $tokenLastUsed);
96-
}
97-
$this->tokenProvider->updateToken($series, $tokenValue, $tokenLastUsed);
98-
99-
$this->createCookie($rememberMeDetails->withValue($series.':'.$tokenValue));
97+
if ($persistentToken->getLastUsed()->getTimestamp() + 60 >= time()) {
98+
return;
10099
}
100+
101+
$tokenValue = strtr(base64_encode(random_bytes(33)), '+/=', '-_~');
102+
$tokenLastUsed = new \DateTime();
103+
if ($this->tokenVerifier) {
104+
$this->tokenVerifier->updateExistingToken($persistentToken, $tokenValue, $tokenLastUsed);
105+
}
106+
$this->tokenProvider->updateToken($series, $tokenValue, $tokenLastUsed);
107+
108+
$this->createCookie($rememberMeDetails->withValue($series.':'.$tokenValue));
101109
}
102110

103111
/**
@@ -124,9 +132,4 @@ public function getTokenProvider(): TokenProviderInterface
124132
{
125133
return $this->tokenProvider;
126134
}
127-
128-
private function generateHash(string $tokenValue): string
129-
{
130-
return hash_hmac('sha256', $tokenValue, $this->secret);
131-
}
132135
}

‎src/Symfony/Component/Security/Http/RememberMe/RememberMeDetails.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,14 @@ public function __construct(string $userFqcn, string $userIdentifier, int $expir
3636

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

4748
return new static(...$cookieParts);
4849
}
@@ -83,6 +84,6 @@ public function getValue(): string
8384
public function toString(): string
8485
{
8586
// $userIdentifier is encoded because it might contain COOKIE_DELIMITER, we assume other values don't
86-
return base64_encode(implode(self::COOKIE_DELIMITER, [$this->userFqcn, base64_encode($this->userIdentifier), $this->expires, $this->value]));
87+
return implode(self::COOKIE_DELIMITER, [strtr($this->userFqcn, '\\', '.'), strtr(base64_encode($this->userIdentifier), '+/=', '-_~'), $this->expires, $this->value]);
8788
}
8889
}

‎src/Symfony/Component/Security/Http/RememberMe/SignatureRememberMeHandler.php

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,20 @@ public function createRememberMeCookie(UserInterface $user): void
5353
$this->createCookie($details);
5454
}
5555

56-
/**
57-
* {@inheritdoc}
58-
*/
59-
public function processRememberMe(RememberMeDetails $rememberMeDetails, UserInterface $user): void
56+
public function consumeRememberMeCookie(RememberMeDetails $rememberMeDetails): UserInterface
57+
{
58+
try {
59+
$this->signatureHasher->acceptSignatureHash($rememberMeDetails->getUserIdentifier(), $rememberMeDetails->getExpires(), $rememberMeDetails->getValue());
60+
} catch (InvalidSignatureException $e) {
61+
throw new AuthenticationException('The cookie\'s hash is invalid.', 0, $e);
62+
} catch (ExpiredSignatureException $e) {
63+
throw new AuthenticationException('The cookie has expired.', 0, $e);
64+
}
65+
66+
return parent::consumeRememberMeCookie($rememberMeDetails);
67+
}
68+
69+
protected function processRememberMe(RememberMeDetails $rememberMeDetails, UserInterface $user): void
6070
{
6171
try {
6272
$this->signatureHasher->verifySignatureHash($user, $rememberMeDetails->getExpires(), $rememberMeDetails->getValue());

‎src/Symfony/Component/Security/Http/Tests/LoginLink/LoginLinkHandlerTest.php

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public function testCreateLoginLink($user, array $extraProperties, Request $requ
6868
// allow a small expiration offset to avoid time-sensitivity
6969
&& abs(time() + 600 - $parameters['expires']) <= 1
7070
// make sure hash is what we expect
71-
&& $parameters['hash'] === $this->createSignatureHash('weaverryan', $parameters['expires'], array_values($extraProperties));
71+
&& $parameters['hash'] === $this->createSignatureHash('weaverryan', $parameters['expires'], $extraProperties);
7272
}),
7373
UrlGeneratorInterface::ABSOLUTE_URL
7474
)
@@ -115,7 +115,7 @@ public function provideCreateLoginLinkData()
115115
public function testConsumeLoginLink()
116116
{
117117
$expires = time() + 500;
118-
$signature = $this->createSignatureHash('weaverryan', $expires, ['ryan@symfonycasts.com', 'pwhash']);
118+
$signature = $this->createSignatureHash('weaverryan', $expires);
119119
$request = Request::create(sprintf('/login/verify?user=weaverryan&hash=%s&expires=%d', $signature, $expires));
120120

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

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

139-
$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
140-
$this->userProvider->createUser($user);
141-
142138
$linker = $this->createLinker(['max_uses' => 3]);
139+
$this->expectException(ExpiredLoginLinkException::class);
143140
$linker->consumeLoginLink($request);
144141
}
145142

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

151147
$linker = $this->createLinker();
148+
$this->expectException(InvalidLoginLinkException::class);
152149
$linker->consumeLoginLink($request);
153150
}
154151

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

160-
$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
161-
$this->userProvider->createUser($user);
162-
163156
$linker = $this->createLinker();
157+
$this->expectException(InvalidLoginLinkException::class);
164158
$linker->consumeLoginLink($request);
165159
}
166160

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

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

181174
$linker = $this->createLinker(['max_uses' => 3]);
175+
$this->expectException(ExpiredLoginLinkException::class);
182176
$linker->consumeLoginLink($request);
183177
}
184178

@@ -206,15 +200,12 @@ public function testConsumeLoginLinkWithMissingExpiration()
206200
$linker->consumeLoginLink($request);
207201
}
208202

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

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

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

299290
public function refreshUser(UserInterface $user): TestLoginLinkHandlerUser
300291
{
301-
return $this->users[$username];
292+
return $this->users[$user->getUserIdentifier()];
302293
}
303294

304295
public function supportsClass(string $class): bool

‎src/Symfony/Component/Security/Http/Tests/RememberMe/PersistentRememberMeHandlerTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ public function testConsumeRememberMeCookieValid()
9393

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

9999
$this->assertSame($rememberParts[0], $cookieParts[0]); // class
100100
$this->assertSame($rememberParts[1], $cookieParts[1]); // identifier

0 commit comments

Comments
 (0)
0