Problem hashing user passwords for storage

<p>I&#39;m trying to implement a way to securely store user passwords from a Go application. Following this cheat sheet (<a href="https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet" rel="nofollow">https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet</a>) I create a 64 byte long salt and use it to hash the password using the pbkdf2 algorithm (Argon2 doesn&#39;t seem to have a Go implementation yet). I then return the salt concatenated with the actual hash (again, following the cheat sheet).</p> <p>To test this I created a simple loop which creates a new salt in each iteration, creates the hash, and prints both to the console. Each iteration prints different salts and different hashes, which is what I would expect. </p> <p>However, I create a second hash from a different password using the same salt and compare both hashes. I would expect this comparison to fail (return false), but this isn&#39;t the case. It seems like the password doesn&#39;t matter. As long as the salts are the same the pbkdf2 function returns the same hash.</p> <p>I&#39;ve created a playground link which demonstrates this (copied the pbkdf2 algorithm from golang.org/x/crypto/pbkdf2 because it isn&#39;t available as an import in the playground). Can anyone spot my mistake?</p> <p><a href="https://play.golang.org/p/7YKbEKCKr3" rel="nofollow">https://play.golang.org/p/7YKbEKCKr3</a></p> <hr/>**评论：**<br/><br/>NikkoTheGreeko: <pre><p>Why not use bcrypt?</p> <pre><code>import ( &#34;golang.org/x/crypto/bcrypt&#34; ) func HashPass(pass []byte) ([]byte, error) { pass, err := bcrypt.GenerateFromPassword(pass, 10) if err != nil { return nil, err } return pass, nil } func CheckPassHash(hash, pass []byte) error { return bcrypt.CompareHashAndPassword(hash, pass) } </code></pre></pre>Crossi36: <pre><p>Didn&#39;t really look into it to be honest. The cheat sheet more or less lists it as kind of a last resort if the other methods aren&#39;t available. pbkdf2 was the first from that list (top to bottom) that was available in go without cgo. Will look into it tomorrow if I can&#39;t get pbkdf2 to work until then. I kind of want to know what I&#39;m doing wrong in my current implementation though.</p></pre>tcrypt: <pre><p>I&#39;m not sure why the OWASP recommendation appears to be PBKDF2 but I strongly recommend bcrypt. <a href="https://security.stackexchange.com/questions/4781/do-any-security-experts-recommend-bcrypt-for-password-storage" rel="nofollow">This guy</a> says it better than I could. Bcrypt has a number of advantages and has great support in Go.</p> <p>Edit: After looking at your code I would quadruple my recommendation. You should not implement you&#39;re own equality code for cryptography. The Bcrypt library handles that for you.</p></pre>circuitously: <pre><p>I encountered the same thing a year or so ago when looking at the NIST security guidelines, for our PHP app which has been using bcrypt for years. No mention of bcryt in the guidelines, but as with most of these sorts of things, if you can justify your decision to use something else not on the list, you&#39;ll be fine. </p></pre>Crossi36: <pre><p>Thank you for the hint on the equality check. It seems that the standard library has this covered though: <a href="https://golang.org/pkg/crypto/subtle/#ConstantTimeCompare" rel="nofollow">https://golang.org/pkg/crypto/subtle/#ConstantTimeCompare</a></p> <p>Actually, this is also used in the bcrypt implementation: <a href="https://github.com/golang/crypto/blob/master/bcrypt/bcrypt.go#L111" rel="nofollow">https://github.com/golang/crypto/blob/master/bcrypt/bcrypt.go#L111</a></p> <p>You&#39;re provided link was also a very interesting read!</p></pre>maddiez: <pre><p>I have used <code>bcrypt</code> in several of my small projects and it works wonderfully, it&#39;s very easy to use too. I would suggest that you use functions like these if you&#39;re not sure about password hashing etc.</p></pre>9nut: <pre><p>The &#39;key&#39; part of &#39;hash&#39; in overwritten in comparePassword. in comparePassword, hashPassword is called with a slice that is the first part of &#39;hash&#39;. this points to the storage that is appended to in hashPassword. after hashPassword returns, bytes.Equal is essentially comparing hash to itself. Compare your version to the following (only two lines are added)</p> <p><a href="https://play.golang.org/p/tAZtO7L6pm" rel="nofollow">https://play.golang.org/p/tAZtO7L6pm</a></p></pre>Crossi36: <pre><p>Thank you!</p></pre>KevBurnsJr: <pre><p>It appears that hashPassword is altering the value of hash on line 42 ensuring that they&#39;re always equal.</p> <p><a href="https://play.golang.org/p/jyC_jmkK_P" rel="nofollow">https://play.golang.org/p/jyC_jmkK_P</a></p></pre>Crossi36: <pre><p>You&#39;re right! 9nut gave a great explanation if you&#39;re interested.</p></pre>tscs37: <pre><p>Since most other problems are solved; do not use bytes.Equal to compare hashes, like ever, as a general rule you should be using a constant time compare to check if you compare hashes in a security context.</p></pre>luckyleprechaun98: <pre><p>I always use simple-scrypt which chooses sensible parameters for Colin Percival&#39;s algorithm. </p> <p><a href="https://github.com/elithrar/simple-scrypt" rel="nofollow">https://github.com/elithrar/simple-scrypt</a></p></pre>