Is transporting a salt, IV, ciphertext and HMAC in an encrypted token an acceptable practice?

Question

I’m porting over some crypto code from Mcrypt (deprecated) to OpenSSL. I’m following a process that a former employee implemented, which is as follows:

1. Create a random salt for use in a PBKDF2 key derivation function.

2. Create a random IV for symmetric AES-CBC encryption.

3. Encrypt the plaintext using the derived key from step 1 and the IV from step 2.

4. Create an HMAC of the token (concatenated IV + salt + ciphertext).

5. Append the HMAC from step 4 to the token.

Is this process acceptable (if implemented properly), or does it have some structural weakness that leaves it vulnerable to attacks?

---

Ps. Here's my actual PHP code. I realize this is sort of a large chunk of code to digest, but I hope that it may help fill in any relevant details that I may have left out from the high-level description above.

final class SymmetricEncryption
{
    /**
     * @const int
     */
    private const SALT_LENGTH_IN_BYTES = 32;

    /**
     * @const int
     */
    private const IV_LENGTH_IN_BYTES = 16;

    /**
     * @const int
     */
    private const KEY_LENGTH_IN_BYTES = 32;

    /**
     * @const int
     */
    private const HMAC_LENGTH_IN_BYTES = 64;

    /**
     * @const string
     */
    private const SYMMETRIC_ALGORITHM = "aes-256-cbc";

    /**
     * @const string
     */
    private const PBKDF2_ALGORITHM = "sha256";

    /**
     * @const string
     */
    private const HMAC_ALGORITHM = "sha256";

    /**
     * Default number of iterations used by the hash_pbkdf2 function to derive a key
     *
     * @var int
     */
    private $pbkdf2_iterations = 10000;

    /**
     * Symmetric key used to encrypt/decrypt using SYMMETRIC_ALGORITHM.
     * Must be a 32-bit key
     *
     * @var string
     */
    private $symmetric_key;

    public function __construct($symmetric_key, $pbkdf2_iterations = 10000)
    {
        $this->symmetric_key     = $symmetric_key;
        $this->pbkdf2_iterations = $pbkdf2_iterations;
    }

    /**
     * Steps:
     *
     * 1) Compute a random salt for the pbkdf2 key derivation function
     * 2) Create a random IV for the aes encryption
     * 3) Derive a key via pbkdf2
     * 4) Encrypt the plaintext with the derived key from step 3
     * 5) Concatenate the iv, salt, and ciphertext in hexadecimal format.
     * 6) Compute an hmac of the token created in step 5
     * 7) Concatenate the token with the hmac
     *
     * @param $plainText
     * @return string
     */
    public function encrypt($plainText)
    {
        $salt          = openssl_random_pseudo_bytes(self::SALT_LENGTH_IN_BYTES);
        $salt_hex      = bin2hex($salt);
        $iv            = openssl_random_pseudo_bytes(self::IV_LENGTH_IN_BYTES);
        $iv_hex        = bin2hex($iv);
        $key           = $this->computePbkdf2KeyDerivation($this->symmetric_key, $salt);
        $cipher        = openssl_encrypt($plainText, self::SYMMETRIC_ALGORITHM, $key, OPENSSL_RAW_DATA, $iv);
        $token         = $iv_hex . $salt_hex . bin2hex($cipher);
        $hmac_of_token = hash_hmac(self::HMAC_ALGORITHM, $token, $key);

        return $token . $hmac_of_token;
    }

    /**
     * Steps:
     *
     * 1) Extract the iv, salt, cipher, and hmac from the token. Confirm that they are each valid strings.
     * 2) Compute the pbkdf2 key derivation function with the symmetric key and extracted salt (binary)
     * 3) Compute the hmac of the token using the derived key from step 2
     * 4) If the computed hmac matches the expected hmac, decrypt the cipher (binary), otherwise throw an exception.
     *
     * @param $token
     * @return string
     * @throws InvalidTokenException
     */
    public function decrypt($token)
    {
        $iv_length     = self::IV_LENGTH_IN_BYTES * 2;
        $salt_length   = self::SALT_LENGTH_IN_BYTES * 2;
        $cipher_length = strlen($token) - $iv_length - $salt_length - self::HMAC_LENGTH_IN_BYTES;
        //Extract the iv, salt, cipher, and expected hmac from the token
        $iv_hex        = substr($token, 0, $iv_length);
        $salt_hex      = substr($token, $iv_length, $salt_length);
        $cipher_hex    = substr($token, $iv_length + $salt_length, $cipher_length);
        $expected_hmac = substr($token, self::HMAC_LENGTH_IN_BYTES * -1, self::HMAC_LENGTH_IN_BYTES);
        if(!$iv_hex || !$salt_hex || !$cipher_hex || !$expected_hmac) {
            throw new InvalidTokenException("Invalid token");
        }
        //Convert hexadecimal representations of the iv, salt, and cipher into binary
        $iv_binary     = hex2bin($iv_hex);
        $salt_binary   = hex2bin($salt_hex);
        $cipher_binary = hex2bin($cipher_hex);
        //Derive a key given the salt that we found in the string and compute an hmac value
        $derived_key        = $this->computePbkdf2KeyDerivation($this->symmetric_key, $salt_binary);
        $token_without_hmac = substr($token, 0, strlen($token) - self::HMAC_LENGTH_IN_BYTES);
        $computed_hmac      = hash_hmac(self::HMAC_ALGORITHM, $token_without_hmac, $derived_key);
        //If the expected hmac matches the computed hmac decrypt the data
        if(strcmp($expected_hmac, $computed_hmac) === 0) {
            return openssl_decrypt(
                $cipher_binary,
                self::SYMMETRIC_ALGORITHM,
                $derived_key,
                OPENSSL_RAW_DATA,
                $iv_binary
            );
        }

        throw new InvalidTokenException("Invalid hmac");
    }

    /**
     * The key and salt should both be binary strings.
     *
     *
     * @link http://php.net/manual/en/function.hash-pbkdf2.php
     * @param $key
     * @param $salt
     * @return string - binary encoded string
     */
    private function computePbkdf2KeyDerivation($key, $salt) {
        return hash_pbkdf2(
            self::PBKDF2_ALGORITHM,
            $key,
            $salt,
            $this->pbkdf2_iterations,
            self::KEY_LENGTH_IN_BYTES,
            true
        );
    }
}

Answer 1

Is transporting a salt, iv, cipher and hmac in an encrypted token an acceptable practice?

No, but that's not what is taking place. You are transporting a salt, IV, cipher(text) and HMAC over the former, and that's normal practice for encrypt-then-MAC. The salt doesn't really need to be authenticated because any alteration would also change the HMAC key, but that's OK.

---

Code comments

private $pbkdf2_iterations = 10000;

This should be configurable somehow, 10K iterations is generally thought to be on the low side nowadays (this is a moving target, which is why the number of iterations is often stored or derived from information with the password hash or ciphertext).

...
/* Must be a 32-bit key
...
private $symmetric_key;

Not really, must be password < 64 bytes after encoding.

---

Other notes

Hexadecimals are relatively inefficient. Why not first concatenate and then base64 encode?

Using the key for encryption and HMAC is not considered secure in a strictly theoretical sense. If somebody was to use e.g. AES-CBC-MAC instead of HMAC then you may run into trouble.

The salt size is relatively large, 16 bytes should suffice in case you need the space.