Skip to content

[Security] Allow to stick to a specific password hashing algorithm #34020

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

Jump to bottom
Merged
merged 1 commit into from
Oct 27, 2019
Merged

[Security] Allow to stick to a specific password hashing algorithm #34020

merged 1 commit into from
Oct 27, 2019

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Oct 18, 2019

Q A
Branch? 4.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #33054
License MIT
Doc PR todo

Allows using argon2i, argon2id and bcrypt.

@chalasr chalasr added this to the next milestone Oct 18, 2019
@chalasr chalasr force-pushed the sec-fixed-algo branch 2 times, most recently from 9fe4347 to d18baac Compare October 18, 2019 08:08
@stof
Copy link
Member

stof commented Oct 18, 2019

Some tests are failing

@chalasr chalasr force-pushed the sec-fixed-algo branch 3 times, most recently from 6b03279 to 7f049a1 Compare October 19, 2019 01:15
@chalasr
Copy link
Member Author

chalasr commented Oct 19, 2019

Got it, the PASSWORD_* constants will be strings as of PHP 7.4, not integers (https://wiki.php.net/rfc/password_registry).
Now green.

nicolas-grekas
nicolas-grekas reviewed Oct 19, 2019
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

How can one configure a migrating encoder that uses bcrypt as the main one? I think that should be the main supported use case here.

@chalasr chalasr force-pushed the sec-fixed-algo branch 12 times, most recently from 3e7cef8 to 3d3f2d4 Compare October 27, 2019 07:05
@chalasr chalasr changed the title [Security] Allow to stick to a specific password hashing algorithm [Security] Allow to stick/migrate to a specific password hashing algorithm Oct 27, 2019
@chalasr
Copy link
Member Author

chalasr commented Oct 27, 2019

@nicolas-grekas Now possible via a new encoder option:

encoders:
    App\User:
        algorithm: argon2id
        migrating_from: bcrypt # or ['bcrypt, 'argon2i']

Comments addressed also, thanks.

@chalasr chalasr mentioned this pull request Oct 27, 2019
Merged
@chalasr chalasr force-pushed the sec-fixed-algo branch 2 times, most recently from ce7a6b7 to 0641859 Compare October 27, 2019 10:10
@chalasr chalasr mentioned this pull request Oct 27, 2019
Merged
@chalasr chalasr changed the title [Security] Allow to stick/migrate to a specific password hashing algorithm [Security] Allow to stick to a specific password hashing algorithm Oct 27, 2019
@chalasr
Copy link
Member Author

chalasr commented Oct 27, 2019

Migrations related part moved to #34139

@chalasr chalasr force-pushed the sec-fixed-algo branch 3 times, most recently from 2cdadfe to 6f12f71 Compare October 27, 2019 10:41
@nicolas-grekas
Copy link
Member

Thank you @chalasr.

nicolas-grekas added a commit that referenced this pull request Oct 27, 2019
@nicolas-grekas
feature #34020 [Security] Allow to stick to a specific password hashi…
b5a47df 
…ng algorithm (chalasr)

This PR was merged into the 4.4 branch.

Discussion
----------

[Security] Allow to stick to a specific password hashing algorithm

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #33054
| License       | MIT
| Doc PR        | todo

Allows using `argon2i`, `argon2id` and `bcrypt`.

Commits
-------

6712d1e [Security] Allow to set a fixed algorithm
@nicolas-grekas nicolas-grekas merged commit 6712d1e into symfony:4.4 Oct 27, 2019
@nicolas-grekas nicolas-grekas mentioned this pull request Oct 27, 2019
Closed
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
This was referenced Nov 12, 2019
Merged
Merged
@chalasr chalasr deleted the sec-fixed-algo branch April 14, 2020 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@darielopper darielopper darielopper approved these changes

Assignees
No one assigned
Labels
Feature Security SecurityBundle Status: Reviewed
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.

[RFC] Give back control of password hashing algorithm to app developer
5 participants
@chalasr @stof @nicolas-grekas @darielopper @carsonbot