Hi all,
A user requested that crypt() should raise error without 2nd(slat)
parameter.
https://bugs.php.net/bug.php?id=55036
crypt() without salt generates extremely weak password hash. In addition to
this,
PHP 5.5 has password_hash()
This change should be applied from 5.5, IMHO.
Any comments?
--
Yasuo Ohgaki
yohgaki@ohgaki.net
A user requested that
crypt()should raise error without 2nd(slat)
parameter.https://bugs.php.net/bug.php?id=55036
crypt()without salt generates extremely weak password hash.
The docs seem to indicate that some implementations generate their own
random salt if one is not supplied? It doesn't seem right to raise a
warning if it doesn't apply to all cases.
A user requested that
crypt()should raise error without 2nd(slat)
parameter.https://bugs.php.net/bug.php?id=55036
crypt()without salt generates extremely weak password hash.The docs seem to indicate that some implementations generate their own
random salt if one is not supplied? It doesn't seem right to raise a
warning if it doesn't apply to all cases.
I do get a md5 with a salt when calling crypt, and looking at php that
seems
to be the intended behavior, not something system dependant (that's done
since 5.3, according to the docs).
I see a problem in that it uses php_rand() to generate the salt, but the
solution
should be to use php_password_make_salt for creating the salt, not the
warning.
Hi Yasuo
2013/8/7 Yasuo Ohgaki yohgaki@ohgaki.net:
Hi all,
A user requested that
crypt()should raise error without 2nd(slat)
parameter.https://bugs.php.net/bug.php?id=55036
crypt()without salt generates extremely weak password hash. In addition to
this,
PHP 5.5 haspassword_hash()This change should be applied from 5.5, IMHO.
This is a BC break, as the second parameter as noted is optional,
while I believe we can do it in 5.5, I don't think it is worth the
effort, neither to put a notice or similar (E_STRICT even), if the
second parameter is left out, and it should go in master instead which
I'm in favor of.
--
regards,
Kalle Sommer Nielsen
kalle@php.net
Yasuo,
Hi all,
A user requested that
crypt()should raise error without 2nd(slat)
parameter.https://bugs.php.net/bug.php?id=55036
crypt()without salt generates extremely weak password hash. In addition to
this,
PHP 5.5 haspassword_hash()This change should be applied from 5.5, IMHO.
Any comments?
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Well, first off, a BC break like this should never go into a current
release. So I'd support for master / NEXT only anyway.
I did some digging, and it appears that the python module also does similar
behavior (allowing a null salt). But their behavior is to generate a strong
salt for the strongest algorithm available. So their behavior is actually
useful.
The other implementations that I've looked at would all error or simply
ignore the fact that the salt was empty (as a valid DES salt).
So, keeping with standard practices, I think we should E_DEPRECATE the
usage with only 1 parameter (no salt), and then in NEXT.NEXT change the
zend_parse_parameters definition to require 2 parameters...
My $0.02...
Hi!
A user requested that
crypt()should raise error without 2nd(slat)
parameter.https://bugs.php.net/bug.php?id=55036
crypt()without salt generates extremely weak password hash. In addition to
this,
I see that when I run crypt with one parameter, it generates salted
password hash. I imagine since on many systems it will produce md5-based
hash which is no longer considered adequate for many applications, it
may be not the best way to use it, but I don't see how it is an error to
do it. I'd rather have crypt() use stronger hash by default or maybe
have parameter that sets which hash is being used.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi all,
It seems there are 2 options for master branch when crypt()'s 2nd parameter
is omitted.
- raise E_DEPRECIATED that advice use of stronger salt or
password_hash()
and make 2nd parameter required for future release. - make
crypt()use stronger default salt/hash w/o error
Since password_hash() is supposed to do better job, first option seems
better to me.
Do I have to setup vote?
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi all,
It seems there are 2 options for master branch when
crypt()'s 2nd parameter
is omitted.
- raise E_DEPRECIATED that advice use of stronger salt or
password_hash()
and make 2nd parameter required for future release.- make
crypt()use stronger default salt/hash w/o errorSince
password_hash()is supposed to do better job, first option seems
better to me.
Deprecating it means it will be removed in the future.
Please leave the function alone. This should be solved with education,
not a gun to peoples head.
-Hannes
Hi Hannes,
On Thu, Aug 8, 2013 at 1:22 PM, Hannes Magnusson <hannes.magnusson@gmail.com
wrote:
Hi all,
It seems there are 2 options for master branch when
crypt()'s 2nd
parameter
is omitted.
- raise E_DEPRECIATED that advice use of stronger salt or
password_hash()
and make 2nd parameter required for future release.- make
crypt()use stronger default salt/hash w/o errorSince
password_hash()is supposed to do better job, first option seems
better to me.Deprecating it means it will be removed in the future.
Please leave the function alone. This should be solved with education,
not a gun to peoples head.
This would be third option.
I agree that good documentation is always good.
E_NOTICE might be better as E_DEPRECIATED means obsolete.
I'll write RFC for voting later. Please comment so that your comments are
in RFC.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi all,
A user requested that
crypt()should raise error without 2nd(slat)
parameter.https://bugs.php.net/bug.php?id=55036
crypt()without salt generates extremely weak password hash. In addition
to this,
PHP 5.5 haspassword_hash()This change should be applied from 5.5, IMHO.
Any comments?
I've made RFC for this issue.
https://wiki.php.net/rfc/crypt_function_salt
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net