From 55d8aec7597ab29031abda47da9afa15a16b0a4a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 9 Jan 2023 14:53:12 +0100 Subject: [PATCH 1/4] fix(authentication): Only verify each hash once Signed-off-by: Joas Schilling --- lib/private/Authentication/Token/PublicKeyTokenProvider.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php index 7f1b10e0956f0..651540f934a71 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -448,9 +448,11 @@ public function updatePasswords(string $uid, string $password) { // Update the password for all tokens $tokens = $this->mapper->getTokenByUser($uid); $passwordHash = $this->hashPassword($password); + $verifiedHashes = []; foreach ($tokens as $t) { - $publicKey = $t->getPublicKey(); - if ($t->getPasswordHash() === null || $this->hasher->verify(sha1($password) . $password, $t->getPasswordHash())) { + if ($t->getPasswordHash() === null || isset($verifiedHashes[$t->getPasswordHash()]) || $this->hasher->verify(sha1($password) . $password, $t->getPasswordHash())) { + $verifiedHashes[$t->getPasswordHash() ?: ''] = true; + $publicKey = $t->getPublicKey(); $t->setPassword($this->encryptPassword($password, $publicKey)); $t->setPasswordHash($passwordHash); $t->setPasswordInvalid(false); From c5bb19641c12757e2b11376ee431d2a941e98e3f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 9 Jan 2023 15:13:08 +0100 Subject: [PATCH 2/4] fix(authentication): Invert the logic to the original intention We need to store the new authentication details when the hash did **not** verify the old password. Signed-off-by: Joas Schilling --- lib/private/Authentication/Token/PublicKeyTokenProvider.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php index 651540f934a71..adac86dd07316 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -450,13 +450,14 @@ public function updatePasswords(string $uid, string $password) { $passwordHash = $this->hashPassword($password); $verifiedHashes = []; foreach ($tokens as $t) { - if ($t->getPasswordHash() === null || isset($verifiedHashes[$t->getPasswordHash()]) || $this->hasher->verify(sha1($password) . $password, $t->getPasswordHash())) { - $verifiedHashes[$t->getPasswordHash() ?: ''] = true; + if ($t->getPasswordHash() === null || !isset($verifiedHashes[$t->getPasswordHash()]) || !$this->hasher->verify(sha1($password) . $password, $t->getPasswordHash())) { $publicKey = $t->getPublicKey(); $t->setPassword($this->encryptPassword($password, $publicKey)); $t->setPasswordHash($passwordHash); $t->setPasswordInvalid(false); $this->updateToken($t); + } else { + $verifiedHashes[$t->getPasswordHash() ?: ''] = true; } } } From 28b18d561cea2f77ca6cc70c4052001e41b57620 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 9 Jan 2023 15:58:26 +0100 Subject: [PATCH 3/4] fix(authentication): Only hash the new password when needed Signed-off-by: Joas Schilling --- .../Authentication/Token/PublicKeyTokenProvider.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php index adac86dd07316..6cf6b8f858c74 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -447,13 +447,17 @@ public function updatePasswords(string $uid, string $password) { // Update the password for all tokens $tokens = $this->mapper->getTokenByUser($uid); - $passwordHash = $this->hashPassword($password); + $newPasswordHash = null; $verifiedHashes = []; foreach ($tokens as $t) { if ($t->getPasswordHash() === null || !isset($verifiedHashes[$t->getPasswordHash()]) || !$this->hasher->verify(sha1($password) . $password, $t->getPasswordHash())) { + if ($newPasswordHash === null) { + $newPasswordHash = $this->hashPassword($password); + } + $publicKey = $t->getPublicKey(); $t->setPassword($this->encryptPassword($password, $publicKey)); - $t->setPasswordHash($passwordHash); + $t->setPasswordHash($newPasswordHash); $t->setPasswordInvalid(false); $this->updateToken($t); } else { From 2fb4dac7adbafc8c2896bf72eb158fb90abf05a2 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 9 Jan 2023 16:12:01 +0100 Subject: [PATCH 4/4] fix(authentication): Update the token when the hash is null or can not be verified Signed-off-by: Joas Schilling --- .../Token/PublicKeyTokenProvider.php | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php index 6cf6b8f858c74..c8adec24b31d6 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -448,9 +448,28 @@ public function updatePasswords(string $uid, string $password) { // Update the password for all tokens $tokens = $this->mapper->getTokenByUser($uid); $newPasswordHash = null; - $verifiedHashes = []; + + /** + * - true: The password hash could not be verified anymore + * and the token needs to be updated with the newly encrypted password + * - false: The hash could still be verified + * - missing: The hash needs to be verified + */ + $hashNeedsUpdate = []; + foreach ($tokens as $t) { - if ($t->getPasswordHash() === null || !isset($verifiedHashes[$t->getPasswordHash()]) || !$this->hasher->verify(sha1($password) . $password, $t->getPasswordHash())) { + if (!isset($hashNeedsUpdate[$t->getPasswordHash()])) { + if ($t->getPasswordHash() === null) { + $hashNeedsUpdate[$t->getPasswordHash() ?: ''] = true; + } elseif (!$this->hasher->verify(sha1($password) . $password, $t->getPasswordHash())) { + $hashNeedsUpdate[$t->getPasswordHash() ?: ''] = true; + } else { + $hashNeedsUpdate[$t->getPasswordHash() ?: ''] = false; + } + } + $needsUpdating = $hashNeedsUpdate[$t->getPasswordHash() ?: ''] ?? true; + + if ($needsUpdating) { if ($newPasswordHash === null) { $newPasswordHash = $this->hashPassword($password); } @@ -460,8 +479,6 @@ public function updatePasswords(string $uid, string $password) { $t->setPasswordHash($newPasswordHash); $t->setPasswordInvalid(false); $this->updateToken($t); - } else { - $verifiedHashes[$t->getPasswordHash() ?: ''] = true; } } }