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

[Xuelei Fan]

Hi Tim,

Sorry, I should have been cleaner on this point.  What I wanted to
express is that I understood your concerns.  Thanks for the detailed
description and examples.

Thanks,
Xuelei

On 3/10/2014 3:37 PM, Tim Whittington wrote:
> 
> On 10/03/2014, at 1:12 pm, Xuelei Fan <xuelei.fan at oracle.com> wrote:
> 
>> On 3/10/2014 2:31 AM, Tim Whittington wrote:
>>>
>>> On 9/03/2014, at 10:50 pm, Tim Whittington
>>> <jdk-security-dev at whittington.net.nz
>>> <mailto:jdk-security-dev at whittington.net.nz>> wrote:
>>>
>>>>
>>>> On 7/03/2014, at 9:14 am, Philipp Heckel <philipp.heckel at gmail.com
>>>> <mailto:philipp.heckel at gmail.com>> wrote:
>>>>
>>>>> - Using javax.crypto.CipherInputStream with a cipher in GCM mode and
>>>>> the SunJCE provider (JDK8+) is secure, but cannot be used large
>>>>> files, because it will buffer all data until the tag is verified (as
>>>>> defined by the GCM spec) [1]
>>>>
>>>> This (the part about it being secure) is not correct - when using
>>>> javax.crypto.CipherInputStream with a cipher in GCM mode and the
>>>> SunJCE provider (JDK8+) any tampering with the ciphertext will
>>>> silently treat the result as a 0 byte authenticated stream.
>>>>
>>>
>>> Sorry, I should have been clearer here - this problem occurs with any
>>> provider (and any AE mode) not just the SunJCE GCM implementation.
>>>
>> It only happens before the authentication tag get checked.  If the authN
>> tag get read, the tampering can be detected.
>>
>> Your concerns happen to any mode, even CBC mode.  Better to make an
>> improvement here if possible.
> 
> I don’t quite understand what you’re saying here - what I’m saying is that the auth tag is read, the tampering is detected, and then CipherInputStream suppresses the AEADBadTagException, resulting in the reader of the stream interpreting it as a valid (but empty) ciphertext.
> 
> i.e. the code snippet below should print:
> 
> Via CipherInputStream: ciphertext is invalid
> Via Cipher: ciphertext is invalid
> 
> but instead it prints:
> 
> Via CipherInputStream: ciphertext is valid and -1 bytes long
> Via Cipher: ciphertext is invalid
> 
> If you repeat the test with a 0 byte plaintext, and without tampering with the ciphertext, it will print:
> 
> Via CipherInputStream: ciphertext is valid and -1 bytes long
> Via Cipher: ciphertext is valid and 0 bytes long
> 
> Notice that when using CipherInputStream, a tampered ciphertext and a 0 byte valid ciphertext produce the same result, while Cipher behaves correctly by failing the auth on the tampered one.
> This behaviour is what I was saying is insecure - that javax.crypto.CipherInputStream does similar questionable things for bad padding in CBC mode makes no difference.
> 
> From the JavaDoc of InputStream.read(byte[]):  IOException is thrown "If the first byte cannot be read for any reason other than the end of the file”.
> (The first byte in this case is the first byte in the stream that would be placed at b[0]). 
> In my opinion, an AEADBadTagException thrown by a Cipher that prevents it from returning any bytes is such a reason, and as such should cause an IOException to be thrown.
> 
> tim
> 
> 
> ——
> 
>             Cipher c = Cipher.getInstance("AES/GCM/NoPadding", "SunJCE");
>             c.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(new byte[16], "AES"), new GCMParameterSpec(128, new byte[16]));
>             byte[] ct = c.doFinal(new byte[100]);
> 
>             c.init(Cipher.DECRYPT_MODE, new SecretKeySpec(new byte[16], "AES"), new GCMParameterSpec(128, new byte[16]));
>             ct[ct.length - 1] = (byte)(ct[ct.length - 1] + 1);
> 
>             byte[] read = new byte[1000];
>             InputStream in = new javax.crypto.CipherInputStream(new ByteArrayInputStream(ct), c);
>             try
>             {
>                 int size = in.read(read);
>                 System.out.println("Via CipherInputStream: ciphertext is valid and " + size + " bytes long");
>             }
>             catch (IOException e)
>             {
>                 System.errr.println("Via CipherInputStream: ciphertext is invalid");
>             }
>             finally
>             {
>                 in.close();
>             }
> 
>             try
>             {
>                 byte[] pt = c.doFinal(ct);
>                 System.out.println("Via Cipher: ciphertext is valid and " + pt.length + " bytes long");
>             }
>             catch (AEADBadTagException e)
>             {
>                 System.err.println("Via CipherInputStream: ciphertext is invalid");
>             }
> 
>