What's wrong with this PHP4 class to do AES encryption?

Question

When I decrypt something encrypted with this function, the decrypted version doesn't equal the original.

 class AES256encryption {

    var $secret = '';
    var $cipher_key = '';

    function AES256encryption($secret='') {
        if (empty($secret)) {
            global $secret;         
            if (empty($secret)) {
                $secret = "some random secret string";
            }
        }
        $this->secret = $secret;
    }

    function gen_cipher() {
        if (empty($this->cipher_key)) {
            $this->cipher_key = substr(sha1($this->secret),0,20);
        }
    }
    function mciv() {
        return mcrypt_create_iv(mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_CBC), MCRYPT_RAND);
    }
    function encrypt($text) {
        $this->gen_cipher();
        return trim(base64_encode(mcrypt_encrypt(MCRYPT_RIJNDAEL_256, $this->cipher_key, $text, MCRYPT_MODE_CBC, $this->mciv()))); 
    }
    function decrypt($text) {
        $this->gen_cipher();
        return trim(mcrypt_decrypt(MCRYPT_RIJNDAEL_256, $this->cipher_key, base64_decode($text), MCRYPT_MODE_CBC, $this->mciv())); 
    }
}

Answer 1

Don't create a new IV each time you want to encrypt/decrypt something. You need the same IV at encryption and decryption times. In CBC mode, there is no need to get IV secret as long as it is random at its creation. So your code should be something like:

class AES256encryption {

    var $secret = '';
    var $cipher_key = '';
    var $mciv = NULL;

    function AES256encryption($secret='') {
        if (empty($secret)) {
            global $secret;⋅⋅⋅⋅⋅⋅⋅⋅⋅
            if (empty($secret)) {
                $secret = "some random secret string";
            }
        }
        $this->secret = $secret;
        $this->gen_mciv();
    }   

    function gen_cipher() {
        if (empty($this->cipher_key)) {
            $this->cipher_key = substr(sha1($this->secret),0,20);
        }   
    }   

    function gen_mciv() {
        if(NULL === $this->mciv)
        {
            $this->mciv = mcrypt_create_iv(mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_CBC), MCRYPT_RAND);
        }   
    }   

    function encrypt($text) {
        $this->gen_cipher();
        return trim(base64_encode(mcrypt_encrypt(MCRYPT_RIJNDAEL_256, $this->cipher_key, $text, MCRYPT_MODE_CBC, $this->mciv)));
    }   
    function decrypt($text) {
        $this->gen_cipher();
        return trim(mcrypt_decrypt(MCRYPT_RIJNDAEL_256, $this->cipher_key, base64_decode($text), MCRYPT_MODE_CBC, $this->mciv));
    }   
}   



$ac = new AES256encryption('my secret pass');
$z = $ac->encrypt('test');
var_dump($z);
$u = $ac->decrypt($z);
var_dump($u);

And that seems to work:

mycroft:~ $ php test_aes.php 
string(44) "+KRlfrPp37FfwB4gJXQ67X+8bjbjxEFHjOn55YOgU5o="
string(4) "test"

Please check block cipher modes of operation which resumes how this work.