What's good/bad about this custom password hashing?

Question

For the record, this was purely for learning purposes, I've been told don't even use this for a personal website that I don't expect anyone to visit, so this post is unfortunately the last I'll see of this code. However, I've done all the things I read about and it seems decent to me (but I'm no expert), so I'm just curious as to which parts could potentially be bad enough that the recommendation to everyone is to never ever use a custom solution?
Not naming anyone, but I'm pretty sure I've already done a miles better job than the free webhost that begins with a triple number haha, my plaintext password was stolen from there.

Here's the basic idea of what I did:

Salt and IV
Each salt is generated by a random but not cryptographically secure function in PHP (it's better than the default random though). It is then encrypted with blowfish using a key defined in the code, and an IV defined in an actual file (which is protected by htaccess). The encrypted value is stored, so it's very easy to change the IV or salt key to invalidate all passwords. Also if the database is compromised, the salts still can't be decrypted without getting access to the file system.

Hash
It uses key stretching and sha512. The amount of key stretching is defined by a modulus of a crc32 value of the password and salt, so each user has a different amount, and you can change the value to allow longer processing times. Each repeat hash uses a few hashes inside it in an attempt to keep it pretty random.

Here's the actual code I have, if you don't know PHP it's still fairly easy to see what's going on line by line:

$salt_key = 'íSgüý[Îaí²zÿCV™F÷œDDa©^fU©êm���“.Ù   ®Kâ³ÃX}rÿ,¢\ˆÿ7L³¢QÆH’YeTsÙõŸ¥éÙí#W—c´VÆ';

function get_iv(){
    // It stores it as a file, but there's no point showing that part so here's the actual code bit
    return mcrypt_create_iv(mcrypt_get_iv_size(MCRYPT_BLOWFISH, MCRYPT_MODE_CBC), MCRYPT_RAND);
}

function generate_salt(){
    global $salt_key;
    $salt = substr(uniqid(mt_rand(), true), 0, 32);
    $key = hash('sha256', $salt_key, true);
    $iv = get_iv();
    $encrypt = mcrypt_encrypt(MCRYPT_BLOWFISH, $key, $salt, MCRYPT_MODE_CFB, $iv);
    return array($salt, $encrypt);
}

function decrypt_salt($encrypted){
    global $salt_key;
    $key = hash('sha256', $salt_key, true);
    $iv = get_iv();
    return mcrypt_decrypt(MCRYPT_BLOWFISH, $key, $encrypted, MCRYPT_MODE_CFB, $iv);
}

function hash_password($password, $salt){
    #return hash('sha512', $password.$salt, true);
    $hash = $salt;
    for($i=0; $i<(crc32($salt.$password) % 129197); $i++){
        $hash = hash('sha512', $hash.$password.$salt.hash('sha512', $salt.$hash.$password).md5($salt.md5($password.$hash)), true);
    }
    return $hash;
}

The part to check if the password is valid basically just checks the new hash against the old one so I left it out. The only drawback based on the things I've read a good algorithm should do, is it doesn't use a lot of memory.

Answer 1

Open questions:

• Why aren't you using a cryptographically secure RNG for the salt? You're not going to be generating them so frequently as to risk depleting the system entropy pool on a modern system.
• How was your salt encryption key generated?
• How long is your salt_key (before you hash it)?
• Why are you hashing your salt_key? That adds no security.
• Why the overly-complicated nested hashing (including use of MD5, of all things) in your key-stretching loop? It puts me in mind of Moxie Marlinspike's commentary on the (broken) MS-CHAP v2 scheme (search for the string "unnecessary complexity").

Notable issues:

• You do, in fact, want to use securely-random salts (and IVs). It matters how predictable the salt is; predictable salts allow attackers to pre-compute lookup tables for the predicted salts against likely passwords. Predictable IVs are probably less of a risk in this context but there's no reason not to use securely random ones.
• Your key stretching iteration algorithm provides little-if-any increase in security - for each trial password, the attacker can compute the iteration count just fine, assuming they know the salt (and if they don't then they can't try to brute-force the password verifier at all) - and puts you at risk of running far fewer rounds of hashing than you intend. In particular, if the CRC is an exact multiple of 127197, the user's salt is stored as their password verifier and the password is not used at all, so any password at all will work for that user! If you get a million users, about 8 of them will have no password protection at all.
• You have no method for rotating or upgrading the algorithm for password verification, at least nothing visible in what you've presented here. Hardcoded critical attributes (such as the key, the cryptographic primitives, and the way iteration count is determined) may need to change at some point in the future, at which point you would need to be able to verify a user's password under the old scheme before generating a new password verifier with the new scheme (unless you just plan to force everybody to reset their password, which is a hassle and should only be done when there's reason to expect the old ones are compromised). Therefore, you need a way to specify a password verification algorithm version, or at least need to store identifiers (probably indices) for your various currently-constant values, and look the values up based on each user's stored version identifier / index.
• In the worst case, 129+k iterations of SHA-512 (never mind SHA512(SHA512(MD5(MD5)))) is going to take a long time on your server. An attacker that finds a combination of username and password for which it is so expensive to compute the verifier can just submit a bunch of requests with credentials like that (especially since, due to the reversibility of CRC, the attacker can manipulate the input to get exactly the desired checksum). That will probably cause a denial-of-service on your server. Work factors for password verification need to be high enough to slow down brute-forcing, but must not be high enough to be a way that an attacker can overload a server.

Answer 2

One bad thing about it is that it's complicated, and any developer joining the project won't be able to easily tell whether it's secure unless they take the time to read through it and understand it. If they care about security and about system quality in general then they may be motivated to take that time even if if's not necessary for their immediate tasks.

The simple solution, in PHP 5.5 or later, is to just use the built in password_hash and password_verify functions.