The purpose of the buffering is for security or conforming to the spec as far as I am concerned. We can't do the buffering in CICO classes as JCE Cipher class can be used directly. It seems best to handle this in the internal GCM implementation classes since CipherCore is somewhat general and I'd like to avoid the potentially-extensive (isGCM) check if we were to add the handling at the CipherCore level.
As for the tag being appended to the cipher text, I think that's somewhat an industry convention for GCM impl, at least Solaris native Ucrypto library also assumes that the tag is appended after cipher text.
It's true that some existing code have problem handing GCM due to its different nature, i.e. AEAD cipher which has AAD and generate/verify tag. We can only do our best to fit GCM into the current model. When people start using GCM and encounter problems, they'd have to fix problems in their code accordingly. Nothing we can do on our end.
Once we agree on the buffering is necessary, then we can see how to minimize the performance impact, e.g. perhaps place an upper limit on input size, etc.
Thanks, Valerie On 07/04/13 06:20, Xuelei Fan wrote:
Hi Valerie, CC Brad. We start to run into the pain to get authentication tag appended in the tail of cipher text in our design. I looked back the discussion when we designed the APIs for GCM cipher in JDK 7. I found that when we run into GCM mode cipher operations, we may need to update the source code here or there because GCM cipher operation is a lot different than the old cipher modes. For example, in JSSE, we have to update the cipher operations to accept GCM mode ciphers. I two major concerns about buffering all cipher text in the underlying crypto library. One is about the performance impact, another one is about compatibility issues. When the size of cipher text/data is huge or the application is running in a heavy loaded system, there is a big impact on memory load and may impact the performance a lot. Even if we have to buffer the data in some special context, it would be better to do it in the particular context, but not in the crypto library level. There is two issues about compatibility. One is about how to use the Cipher.getOutputSize(). Yes, it is supposed to call this method before doing any update operation. But in practice, I think there is a lot applications that does not call this method. For example, in JSSE, it is supposed that the output size is not too much longer than the current input data length. In this fix, the application would have to heavily depends on the Cipher.getOutputSize() because the cipher text is buffered. And the return value of Cipher.getOutputSize() may be huge, which will prevent the reuse of memory (for example, ByteBuffer and array). Here is another performance impact. Another issue about compatibility is about the return value of Cipher.update(). In the past, it may be always can return a positive value if the input data is big enough (bigger than the block size). For example, in JSSE provider, we have code: int newLen = cipher.update(buf, offset, len, buf, offset); if (newLen != len) { throw new RuntimeException("Cipher buffering error " + "in JCE provider " + cipher.getProvider().getName()); } int newLen = cipher.update(dup, bb); if (newLen != len) { // catch BouncyCastle buffering error throw new RuntimeException("Cipher buffering error " + "in JCE provider " + cipher.getProvider().getName()); } If I'm correct, the above code will run into problems with this fix, because the return value the Cipehr.update() for GCM will return 0. I believe it is common in practice. Backing to this bug, if you want to buffering, I would suggest to doing the buffering in CI/CO classes rather than in GCM cipher implementation (performance issue is still a concern of mine, but the scope of the impact is limited). I think it is accept to me that we only checking the authentication tag at the end (CI/CO.close(), with IOException to catch authentication tag checking failure). The data may have been write to the I/O, therefor, applications who want to use GCM and CI/CO may need to take additional actions to handle the IOException. Xuelei On 6/12/2013 7:16 AM, Valerie (Yu-Ching) Peng wrote:Xuelei, Here is another GCM and CipherInputStream/CipherOutputStream related fix, i.e. for 8012900: CICO ignores AAD in GCM mode The key changes are in CipherCore.java, GalorisCounterMode.java, the rest files only have minor changes. Essentially, when using AES/GCM cipher in decryption mode, the data will be buffered and processed AFTER the tag has been verified. Otherwise, most of the recovered text would be returned even if tag verification failed later. Given that CipherCore is shared by most modes, this particular buffering is done inside the GCM impl. But then some more methods have to be added/modified slightly so CipherCore will include this additional buffering from the underneath level in its output size calculations, etc. The webrev is at: http://cr.openjdk.java.net/~valeriep/8012900/webrev.00/ Thanks, Valerie
