FileCipher follows a "Decrypt then Verify MAC" Construction

[paragonie-scott]

https://github.com/zendframework/zend-crypt/blob/4c3472d67764e64f98e96651865a9c9c2e71eecd/src/FileCipher.php#L333-L346

Decrypt-then-Verify violates the cryptographic doom principle.

if you have to perform any cryptographic operation before verifying the MAC on a message you’ve received, it will somehow inevitably lead to doom

---

I maintain a libsodium wrapper called Halite, and file encryption/decryption is one of the features I sought to provide.

In Halite's implementation, the first thing the code does is create an array of HMACs for each 1MB chunk while recalculating the HMAC of the ciphertext. Then it compares the MAC stored at the end of the file with the one we recalculated.

Upon HMAC verification, Halite will verify each ciphertext chunk with the HMAC stored in memory before proceeding to attempt to decrypt that chunk. It does this to prevent attackers from exploiting race conditions (TOCTOU) to bypass MAC validation to facilitate chosen-ciphertext attacks.

In another vein, Defuse Security's PHP encryption library is going to adopt a similar strategy, using OpenSSL for AES-256-CTR and HMAC-SHA256 instead of libsodium (XSalsa20 + HMAC-SHA-512/256). This implementation is not stable yet, however, and might change before 2.0.0 is released.

I don't have any PoC to show for this issue, and it's likely to be a very corner-case in terms of exploitability, but then again I could be wrong. Attacks only get better. :)

(Just in case anyone complains: If I thought this was exploitable, I would have emailed it instead of posting it here.)

[ezimuel]

@paragonie-scott in the FileCipher class we divide a file into blocks using a buffer of data (1Mb by default); for each block we provide a encrypt-then-authenticate (using AES + HMAC-SHA256). The authentication is built using a chain of HMAC for all the encrypted block, the HMAC of a previous block is concatenated to the input of the next one.

The order of the decrypt and authentication instructions reported here is not relevant, because we don't use the decrypted data in the authentication mechanism, only the encrypted $data. We can move the authentication line before the decryption, it's the same code.

That said, I don't think this code violates the The Cryptographic Doom Principle.

[paragonie-scott]

Question: What are you doing to the IV after each call to $this->cipher->encrypt()?

We can move the authentication line before the decryption, it's the same code.

Okay, I recommend doing that. :)

[ezimuel]

@paragonie-scott because we are using the CBC mode, I take the last $saltSize and I use it as initial IV of the next block iteration. Basically, it's like to use the CBC with original IV on the entire file size. I did this to support encryption/decryption of big file.

I'll move the authentication line before the encryption, so will be more clear that we are following best practices, thanks!