Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use bcrypt based password hash #1977

Closed
ghost opened this issue Oct 11, 2013 · 15 comments
Closed

Use bcrypt based password hash #1977

ghost opened this issue Oct 11, 2013 · 15 comments

Comments

@ghost
Copy link

ghost commented Oct 11, 2013

Created by Cauan Cabral, 12th Jul 2012. (originally Lighthouse ticket #3032):


Quoting Nikita post (http://nikic.github.com/2012/07/10/What-PHP-5-5-might-look-like.html):

"The recent password leaks (from LinkedIn etc) have shown that even large websites don’t get how to properly hash passwords."

Well, CakePHP as all major php frameworks, has provided a simple sha1/md5 plus a salt to store passwords and authenticate users.

I think the new branch 3.0 is fine to change that.

What do you think guys?

Another good source: https://wiki.php.net/rfc/password_hash

@ghost
Copy link
Author

ghost commented Oct 11, 2013

12th Jul 2012, Jose Lorenzo Rodríguez said:


I think 3.0 is a good opportunity to change this

@ghost
Copy link
Author

ghost commented Oct 11, 2013

13th Jul 2012, Mark Story said:


I agree, we should continue offering the existing sha1 option though to make upgrading doable.

@ghost
Copy link
Author

ghost commented Oct 11, 2013

23rd Jul 2012, Mark Story said:


Bcrypt support has been added for 2.3. It is not the default yet, but its in there if people want to use it now. I think that making bcrypt the default in 3.0 makes sense. There should certainly be an example on how people can integrate bcrypt for new passwords and allow old passwords to stay as sha1.

@ghost
Copy link
Author

ghost commented Oct 11, 2013

27th Jul 2012, Aric said:


Perhaps the Mozilla Secure Coding Guidelines be used as a reference for the new bcrypt implementation?

https://wiki.mozilla.org/WebAppSec/Secure_Coding_Guidelines#Password_Storage

H: password -> bcrypt(HMAC(password, local_salt), bcrypt_salt)

@ghost
Copy link
Author

ghost commented Oct 11, 2013

28th Jul 2012, Mark Story said:


That's a possibility, as of 2.3 Security::hash() can easily be used to generate bcrypt hashes, but without the additional HMAC pre-hashing. I'm not totally sure what the benefits of the additional round of hashing are other than to theoretically protect against issues in bcrypt.

@ghost
Copy link
Author

ghost commented Oct 11, 2013

30th Jul 2012, Aric said:


Theoretically, it ensures that if a password database was compromised and vulnerabilities were to exist in bcrypt, things would remain safe.

The local salt is a random value that is stored only on the server, never in the database. Why is this good? If an attacker steals one of our password databases, they would need to also separately attack one of our web servers to get file access in order to discover this local salt value. If they don’t manage to pull off two successful attacks, their stolen data is largely useless.

https://blog.mozilla.org/webdev/2012/06/08/lets-talk-about-password-storage/

@ghost
Copy link
Author

ghost commented Oct 11, 2013

25th Aug 2012, Heath said:


Adding Security.salt to the beginning or ending of the plain text password before it is hashed with bcrypt sounds fine. It adds another vector for the cracker to deal with and helps protect against short passwords. This also means the hashes are bound to that particular installation. While this is beneficial for security (as long as the attacker can't access your file system), it also makes the hashes non-portable. Bcrypt hashed passwords (w/o prepended salt) are actually quite portable and can be moved from one installation to another without needing to be converted or reset. Security should probably trump convenience but it is a factor.

As I understand HMAC, it is about data verification, identification, and secure transport using a preshared key. In other words it is similar to doing a md5 hash of some file that you want to verify hasn't changed later. The context for this use is transport over the internet and therefore most implementations use fast algorithms. This means you don't want to do HMAC before hashing with bcrypt as you are then storing an easily crackable version of the password. HMAC use on the hashed (by bcrypt) password is debatable IMO. You are likely to want to store the result in the database since you need to store a HMAC record for each password hash. If the database is compromised, the attacker would have access to the password hashes as well as the HMAC hashes.

@ghost
Copy link
Author

ghost commented Oct 11, 2013

25th Aug 2012, Mark Story said:


I'm not really convinced that the additional hashing step is really required. In the past all previous hashing methods became in-secure/not-usable as creating collisions became easier with more modern hardware. If this were to happen with bcrypt, no amount of additional hashing would be required as a collision would still be possible. The current bcrypt implementation creates a new salt for every hashed password, which should be create more than enough of a deterrent.

@ghost
Copy link
Author

ghost commented Oct 11, 2013

20th Sep 2012, Adrian said:


PHP is going to provide a native API for hashing passwords based on bcrypt in PHP 5.5:
https://wiki.php.net/rfc/password_hash

There is also a compatibility library on github, although it needs PHP >= 5.3.7:
https://github.com/ircmaxell/password_compat/blob/master/lib/password.php

@ghost
Copy link
Author

ghost commented Oct 11, 2013

21st Sep 2012, Mark Story said:


Nice, bcrypt support has been added to CakePHP2.3 and will be the default in 3.x. When 5.5 comes out we can always add an adapter for the new functions as well.

@ghost
Copy link
Author

ghost commented Oct 11, 2013

5th Nov 2012, Przemysław Pawliczuk said:


Ok, but how about supporting libraries like phpass?

@ghost
Copy link
Author

ghost commented Oct 11, 2013

6th Nov 2012, Mark Story said:


You can use a separate authentication adapter for that. In 5.4 there is no reason to use phpass if all we need is access to bcrypt.

@ghost
Copy link
Author

ghost commented Oct 11, 2013

29th Jan 2013, Nick said:


I understand that CakePHP 3.0 is going to be PHP 5.4+. That being the case, is there a plan to use "$2y$" instead of "$2a$" for the beginning of the salt? PHP's crypt documentation says:

developers targeting only PHP 5.3.7 and later should use "$2y$" in preference to "$2a$"

However, there is the concern of backwards compatibility. I'm just wondering what I can expect CakePHP 3.0 to do so that I can plan ahead.

@dereuromark
Copy link
Member

Using $2y$ sure makes sense. For BC we could maybe add a "deprecated" BC hashing method "brycpt", "blowfish_deprecated" or sth. It would only be needed for those who have been using blowfish hashing with the old prefix.
Alternatively, depending on how often this will change in the future, we can make this prefix an attribute of the security class to easily modify.

@markstory
Copy link
Member

Closing. The default hashing in 3.0 is now bcrypt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants