silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
721 stars 822 forks source link

Implement scrypt and/or argon2 password hasher and/or increase work factor for blowfish hasher #10949

Open GuySartorelli opened 11 months ago

GuySartorelli commented 11 months ago

According to owasp, bcrypt (aka blowfish) is recommended for "legacy systems". Furthermore, their minimum recommended work factor is 10, which is what our blowfish encryptor's default work factor is set at. Basically, out of the box we're providing the bare minimum security for password hashing according to owasp - which is fine for now, but as time goes on we'll want to at the very least bump up that work factor, if not implement a more modern hasher.

Their recommendation is to use argon2id if available, and to use scrypt otherwise. Having given a decent look around online, I have found the following:

Argon2 and scrypt are both more memory intensive than bcrypt by design, so that attackers require more specialised and expensive equipment to attack them - it makes them more resistant to attacks run on the GPU, which bcrypt is somewhat resistant to, but to a much lesser degree than these algorithms. Argon2 is reportedly more of a resource hog than scrypt and may not be suitable for sites with high traffic. For this reason I'm leaning more towards scrypt.

This graph from the scrypt paper [PDF] shows a comparison of scrypt vs bcrypt and PBKDF2: image

Options

  1. Implement argon2 hasher
  2. Implement bcrypt hasher
  3. Increase work factor for blowfish hasher
  4. Any combination of the above
  5. Leave as is for now
  6. https://github.com/silverstripe/silverstripe-framework/issues/8526#issuecomment-435757433

Additional considerations

kinglozzer commented 11 months ago

I remember this coming up a while back, this may be good for some inspiration, particularly with re-hashing passwords: https://github.com/silverstripe/silverstripe-framework/pull/8806. We never actually implemented support for PHP’s native password_hash() function, it looks like that could allow us to make argon2 (and cost for bcrypt) an option that’s configurable

GuySartorelli commented 11 months ago

Ooooh nice. https://github.com/silverstripe/silverstripe-framework/issues/8526 was also linked from that PR which gives some good background and inspiration for approaches as well.