[Security] add PasswordEncoderInterface::needsRehash()

nicolas-grekas · nicolas-grekas · commit 25c197babe5a · 2019-05-24T10:01:39.000+02:00
diff --git a/UPGRADE-4.4.md b/UPGRADE-4.4.md
@@ -19,3 +19,8 @@ DependencyInjection
      my_service:
        factory: ['@factory_service', method]
    ```
+
+Security
+--------
+
+ * Implementations of `PasswordEncoderInterface` and `UserPasswordEncoderInterface` should add a new `needsRehash()` method
diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md
@@ -304,6 +304,7 @@ Routing
 Security
 --------
 
+ * Implementations of `PasswordEncoderInterface` and `UserPasswordEncoderInterface` must have a new `needsRehash()` method
  * The `Role` and `SwitchUserRole` classes have been removed.
  * The `getReachableRoles()` method of the `RoleHierarchy` class has been removed. It has been replaced by the new
    `getReachableRoleNames()` method.
diff --git a/src/Symfony/Component/Security/CHANGELOG.md b/src/Symfony/Component/Security/CHANGELOG.md
@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+4.4.0
+-----
+
+ * Added method `needsRehash()` to `PasswordEncoderInterface` and `UserPasswordEncoderInterface`
+
 4.3.0
 -----
 
diff --git a/src/Symfony/Component/Security/Core/Encoder/BasePasswordEncoder.php b/src/Symfony/Component/Security/Core/Encoder/BasePasswordEncoder.php
@@ -20,6 +20,14 @@ abstract class BasePasswordEncoder implements PasswordEncoderInterface
 {
     const MAX_PASSWORD_LENGTH = 4096;
 
+    /**
+     * {@inheritdoc}
+     */
+    public function needsRehash(string $encoded): bool
+    {
+        return false;
+    }
+
     /**
      * Demerges a merge password and salt string.
      *
diff --git a/src/Symfony/Component/Security/Core/Encoder/NativePasswordEncoder.php b/src/Symfony/Component/Security/Core/Encoder/NativePasswordEncoder.php
@@ -87,4 +87,12 @@ public function isPasswordValid($encoded, $raw, $salt)
 
         return \strlen($raw) <= self::MAX_PASSWORD_LENGTH && password_verify($raw, $encoded);
     }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function needsRehash(string $encoded): bool
+    {
+        return password_needs_rehash($encoded, $this->algo, $this->options);
+    }
 }
diff --git a/src/Symfony/Component/Security/Core/Encoder/PasswordEncoderInterface.php b/src/Symfony/Component/Security/Core/Encoder/PasswordEncoderInterface.php
@@ -17,6 +17,8 @@
  * PasswordEncoderInterface is the interface for all encoders.
  *
  * @author Fabien Potencier <fabien@symfony.com>
+ *
+ * @method bool needsRehash(string $encoded)
  */
 interface PasswordEncoderInterface
 {
diff --git a/src/Symfony/Component/Security/Core/Encoder/SodiumPasswordEncoder.php b/src/Symfony/Component/Security/Core/Encoder/SodiumPasswordEncoder.php
@@ -94,4 +94,20 @@ public function isPasswordValid($encoded, $raw, $salt)
 
         throw new LogicException('Libsodium is not available. You should either install the sodium extension, upgrade to PHP 7.2+ or use a different encoder.');
     }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function needsRehash(string $encoded): bool
+    {
+        if (\function_exists('sodium_crypto_pwhash_str_needs_rehash')) {
+            return \sodium_crypto_pwhash_str_needs_rehash($encoded, $this->opsLimit, $this->memLimit);
+        }
+
+        if (\extension_loaded('libsodium')) {
+            return \Sodium\crypto_pwhash_str_needs_rehash($encoded, $this->opsLimit, $this->memLimit);
+        }
+
+        throw new LogicException('Libsodium is not available. You should either install the sodium extension, upgrade to PHP 7.2+ or use a different encoder.');
+    }
 }
diff --git a/src/Symfony/Component/Security/Core/Encoder/UserPasswordEncoder.php b/src/Symfony/Component/Security/Core/Encoder/UserPasswordEncoder.php
@@ -46,4 +46,14 @@ public function isPasswordValid(UserInterface $user, $raw)
 
         return $encoder->isPasswordValid($user->getPassword(), $raw, $user->getSalt());
     }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function needsRehash(UserInterface $user, string $encoded): bool
+    {
+        $encoder = $this->encoderFactory->getEncoder($user);
+
+        return method_exists($encoder, 'needsRehash') && $encoder->needsRehash($encoded);
+    }
 }
diff --git a/src/Symfony/Component/Security/Core/Encoder/UserPasswordEncoderInterface.php b/src/Symfony/Component/Security/Core/Encoder/UserPasswordEncoderInterface.php
@@ -17,6 +17,8 @@
  * UserPasswordEncoderInterface is the interface for the password encoder service.
  *
  * @author Ariel Ferrandini <arielferrandini@gmail.com>
+ *
+ * @method bool needsRehash(UserInterface $user, string $encoded)
  */
 interface UserPasswordEncoderInterface
 {
diff --git a/src/Symfony/Component/Security/Core/Tests/Encoder/BasePasswordEncoderTest.php b/src/Symfony/Component/Security/Core/Tests/Encoder/BasePasswordEncoderTest.php
@@ -60,6 +60,12 @@ public function testIsPasswordTooLong()
         $this->assertFalse($this->invokeIsPasswordTooLong(str_repeat('a', 10)));
     }
 
+    public function testNeedsRehash()
+    {
+        $encoder = new PasswordEncoder();
+        $this->assertFalse($encoder->needsRehash('foo'));
+    }
+
     protected function invokeDemergePasswordAndSalt($password)
     {
         $encoder = new PasswordEncoder();
diff --git a/src/Symfony/Component/Security/Core/Tests/Encoder/NativePasswordEncoderTest.php b/src/Symfony/Component/Security/Core/Tests/Encoder/NativePasswordEncoderTest.php
@@ -67,4 +67,17 @@ public function testCheckPasswordLength()
         $this->assertFalse($encoder->isPasswordValid($result, str_repeat('a', 73), 'salt'));
         $this->assertTrue($encoder->isPasswordValid($result, str_repeat('a', 72), 'salt'));
     }
+
+    public function testNeedsRehash()
+    {
+        $encoder = new NativePasswordEncoder(4, 11000, 4);
+
+        $this->assertTrue($encoder->needsRehash('dummyhash'));
+
+        $hash = $encoder->encodePassword('foo', 'salt');
+        $this->assertFalse($encoder->needsRehash($hash));
+
+        $encoder = new NativePasswordEncoder(5, 11000, 5);
+        $this->assertTrue($encoder->needsRehash($hash));
+    }
 }
diff --git a/src/Symfony/Component/Security/Core/Tests/Encoder/SodiumPasswordEncoderTest.php b/src/Symfony/Component/Security/Core/Tests/Encoder/SodiumPasswordEncoderTest.php
@@ -54,4 +54,17 @@ public function testUserProvidedSaltIsNotUsed()
         $result = $encoder->encodePassword('password', 'salt');
         $this->assertTrue($encoder->isPasswordValid($result, 'password', 'anotherSalt'));
     }
+
+    public function testNeedsRehash()
+    {
+        $encoder = new SodiumPasswordEncoder(4, 11000);
+
+        $this->assertTrue($encoder->needsRehash('dummyhash'));
+
+        $hash = $encoder->encodePassword('foo', 'salt');
+        $this->assertFalse($encoder->needsRehash($hash));
+
+        $encoder = new SodiumPasswordEncoder(5, 11000);
+        $this->assertTrue($encoder->needsRehash($hash));
+    }
 }
diff --git a/src/Symfony/Component/Security/Core/Tests/Encoder/UserPasswordEncoderTest.php b/src/Symfony/Component/Security/Core/Tests/Encoder/UserPasswordEncoderTest.php
@@ -12,7 +12,10 @@
 namespace Symfony\Component\Security\Core\Tests\Encoder;
 
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
+use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder;
 use Symfony\Component\Security\Core\Encoder\UserPasswordEncoder;
+use Symfony\Component\Security\Core\User\User;
 
 class UserPasswordEncoderTest extends TestCase
 {
@@ -68,4 +71,23 @@ public function testIsPasswordValid()
         $isValid = $passwordEncoder->isPasswordValid($userMock, 'plainPassword');
         $this->assertTrue($isValid);
     }
+
+    public function testNeedsRehash()
+    {
+        $user = new User('username', null);
+        $encoder = new NativePasswordEncoder(4, 20000, 4);
+
+        $mockEncoderFactory = $this->getMockBuilder(EncoderFactoryInterface::class)->getMock();
+        $mockEncoderFactory->expects($this->any())
+            ->method('getEncoder')
+            ->with($user)
+            ->will($this->onConsecutiveCalls($encoder, $encoder, new NativePasswordEncoder(5, 20000, 5), $encoder));
+
+        $passwordEncoder = new UserPasswordEncoder($mockEncoderFactory);
+
+        $hash = $passwordEncoder->encodePassword($user, 'foo', 'salt');
+        $this->assertFalse($passwordEncoder->needsRehash($user, $hash));
+        $this->assertTrue($passwordEncoder->needsRehash($user, $hash));
+        $this->assertFalse($passwordEncoder->needsRehash($user, $hash));
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,14 @@ abstract class BasePasswordEncoder implements PasswordEncoderInterface`
`20`	`20`	`{`
`21`	`21`	`const MAX_PASSWORD_LENGTH = 4096;`
`22`	`22`
	`23`	`+ /**`
	`24`	`+ * {@inheritdoc}`
	`25`	`+ */`
	`26`	`+ public function needsRehash(string $encoded): bool`
	`27`	`+ {`
	`28`	`+ return false;`
	`29`	`+ }`
	`30`	`+`
`23`	`31`	`/**`
`24`	`32`	`* Demerges a merge password and salt string.`
`25`	`33`	`*`
Original file line number	Diff line number	Diff line change
`@@ -87,4 +87,12 @@ public function isPasswordValid($encoded, $raw, $salt)`
`87`	`87`
`88`	`88`	`return \strlen($raw) <= self::MAX_PASSWORD_LENGTH && password_verify($raw, $encoded);`
`89`	`89`	`}`
	`90`	`+`
	`91`	`+ /**`
	`92`	`+ * {@inheritdoc}`
	`93`	`+ */`
	`94`	`+ public function needsRehash(string $encoded): bool`
	`95`	`+ {`
	`96`	`+ return password_needs_rehash($encoded, $this->algo, $this->options);`
	`97`	`+ }`
`90`	`98`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,8 @@`
`17`	`17`	`* PasswordEncoderInterface is the interface for all encoders.`
`18`	`18`	`*`
`19`	`19`	`* @author Fabien Potencier <fabien@symfony.com>`
	`20`	`+ *`
	`21`	`+ * @method bool needsRehash(string $encoded)`
`20`	`22`	`*/`
`21`	`23`	`interface PasswordEncoderInterface`
`22`	`24`	`{`
Original file line number	Diff line number	Diff line change
`@@ -46,4 +46,14 @@ public function isPasswordValid(UserInterface $user, $raw)`
`46`	`46`
`47`	`47`	`return $encoder->isPasswordValid($user->getPassword(), $raw, $user->getSalt());`
`48`	`48`	`}`
	`49`	`+`
	`50`	`+ /**`
	`51`	`+ * {@inheritdoc}`
	`52`	`+ */`
	`53`	`+ public function needsRehash(UserInterface $user, string $encoded): bool`
	`54`	`+ {`
	`55`	`+ $encoder = $this->encoderFactory->getEncoder($user);`
	`56`	`+`
	`57`	`+ return method_exists($encoder, 'needsRehash') && $encoder->needsRehash($encoded);`
	`58`	`+ }`
`49`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -60,6 +60,12 @@ public function testIsPasswordTooLong()`
`60`	`60`	`$this->assertFalse($this->invokeIsPasswordTooLong(str_repeat('a', 10)));`
`61`	`61`	`}`
`62`	`62`
	`63`	`+ public function testNeedsRehash()`
	`64`	`+ {`
	`65`	`+ $encoder = new PasswordEncoder();`
	`66`	`+ $this->assertFalse($encoder->needsRehash('foo'));`
	`67`	`+ }`
	`68`	`+`
`63`	`69`	`protected function invokeDemergePasswordAndSalt($password)`
`64`	`70`	`{`
`65`	`71`	`$encoder = new PasswordEncoder();`