Page 1 of 1
Please critique my use of mcrypt
Posted: Tue Feb 07, 2012 5:38 pm
by ThePianoGuy
Hi everyone,
I've been sifting through this forum trying to learn as much as I can about how to correctly use mcrypt. I'd appreciate comments on what I've got so far:
Code: Select all
function encrypt($decrypted, $password) {
$key = md5($password);
$decrypted = rtrim($decrypted, "\0\4");
$hash = md5($decrypted);
$iv = mcrypt_create_iv(mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_CBC), MCRYPT_RAND);
$encrypted = trim(base64_encode(mcrypt_encrypt(MCRYPT_RIJNDAEL_256, $key, $decrypted, MCRYPT_MODE_CBC, $iv)));
return $hash . "\n----------\n" . base64_encode($iv) . "\n----------\n" . $encrypted;
}
function decrypt($encrypted, $password) {
$key = md5($password);
list($hash, $iv, $encrypted) = explode("\n----------\n", $encrypted, 3);
$decrypted = rtrim(mcrypt_decrypt(MCRYPT_RIJNDAEL_256, $key, base64_decode($encrypted), MCRYPT_MODE_CBC, base64_decode($iv)), "\0\4");
if (md5($decrypted) != $hash) return false;
return $decrypted;
}
Questions:
1) I've read that using the password as the key is bad. Is an md5 of the password sufficient?
2) The script and the data are stored in the filesystem of the server. The password is to be memorized and entered by the operator. Should I salt the key, and if so, how and why?
3) Is there any issue with storing the iv or md5 of the data in clear text?
Thanks to anyone who can answer any of my questions or offer advice about my code to make it more secure.
Re: Please critique my use of mcrypt
Posted: Wed Feb 08, 2012 6:30 am
by social_experiment
ThePianoGuy wrote:1) I've read that using the password as the key is bad. Is an md5 of the password sufficient?
Not really; you are still using a value which in the event of an attack will probably be tested first rendering the hashed value useless
ThePianoGuy wrote:2) The script and the data are stored in the filesystem of the server. The password is to be memorized and entered by the operator. Should I salt the key, and if so, how and why?
Yes, but don't simply add a value to the password en then pass it to the function; Make the key a combination of the salt + password, then hash that and use it as the key. You would then store a hash which is made up of two parts; (this should probably be smaller than the keysize of the encryption algorigthm)
ThePianoGuy wrote:3) Is there any issue with storing the iv or md5 of the data in clear text?
Yeah many but the main one is that it's easier to 'read' than a hashed value.
md5 is not a safe hash anymore, if you want to hash, use atleast sha256 or higher.
Re: Please critique my use of mcrypt
Posted: Wed Feb 08, 2012 9:55 am
by ThePianoGuy
Thanks for your reply. Pardon my n00bness. If my data were attacked, the attacker would also have access to the script, and thus the salt. So how would the hash of salt+password be more secure?
So you're saying I should replace the key with hash('sha256', 'asdf324!.#qQ' . $password)?
Re: Please critique my use of mcrypt
Posted: Wed Feb 08, 2012 3:10 pm
by social_experiment
ThePianoGuy wrote:would also have access to the script, and thus the salt.
The hashed key is a combination of the password and a salt; if an attacker gets the salt the password is still missing; for them to retrieve the password they would have to break your hashed value or do a dictionary attack against the password
ThePianoGuy wrote:So you're saying I should replace the key with hash('sha256', 'asdf324!.#qQ' . $password)?
Yes, but make the 'asdf324!.#qQ a random salt value; you could probably make this update after each successful "login"
Re: Please critique my use of mcrypt
Posted: Wed Feb 08, 2012 4:10 pm
by ThePianoGuy
Ok, but if I don't use a salt in preparing the hashed key, the attacker would still have to break the hashed value or do a dictionary attack against the password...no?
Re: Please critique my use of mcrypt
Posted: Thu Feb 09, 2012 12:37 am
by social_experiment
Yes, the salt is merely extra protection against this; storing the password hash makes it vulnerable to rainbow tables;
search.php?keywords=password+hashing
Have a look at some of these urls in the form
Re: Please critique my use of mcrypt
Posted: Thu Feb 09, 2012 8:35 am
by ThePianoGuy
But I'm not storing the password hash.
Re: Please critique my use of mcrypt
Posted: Thu Feb 09, 2012 8:43 am
by social_experiment

my bad; i meant the key
Edit
Thinking about this again you probably don't want to store the key outright but store a salt value; this salt value + the password makes up the key. I'm not sure if this is what i conveyed earlier because i re-read the original post and found that i might have explained poorly
Re: Please critique my use of mcrypt
Posted: Fri Feb 10, 2012 4:18 am
by Mordred
A few quick notes:
- Salting is basically for adding entropy to weak passwords. If you're not going to allow users to key this encryption, you can just pick a strong password and not deal with salts at all.
- There are
block cipher modes that provide encryption + authentication; I'm not sure if PHP's mcrypt supports them though
- If not, you're better off with encrypting the plaintext, then keeping a HMAC of the encrypted text, then you can check for validity of the data without decrypting it.
- Why are you doing those trims? Seems like a bug to me, they will damage the data.
On your questions:
1) In all human-password-keyed systems you'd use the password as a master key to generate other keys. What you do seems fine, provided that the password is strong enough (i.e. it's entirely under your control)
2) See above about salting. May be useful, may be not.
3) No, there is no need for the IV or the data integrity hash to be secret, but see above for hashing plaintext vs hashing the encrypted text.
Re: Please critique my use of mcrypt
Posted: Fri Feb 10, 2012 12:30 pm
by ThePianoGuy
Thank you for your reply
