RFR: 8253821: Improve ByteBuffer performance with GCM [v2]

[Anthony Scarpino]

On Thu, 8 Oct 2020 00:13:37 GMT, Anthony Scarpino <ascarpino at openjdk.org> wrote:

>> src/java.base/share/classes/com/sun/crypto/provider/CipherCore.java line 939:
>> 
>>> 937:             // if the size of specified output buffer is less than
>>> 938:             // the length of the cipher text, then the current
>>> 939:             // content of cipher has to be preserved in order for
>> 
>> This change is somewhat dangerous. When using the supplied output buffer directly, you may corrupt its content w/ padding bytes. This optimization should only be applied when no padding is involved. In addition, input and output can point to the same buffer with all sorts of index combinations, i.e. inOfs == outOfs, inOfs < outOfs, inOfs > outOfs. With "outWithPadding" approach, no need to check. However, for "internalOutput", data corruption may happen. This kind of problem can be very hard to diagnose. Have to be very very careful here as this code may impact all ciphers...
>
> - I do not understand where the corruption comes from.  The user provides a buffer that output is suppose to be placed into, what could it be corrupting?  The existing tests (SameBuffer, in particular) works fine with this and the ByteBuffer calls.  I spent a lot of time trying to get those same buffer tests to pass.  
> - It was my expectation that checkOutputCapacity() is making sure all the inOfs ==<> outOfs are going to work. Does that not catch all cases?
> - outWithPadding" is excessive because it doubles the allocation for every operation followed by a copy to the original buffer, even if the original buffer was adequate.  I'm ok with doing the extra alloc & copy in certain situations, but not everytime.  Can you be more specific what things may fail that we don't already check for in the regression tests?

I wrote a whole new tests to exercise all the buffers with and without offsets.  Hopefully that covers the concern.  The test found 4 bugs..

-------------

PR: https://git.openjdk.java.net/jdk/pull/411