2

I need to encrypt some data and have it decrypted on a later point in time. The data is tied to specific users. I've gathered two possible solutions...

1: The first one is derived from the official docs (example #1 @ http://php.net/manual/en/function.mcrypt-encrypt.php):

function encrypt($toEncrypt)
{
    global $key;
    $iv_size = mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_CBC);
    $iv = mcrypt_create_iv($iv_size, MCRYPT_RAND);
    return base64_encode($iv . mcrypt_encrypt(MCRYPT_RIJNDAEL_256, $key, $toEncrypt, MCRYPT_MODE_CBC, $iv));
}

function decrypt($toDecrypt)
{
    global $key;
    $iv_size = mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_CBC);
    $toDecrypt = base64_decode($toDecrypt);
    return rtrim(mcrypt_decrypt(MCRYPT_RIJNDAEL_256, $key, substr($toDecrypt, $iv_size), MCRYPT_MODE_CBC, substr($toDecrypt, 0, $iv_size)));
}

The key is generated once using:

echo bin2hex(openssl_random_pseudo_bytes(mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_CBC)))

And then later referred to as this:

$key = pack('H*', [result of above]);

1.1: I've noticed that the encrypted result always ends in two equal signs ('=='). Why? - Using bin2hex() and hex2bin() in encrypt() and decrypt() instead of base64_encode()/base64_decode() respectively does not yield these results.

1.2: Will using bin2hex()/hex2bin() have any consequence on the outcome (other than length)?

1.3: There seems to be some discussion whether or not to call a trim-function on the return result when decrypting (this applies to the solution below as well). Why would this be necessary?


2: Second solution comes from here, Stackoverflow (Simplest two-way encryption using PHP):

function encrypt($key, $toEncrypt)
{
    return base64_encode(mcrypt_encrypt(MCRYPT_RIJNDAEL_256, md5($key), $toEncrypt, MCRYPT_MODE_CBC, md5(md5($key))));
}

function decrypt($key, $toDecrypt)
{
    return rtrim(mcrypt_decrypt(MCRYPT_RIJNDAEL_256, md5($key), base64_decode($toDecrypt), MCRYPT_MODE_CBC, md5(md5($key))), "\0");
}

I'm aware that both approaches to the key handling is interchangeable, I purposely made them different in that respect in order to highlight possible solutions, please feel free to mix and match.

Personally I feel that the first one offers tighter security since both key and initialization vector is properly randomized. The second solution however, does offer some form of non-predictability since the key is unique for each piece of encrypted data (even though it suffers under the weak randomization of md5()). The key could for example be the user's name.

3: So, which one is preferable? I'm slightly in the dark since the Stackoverflow answer got a whopping 105 votes. Other thoughts, tips?

4: Bonus question!: I'm not incredibly brainy on server security aspects, but obviously gaining access to the PHP files would expose the key, which as a direct result, would render the encryption useless, assuming the attacker also has access to the DB. Is there any way to obscure the key?

Thank you for reading and have a nice day!

EDIT: All things considered, this seems to be my best bet:

function encrypt($toEncrypt)
{
    global $key;
    $iv_size = mcrypt_get_iv_size(MCRYPT_RIJNDAEL_128, MCRYPT_MODE_CBC);
    $iv = mcrypt_create_iv(mcrypt_get_iv_size(MCRYPT_RIJNDAEL_128, MCRYPT_MODE_CBC), MCRYPT_RAND);
    return base64_encode($iv . mcrypt_encrypt(MCRYPT_RIJNDAEL_128, $key, $toEncrypt, MCRYPT_MODE_CBC, $iv));
}

function decrypt($toDecrypt)
{
    global $key;
    $iv_size = mcrypt_get_iv_size(MCRYPT_RIJNDAEL_128, MCRYPT_MODE_CBC);
    $toDecrypt = base64_decode($toDecrypt);
    return rtrim(mcrypt_decrypt(MCRYPT_RIJNDAEL_128, $key, substr($toDecrypt, $iv_size), MCRYPT_MODE_CBC, substr($toDecrypt, 0, $iv_size)));
}

Using a key created once using the following:

bin2hex(openssl_random_pseudo_bytes(32)));
Community
  • 1
  • 1
user2026991
  • 21
  • 1
  • 3
  • I blieve this would be best suited for http://security.stackexchange.com/ – Shaeldon Oct 09 '14 at 08:35
  • @Shaeldon Only if it was substantially rewritten to explain the protocol instead of using code. – Maarten Bodewes Oct 09 '14 at 11:32
  • It was [cross posted there](http://security.stackexchange.com/questions/69259/two-takes-on-php-two-way-encryption-which-one-is-preferable). @user2026991, cross posting is against site policy. – mikeazo Oct 09 '14 at 19:53

4 Answers4

2

Q1: Choose this!

Disclosure: I (re-)wrote the mcrypt_encrypt code sample. So I opt for 1.

Personally I would not recommend to use MCRYPT_RIJNDAEL_256. You use AES-256 by using a key with a key size of 32 bytes (256 bit) for the MCRYPT_RIJNDAEL_128 algorithm, not by selecting a Rijndael with a block size of 256. I explicitly rewrote the sample to remove MCRYPT_RIJNDAEL_256 – among other mistakes – and put in comments why you should use MCRYPT_RIJNDAEL_128 instead.

Q 1.1: Padding byte for base64

= is a padding character for base 64 encoding. Base64 encodes 3 bytes into 4 characters. To have a number of characters that is an exact multiple of 4 they use these padding bytes, if required.

Q1.2: Will using bin2hex()/hex2bin() have any consequence on the outcome (other than length)?

No, as both hex and base64 are deterministic and fully reversible.

Q1.3: On rtrim

The same goes for the rtrim. This is required as PHP's mcrypt uses the non-standard zero padding, up to the block size (it fills the plaintext with 00 valued bytes at the right). This is fine for ASCII & UTF-8 strings where the 00 byte is not in the range of printable characters, but you may want to look further if you want to encrypt binary data. There are examples of PKCS#7 padding in the comments section of mcrypt_encrypt. Minor note: rtrim may only work for some languages such as PHP, other implementations may leave trailing 00 characters as 00 is not considered white space.

Q2: Disqualification

The other SO answer uses MD5 for password derivation and MD5 over the password for IV calculation. This fully disqualifies it as a good answer. If you have a password instead of a key, please check this Q/A.

And it doesn't use AES either, choosing to opt for MCRYPT_RIJNDAEL_256.

Q3: On the votes

As long as SO community keeps voting on answers that seem to work for a certain language/configuration instead of voting on answers that are cryptographically secure, you will find absolute trap like the answer in Q2. Unfortunately, most people that come here are not cryptographers; the other answer would be absolutely smitten on crypto.stackexchange.com.

Note that just yesterday I had to explain to somebody on SO why it is not possibly to decrypt MCRYPT_RIJNDAEL_256 using CCCrypt on iOS because only AES is available.

Q4: Obfuscation

You can obfuscate the key, but not much else if you store an AES key in software or configuration file.

Either you need to use a public key (e.g. RSA) and hybrid cryptography, or you need to store the key somewhere safe such as a HSM or smart card. Key management is a complex part of crypto, possibly the most complex part.

Community
  • 1
  • 1
Maarten Bodewes
  • 90,524
  • 13
  • 150
  • 263
2

The main difference between the two code samples is that the first one generates a random initialization vector (IV) for each message, while the second one always uses a fixed IV derived from the key.

If you never encrypt more than one message with the same key, both methods are OK. However, encrypting multiple messages with the same key and IV is dangerous, so you should never use the second code sample to encrypt more than one message with the same key.


Another difference is that the first code sample passes the key directly to the block cipher (Rijndael), whereas the second one first runs it through md5(), apparently in a weak attempt to use it as a key derivation function.

If the key is already a random bitstring (of suitable length), like your sample key generation code would produce, there's no need to run it through md5(). If, instead, it's something like a user-provided password, there might be some advantage to hashing it — but in that case, you really ought to use a proper key derivation function like PBKDF2 instead, e.g. like this:

$cipher = MCRYPT_RIJNDAEL_128;  // = AES-256
$mode   = MCRYPT_MODE_CBC;
$keylen = mcrypt_get_key_size( $cipher, $mode );

$salt   = mcrypt_create_iv( $keylen, MCRYPT_DEV_URANDOM );
$iterations = 10000;  // higher = slower; make this as high as you can tolerate

$key = hash_pbkdf2( 'sha256', $password, $salt, $iterations, $keylen, true );

Note that the correct $salt and $iterations values will be needed to reconstruct the key from the password for decryption, so remember to store them somewhere, e.g. by prepending them to the ciphertext. The length of the salt doesn't matter much, as long as it's not very short; making it equal to the key length is a safe enough choice.

(Incidentally, this is also a pretty good way to hash a password to verify its correctness. Obviously, you shouldn't use the same $key value for both encryption and password verification, but you could safely store, say, hash( 'sha256', $key, true ) alongside the ciphertext to let you verify that the password / key is correct.)


A few other issues I see with the two code snippets:

  • Both snippets use MCRYPT_RIJNDAEL_256, which is, apparently, not AES-256, but rather the non-standard Rijndael-256/256 variant, with a 256-bit block size (and key size). It's probably secure, but the 256-bit-block-size variants of Rijndael have receive much less cryptanalytic scrutiny than the 128-bit-block-size ones (which were standardized as AES), so you're taking a slightly higher risk by using them.

    Thus, if you want to play it safe, need to interoperate with other software using standard AES, or just need to be able to tell your boss that, yes, you're using a standard NIST-approved cipher, the you should go with MCRYPT_RIJNDAEL_128 (which, apparently, is what mcrypt calls AES-256) instead.

  • In your key generation code, pack( 'H*', bin2hex( ... ) ) is a no-op: bin2hex() converts the key from binary to hexadecimal, and pack( 'H*', ... ) then does the reverse. Just get rid of both functions.

    Also, you're generating a key, not an IV, so you should use mcrypt_get_key_size(), not mcrypt_get_iv_size(). As it happens, for MCRYPT_RIJNDAEL_256 there's no difference (since both the IV size and the key size are 32 bytes = 256 bits), but for MCRYPT_RIJNDAEL_128 (and many other ciphers) there is.

  • As owlstead notes, mcrypt's implementation of CBC mode apparently uses a non-standard zero-padding scheme. You second code sample correctly removes the padding with rtrim( $msg, "\0" ); the first one just calls rtrim( $msg ), which will also trim any whitespace off the end of the message.

    Also, obviously, this zero-padding scheme won't work properly if your data can legitimately contain zero bytes at the end. You could instead switch to some other cipher mode, like MCRYPT_MODE_CFB or MCRYPT_MODE_OFB, which do not require any padding. (Out of those two, I would generally recommend CFB, since accidental IV reuse is very bad for OFB. It's not good for CFB or CBC either, but their failure mode is much less catastrophic.)

Community
  • 1
  • 1
Ilmari Karonen
  • 49,047
  • 9
  • 93
  • 153
  • Hmm, yeah, I guess I should mail the PHP maintainers again to get that `rtrim` feature fixed. I'm not a hugely experienced PHP dev., but I hereby advice to use `rtrim( $msg, "\0" )` :) (+1 of course) – Maarten Bodewes Oct 09 '14 at 15:38
2

First of all I apologize for the length of this answer.

I just came across this thread and I hope that this class may be of help to anyone reading this thread looking for an answer and source code they can use.

Description:

This class will first take the supplied encryption key and run it through the PBKDF2 implementation using the SHA-512 algorithm at 1000 iterations.

When encrypting data this class will compress the data and compute an md5 digest of the compressed data before encryption. It will also calculate the length of the data after compression. These calculated values are then encrypted with with the compressed data and the IV is prepended to the encrypted output.

A new IV is generated using dev/urandom before each encryption operation. If the script is running on a Windows machine and the PHP version is less than 5.3, the class will use MCRYPT_RAND to generate an IV.

Depending on if parameter $raw_output is true or false, the encryption method will return lowercase hexit by default or raw binary of the encrypted data.

Decryption will reverse the encryption process and check that the computed md5 digest is equal to the stored md5 digest that was encrypted with the data. If the hashes are not the same, the decryption method will return false. It will also use the stored length of the compressed data to ensure all padding is removed before decompression.

This class uses Rijndael 128 in CBC mode.

This class will work cross platform and has been tested on PHP 5.2, 5.3, 5.4, 5.5 and 5.6


File: AesEncryption.php

<?php

/**
 * This file contains the class AesEncryption
 *
 * AesEncryption can safely encrypt and decrypt plain or binary data and
 * uses verification to ensure decryption was successful.
 *
 * PHP version 5
 *
 * LICENSE: This source file is subject to version 2.0 of the Apache license
 * that is available through the world-wide-web at the following URI:
 * https://www.apache.org/licenses/LICENSE-2.0.html.
 *
 * @author     Michael Bush <michael(.)bush(@)hotmail(.)co(.)uk>
 * @license    https://www.apache.org/licenses/LICENSE-2.0.html Apache 2.0
 * @copyright  2015 Michael Bush
 * @version    1.0.0
 */

/**
 * @version    1.0.0
 */
final class AesEncryption
{
    /**
     * @var string
     */
    private $key;

    /**
     * @var string
     */
    private $iv;

    /**
     * @var resource
     */
    private $mcrypt;

    /**
     * Construct the call optionally providing an encryption key
     *
     * @param string $key
     * @return Encryption
     * @throws RuntimeException if the PHP installation is missing critical requirements
     */
    public function __construct($key = null) {
        if (!extension_loaded ('mcrypt')) {
            throw new RuntimeException('MCrypt library is not availble');
        }
        if (!extension_loaded ('hash')) {
            throw new RuntimeException('Hash library is not availble');
        }
        if (!in_array('rijndael-128', mcrypt_list_algorithms(), true)) {
            throw new RuntimeException('MCrypt library does not contain an implementation of rijndael-128');
        }
        if (!in_array('cbc', mcrypt_list_modes(), true)) {
            throw new RuntimeException('MCrypt library does not support CBC encryption mode');
        }
        $this->mcrypt = mcrypt_module_open('rijndael-128', '', 'cbc', '');
        if(isset($key)) {
            $this->SetKey($key);
        }
    }

    /**
     * @return void
     */
    public function __destruct() {
        if (extension_loaded ('mcrypt')) {
            if (isset($this->mcrypt)) {
                mcrypt_module_close($this->mcrypt);
            }
        }
    }

    /**
     * Set the key to be used for encryption and decryption operations.
     *
     * @param string $key
     * @return void
     */
    public function SetKey($key){
        $this->key = $this->pbkdf2('sha512', $key, hash('sha512', $key, true), 1000, mcrypt_enc_get_key_size($this->mcrypt), true);
    }

    /**
     * Encrypts data
     *
     * @param string $data
     * @param bool $raw_output if false this method will return lowercase hexit, if true this method will return raw binary
     * @return string
     */
    public function Encrypt($data, $raw_output = false) {
        $data = gzcompress($data, 9);
        $hash = md5($data, true);
        $datalen = strlen($data);
        $datalen = pack('N', $datalen);
        $data = $datalen . $hash . $data;
        if (version_compare(PHP_VERSION, '5.3.0', '<=')) {
            if (strtolower (substr (PHP_OS, 0, 3)) == 'win') {
                $this->iv = mcrypt_create_iv(mcrypt_enc_get_iv_size($this->mcrypt), MCRYPT_RAND);
            } else {
                $this->iv = mcrypt_create_iv(mcrypt_enc_get_iv_size($this->mcrypt), MCRYPT_DEV_URANDOM);
            }
        } else {
            $this->iv = mcrypt_create_iv(mcrypt_enc_get_iv_size($this->mcrypt), MCRYPT_DEV_URANDOM);
        }
        $this->initialize();
        $data = mcrypt_generic($this->mcrypt, $data);
        $this->deinitialize();
        $data = $this->iv . $data;
        $this->iv = null;
        if ($raw_output) {
            return $data;
        }
        $data = unpack('H*',$data);
        $data = end($data);
        return $data;
    }

    /**
     * Decrypts data
     *
     * @param string $data
     * @return string This method will return false if an error occurs
     */
    public function Decrypt($data) {
        if (ctype_xdigit($data)) {
            $data = pack ('H*',$data);
        }
        $this->iv = substr ($data, 0, mcrypt_enc_get_iv_size($this->mcrypt));
        $data = substr ($data, mcrypt_enc_get_iv_size($this->mcrypt));
        $this->initialize();
        $data = mdecrypt_generic($this->mcrypt, $data);
        $this->deinitialize();
        $datalen = substr($data, 0, 4);
        $len = unpack('N', $datalen);
        $len = end($len);
        $hash = substr($data, 4, 16);
        $data = substr($data, 20, $len);
        $datahash = md5($data, true);
        if ($this->compare($hash,$datahash)) {
            $data = @gzuncompress($data);
            return $data;
        }
        return false;
    }

    /**
     * Initializes the mcrypt module
     *
     * @return void
     */
    private function initialize() {
        mcrypt_generic_init($this->mcrypt, $this->key, $this->iv);
    }

    /**
     * Deinitializes the mcrypt module and releases memory.
     *
     * @return void
     */
    private function deinitialize() {
        mcrypt_generic_deinit($this->mcrypt);
    }

    /**
     * Implementation of a timing-attack safe string comparison algorithm, it will use hash_equals if it is available
     *
     * @param string $safe
     * @param string $supplied
     * @return bool
     */
    private function compare($safe, $supplied) {
        if (function_exists('hash_equals')) {
            return hash_equals($safe, $supplied);
        }
        $safe .= chr(0x00);
        $supplied .= chr(0x00);
        $safeLen = strlen($safe);
        $suppliedLen = strlen($supplied);
        $result = $safeLen - $suppliedLen;
        for ($i = 0; $i < $suppliedLen; $i++) {
            $result |= (ord($safe[$i % $safeLen]) ^ ord($supplied[$i]));
        }
        return $result === 0;
    }

    /**
     * Implementation of the keyed-hash message authentication code algorithm, it will use hash_hmac if it is available
     *
     * @param string $algo
     * @param string $data
     * @param string $key
     * @param bool $raw_output
     * @return string
     *
     * @bug method returning wrong result for joaat algorithm
     * @id 101275
     * @affects PHP installations without the hash_hmac function but they do have the joaat algorithm
     * @action wont fix
     */
    private function hmac($algo, $data, $key, $raw_output = false) {
        $algo = strtolower ($algo);
        if (function_exists('hash_hmac')) {
            return hash_hmac($algo, $data, $key, $raw_output);
        }
        switch ( $algo ) {
            case 'joaat':
            case 'crc32':
            case 'crc32b':
            case 'adler32':
            case 'fnv132':
            case 'fnv164':
            case 'fnv1a32':
            case 'fnv1a64':
                $block_size = 4;
                break;
            case 'md2':
                $block_size = 16;
                break;
            case 'gost':
            case 'gost-crypto':
            case 'snefru':
            case 'snefru256':
                $block_size = 32;
                break;
            case 'sha384':
            case 'sha512':
            case 'haval256,5':
            case 'haval224,5':
            case 'haval192,5':
            case 'haval160,5':
            case 'haval128,5':
            case 'haval256,4':
            case 'haval224,4':
            case 'haval192,4':
            case 'haval160,4':
            case 'haval128,4':
            case 'haval256,3':
            case 'haval224,3':
            case 'haval192,3':
            case 'haval160,3':
            case 'haval128,3':
                $block_size = 128;
                break;
            default:
                $block_size = 64;
                break;
        }
        if (strlen($key) > $block_size) {
            $key=hash($algo, $key, true);
        } else {
            $key=str_pad($key, $block_size, chr(0x00));
        }
        $ipad=str_repeat(chr(0x36), $block_size);
        $opad=str_repeat(chr(0x5c), $block_size);
        $hmac = hash($algo, ($key^$opad) . hash($algo, ($key^$ipad) . $data, true), $raw_output);
        return $hmac;
    }

    /**
     * Implementation of the pbkdf2 algorithm, it will use hash_pbkdf2 if it is available
     *
     * @param string $algorithm
     * @param string $password
     * @param string $salt
     * @param int $count
     * @param int $key_length
     * @param bool $raw_output
     * @return string
     * @throws RuntimeException if the algorithm is not found
     */
    private function pbkdf2($algorithm, $password, $salt, $count = 1000, $key_length = 0, $raw_output = false) {
        $algorithm = strtolower ($algorithm);
        if (!in_array($algorithm, hash_algos(), true)) {
            throw new RuntimeException('Hash library does not contain an implementation of ' . $algorithm);
        }
        if (function_exists('hash_pbkdf2')) {
            return hash_pbkdf2($algorithm, $password, $salt, $count, $key_length, $raw_output);
        }
        $hash_length = strlen(hash($algorithm, '', true));
        if ($count <= 0) {
            $count = 1000;
        }
        if($key_length <= 0) {
            $key_length = $hash_length * 2;
        }
        $block_count = ceil($key_length / $hash_length);
        $output = '';
        for($i = 1; $i <= $block_count; $i++) {
            $last = $salt . pack('N', $i);
            $last = $xorsum = $this->hmac($algorithm, $last, $password, true);
            for ($j = 1; $j < $count; $j++) {
                $xorsum ^= ($last = $this->hmac($algorithm, $last, $password, true));
            }
            $output .= $xorsum;
        }
        if ($raw_output) {
            return substr($output, 0, $key_length);
        }
        $output = unpack('H*',$output);
        $output = end ($output);
        return substr($output, 0, $key_length);
    }
}

Example usage:

<?php

include 'AesEncryption.php';

$key = 'my secret key';
$string = 'hello world';

try
{
    $aes = new AesEncryption($key); // exception can be thrown here if the class is not supported

    $data = $aes->Encrypt($string, true); // expecting return of a raw byte string
    $decr = $aes->Decrypt($data); // expecting the return of "hello world"
    var_dump ($decr);

    // encrypt something else with a different key
    $aes->SetKey('my other secret key'); // exception can be thrown here if the class is not supported

    $data2 = $aes->Encrypt($string); // return the return of a lowercase hexit string
    $decr = $aes->Decrypt($data2); // expecting the return of "hello world"
    var_dump ($decr);

    // proof that the key was changed
    $decr = $aes->Decrypt($data); // expecting return of Boolean False
    var_dump ($decr);

    // reset the key back
    $aes->SetKey($key); // exception can be thrown here if the class is not supported
    $decr = $aes->Decrypt($data); // expecting hello world
    var_dump ($decr);
}

catch (Exception $e)
{
    print 'Error running AesEncryption class; reason: ' . $e->getMessage ();
}
Michael Bush
  • 126
  • 2
0

1.1 It's just padding. It happens with most input to base64, but not all.

1.2 No difference. Keep with base64, it's standard with encryption.

1.3 I don't see reason why it would be necessary. People sometimes solve problems at wrong places. Instead of fixing the input, they modify the output. Where's this discussion?

  1. Definitely DO NOT use this. You change your key of some legnth to 128 bit MD5. And that is not secure.

  2. Use asymetric encryption, if decrypting is on different machine, or different user.

Marek
  • 7,337
  • 1
  • 22
  • 33