Use native PHP Password API

[apiuser]

Feature request from airbone42, posted on GitHub Jan 21, 2015

PHP 5.5 introduced a new password API natively to PHP. http://php.net/manual/en/book.password.php

As using BCRYPT for the default hashing algorithm it's not only more secure than the current implementaiton of md5 and sha256. But will even be automatically maintained with newer PHP versions and does not depend on any maintenance or upgrades by Magento.

So my suggestion is to replace the current hashing implementation in the Encryptor with using native password_hash and password_verify. Especially for an e-commerce system security should have a very high priority.

So rumors tell that Magento 2 will soon raise min. requirements to PHP 5.5, so that would be the best point to integrate this. Anyway if that min. version update might not come there's also a backward compatibility library available at https://github.com/ircmaxell/password_compat which could be used for PHP <5.5.

If Magento needs MD5 and SHA256 for b/c to Magento 1 hashes, I would suggest to move that into a separate module, so new Magento 2 shops without old data don't need to bother about this old hasing algorithms and the code coming with it. Even shops with older Magento 1 data could remove that b/c module after all customers have updated their password over time or by enforcing them after the first login. This reduces amount of code and complexity, buy having these Mage1 b/c modules and migrations optional.

Anyway, would Magento be interested in porting the Encryptor into that way? If it will get accepted I would definitely dig into this and try to create a PR to speed up development.

[apiuser]

Comment from davidalger, posted on GitHub Jan 21, 2015

If I remember correctly, @ericthehacker mentioned to me the other day that he was planning on porting/integrating his M1 module (https://github.com/ericthehacker/magento-phpnativepasswords) for supporting this to M2 and submitting a PR. Don't know if he's started yet or not. I think the idea is a good move in the right direction.

For supporting the old hashes, it should be pretty trivial to support multiple hash types, but I think supporting them on an ongoing basis is a must and IMO it wouldn't make sense to split different hash types out into separate modules.

[apiuser]

Comment from ericthehacker, posted on GitHub Jan 21, 2015

I can confirm that I am planning to port my M1 module to M2 and issue a pull request.

[apiuser]

Comment from ericthehacker, posted on GitHub Jan 21, 2015

Work in progress implementation on https://github.com/ericthehacker/magento2/compare/feature/native-password-hashing .

[apiuser]

Comment from ericthehacker, posted on GitHub Jan 22, 2015

@airbone42 @DavidAlger The implementation is complete, with unit tests, on https://github.com/ericthehacker/magento2/compare/feature/native-password-hashing .

The only remaining issue is to determine the best way to include the compat library. Although it is composer friendly, it doesn't use any particular class naming standard -- it simply defines the collection of four global functions which comprise the native PHP password hashing API -- so I can't quickly think of a good way to ensure it's available without relying on the autoloader.

I'll probably take a look at this tomorrow, but any ideas on how to import this compat library on PHP < 5.5.0 are welcome in the meantime.

[apiuser]

Comment from ericthehacker, posted on GitHub Jan 23, 2015

It's still not clear to me how to import the compatibility library since it doesn't contain a class which the autoloader will pick up on.

Obviously, I could follow the M1 approach of using require_once in the model which uses it, and this of course works perfectly: https://gist.github.com/ericthehacker/09ed7e8f1ac4d567c90c .

However, in the M2 core, I can't find a clear precedent for arbitrarily requiring external libraries, nor can I find a similar example of including a vendor function-only library.

Of course, if bumping the minimum PHP requirement to 5.5.0 is an option (which I would support), this issue may be irrelevant.

Any ideas?

[apiuser]

Comment from buskamuza, posted on GitHub Jan 23, 2015

@ericthehacker , we're going to stop supporting PHP 5.4 soon. The work is already in progress. So maybe you just don't need compatibility library then?

[apiuser]

Comment from alankent, posted on GitHub Jan 23, 2015

I have not looked at any code, but just for clarity a requirement is going to be to upgrade customers from M1 with currently encrypted passwords. So while using a better hash for new passwords sounds great (1) we need to migrate old passwords during an upgrade, and (2) we would need to schedule an internal security audit of the new code as a safeguard.

[apiuser]

Comment from ericthehacker, posted on GitHub Jan 23, 2015

@buskamuza Thanks for the clarification of regarding PHP version support.

@alankent Upgrading previously hashed customer passwords shouldn't be a problem -- I've already implemented this in my M1 module but wasn't sure if it was necessary for M2.

[apiuser]

Comment from avoelkl, posted on GitHub Jan 23, 2015

Good thing to use the standard PHP password functions from PHP >= 5.5! As PHP 5.4 is in extended support with security fixes already and will be at end of live at the time when M2 is released, this is the chance to do so (as @buskamuza already confirmed). password_hash/password_verify are available from PHP 5 >= 5.5.0 on so I don't see a need for the compatibility library then.

Regarding migration: I like the way @airbone42 suggested by putting the old encryption functionality into a seperate module which is optional. This keeps the old hashing from M2 away. So there won't be a need to migrate old passwords all at once during an upgrade as this extra module could just check the password at login with the old hashing functions and create a new password with the new PHP password function.