Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[Security] Replace Argon2*PasswordEncoder by SodiumPasswordEncoder #31019

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 9, 2019
Merged

[Security] Replace Argon2*PasswordEncoder by SodiumPasswordEncoder #31019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions 6 UPGRADE-4.3.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,12 @@ Security
}
```

* Using `Argon2iPasswordEncoder` while only the `argon2id` algorithm is supported
is deprecated, use `Argon2idPasswordEncoder` instead
* The `Argon2iPasswordEncoder` class has been deprecated, use `SodiumPasswordEncoder` instead.

SecurityBundle
--------------

* Configuring encoders using `argon2i` as algorithm while only `argon2id` is
supported is deprecated, use `argon2id` instead
* Configuring encoders using `argon2i` as algorithm has been deprecated, use `sodium` instead.

TwigBridge
----------
Expand Down
6 changes: 2 additions & 4 deletions 6 UPGRADE-5.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,7 @@ Security
}
```

* Using `Argon2iPasswordEncoder` while only the `argon2id` algorithm is supported
now throws a `\LogicException`, use `Argon2idPasswordEncoder` instead
* The `Argon2iPasswordEncoder` class has been removed, use `SodiumPasswordEncoder` instead.

SecurityBundle
--------------
Expand All @@ -348,8 +347,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 while only `argon2id` is supported
now throws a `\LogicException`, use `argon2id` instead
* Configuring encoders using `argon2i` as algorithm is not supported anymore, use `sodium` instead.

Serializer
----------
Expand Down
4 changes: 1 addition & 3 deletions 4 src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ CHANGELOG
option is deprecated and will be disabled in Symfony 5.0. This affects to cookies
with dashes in their names. For example, starting from Symfony 5.0, the `my-cookie`
name will delete `my-cookie` (with a dash) instead of `my_cookie` (with an underscore).
* Deprecated configuring encoders using `argon2i` as algorithm while only `argon2id` is supported,
use `argon2id` instead

* Deprecated configuring encoders using `argon2i` as algorithm, use `sodium` instead

4.2.0
-----
Expand Down
21 changes: 8 additions & 13 deletions 21 src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
use Symfony\Component\Security\Core\Encoder\Argon2idPasswordEncoder;
use Symfony\Component\Security\Core\Encoder\Argon2iPasswordEncoder;
use Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder;
use Symfony\Component\Security\Core\User\UserProviderInterface;
use Symfony\Component\Security\Http\Controller\UserValueResolver;
use Symfony\Component\Templating\PhpEngine;
Expand Down Expand Up @@ -565,14 +565,14 @@ private function createEncoder($config, ContainerBuilder $container)

// Argon2i encoder
if ('argon2i' === $config['algorithm']) {
@trigger_error('Configuring an encoder with "argon2i" as algorithm is deprecated since Symfony 4.3, use "sodium" instead.', E_USER_DEPRECATED);

if (!Argon2iPasswordEncoder::isSupported()) {
if (\extension_loaded('sodium') && !\defined('SODIUM_CRYPTO_PWHASH_SALTBYTES')) {
throw new InvalidConfigurationException('The installed libsodium version does not have support for Argon2i. Use Bcrypt instead.');
}

throw new InvalidConfigurationException('Argon2i algorithm is not supported. Install the libsodium extension or use BCrypt instead.');
} elseif (!\defined('PASSWORD_ARGON2I') && Argon2idPasswordEncoder::isDefaultSodiumAlgorithm()) {
@trigger_error('Configuring an encoder based on the "argon2i" algorithm while only "argon2id" is supported is deprecated since Symfony 4.3, use "argon2id" instead.', E_USER_DEPRECATED);
}

return [
Expand All @@ -585,19 +585,14 @@ private function createEncoder($config, ContainerBuilder $container)
];
}

// Argon2id encoder
if ('argon2id' === $config['algorithm']) {
if (!Argon2idPasswordEncoder::isSupported()) {
throw new InvalidConfigurationException('Argon2i algorithm is not supported. Install the libsodium extension or use BCrypt instead.');
if ('sodium' === $config['algorithm']) {
if (!SodiumPasswordEncoder::isSupported()) {
throw new InvalidConfigurationException('Libsodium is not available. Install the sodium extension or use BCrypt instead.');
}

return [
'class' => Argon2idPasswordEncoder::class,
'arguments' => [
$config['memory_cost'],
$config['time_cost'],
$config['threads'],
],
'class' => SodiumPasswordEncoder::class,
'arguments' => [],
];
}

Expand Down
25 changes: 15 additions & 10 deletions 25 src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
use Symfony\Component\Security\Core\Encoder\Argon2idPasswordEncoder;
use Symfony\Component\Security\Core\Encoder\Argon2iPasswordEncoder;
use Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder;

abstract class CompleteConfigurationTest extends TestCase
{
Expand Down Expand Up @@ -314,11 +314,11 @@ public function testEncoders()

public function testEncodersWithLibsodium()
{
if (!Argon2iPasswordEncoder::isSupported() || \defined('SODIUM_CRYPTO_PWHASH_ALG_ARGON2ID13')) {
$this->markTestSkipped('Argon2i algorithm is not supported.');
if (!SodiumPasswordEncoder::isSupported()) {
$this->markTestSkipped('Libsodium is not available.');
}

$container = $this->getContainer('argon2i_encoder');
$container = $this->getContainer('sodium_encoder');

$this->assertEquals([[
'JMS\FooBundle\Entity\User1' => [
Expand Down Expand Up @@ -359,19 +359,24 @@ public function testEncodersWithLibsodium()
'arguments' => [15],
],
'JMS\FooBundle\Entity\User7' => [
'class' => 'Symfony\Component\Security\Core\Encoder\Argon2iPasswordEncoder',
'arguments' => [256, 1, 2],
'class' => 'Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder',
'arguments' => [],
],
]], $container->getDefinition('security.encoder_factory.generic')->getArguments());
}

public function testEncodersWithArgon2id()
/**
* @group legacy
*
* @expectedDeprecation Configuring an encoder with "argon2i" as algorithm is deprecated since Symfony 4.3, use "sodium" instead.
*/
public function testEncodersWithArgon2i()
{
if (!Argon2idPasswordEncoder::isSupported()) {
if (!Argon2iPasswordEncoder::isSupported()) {
$this->markTestSkipped('Argon2i algorithm is not supported.');
}

$container = $this->getContainer('argon2id_encoder');
$container = $this->getContainer('argon2i_encoder');

$this->assertEquals([[
'JMS\FooBundle\Entity\User1' => [
Expand Down Expand Up @@ -412,7 +417,7 @@ public function testEncodersWithArgon2id()
'arguments' => [15],
],
'JMS\FooBundle\Entity\User7' => [
'class' => 'Symfony\Component\Security\Core\Encoder\Argon2idPasswordEncoder',
'class' => 'Symfony\Component\Security\Core\Encoder\Argon2iPasswordEncoder',
'arguments' => [256, 1, 2],
],
]], $container->getDefinition('security.encoder_factory.generic')->getArguments());
Expand Down
5 changes: 1 addition & 4 deletions 5 ...jection/Fixtures/php/argon2id_encoder.php → ...Injection/Fixtures/php/sodium_encoder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@
$container->loadFromExtension('security', [
'encoders' => [
'JMS\FooBundle\Entity\User7' => [
'algorithm' => 'argon2id',
'memory_cost' => 256,
'time_cost' => 1,
'threads' => 2,
'algorithm' => 'sodium',
],
],
]);
2 changes: 1 addition & 1 deletion 2 ...jection/Fixtures/xml/argon2id_encoder.xml → ...Injection/Fixtures/xml/sodium_encoder.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
</imports>

<sec:config>
<sec:encoder class="JMS\FooBundle\Entity\User7" algorithm="argon2id" memory_cost="256" time_cost="1" threads="2" />
<sec:encoder class="JMS\FooBundle\Entity\User7" algorithm="sodium" />
</sec:config>

</container>
10 changes: 0 additions & 10 deletions 10 ...Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/argon2id_encoder.yml
View file
Open in desktop

This file was deleted.

7 changes: 7 additions & 0 deletions 7 src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/sodium_encoder.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
imports:
- { resource: container1.yml }

security:
encoders:
JMS\FooBundle\Entity\User7:
algorithm: sodium
45 changes: 25 additions & 20 deletions 45 src/Symfony/Bundle/SecurityBundle/Tests/Functional/UserPasswordEncoderCommandTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
use Symfony\Bundle\SecurityBundle\Command\UserPasswordEncoderCommand;
use Symfony\Component\Console\Application as ConsoleApplication;
use Symfony\Component\Console\Tester\CommandTester;
use Symfony\Component\Security\Core\Encoder\Argon2idPasswordEncoder;
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\Pbkdf2PasswordEncoder;
use Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder;

/**
* Tests UserPasswordEncoderCommand.
Expand Down Expand Up @@ -71,9 +71,12 @@ public function testEncodePasswordBcrypt()
$this->assertTrue($encoder->isPasswordValid($hash, 'password', null));
}

/**
* @group legacy
*/
public function testEncodePasswordArgon2i()
{
if (!Argon2iPasswordEncoder::isSupported() || !\defined('PASSWORD_ARGON2I') && Argon2idPasswordEncoder::isDefaultSodiumAlgorithm()) {
if (!Argon2iPasswordEncoder::isSupported()) {
$this->markTestSkipped('Argon2i algorithm not available.');
}
$this->setupArgon2i();
Expand All @@ -87,30 +90,29 @@ public function testEncodePasswordArgon2i()
$this->assertContains('Password encoding succeeded', $output);

$encoder = new Argon2iPasswordEncoder();
preg_match('# Encoded password\s+(\$argon2i?\$[\w,=\$+\/]+={0,2})\s+#', $output, $matches);
preg_match('# Encoded password\s+(\$argon2id?\$[\w,=\$+\/]+={0,2})\s+#', $output, $matches);
$hash = $matches[1];
$this->assertTrue($encoder->isPasswordValid($hash, 'password', null));
}

public function testEncodePasswordArgon2id()
public function testEncodePasswordSodium()
{
if (!Argon2idPasswordEncoder::isSupported()) {
$this->markTestSkipped('Argon2id algorithm not available.');
if (!SodiumPasswordEncoder::isSupported()) {
$this->markTestSkipped('Libsodium is not available.');
}
$this->setupArgon2id();
$this->setupSodium();
$this->passwordEncoderCommandTester->execute([
'command' => 'security:encode-password',
'password' => 'password',
'user-class' => 'Custom\Class\Argon2id\User',
'user-class' => 'Custom\Class\Sodium\User',
], ['interactive' => false]);

$output = $this->passwordEncoderCommandTester->getDisplay();
$this->assertContains('Password encoding succeeded', $output);

$encoder = new Argon2idPasswordEncoder();
preg_match('# Encoded password\s+(\$argon2id?\$[\w,=\$+\/]+={0,2})\s+#', $output, $matches);
preg_match('# Encoded password\s+(\$?\$[\w,=\$+\/]+={0,2})\s+#', $output, $matches);
$hash = $matches[1];
$this->assertTrue($encoder->isPasswordValid($hash, 'password', null));
$this->assertTrue((new SodiumPasswordEncoder())->isPasswordValid($hash, 'password', null));
}

public function testEncodePasswordPbkdf2()
Expand Down Expand Up @@ -173,10 +175,13 @@ public function testEncodePasswordBcryptOutput()
$this->assertNotContains(' Generated salt ', $this->passwordEncoderCommandTester->getDisplay());
}

/**
* @group legacy
*/
public function testEncodePasswordArgon2iOutput()
{
if (!Argon2iPasswordEncoder::isSupported() || !\defined('PASSWORD_ARGON2I') && Argon2idPasswordEncoder::isDefaultSodiumAlgorithm()) {
$this->markTestSkipped('Argon2id algorithm not available.');
if (!Argon2iPasswordEncoder::isSupported()) {
$this->markTestSkipped('Argon2i algorithm not available.');
}

$this->setupArgon2i();
Expand All @@ -189,17 +194,17 @@ public function testEncodePasswordArgon2iOutput()
$this->assertNotContains(' Generated salt ', $this->passwordEncoderCommandTester->getDisplay());
}

public function testEncodePasswordArgon2idOutput()
public function testEncodePasswordSodiumOutput()
{
if (!Argon2idPasswordEncoder::isSupported()) {
$this->markTestSkipped('Argon2id algorithm not available.');
if (!SodiumPasswordEncoder::isSupported()) {
$this->markTestSkipped('Libsodium is not available.');
}

$this->setupArgon2id();
$this->setupSodium();
$this->passwordEncoderCommandTester->execute([
'command' => 'security:encode-password',
'password' => 'p@ssw0rd',
'user-class' => 'Custom\Class\Argon2id\User',
'user-class' => 'Custom\Class\Sodium\User',
], ['interactive' => false]);

$this->assertNotContains(' Generated salt ', $this->passwordEncoderCommandTester->getDisplay());
Expand Down Expand Up @@ -298,10 +303,10 @@ private function setupArgon2i()
$this->passwordEncoderCommandTester = new CommandTester($passwordEncoderCommand);
}

private function setupArgon2id()
private function setupSodium()
{
putenv('COLUMNS='.(119 + \strlen(PHP_EOL)));
$kernel = $this->createKernel(['test_case' => 'PasswordEncode', 'root_config' => 'argon2id.yml']);
$kernel = $this->createKernel(['test_case' => 'PasswordEncode', 'root_config' => 'sodium.yml']);
$kernel->boot();

$application = new Application($kernel);
Expand Down
7 changes: 0 additions & 7 deletions 7 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordEncode/argon2id.yml
View file
Open in desktop

This file was deleted.

7 changes: 7 additions & 0 deletions 7 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordEncode/sodium.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
imports:
- { resource: config.yml }

security:
encoders:
Custom\Class\Sodium\User:
algorithm: sodium
4 changes: 1 addition & 3 deletions 4 src/Symfony/Component/Security/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ CHANGELOG
* Dispatch `AuthenticationFailureEvent` on `security.authentication.failure`
* Dispatch `InteractiveLoginEvent` on `security.interactive_login`
* Dispatch `SwitchUserEvent` on `security.switch_user`
* Added `Argon2idPasswordEncoder`
* Deprecated using `Argon2iPasswordEncoder` while only the `argon2id` algorithm
is supported, use `Argon2idPasswordEncoder` instead
* deprecated `Argon2iPasswordEncoder`, use `SodiumPasswordEncoder` instead
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we actually deprecate it ? It can use the native API of PHP 7.2+ (and Argon2idPasswordEncoder could as well, giving control over the variant). SodiumPasswordEncoder is only based on the sodium extension.

IMO, we should only deprecated the sodium-based fallback in the Argon encoders, not deprecate the encoders themselves.

Copy link
Member

@nicolas-grekas nicolas-grekas Apr 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having encoders bound to one algo is a bad practice. The documented best practice is to have a progressive encoder that knows how to verify old passwords while using the best available algo to encode new ones. So yes: this should be deprecated with this reasoning.
Of course, there is one missing step: rehashing. See #31139

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documented best practice is to have a progressive encoder that knows how to verify old passwords while using the best available algo to encode new ones.

The chain / delegating encoder should be composed of encoders using specific algorithms. At least, that's the case in Spring Security (see links above).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The chain / delegating encoder should be composed of encoders using specific algorithms

PHP is a glue language while Java implements everything itself. The situation in PHP is that password_hash()/Sodium\crypto_pwhash_str already embed the chain and there is zero need to expose the individual algos as encoders.

I agree a chain encoder would be nice to provide a migration path from Pbkdf2/MessageDigest/Plaintext to Native/Sodium thought. But we need #31139 first.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The situation in PHP is that password_hash()/Sodium\crypto_pwhash_str already embed the chain and there is zero need to expose the individual algos as encoders.

Having a default (like with password_hash) / fallback (like with sodium_crypto_pwhash_str) mechanism does not create a "chain" (to be understood as for the purpose of rehashing). Anyway, I don't understand that argument of PHP vs Java, when it's been pointed out that it's indeed possible to provide encoders for specific algorithms (with extra effort in the case of sodium_crypto_pwhash).


4.2.0
-----
Expand Down
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.