Skip to content

Commit

Permalink
[Security] add password rehashing capabilities
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolas-grekas committed May 21, 2019
1 parent c315767 commit 41bc2c0
Show file tree
Hide file tree
Showing 21 changed files with 268 additions and 14 deletions.
19 changes: 18 additions & 1 deletion src/Symfony/Bridge/Doctrine/Security/User/EntityUserProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Doctrine\Common\Persistence\ManagerRegistry;
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Core\User\UserProviderInterface;

Expand All @@ -25,7 +26,7 @@
* @author Fabien Potencier <fabien@symfony.com>
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
*/
class EntityUserProvider implements UserProviderInterface
class EntityUserProvider implements UserProviderInterface, PasswordUpgraderInterface
{
private $registry;
private $managerName;
Expand Down Expand Up @@ -107,6 +108,22 @@ public function supportsClass($class)
return $class === $this->getClass() || is_subclass_of($class, $this->getClass());
}

/**
* {@inheritdoc}
*/
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
{
$class = $this->getClass();
if (!$user instanceof $class) {
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', \get_class($user)));
}

$repository = $this->getRepository();
if ($repository instanceof PasswordUpgraderInterface) {
$repository->upgradePassword($user, $newEncodedPassword);
}
}

private function getObjectManager()
{
return $this->registry->getManager($this->managerName);
Expand Down
5 changes: 3 additions & 2 deletions src/Symfony/Bridge/Doctrine/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"symfony/property-access": "~3.4|~4.0",
"symfony/property-info": "~3.4|~4.0",
"symfony/proxy-manager-bridge": "~3.4|~4.0",
"symfony/security": "~3.4|~4.0",
"symfony/security": "^4.4",
"symfony/expression-language": "~3.4|~4.0",
"symfony/validator": "~3.4|~4.0",
"symfony/translation": "~3.4|~4.0",
Expand All @@ -48,7 +48,8 @@
"phpunit/phpunit": "<4.8.35|<5.4.3,>=5.0",
"symfony/dependency-injection": "<3.4",
"symfony/form": "<4.3",
"symfony/messenger": "<4.2"
"symfony/messenger": "<4.2",
"symfony/security-core": "<4.4"
},
"suggest": {
"symfony/form": "",
Expand Down
7 changes: 7 additions & 0 deletions src/Symfony/Component/Security/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
CHANGELOG
=========

4.4.0
-----

* Added `ChainPasswordEncoder`
* Added methods `PasswordEncoderInterface::needsRehash()` and `UserPasswordEncoderInterface::needsRehash()`
* Added and implemented `PasswordUpgraderInterface`

4.3.0
-----

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\Security\Core\Exception\AuthenticationServiceException;
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
use Symfony\Component\Security\Core\User\UserCheckerInterface;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Core\User\UserProviderInterface;
Expand Down Expand Up @@ -54,9 +55,15 @@ protected function checkAuthentication(UserInterface $user, UsernamePasswordToke
throw new BadCredentialsException('The presented password cannot be empty.');
}

if (!$this->encoderFactory->getEncoder($user)->isPasswordValid($user->getPassword(), $presentedPassword, $user->getSalt())) {
$encoder = $this->encoderFactory->getEncoder($user);

if (!$encoder->isPasswordValid($user->getPassword(), $presentedPassword, $user->getSalt())) {
throw new BadCredentialsException('The presented password is invalid.');
}

if ($this->userProvider instanceof PasswordUpgraderInterface && method_exists($encoder, 'needsRehash') && $encoder->needsRehash($user->getPassword())) {
$this->userProvider->upgradePassword($user, $encoder->encodePassword($presentedPassword, $user->getSalt()));
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Security\Core\Encoder;

/**
* Hashes passwords using the best available encoder.
* Validates them using a chain of encoders.
*
* /!\ Don't put a PlaintextPasswordEncoder in the list as that'd mean a leaked hash
* could be used to authenticate successfully without knowing the cleartext password.
*
* @author Nicolas Grekas <p@tchwork.com>
*/
final class ChainPasswordEncoder extends BasePasswordEncoder implements SelfSaltingEncoderInterface
{
private $bestEncoder;
private $extraEncoders;

public function __construct(PasswordEncoderInterface $bestEncoder, PasswordEncoderInterface ...$extraEncoders)
{
$this->bestEncoder = $bestEncoder;
$this->extraEncoders = $extraEncoders;
}

/**
* {@inheritdoc}
*/
public function encodePassword($raw, $salt)
{
return $this->bestEncoder->encodePassword($raw, $salt);
}

/**
* {@inheritdoc}
*/
public function isPasswordValid($encoded, $raw, $salt)
{
if ($this->bestEncoder->isPasswordValid($encoded, $raw, $salt)) {
return true;
}

if (!$this->bestEncoder->needsRehash($encoded)) {
return false;
}

foreach ($this->extraEncoders as $encoder) {
if ($encoder->isPasswordValid($encoded, $raw, $salt)) {
return true;
}
}

return false;
}

/**
* {@inheritdoc}
*/
public function needsRehash(string $encoded): bool
{
return $this->bestEncoder->needsRehash($encoded);
}
}
14 changes: 13 additions & 1 deletion src/Symfony/Component/Security/Core/Encoder/EncoderFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,19 @@ private function createEncoder(array $config)
private function getEncoderConfigFromAlgorithm($config)
{
if ('auto' === $config['algorithm']) {
$config['algorithm'] = SodiumPasswordEncoder::isSupported() ? 'sodium' : 'native';
$encoderChain = [];
// "plaintext" is not listed as any leaked hashes could then be used to authenticate directly
foreach (['sodium', 'native', 'pbkdf2', $config['hash_algorithm']] as $algo) {
if ('sodium' !== $algo || SodiumPasswordEncoder::isSupported()) {
$config['algorithm'] = $algo;
$encoderChain[] = $this->createEncoder($config);
}
}

return [
'class' => ChainPasswordEncoder::class,
'arguments' => $encoderChain,
];
}

switch ($config['algorithm']) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,6 @@ public function encodePassword($raw, $salt)
*/
public function isPasswordValid($encoded, $raw, $salt)
{
return !$this->isPasswordTooLong($raw) && $this->comparePasswords($encoded, $this->encodePassword($raw, $salt));
return '$' !== substr($encoded, 0, 1) && !$this->isPasswordTooLong($raw) && $this->comparePasswords($encoded, $this->encodePassword($raw, $salt));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
* PasswordEncoderInterface is the interface for all encoders.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @method bool needsRehash(string $encoded)
*/
interface PasswordEncoderInterface
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,6 @@ public function encodePassword($raw, $salt)
*/
public function isPasswordValid($encoded, $raw, $salt)
{
return !$this->isPasswordTooLong($raw) && $this->comparePasswords($encoded, $this->encodePassword($raw, $salt));
return '$' !== substr($encoded, 0, 1) && !$this->isPasswordTooLong($raw) && $this->comparePasswords($encoded, $this->encodePassword($raw, $salt));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
14 changes: 13 additions & 1 deletion src/Symfony/Component/Security/Core/User/ChainUserProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
*/
class ChainUserProvider implements UserProviderInterface
class ChainUserProvider implements UserProviderInterface, PasswordUpgraderInterface
{
private $providers;

Expand Down Expand Up @@ -104,4 +104,16 @@ public function supportsClass($class)

return false;
}

/**
* {@inheritdoc}
*/
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
{
foreach ($this->providers as $provider) {
if ($provider instanceof PasswordUpgraderInterface) {
$provider->upgradePassword($user, $newEncodedPassword);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*
* @author Fabien Potencier <fabien@symfony.com>
*/
class InMemoryUserProvider implements UserProviderInterface
class InMemoryUserProvider implements UserProviderInterface, PasswordUpgraderInterface
{
private $users;

Expand Down Expand Up @@ -90,6 +90,19 @@ public function supportsClass($class)
return 'Symfony\Component\Security\Core\User\User' === $class;
}

/**
* {@inheritdoc}
*/
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
{
if (!$user instanceof User) {
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', \get_class($user)));
}

$this->getUser($user->getUsername())->setPassword($newEncodedPassword);
$user->setPassword($newEncodedPassword);
}

/**
* Returns the user by given username.
*
Expand Down
Loading

0 comments on commit 41bc2c0

Please sign in to comment.