Skip to content
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.

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

Add fallback routines for empty secret cases #31499

Merged
merged 5 commits into from
Oct 17, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
29 changes: 24 additions & 5 deletions lib/private/Authentication/Token/PublicKeyTokenProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,14 @@ public function getToken(string $tokenId): IToken {
$token = $this->mapper->getToken($this->hashToken($tokenId));
$this->cache[$token->getToken()] = $token;
} catch (DoesNotExistException $ex) {
$this->cache[$tokenHash] = $ex;
throw new InvalidTokenException("Token does not exist: " . $ex->getMessage(), 0, $ex);
try {
$token = $this->mapper->getToken($this->hashTokenWithEmptySecret($tokenId));
$this->cache[$token->getToken()] = $token;
$this->rotate($token, $tokenId, $tokenId);
} catch (DoesNotExistException $ex2) {
$this->cache[$tokenHash] = $ex2;
throw new InvalidTokenException("Token does not exist: " . $ex->getMessage(), 0, $ex);
}
}
}

Expand Down Expand Up @@ -185,6 +191,7 @@ public function invalidateToken(string $token) {
$this->cache->clear();

$this->mapper->invalidate($this->hashToken($token));
$this->mapper->invalidate($this->hashTokenWithEmptySecret($token));
}

public function invalidateTokenById(string $uid, int $id) {
Expand Down Expand Up @@ -301,9 +308,14 @@ private function decrypt(string $cipherText, string $token): string {
try {
return $this->crypto->decrypt($cipherText, $token . $secret);
} catch (\Exception $ex) {
// Delete the invalid token
$this->invalidateToken($token);
throw new InvalidTokenException("Could not decrypt token password: " . $ex->getMessage(), 0, $ex);
// Retry with empty secret as a fallback for instances where the secret might not have been set by accident
try {
return $this->crypto->decrypt($cipherText, $token);
} catch (\Exception $ex2) {
// Delete the invalid token
$this->invalidateToken($token);
throw new InvalidTokenException("Could not decrypt token password: " . $ex->getMessage(), 0, $ex2);
}
}
}

Expand All @@ -326,6 +338,13 @@ private function hashToken(string $token): string {
return hash('sha512', $token . $secret);
}

/**
* @depreacted Fallback for instances where the secret might not have been set by accident
CarlSchwan marked this conversation as resolved.
Show resolved Hide resolved
*/
private function hashTokenWithEmptySecret(string $token): string {
juliusknorr marked this conversation as resolved.
Show resolved Hide resolved
return hash('sha512', $token);
}

/**
* @throws \RuntimeException when OpenSSL reports a problem
*/
Expand Down
18 changes: 16 additions & 2 deletions lib/private/Security/Crypto.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,23 @@ public function encrypt(string $plaintext, string $password = ''): string {
* @throws Exception If the decryption failed
*/
public function decrypt(string $authenticatedCiphertext, string $password = ''): string {
if ($password === '') {
$password = $this->config->getSystemValue('secret');
$secret = $this->config->getSystemValue('secret');
try {
if ($password === '') {
return $this->decryptWithoutSecret($authenticatedCiphertext, $secret);
}
return $this->decryptWithoutSecret($authenticatedCiphertext, $password);
} catch (Exception $e) {
if ($password === '') {
// Retry with empty secret as a fallback for instances where the secret might not have been set by accident
return $this->decryptWithoutSecret($authenticatedCiphertext, '');
}

CarlSchwan marked this conversation as resolved.
Show resolved Hide resolved
throw $e;
}
}

private function decryptWithoutSecret(string $authenticatedCiphertext, string $password = ''): string {
$hmacKey = $encryptionKey = $password;

$parts = explode('|', $authenticatedCiphertext);
Expand Down
9 changes: 9 additions & 0 deletions lib/private/Security/Hasher.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,15 @@ protected function legacyHashVerify($message, $hash, &$newHash = null): bool {
return true;
}

// Verify whether it matches a legacy PHPass or SHA1 string
// Retry with empty passwordsalt for cases where it was not set
$hashLength = \strlen($hash);
if (($hashLength === 60 && password_verify($message, $hash)) ||
($hashLength === 40 && hash_equals($hash, sha1($message)))) {
$newHash = $this->hash($message);
return true;
}

return false;
}

Expand Down
9 changes: 7 additions & 2 deletions lib/private/Security/VerificationToken/VerificationToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,15 @@ public function check(string $token, ?IUser $user, string $subject, string $pass
try {
$decryptedToken = $this->crypto->decrypt($encryptedToken, $passwordPrefix.$this->config->getSystemValue('secret'));
} catch (\Exception $e) {
$this->throwInvalidTokenException(InvalidTokenException::TOKEN_DECRYPTION_ERROR);
// Retry with empty secret as a fallback for instances where the secret might not have been set by accident
try {
$decryptedToken = $this->crypto->decrypt($encryptedToken, $passwordPrefix);
} catch (\Exception $e2) {
$this->throwInvalidTokenException(InvalidTokenException::TOKEN_DECRYPTION_ERROR);
}
}

$splitToken = explode(':', $decryptedToken ?? '');
$splitToken = explode(':', $decryptedToken);
if (count($splitToken) !== 2) {
$this->throwInvalidTokenException(InvalidTokenException::TOKEN_INVALID_FORMAT);
}
Expand Down