Are my basic implemetations of AES-CBC and AES-GCM in PHP secure?

Question

I need to encrypt and decrypt some passwords in PHP. Normally I would hash them, but I need to be able to decrypt them again because they're keys for an external service.

I googled around a bit, but the examples I found seem a little bit incomplete, so I cobbled this together from what I could find.

Some questions:

1. Is AES-256-CBC still considered good?
2. The encryption key should be exactly 32 bytes, right? If I understand correctly, any longer and it will be truncated, any shorter and it will be padded with NUL byes. The key length depends on the cipher. The cipher is 256 bits so the key should be 256/8 == 32 bytes, right?

abstract class Encrypt {
    private const CIPHER='AES-256-CBC';

    public static function encryptString(string $data): string {
        $ivLength = openssl_cipher_iv_length(self::CIPHER);
        $iv = openssl_random_pseudo_bytes($ivLength);
        $key = base64_decode(env('ENCRYPTION_KEY'));
        assert(strlen($key) === 32);
        $encrypted = openssl_encrypt($data, self::CIPHER, $key, OPENSSL_RAW_DATA, $iv);
        assert($encrypted !== false);
        return base64_encode($iv . $encrypted);
    }

    public static  function decryptString(string $encrypted) {
        $data = base64_decode($encrypted);
        $ivLength = openssl_cipher_iv_length(self::CIPHER);
        $iv = substr($data, 0, $ivLength);
        $encryptedString = substr($data, $ivLength);
        $key = base64_decode(env('ENCRYPTION_KEY'));
        assert(strlen($key) === 32);
        $decrypted = openssl_decrypt($encryptedString, self::CIPHER, $key, OPENSSL_RAW_DATA, $iv);
        assert($decrypted !== false);
        return $decrypted;
    }
}

The list of ciphers I have available is below. I'm not sure how to pick.

aes-128-cbc aes-128-cbc-hmac-sha1 aes-128-cbc-hmac-sha256 aes-128-ccm aes-128-cfb aes-128-cfb1 aes-128-cfb8 aes-128-ctr aes-128-ecb aes-128-gcm aes-128-ocb aes-128-ofb aes-128-xts aes-192-cbc aes-192-ccm aes-192-cfb aes-192-cfb1 aes-192-cfb8 aes-192-ctr aes-192-ecb aes-192-gcm aes-192-ocb aes-192-ofb aes-256-cbc aes-256-cbc-hmac-sha1 aes-256-cbc-hmac-sha256 aes-256-ccm aes-256-cfb aes-256-cfb1 aes-256-cfb8 aes-256-ctr aes-256-ecb aes-256-gcm aes-256-ocb aes-256-ofb aes-256-xts aria-128-cbc aria-128-ccm aria-128-cfb aria-128-cfb1 aria-128-cfb8 aria-128-ctr aria-128-ecb aria-128-gcm aria-128-ofb aria-192-cbc aria-192-ccm aria-192-cfb aria-192-cfb1 aria-192-cfb8 aria-192-ctr aria-192-ecb aria-192-gcm aria-192-ofb aria-256-cbc

---

Here's my revised class that uses GCM:

abstract class Encrypt {
    private const CIPHER = 'aes-256-gcm';
    private const TAG_LEN = 16;

    private static function byteLen(string $data): int {
        return mb_strlen($data, '8bit');
    }

    public static function encryptString(string $data): string {
        $key = base64_decode(env('ENCRYPTION_KEY'));
        assert($key !== false && self::byteLen($key) === 32);

        $iv_length = openssl_cipher_iv_length(self::CIPHER);
        assert($iv_length !== false);
        $iv = openssl_random_pseudo_bytes($iv_length, $strong);
        assert($strong);
        $encrypted = openssl_encrypt($data, self::CIPHER, $key, OPENSSL_RAW_DATA, $iv, $tag);
        assert($encrypted !== false);
        assert(self::byteLen($tag) === self::TAG_LEN);
        return base64_encode($iv . $tag . $encrypted);
    }

    public static function decryptString(string $base64_data) {
        $key = base64_decode(env('ENCRYPTION_KEY'));
        assert($key !== false && self::byteLen($key) === 32);

        $mix = base64_decode($base64_data);
        assert($mix !== false);
        $iv_length = openssl_cipher_iv_length(self::CIPHER);
        assert($iv_length !== false);

        $iv = substr($mix, 0, $iv_length);
        $tag = substr($mix, $iv_length, self::TAG_LEN);
        $encrypted = substr($mix, $iv_length + self::TAG_LEN);

        $data = openssl_decrypt($encrypted, self::CIPHER, $key, OPENSSL_RAW_DATA, $iv, $tag);
        assert($data !== false);

        return $data;
    }
}

Answer 1

Using CBC mode with padding (which is the default) can make you vulnerable to padding oracle attacks, if an attacker is able to test different ciphertexts and get feedback on whether the padding was correct. It's generally risky to rely on unauthenticated encryption. You should use a mode like Galois/Counter Mode (GCM). Even better: Avoid OpenSSL calls in favor of a modern high-level library like libsodium which is bundled with PHP since version 7.2.0.

Also note that strlen doesn't necessarily count bytes depending on your PHP version and configuration. In PHP prior to version 8.0.0, it's possible to override the function and make it count characters rather than bytes, which can lead to unexpected results if PHP (erroneously) detects multiple bytes as a single multibyte character. A safer choice is mb_strlen($str, '8bit').

As to your second question: Yes, the key for AES-256 must be exactly 32 bytes long.