Is this overkill? Dynamic salt encryption for whirlpool password encryption

Question

I think I might've generated a bit of an overkill salt when it comes to password encryption in whirlpool.

This is what the salt generation code does, step by step

1. A pseudo-random string with a length of 10 is generated, It has these possible values :

   0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ

2. A truly random number from -2 to 123 is generated by atmospheric noise

3. This pseudo random string and completely random number are shuffled together with a unix timestamp.
4. This pre-salt is then encrypted with md5.

Thats simply the salt generation, I also have the whole thing encrypted in whirlpool

hash( 'whirlpool',$salt.$password);

Is there a security flaw in my code? Should I md5 hash the values before they are joined or after?

Here is my code to check...

<?php
function generateRandomString($length = 10) {
    $characters = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
    $randomString = '';
    for ($i = 0; $i < $length; $i++) {
        $randomString .= $characters[rand(0, strlen($characters) - 1)];
    }
    return $randomString;
}

function get_true_random_number($min = 1, $max = 100) {
    $max = ((int) $max >= 1) ? (int) $max : 100;
    $min = ((int) $min < $max) ? (int) $min : 1;
    $options = array(
        CURLOPT_RETURNTRANSFER => true,
        CURLOPT_HEADER => false,
        CURLOPT_FOLLOWLOCATION => true,
        CURLOPT_ENCODING => '',
        CURLOPT_USERAGENT => 'PHP',
        CURLOPT_AUTOREFERER => true,
        CURLOPT_CONNECTTIMEOUT => 120,
        CURLOPT_TIMEOUT => 120,
        CURLOPT_MAXREDIRS => 10,
    );

    $ch = curl_init('http://www.random.org/integers/?num=1&min='
        . $min . '&max=' . $max . '&col=1&base=10&format=plain&rnd=new');
    curl_setopt_array($ch, $options);
    $content = curl_exec($ch);
    curl_close($ch);

    if(is_numeric($content)) {
        return trim($content);
    } else {
        return rand(-10,127); //INCASE RANDOM.ORG returns a server busy error
    }
}

function generateSalt() {
    $string = generateRandomString(10);
    $int = get_true_random_number(-2,123);
    $shuffled_mixture = str_shuffle(Time().$int.$string);
    return $salt = md5($shuffled_mixture);
}

echo generateSalt();
?>

Also, just how long should a salt be? I personally don't think hashing it's other types of characters matters that much simply because no one brute forces an entire salt, I don't think. That's all! Thanks in advance. Also, any better or more efficient method would be helpful.

Answer 1

A general guideline regarding cryptography: Don't ever write your own crypto! That includes creating your own algorithms and implementing publicly peer reviewed systems. It's far too difficult to get right (even for experts).

Starting with PHP 5.5 there is a very simple and sound API you should use for your password hashing: password_hash() and password_verify()

---

Now regarding your implementation and algorithm:

You are worrying about the wrong aspects of your password hashing algorithm. The salt you use does not have to include ultra high entropy values and even those can be collected in a much more simple fashion (if you are on Linux for example read 32 bytes from /dev/urandom).

The aspect you should instead worry about is how hard it is to brute force the password given the salt and the hash (the scenario of a database breach). That effectively means hashing all likely passwords with the saved salt and comparing it against the stored reference hash. To slow down an attacker you want to use a slow hash function to prevent a lot password comparisons. An example would be to hash the resulting hash again and again (a few thousand times).

The salt protects against rainbow table attacks, where huge precomputed databases can be queried to look up the original hash. You (successfully) prevent that attack vector by randomizing the hash function via the salt.

Answer 2

There is an excellent article related to this specific subject by Miguel Ibarra Romero:

Risks and Challenges of Password Hashing

He states that salts should be 64 characters long to prevent bruteforce attacks.

On the implementation side: I highly recommend against using a source of true random numbers that you do not have any physical control over. Getting random numbers from someone else's web site, as convenient as it may be, is not sound security practice. You do not know with cryptographically secure certainty where those numbers came from, how they were generated, how biased they are, and if there is a record of you receiving them.