Decide whether to zeroize buffer or encrypt it back on tag check failure

[newpavlov]

With one-pass decryption we start to decrypt ciphertext before its integrity was verified. In the case if tag check has failed in the end we need to erase decrypted data. It can be done either by zeroizing the input buffer (as done in ascon-aead, see #659) or by encrypting it back (as done in aes-gcm, see #551).

We probably should look into what is done in other libraries outside of the Rust ecosystem.

[tarcieri]

The main argument for re-encryption is having a consistent behavior across all of our crates, IMO.

Currently all of them except for ascon-aead will return a buffer containing the ciphertext.

[newpavlov]

I agree that we should be consistent. My main concern is that implementing re-encryption may not be always easy. It's far easier to just zeroize output buffer and test it using the test macro.

I guess if ascon-aead will be migrated to re-encryption, we can close this issue.

[tarcieri]

Nearly all of the current implementations are currently two-pass and return the unaltered ciphertext buffer on tag verification failure. No mutation of the buffer is performed at all in that case, which seems desirable.

If we want to add a test for the behavior on decryption failure as I suggested in RustCrypto/traits#1803 and tested for an all-zero output, we would need to modify all of the AEADs to modify the buffer on decryption failure rather than leaving it unchanged, which seems undesirable.

[newpavlov]

What do you think about buffer-to-buffer operations with two-pass modes? Should we always copy ciphertext to the output buffer on tag mismatch?

we would need to modify all of the AEADs to modify the buffer on decryption failure rather than leaving it unchanged

This should be relatively straightforward to implement, so I don't think it's a big issue. Especially considering that we would have to add the ciphertext copying for all two-pass crates. In other words, it will be far easier to write buf.get_out().fill(0) in the tag mismatch branch for all one- and two-pass crates, than to add separate handling of disjoint buffers in two-pass crates and re-encryption of the output buffer in one-pass ones.

I agree with your point (voiced in the Zulip chat) that re-encryption is closer to the ideal of "do not decrypt until ciphertext integrity is verified", though it looks a bit weird to copy ciphertext in the buffer-to-buffer case.

[newpavlov]

As a point of reference, it looks like aws-lc-rs zeroizes output buffer (only the ciphertext part, appended tag stays intact).

[thealtofwar]

If the intent is to guard against chosen-ciphertext attacks when the caller fails to check the tag (a catastrophic failure by the caller), I think it would be best to zeroize the output, because if the original ciphertext is left in the buffer, an attacker could still accomplish the same chosen-ciphertext attack by submitting an incorrect tag with the chosen plaintext.

[tarcieri]

A chosen ciphertext attack requires a decryption oracle.

If a user submits any message as the "ciphertext", what they get out of re-encryption is the original message they submitted, not an encryption/decryption thereof.