diff --git a/core/core.services.yml b/core/core.services.yml index 6d213bc..cd13586 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -668,7 +668,7 @@ services: # @todo increase by 1 every Drupal version in order to counteract increases in # the speed and power of computers available to crack the hashes. password: - class: Drupal\Core\Password\Password + class: Drupal\Core\Password\PhpPassword arguments: ['%password_hash_cost%', '@drupal7_password'] drupal7_password: class: Drupal\Core\Password\PhpassHashedPassword diff --git a/core/lib/Drupal/Core/Password/Password.php b/core/lib/Drupal/Core/Password/PhpPassword.php similarity index 89% rename from core/lib/Drupal/Core/Password/Password.php rename to core/lib/Drupal/Core/Password/PhpPassword.php index 99e90f2..a26b8e4 100644 --- a/core/lib/Drupal/Core/Password/Password.php +++ b/core/lib/Drupal/Core/Password/PhpPassword.php @@ -2,7 +2,7 @@ /** * @file - * Contains \Drupal\Core\Password\Password. + * Contains \Drupal\Core\Password\PhpPassword. */ namespace Drupal\Core\Password; @@ -15,7 +15,7 @@ * * @see http://php.net/manual/en/ref.password.php */ -class Password implements PasswordInterface { +class PhpPassword implements PasswordInterface { /** * The algorithmic cost that should be used. @@ -48,6 +48,11 @@ function __construct($cost, PasswordInterface $drupal7_password) { * {@inheritdoc} */ public function hash($password) { + // Prevent DoS attacks by refusing to hash large passwords. + if (strlen($password) > 512) { + return FALSE; + } + return password_hash($password, PASSWORD_DEFAULT, $this->getOptions()); } diff --git a/core/modules/user/tests/modules/user_custom_pass_hash_params_test/user_custom_pass_hash_params_test.services.yml b/core/modules/user/tests/modules/user_custom_pass_hash_params_test/user_custom_pass_hash_params_test.services.yml index 770e7c4..ed9c05e 100644 --- a/core/modules/user/tests/modules/user_custom_pass_hash_params_test/user_custom_pass_hash_params_test.services.yml +++ b/core/modules/user/tests/modules/user_custom_pass_hash_params_test/user_custom_pass_hash_params_test.services.yml @@ -1,6 +1,6 @@ services: password: - class: Drupal\Core\Password\Password + class: Drupal\Core\Password\PhpPassword arguments: [11, '@drupal7_password'] drupal7_password: class: Drupal\Core\Password\PhpassHashedPassword diff --git a/core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php b/core/tests/Drupal/Tests/Core/Password/PhpPasswordTest.php similarity index 62% copy from core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php copy to core/tests/Drupal/Tests/Core/Password/PhpPasswordTest.php index f304be0..6d0d4f3 100644 --- a/core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php +++ b/core/tests/Drupal/Tests/Core/Password/PhpPasswordTest.php @@ -2,21 +2,22 @@ /** * @file - * Contains Drupal\system\Tests\System\PasswordHashingTest. + * Contains Drupal\system\Tests\System\PhpPasswordTest. */ namespace Drupal\Tests\Core\Password; +use Drupal\Core\Password\PhpPassword; use Drupal\Core\Password\PhpassHashedPassword; use Drupal\Tests\UnitTestCase; /** * Unit tests for password hashing API. * - * @coversDefaultClass \Drupal\Core\Password\PhpassHashedPassword + * @coversDefaultClass \Drupal\Core\Password\PhpPassword * @group System */ -class PasswordHashingTest extends UnitTestCase { +class PhpPasswordTest extends UnitTestCase { /** * The user for testing. @@ -49,7 +50,7 @@ class PasswordHashingTest extends UnitTestCase { /** * The password hasher under test. * - * @var \Drupal\Core\Password\PhpassHashedPassword + * @var \Drupal\Core\Password\PasswordInterface */ protected $passwordHasher; @@ -61,21 +62,12 @@ protected function setUp() { $this->user = $this->getMockBuilder('Drupal\user\Entity\User') ->disableOriginalConstructor() ->getMock(); - $this->passwordHasher = new PhpassHashedPassword(1); - } + $this->passwordHasher = new PhpPassword(4, new PhpassHashedPassword(1)); - /** - * Tests the hash count boundaries are enforced. - * - * @covers ::enforceLog2Boundaries - */ - public function testWithinBounds() { - $hasher = new FakePhpassHashedPassword(); - $this->assertEquals(PhpassHashedPassword::MIN_HASH_COUNT, $hasher->enforceLog2Boundaries(1), "Min hash count enforced"); - $this->assertEquals(PhpassHashedPassword::MAX_HASH_COUNT, $hasher->enforceLog2Boundaries(100), "Max hash count enforced"); + $this->password = $this->randomMachineName(); + $this->md5Password = md5($this->password); } - /** * Test a password needs update. * @@ -86,15 +78,13 @@ public function testPasswordNeedsUpdate() { ->method('getPassword') ->will($this->returnValue($this->md5Password)); // The md5 password should be flagged as needing an update. - $this->assertTrue($this->passwordHasher->userNeedsNewHash($this->user), 'User with md5 password needs a new hash.'); + $this->assertTrue($this->passwordHasher->userNeedsNewHash($this->user)); } /** * Test password hashing. * * @covers ::hash - * @covers ::getCountLog2 - * @covers ::check * @covers ::userNeedsNewHash */ public function testPasswordHashing() { @@ -102,45 +92,38 @@ public function testPasswordHashing() { $this->user->expects($this->any()) ->method('getPassword') ->will($this->returnValue($this->hashedPassword)); - $this->assertSame($this->passwordHasher->getCountLog2($this->hashedPassword), PhpassHashedPassword::MIN_HASH_COUNT, 'Hashed password has the minimum number of log2 iterations.'); - $this->assertNotEquals($this->hashedPassword, $this->md5Password, 'Password hash changed.'); - $this->assertTrue($this->passwordHasher->check($this->password, $this->user), 'Password check succeeds.'); - // Since the log2 setting hasn't changed and the user has a valid password, - // userNeedsNewHash() should return FALSE. - $this->assertFalse($this->passwordHasher->userNeedsNewHash($this->user), 'User does not need a new hash.'); + $this->assertNotEquals($this->hashedPassword, $this->md5Password); + $this->assertTrue($this->passwordHasher->check($this->password, $this->user)); + $this->assertFalse($this->passwordHasher->userNeedsNewHash($this->user)); } /** * Tests password rehashing. * * @covers ::hash - * @covers ::getCountLog2 - * @covers ::check * @covers ::userNeedsNewHash */ public function testPasswordRehashing() { - - // Increment the log2 iteration to MIN + 1. - $this->passwordHasher = new PhpassHashedPassword(PhpassHashedPassword::MIN_HASH_COUNT + 1); - $this->assertTrue($this->passwordHasher->userNeedsNewHash($this->user), 'User needs a new hash after incrementing the log2 count.'); + // Increment the cost from 4 to 5. + $this->passwordHasher = new PhpPassword(5, new PhpassHashedPassword(1)); + $this->assertTrue($this->passwordHasher->userNeedsNewHash($this->user)); // Re-hash the password. $rehashed_password = $this->passwordHasher->hash($this->password); $this->user->expects($this->any()) ->method('getPassword') ->will($this->returnValue($rehashed_password)); - $this->assertSame($this->passwordHasher->getCountLog2($rehashed_password), PhpassHashedPassword::MIN_HASH_COUNT + 1, 'Re-hashed password has the correct number of log2 iterations.'); - $this->assertNotEquals($rehashed_password, $this->hashedPassword, 'Password hash changed again.'); + $this->assertNotEquals($rehashed_password, $this->hashedPassword); // Now the hash should be OK. - $this->assertFalse($this->passwordHasher->userNeedsNewHash($this->user), 'Re-hashed password does not need a new hash.'); - $this->assertTrue($this->passwordHasher->check($this->password, $this->user), 'Password check succeeds with re-hashed password.'); + $this->assertFalse($this->passwordHasher->userNeedsNewHash($this->user)); + $this->assertTrue($this->passwordHasher->check($this->password, $this->user)); } /** * Verifies that passwords longer than 512 bytes are not hashed. * - * @covers ::crypt + * @covers ::hash * * @dataProvider providerLongPasswords */ @@ -178,19 +161,3 @@ public function providerLongPasswords() { } } - -/** - * A fake class for tests. - */ -class FakePhpassHashedPassword extends PhpassHashedPassword { - - function __construct() { - // Noop. - } - - // Expose this method as public for tests. - public function enforceLog2Boundaries($count_log2) { - return parent::enforceLog2Boundaries($count_log2); - } - -} diff --git a/core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php b/core/tests/Drupal/Tests/Core/Password/PhpassHashedPasswordTest.php similarity index 97% rename from core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php rename to core/tests/Drupal/Tests/Core/Password/PhpassHashedPasswordTest.php index f304be0..2a870ed 100644 --- a/core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php +++ b/core/tests/Drupal/Tests/Core/Password/PhpassHashedPasswordTest.php @@ -2,7 +2,7 @@ /** * @file - * Contains Drupal\system\Tests\System\PasswordHashingTest. + * Contains Drupal\system\Tests\System\PhpassHashedPasswordTest. */ namespace Drupal\Tests\Core\Password; @@ -16,7 +16,7 @@ * @coversDefaultClass \Drupal\Core\Password\PhpassHashedPassword * @group System */ -class PasswordHashingTest extends UnitTestCase { +class PhpassHashedPasswordTest extends UnitTestCase { /** * The user for testing.