doc: correct pbkdf2 salt length recommendation

[willclarktech]

Corrects the recommended salt length recommendation in the documentation for crypto.pbkdf2 and crypto.pbkdf2Sync.

Checklist

• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

doc

[tniessen]

Technically correct.

[bnoordhuis]

LGTM, although it raises some questions:

1. Longer salts don't really affect PBKDF2's performance so why stop at at 16 bytes?
2. Shorter salts don't affect PBKDF2's security properties so why stop at 16 bytes?

(The salt thwarts precomputed dictionary attacks; even a two-byte salt goes a long way towards mitigating that.)

[tniessen]

Regardless of my understanding of the implications below, NIST is a reasonable reference and it should not be up to us to make any security recommendations.

---

Longer salts don't really affect PBKDF2's performance so why stop at at 16 bytes?

This is a minimum recommendation so longer values are always permitted, but as salts need to be stored along with their hash values, it would be inefficient to store needlessly long salts.

Shorter salts don't affect PBKDF2's security properties so why stop at 16 bytes? [...] even a two-byte salt goes a long way towards mitigating that

Shorter salts are suited to prevent dictionary attacks, but PBKDF2 can also be used to reduce the chance of key reuse, which heavily relies on the salt length. For example, 100 operations using the same password and a two-byte salt would have a collision probability of 7%, and after 400 more operations, there is an 85% chance of having reused the same key.

[bnoordhuis]

Regardless of my understanding of the implications below, NIST is a reasonable reference and it should not be up to us to make any security recommendations.

Oh, I'm not disagreeing but if you look at e.g. OWASP's recommendations, it says to use a 32 byte or even 64 byte salt. (And if you look elsewhere, you can find recommendations of as little as 4 or 8 bytes. So many options to chose from...)

[Trott]

CI: https://ci.nodejs.org/job/node-test-pull-request-lite/6/

[BridgeAR]

I guess the question here is what we personally feel like what is right to recommend. So do we want to have the current recommendation to be the same as NIST or recommend a higher value as currently?

[tniessen]

As long as we reference the NIST SP, this pull request is correct, so I am going to go ahead and land this.

Apart from that, we should not make any quantitative security recommendations apart from those specified by appropriate institutions, unless they are specific to node. As @bnoordhuis pointed out, other recommendations suggest using different salt lengths, which is a fact we could mention at this point as part of the documentation.

[tniessen]

Landed in 560c182, thank you for your first contribution! 🎉