OWASP / ASVS

Application Security Verification Standard
Creative Commons Attribution Share Alike 4.0 International
2.7k stars 658 forks source link

2.4.3 is weak #1003

Closed jmanico closed 3 years ago

jmanico commented 3 years ago

If FIPS-140 compliance is required, use PBKDF2 with a work factor of 310,000 or more and set with an internal hash function of HMAC-SHA-256.

https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html

Sjord commented 3 years ago

2.4.3 Verify that if PBKDF2 is used, the iteration count SHOULD be as large as verification server performance will allow, typically at least 100,000 iterations.

pbkdf2 time
100000 25 ms
200000 50 ms
300000 75 ms
310000 76 ms

@jmanico, do you have more information on what 310,000 was based on?

jmanico commented 3 years ago

This is from one of the hashcat team leads who helped write the recent update to:

https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html

-- Jim Manico

On Jun 3, 2021, at 3:54 AM, Sjoerd Langkemper @.***> wrote:

 2.4.3 Verify that if PBKDF2 is used, the iteration count SHOULD be as large as verification server performance will allow, typically at least 100,000 iterations.

pbkdf2 time 100000 25 ms 200000 50 ms 300000 75 ms 310000 76 ms @jmanico, do you have more information on what 310,000 was based on?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

jmanico commented 3 years ago

can we round up to 400k or more?

cmlh commented 3 years ago

I would prefer not to exclude older hardware of legacy apps since SP 800-132 Recommendation for Password-Based Key Derivation: Part 1: Storage Applications does not specify the work factor for PBKDF2 but can still be verified (i.e. the work factor).

I'd support a fork of ASVS adhering to the Password Storage Cheat Sheet too.

jmanico commented 3 years ago

The change is already live and consistent with the password storage cheetsheet so closing out for now

cmlh commented 3 years ago

The change is already live and consistent with the password storage cheetsheet so closing out for now

I believe this change needs additional approval from other Project Leaders otherwise it appears to be a conflict of interest and is also not aligned to SP 800-132.

Sjord commented 3 years ago

Can you be more specific as to how the ASVS conflicts with SP 800-132? Jim said

internal hash function of HMAC-SHA-256.

and SP 800-132 says:

This Recommendation approves PBKDF2 as the PBKDF using HMAC with any approved hash function as the PRF.

So this doesn't seem to conflict.

On the work factor, SP 800-132 says:

A minimum iteration count of 1,000 is recommended.

But this is really a theoretical minimum and they encourage you to increase this so the hashing takes several seconds. So I think the ASVS and SP 800-132 do not necessarily disagree. What do you think, @cmlh?

jmanico commented 3 years ago

I worked with the hashcat team and math to come up these recommendations and I stand by them. Most standards that cite how to use PBKDF2 are wrong and I’m standing by this rather extensive work to come up with these values. Thanks.

Sjord commented 3 years ago

Do you have notes of your meeting with the hashcat team? Can we view the math you used somewhere?

cmlh commented 3 years ago

My core issue by excluding legacy hardware that can only support the "...minimum iteration count of 1,000" then these well intended changes can have unintended consequences similar to that of https://github.com/pyca/cryptography/issues/5771

I would have preferred we socialized the changes upstream with NIST against SP 800-132 Recommendation for Password-Based Key Derivation: Part 1: Storage Applications [if we haven't already] and then merge the ASVS Fork of the recently updated Password Storage Cheat Sheet.