proposal: crypto/pbkdf2: add package

[qmuntal]

Proposal Details

I propose to move the golang.org/x/crypto/pbkdf2 package into the standard library with the name crypto/pbkdf2. This proposal is a #65269 spin off, as requested by the following comment from @rsc:

I agree with this in principle but there should be review of individual packages rather than a blank check to usher significant amounts of never-reviewed API into the standard library. https://github.com/golang/go/issues/65269#issuecomment-1935112912

pbkdf2 is a low-hanging fruit here as it only exports a single function. I propose to move it to the standard library as is, just adding an error return parameter in case we want to implement some additional parameter validation in the future (e.g. minimum salt length or minimum iterations). The proposed API looks like this:

// Key derives a key from the password, salt and iteration count,
// returning a []byte of length keyLen that can be used as cryptographic key.
// The key is derived based on the method described as PBKDF2 with the
// HMAC variant using the supplied hash function.
func Key(password, salt []byte, iter, keyLen int, h func() hash.Hash) ([]byte, error)

The pbkdf2.Key function is enough to cover most pbkdf2 use cases. As a data point, OpenSSL exports the PKCS5_PBKDF2_HMAC function, accepting the same inputs as pbkdf2.Key.

@golang/security

[gabyhelp]

Related Issues and Documentation

• proposal: crypto/hkdf: add package #61477

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[ericlagergren]

My one hangup about this is that new projects shouldn't be using PBKDF2 without a good reason. There are significantly better password-based KDFs, like Argon2 or even scrypt.

IMO, it's misleading to move this into the stdlib without updating the docs to point people to better algorithms.

[qmuntal]

My one hangup about this is that new projects shouldn't be using PBKDF2 without a good reason. There are significantly better password-based KDFs, like Argon2 or even scrypt.

Reasons aside, pbkdf2 is much more popular that scrypt and argon2, at least if we look the number of times the corresponding golang.org/x/crypto package is imported. As of writing this, pbkdf2 is imported by 7081 modules, scrypt 3227 modules and argon2 1559 modules.

Having said this, it is fine for me to add any doc explaining better alternatives with the corresponding reasoning.

[rolandshoemaker]

We've had higher level discussions of providing a more opaque password hashing API, wherein you just have a password you want to hash, and we return what we consider a secure hash (and also implement the verification side to transparently support multiple hashes, so we can easily rotate what we return). I think that is the long term solution to these kinds of problems where people are still using an older, less ideal method simply because they don't know they should update.

In the short-term adding pbkdf2 to the standard library seems mostly fine to me. In the long term we'll probably want to continue to provide these primitives anyway, since people will continue to need them for backwards and third-party compatibility.

I think the other option is to leave it in x/crypto as one of the "frozen" libraries we don't expect to see any further development. There are currently no other open issues for the library (and only 2 closed historical issues), so perhaps that would be reasonable if we don't expect any new API additions, and don't have any plans to make any other substantial changes to the package.

If we do add it, we should definitely add a note to the documentation though pointing out you should only be using this for compat reasons, and pointing to the better choices.

cc @FiloSottile

[qmuntal]

Thanks for the insights @rolandshoemaker.

I think the other option is to leave it in x/crypto as one of the "frozen" libraries we don't expect to see any further development. There are currently no other open issues for the library (and only 2 closed historical issues), so perhaps that would be reasonable if we don't expect any new API additions, and don't have any plans to make any other substantial changes to the package.

In the issue description I mentioned a small API change that could make pbkdf2 a little-bit harder to misuse: adding an error return value so we can validate the parameters and fail if they are not "secure" enough. I'm specifically thinking about implementing NIST SP 800-132 recommendations:

• Minimum iteration count of 1000.
• The length of the randomly-generated portion of the salt shall be at least 128 bits.
• The key length shall be at least 112 bits in length.

[rolandshoemaker]

Oh sorry, I somehow completely glossed over this while reading through this. In that case I think it's reasonable to move into the standard library with these specific changes.