# Random string + encrypt/decrypt

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;
}

• I don't think “critique my code” is appropriate on this site. “Critique my protocol” could be, although a precise description of your security objectives would be preferable. There's a site for code critique in the Stack Exchange network: Code Review. Do not repost there for the time being; I've flagged a moderator, and if the moderators agree your question is fine for Code Review, they will migrate it there. – Gilles 'SO- stop being evil' Nov 20 '11 at 17:41
• How do you encrypt something with a salt? That doesn't make much sense. Typically you encrypt something with a key. Now, if instead of salt you meant to say key, then your scheme is very insecure. Think about it. You are storing the key and the ciphertext in the same place. If the attacker can access the database, she has both and can decrypt the ciphertext. – mikeazo Nov 20 '11 at 18:14
• Gilles - points 1 and 2 need critiquing, not the code. The code is just there to elaborate on my idea. – StackOverflowNewbie Nov 20 '11 at 22:54
• @mikeazo I believe he is salting like one should do in a hash. It's not a problem with his terminology, just that there's no reason to salt an encryption. – Kevin Nov 21 '11 at 15:08

## 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.

## 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.

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:

• 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).