On Tue, 1 Dec 2020 23:14:19 GMT, Valerie Peng <[email protected]> wrote:
>> Anthony Scarpino has updated the pull request incrementally with five
>> additional commits since the last revision:
>>
>> - test updates
>> - test check mixup
>> - Overlap protection
>> - Updated code comments, all tests pass
>> - Updated code comments, all tests pass
>
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java
> line 657:
>
>> 655: */
>> 656: int decrypt(byte[] in, int inOfs, int len, byte[] out, int outOfs) {
>> 657: checkDataLength(ibuffer.size(), len);
>
> Just curious, why remove this checkDataLength(...) call? Seems inconsistent
> to other methods?
All decrypt() does is put the data into the ibuffer. I didn't find it useful
to calculate that data lengths for the GCM rollover when decryption was not
happening. It's probably a personal preference to wait until doFinal(). The
situation when ibuffer is > 64M is very unlikely.
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java
> line 910:
>
>> 908: // Decrypt the all the input data and put it into dst
>> 909: doLastBlock(buffer, ct, dst);
>> 910: dst.limit(dst.position());
>
> Based on Cipher javadoc, the output buffer's limit should remain the same,
> just its position be advanced by the number of bytes written into it. So, we
> should not call limit(int) to set a new limit on user-specified ByteBuffer
> objects.
ok
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java
> line 935:
>
>> 933: */
>> 934: ByteBuffer overlapDetection(ByteBuffer src, ByteBuffer dst) {
>> 935: if (src.isDirect() && dst.isDirect()) {
>
> originalDst is only set to null at construction time. It should be reset here
> or in restoreDst(). Otherwise, there may be some strange interaction between
> various ByteBuffer calls using the same Cipher object. Say, the first call
> uses an extra copy and set originalDst, then the second call does not, but
> originalDst is still non-null.
Yeah, that makes sense.
-------------
PR: https://git.openjdk.java.net/jdk/pull/411
