CipherInputStream for AEAD modes is insecure (GCM, etc.): ciphertext tampering without detection possible

[Valerie (Yu-Ching) Peng]

I view this more as a major vulnerability in BC provider than 
javax.crypto.CipherInputStream class and this should be reported to 
BouncyCastle so they can fix their provider code.

If you tried the same test with SunJCE provider, you will find that none 
of the decrypted data is returned to the caller when the tag doesn't 
match. If the providers weren't modified to not returning any of the 
decrypted data AFTER the tag is verified, the root cause is not fixed 
and you can simply use Cipher to get hold of the decrypted data instead.

Thus, CipherInputStream class ignores AEADBadTagException isn't really 
the problem here, as the some of the decrypted data may have been 
returned to the caller before the tag is verified. Whether 
AEADBadTagException is ignored or not doesn't matter here.

Regards,
Valerie

On 03/03/14 15:19, Philipp Heckel wrote:
> Hello everyone,
>
> I apologize if this is not the right place so ask/report this. Please 
> direct me to the correct place if it isn't.
>
> I recently noticed that that the current 
> javax.crypto.CipherInputStream implementation in OpenJDK 6/7/8 is 
> insecure when AEAD modes are used, e.g. 
> Cipher.getInstance("AES/GCM/NoPadding") with the BC provider. In 
> particular, it is possible to tamper with the ciphertext -- without 
> any exceptions being passed on to the application.
>
> In an example at [1] (see "Test B"), I was able to decrypt an altered 
> ciphertext to "Confirm 900$ pay" (original plaintext was "Confirm 100$ 
> pay") by simply XORing the original ciphertext with 0x08 at index 8. 
> The underlying GCM engine behaves correctly as "Test A" shows (a 
> BadPaddingException "mac check inGCM failed" is thrown), but the 
> j.c.CipherInputStream simply ignores this exception in the close() method:
>
> public void close() throws IOException {
> ...
>   try {
>     cipher.doFinal();
>   }
>   catch (BadPaddingException ex) {
>      /// <<<<<< When GCM tag verification fails, a BadPaddingException 
> is thrown
>      /// <<<<<< The CipherInputStream unfortunately ignores this 
> exception!
>   }
> ...
> }
>
> A discussion about this issue can be found in the Bouncy Castle 
> mailing list [2], or in my blog post [3].
>
> I hope I'm not the only one who thinks that this is a very serious 
> security issue, because it gives developers a false sense of 
> authenticated encryption -- where in fact at this stage AES/GCM is not 
> better than AES/CTR when used with a CipherInputStream. I found this 
> issue by chance, so there are probably many applications out there 
> that are also vulnerable.
>
> Current workaround: Bouncy Castle provides its own 
> org.bouncycastle.crypto.io.CipherInputStream that (unfortunately) is 
> also broken in v1.50, but will be fixed in 1.51. David Hook provided a 
> fix [4] in response to the discussion; the full class can be found at [5].
>
> Best,
> Philipp
>
> [1] 
> https://github.com/binwiederhier/cipherinputstream-aes-gcm/blob/e9759ca71557e5d1da26ae72f6ce5aac918e34b0/src/CipherInputStreamIssuesTests.java#L89
> [2] http://bouncycastle.org/devmailarchive/msg13615.html
> [3] 
> http://blog.philippheckel.com/2014/03/01/cipherinputstream-for-aead-modes-is-broken-in-jdk7-gcm/
> [4] 
> https://github.com/bcgit/bc-java/commit/933119114c96f703d1303a3c77d9ac405091270d
> [5] 
> https://raw.github.com/bcgit/bc-java/933119114c96f703d1303a3c77d9ac405091270d/core/src/main/java/org/bouncycastle/crypto/io/CipherInputStream.java