Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit ec78c9b

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
[Security/Http] Check tokens before loading users from providers
1 parent 01344e3 commit ec78c9b
Copy full SHA for ec78c9b

9 files changed

+119
-105
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Signature/SignatureHasher.php
+33-6Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/LoginLink/LoginLinkHandler.php
+11-11Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/RememberMe/PersistentRememberMeHandler.php
+24-21Lines changed: 24 additions & 21 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
/**
@@ -113,7 +121,7 @@ public function clearRememberMeCookie(): void
113121
}
114122

115123
$rememberMeDetails = RememberMeDetails::fromRawCookie($cookie);
116-
[$series, ] = explode(':', $rememberMeDetails->getValue());
124+
[$series] = explode(':', $rememberMeDetails->getValue());
117125
$this->tokenProvider->deleteTokenBySeries($series);
118126
}
119127

@@ -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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/RememberMe/RememberMeDetails.php
+4-3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/RememberMe/SignatureRememberMeHandler.php
+14-4Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Tests/LoginLink/LoginLinkHandlerTest.php
+15-23Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ protected function setUp(): void
5353

5454
/**
5555
* @group time-sensitive
56+
*
5657
* @dataProvider provideCreateLoginLinkData
5758
*/
5859
public function testCreateLoginLink($user, array $extraProperties, Request $request = null)
@@ -68,7 +69,7 @@ public function testCreateLoginLink($user, array $extraProperties, Request $requ
6869
// allow a small expiration offset to avoid time-sensitivity
6970
&& abs(time() + 600 - $parameters['expires']) <= 1
7071
// make sure hash is what we expect
71-
&& $parameters['hash'] === $this->createSignatureHash('weaverryan', $parameters['expires'], array_values($extraProperties));
72+
&& $parameters['hash'] === $this->createSignatureHash('weaverryan', $parameters['expires'], $extraProperties);
7273
}),
7374
UrlGeneratorInterface::ABSOLUTE_URL
7475
)
@@ -115,7 +116,7 @@ public function provideCreateLoginLinkData()
115116
public function testConsumeLoginLink()
116117
{
117118
$expires = time() + 500;
118-
$signature = $this->createSignatureHash('weaverryan', $expires, ['ryan@symfonycasts.com', 'pwhash']);
119+
$signature = $this->createSignatureHash('weaverryan', $expires);
119120
$request = Request::create(sprintf('/login/verify?user=weaverryan&hash=%s&expires=%d', $signature, $expires));
120121

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

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

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

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

151148
$linker = $this->createLinker();
149+
$this->expectException(InvalidLoginLinkException::class);
152150
$linker->consumeLoginLink($request);
153151
}
154152

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

160-
$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
161-
$this->userProvider->createUser($user);
162-
163157
$linker = $this->createLinker();
158+
$this->expectException(InvalidLoginLinkException::class);
164159
$linker->consumeLoginLink($request);
165160
}
166161

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

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

181175
$linker = $this->createLinker(['max_uses' => 3]);
176+
$this->expectException(ExpiredLoginLinkException::class);
182177
$linker->consumeLoginLink($request);
183178
}
184179

@@ -206,15 +201,12 @@ public function testConsumeLoginLinkWithMissingExpiration()
206201
$linker->consumeLoginLink($request);
207202
}
208203

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

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

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

299291
public function refreshUser(UserInterface $user): TestLoginLinkHandlerUser
300292
{
301-
return $this->users[$username];
293+
return $this->users[$user->getUserIdentifier()];
302294
}
303295

304296
public function supportsClass(string $class): bool

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Tests/RememberMe/PersistentRememberMeHandlerTest.php
+2-2Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.