RFR: 8255557: Decouple GCM from CipherCore [v7]

Valerie Peng valeriep at openjdk.java.net
Tue Jun 1 19:47:43 UTC 2021


On Wed, 26 May 2021 21:13:56 GMT, Anthony Scarpino <ascarpino at openjdk.org> wrote:

>> Hi,
>> 
>> I need a review of this rather large change to GCM.  GCM will no longer use CipherCore, and AESCrypt  to handle it's buffers and other objects.  It is also a major code redesign limits the amount of data copies and make some performance-based decisions.
>> 
>> Thanks
>> 
>> Tony
>
> Anthony Scarpino has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove GCTR reset() calls because GCTR is released after the operation
>   some variable name consistency
>   other small cleanup

Most of the updates look fine, but some of my review comments in GaloisCounterMode didn't get a reply, just want to be sure that you saw them?

Thanks,
Valerie

src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java line 156:

> 154:         }
> 155:     }
> 156: */

Remove?

src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 96:

> 94: 
> 95:     // Default value is 128bits, this is in bytes.
> 96:     int tagLenBytes = 16;

nit: use DEFAULT_TAG_LEN instead of 16

src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 744:

> 742:             int resultLen = 0;
> 743: 
> 744:             int bLen = getBufferedLength();

Well, it seems strange to calculate bLen based on 'ibuffer' instead of the specified 'buffer'. For code clarity, it seems better to just use 'buffer'?

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

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



More information about the security-dev mailing list