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....@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-...@whittington.net.nz >>> <mailto:jdk-security-...@whittington.net.nz>> wrote: >>> >>>> >>>> On 7/03/2014, at 9:14 am, Philipp Heckel <philipp.hec...@gmail.com >>>> <mailto:philipp.hec...@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"); > } > >
