diff --git a/src/Nexus/Password/Hash/Pbkdf2Hash.php b/src/Nexus/Password/Hash/Pbkdf2Hash.php index 854b1cb..1864ab2 100644 --- a/src/Nexus/Password/Hash/Pbkdf2Hash.php +++ b/src/Nexus/Password/Hash/Pbkdf2Hash.php @@ -43,12 +43,6 @@ */ private int $length; - /** - * Used on `::verify()` to return early if provided hash's length - * does not match this hasher's instance's supposed hash length. - */ - private int $hashLength; - /** * @param array{ * iterations?: int, @@ -79,8 +73,6 @@ public function __construct( $this->defaultIterations(), self::DEFAULT_LENGTH, ); - - $this->hashLength = \strlen($this->hash('password', salt: random_bytes(16))); } /** @@ -101,7 +93,19 @@ public function hash(#[\SensitiveParameter] string $password, array $options = [ $this->length, ); - return hash_pbkdf2($this->algorithm->value, $password, $salt, $iterations, $length); + return \sprintf( + '$%s$i=%d,l=%d$%s$%s', + $this->algorithm->value, + $iterations, + $length, + base64_encode($salt), + base64_encode($this->pbkdf2( + $password, + $salt, + $iterations, + $length, + )), + ); } public function needsRehash(string $hash, array $options = []): bool @@ -115,15 +119,49 @@ public function verify(string $password, string $hash, string $salt = ''): bool return false; } - if (\strlen($hash) !== $this->hashLength) { + if (preg_match('/^\$([^\$]+)\$([^\$]+)\$([^\$]+)\$([^\$]+)$/', $hash, $parts) !== 1) { + return false; + } + + array_shift($parts); + + if (Algorithm::tryFrom($parts[0]) === null) { + return false; + } + + if (preg_match('/i=(\-?\d+),l=(\-?\d+)/', $parts[1], $matches) !== 1) { + return false; + } + + try { + ['iterations' => $iterations,'length' => $length] = $this->validatedOptions( + [], + (int) $matches[1], + (int) $matches[2], + ); + } catch (HashException) { return false; } - if (str_contains($hash, '$')) { + if (base64_decode($parts[2], true) === false) { return false; } - return hash_equals($hash, $this->hash($password, salt: $salt)); + $rawHash = base64_decode($parts[3], true); + + if (false === $rawHash) { + return false; + } + + return hash_equals( + $rawHash, + $this->pbkdf2( + $password, + $salt, + $iterations, + $length, + ), + ); } public function valid(): bool @@ -131,6 +169,22 @@ public function valid(): bool return \function_exists('hash_pbkdf2'); } + /** + * @param int<1, max> $iterations + * @param int<0, max> $length + */ + private function pbkdf2(string $password, string $salt, int $iterations, int $length): string + { + return hash_pbkdf2( + $this->algorithm->value, + $password, + $salt, + $iterations, + $length, + true, + ); + } + /** * @see https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2 */ diff --git a/tests/Password/Hash/Pbkdf2HashTest.php b/tests/Password/Hash/Pbkdf2HashTest.php index d09df47..64dd82a 100644 --- a/tests/Password/Hash/Pbkdf2HashTest.php +++ b/tests/Password/Hash/Pbkdf2HashTest.php @@ -17,7 +17,6 @@ use Nexus\Password\Hash\AbstractHash; use Nexus\Password\Hash\Pbkdf2Hash; use Nexus\Password\HashException; -use Nexus\Password\Password; use Nexus\PHPUnit\Tachycardia\Attribute\TimeLimit; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; @@ -77,32 +76,6 @@ public static function provideDefaultIterationsCases(): iterable yield 'sha512' => [Algorithm::Pbkdf2HmacSha512, 210_000]; } - #[DataProvider('provideLengthProvidedCorrespondsToHashLengthCases')] - public function testLengthProvidedCorrespondsToHashLength(int $result, int $length, string $algo): void - { - $algorithm = Algorithm::from($algo); - $hasher = new Pbkdf2Hash($algorithm, compact('length')); - $hash = $hasher->hash('password'); - - self::assertSame($result, \strlen($hash)); - } - - /** - * @return iterable - */ - public static function provideLengthProvidedCorrespondsToHashLengthCases(): iterable - { - yield 'sha1 0' => [40, 0, 'sha1']; - - yield 'sha256 0' => [64, 0, 'sha256']; - - yield 'sha512 0' => [128, 0, 'sha512']; - - foreach (['sha1', 'sha256', 'sha512'] as $algo) { - yield $algo.' 30' => [30, 30, $algo]; - } - } - #[TimeLimit(3.0)] public function testBasicPasswordHashing(): void { @@ -114,7 +87,6 @@ public function testBasicPasswordHashing(): void $hash = $hasher->hash($password, salt: $salt); self::assertFalse($hasher->needsRehash($hash)); - self::assertTrue($hasher->verify($password, $hash, $salt)); } public function testInvalidPasswordForHash(): void @@ -125,26 +97,57 @@ public function testInvalidPasswordForHash(): void (new Pbkdf2Hash(Algorithm::Pbkdf2HmacSha256))->hash('aa'); } + #[DataProvider('providePasswordVerifyCases')] #[TimeLimit(3.0)] - public function testPasswordVerify(): void + public function testPasswordVerify(bool $result, ?string $password, string $hash): void { - $pass1 = "abcd\0e"; - $pass2 = 'password'; - $pass3 = 'pass'; - $salt = random_bytes(16); + $password ??= 'password'; $hasher = new Pbkdf2Hash(Algorithm::Pbkdf2HmacSha256); - $hash = $hasher->hash($pass2, salt: $salt); - self::assertFalse($hasher->verify($pass1, $hash, $salt)); - self::assertTrue($hasher->verify($pass2, $hash, $salt)); - self::assertFalse($hasher->verify($pass3, $hash, $salt)); - self::assertFalse($hasher->verify( - $pass2, - substr(Password::fromAlgorithm(Algorithm::Argon2i)->hash($pass2), 0, 40), - )); - self::assertFalse($hasher->verify( - $pass2, - (new Pbkdf2Hash(Algorithm::Pbkdf2HmacSha256, ['length' => 0]))->hash($pass2, salt: $salt), - )); + self::assertSame($result, $hasher->verify($password, $hash, 'salt')); + } + + /** + * @return iterable + */ + public static function providePasswordVerifyCases(): iterable + { + $hash = (new Pbkdf2Hash(Algorithm::Pbkdf2HmacSha256))->hash('password', [], 'salt'); + + yield 'empty' => [false, '', $hash]; + + yield 'nul' => [false, "pass\0word", $hash]; + + yield 'short' => [false, 'pass', $hash]; + + yield 'not hash' => [false, null, 'hash']; + + yield 'truncated' => [false, null, substr($hash, 7)]; + + yield 'not hash hmac' => [false, null, str_replace('sha256', 'ghost', $hash)]; + + yield 'alternated options' => [false, null, preg_replace( + '/(i=\d+),(l=\d+)/', + '$2,$1', + $hash, + ) ?? $hash]; + + yield 'invalid iterations' => [false, null, str_replace('i=600000', 'i=900', $hash)]; + + yield 'invalid length' => [false, null, str_replace('l=40', 'l=-1', $hash)]; + + yield 'invalid base 64 salt' => [false, null, preg_replace( + '/^\$([^\$]+)\$([^\$]+)\$([^\$]+)\$([^\$]+)$/', + '\$$1\$$2\$$3=\$$4', + $hash, + ) ?? $hash]; + + yield 'invalid base 64 hash' => [false, null, preg_replace( + '/^\$([^\$]+)\$([^\$]+)\$([^\$]+)\$([^\$]+)$/', + '\$$1\$$2\$$3\$$4=', + $hash, + ) ?? $hash]; + + yield 'valid hash' => [true, null, $hash]; } }