Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR for Password Hash RFC #191

Merged
merged 48 commits into from Oct 16, 2012
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
c77f2c2
Base structure for passsword_create and password_make_salt
Jun 25, 2012
7e41980
Actually complete password_create()
Jun 25, 2012
6574028
Implement password_verify
Jun 25, 2012
f7097d9
Fix memory leak on branch
Jun 25, 2012
18d3bd9
Basic random generator added to make_salt
Jun 25, 2012
618f262
More error checking, and some cleaning up for password.c
Jun 25, 2012
41d7374
Implement openssl support for make_salt
ircmaxell Jun 25, 2012
2d4b7cb
Refactor salt generation, rename password_create to password_hash
Jun 26, 2012
232da90
Implement php.ini setting password.bcrypt_cost
Jun 27, 2012
e505316
Add tests for password hashing
Jun 27, 2012
2b9591f
Update tests to check ini setting
Jun 27, 2012
5f44be0
Add tests and error checking for large salt requested values to preve…
Jun 27, 2012
0dd2f16
Fix formatting issues in password.c
ircmaxell Jun 27, 2012
6bb3865
Refactor crypt to use an external working function
ircmaxell Jun 28, 2012
da3d8bf
Refactor password.c a bit, add different error checking
ircmaxell Jun 28, 2012
9e18e57
Merge remote branch 'upstream/master' into hash_password
ircmaxell Jun 29, 2012
9c1445c
More refactoring of crypt into php_crypt, and fixing memory allocation
ircmaxell Jun 29, 2012
f53112f
Update password.c to use safe_emalloc in sensitive places
ircmaxell Jun 29, 2012
6cc3c65
Remove php.ini setting for default bcrypt cost
Jul 3, 2012
6943f2a
Some more refactoring, make algo no longer optional
Jul 3, 2012
886527d
Update signature info for changing algo to an ordinal
Jul 3, 2012
5160dc1
Implement password_needs_rehash() function
ircmaxell Jul 5, 2012
db86d54
Fix issue with int vs long parameter
ircmaxell Jul 5, 2012
ee7e799
Implement password_get_info() function
ircmaxell Jul 5, 2012
9d3630b
Cleanup whitespace issues
ircmaxell Jul 5, 2012
99b7956
Merge remote branch 'upstream/master' into hash_password
ircmaxell Jul 10, 2012
707c907
Switch second parameter to password_make_salt to be a flag
ircmaxell Jul 12, 2012
e05413c
Remove password_make_salt() from the implementation
ircmaxell Aug 28, 2012
824f1f4
Merge remote branch 'upstream/master' into hash_password
ircmaxell Sep 4, 2012
db41f9f
Refactoring to use size_t instead of int most places
ircmaxell Sep 4, 2012
e8b7f5b
Add tests for password_get_info and password_needs_rehash
ircmaxell Sep 12, 2012
e9a7bde
Switch test to using strict comparison for crypt fallback
ircmaxell Sep 12, 2012
ebe0bd5
Remove bcrypt_cost ini entry from declaration
ircmaxell Sep 12, 2012
76f3295
Expose PASSWORD_BCRYPT_DEFAULT_COST constant and update test to use it
ircmaxell Sep 12, 2012
3e383dc
Merge remote branch 'upstream/master' into hash_password
ircmaxell Sep 12, 2012
7161c3d
Add news entry for password API
ircmaxell Sep 12, 2012
7ec80e1
Fix incorrect arg info required param count for password_hash
ircmaxell Sep 12, 2012
83cfff4
Switch to using an ENUM for algorithms instead of a constant
ircmaxell Sep 13, 2012
e034a46
A bunch of naming convention fixes. No functionality changes
ircmaxell Sep 17, 2012
44c2624
Fix ucwords error casing
ircmaxell Sep 17, 2012
6fd5ba5
Fix arg info for required params passed to needs_rehash
ircmaxell Sep 17, 2012
8bd79d1
Refactor slightly to enable cleaner readability
ircmaxell Sep 17, 2012
4a7d18c
Fix some double free issues, and more cleanup work
ircmaxell Oct 5, 2012
25b2d36
Fix issue with possible memory leak
ircmaxell Oct 5, 2012
1751d5f
Really fix leaks, add test cases to prove it...
ircmaxell Oct 6, 2012
76e83f7
fix allocation and copy issue
ircmaxell Oct 6, 2012
37b2207
Clean up unreported memory leak by switching to zval_ptr_dtor
ircmaxell Oct 7, 2012
0bc9ca3
Refactor to using a stack based zval instead of dynamic allocation
ircmaxell Oct 7, 2012
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS
Expand Up @@ -3,6 +3,8 @@ PHP NEWS
?? ??? 201?, PHP 5.5.0

- General improvements:
. Add simplified password hashing API
(https://wiki.php.net/rfc/password_hash). (Anthony Ferrara)
. Support list in foreach (https://wiki.php.net/rfc/foreachlist). (Laruence)
. Implemented 'finally' keyword (https://wiki.php.net/rfc/finally). (Laruence)
. Drop Windows XP and 2003 support. (Pierre)
Expand Down
24 changes: 24 additions & 0 deletions ext/standard/basic_functions.c
Expand Up @@ -1854,6 +1854,25 @@ ZEND_END_ARG_INFO()
ZEND_BEGIN_ARG_INFO(arginfo_getlastmod, 0)
ZEND_END_ARG_INFO()
/* }}} */
/* {{{ password.c */
ZEND_BEGIN_ARG_INFO_EX(arginfo_password_hash, 0, 0, 2)
ZEND_ARG_INFO(0, password)
ZEND_ARG_INFO(0, algo)
ZEND_ARG_INFO(0, options)
ZEND_END_ARG_INFO()
ZEND_BEGIN_ARG_INFO_EX(arginfo_password_get_info, 0, 0, 1)
ZEND_ARG_INFO(0, hash)
ZEND_END_ARG_INFO()
ZEND_BEGIN_ARG_INFO_EX(arginfo_password_needs_rehash, 0, 0, 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here again the last arg should be 2 (as the algo is required too).

ZEND_ARG_INFO(0, hash)
ZEND_ARG_INFO(0, algo)
ZEND_ARG_INFO(0, options)
ZEND_END_ARG_INFO()
ZEND_BEGIN_ARG_INFO_EX(arginfo_password_verify, 0, 0, 2)
ZEND_ARG_INFO(0, password)
ZEND_ARG_INFO(0, hash)
ZEND_END_ARG_INFO()
/* }}} */
/* {{{ proc_open.c */
#ifdef PHP_CAN_SUPPORT_PROC_OPEN
ZEND_BEGIN_ARG_INFO_EX(arginfo_proc_terminate, 0, 0, 1)
Expand Down Expand Up @@ -2864,6 +2883,10 @@ const zend_function_entry basic_functions[] = { /* {{{ */
PHP_FE(base64_decode, arginfo_base64_decode)
PHP_FE(base64_encode, arginfo_base64_encode)

PHP_FE(password_hash, arginfo_password_hash)
PHP_FE(password_get_info, arginfo_password_get_info)
PHP_FE(password_needs_rehash, arginfo_password_needs_rehash)
PHP_FE(password_verify, arginfo_password_verify)
PHP_FE(convert_uuencode, arginfo_convert_uuencode)
PHP_FE(convert_uudecode, arginfo_convert_uudecode)

Expand Down Expand Up @@ -3614,6 +3637,7 @@ PHP_MINIT_FUNCTION(basic) /* {{{ */
BASIC_MINIT_SUBMODULE(browscap)
BASIC_MINIT_SUBMODULE(standard_filters)
BASIC_MINIT_SUBMODULE(user_filters)
BASIC_MINIT_SUBMODULE(password)

#if defined(HAVE_LOCALECONV) && defined(ZTS)
BASIC_MINIT_SUBMODULE(localeconv)
Expand Down
2 changes: 1 addition & 1 deletion ext/standard/config.m4
Expand Up @@ -580,7 +580,7 @@ PHP_NEW_EXTENSION(standard, array.c base64.c basic_functions.c browscap.c crc32.
incomplete_class.c url_scanner_ex.c ftp_fopen_wrapper.c \
http_fopen_wrapper.c php_fopen_wrapper.c credits.c css.c \
var_unserializer.c ftok.c sha1.c user_filters.c uuencode.c \
filters.c proc_open.c streamsfuncs.c http.c)
filters.c proc_open.c streamsfuncs.c http.c password.c)

PHP_ADD_MAKEFILE_FRAGMENT
PHP_INSTALL_HEADERS([ext/standard/])
2 changes: 1 addition & 1 deletion ext/standard/config.w32
Expand Up @@ -19,7 +19,7 @@ EXTENSION("standard", "array.c base64.c basic_functions.c browscap.c \
versioning.c assert.c strnatcmp.c levenshtein.c incomplete_class.c \
url_scanner_ex.c ftp_fopen_wrapper.c http_fopen_wrapper.c \
php_fopen_wrapper.c credits.c css.c var_unserializer.c ftok.c sha1.c \
user_filters.c uuencode.c filters.c proc_open.c \
user_filters.c uuencode.c filters.c proc_open.c password.c \
streamsfuncs.c http.c flock_compat.c", false /* never shared */);
PHP_INSTALL_HEADERS("", "ext/standard");
if (PHP_MBREGEX != "no") {
Expand Down
185 changes: 90 additions & 95 deletions ext/standard/crypt.c
Expand Up @@ -145,100 +145,54 @@ static void php_to64(char *s, long v, int n) /* {{{ */
}
/* }}} */

/* {{{ proto string crypt(string str [, string salt])
Hash a string */
PHP_FUNCTION(crypt)
PHPAPI int php_crypt(const char *password, const int pass_len, const char *salt, int salt_len, char **result)
{
char salt[PHP_MAX_SALT_LEN + 1];
char *str, *salt_in = NULL;
int str_len, salt_in_len = 0;
char *crypt_res;
salt[0] = salt[PHP_MAX_SALT_LEN] = '\0';

/* This will produce suitable results if people depend on DES-encryption
* available (passing always 2-character salt). At least for glibc6.1 */
memset(&salt[1], '$', PHP_MAX_SALT_LEN - 1);

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|s", &str, &str_len, &salt_in, &salt_in_len) == FAILURE) {
return;
}

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

/* The automatic salt generation covers standard DES, md5-crypt and Blowfish (simple) */
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);
strncpy(&salt[11], "$", PHP_MAX_SALT_LEN - 11);
#elif PHP_STD_DES_CRYPT
php_to64(&salt[0], PHP_CRYPT_RAND, 2);
salt[2] = '\0';
#endif
salt_in_len = strlen(salt);
} else {
salt_in_len = MIN(PHP_MAX_SALT_LEN, salt_in_len);
}

/* Windows (win32/crypt) has a stripped down version of libxcrypt and
a CryptoApi md5_crypt implementation */
#if PHP_USE_PHP_CRYPT_R
{
struct php_crypt_extended_data buffer;

if (salt[0]=='$' && salt[1]=='1' && salt[2]=='$') {
char output[MD5_HASH_MAX_LEN];
char output[MD5_HASH_MAX_LEN], *out;

RETURN_STRING(php_md5_crypt_r(str, salt, output), 1);
out = php_md5_crypt_r(password, salt, output);
if (out) {
*result = estrdup(out);
return SUCCESS;
}
return FAILURE;
} else if (salt[0]=='$' && salt[1]=='6' && salt[2]=='$') {
const char sha512_salt_prefix[] = "$6$";
const char sha512_rounds_prefix[] = "rounds=";
char *output;
int needed = (sizeof(sha512_salt_prefix) - 1
+ sizeof(sha512_rounds_prefix) + 9 + 1
+ salt_in_len + 1 + 86 + 1);
output = emalloc(needed);
salt[salt_in_len] = '\0';
output = emalloc(PHP_MAX_SALT_LEN);

crypt_res = php_sha512_crypt_r(str, salt, output, needed);
crypt_res = php_sha512_crypt_r(password, salt, output, PHP_MAX_SALT_LEN);
if (!crypt_res) {
if (salt[0]=='*' && salt[1]=='0') {
RETVAL_STRING("*1", 1);
} else {
RETVAL_STRING("*0", 1);
}
memset(output, 0, PHP_MAX_SALT_LEN);
efree(output);
return FAILURE;
} else {
RETVAL_STRING(output, 1);
*result = estrdup(output);
memset(output, 0, PHP_MAX_SALT_LEN);
efree(output);
return SUCCESS;
}

memset(output, 0, needed);
efree(output);
} else if (salt[0]=='$' && salt[1]=='5' && salt[2]=='$') {
const char sha256_salt_prefix[] = "$5$";
const char sha256_rounds_prefix[] = "rounds=";
char *output;
int needed = (sizeof(sha256_salt_prefix) - 1
+ sizeof(sha256_rounds_prefix) + 9 + 1
+ salt_in_len + 1 + 43 + 1);
output = emalloc(needed);
salt[salt_in_len] = '\0';
output = emalloc(PHP_MAX_SALT_LEN);

crypt_res = php_sha256_crypt_r(str, salt, output, needed);
crypt_res = php_sha256_crypt_r(password, salt, output, PHP_MAX_SALT_LEN);
if (!crypt_res) {
if (salt[0]=='*' && salt[1]=='0') {
RETVAL_STRING("*1", 1);
} else {
RETVAL_STRING("*0", 1);
}
memset(output, 0, PHP_MAX_SALT_LEN);
efree(output);
return FAILURE;
} else {
RETVAL_STRING(output, 1);
*result = estrdup(output);
memset(output, 0, PHP_MAX_SALT_LEN);
efree(output);
return SUCCESS;
}

memset(output, 0, needed);
efree(output);
} else if (
salt[0] == '$' &&
salt[1] == '2' &&
Expand All @@ -251,31 +205,25 @@ PHP_FUNCTION(crypt)

memset(output, 0, PHP_MAX_SALT_LEN + 1);

crypt_res = php_crypt_blowfish_rn(str, salt, output, sizeof(output));
crypt_res = php_crypt_blowfish_rn(password, salt, output, sizeof(output));
if (!crypt_res) {
if (salt[0]=='*' && salt[1]=='0') {
RETVAL_STRING("*1", 1);
} else {
RETVAL_STRING("*0", 1);
}
memset(output, 0, PHP_MAX_SALT_LEN + 1);
return FAILURE;
} else {
RETVAL_STRING(output, 1);
*result = estrdup(output);
memset(output, 0, PHP_MAX_SALT_LEN + 1);
return SUCCESS;
}

memset(output, 0, PHP_MAX_SALT_LEN + 1);
} else {
memset(&buffer, 0, sizeof(buffer));
_crypt_extended_init_r();

crypt_res = _crypt_extended_r(str, salt, &buffer);
crypt_res = _crypt_extended_r(password, salt, &buffer);
if (!crypt_res) {
if (salt[0]=='*' && salt[1]=='0') {
RETURN_STRING("*1", 1);
} else {
RETURN_STRING("*0", 1);
}
return FAILURE;
} else {
RETURN_STRING(crypt_res, 1);
*result = estrdup(crypt_res);
return SUCCESS;
}
}
}
Expand All @@ -291,21 +239,68 @@ PHP_FUNCTION(crypt)
# else
# error Data struct used by crypt_r() is unknown. Please report.
# endif
crypt_res = crypt_r(str, salt, &buffer);
crypt_res = crypt_r(password, salt, &buffer);
if (!crypt_res) {
if (salt[0]=='*' && salt[1]=='0') {
RETURN_STRING("*1", 1);
} else {
RETURN_STRING("*0", 1);
}
return FAILURE;
} else {
RETURN_STRING(crypt_res, 1);
*result = estrdup(crypt_res);
return SUCCESS;
}
}
# endif
#endif
}
/* }}} */


/* {{{ proto string crypt(string str [, string salt])
Hash a string */
PHP_FUNCTION(crypt)
{
char salt[PHP_MAX_SALT_LEN + 1];
char *str, *salt_in = NULL, *result = NULL;
int str_len, salt_in_len = 0;
salt[0] = salt[PHP_MAX_SALT_LEN] = '\0';

/* This will produce suitable results if people depend on DES-encryption
* available (passing always 2-character salt). At least for glibc6.1 */
memset(&salt[1], '$', PHP_MAX_SALT_LEN - 1);

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|s", &str, &str_len, &salt_in, &salt_in_len) == FAILURE) {
return;
}

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

/* The automatic salt generation covers standard DES, md5-crypt and Blowfish (simple) */
if (!*salt) {
#if PHP_MD5_CRYPT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean that my salt will change depending on the platform?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's core crypt() code that exists right now (as of 5.3.0). The reason for the change here is that I refactored it into two functions so I could call the implementation from C.

But now, the salt will not change (assuming that you're comment is ment for this line)...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, alright. Never mind :)

strncpy(salt, "$1$", PHP_MAX_SALT_LEN);
php_to64(&salt[3], PHP_CRYPT_RAND, 4);
php_to64(&salt[7], PHP_CRYPT_RAND, 4);
strncpy(&salt[11], "$", PHP_MAX_SALT_LEN - 11);
#elif PHP_STD_DES_CRYPT
php_to64(&salt[0], PHP_CRYPT_RAND, 2);
salt[2] = '\0';
#endif
salt_in_len = strlen(salt);
} else {
salt_in_len = MIN(PHP_MAX_SALT_LEN, salt_in_len);
}
salt[salt_in_len] = '\0';

if (php_crypt(str, str_len, salt, salt_in_len, &result) == FAILURE) {
if (salt[0] == '*' && salt[1] == '0') {
RETURN_STRING("*1", 1);
} else {
RETURN_STRING("*0", 1);
}
}
RETURN_STRING(result, 0);
}
/* }}} */
#endif

/*
Expand Down