search

brockallen / BrockAllen.MembershipReboot

MembershipReboot is a user identity management and authentication library.
Other
742 stars 238 forks source link

question: Should two users with the same password have the same hash? #603

Closed looking-promising closed 8 years ago

looking-promising commented 8 years ago

I'm not completely clear on the password hashing algorithm. I know @brockallen has written about it in his blog but w/ my primitive understanding of the hashing crypto, I don't understand 2 things:

  1. Why does it appear that two users with the same password have the same password hash. I thought the hashing algorithm (Rfc2898DeriveBytes/PBKDF2?) produced a unique password salt each time which should ensure this never happens. Do I have something misconfigured?
  2. If I change the number of hashing iterations in the Membership Reboot config, will that invalidate all the existing passwords? If the password iteration count and password salt are not stored and associated with the user, how is there ever enough information to re-hash the user's password for comparison during login/authentication time?

I'm happy to do additional reading on the subject if anyone has link or book suggestions.

Thanks.

nicholas-brooks commented 8 years ago

I've been looking at this code today, so thought I'd take stab at answering. Disclaimer - newbie to membershipreboot and to security in general.

  1. This doesn't answer your question directly, but from what I can see in BrockAllen.MembershipReboot/Crypto/System.Web.Helpers.Crypto.cs the HashPassword method returns a base64 encoded byte array of the salt + the calculated hash. If you just looked at the first bit of the hashed value in the database, it might appear they are the same however.. see point 2 as to why.
  2. No it shouldn't as the iteration count used originally is stored with the hashed value. If you look at HashPassword in BrockAllen.MembershipReboot/Crypto/DefaultCrypto.cs you can see that the string returned (that ends up being persisted) is the iterationCount + PasswordHashingIterationCountSeparator + Hashed Password (salt + hash).

As for suggested reading, me too!

brockallen commented 8 years ago

what @nicholas-brooks said :)

and to clarify -- each hashed password should be different because of the salt.

looking-promising commented 8 years ago

Thanks @nicholas-brooks. Your explanation makes perfect sense (and is blatantly obvious in retrospect given the referenced code) and clarifies both of my questions. I'm sure that when I visually compared two different hashes for the same password, they looked the same when in fact they were not.