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!