Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

PHP doesn't have a built-in functions for AES (specifically, AES-128) encoding/decoding, so I've had to implement my own, and this is what I have come up to (of course, taken from different many sources, mostly not coded by me).

My question is, are these correct? Not the code itself, but what they do. The correctness of the algorithm. I have tested them, and they apparently do work. But there might be a subtle error that I'm overlooking...


Encoding algorithm:

function aes128_encode($data, $mode)
{
        switch ($mode) {
        case "ECB":
        case "CBC":
            if ($mode === "ECB") {
                $cipher = mcrypt_module_open(MCRYPT_RIJNDAEL_128, '', MCRYPT_MODE_ECB, '');
            } else {
                $cipher = mcrypt_module_open(MCRYPT_RIJNDAEL_128, '', MCRYPT_MODE_CBC, '');
            }

            $iv = mcrypt_create_iv(mcrypt_enc_get_iv_size($cipher), MCRYPT_RAND);
            $key = substr(CIPHERKEY, 0, mcrypt_enc_get_key_size($cipher));

            if (mcrypt_generic_init($cipher, $key, $iv) != 1) {
                $cipherData = mcrypt_generic($cipher, $data);

                mcrypt_generic_deinit($cipher);
                mcrypt_module_close($cipher);

                if ($mode === "ECB") {
                    $sanitizedCipherData = trim(base64_encode($cipherData)); 
                } else {
                    $sanitizedCipherData = trim(base64_encode($iv)."_".base64_encode($cipherData));
                }

                return $sanitizedCipherData;
            } else {
                return false;
            }
            break;

        default:
            return false;
            break;
        }
    }

Decoding algorithm:

function aes128_decode($data, $mode)
{
    switch ($mode) {
    case "ECB":
        $cipher = mcrypt_module_open(MCRYPT_RIJNDAEL_128, '', MCRYPT_MODE_ECB, '');
        $key = substr(CIPHERKEY, 0, mcrypt_enc_get_key_size($cipher));

        // Fake iv to call mcrypt_generic_init
        $iv = mcrypt_create_iv(mcrypt_enc_get_iv_size($cipher), MCRYPT_RAND);
        $cipherData = base64_decode($data);

        if (mcrypt_generic_init($cipher, $key, $iv) != -1) {
            $originalData = mdecrypt_generic($cipher, $cipherData);
            mcrypt_generic_deinit($cipher);
            mcrypt_module_close($cipher);

            return $originalData;
        } else {
            return false;
        }
        break;

    case "CBC":
        $cipher = mcrypt_module_open(MCRYPT_RIJNDAEL_128, '', MCRYPT_MODE_CBC, '');
        $key = substr(CIPHERKEY, 0, mcrypt_enc_get_key_size($cipher));
        $parts = explode("_", $data);
        $iv = base64_decode($parts[0]); 
        $cipherData = base64_decode($parts[1]);

        if (mcrypt_generic_init($cipher, $key, $iv) != -1) {
            $originalData = mdecrypt_generic($cipher, $cipherData);
            mcrypt_generic_deinit($cipher);
            mcrypt_module_close($cipher);

            return $originalData;
        } else {
            return false;
        }
        break;

    default:
        return false;
        break;
    }
}

NOTE: The $mode variable indicates which mode of operation will be used (ECB or CBC). The CIPHERKEY is a constant with a randomized value that should be only known by the admin of the website.

share|improve this question

migrated from stackoverflow.com Oct 17 '11 at 21:00

This question came from our site for professional and enthusiast programmers.

2  
It is SO easy to create a broken implementation of encryption that "some guy on StackOverflow" doesn't realize is broken. Can you make a native call to a proven implementation on your platform? – Eric J. Oct 17 '11 at 18:44
    
Assuming the encryption is happening to store the data securely, I would recommend seeing if your storage platform can manage this task. For example, MySQL offers AES_ENCRYPT() and AES_DECRYPT() methods. – bitsoflogic Oct 21 '11 at 13:41
    
Yes, that's precisely what I did. AES encryption is too complex to just "hope it works". But I'm not too keen on using external libraries. MySQL's AES_ENCRYPT()/AES_DECRYPT() do the job perfectly – federicot Oct 21 '11 at 18:35

Implementing AES alone is very risky, mainly because it is very easy to make a mistake. I strongly recommend you use phpseclib for this.

Also you should not use ECB mode unless you are encrypting only 1 block.

share|improve this answer

As I understand it, the only difference between AES and Rijndael is that AES is restricted to a 128-bit block size and can only use key sizes of 128, 192, and 256 bit. So if you are using Rijndael_128 with one of those three key lengths then you are uses AES.

If you must have something that says AES, then, as Chris Smith said, use phpseclib.

share|improve this answer

PHP may not have a built-in AES Encryption library but free one exist for ECB or for other type you may check that open source project. I strongly suggest to not reinvent the wheel because it can be error prone.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.