proper PHP mcrypt encryption methods?

Question

Ok, I have tried to create my own encryption/decryption methods using PHP mcrypt, and when I posted them a while back some called them "trash". They were mentioning things about "Initialization Vectors" and such. Basically, how can I make these cryptography methods better:

function encrypt($key, $data){
    $encrypted_data = mcrypt_cbc(MCRYPT_RIJNDAEL_192, $key, $data, MCRYPT_ENCRYPT);
    return base64_encode($encrypted_data);
}

function decrypt($key, $encryptedData){
    $dec = base64_decode($encryptedData);
    $decrypt = mcrypt_cbc(MCRYPT_RIJNDAEL_192, $key, $dec, MCRYPT_DECRYPT);
    return trim($decrypt);
}

I want these to work the best they can except I am a duck in a brand new world when it comes to mcrypt, any suggestions are welcome, thanks!

gen_Eric · Accepted Answer · 2012-10-24T20:18:30.120

3

Here is a snippet of the mcrypt functions I use. They use mcrypt_generic and mdecrypt_generic, which should be used according to the PHP manual.

function encrypt($key, $data){
    $b = mcrypt_get_block_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_CBC);
    $enc = mcrypt_module_open(MCRYPT_RIJNDAEL_256, '', MCRYPT_MODE_CBC, '');
    $iv = mcrypt_create_iv(mcrypt_enc_get_iv_size($enc), MCRYPT_DEV_URANDOM);
    mcrypt_generic_init($enc, md5($key), $iv);

    // PKCS7 Padding from: https://gist.github.com/1077723
    $dataPad = $b-(strlen($data)%$b);
    $data .= str_repeat(chr($dataPad), $dataPad);

    $encrypted_data = mcrypt_generic($enc, $data);

    mcrypt_generic_deinit($enc);
    mcrypt_module_close($enc);

    return array(
        'data' => base64_encode($encrypted_data),
        'iv' => base64_encode($iv)
    );
}

function decrypt($key, $iv, $encryptedData){
    $iv = base64_decode($iv);
    $enc = mcrypt_module_open(MCRYPT_RIJNDAEL_256, '', MCRYPT_MODE_CBC, '');
    mcrypt_generic_init($enc, md5($key), $iv);

    $encryptedData = base64_decode($encryptedData);
    $data = mdecrypt_generic($enc, $encryptedData);
    mcrypt_generic_deinit($enc);
    mcrypt_module_close($enc);

    // PKCS7 Padding from: https://gist.github.com/1077723
    $dataPad = ord($data[strlen($data)-1]);

    return substr($data, 0, -$dataPad);
}

I don't know much about mcrypt either, so I just kinda hacked these together. I md5 the key so it's always 32 characters (the max key length), and I randomly calculate an "Initialization Vector".

Using PKCS7 Padding is better because you can have strings that end in white space (as trim would remove that), also the encryption is more efficient when the string is a certain length.

I'm using AES 256 (MCRYPT_RIJNDAEL_256) here, but AES 192 (MCRYPT_RIJNDAEL_192) would work too.

Demo: http://ideone.com/WA5Tk

edited Oct 24 '12 at 20:18

answered Sep 16 '11 at 18:27

gen_Eric

223,194
41
299
337

I'm just wondering if you've heard any feedback about the strength of these functions. I think they look good, might even use them myself... but this is a bit above me, to be honest... – Shackrock Nov 16 '11 at 23:59
I wrote these myself after searching around and finding other (similar) functions. They seem strong, not sure how strong they are, to be honest. – gen_Eric Nov 17 '11 at 04:29
So we need to store the IV to decrypt the data? – xendi Mar 17 '14 at 23:28
@xendi: Yes, the IV is needed in order to decrypt the data. – gen_Eric Mar 17 '14 at 23:29
Shouldn't we be using something better than md5() to hash the key? Perhapse a better hashing algo with a salt? – xendi Mar 18 '14 at 05:23
AES256 will only allow a key size of 32? If that is the case we have other 32char options here: http://us2.php.net/manual/en/function.hash.php . I was reading and some have possible attacks. What about using each algo that produces 32 and hash the rusult with the previous result, effectively hashing 7 times? – xendi Mar 18 '14 at 05:47
Also, I don't understand why you didn't have to base64_decode they key in the decrypt function. Also, I don't understand why we have to base64encode anything at all. – xendi Mar 18 '14 at 06:12
@xendi: In the encode function, both `mcrypt_generic` and `mcrypt_create_iv` output binary data. The base64 is only there because of that. If you want to remove that and store it as binary data, that's fine. – gen_Eric Mar 18 '14 at 14:13
@hexalys: I'm not really sure. I don't know much about encryption. – gen_Eric Jan 26 '15 at 20:58

Michael Berkowski · Answer 2 · 2011-09-16T19:13:21.793

You can create an iv with mcrypt_create_iv(), using the appropriate size for your encryption mode.

$size = mcrypt_get_iv_size(MCRYPT_RIJNDAEL_192, MCRYPT_MODE_CBC);
$iv = mcrypt_create_iv($size, MCRYPT_DEV_RANDOM);

Then pass it to mcrypt_cbc() as the optional 5th parameter. The only changes I've made here to your original functions are to pass in $iv:

function encrypt($key, $data, $iv){
    $encrypted_data = mcrypt_cbc(MCRYPT_RIJNDAEL_192, $key, $data, MCRYPT_ENCRYPT, $iv);
    return base64_encode($encrypted_data);
}

function decrypt($key, $encryptedData, $iv){
    $dec = base64_decode($encryptedData);
    $decrypt = mcrypt_cbc(MCRYPT_RIJNDAEL_192, $key, $dec, MCRYPT_DECRYPT, $iv);
    return trim($decrypt);
}

In the encryption he's using `MCRYPT_RIJNDAEL_192`, so make sure you use that in `mcrypt_get_iv_size` to make sure the Initialization Vector is the right length. — gen_Eric, Sep 16 '11 at 18:50

proper PHP mcrypt encryption methods?

2 Answers2

Linked