build-web-application-with-golang icon indicating copy to clipboard operation
build-web-application-with-golang copied to clipboard

Replace all MD5 Password Hashing w/ bcrypt

Open elithrar opened this issue 11 years ago • 7 comments

MD5 hashes are easily brute forced (or bypassed with a rainbow table) - especially so when unsalted.

I strongly suggest changing your Custom Authentication example to use Go's bcrypt package, which is both simple to use and extremely secure.

The login code would therefore become:

err := bcrypt.CompareHashAndPassword(userInfo.Password, []byte(password))
if err != nil {
    this.Data["PasswordErr"] = "Password error, please try again"
    return
}

... and in the registration process:


// After checkPassword(password)

hash, err := bcrypt.GenerateFromPassword([]byte(password))
if err != nil {
    this.Data["PasswordErr"] = "Password error, please try again"
    return
}

...

users.Password = hash

For further reading: http://yorickpeterse.com/articles/use-bcrypt-fool/

elithrar avatar Aug 15 '14 00:08 elithrar

Password storage

  • Common solution
  • Advanced solution
  • Professional solution

https://github.com/astaxie/build-web-application-with-golang/blob/master/en/eBook/09.5.md

astaxie avatar Aug 15 '14 01:08 astaxie

Most of the solutions there don't solve the problem:

  • Salted MD5 is still a very, very bad choice (esp. using application-wide salts)
  • Salted SHA-1 or SHA-2 (i.e. SHA-256) are only slightly better, but still bad
  • You mention scrypt in passing only
  • The example code you are providing in a tutorial for new Go developers is severely insecure, and sets a bad standard.

Your examples should be realistic - writing about password storage and then going on (in a later chapter!) to use an insecure method is harmful to new developers.

On Fri, Aug 15, 2014 at 9:10 AM, astaxie [email protected] wrote:

https://github.com/astaxie/build-web-application-with-golang/blob/master/en/eBook/09.5.md

— Reply to this email directly or view it on GitHub https://github.com/astaxie/build-web-application-with-golang/issues/354#issuecomment-52264507 .

elithrar avatar Aug 15 '14 01:08 elithrar

Further - https://github.com/astaxie/build-web-application-with-golang/blob/master/en/eBook/09.6.md#base64-encryption-and-decryption

  • is not encryption and provides no data security at all.

base64 is an encoding type, not an encryption method. There are no secrets, no keys. I would remove the base64 section from that chapter entirely.

On Fri, Aug 15, 2014 at 9:16 AM, Matt S [email protected] wrote:

Most of the solutions there don't solve the problem:

  • Salted MD5 is still a very, very bad choice (esp. using application-wide salts)
  • Salted SHA-1 or SHA-2 (i.e. SHA-256) are only slightly better, but still bad
  • You mention scrypt in passing only
  • The example code you are providing in a tutorial for new Go developers is severely insecure, and sets a bad standard.

Your examples should be realistic - writing about password storage and then going on (in a later chapter!) to use an insecure method is harmful to new developers.

On Fri, Aug 15, 2014 at 9:10 AM, astaxie [email protected] wrote:

https://github.com/astaxie/build-web-application-with-golang/blob/master/en/eBook/09.5.md

— Reply to this email directly or view it on GitHub https://github.com/astaxie/build-web-application-with-golang/issues/354#issuecomment-52264507 .

elithrar avatar Aug 15 '14 01:08 elithrar

Your examples should be realistic

yep, I should improve these code.

base64 is an encoding type, not an encryption method.

I have mentioned at start:

If the Web application is simple enough, data security is no less stringent requirements, then you can use a relatively simple method of encryption and decryption is base64

astaxie avatar Aug 15 '14 01:08 astaxie

No. You have said encryption. base64 does not encrypt anything at all and is not a "relatively simple method of encryption".

Base64 (http://en.wikipedia.org/wiki/Base64) is just a way to encode data within a defined set of characters. There's no security - not even "simple" security. Delete that entire section from the document.

On Fri, Aug 15, 2014 at 9:23 AM, astaxie [email protected] wrote:

Your examples should be realistic

yep, I should improve these code.

base64 is an encoding type, not an encryption method.

I have mentioned at start:

If the Web application is simple enough, data security is no less stringent requirements, then you can use a relatively simple method of encryption and decryption is base64

— Reply to this email directly or view it on GitHub https://github.com/astaxie/build-web-application-with-golang/issues/354#issuecomment-52265211 .

elithrar avatar Aug 15 '14 01:08 elithrar

You're teaching people the wrong thing. MD5 should never be used for anything security-related, even in "small" programs. Do it correctly and use something like bcrypt.

Your claim that base64 is encryption/decryption is also plain wrong. There's no debate about this. Base64 is not a form of cryptography in any way whatsoever.

Here's some material you can read to learn more about the subject:

  • https://en.wikipedia.org/wiki/MD5
  • https://en.wikipedia.org/wiki/Cryptography
  • https://en.wikipedia.org/wiki/Base64

veegee avatar Mar 28 '16 07:03 veegee

This was resolved in https://github.com/astaxie/build-web-application-with-golang/pull/824... 7 years ago 😅

mubarak-j avatar Jan 14 '24 00:01 mubarak-j