yohgaki

Yasuo Ohgaki

Contents

PHP RFC: Change crypt() behavior w/o salt

Introduction

crypt() without “salt” parameter produces too weak password hash. However, many users are using crypt() without salt parameter.

Proposal

There are several options for this issue.

  1. Make crypt() generates stronger salt and uses strong hash automatically.
  2. Raise E_STRICT or E_NOTICE when salt is omitted.
  3. Leave crypt() as it is now and warn users by document.

Backward Incompatible Changes

  • Option 1 may generate backward incompatible hash.
  • Option 2 may break apps due to error.
  • Option 3 does not change crypt() and no issue.

Proposed PHP Version(s)

master branch

Proposed Voting Choices

Option 1 to 3.

Patches and Tests

Patch for option 1 is posted to internals ML by Platonides platonides@gmail.com

diff --git a/ext/standard/crypt.c b/ext/standard/crypt.c
index 113a5bd..7731808 100644
--- a/ext/standard/crypt.c
+++ b/ext/standard/crypt.c
@@ -98,8 +98,6 @@
 #define PHP_STD_DES_CRYPT 1
 #endif

-#define PHP_CRYPT_RAND php_rand(TSRMLS_C)
-
 PHP_MINIT_FUNCTION(crypt) /* {{{ */
 {
        REGISTER_LONG_CONSTANT("CRYPT_SALT_LENGTH", PHP_MAX_SALT_LEN, CONST_CS | CONST_PERSISTENT);
@@ -134,17 +132,6 @@ PHP_MSHUTDOWN_FUNCTION(crypt) /* {{{ */
 }
 /* }}} */

-static unsigned char itoa64[] = "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
-
-static void php_to64(char *s, long v, int n) /* {{{ */
-{
-       while (--n >= 0) {
-               *s++ = itoa64[v&0x3f];
-               v >>= 6;
-       }
-}
-/* }}} */
-
 PHPAPI int php_crypt(const char *password, const int pass_len, const char *salt, int salt_len, char **result)
 {
        char *crypt_res;
@@ -278,11 +265,14 @@ PHP_FUNCTION(crypt)
        if (!*salt) {
 #if PHP_MD5_CRYPT
                strncpy(salt, "$1$", PHP_MAX_SALT_LEN);
-               php_to64(&salt[3], PHP_CRYPT_RAND, 4);
-               php_to64(&salt[7], PHP_CRYPT_RAND, 4);
+               if (php_password_make_salt(8, &salt[3] TSRMLS_CC) == FAILURE) {
+                       RETURN_FALSE;
+               }
                strncpy(&salt[11], "$", PHP_MAX_SALT_LEN - 11);
 #elif PHP_STD_DES_CRYPT
-               php_to64(&salt[0], PHP_CRYPT_RAND, 2);
+               if (php_password_make_salt(2, &salt[0] TSRMLS_CC) == FAILURE) {
+                       RETURN_FALSE;
+               }
                salt[2] = '\0';
 #endif
                salt_in_len = strlen(salt);

References

Votes

An option needs 50%+1 votes to win

Change crypt behavior when salt parameter is omitted. (85.7% approved)
User Vote
aharvey Generate E_NOTICE error
datibbaw Keep current behavior (Use weak hash)
dm Generate E_NOTICE error
indeyets Generate E_NOTICE error
kassner Generate E_NOTICE error
krakjoe Generate E_NOTICE error
levim Generate E_NOTICE error
lstrojny Generate E_NOTICE error
nikic Generate E_NOTICE error
pajoye Generate E_NOTICE error
remi Generate E_NOTICE error
stas Keep current behavior (Use weak hash)
tyrael Generate E_NOTICE error
yohgaki Generate E_NOTICE error