I wrote a PHP function that generates salts for use with password hashing:
function getSalt() {
$charset = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789/\\][{}\'";:?.>,<!@#$%^&*()-_=+|';
$randString = "";
$randStringLen = 64;
while(strlen($randString) < $randStringLen) {
$randChar = substr(str_shuffle($charset), mt_rand(0, strlen($charset)), 1);
$randString .= $randChar;
}
return $randString;
}
To me, this seems like a secure salt generator because the use of the while()
loop and string concatenation for every iteration of substr()
and str_shuffle
will potentially use the same character in $charset
twice (as opposed to simply str_shuffle()
'ing once).
2 Answers 2
It is not necessary for a salt to be perfectly random, it just should be unique for every password you save to defeat rainbow tables and and cracking several passwords at the same time. If it would be required to be perfectly random you would have to use a secure random number generator like openssl_random_pseudo_bytes
.
That being said your algorithm looks overly complex to me. As you add a new character to your salt in every iteration of the loop you can use a for
loop here: for ($i = 0; $i < $randStringLen; $i++)
.
There also is no need for the str_shuffle
, you are picking a random index from your character set anyway. Note that I have to subtract one from the string length, as everything is zero indexed here. In your version it did not matter, as you are not looping for a fixed number of iterations, but until your string is long enough. I would consider it a bug none-the-less. So this can be reduced to a simple:
substr($charset, mt_rand(0, strlen($charset) - 1), 1);
or even
$charset[mt_rand(0, strlen($charset) - 1)];
Further I suggest reordering your variables. Currently the "internal parameters" are interleaved with the return value (randString
). I would move the return value down.
All in all your algorithm would look something like this:
function getSalt() {
$charset = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789/\\][{}\'";:?.>,<!@#$%^&*()-_=+|';
$randStringLen = 64;
$randString = "";
for ($i = 0; $i < $randStringLen; $i++) {
$randString .= $charset[mt_rand(0, strlen($charset) - 1)];
}
return $randString;
}
But in the end it does not matter how your salt generation algorithm looks like, as you should be using password_hash
for the exact reason that you do not have to worry about these kind of issues!
-
\$\begingroup\$ Thank you for the direct critique without jumping straight to "use
password_hash()
for this!". I do understand that every salt should be unique and not just random, but the random factor plays a part in a salt's uniqueness. What is the advantage of using thefor()
loop over thewhile()
loop in this scenario? I was looking intopassword_hash()
on PHP.net's manual and the reason I was reluctant to use that right away was because I want to store the salt in the database. From the looks of it,password_hash()
returns the salt used, but how exactly would that salt be stored then? \$\endgroup\$Matthew U.– Matthew U.2015年06月07日 01:11:06 +00:00Commented Jun 7, 2015 at 1:11 -
\$\begingroup\$ @MatthewU. You really should use
password_hash
, though :-) \$\endgroup\$TimWolla– TimWolla2015年06月07日 01:12:04 +00:00Commented Jun 7, 2015 at 1:12 -
\$\begingroup\$ Unfortunately I'm using Debian Wheezy in this scenario which currently runs PHP 5.4, so
password_hash()
is not available for me. After looking intopassword_hash()
examples further, I now have a thorough understanding of it and will be utilizing that now after I upgrade my system and packages. :) \$\endgroup\$Matthew U.– Matthew U.2015年06月07日 01:35:22 +00:00Commented Jun 7, 2015 at 1:35 -
1\$\begingroup\$ @MatthewU. Use bcrypt then. You can reuse your salt generation function with a different charset. \$\endgroup\$TimWolla– TimWolla2015年06月07日 01:41:06 +00:00Commented Jun 7, 2015 at 1:41
-
\$\begingroup\$ @MatthewU. - there is a password-hash-compatibility script for PHP 5.4 (and patched 5.3.7+) by ircmaxell on github. It uses the same "guts" -
mcrypt_create_iv()
usingdev/urand
for the salt,bcrypt
for the KDF - but the back-and-forth between PHP and the binaries means it's just a little slower. You can usefunction_exists()
to determine whether or not you need to load the script. The salt (and algo and settings for the algo) is stored in the returned hash string. You don't need another column to store the hash;bcrypt
can extract it. \$\endgroup\$Stan Rogers– Stan Rogers2015年06月07日 07:26:32 +00:00Commented Jun 7, 2015 at 7:26
mt_rand
is seeded by a 32 bit integer. So there is little point in generating a 384 bit string. You'll still only get 4 billion different salts.- PHP's bcrypt API will ignore anything beyond the first 128 bits and expects a Base64 encoded salt, so your salt isn't usable for that either.
To generate random numbers you can either use
mcrypt_create_iv($size, MCRYPT_DEV_URANDOM)
oropenssl_random_pseudo_bytes
or (random_bytes($size)
in php 7). Note that with the openssl variant you should check the second parameter to see if you got back good random data.Then encode the output with Base64.
Just use the
password_hash
API (php 5.5) or its polyfill on older versions. Its easy to use and secure.