From 80955be45d905a17075ed4b8043dc3007154d91b Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Sun, 27 Oct 2019 11:19:19 +0100 Subject: [PATCH] [Security] Add migrating encoder configuration --- .../Bundle/SecurityBundle/CHANGELOG.md | 1 + .../DependencyInjection/MainConfiguration.php | 4 + .../DependencyInjection/SecurityExtension.php | 4 + .../CompleteConfigurationTest.php | 78 ++++++++++++++++++- .../Fixtures/php/migrating_encoder.php | 14 ++++ .../Fixtures/xml/migrating_encoder.xml | 18 +++++ .../Fixtures/yml/migrating_encoder.yml | 10 +++ .../Security/Core/Encoder/EncoderFactory.php | 41 +++++++++- .../Core/Tests/Encoder/EncoderFactoryTest.php | 41 ++++++++++ 9 files changed, 206 insertions(+), 5 deletions(-) create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/migrating_encoder.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/migrating_encoder.xml create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/migrating_encoder.yml diff --git a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md index b53fa3bf55..5a18e1f7b0 100644 --- a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG 4.4.0 ----- + * Added `migrate_from` option to encoders configuration. * Added new `argon2id` encoder, undeprecated the `bcrypt` and `argon2i` ones (using `auto` is still recommended by default.) * Deprecated the usage of "query_string" without a "search_dn" and a "search_password" config key in Ldap factories. * Marked the `SecurityDataCollector` class as `@final`. diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php index 20c6a25984..58ea0f3fe4 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php @@ -394,6 +394,10 @@ class MainConfiguration implements ConfigurationInterface ->beforeNormalization()->ifString()->then(function ($v) { return ['algorithm' => $v]; })->end() ->children() ->scalarNode('algorithm')->cannotBeEmpty()->end() + ->arrayNode('migrate_from') + ->prototype('scalar')->end() + ->beforeNormalization()->castToArray()->end() + ->end() ->scalarNode('hash_algorithm')->info('Name of hashing algorithm for PBKDF2 (i.e. sha256, sha512, etc..) See hash_algos() for a list of supported algorithms.')->defaultValue('sha512')->end() ->scalarNode('key_length')->defaultValue(40)->end() ->booleanNode('ignore_case')->defaultFalse()->end() diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index 39cf8986b2..480768a6d0 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -512,6 +512,10 @@ class SecurityExtension extends Extension implements PrependExtensionInterface return new Reference($config['id']); } + if ($config['migrate_from'] ?? false) { + return $config; + } + // plaintext encoder if ('plaintext' === $config['algorithm']) { $arguments = [$config['ignore_case']]; diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php index 66a6c6842a..8b1ce20bd5 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php @@ -287,6 +287,7 @@ abstract class CompleteConfigurationTest extends TestCase 'memory_cost' => null, 'time_cost' => null, 'threads' => null, + 'migrate_from' => [], ], 'JMS\FooBundle\Entity\User3' => [ 'algorithm' => 'md5', @@ -299,6 +300,7 @@ abstract class CompleteConfigurationTest extends TestCase 'memory_cost' => null, 'time_cost' => null, 'threads' => null, + 'migrate_from' => [], ], 'JMS\FooBundle\Entity\User4' => new Reference('security.encoder.foo'), 'JMS\FooBundle\Entity\User5' => [ @@ -320,6 +322,7 @@ abstract class CompleteConfigurationTest extends TestCase 'memory_cost' => null, 'time_cost' => null, 'threads' => null, + 'migrate_from' => [], ], ]], $container->getDefinition('security.encoder_factory.generic')->getArguments()); } @@ -348,6 +351,7 @@ abstract class CompleteConfigurationTest extends TestCase 'memory_cost' => null, 'time_cost' => null, 'threads' => null, + 'migrate_from' => [], ], 'JMS\FooBundle\Entity\User3' => [ 'algorithm' => 'md5', @@ -360,6 +364,7 @@ abstract class CompleteConfigurationTest extends TestCase 'memory_cost' => null, 'time_cost' => null, 'threads' => null, + 'migrate_from' => [], ], 'JMS\FooBundle\Entity\User4' => new Reference('security.encoder.foo'), 'JMS\FooBundle\Entity\User5' => [ @@ -401,6 +406,7 @@ abstract class CompleteConfigurationTest extends TestCase 'memory_cost' => null, 'time_cost' => null, 'threads' => null, + 'migrate_from' => [], ], 'JMS\FooBundle\Entity\User3' => [ 'algorithm' => 'md5', @@ -413,6 +419,7 @@ abstract class CompleteConfigurationTest extends TestCase 'memory_cost' => null, 'time_cost' => null, 'threads' => null, + 'migrate_from' => [], ], 'JMS\FooBundle\Entity\User4' => new Reference('security.encoder.foo'), 'JMS\FooBundle\Entity\User5' => [ @@ -430,9 +437,14 @@ abstract class CompleteConfigurationTest extends TestCase ]], $container->getDefinition('security.encoder_factory.generic')->getArguments()); } - public function testEncodersWithBCrypt() + public function testMigratingEncoder() { - $container = $this->getContainer('bcrypt_encoder'); + if (!($sodium = SodiumPasswordEncoder::isSupported() && !\defined('SODIUM_CRYPTO_PWHASH_ALG_ARGON2ID13')) && !\defined('PASSWORD_ARGON2I')) { + $this->markTestSkipped('Argon2i algorithm is not supported.'); + } + + $container = $this->getContainer('migrating_encoder'); + $this->assertEquals([[ 'JMS\FooBundle\Entity\User1' => [ 'class' => 'Symfony\Component\Security\Core\Encoder\PlaintextPasswordEncoder', @@ -449,6 +461,7 @@ abstract class CompleteConfigurationTest extends TestCase 'memory_cost' => null, 'time_cost' => null, 'threads' => null, + 'migrate_from' => [], ], 'JMS\FooBundle\Entity\User3' => [ 'algorithm' => 'md5', @@ -461,6 +474,67 @@ abstract class CompleteConfigurationTest extends TestCase 'memory_cost' => null, 'time_cost' => null, 'threads' => null, + 'migrate_from' => [], + ], + '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' => [ + 'algorithm' => 'argon2i', + 'hash_algorithm' => 'sha512', + 'key_length' => 40, + 'ignore_case' => false, + 'encode_as_base64' => true, + 'iterations' => 5000, + 'cost' => null, + 'memory_cost' => 256, + 'time_cost' => 1, + 'threads' => null, + 'migrate_from' => ['bcrypt'], + ], + ]], $container->getDefinition('security.encoder_factory.generic')->getArguments()); + } + + 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, + 'migrate_from' => [], + ], + 'JMS\FooBundle\Entity\User3' => [ + 'algorithm' => 'md5', + '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, + 'migrate_from' => [], ], 'JMS\FooBundle\Entity\User4' => new Reference('security.encoder.foo'), 'JMS\FooBundle\Entity\User5' => [ diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/migrating_encoder.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/migrating_encoder.php new file mode 100644 index 0000000000..14c008be9d --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/migrating_encoder.php @@ -0,0 +1,14 @@ +load('container1.php', $container); + +$container->loadFromExtension('security', [ + 'encoders' => [ + 'JMS\FooBundle\Entity\User7' => [ + 'algorithm' => 'argon2i', + 'memory_cost' => 256, + 'time_cost' => 1, + 'migrate_from' => 'bcrypt', + ], + ], +]); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/migrating_encoder.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/migrating_encoder.xml new file mode 100644 index 0000000000..d820118075 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/migrating_encoder.xml @@ -0,0 +1,18 @@ + + + + + + + + + + + bcrypt + + + + diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/migrating_encoder.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/migrating_encoder.yml new file mode 100644 index 0000000000..9eda61c188 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/migrating_encoder.yml @@ -0,0 +1,10 @@ +imports: + - { resource: container1.yml } + +security: + encoders: + JMS\FooBundle\Entity\User7: + algorithm: argon2i + memory_cost: 256 + time_cost: 1 + migrate_from: bcrypt diff --git a/src/Symfony/Component/Security/Core/Encoder/EncoderFactory.php b/src/Symfony/Component/Security/Core/Encoder/EncoderFactory.php index 9ec8c7c3f2..b3d5ff3809 100644 --- a/src/Symfony/Component/Security/Core/Encoder/EncoderFactory.php +++ b/src/Symfony/Component/Security/Core/Encoder/EncoderFactory.php @@ -65,9 +65,10 @@ class EncoderFactory implements EncoderFactoryInterface * * @throws \InvalidArgumentException */ - private function createEncoder(array $config): PasswordEncoderInterface + private function createEncoder(array $config, bool $isExtra = false): PasswordEncoderInterface { if (isset($config['algorithm'])) { + $rawConfig = $config; $config = $this->getEncoderConfigFromAlgorithm($config); } if (!isset($config['class'])) { @@ -79,7 +80,23 @@ class EncoderFactory implements EncoderFactoryInterface $reflection = new \ReflectionClass($config['class']); - return $reflection->newInstanceArgs($config['arguments']); + $encoder = $reflection->newInstanceArgs($config['arguments']); + + if ($isExtra || !\in_array($config['class'], [NativePasswordEncoder::class, SodiumPasswordEncoder::class], true)) { + return $encoder; + } + + if ($rawConfig ?? null) { + $extraEncoders = array_map(function (string $algo) use ($rawConfig): PasswordEncoderInterface { + $rawConfig['algorithm'] = $algo; + + return $this->createEncoder($rawConfig); + }, ['pbkdf2', $rawConfig['hash_algorithm'] ?? 'sha512']); + } else { + $extraEncoders = [new Pbkdf2PasswordEncoder(), new MessageDigestPasswordEncoder()]; + } + + return new MigratingPasswordEncoder($encoder, ...$extraEncoders); } private function getEncoderConfigFromAlgorithm(array $config): array @@ -89,7 +106,25 @@ class EncoderFactory implements EncoderFactoryInterface // "plaintext" is not listed as any leaked hashes could then be used to authenticate directly foreach ([SodiumPasswordEncoder::isSupported() ? 'sodium' : 'native', 'pbkdf2', $config['hash_algorithm']] as $algo) { $config['algorithm'] = $algo; - $encoderChain[] = $this->createEncoder($config); + $encoderChain[] = $this->createEncoder($config, true); + } + + return [ + 'class' => MigratingPasswordEncoder::class, + 'arguments' => $encoderChain, + ]; + } + + if ($fromEncoders = ($config['migrate_from'] ?? false)) { + $encoderChain = []; + foreach ($fromEncoders as $name) { + if ($encoder = $this->encoders[$name] ?? false) { + $encoder = $encoder instanceof PasswordEncoderInterface ? $encoder : $this->createEncoder($encoder, true); + } else { + $encoder = $this->createEncoder(['algorithm' => $name], true); + } + + $encoderChain[] = $encoder; } return [ diff --git a/src/Symfony/Component/Security/Core/Tests/Encoder/EncoderFactoryTest.php b/src/Symfony/Component/Security/Core/Tests/Encoder/EncoderFactoryTest.php index d7943d2a07..c8d73d5b15 100644 --- a/src/Symfony/Component/Security/Core/Tests/Encoder/EncoderFactoryTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Encoder/EncoderFactoryTest.php @@ -15,6 +15,9 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Encoder\EncoderAwareInterface; use Symfony\Component\Security\Core\Encoder\EncoderFactory; use Symfony\Component\Security\Core\Encoder\MessageDigestPasswordEncoder; +use Symfony\Component\Security\Core\Encoder\MigratingPasswordEncoder; +use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder; +use Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder; use Symfony\Component\Security\Core\User\User; use Symfony\Component\Security\Core\User\UserInterface; @@ -131,6 +134,44 @@ class EncoderFactoryTest extends TestCase $expectedEncoder = new MessageDigestPasswordEncoder('sha1'); $this->assertEquals($expectedEncoder->encodePassword('foo', ''), $encoder->encodePassword('foo', '')); } + + public function testMigrateFrom() + { + if (!SodiumPasswordEncoder::isSupported()) { + $this->markTestSkipped('Sodium is not available'); + } + + $factory = new EncoderFactory([ + 'digest_encoder' => $digest = new MessageDigestPasswordEncoder('sha256'), + 'pbdkf2' => $digest = new MessageDigestPasswordEncoder('sha256'), + 'bcrypt_encoder' => ['algorithm' => 'bcrypt'], + SomeUser::class => ['algorithm' => 'sodium', 'migrate_from' => ['bcrypt_encoder', 'digest_encoder']], + ]); + + $encoder = $factory->getEncoder(SomeUser::class); + $this->assertInstanceOf(MigratingPasswordEncoder::class, $encoder); + + $this->assertTrue($encoder->isPasswordValid((new SodiumPasswordEncoder())->encodePassword('foo', null), 'foo', null)); + $this->assertTrue($encoder->isPasswordValid((new NativePasswordEncoder(null, null, null, \PASSWORD_BCRYPT))->encodePassword('foo', null), 'foo', null)); + $this->assertTrue($encoder->isPasswordValid($digest->encodePassword('foo', null), 'foo', null)); + } + + public function testDefaultMigratingEncoders() + { + $this->assertInstanceOf( + MigratingPasswordEncoder::class, + (new EncoderFactory([SomeUser::class => ['class' => NativePasswordEncoder::class, 'arguments' => []]]))->getEncoder(SomeUser::class) + ); + + if (!SodiumPasswordEncoder::isSupported()) { + return; + } + + $this->assertInstanceOf( + MigratingPasswordEncoder::class, + (new EncoderFactory([SomeUser::class => ['class' => SodiumPasswordEncoder::class, 'arguments' => []]]))->getEncoder(SomeUser::class) + ); + } } class SomeUser implements UserInterface