Update: It appears that the IV is a static value rather than a per-message random value. This sort of defeats the entire purpose of using CBC mode.

@paragonie-scott Would you be able to provide a mergeable pull request with the changes you are proposing please? Code speaks louder than comments :-)

You won't like my pull request here.

Unfortunately the php-encryption library requires PHP 5.4 while we're still in 5.3.10+ world (ya, shoot us ;-) ). So that's not much of an option.

@joomla/security Some help on this issue would be appreciated. I'm in over my head on this.

I only have very limited knowledge on crypto, not really sure how to help with this :(

You could reference what defuse/php-encryption v1.1 did (or, even better, just use that library outright). It uses mcrypt and supports PHP 5.3.

To prevent a chosen ciphertext attack:

1. Split your key into two keys: $eKey and $aKey. HKDF-SHA-256 can do this safely (Defuse's library includes an implementation).
2. Pad your plaintext using PKCS7.
3. Encrypt your data with $eKey (and preferably with a 16-byte random IV from random_bytes(), provided by random_compat).
4. Take the ciphertext (and the random IV) from step 3, authenticate the entire string with HMAC-SHA-256, and append that to the message.
5. (Optional) base64 or hex encode the output for safe storage / transport.

When decrypting an encrypted message:

1. (Optional) Decode back to raw binary. Validate that nothing failed.
2. Split the master key into $eKey and $aKey again.
3. Recalculate the MAC of the given IV and ciphertext, using $aKey.
4. Compare (using hash_equals()) that the MAC provided with the message is equal to the one you just recalcualted. If it fails, throw an exception. (If the user fails to handle the exception, it must terminate execution.)
5. Decrypt the ciphertext using the IV and $eKey.
6. Remove the PKCS7 padding.

This also resolves the data corruption by trim() issue. But seriously, use the version 1.1 tag of defuse/php-encryption instead of writing your own. When you can support modern PHP (5.6+ or, by the time that comes up, likely 7.x+), you can just change the version requirements for Defuse's library to 1.2 or 2.0.

cc @defuse who can agree/disagree with whether using 1.1 of his library is a good idea or not. (I rewrote it to leverage OpenSSL for 1.2, which introduced the 5.4+ requirement.)

That said, PR incomin'

That said, PR incomin

Awesome, thanks a lot!

I think we can remove the no-code-attached label now ;)

I'm going to close this issue so that the discussion can continue on the Pull Request :)