On 4/03/2014, at 12:49 pm, Valerie (Yu-Ching) Peng <valerie.p...@oracle.com> wrote:
> > 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. There are several things wrong with this thinking in my opinion (in no particular order): 1. It has serious and unexpected performance problems. The JDK 8 GCM implementation is effectively useless for any large scale encryption (such as large files that need to be protected at rest, or large volumes of moderately sized messages) due to it buffering all ciphertext during decrypt (this is leaving aside the hit of all the data copying in the implementation of that buffering, and the unnecessary buffering of AD). 2. Cipher is the wrong place in a security architecture to apply this kind of restriction - Cipher (as well as the BC AEADBlockCipher API and openssl cipher APIs) is framed in terms of a stream of ciphertext blocks in and a stream of plaintext blocks out, and all of the current mainstream AE modes utilise trailing authentication tags. It’s a natural and efficient pattern to apply that authentication at the conclusion of the ciphertext stream. From my POV handling of authentication belongs at a protocol level, built on the available cryptographic primitives - just as it is now when Cipher and Mac are used in a generic composition ala TLS, SSH. As a protocol designer that allows me to packetise my plaintext/ciphertext, implement a higher level AEAD framing mode, or utilise appropriate (i.e. not RAM) temporary storage for large decrypted plaintexts until they are fully authenticated. The support for AD in the Cipher API already implies a higher level protocol that is coordinating the authentication, so it seems odd for the Cipher to take a kind of moral custodian role by not relinquishing plaintext until authentication is complete. If you believe there should be a safer (from misuse) primitive (such as the NaCl secret box, or an incremental/framed AEAD mode as is being discussed on CFRG) then I’m all for that, but the API contract and performance characteristics of that API is distinct from that of the existing notion of a Cipher - degrading the only fast online authenticated cipher supported in the JDK to a slow, packet mode cipher is a poor bandaid over the lack of such an API, and prevents higher level designs from making more appropriate choices for their particular application. 3. It’s inconsistent with expectations and other behaviour. Third party JCE providers (such as BC) that reasonably implement the Cipher/AEADBadTagException API as documented and/or with natural expectations of the behaviour based on the published API documentation will behave significantly differently when used with CipherInputStream - CipherInputStream should operate against the published API of Cipher/AEADBadTagException, and not an internal JDK interpretation of how that should be implemented. > > 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. Even if you accept that the Cipher instance should return no data until the tag is validated, CipherInputStream is still broken - it will silently treat an unauthenticated ciphertext with a nonzero plaintext length the same way it treats an authenticated zero-length ciphertext. i.e. it allows an attacker to forge the transmission of an ‘empty’ message merely by flipping a bit of the authenticated ciphertext in transit. If you add to that the long standing silent truncation of decrypted plaintexts in non-authenticated modes when padding is invalid (either by truncation or invalid key) using CipherInputStream is a minefield. cheers tim
