RFR 8177784 Use CounterMode intrinsic for AES/GCM

[Paul Sandoz]

> On 7 Apr 2017, at 12:47, Anthony Scarpino <anthony.scarpino at oracle.com> wrote:
> 
> On 04/07/2017 06:58 AM, Chris Hegarty wrote:
>> On 06/04/17 21:39, Anthony Scarpino wrote:
>>> 
>>> I'd like to get a review for this performance change to use the existing
>>> CounterMode parallelized intrinsic instead of GCTR's own version. The
>>> two classes were nearly identical except for the doFinal() method which
>>> doesn't belong in CounterMode.java.
>>> 
>>> I could have been more aggressive with this change, but I'm looking to
>>> get this into 9, so I stayed away from completely merging GCTR into
>>> CounterMode in case of incompatibilities.  All tests security and
>>> hotspot tests pass.
>>> 
>>> http://cr.openjdk.java.net/~ascarpino/8177784/webrev/
>> 
>> This change looks good to me. Trivially, the class-level comment in
>> GCTR should be updated ( it refers to removed fields ). Also,
>> CounterMode.counter could be protected ( rather than package-private ).
>> 
>> -Chris.
> 
> Thanks Chris,
> 
> I left CounterMode.counter as package-private because the package is what becomes the SunJCE provider.  I don't believe there should be any outside package classes accessing this code.
> 
> I updated the webrev at with the comment update:
>  http://cr.openjdk.java.net/~ascarpino/8177784/webrev.01/
> 

It’s the same pattern for “iv” with CounterMode and FeedbackCipher, although i prefer protected as well if never accessed by other classes in the same package, it makes it easier to reason about the code, since i know more about what can access it.

Suggestion, take it or leave it: personally i would prefer to see an additional constructor in CounterMode:

  GCTR(SymmetricCipher cipher, byte[] initialCounterBlk) {
    super(cipher, checkBlockSize(initialCounterBlk));
   }

Where new CounterMode constructor performs the clone, assignmentm and reset, the static method checkBlockSize checks the array length is equal to AES_BLOCK_SIZE Thereby you have a cleaner separation of concerns.

Paul.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20170410/4580f7db/signature.asc>