Skip to content

Bump default BCrypt strength to 12 #10447

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

Closed
wants to merge 3 commits into from

Conversation

larsgrefer
Copy link
Contributor

see #7411

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 28, 2021
@eleftherias eleftherias added in: crypto An issue in spring-security-crypto type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 28, 2021
@jzheaux jzheaux self-assigned this Nov 2, 2021
@jzheaux
Copy link
Contributor

jzheaux commented Nov 5, 2021

Thanks for the PR, @larsgrefer. Based on OWASP/CheatSheetSeries#601 it seems like OWASP continues to recommend 10 as the minimum work factor.

I lean towards sticking with OWASP's recommendations - why do you think it should be increased to 12?

@jzheaux jzheaux added the status: waiting-for-feedback We need additional information before we can continue label Nov 5, 2021
@larsgrefer
Copy link
Contributor Author

In my opinion, 10 is way too fast for any near-modern hardware. See #7411 (comment) for some numbers.

The Spring Security documentation itself states:

It is recommended that the "work factor" be tuned to take about 1 second to verify a password on your system.

With below 100ms, 10 is nearly two orders of magnitude too fast.

Also the current default of 10 was not adjusted since 8565116 10 years ago.

With the concerns you mentioned in #7411 (comment) in mind I'd recommend to bump it "only" to 12 for Spring Security 5.x and then bump it to 13 for 6.x

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 5, 2021
@Sc00bz
Copy link

Sc00bz commented Nov 11, 2021

Just had a look and you should be more worried about increasing the defaults for Argon2, scrypt, and PBKDF2. Since those don't even meet the bare minimum settings. Also there's a bug in PBKDF2's defaults.

Argon2's setting are m=4 MiB, t=3, p=1 either set it to 10 MiB or 7 iterations.
scrypt's settings are N=2^14, r=8, p=1 (16 MiB) either set N to 2^16 (64 MiB) or p to 4.
PBKDF2 settings are SHA1, 185k iterations, 256 bits of output. This should be changed to SHA512 and 120k iterations.
bcrypt setting is cost 10. Cost 9 is actually quite strong and is similar to PBKDF2-SHA512 with 230k iterations in strength.

Anyway those are just the bare minimums.

@Sc00bz
Copy link

Sc00bz commented Nov 11, 2021

PBKDF2's bug reported here: #10489

@jzheaux
Copy link
Contributor

jzheaux commented Jan 31, 2022

@larsgrefer, I appreciate the research you've done here. I'd like to adhere to the OWASP password recommendations since they are a single reliable source we can use for decision making. If 10 is too low, let's start with OWASP to get their recommendation increased to 12.

@jzheaux jzheaux closed this Jan 31, 2022
@jzheaux jzheaux added the status: declined A suggestion or change that we don't feel we should currently apply label Jan 31, 2022
@larsgrefer
Copy link
Contributor Author

@jzheaux OWASP explicitly asked Spring Security to use a strength of 12: OWASP/CheatSheetSeries#601 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: crypto An issue in spring-security-crypto status: declined A suggestion or change that we don't feel we should currently apply status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants