diff --git a/core/core.services.yml b/core/core.services.yml index 5c42b23..2ff42ef 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -41,6 +41,7 @@ parameters: exposedHeaders: false maxAge: false supportsCredentials: false + password_hash_cost: 10 services: # Simple cache contexts, directly derived from the request context. cache_context.ip: @@ -914,14 +915,17 @@ services: class: Drupal\Core\Path\PathValidator arguments: ['@router', '@router.no_access_checks', '@current_user', '@path_processor_manager'] -# The argument to the hashing service defined in services.yml, to the -# constructor of PhpassHashedPassword is the log2 number of iterations for -# password stretching. -# @todo increase by 1 every Drupal version in order to counteract increases in -# the speed and power of computers available to crack the hashes. The current -# password hashing method was introduced in Drupal 7 with a log2 count of 15. +# The first argument of the hashing service (constructor of PhpPassword) is +# the 'cost' option of password_hash(). In Drupal 8 the 'cost' has the default +# value used by password_hash() which is 10. Future versions of Drupal may +# increase this value in order to counteract increases in the speed and power +# of computers available to crack the hashes. Note that an increase of 1 will +# double the time needed for password hashing. password: - class: Drupal\Core\Password\PhpassHashedPassword + class: Drupal\Core\Password\PhpPassword + arguments: ['%password_hash_cost%', '@drupal7_password'] + drupal7_password: + class: Drupal\Core\Password\Drupal7Password arguments: [16] request_format_route_filter: class: Drupal\Core\Routing\RequestFormatRouteFilter diff --git a/core/lib/Drupal/Core/Password/PhpassHashedPassword.php b/core/lib/Drupal/Core/Password/Drupal7Password.php similarity index 95% rename from core/lib/Drupal/Core/Password/PhpassHashedPassword.php rename to core/lib/Drupal/Core/Password/Drupal7Password.php index 28313d2..4a39f68 100644 --- a/core/lib/Drupal/Core/Password/PhpassHashedPassword.php +++ b/core/lib/Drupal/Core/Password/Drupal7Password.php @@ -8,9 +8,13 @@ * Secure password hashing functions based on the Portable PHP password * hashing framework. * + * This is a custom password hashing mechanism used in Drupal 7. We keep this + * password hasher in order to validate passwords migrated from Drupal 7 that + * are being rehashed on first user login. + * * @see http://www.openwall.com/phpass/ */ -class PhpassHashedPassword implements PasswordInterface { +class Drupal7Password implements PasswordInterface { /** * The minimum allowed log2 number of iterations for password stretching. */ @@ -218,7 +222,7 @@ public function hash($password) { /** * {@inheritdoc} */ - public function check($password, $hash) { + public function verify($password, $hash) { if (substr($hash, 0, 2) == 'U$') { // This may be an updated password from user_update_7000(). Such hashes // have 'U' added as the first character and need an extra md5() (see the @@ -247,8 +251,7 @@ public function check($password, $hash) { return FALSE; } - // Compare using hashEquals() instead of === to mitigate timing attacks. - return $computed_hash && Crypt::hashEquals($stored_hash, $computed_hash); + return ($computed_hash && $stored_hash === $computed_hash); } /** diff --git a/core/lib/Drupal/Core/Password/PasswordInterface.php b/core/lib/Drupal/Core/Password/PasswordInterface.php index b38cc79..c2d14e9 100644 --- a/core/lib/Drupal/Core/Password/PasswordInterface.php +++ b/core/lib/Drupal/Core/Password/PasswordInterface.php @@ -34,7 +34,7 @@ public function hash($password); * @return bool * TRUE if the password is valid, FALSE if not. */ - public function check($password, $hash); + public function verify($password, $hash); /** * Check whether a hashed password needs to be replaced with a new hash. diff --git a/core/lib/Drupal/Core/Password/PhpPassword.php b/core/lib/Drupal/Core/Password/PhpPassword.php new file mode 100644 index 0000000..fa2ef02 --- /dev/null +++ b/core/lib/Drupal/Core/Password/PhpPassword.php @@ -0,0 +1,97 @@ +=5.5.0) password hashing + * functions. + * + * @see http://php.net/manual/en/book.password.php + */ +class PhpPassword implements PasswordInterface { + + /** + * The algorithmic cost that should be used. This is the same 'cost' option as + * is used by the PHP (>= 5.5.0) password_hash() function. + * + * @var int + * + * @see password_hash(). + * @see http://php.net/manual/en/ref.password.php + */ + protected $cost; + + /** + * The Drupal 7 password hashing service. + * + * @var \Drupal\Core\Password\Drupal7Password + */ + protected $drupal7Password; + + /** + * Constructs a new password hashing instance. + * + * @param int $cost + * The algorithmic cost that should be used. + * @param \Drupal\Core\Password\PasswordInterface $drupal7_password + * The Drupal7 password hashing service. + */ + function __construct($cost, PasswordInterface $drupal7_password) { + $this->cost = $cost; + $this->drupal7Password = $drupal7_password; + } + + /** + * {@inheritdoc} + */ + public function hash($password) { + // Prevent DoS attacks by refusing to hash large passwords. + if (strlen($password) > static::PASSWORD_MAX_LENGTH) { + return FALSE; + } + + return password_hash($password, PASSWORD_BCRYPT, $this->getOptions()); + } + + /** + * {@inheritdoc} + */ + public function verify($password, $hash) { + // Try to verify an legacy hash. This may be an Drupal 6 or 7 hash. + if (substr($hash, 0, 4) != '$2y$') { + return $this->drupal7Password->verify($password, $hash); + } + + return password_verify($password, $hash); + } + + /** + * {@inheritdoc} + */ + public function needsRehash($hash) { + // The PHP 5.5 password_needs_rehash() will return TRUE in two cases: + // - The password is either a Drupal 6 MD5 hash rehashed during migration, + // either a Drupal 7 hashed password. In this case the legacy hash will + // not comply the expected password_needs_rehash() format (prefix '$2y$'). + // - The parameters of hashing engine were changed. For example the + // parameter 'password_hash_cost' (the hashing cost) has been increased in + // core.services.yml. + return password_needs_rehash($hash, PASSWORD_BCRYPT, $this->getOptions()); + } + + /** + * Returns password options. + * + * @return array + * Associative array with password options. + */ + protected function getOptions() { + return ['cost' => $this->cost]; + } + +} diff --git a/core/modules/migrate_drupal/tests/fixtures/drupal7.php b/core/modules/migrate_drupal/tests/fixtures/drupal7.php index 98e5638..e2928a3 100644 --- a/core/modules/migrate_drupal/tests/fixtures/drupal7.php +++ b/core/modules/migrate_drupal/tests/fixtures/drupal7.php @@ -34021,7 +34021,7 @@ 'weight' => '0', )) ->values(array( - 'name' => 'PasswordHashingTest', + 'name' => 'Drupal7PasswordTest', 'type' => 'class', 'filename' => 'modules/simpletest/tests/password.test', 'module' => 'simpletest', diff --git a/core/modules/simpletest/src/KernelTestBase.php b/core/modules/simpletest/src/KernelTestBase.php index fbf58a5..cd39245 100644 --- a/core/modules/simpletest/src/KernelTestBase.php +++ b/core/modules/simpletest/src/KernelTestBase.php @@ -364,8 +364,9 @@ public function containerBuild(ContainerBuilder $container) { $definition->clearTag('path_processor_inbound')->clearTag('path_processor_outbound'); } - if ($container->hasDefinition('password')) { - $container->getDefinition('password')->setArguments(array(1)); + if ($container->hasDefinition('password') && $container->hasDefinition('drupal7_password')) { + $container->getDefinition('drupal7_password')->setArguments([1]); + $container->getDefinition('password')->setArguments([4, $container->get('drupal7_password')]); } // Register the stream wrapper manager. diff --git a/core/modules/user/src/Entity/User.php b/core/modules/user/src/Entity/User.php index 062af44..3d6fbd2 100644 --- a/core/modules/user/src/Entity/User.php +++ b/core/modules/user/src/Entity/User.php @@ -395,7 +395,7 @@ public function setExistingPassword($password) { * {@inheritdoc} */ public function checkExistingPassword(UserInterface $account_unchanged) { - return strlen($this->get('pass')->existing) > 0 && \Drupal::service('password')->check(trim($this->get('pass')->existing), $account_unchanged->getPassword()); + return strlen($this->get('pass')->existing) > 0 && \Drupal::service('password')->verify(trim($this->get('pass')->existing), $account_unchanged->getPassword()); } /** diff --git a/core/modules/user/src/Tests/UserLoginTest.php b/core/modules/user/src/Tests/UserLoginTest.php index af00c74..19353b4 100644 --- a/core/modules/user/src/Tests/UserLoginTest.php +++ b/core/modules/user/src/Tests/UserLoginTest.php @@ -4,6 +4,7 @@ use Drupal\simpletest\WebTestBase; use Drupal\user\Entity\User; +use Drupal\user\UserInterface; /** * Ensure that login works as expected. @@ -13,6 +14,20 @@ class UserLoginTest extends WebTestBase { /** + * Drupal password hasher service. + * + * @var \Drupal\Core\Password\PasswordInterface + */ + private $passwordHasher; + + /** + * Drupal 7 password hasher service. + * + * @var \Drupal\Core\Password\PasswordInterface + */ + private $drupal7PasswordHasher; + + /** * Tests login with destination. */ function testLoginCacheTagsAndDestination() { @@ -26,6 +41,9 @@ function testLoginCacheTagsAndDestination() { $edit = array('name' => $user->getUserName(), 'pass' => $user->pass_raw); $this->drupalPostForm(NULL, $edit, t('Log in')); $this->assertUrl('foo', [], 'Redirected to the correct URL'); + + $this->passwordHasher = $this->container->get('password'); + $this->drupal7PasswordHasher = $this->container->get('drupal7_password'); } /** @@ -106,30 +124,23 @@ function testPerUserLoginFloodControl() { } /** - * Test that user password is re-hashed upon login after changing $count_log2. + * Test that user password is re-hashed upon login after changing the cost. */ function testPasswordRehashOnLogin() { - // Determine default log2 for phpass hashing algorithm - $default_count_log2 = 16; - - // Retrieve instance of password hashing algorithm - $password_hasher = $this->container->get('password'); - // Create a new user and authenticate. $account = $this->drupalCreateUser(array()); $password = $account->pass_raw; $this->drupalLogin($account); $this->drupalLogout(); - // Load the stored user. The password hash should reflect $default_count_log2. + // Load the stored user. The password hash should reflect $default_cost. $user_storage = $this->container->get('entity.manager')->getStorage('user'); $account = User::load($account->id()); - $this->assertIdentical($password_hasher->getCountLog2($account->getPassword()), $default_count_log2); + $this->assertTrue($this->passwordHasher->verify($password, $account->getPassword())); - // Change the required number of iterations by loading a test-module - // containing the necessary container builder code and then verify that the - // users password gets rehashed during the login. - $overridden_count_log2 = 19; - \Drupal::service('module_installer')->install(array('user_custom_phpass_params_test')); + // Change the required cost by loading a test-module containing the + // necessary container builder code and then verify that the users password + // gets rehashed during the login. + \Drupal::service('module_installer')->install(array('user_custom_pass_hash_params_test')); $this->resetAll(); $account->pass_raw = $password; @@ -137,11 +148,51 @@ function testPasswordRehashOnLogin() { // Load the stored user, which should have a different password hash now. $user_storage->resetCache(array($account->id())); $account = $user_storage->load($account->id()); - $this->assertIdentical($password_hasher->getCountLog2($account->getPassword()), $overridden_count_log2); - $this->assertTrue($password_hasher->check($password, $account->getPassword())); + $this->assertTrue($this->passwordHasher->verify($password, $account->getPassword())); } /** + * Test MD5 (Drupal 6) passwords rehashing. + */ + public function testDrupal6MigratedPasswordRehashing() { + /** @var \Drupal\user\UserInterface $account */ + $account = $this->drupalCreateUser(); + $plain = $account->pass_raw; + + // We pretend that the user was migrated from Drupal 6. + $md5_pass = md5($plain); + $migrated_pass = 'U' . $this->passwordHasher->hash($md5_pass); + $this->storeHashedPassword($account, $migrated_pass); + + // User first login after migration. + $this->drupalLogin($account); + $this->drupalLogout(); + + // After logging in the user password has been rehashed and is valid. + $this->assertTrue($this->passwordHasher->verify($plain, $account->getPassword())); +} + + /** + * Test Drupal 7 passwords rehashing. + */ + public function testDrupal7MigratedPasswordRehashing() { + /** @var \Drupal\user\UserInterface $account */ + $account = $this->drupalCreateUser(); + $plain = $account->pass_raw; + + // We pretend that the user was migrated from Drupal 7. + $d7_hash = $this->drupal7PasswordHasher->hash($plain); + $this->storeHashedPassword($account, $d7_hash); + + // User first login after migration. + $this->drupalLogin($account); + $this->drupalLogout(); + + // After logging in the user password has been rehashed and is valid. + $this->assertTrue($this->passwordHasher->verify($plain, $account->getPassword())); +} + + /** * Make an unsuccessful login attempt. * * @param \Drupal\user\Entity\User $account @@ -175,4 +226,22 @@ function assertFailedLogin($account, $flood_trigger = NULL) { } } + /** + * Updates the hashed user password bypassing the API. + * + * We want to set an already hashed password. + * + * @param \Drupal\user\UserInterface $account + * The user account. + * @param string $hashed_password + * An already hashed password. + */ + protected function storeHashedPassword(UserInterface $account, $hashed_password) { + $account->setPassword($hashed_password); + db_update('users_field_data') + ->fields(['pass' => $hashed_password]) + ->condition('uid', $account->id()) + ->execute(); + } + } diff --git a/core/modules/user/src/UserAuth.php b/core/modules/user/src/UserAuth.php index 9fbcf09..30da858 100644 --- a/core/modules/user/src/UserAuth.php +++ b/core/modules/user/src/UserAuth.php @@ -47,7 +47,7 @@ public function authenticate($username, $password) { $account_search = $this->entityManager->getStorage('user')->loadByProperties(array('name' => $username)); if ($account = reset($account_search)) { - if ($this->passwordChecker->check($password, $account->getPassword())) { + if ($this->passwordChecker->verify($password, $account->getPassword())) { // Successful authentication. $uid = $account->id(); diff --git a/core/modules/user/tests/modules/user_custom_pass_hash_params_test/user_custom_pass_hash_params_test.info.yml b/core/modules/user/tests/modules/user_custom_pass_hash_params_test/user_custom_pass_hash_params_test.info.yml new file mode 100644 index 0000000..aca50c4 --- /dev/null +++ b/core/modules/user/tests/modules/user_custom_pass_hash_params_test/user_custom_pass_hash_params_test.info.yml @@ -0,0 +1,6 @@ +name: 'User custom password hash params test' +type: module +description: 'Support module for testing custom hashing password algorithm parameters.' +package: Testing +version: VERSION +core: 8.x 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 new file mode 100644 index 0000000..56c1281 --- /dev/null +++ b/core/modules/user/tests/modules/user_custom_pass_hash_params_test/user_custom_pass_hash_params_test.services.yml @@ -0,0 +1,7 @@ +services: + password: + class: Drupal\Core\Password\PhpPassword + arguments: [11, '@drupal7_password'] + drupal7_password: + class: Drupal\Core\Password\Drupal7Password + arguments: [16] diff --git a/core/modules/user/tests/modules/user_custom_phpass_params_test/user_custom_phpass_params_test.info.yml b/core/modules/user/tests/modules/user_custom_phpass_params_test/user_custom_phpass_params_test.info.yml deleted file mode 100644 index 68b8ff9..0000000 --- a/core/modules/user/tests/modules/user_custom_phpass_params_test/user_custom_phpass_params_test.info.yml +++ /dev/null @@ -1,6 +0,0 @@ -name: 'User custom phpass params test' -type: module -description: 'Support module for testing custom phpass password algorithm parameters.' -package: Testing -version: VERSION -core: 8.x diff --git a/core/modules/user/tests/modules/user_custom_phpass_params_test/user_custom_phpass_params_test.services.yml b/core/modules/user/tests/modules/user_custom_phpass_params_test/user_custom_phpass_params_test.services.yml deleted file mode 100644 index 8950bfe..0000000 --- a/core/modules/user/tests/modules/user_custom_phpass_params_test/user_custom_phpass_params_test.services.yml +++ /dev/null @@ -1,4 +0,0 @@ -services: - password: - class: Drupal\Core\Password\PhpassHashedPassword - arguments: [19] diff --git a/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserTest.php b/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserTest.php index 33952d1..f876455 100644 --- a/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserTest.php +++ b/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserTest.php @@ -148,7 +148,7 @@ public function testUser() { // conform to Drupal >= 7 for non-admin users. if ($user->id() != 1) { $this->assertTrue(\Drupal::service('password') - ->check($source->pass_plain, $user->getPassword())); + ->verify($source->pass_plain, $user->getPassword())); } } // Rollback the migration and make sure everything is deleted but uid 1. diff --git a/core/modules/user/tests/src/Unit/UserAuthTest.php b/core/modules/user/tests/src/Unit/UserAuthTest.php index 1b96d8f..6d28101 100644 --- a/core/modules/user/tests/src/Unit/UserAuthTest.php +++ b/core/modules/user/tests/src/Unit/UserAuthTest.php @@ -129,7 +129,7 @@ public function testAuthenticateWithIncorrectPassword() { ->will($this->returnValue(array($this->testUser))); $this->passwordService->expects($this->once()) - ->method('check') + ->method('verify') ->with($this->password, $this->testUser->getPassword()) ->will($this->returnValue(FALSE)); @@ -152,7 +152,7 @@ public function testAuthenticateWithCorrectPassword() { ->will($this->returnValue(array($this->testUser))); $this->passwordService->expects($this->once()) - ->method('check') + ->method('verify') ->with($this->password, $this->testUser->getPassword()) ->will($this->returnValue(TRUE)); @@ -179,7 +179,7 @@ public function testAuthenticateWithZeroPassword() { ->will($this->returnValue(array($this->testUser))); $this->passwordService->expects($this->once()) - ->method('check') + ->method('verify') ->with(0, 0) ->will($this->returnValue(TRUE)); @@ -207,7 +207,7 @@ public function testAuthenticateWithCorrectPasswordAndNewPasswordHash() { ->will($this->returnValue(array($this->testUser))); $this->passwordService->expects($this->once()) - ->method('check') + ->method('verify') ->with($this->password, $this->testUser->getPassword()) ->will($this->returnValue(TRUE)); $this->passwordService->expects($this->once()) diff --git a/core/tests/Drupal/KernelTests/KernelTestBase.php b/core/tests/Drupal/KernelTests/KernelTestBase.php index 4da96ba..3b72863 100644 --- a/core/tests/Drupal/KernelTests/KernelTestBase.php +++ b/core/tests/Drupal/KernelTests/KernelTestBase.php @@ -624,9 +624,11 @@ public function register(ContainerBuilder $container) { ->clearTag('path_processor_outbound'); } - if ($container->hasDefinition('password')) { + if ($container->hasDefinition('password') && $container->hasDefinition('drupal7_password')) { + $container->getDefinition('drupal7_password') + ->setArguments([1]); $container->getDefinition('password') - ->setArguments(array(1)); + ->setArguments([4, $container->get('drupal7_password')]); } TestServiceProvider::addRouteProvider($container); } diff --git a/core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php b/core/tests/Drupal/Tests/Core/Password/Drupal7PasswordTest.php similarity index 73% rename from core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php rename to core/tests/Drupal/Tests/Core/Password/Drupal7PasswordTest.php index 5c378ed..70795f8 100644 --- a/core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php +++ b/core/tests/Drupal/Tests/Core/Password/Drupal7PasswordTest.php @@ -2,22 +2,22 @@ /** * @file - * Contains \Drupal\Tests\Core\Password\PasswordHashingTest. + * Contains Drupal\system\Tests\System\Drupal7PasswordTest. */ namespace Drupal\Tests\Core\Password; -use Drupal\Core\Password\PhpassHashedPassword; +use Drupal\Core\Password\Drupal7Password; use Drupal\Core\Password\PasswordInterface; use Drupal\Tests\UnitTestCase; /** * Unit tests for password hashing API. * - * @coversDefaultClass \Drupal\Core\Password\PhpassHashedPassword + * @coversDefaultClass \Drupal\Core\Password\Drupal7Password * @group System */ -class PasswordHashingTest extends UnitTestCase { +class Drupal7PasswordTest extends UnitTestCase { /** * The user for testing. @@ -50,7 +50,7 @@ class PasswordHashingTest extends UnitTestCase { /** * The password hasher under test. * - * @var \Drupal\Core\Password\PhpassHashedPassword + * @var \Drupal\Core\Password\Drupal7Password */ protected $passwordHasher; @@ -60,7 +60,7 @@ class PasswordHashingTest extends UnitTestCase { protected function setUp() { parent::setUp(); $this->password = $this->randomMachineName(); - $this->passwordHasher = new PhpassHashedPassword(1); + $this->passwordHasher = new Drupal7Password(1); $this->hashedPassword = $this->passwordHasher->hash($this->password); $this->md5HashedPassword = 'U' . $this->passwordHasher->hash(md5($this->password)); } @@ -71,12 +71,11 @@ protected function setUp() { * @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"); + $hasher = new FakeDrupal7Password(); + $this->assertEquals(Drupal7Password::MIN_HASH_COUNT, $hasher->enforceLog2Boundaries(1), 'Min hash count enforced'); + $this->assertEquals(Drupal7Password::MAX_HASH_COUNT, $hasher->enforceLog2Boundaries(100), 'Max hash count enforced'); } - /** * Test a password needs update. * @@ -92,14 +91,14 @@ public function testPasswordNeedsUpdate() { * * @covers ::hash * @covers ::getCountLog2 - * @covers ::check + * @covers ::verify * @covers ::needsRehash */ public function testPasswordHashing() { - $this->assertSame($this->passwordHasher->getCountLog2($this->hashedPassword), PhpassHashedPassword::MIN_HASH_COUNT, 'Hashed password has the minimum number of log2 iterations.'); + $this->assertSame($this->passwordHasher->getCountLog2($this->hashedPassword), Drupal7Password::MIN_HASH_COUNT); $this->assertNotEquals($this->hashedPassword, $this->md5HashedPassword, 'Password hashes not the same.'); - $this->assertTrue($this->passwordHasher->check($this->password, $this->md5HashedPassword), 'Password check succeeds.'); - $this->assertTrue($this->passwordHasher->check($this->password, $this->hashedPassword), 'Password check succeeds.'); + $this->assertTrue($this->passwordHasher->verify($this->password, $this->md5HashedPassword), 'Password check succeeds.'); + $this->assertTrue($this->passwordHasher->verify($this->password, $this->hashedPassword), '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->needsRehash($this->hashedPassword), 'Does not need a new hash.'); @@ -110,22 +109,22 @@ public function testPasswordHashing() { * * @covers ::hash * @covers ::getCountLog2 - * @covers ::check + * @covers ::verify * @covers ::needsRehash */ public function testPasswordRehashing() { // Increment the log2 iteration to MIN + 1. - $password_hasher = new PhpassHashedPassword(PhpassHashedPassword::MIN_HASH_COUNT + 1); + $password_hasher = new Drupal7Password(Drupal7Password::MIN_HASH_COUNT + 1); $this->assertTrue($password_hasher->needsRehash($this->hashedPassword), 'Needs a new hash after incrementing the log2 count.'); // Re-hash the password. $rehashed_password = $password_hasher->hash($this->password); - $this->assertSame($password_hasher->getCountLog2($rehashed_password), PhpassHashedPassword::MIN_HASH_COUNT + 1, 'Re-hashed password has the correct number of log2 iterations.'); + $this->assertSame($password_hasher->getCountLog2($rehashed_password), Drupal7Password::MIN_HASH_COUNT + 1, 'Re-hashed password has the correct number of log2 iterations.'); $this->assertNotEquals($rehashed_password, $this->hashedPassword, 'Password hash changed again.'); // Now the hash should be OK. $this->assertFalse($password_hasher->needsRehash($rehashed_password), 'Re-hashed password does not need a new hash.'); - $this->assertTrue($password_hasher->check($this->password, $rehashed_password), 'Password check succeeds with re-hashed password.'); - $this->assertTrue($this->passwordHasher->check($this->password, $rehashed_password), 'Password check succeeds with re-hashed password with original hasher.'); + $this->assertTrue($password_hasher->verify($this->password, $rehashed_password), 'Password check succeeds with re-hashed password.'); + $this->assertTrue($this->passwordHasher->verify($this->password, $rehashed_password), 'Password check succeeds with re-hashed password with original hasher.'); } /** @@ -175,7 +174,7 @@ public function providerLongPasswords() { /** * A fake class for tests. */ -class FakePhpassHashedPassword extends PhpassHashedPassword { +class FakeDrupal7Password extends Drupal7Password { function __construct() { // Noop. diff --git a/core/tests/Drupal/Tests/Core/Password/PhpPhpPasswordTest.php b/core/tests/Drupal/Tests/Core/Password/PhpPhpPasswordTest.php new file mode 100644 index 0000000..7fb679c --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Password/PhpPhpPasswordTest.php @@ -0,0 +1,162 @@ +passwordHasher = new PhpPassword(4, $d7_hashing_service); + $this->password = $this->randomMachineName(); + $this->md5Hash = md5($this->password); + $this->d7Hash = $d7_hashing_service->hash($this->password); + } + + /** + * Test a password needs update. + * + * @covers ::needsRehash + */ + public function testPasswordNeedsUpdate() { + // The md5 password should be flagged as needing an rehashing. + $this->assertTrue($this->passwordHasher->needsRehash($this->md5Hash)); + // The Drupal 7 password should be flagged as needing an rehashing. + $this->assertTrue($this->passwordHasher->needsRehash($this->d7Hash)); + } + + /** + * Test password hashing. + * + * @covers ::hash + * @covers ::verify + * @covers ::needsRehash + */ + public function testPasswordHashing() { + $this->hashedPassword = $this->passwordHasher->hash($this->password); + + $this->assertNotEquals($this->hashedPassword, $this->md5Hash); + $this->assertNotEquals($this->hashedPassword, $this->d7Hash); + + $this->assertTrue($this->passwordHasher->verify($this->password, $this->hashedPassword)); + $this->assertFalse($this->passwordHasher->needsRehash($this->hashedPassword)); + } + + /** + * Tests password rehashing. + * + * @covers ::hash + * @covers ::check + * @covers ::needsRehash + */ + public function testPasswordRehashing() { + // Increment the cost from 4 to 5. + $this->passwordHasher = new PhpPassword(5, new Drupal7Password(1)); + $this->assertTrue($this->passwordHasher->needsRehash($this->hashedPassword)); + + // Re-hash the password. + $rehashed_password = $this->passwordHasher->hash($this->password); + $this->assertNotEquals($rehashed_password, $this->hashedPassword); + + // Now the hash should be OK. + $this->assertFalse($this->passwordHasher->needsRehash($rehashed_password)); + $this->assertTrue($this->passwordHasher->verify($this->password, $rehashed_password)); + } + + /** + * Verifies that passwords longer than 512 bytes are not hashed. + * + * @covers ::hash + * + * @dataProvider providerLongPasswords + */ + public function testLongPassword($password, $allowed) { + + $hashed_password = $this->passwordHasher->hash($password); + + if ($allowed) { + $this->assertNotFalse($hashed_password); + } + else { + $this->assertFalse($hashed_password); + } + } + + /** + * Provides the test matrix for testLongPassword(). + */ + public function providerLongPasswords() { + // '512 byte long password is allowed.' + $passwords['allowed'] = array(str_repeat('x', PasswordInterface::PASSWORD_MAX_LENGTH), TRUE); + // 513 byte long password is not allowed. + $passwords['too_long'] = array(str_repeat('x', PasswordInterface::PASSWORD_MAX_LENGTH + 1), FALSE); + + // Check a string of 3-byte UTF-8 characters, 510 byte long password is + // allowed. + $len = floor(PasswordInterface::PASSWORD_MAX_LENGTH / 3); + $diff = PasswordInterface::PASSWORD_MAX_LENGTH % 3; + $passwords['utf8'] = array(str_repeat('€', $len), TRUE); + // 512 byte long password is allowed. + $passwords['ut8_extended'] = array($passwords['utf8'][0] . str_repeat('x', $diff), TRUE); + + // Check a string of 3-byte UTF-8 characters, 513 byte long password is + // allowed. + $passwords['utf8_too_long'] = array(str_repeat('€', $len + 1), FALSE); + return $passwords; + } + +}