On 03/04/2014 08:49 AM, Philipp Heckel wrote:
Although Tim and Matthew already mentioned the main points, I'd like to
voice my concerns as well -- in particular because I think that this is
*not* a philosophical argument: Security must always be more important
than the supposedly correct semantics of a class or method.
You can't increase security by implementing incorrect semantics, you
just get something that is broken. Your complaint about buffering shows
that. :-)
The whole purpose of Input/OutputStreams is to be able to avoid large
buffers and/or temporary files of any sort. For modes in which the tag
is appended to the ciphertext (including GCM), buffering all the data is
simply unacceptable, since if would make processing of large files
simply impossible.
The problem is that you cannot, in general, implement non-chunked
integrity checks securely by returning unverified chunks and throwing an
exception once you reach the end and detect that you've returned bad
data somewhere along the way.
Using the Cipher class directly is not an option either, because many
applications nest streams into one another. Chaining gzip+encryption is
not uncommon. If you process large amount of data, again, manually
unpacking/packing is not feasible.
And it's also not uncommon that you don't want to expose your
decompression code to unsigned data, so AEAD without chunking is just a
poor choice here.
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.
Adding to what Matthew said: I think it does matter. Security-related
exceptions should never be hidden from the developer!
Except when you have to do this to avoid creating an oracle. Since
CipherInputStream is used with other ciphers, this might have been the
reason why the cryptographic-related exceptions are swallowed.
That being said, adding a warning that this happens to the
CipherInputStream documentation seems prudent.
--
Florian Weimer / Red Hat Product Security Team
