Skip to content

Consider changing default encoder in PasswordEncoderFactories #16879

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.

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

Open
jgrandja opened this issue Apr 4, 2025 · 3 comments
Open

Consider changing default encoder in PasswordEncoderFactories #16879

jgrandja opened this issue Apr 4, 2025 · 3 comments
Labels
for: team-attention This ticket should be discussed as a team before proceeding type: enhancement A general enhancement
Milestone

Comments

@jgrandja
Copy link
Contributor

jgrandja commented Apr 4, 2025

The default PasswordEncoder in PasswordEncoderFactories is BCryptPasswordEncoder.

We should consider changing the default to another PasswordEncoder based on the recommendations in OWASP Password Storage Cheat Sheet.

@jgrandja jgrandja added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 4, 2025
@jgrandja jgrandja added this to the 7.0.x milestone Apr 4, 2025
@evgeniycheban
Copy link
Contributor

Hi, @jgrandja I can work on it.

@evgeniycheban
Copy link
Contributor

@jgrandja according to the link you provided we should use Argon2 by default instead of bcrypt, I see that currently we have 2 factory methods that produce Argon2PasswordEncoder for spring security 5.2 and 5.8 with different configuration to keep backward compatibility with passwords that were encoded in previous versions of spring security, so I would add a new factory-method defaultsForSpringSecurity_v7 that returns Argon2PasswordEncoder with recommended configuration of 19 MiB of memory, an iteration count of 2, and 1 degree of parallelism as suggested in the link. Then in the method PasswordEncoderFactories#createDelegatingPasswordEncoder we can add it to encoders map like argon2@Spring_Security_v7 and switch default encoder to argon2@Spring_Security_v7 as well.

@jgrandja jgrandja added the for: team-attention This ticket should be discussed as a team before proceeding label Apr 16, 2025
@jgrandja
Copy link
Contributor Author

Thanks again @evgeniycheban.

I added the for:team-attention label to this ticket as well as gh-16880 as the team needs to discuss this before we proceed with any changes. I will get back to you as soon as we decide on the approach. Thanks for your patience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: team-attention This ticket should be discussed as a team before proceeding type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants