Skip to content

Commit 9eb99ec

Browse files
authored
Merge pull request #35605 from nextcloud/backport/stable25/31499
[stable25] Add fallback routines for empty secret cases
2 parents c779928 + 7416f44 commit 9eb99ec

File tree

5 files changed

+70
-12
lines changed

5 files changed

+70
-12
lines changed

lib/private/Authentication/Token/PublicKeyTokenProvider.php

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,14 @@ public function getToken(string $tokenId): IToken {
119119
$token = $this->mapper->getToken($this->hashToken($tokenId));
120120
$this->cache[$token->getToken()] = $token;
121121
} catch (DoesNotExistException $ex) {
122-
$this->cache[$tokenHash] = $ex;
123-
throw new InvalidTokenException("Token does not exist: " . $ex->getMessage(), 0, $ex);
122+
try {
123+
$token = $this->mapper->getToken($this->hashTokenWithEmptySecret($tokenId));
124+
$this->cache[$token->getToken()] = $token;
125+
$this->rotate($token, $tokenId, $tokenId);
126+
} catch (DoesNotExistException $ex2) {
127+
$this->cache[$tokenHash] = $ex2;
128+
throw new InvalidTokenException("Token does not exist: " . $ex->getMessage(), 0, $ex);
129+
}
124130
}
125131
}
126132

@@ -198,6 +204,7 @@ public function invalidateToken(string $token) {
198204
$this->cache->clear();
199205

200206
$this->mapper->invalidate($this->hashToken($token));
207+
$this->mapper->invalidate($this->hashTokenWithEmptySecret($token));
201208
}
202209

203210
public function invalidateTokenById(string $uid, int $id) {
@@ -314,9 +321,14 @@ private function decrypt(string $cipherText, string $token): string {
314321
try {
315322
return $this->crypto->decrypt($cipherText, $token . $secret);
316323
} catch (\Exception $ex) {
317-
// Delete the invalid token
318-
$this->invalidateToken($token);
319-
throw new InvalidTokenException("Could not decrypt token password: " . $ex->getMessage(), 0, $ex);
324+
// Retry with empty secret as a fallback for instances where the secret might not have been set by accident
325+
try {
326+
return $this->crypto->decrypt($cipherText, $token);
327+
} catch (\Exception $ex2) {
328+
// Delete the invalid token
329+
$this->invalidateToken($token);
330+
throw new InvalidTokenException("Could not decrypt token password: " . $ex->getMessage(), 0, $ex2);
331+
}
320332
}
321333
}
322334

@@ -339,6 +351,13 @@ private function hashToken(string $token): string {
339351
return hash('sha512', $token . $secret);
340352
}
341353

354+
/**
355+
* @deprecated Fallback for instances where the secret might not have been set by accident
356+
*/
357+
private function hashTokenWithEmptySecret(string $token): string {
358+
return hash('sha512', $token);
359+
}
360+
342361
/**
343362
* @throws \RuntimeException when OpenSSL reports a problem
344363
*/

lib/private/Security/Crypto.php

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,22 @@ public function encrypt(string $plaintext, string $password = ''): string {
122122
* @throws Exception If the decryption failed
123123
*/
124124
public function decrypt(string $authenticatedCiphertext, string $password = ''): string {
125-
if ($password === '') {
126-
$password = $this->config->getSystemValue('secret');
125+
$secret = $this->config->getSystemValue('secret');
126+
try {
127+
if ($password === '') {
128+
return $this->decryptWithoutSecret($authenticatedCiphertext, $secret);
129+
}
130+
return $this->decryptWithoutSecret($authenticatedCiphertext, $password);
131+
} catch (Exception $e) {
132+
if ($password === '') {
133+
// Retry with empty secret as a fallback for instances where the secret might not have been set by accident
134+
return $this->decryptWithoutSecret($authenticatedCiphertext, '');
135+
}
136+
throw $e;
127137
}
138+
}
139+
140+
private function decryptWithoutSecret(string $authenticatedCiphertext, string $password = ''): string {
128141
$hmacKey = $encryptionKey = $password;
129142

130143
$parts = explode('|', $authenticatedCiphertext);

lib/private/Security/Hasher.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,15 @@ protected function legacyHashVerify($message, $hash, &$newHash = null): bool {
137137
return true;
138138
}
139139

140+
// Verify whether it matches a legacy PHPass or SHA1 string
141+
// Retry with empty passwordsalt for cases where it was not set
142+
$hashLength = \strlen($hash);
143+
if (($hashLength === 60 && password_verify($message, $hash)) ||
144+
($hashLength === 40 && hash_equals($hash, sha1($message)))) {
145+
$newHash = $this->hash($message);
146+
return true;
147+
}
148+
140149
return false;
141150
}
142151

lib/private/Security/VerificationToken/VerificationToken.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,15 @@ public function check(string $token, ?IUser $user, string $subject, string $pass
8484
try {
8585
$decryptedToken = $this->crypto->decrypt($encryptedToken, $passwordPrefix.$this->config->getSystemValue('secret'));
8686
} catch (\Exception $e) {
87-
$this->throwInvalidTokenException(InvalidTokenException::TOKEN_DECRYPTION_ERROR);
87+
// Retry with empty secret as a fallback for instances where the secret might not have been set by accident
88+
try {
89+
$decryptedToken = $this->crypto->decrypt($encryptedToken, $passwordPrefix);
90+
} catch (\Exception $e2) {
91+
$this->throwInvalidTokenException(InvalidTokenException::TOKEN_DECRYPTION_ERROR);
92+
}
8893
}
8994

90-
$splitToken = explode(':', $decryptedToken ?? '');
95+
$splitToken = explode(':', $decryptedToken);
9196
if (count($splitToken) !== 2) {
9297
$this->throwInvalidTokenException(InvalidTokenException::TOKEN_INVALID_FORMAT);
9398
}

tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -310,9 +310,12 @@ public function testSetPasswordInvalidToken() {
310310
}
311311

312312
public function testInvalidateToken() {
313-
$this->mapper->expects($this->once())
313+
$this->mapper->expects($this->at(0))
314314
->method('invalidate')
315315
->with(hash('sha512', 'token7'.'1f4h9s'));
316+
$this->mapper->expects($this->at(1))
317+
->method('invalidate')
318+
->with(hash('sha512', 'token7'));
316319

317320
$this->tokenProvider->invalidateToken('token7');
318321
}
@@ -443,13 +446,22 @@ public function testGetToken(): void {
443446
public function testGetInvalidToken() {
444447
$this->expectException(InvalidTokenException::class);
445448

446-
$this->mapper->method('getToken')
449+
$this->mapper->expects($this->at(0))
450+
->method('getToken')
447451
->with(
448-
$this->callback(function (string $token) {
452+
$this->callback(function (string $token): bool {
449453
return hash('sha512', 'unhashedToken'.'1f4h9s') === $token;
450454
})
451455
)->willThrowException(new DoesNotExistException('nope'));
452456

457+
$this->mapper->expects($this->at(1))
458+
->method('getToken')
459+
->with(
460+
$this->callback(function (string $token): bool {
461+
return hash('sha512', 'unhashedToken') === $token;
462+
})
463+
)->willThrowException(new DoesNotExistException('nope'));
464+
453465
$this->tokenProvider->getToken('unhashedToken');
454466
}
455467

0 commit comments

Comments
 (0)