SEC-1932 Add Pbkdf2PasswordEncoder

[rworsnop]

• Also moved some logic into a new class, AbstractPasswordEncoder. Both PBKDF2PasswordEncoder and the now-simplified StandardPasswordEncoder extend AbstractPasswordEncoder.
• Added tests for PBKDF2PasswordEncoder .

I have signed and agree to the terms of the SpringSource Individual Contributor License Agreement.

Fixes gh-2158

[snhrdt]

This shows: You should never do two tasks at once (at least that's true for me ...).

Somehow, I mixed up StandardPE and NoOpPE - silly me. Sorry for the unfounded comment.

Regarding BCryptPasswordEncoder: If we have a new AbstractPasswordEncoder, I think that the expectation is that all concrete PEs extend that; in my last comment, I did not mean that BCryptPE does extend AbstractPE, but that it would be nice to somehow unify all PEs to use the same AbstractPE.

Be that as it may: Great that PBKDF2 is now available in spring-security.

I will remove my previous comment as it is clearly in the wrong.

[rwinch]

Thanks for the Pull Request @rworsnop and sorry for the delays in merging this! I'm trying to get caught up on PRs now and have made it a personal goal to do much better in the future.

I'm looking over the pull request and wanted to get some discussion going around some of the defaults we should use.

• Iterations: Why 1024 default iterations? While I haven't seen any concrete recommendations for number of iterations I believe a number a bit larger may be a better default since in 2010 the minimum was 1,000. Big names like LastPass & Apple appear to be using 10K which is something I think sounds more appropriate.
• Salt length: The current salt length is 8 bytes (64 bits) It seems that RFC 2898 minimum salt size is 64 bits, but I'm thinking a more appropriate default may be the NIST recommendation of at least 128 bits (see Section 5.1 The Salt (S))
• dkLen / hashWidth / keyLength - It seems this goes by a number of names depending if you look at this source, the specification, or the Javadoc on PBEKeySpec. However, I'm wondering where the default of 160 came from. I haven't found any concrete recommendations for this. I do see that WPA2 uses 256, but I'm not sure if this is a good choice or not.

If anyone has any thoughts or references to credible resources on what the default values should be please provide them.

Thanks again @rworsnop for your efforts and sorry for the delays!

[rworsnop]

Thanks for picking this up. It's been a while since I did this, so my memory of where these numbers came from is not completely clear!

• Iterations: I agree 1024 is a little low, but isn't the default in LastPass 5000? I suppose any bigger number is good as long as it doesn't excessively load a typical 2014 server CPU (I've seen 8 ms recommended).
• Salt length: Following the NIST recommendation seems wise.
• dkLen: 160 was almost certainly due to a misunderstanding of how the algorithm works.

[rwinch]

In terms of iterations, I have also seen 8ms as a recommendation, but I do believe this is one of those things that is a case by case basis making a default a difficult choice. Also I need to correct myself on LastPass. They seem to use 100K iterations see http://blog.lastpass.com/2011/05/lastpass-security-notification.html Does 100K seem excessive?

I'm still a bit unsure about the dkLen defaults at this point.

Also worth mentioning is that it would be ideal if we could use PBKDF2 SHA-256, but Java doesn't support it.

I'm going to let this sit for a few days in hopes that a few people I have pinged will have some sort of additional insights.

PS: I think eventually we probably need a mechanism that will allow better flexibility in supporting multiple algorithms / rounds / etc and automatic migrations. But this is certainly out of the scope for this specific ticket.

[rworsnop]

Ah, I was confusing LastPass's client-side encryption (which does default to 5K rounds) with what they do server-side for authentication.

Based on some tests I just did, 100K does seem excessive for a default.
On my laptop, using dkLen=256, it's averaging 240 ms for 100K rounds. (Dropping back to dkLen=160 roughly halves the time.)

By way of comparison, I ran the same test with a default BCryptPasswordEncoder and it averaged 84 ms. I then tried PBKDF2PasswordEncoder with 10K rounds (dkLen=256) and it averaged 83 ms. Spooky.

I don't really know what the most sensible default for dkLen is; but keep in mind that WPA2 uses PBKDF2 to convert a text password to a key for use with AES-256; so that's why it uses dkLen=256.

[rwinch]

Thanks for your additional efforts @rworsnop! I have a colleague that is well versed in such things and has agreed to give feedback later today. At that point I think we can collect all the information and get some defaults, document some of the parameters a bit better (i.e. why the defaults were chosen) and merge the request. Thanks again for your efforts!

[rwinch]

@rworsnop I just wanted to let you know that finding good defaults has been a bit challenging. With the help of @abedra we were able to recruit jOHN Steven to help figure out reasonable defaults. He is a bit busy this week, but should be able to provide feedback next week. This will be plenty early for the first milestone release.

[m1spl4c3ds0ul]

All,

Thanks for the patience, ‘life’s been wild recently. Find my notes from a conversation with Rob below. Discussion considers 4.0.0.M1 from a perspective of “refactor” or “update” rather than “bug fix”. The objective is to secure password storage starting from the existing code mindful that it has to support a wide range of adopters that don’t want to devote time to thinking about it ;-)

[Choice of Password Encoder]
I recommend the following as essential to achieve a minimum acceptable level of security in the event of a stolen password database:

*ACTION 01: Use an adaptive-one way function for the *DEFAULT PW storage scheme;

It appears as though 4.0.0 uses a fork of the mind rot implementation of Bcrypt: a fine choice. It’s default work factor (c=10) is also a good choice—it places password verify time around or above 0.5 seconds real time.

Consider adding configuration for use of PBKDF2 as an alternative encoder IF adopters require usage of an implementation from the default JCE crypto provider—an issue I’ve encountered a few times. Note, however, that PBKDF2 allows the attacker to leverage GPU-based optimizations for brute force attacks that Bcrypt does not. I’ll provide more information if desired.

Elsewhere, I suggest alternative approaches to adaptive one-way functions for good security and performance reasons. Here, again, I suggest the aforementioned approach.

**ACTION 02: REMOVE or provide salted digest encodings on a DEPRECATED basis only.

This advice should be applied to the salted SHA2 iterated 1024 times as well. Effectively, this scheme and its ilk provide no appreciable protection. More subtlety, iteration of a hash, such as SHAx, allows for attacks that iterating HMACs, due to their construction, does not. This is one reason why PBKDF2 uses HMACSHA rather than SHA.

[Work Factors]
A note on work factors: Work factors should be tuned for real time, rather than to a particular value (i.e. c=10,000). If another adaptive one-way function (PBKDF2, Scrypt) are chosen, tune work factor based on experienced real time.

Historically, the crypt function used for Unix passwords was tuned to take 1 second on hardware of the time. Security consultants have posted 0.08, 0.10, 0.2, and 0.5 seconds for balance between responsiveness and security.

My recommendation is 0.5 seconds for those where that response time is deemed acceptable.

[Salt Length]
Salts prevent two identical credentials from possessing the same value in their stored form. Because users choose common passwords, salts raise the difficultly of lookup-based attacks.

**ACTION 03: Use a cryptographically-strong (pseudo-) randomly generated salt for each credential being stored.

Yes, using a 128b salt is a good choice. Adopters sometimes complain that long salts affect performance or storage unacceptably, though I’ve not seen evidence of this. An minimum size of 64b prevents lookup-based attacks.

[Key Length]
Key-derivation functions, such as PBKDF2 support a parameter governing output key length. This allows callers to derive the appropriate size key for whatever their ultimate purpose is. For instance, a caller using PBKDF2 to derive an AES key could specify a 128, 192, or 256b length. Derived key length does not affect the security of PW encoding schemes as brute force and lookup-based attacks are constrained by the size of the input (not output) space. As such, the default output for PBKDF2 suffices.

[Planning for Change]
Selecting a PW storage design based on adaptive one-way functions implies that the work factor will change over the lifetime of the deployed system. The following actions facilitate this change:

**ACTION 04: Append metadata to credentials’ stored form indicative of the digesting algorithm, any work factor information, and the salt.

Bcrypt’s output format already provides much of this information, taking the form:

$<ver>$<iter count>$<salt>$<digest> 

Because existing stable versions of SpringSecurity, and deployed PW databases, use an insecure salted digest scheme, I recommend prepending a scheme identifier to the Bcrypt format. An adapter should apply this format to other password encoding scheme’s output.

Remember, the output of digests, and adaptive one-way functions are irreversible. Long inactive users may thus be one (or more) encoding schemes behind the deployment’s present scheme. Additionally, as has been suggested in forum discussions, deployers may want to use more onerous work factors for more privileged accounts.

**ACTION 05: Update verify implementation to read prepended scheme identifier and route the stored credential to the right verification routine, stripping unexpected metadata as appropriate for the verification algorithm.

[Harder stuff]

1. Creating a cryptographically-strong random salt is challenging. Some security practitioners demand blocking sources of randomness, whereas others suggest use of a non-blocking source that leverages a cryptographic transform prior to emission (such as something based on SHA1). For the purposes of salt generation, I recommend a non-blocking source for a variety of reasons but care must be taken to reseed appropriately.

2. Remember that tuning work factor to server performance may not reflect the speed at which an attacker can make PW guesses. Depending on the threat agent, and chosen PW encoding, attackers may be using optimized cracking software, perhaps even executed on custom GPU-/FPGA-based hardware. Regardless of chosen scheme, Bcrypt included, I expect an attacker to enjoy at least an order of magnitude speed advantage over the defender.

3. In case of attack:
   a. Use of a one-way function means there’s no way to conduct offline update of users’ passwords;
   b. Adding work factor (e.g. migrating from c=10 to c=15) or wrapping compromised stored credentials in additional digesting rounds does nothing to prevent an attacker from using credentials they’ve reversed on the site (or others sharing those users’ credentials)

4. There’s plenty of real-world evidence of adaptive one-way functions hurting defender performance in practice. The very properties designed to frustrate attackers impact defender memory footprint, CPU load, and cache state. It is for this reason, amongst others, that I’ve recommended alternative solutions elsewhere.

[rwinch]

@JSteven Thanks for your response and continued collaboration on this. I updated the "Password Modernization" JIRA based upon additional details I picked up. One thing I'd like to follow up on is:

In the comment on github you state:

For the purposes of salt generation, I recommend a non-blocking source for a variety of reasons but care must be taken to reseed appropriately.

Is there a way to guarantee a non-blocking source with Java? My understanding is that the SecureRandom implementations are vendor specific and may/may not be blocking.

[michalklempa]

Hi, this is a very nice discussion, and I learned a lot from it. But the problem is still unresolved and I had to deploy PBKDF2 into production. So until this feature is approved, let me kindly redirect people who need this, to my how to add pbkdf2 to spring security i wrote.

[artgo]

+1 for the feature. Branch has conflicts.