[VOTE] Change crypt() behavior w/o salt View as Markdown

As a non-english speaker i'd like to throw my 2cents into the bag: Notice:
crypt(): weak hash generated due to lack of given salt in Command line code
on line 1

Regards,

Robin Speekenbrink

2013/10/22 Joe Watkins krakjoe@php.net

Hi all,

Any comments patch for this RFC?
Better E_NOTICE message is welcome.

diff --git a/UPGRADING b/UPGRADING
index b19c3dd..8f5033a 100755
--- a/UPGRADING
+++ b/UPGRADING
@@ -64,6 +64,10 @@ PHP X.Y UPGRADE NOTES

4. Changed Functions
   ==============================**==========

+- Standard:

• crypt() without salt parameter raises E_NOTICE.

• See https://wiki.php.net/rfc/**crypt_function_salthttps://wiki.php.net/rfc/crypt_function_salt

• • cURL:
    CURLOPT_SAFE_UPLOAD is now turned on by default and uploads with @file
    do not work unless it is explicitly set to false.
    diff --git a/ext/standard/crypt.c b/ext/standard/crypt.c
    index 113a5bd..eea6f78 100644
    --- a/ext/standard/crypt.c
    +++ b/ext/standard/crypt.c
    @@ -272,6 +272,8 @@ PHP_FUNCTION(crypt)

  if (salt_in) {
  memcpy(salt, salt_in, MIN(PHP_MAX_SALT_LEN, salt_in_len));

• } else {

• php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Weak hashed password is
  generated without salt parameter");
  }

  /* The automatic salt generation covers standard DES, md5-crypt and
  Blowfish (simple) */

Output:
[yohgaki@dev PHP-master]$ ./php-bin -d error_reporting=-1 -r
"crypt('asdfa');"
Notice: crypt(): Weak hashed password is generated without salt parameter
in Command line code on line 1

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Hi all,

Vote period is ended and the result was

0 Generate strong salt by default
12 Generate E_NOTICE error
2 Keep current behavior (Use weak hash)

I'll prepare patch for it later.

Thank you.

--
Yasuo Ohgaki
yohgaki@ohgaki.net

On Wed, Sep 25, 2013 at 1:17 PM, Yasuo Ohgaki yohgaki@ohgaki.net
wrote:

Hi Alexey,

On Tue, Sep 24, 2013 at 11:22 PM, Alexey Zakhlestin <indeyets@gmail.com

wrote:

strangely, this RFC is not shown in "In voting phase" section here:

https://wiki.php.net/rfc
is that done manually?

I should have done this.
Thank you for point it out.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

The wording is not very good/does not make sense:

Notice: crypt(): weak hash generated without salt in Command line code on
line 1

Cheers
Joe

Any comments patch for this RFC?
Better E_NOTICE message is welcome.

I'm a native English speaker, how about "Calling crypt() without giving a
salt will not produce strong password hashes."?

It doesn't necessarily say you will produce a strong hash with it (other
factors are at play), but it does say that you can't without it.

Perhaps "secure" might be better than "strong".

I think I'd prefer the wording to be a little stronger, since this is
going to be shown when the user has actually done that. How about:

"Generating an insecure weak hash as no salt was given: please ensure
the salt parameter is specified and uses a strong hash type in order
to generate a cryptographically secure hash"

On the bright side, at least php_error_docref() will ensure there's a
link to the crypt() manual page in most setups. Rereading that, we may
actually want to be slightly more opinionated there about which hash
types are good and which are bad (it's not at all obvious that the DES
and MD5 types shouldn't generally be used).

Adam