Random string + encrypt/decrypt

Question

Are there any security flaws in what I plan to do?

I need to store the following in my DB:

1. a random string to act as a salt for encrypting a password
2. the encrypted password that used the salt in #1

Here's the PHP code I have to accomplish the above tasks:

<?php
    function decrypt($string, $encryption_key = '')
    {
        $initialization_vector = get_initialization_vector();

        // Convert hexadecimal data into binary representation
        $string = hex2bin($string);

        // See: http://php.net/manual/en/mcrypt.ciphers.php
        return trim(mcrypt_decrypt(MCRYPT_RIJNDAEL_128, $encryption_key, $string, MCRYPT_MODE_ECB, $initialization_vector));
    }

    function encrypt($string, $encryption_key = '')
    {
        $initialization_vector = get_initialization_vector();

        // See: http://php.net/manual/en/mcrypt.ciphers.php
        $string = mcrypt_encrypt(MCRYPT_RIJNDAEL_128, $encryption_key, $string, MCRYPT_MODE_ECB, $initialization_vector);

        // Convert binary data into hexadecimal representation
        $string = bin2hex($string);

        return $string;
    }


    function get_initialization_vector()
    {
        // See: http://php.net/manual/en/mcrypt.ciphers.php
        // MCRYPT_BLOWFISH selected as it appears to be one of the "universally"
        // supported ciphers supported by the mcrypt extension
        $initialization_vector_size = mcrypt_get_iv_size(MCRYPT_RIJNDAEL_128, MCRYPT_MODE_ECB);

        // See: http://php.net/manual/en/function.mcrypt-create-iv.php
        // The source can be MCRYPT_RAND (system random number generator), MCRYPT_DEV_RANDOM
        // (read data from /dev/random) and MCRYPT_DEV_URANDOM (read data from /dev/urandom).
        // Prior to 5.3.0, MCRYPT_RAND was the only one supported on Windows.
        $initialization_vector = mcrypt_create_iv($initialization_vector_size, MCRYPT_RAND);

        return $initialization_vector;
    }

    function get_random_string($character_set = '0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz', $minimum_length = 8, $maximum_length = 12)
    {
        if ($minimum_length > $maximum_length)
        {
            $length = mt_rand($maximum_length, $minimum_length);
        }
        else
        {
            $length = mt_rand($minimum_length, $maximum_length);
        }

        $random_string = '';
        for ($i = 0; $i < $length; $i++)
        {
            $random_string .= $character_set[(mt_rand(0, (strlen($character_set) - 1)))];
        }

        return $random_string;
    }

    function hex2bin($hexadecimal_data)
    {
        $binary_representation = '';

        for ($i = 0; $i < strlen($hexadecimal_data); $i += 2)
        {
            $binary_representation .= chr(hexdec($hexadecimal_data{$i} . $hexadecimal_data{($i + 1)}));
        }

        return $binary_representation;
    }

Answer 1

Why Encryption Doesn't Fit the Goal of Password Storage

Cryptographic hashing and encryption have two different purposes. Hashing is not reversible -- given a hash, you can't determine what made it except by testing to see if a source input matches the hash output. When used for passwords, this becomes a validation only question: "does this password (mixed with this salt) match?" With passwords, the goal is to prevent somebody with full access to the database from recovering passwords.

Encrypting passwords handles that goal poorly. To verify a password, one must have the encryption key. That means one is also able to determine the source password from looking at the encrypted version of the password. In the event of a compromise, having those encrypted fields and the key will result in the disclosure of all passwords regardless of how strong they were.

Good Resources

https://security.stackexchange.com/questions/211/how-to-securely-hash-passwords

The Rules of Crypto

You'll hear it a lot around here: don't roll your own. The problem is that the field has changed too much for "common sense" to really be something that's working on our favor. Encryption and password issues have evolved relatively fast on a human-life scale of time. Where not everybody is following that, a lot of varying ideas exist about how to solve these problems and many of them have been proven wrong.

If you're not current, the proper approach is to research the issue you're trying to solve rather than the method you're trying to solve it with.

Answer 2

To summarize what you are doing here:

You use AES in ECB-mode, with some unknown key, to encrypt a password.

You also try to use a random initialization vector as a salt for this encryption.

I see these problems:

• Usually, you don't want to encrypt a password, but hash it instead. See the recent blog article about this topic, which links to relevant questions.

  If this password is for login verification only, there is no need to store it encrypted, instead hash it. Only store the password itself if you need it for login somewhere else. (But this can open another can of worms, so think about using something else here.)

• ECB-mode, which you are using, doesn't have an initialization vector. So your code will actually generate a zero-length random string (or a longer one, which will not be used).

  Encrypting the same password twice will result in the same ciphertext both times. Don't ever use ECB-mode (if you are not sure it is the right thing to use for some reason), use CBC-mode or CTR instead. Then your initialization vector generation actually will do something (see next point).

• You generate a random initialization vector on decryption. This doesn't hit you now, as it isn't actually used (see previous point), but if you are using it, you actually have to store it together with the encrypted data and retrieve it before decrypting, otherwise the first block (16 bytes) of the decrypted plaintext (for CBC) will be garbage.

And a non-security-relevant remark:

• You don't need the hexadecimal encoding, if you define your database column with a binary type (BINARY or VARBINARY) instead of a text type. This will safe some storage space (and a tiny bit of processing time).