From e197398d2ff76698eda9c021e8fda0f1d4157587 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 18 Apr 2019 17:20:05 +0200 Subject: [PATCH] [Security] deprecate BCryptPasswordEncoder in favor of NativePasswordEncoder --- UPGRADE-4.3.md | 3 +- UPGRADE-5.0.md | 3 +- .../Command/UserPasswordEncoderCommand.php | 2 +- .../DependencyInjection/SecurityExtension.php | 2 + .../CompleteConfigurationTest.php | 70 +++++++++++++------ .../Fixtures/php/bcrypt_encoder.php | 12 ++++ .../Fixtures/php/container1.php | 6 +- .../Fixtures/xml/bcrypt_encoder.xml | 16 +++++ .../Fixtures/xml/container1.xml | 6 +- .../Fixtures/yml/bcrypt_encoder.yml | 8 +++ .../Fixtures/yml/container1.yml | 5 +- .../UserPasswordEncoderCommandTest.php | 43 ++++++++++-- .../Functional/app/PasswordEncode/bcrypt.yml | 7 ++ .../Functional/app/PasswordEncode/config.yml | 4 +- src/Symfony/Component/Security/CHANGELOG.md | 3 +- .../Core/Encoder/BCryptPasswordEncoder.php | 4 ++ .../Security/Core/Encoder/EncoderFactory.php | 1 + .../Encoder/BCryptPasswordEncoderTest.php | 2 + 18 files changed, 151 insertions(+), 46 deletions(-) create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/bcrypt_encoder.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/bcrypt_encoder.xml create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/bcrypt_encoder.yml create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordEncode/bcrypt.yml diff --git a/UPGRADE-4.3.md b/UPGRADE-4.3.md index c52cb7436c35..3f5f22164dfa 100644 --- a/UPGRADE-4.3.md +++ b/UPGRADE-4.3.md @@ -168,13 +168,14 @@ Security ``` * The `Argon2iPasswordEncoder` class has been deprecated, use `SodiumPasswordEncoder` instead. + * The `BCryptPasswordEncoder` class has been deprecated, use `NativePasswordEncoder` instead. * Not implementing the methods `__serialize` and `__unserialize` in classes implementing the `TokenInterface` is deprecated SecurityBundle -------------- - * Configuring encoders using `argon2i` as algorithm has been deprecated, use `auto` instead. + * Configuring encoders using `argon2i` or `bcrypt` as algorithm has been deprecated, use `auto` instead. TwigBridge ---------- diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md index 2036cabcd90f..7c409d4630d2 100644 --- a/UPGRADE-5.0.md +++ b/UPGRADE-5.0.md @@ -342,6 +342,7 @@ Security ``` * The `Argon2iPasswordEncoder` class has been removed, use `SodiumPasswordEncoder` instead. + * The `BCryptPasswordEncoder` class has been removed, use `NativePasswordEncoder` instead. * Classes implementing the `TokenInterface` must implement the two new methods `__serialize` and `__unserialize` @@ -364,7 +365,7 @@ SecurityBundle changed to underscores. Before: `my-cookie` deleted the `my_cookie` cookie (with an underscore). After: `my-cookie` deletes the `my-cookie` cookie (with a dash). - * Configuring encoders using `argon2i` as algorithm is not supported anymore, use `sodium` instead. + * Configuring encoders using `argon2i` or `bcrypt` as algorithm is not supported anymore, use `auto` instead. Serializer ---------- diff --git a/src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php b/src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php index 15f59f47540f..84ad3e4c8b92 100644 --- a/src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php +++ b/src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php @@ -70,7 +70,7 @@ protected function configure() security: encoders: Symfony\Component\Security\Core\User\User: plaintext - App\Entity\User: bcrypt + App\Entity\User: auto If you execute the command non-interactively, the first available configured diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index 0463e9011dd9..af4260f04c42 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -558,6 +558,8 @@ private function createEncoder($config, ContainerBuilder $container) // bcrypt encoder if ('bcrypt' === $config['algorithm']) { + @trigger_error('Configuring an encoder with "bcrypt" as algorithm is deprecated since Symfony 4.3, use "auto" instead.', E_USER_DEPRECATED); + return [ 'class' => 'Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder', 'arguments' => [$config['cost'] ?? 13], diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php index 1405ef4bd8fe..ef318946ce66 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php @@ -306,14 +306,10 @@ public function testEncoders() 'arguments' => ['sha1', false, 5, 30], ], 'JMS\FooBundle\Entity\User6' => [ - 'class' => 'Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder', - 'arguments' => [15], - ], - 'JMS\FooBundle\Entity\User7' => [ 'class' => 'Symfony\Component\Security\Core\Encoder\NativePasswordEncoder', 'arguments' => [8, 102400, 15], ], - 'JMS\FooBundle\Entity\User8' => [ + 'JMS\FooBundle\Entity\User7' => [ 'algorithm' => 'auto', 'hash_algorithm' => 'sha512', 'key_length' => 40, @@ -371,25 +367,13 @@ public function testEncodersWithLibsodium() 'arguments' => ['sha1', false, 5, 30], ], 'JMS\FooBundle\Entity\User6' => [ - 'class' => 'Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder', - 'arguments' => [15], + 'class' => 'Symfony\Component\Security\Core\Encoder\NativePasswordEncoder', + 'arguments' => [8, 102400, 15], ], 'JMS\FooBundle\Entity\User7' => [ 'class' => 'Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder', 'arguments' => [8, 128 * 1024 * 1024], ], - 'JMS\FooBundle\Entity\User8' => [ - 'algorithm' => 'auto', - 'hash_algorithm' => 'sha512', - 'key_length' => 40, - 'ignore_case' => false, - 'encode_as_base64' => true, - 'iterations' => 5000, - 'cost' => null, - 'memory_cost' => null, - 'time_cost' => null, - 'threads' => null, - ], ]], $container->getDefinition('security.encoder_factory.generic')->getArguments()); } @@ -441,15 +425,42 @@ public function testEncodersWithArgon2i() 'arguments' => ['sha1', false, 5, 30], ], 'JMS\FooBundle\Entity\User6' => [ - 'class' => 'Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder', - 'arguments' => [15], + 'class' => 'Symfony\Component\Security\Core\Encoder\NativePasswordEncoder', + 'arguments' => [8, 102400, 15], ], 'JMS\FooBundle\Entity\User7' => [ 'class' => 'Symfony\Component\Security\Core\Encoder\Argon2iPasswordEncoder', 'arguments' => [256, 1, 2], ], - 'JMS\FooBundle\Entity\User8' => [ - 'algorithm' => 'auto', + ]], $container->getDefinition('security.encoder_factory.generic')->getArguments()); + } + + /** + * @group legacy + */ + public function testEncodersWithBCrypt() + { + $container = $this->getContainer('bcrypt_encoder'); + + $this->assertEquals([[ + 'JMS\FooBundle\Entity\User1' => [ + 'class' => 'Symfony\Component\Security\Core\Encoder\PlaintextPasswordEncoder', + 'arguments' => [false], + ], + 'JMS\FooBundle\Entity\User2' => [ + 'algorithm' => 'sha1', + 'encode_as_base64' => false, + 'iterations' => 5, + 'hash_algorithm' => 'sha512', + 'key_length' => 40, + 'ignore_case' => false, + 'cost' => null, + 'memory_cost' => null, + 'time_cost' => null, + 'threads' => null, + ], + 'JMS\FooBundle\Entity\User3' => [ + 'algorithm' => 'md5', 'hash_algorithm' => 'sha512', 'key_length' => 40, 'ignore_case' => false, @@ -460,6 +471,19 @@ public function testEncodersWithArgon2i() 'time_cost' => null, 'threads' => null, ], + 'JMS\FooBundle\Entity\User4' => new Reference('security.encoder.foo'), + 'JMS\FooBundle\Entity\User5' => [ + 'class' => 'Symfony\Component\Security\Core\Encoder\Pbkdf2PasswordEncoder', + 'arguments' => ['sha1', false, 5, 30], + ], + 'JMS\FooBundle\Entity\User6' => [ + 'class' => 'Symfony\Component\Security\Core\Encoder\NativePasswordEncoder', + 'arguments' => [8, 102400, 15], + ], + 'JMS\FooBundle\Entity\User7' => [ + 'class' => 'Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder', + 'arguments' => [15], + ], ]], $container->getDefinition('security.encoder_factory.generic')->getArguments()); } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/bcrypt_encoder.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/bcrypt_encoder.php new file mode 100644 index 000000000000..1afad79e4fca --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/bcrypt_encoder.php @@ -0,0 +1,12 @@ +load('container1.php', $container); + +$container->loadFromExtension('security', [ + 'encoders' => [ + 'JMS\FooBundle\Entity\User7' => [ + 'algorithm' => 'bcrypt', + 'cost' => 15, + ], + ], +]); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php index 06afe665d24a..3c9e6104eecc 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php @@ -22,16 +22,12 @@ 'key_length' => 30, ], 'JMS\FooBundle\Entity\User6' => [ - 'algorithm' => 'bcrypt', - 'cost' => 15, - ], - 'JMS\FooBundle\Entity\User7' => [ 'algorithm' => 'native', 'time_cost' => 8, 'memory_cost' => 100, 'cost' => 15, ], - 'JMS\FooBundle\Entity\User8' => [ + 'JMS\FooBundle\Entity\User7' => [ 'algorithm' => 'auto', ], ], diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/bcrypt_encoder.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/bcrypt_encoder.xml new file mode 100644 index 000000000000..a98400c5f043 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/bcrypt_encoder.xml @@ -0,0 +1,16 @@ + + + + + + + + + + + + + diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml index 9a3cae7159a2..c919a7f27673 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml @@ -16,11 +16,9 @@ - + - - - + diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/bcrypt_encoder.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/bcrypt_encoder.yml new file mode 100644 index 000000000000..3f1a52621520 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/bcrypt_encoder.yml @@ -0,0 +1,8 @@ +imports: + - { resource: container1.yml } + +security: + encoders: + JMS\FooBundle\Entity\User7: + algorithm: bcrypt + cost: 15 diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml index a51b0c49cafb..03b9aaf6ef5b 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml @@ -16,14 +16,11 @@ security: iterations: 5 key_length: 30 JMS\FooBundle\Entity\User6: - algorithm: bcrypt - cost: 15 - JMS\FooBundle\Entity\User7: algorithm: native time_cost: 8 memory_cost: 100 cost: 15 - JMS\FooBundle\Entity\User8: + JMS\FooBundle\Entity\User7: algorithm: auto providers: diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/UserPasswordEncoderCommandTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/UserPasswordEncoderCommandTest.php index 15cea530d50e..7e9056224622 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/UserPasswordEncoderCommandTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/UserPasswordEncoderCommandTest.php @@ -18,6 +18,7 @@ use Symfony\Component\Security\Core\Encoder\Argon2iPasswordEncoder; use Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder; use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface; +use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder; use Symfony\Component\Security\Core\Encoder\Pbkdf2PasswordEncoder; use Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder; @@ -54,8 +55,12 @@ public function testEncodeNoPasswordNoInteraction() $this->assertEquals($statusCode, 1); } + /** + * @group legacy + */ public function testEncodePasswordBcrypt() { + $this->setupBcrypt(); $this->passwordEncoderCommandTester->execute([ 'command' => 'security:encode-password', 'password' => 'password', @@ -95,6 +100,23 @@ public function testEncodePasswordArgon2i() $this->assertTrue($encoder->isPasswordValid($hash, 'password', null)); } + public function testEncodePasswordNative() + { + $this->passwordEncoderCommandTester->execute([ + 'command' => 'security:encode-password', + 'password' => 'password', + 'user-class' => 'Custom\Class\Native\User', + ], ['interactive' => false]); + + $output = $this->passwordEncoderCommandTester->getDisplay(); + $this->assertContains('Password encoding succeeded', $output); + + $encoder = new NativePasswordEncoder(); + preg_match('# Encoded password\s{1,}([\w+\/$.,=]+={0,2})\s+#', $output, $matches); + $hash = $matches[1]; + $this->assertTrue($encoder->isPasswordValid($hash, 'password', null)); + } + public function testEncodePasswordSodium() { if (!SodiumPasswordEncoder::isSupported()) { @@ -162,12 +184,12 @@ public function testEncodePasswordEmptySaltOutput() $this->assertNotContains(' Generated salt ', $this->passwordEncoderCommandTester->getDisplay()); } - public function testEncodePasswordBcryptOutput() + public function testEncodePasswordNativeOutput() { $this->passwordEncoderCommandTester->execute([ 'command' => 'security:encode-password', 'password' => 'p@ssw0rd', - 'user-class' => 'Custom\Class\Bcrypt\User', + 'user-class' => 'Custom\Class\Native\User', ], ['interactive' => false]); $this->assertNotContains(' Generated salt ', $this->passwordEncoderCommandTester->getDisplay()); @@ -233,8 +255,8 @@ public function testEncodePasswordAsksNonProvidedUserClass() ], ['decorated' => false]); $this->assertContains(<<passwordEncoderCommandTester = new CommandTester($passwordEncoderCommand); } + private function setupBcrypt() + { + putenv('COLUMNS='.(119 + \strlen(PHP_EOL))); + $kernel = $this->createKernel(['test_case' => 'PasswordEncode', 'root_config' => 'bcrypt.yml']); + $kernel->boot(); + + $application = new Application($kernel); + + $passwordEncoderCommand = $application->get('security:encode-password'); + + $this->passwordEncoderCommandTester = new CommandTester($passwordEncoderCommand); + } + private function setupSodium() { putenv('COLUMNS='.(119 + \strlen(PHP_EOL))); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordEncode/bcrypt.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordEncode/bcrypt.yml new file mode 100644 index 000000000000..1928c0400b72 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordEncode/bcrypt.yml @@ -0,0 +1,7 @@ +imports: + - { resource: config.yml } + +security: + encoders: + Custom\Class\Bcrypt\User: + algorithm: bcrypt diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordEncode/config.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordEncode/config.yml index 82416b095748..9ae543324656 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordEncode/config.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordEncode/config.yml @@ -4,8 +4,8 @@ imports: security: encoders: Symfony\Component\Security\Core\User\User: plaintext - Custom\Class\Bcrypt\User: - algorithm: bcrypt + Custom\Class\Native\User: + algorithm: native cost: 10 Custom\Class\Pbkdf2\User: algorithm: pbkdf2 diff --git a/src/Symfony/Component/Security/CHANGELOG.md b/src/Symfony/Component/Security/CHANGELOG.md index 5dc78560733f..f7b868fcbade 100644 --- a/src/Symfony/Component/Security/CHANGELOG.md +++ b/src/Symfony/Component/Security/CHANGELOG.md @@ -21,7 +21,8 @@ CHANGELOG * Dispatch `AuthenticationFailureEvent` on `security.authentication.failure` * Dispatch `InteractiveLoginEvent` on `security.interactive_login` * Dispatch `SwitchUserEvent` on `security.switch_user` - * Deprecated `Argon2iPasswordEncoder`, use `SodiumPasswordEncoder` + * Deprecated `Argon2iPasswordEncoder`, use `SodiumPasswordEncoder` instead + * Deprecated `BCryptPasswordEncoder`, use `NativePasswordEncoder` instead 4.2.0 ----- diff --git a/src/Symfony/Component/Security/Core/Encoder/BCryptPasswordEncoder.php b/src/Symfony/Component/Security/Core/Encoder/BCryptPasswordEncoder.php index 7855552af61a..d14949465c0e 100644 --- a/src/Symfony/Component/Security/Core/Encoder/BCryptPasswordEncoder.php +++ b/src/Symfony/Component/Security/Core/Encoder/BCryptPasswordEncoder.php @@ -11,11 +11,15 @@ namespace Symfony\Component\Security\Core\Encoder; +@trigger_error(sprintf('The "%s" class is deprecated since Symfony 4.3, use "%s" instead.', BCryptPasswordEncoder::class, NativePasswordEncoder::class), E_USER_DEPRECATED); + use Symfony\Component\Security\Core\Exception\BadCredentialsException; /** * @author Elnur Abdurrakhimov * @author Terje Bråten + * + * @deprecated since Symfony 4.3, use NativePasswordEncoder instead */ class BCryptPasswordEncoder extends BasePasswordEncoder implements SelfSaltingEncoderInterface { diff --git a/src/Symfony/Component/Security/Core/Encoder/EncoderFactory.php b/src/Symfony/Component/Security/Core/Encoder/EncoderFactory.php index 7d87c6f904ef..150190dc4c16 100644 --- a/src/Symfony/Component/Security/Core/Encoder/EncoderFactory.php +++ b/src/Symfony/Component/Security/Core/Encoder/EncoderFactory.php @@ -106,6 +106,7 @@ private function getEncoderConfigFromAlgorithm($config) ], ]; + /* @deprecated since Symfony 4.3 */ case 'bcrypt': return [ 'class' => BCryptPasswordEncoder::class, diff --git a/src/Symfony/Component/Security/Core/Tests/Encoder/BCryptPasswordEncoderTest.php b/src/Symfony/Component/Security/Core/Tests/Encoder/BCryptPasswordEncoderTest.php index 7f9dbe7f33fd..4e8fcde7b069 100644 --- a/src/Symfony/Component/Security/Core/Tests/Encoder/BCryptPasswordEncoderTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Encoder/BCryptPasswordEncoderTest.php @@ -16,6 +16,8 @@ /** * @author Elnur Abdurrakhimov + * + * @group legacy */ class BCryptPasswordEncoderTest extends TestCase {