defuse / password-hashing

Password hashing code.
BSD 2-Clause "Simplified" License
859 stars 222 forks source link

Use "verifier" instead of "hash" #39

Closed defuse closed 9 years ago

defuse commented 10 years ago

Using the word "hash" to refer to CreateHash's return value is a misnomer. It's not a hash. It's a bunch of different things, including the output of PBKDF2 which isn't a hash. We should standardize the codebase and documentation to use the term "verifier" for this string.

defuse commented 10 years ago

I'll do this today.

defuse commented 10 years ago

This is actually going to be a huge change, since it will require changing the public method names.

defuse commented 10 years ago

What about the terminology "Validate" as in "Validate Password". Should it be "Validator"? or "Verify Password?"

defuse commented 10 years ago

Validate: To establish the soundness of; corroborate.

Verify: To determine or test the truth or accuracy of, as by comparison, investigation, or reference:

defuse commented 10 years ago

The "as by comparison" is key. It sounds to me like validate is more internal and a process done on one thing without reference to anything external, i.e. "input validation", whereas passwords are verified against the verifier, so the verb "verify" is more suitable.

redragonx commented 10 years ago

The word verify sounds better

defuse commented 10 years ago

This also requires changing the file names.

defuse commented 10 years ago

I think the file names should be changed to PasswordStorage.*.

defuse commented 10 years ago

Grep turns up 364 hits for "hash". Fuck me.

defuse commented 10 years ago

The worst part is, we can't just replace "hash" with "verifier". It depends on the context. "Password Hashing" should become "Password Storage", and "The Hash" should become "The Verifier". Ugh.

defuse commented 10 years ago

I think this can be done in the shortest amount of time as follows:

defuse commented 10 years ago

Actually, it's probably a lot easier to tackle this one file at a time.

defuse commented 10 years ago

Got PasswordHash.php and PasswordHash.rb done.

sarciszewski commented 10 years ago

Hmm, this is tricky. Most people think "we need a strong hashing strategy", and the phrase "secure cryptographic storage" is almost never uttered outside of infosec certification exams.

We would need to check with @ircmaxell but I believe this is the reason PHP opted for password_hash() and password_verify() for the bcrypt wrapper nomenclature.

ircmaxell commented 10 years ago

Well, here's my view:

It is definitely a cryptographic hash. It fits every property of a a cryptographic hash function:

The difference comes in usage. Normal cryptographic hash functions are designed for speed when used in MAC operations. KDFs on the other hand are designed to be slow and used in dedication based usages.

Therefore all KDF functions are cryptographic hash functions. All hash functions are not KDFs, due to speed.

Now, the normal user (someone who hasn't studied crypto) won't know the difference. All they typically understand is "hashes are one way". And that's why I kept the hash name.

Could there be a more accurate name? Perhaps. Is it worth potentially confusing or mislrading the user base that would benefit most from this? That's the question...

My $0.02 at least...

Anthony

defuse commented 9 years ago

I disagree that what we're returning is a cryptographic hash, since it's actually the output of PBKDF2 encoded alongside the salt, iteration count, etc. At a lower level, PBKDF2 isn't a hash either because it's keyed, so it's really a PRF (or PRF family depending on your crypto terminology, both are valid) where the salt is what's used to select the random function.

That said, I do think keeping the terminology consistent with other libraries is important. So I think we should revert to using "hash" even though it isn't technically correct, in favor of ease-of-use. The only non-nit-picking argument for using "verifier" over "hash" is that the user might think what they're getting out is really a hash, with all of the properties of a cryptographic hash, and then use it in some stupid way like as a checksum for a cookie... but I don't think that's likely to happen.

So I'll revert to "hash."

defuse commented 9 years ago

Done in 133147f4dd

defuse commented 9 years ago

Oops, I forgot to stage everything before I commited, most of it is in here: 93e9016d819bde1