3
\$\begingroup\$

I need an encryption system with a super simple API. I'd prefer something as simple as the following. The output should also be browser-friendly and ideally url-friendly.

$encrypted=encrypt(data);
$decrypted=decrypt(encrypted);

I'm not a cryptographer, however I have come up with the following to achieve this.

<?php
declare(strict_types = 1);

class EasyCrypt
{

    // format: easycryptstart_version_(bytes_of_b64bin)_b64bin_easycryptend
    // b64bin: base64urlencode(bin)
    // bin: versionspecific
    // V1 versionspecific: IV+aes128ctr(encryption_key=hkey,csum+inner_length+data+padding)
    // V1 hkey: substr(sha256(key),16); // sha256 is used as a key compressor/expander
    // V1 inner_length: little_endian_uint64(strlen(data))
    // V1 csum: substr(sha256(inner_length+data+padding),14)
    // V1 padding: null-bytes until strlen(csum+inner_length+data+padding) is divisible by 16 bytes (128 bits), (16-(size%16))%16
    // generate secure key: cat /dev/urandom | head --bytes=15 | base64
    private const EASY_ENCRYPTION_KEY = "CHANGEME";

    private const V1_IV_LENGTH = 16;

    private const V1_ENCRYPT_ALGO = 'aes-128-ctr';

    private const V1_HASH_ALGO = 'sha256';

    private const V1_HASH_TRUNCATE_LENGTH = 14;

    public static function encryptEasy(string $data): string
    {
        return self::encrypt($data, self::EASY_ENCRYPTION_KEY);
    }

    public static function decryptEasy(string $data, string &$decryptionError = null): ?string
    {
        return self::decrypt($data, self::EASY_ENCRYPTION_KEY, $decryptionError);
    }

    public static function encrypt(string $data, string $encryption_key): string
    {
        $version = 1;
        $prefix = "easycryptstart_{$version}_";
        $postfix = "_easycryptend";
        $ret = self::encryptV1($data, $encryption_key);
        $ret = self::base64url_encode($ret);
        $ret = $prefix . strlen($ret) . "_" . $ret . $postfix;
        return $ret;
    }

    public static function decrypt(string $data, string $encryption_key, string &$decryptionError = null): ?string
    {
        // only 1 "version" exist thus far
        $version = 1;

        $data = str_replace(array(
            " ",
            "\r",
            "\n",
            "\t"
        ), "", $data);
        $prefix = "easycryptstart_{$version}_";
        $postfix = "_easycryptend";
        $prefixpos = strpos($data, $prefix);
        if (false === $prefixpos) {
            $decryptionError = "prefix not found";
            return null;
        }
        $postfixpos = strpos($data, $postfix, $prefixpos);
        if (false === $postfixpos) {
            $decryptionError = "postfix not found (even tho prefix was found!)";
            return null;
        }
        $data = substr($data, $prefixpos + strlen($prefix), $postfixpos - ($prefixpos + strlen($prefix)));
        $outer_length_end = strpos($data, "_");
        if (false === $outer_length_end) {
            $decryptionError = "corrupted input, outer length end missing!";
            return null;
        }
        $outer_length = substr($data, 0, $outer_length_end);
        $outer_length = filter_var($outer_length, FILTER_VALIDATE_INT);
        if (false === $outer_length) {
            $decryptionError = "corrupt input, outer_length non-int!";
            return null;
        }
        $data = substr($data, $outer_length_end + strlen("_"));
        $dlen = strlen($data);
        if ($dlen < $outer_length) {
            $decryptionError = "corrupt input, outer length header said {$outer_length} bytes, but only {$dlen} bytes available!";
            return null;
        }
        $data = substr($data, 0, $outer_length);
        $data = self::base64url_decode($data);
        return self::decryptV1($data, $encryption_key, $decryptionError);
    }

    private static function decryptV1(string $data, string $encryption_key, string &$decryptionError = null): ?string
    {
        if (strlen($data) < self::V1_IV_LENGTH) {
            $decryptionError = "corrupt input, IV is missing!";
            return null;
        }
        $IV = substr($data, 0, self::V1_IV_LENGTH);
        $data = substr($data, self::V1_IV_LENGTH);
        // now we have the aes128 data..
        if (strlen($data) < 16 || (strlen($data) % 16) !== 0) {
            $decryptionError = "corrupted input, after removing IV, data is not a multiple of 16 bytes!";
            return null;
        }
        $hkey = hash(self::V1_HASH_ALGO, $encryption_key, true);
        $hkey = substr($hkey, 0, 16);
        $data = openssl_decrypt($data, self::V1_ENCRYPT_ALGO, $hkey, OPENSSL_RAW_DATA | OPENSSL_NO_PADDING, $IV);
        if (! is_string($data)) {
            // should never happen
            throw new \RuntimeException("openssl_decrypt failed! wtf!?");
        }
        if (strlen($data) < self::V1_HASH_TRUNCATE_LENGTH) {
            $decryptionError = "corrupt input, after decryption, checksum hash is missing!";
            return null;
        }
        $checksum_supplied_hash = substr($data, 0, self::V1_HASH_TRUNCATE_LENGTH);
        $data = substr($data, self::V1_HASH_TRUNCATE_LENGTH);
        $checksum_calculated_hash = hash(self::V1_HASH_ALGO, $data, true);
        $checksum_calculated_hash = substr($checksum_calculated_hash, 0, self::V1_HASH_TRUNCATE_LENGTH);
        if (! hash_equals($checksum_calculated_hash, $checksum_supplied_hash)) {
            $decryptionError = "checksum mismatch, possibly wrong decryption key?";
            return null;
        }
        $little_endian_uint64_length = 8;
        if (strlen($data) < $little_endian_uint64_length) {
            $decryptionError = "after decryption, inner_length header is missing!";
            return null;
        }
        $little_endian_uint64 = substr($data, 0, $little_endian_uint64_length);
        $little_endian_uint64 = self::from_little_uint64_t($little_endian_uint64);
        $data = substr($data, $little_endian_uint64_length);
        $dlen = strlen($data);
        if ($dlen < $little_endian_uint64) {
            $decryptionError = "inner_length header said {$little_endian_uint64} bytes, but only {$dlen} bytes remaining, and that includes any padding bytes!";
            return null;
        }
        $data = substr($data, 0, $little_endian_uint64);
        return $data;
    }

    private static function encryptV1(string $data, string $encryption_key): string
    {

        // compress/expand the key so we can accept any encryption key length (instead of the 16 bytes key required by aes-128)
        $hkey = hash(self::V1_HASH_ALGO, $encryption_key, true);
        $hkey = substr($hkey, 0, 16);

        $iv = random_bytes(self::V1_IV_LENGTH);
        $inner_length_bytes = self::to_little_uint64_t(strlen($data));
        $ret = $inner_length_bytes;
        $ret .= $data;
        $padding_length = self::V1_HASH_TRUNCATE_LENGTH + strlen($ret);
        $padding_length = (16 - ($padding_length % 16)) % 16;
        $ret .= str_repeat("\x00", $padding_length);
        $csum = hash(self::V1_HASH_ALGO, $ret, true);
        $csum = substr($csum, 0, self::V1_HASH_TRUNCATE_LENGTH);
        $ret = $csum . $ret;
        $str = openssl_encrypt($ret, self::V1_ENCRYPT_ALGO, $hkey, OPENSSL_RAW_DATA | OPENSSL_NO_PADDING, $iv);
        if (! is_string($str)) {
            // should never happen
            throw new \RuntimeException("openssl_encrypt failed! wtf!?");
        }
        $str = $iv . $str;
        return $str;
    }

    private static function to_uint8_t(int $i): string
    {
        return pack('C', $i);
    }

    private static function from_uint8_t(string $i): int
    {
        // ord($i) , i know.
        $arr = unpack("Cuint8_t", $i);
        return $arr['uint8_t'];
    }

    private static function to_little_uint64_t(int $i): string
    {
        return pack('P', $i);
    }

    private static function from_little_uint64_t(string $i): int
    {
        $arr = unpack('Puint64_t', $i);
        return $arr['uint64_t'];
    }

    private static function base64url_encode($data)
    {
        return rtrim(strtr(base64_encode($data), '+/', '-_'), '=');
    }

    private static function base64url_decode($data)
    {
        return base64_decode(strtr($data, '-_', '+/'));
    }
}

Example usage:

$data = "Hello World!"; // . random_bytes(10*1024*1024);
$decryptionError = "";
$encrypted = EasyCrypt::encryptEasy($data);
$decrypted = EasyCrypt::decryptEasy($encrypted, $decryptionError);
$pretty = [
    "data to encrypt" => $data,
    "encrypted" => $encrypted,
    "decrypted successfully" => $decrypted === $data,
    "decryption error" => $decryptionError
];

var_export($pretty);
$ php EasyCrypt.php  | more
array (
  'data to encrypt' => 'Hello World!',
  'encrypted' => 'easycryptstart_1_86_LvBV6n3yLY-sH3vdhjzIZmbAm56s7VEZ9ah0wh5z4p9-rhJBaIDmOQYaWOTuRSei7yfmXJ6HTbqgvBaQJsQdMg_easycryptend',
  'decrypted successfully' => true,
  'decryption error' => '',
)
\$\endgroup\$
3
  • 2
    \$\begingroup\$ For a widely used, well studied secure implementation, libsodium is a good option. It is available built-in to PHP since version 7.2. \$\endgroup\$
    – jpa
    Commented Sep 9, 2020 at 8:03
  • 1
    \$\begingroup\$ "I need a cryptography system! I'm not a cryptographer, but I'm going to make my own." This is basically the definition of a bad idea. Just use an existing crypto system, which has been designed by cryptographers and is actually secure. \$\endgroup\$
    – nick012000
    Commented Sep 9, 2020 at 10:56
  • \$\begingroup\$ @nick012000 you certainly have a point; but i'm also curious as to what mistakes i did, so having the implementation reviewed may be very educational for me \$\endgroup\$
    – hanshenrik
    Commented Sep 9, 2020 at 12:18

1 Answer 1

4
\$\begingroup\$

Remove encryptEasy and decryptEasy; both functions are insecure and only serve to confuse possible users.

Your current scheme doesn't use a MAC, not a checksum which means the ciphertext is unauthenticated. This is a bad idea. It is recommended to use a HMAC function for authentication, in order to also avoid the risk of hash extension attacks while keeping the output range large.

Encrypt-then-MAC is recommended, while your current approach is hash-then-encrypt which is theoretically vulnerable to CTR bitflipping attacks.

Your padding implementation is incorrect if the data already contains null bytes (Use PKCS#7).

Instead of using a reference param for exceptions, just throw exceptions directly.

\$\endgroup\$
2
  • \$\begingroup\$ thanks for checking it! order to also avoid the risk of hash extension attacks - i don't think length extension attack is feasible on sha256|112 even if the attacker had the encryption key? (for the same reason length extension isn't feasible on SHA2-384) theoretically vulnerable to CTR bitflipping attacks - i would imagine any bitflips would break the sha256|112 hash ending up with checksum mismatch, possibly wrong decryption key? regardless of CTR vulnerabilities? \$\endgroup\$
    – hanshenrik
    Commented Sep 9, 2020 at 6:54
  • \$\begingroup\$ Nowadays you should use authenticated encryption instead of HMAC, if just because starting users are going to forget to e.g. MAC the IV as well. \$\endgroup\$ Commented Oct 18, 2020 at 0:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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