-
-
Notifications
You must be signed in to change notification settings - Fork 24
Add Argon2i for password hashing #1
Comments
I've refactored out the usage of the default constants but it seems like the PHP7.2 binaries from travis-ci have no argon2 support whatsoever, see: travis-ci/travis-ci#8863 Originally posted by @MatthiasKuehneEllerhold at zendframework/zend-crypt#58 (comment) |
Rebased it unto develop and changed the PR target. I've tried to orient myself on the existing bcrypt class. Originally posted by @MatthiasKuehneEllerhold at zendframework/zend-crypt#58 (comment) |
No worries, I'm just suggesting to get rid of some cruft, especially on new classes. @ezimuel needs to check these, since he's the lead here. Originally posted by @Ocramius at zendframework/zend-crypt#58 (comment) |
@MatthiasKuehneEllerhold I forgot to answer to your main questions:
Originally posted by @ezimuel at zendframework/zend-crypt#58 (comment) |
|
This PR adds an extra class for the argon2i password hash algorithm introduced with PHP7.2.
There are some open questions with this:
Do we really want to have 1 new class for each algorithm PHP addes? "Argon2id" is just around the corner...
PHP's
password_verify()
accepts currently both Bcrypt and Argon2i hashes. So basically you could either use theBcrypt
class or theArgon2i
class to verify either hashes.This makes it pretty easy to migrate users from Bcrypt to Argon2i:
Argon2i
class as the dependencyDo we need a wrapper function for
password_needs_rehash()
? This would mean we need some kind of inter-class upgrade path (from classBcrypt
toArgon2i
in future php-versions).Although we dont know WHY
password_needs_rehash()
returns false: is it because of the algorithm or the cost value(s)?This class is marked as PHP7.2+ only (it throws an exception in the constructor). Do we want to provide fallbacks of some kind for older PHP Versions?
Other than the algorithm no other PHP7.2+ specific features were used in this class (e. g. scalar type hints and return types) because a syntax error is much more heavy and harder to catch than a constructor-exception (Pre 7.0).
Should type hints and return types get added because its a PHP7.2+ class anyway?
Originally posted by @MatthiasKuehneEllerhold at zendframework/zend-crypt#58
The text was updated successfully, but these errors were encountered: