[PasswordHasher] accept hashing passwords with nul bytes or longer than 72 bytes when using bcrypt

This commit is contained in:
Nicolas Grekas 2021-04-23 10:39:18 +02:00
parent 6ff9c9718c
commit a5d3b89472
4 changed files with 52 additions and 12 deletions

View File

@ -53,11 +53,11 @@ final class NativePasswordHasher implements PasswordHasherInterface
$algorithms = [1 => \PASSWORD_BCRYPT, '2y' => \PASSWORD_BCRYPT]; $algorithms = [1 => \PASSWORD_BCRYPT, '2y' => \PASSWORD_BCRYPT];
if (\defined('PASSWORD_ARGON2I')) { if (\defined('PASSWORD_ARGON2I')) {
$algorithms[2] = $algorithms['argon2i'] = (string) \PASSWORD_ARGON2I; $algorithms[2] = $algorithms['argon2i'] = \PASSWORD_ARGON2I;
} }
if (\defined('PASSWORD_ARGON2ID')) { if (\defined('PASSWORD_ARGON2ID')) {
$algorithms[3] = $algorithms['argon2id'] = (string) \PASSWORD_ARGON2ID; $algorithms[3] = $algorithms['argon2id'] = \PASSWORD_ARGON2ID;
} }
$this->algorithm = $algorithms[$algorithm] ?? $algorithm; $this->algorithm = $algorithms[$algorithm] ?? $algorithm;
@ -73,10 +73,14 @@ final class NativePasswordHasher implements PasswordHasherInterface
public function hash(string $plainPassword): string public function hash(string $plainPassword): string
{ {
if ($this->isPasswordTooLong($plainPassword) || ((string) \PASSWORD_BCRYPT === $this->algorithm && 72 < \strlen($plainPassword))) { if ($this->isPasswordTooLong($plainPassword)) {
throw new InvalidPasswordException(); throw new InvalidPasswordException();
} }
if (\PASSWORD_BCRYPT === $this->algorithm && (72 < \strlen($plainPassword) || false !== strpos($plainPassword, "\0"))) {
$plainPassword = base64_encode(hash('sha512', $plainPassword, true));
}
return password_hash($plainPassword, $this->algorithm, $this->options); return password_hash($plainPassword, $this->algorithm, $this->options);
} }
@ -87,8 +91,12 @@ final class NativePasswordHasher implements PasswordHasherInterface
} }
if (0 !== strpos($hashedPassword, '$argon')) { if (0 !== strpos($hashedPassword, '$argon')) {
// BCrypt encodes only the first 72 chars // Bcrypt cuts on NUL chars and after 72 bytes
return (72 >= \strlen($plainPassword) || 0 !== strpos($hashedPassword, '$2')) && password_verify($plainPassword, $hashedPassword); if (0 === strpos($hashedPassword, '$2') && (72 < \strlen($plainPassword) || false !== strpos($plainPassword, "\0"))) {
$plainPassword = base64_encode(hash('sha512', $plainPassword, true));
}
return password_verify($plainPassword, $hashedPassword);
} }
if (\extension_loaded('sodium') && version_compare(\SODIUM_LIBRARY_VERSION, '1.0.14', '>=')) { if (\extension_loaded('sodium') && version_compare(\SODIUM_LIBRARY_VERSION, '1.0.14', '>=')) {

View File

@ -80,8 +80,12 @@ final class SodiumPasswordHasher implements PasswordHasherInterface
} }
if (0 !== strpos($hashedPassword, '$argon')) { if (0 !== strpos($hashedPassword, '$argon')) {
if (0 === strpos($hashedPassword, '$2') && (72 < \strlen($plainPassword) || false !== strpos($plainPassword, "\0"))) {
$plainPassword = base64_encode(hash('sha512', $plainPassword, true));
}
// Accept validating non-argon passwords for seamless migrations // Accept validating non-argon passwords for seamless migrations
return (72 >= \strlen($plainPassword) || 0 !== strpos($hashedPassword, '$2')) && password_verify($plainPassword, $hashedPassword); return password_verify($plainPassword, $hashedPassword);
} }
if (\function_exists('sodium_crypto_pwhash_str_verify')) { if (\function_exists('sodium_crypto_pwhash_str_verify')) {

View File

@ -89,13 +89,22 @@ class NativePasswordHasherTest extends TestCase
$this->assertStringStartsWith('$2', $result); $this->assertStringStartsWith('$2', $result);
} }
public function testCheckPasswordLength() public function testBcryptWithLongPassword()
{ {
$hasher = new NativePasswordHasher(null, null, 4); $hasher = new NativePasswordHasher(null, null, 4, \PASSWORD_BCRYPT);
$result = password_hash(str_repeat('a', 72), \PASSWORD_BCRYPT, ['cost' => 4]); $plainPassword = str_repeat('a', 100);
$this->assertFalse($hasher->verify($result, str_repeat('a', 73), 'salt')); $this->assertFalse($hasher->verify(password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]), $plainPassword, 'salt'));
$this->assertTrue($hasher->verify($result, str_repeat('a', 72), 'salt')); $this->assertTrue($hasher->verify($hasher->hash($plainPassword), $plainPassword, 'salt'));
}
public function testBcryptWithNulByte()
{
$hasher = new NativePasswordHasher(null, null, 4, \PASSWORD_BCRYPT);
$plainPassword = "a\0b";
$this->assertFalse($hasher->verify(password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]), $plainPassword, 'salt'));
$this->assertTrue($hasher->verify($hasher->hash($plainPassword), $plainPassword, 'salt'));
} }
public function testNeedsRehash() public function testNeedsRehash()

View File

@ -13,6 +13,7 @@ namespace Symfony\Component\PasswordHasher\Tests\Hasher;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Symfony\Component\PasswordHasher\Exception\InvalidPasswordException; use Symfony\Component\PasswordHasher\Exception\InvalidPasswordException;
use Symfony\Component\PasswordHasher\Hasher\NativePasswordHasher;
use Symfony\Component\PasswordHasher\Hasher\SodiumPasswordHasher; use Symfony\Component\PasswordHasher\Hasher\SodiumPasswordHasher;
class SodiumPasswordHasherTest extends TestCase class SodiumPasswordHasherTest extends TestCase
@ -33,7 +34,7 @@ class SodiumPasswordHasherTest extends TestCase
$this->assertFalse($hasher->verify($result, '', null)); $this->assertFalse($hasher->verify($result, '', null));
} }
public function testBCryptValidation() public function testBcryptValidation()
{ {
$hasher = new SodiumPasswordHasher(); $hasher = new SodiumPasswordHasher();
$this->assertTrue($hasher->verify('$2y$04$M8GDODMoGQLQRpkYCdoJh.lbiZPee3SZI32RcYK49XYTolDGwoRMm', 'abc', null)); $this->assertTrue($hasher->verify('$2y$04$M8GDODMoGQLQRpkYCdoJh.lbiZPee3SZI32RcYK49XYTolDGwoRMm', 'abc', null));
@ -63,6 +64,24 @@ class SodiumPasswordHasherTest extends TestCase
$this->assertTrue($hasher->verify($result, str_repeat('a', 4096), null)); $this->assertTrue($hasher->verify($result, str_repeat('a', 4096), null));
} }
public function testBcryptWithLongPassword()
{
$hasher = new SodiumPasswordHasher(null, null, 4);
$plainPassword = str_repeat('a', 100);
$this->assertFalse($hasher->verify(password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]), $plainPassword, 'salt'));
$this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, 4, \PASSWORD_BCRYPT))->hash($plainPassword), $plainPassword, 'salt'));
}
public function testBcryptWithNulByte()
{
$hasher = new SodiumPasswordHasher(null, null, 4);
$plainPassword = "a\0b";
$this->assertFalse($hasher->verify(password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]), $plainPassword, 'salt'));
$this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, 4, \PASSWORD_BCRYPT))->hash($plainPassword), $plainPassword, 'salt'));
}
public function testUserProvidedSaltIsNotUsed() public function testUserProvidedSaltIsNotUsed()
{ {
$hasher = new SodiumPasswordHasher(); $hasher = new SodiumPasswordHasher();