Code Review Request for 6996769: support AEAD ciphers

Valerie (Yu-Ching) Peng valerie.peng at oracle.com
Wed Nov 7 22:28:13 UTC 2012


Hi, Max,

Thanks for the prompt review!

On 11/07/12 02:41, Weijun Wang wrote:
> Hi Valerie
>
> Test4512704.java:
>
>    Why not test AES/CBC/PKCS5Padding anymore?
This particular test is not really padding-relevant, so I just switching 
to test AES/CBC/NoPadding instead, since GCM mode requires NoPadding.
Unless you feel very strongly about this, I prefer to leave it as is. 
Let me know...

>
> TestKATForGCM.java:
>
>    Is there a URL for the test data?
They are inside the following zip

http://csrc.nist.gov/groups/STM/cavp/documents/mac/gcmtestvectors.zip
whose link is contained in NIST's CAVP page
http://csrc.nist.gov/groups/STM/cavp/index.html

I will add the above info to the test source.
I Will update the webrev w/ your earlier comments and send it out later.
Thanks,
Valerie

> GaloisCounterMode, GCTR, and GHASH are good.
>
> Thanks
> Max
>
>
>
> On 11/06/2012 04:48 PM, Weijun Wang wrote:
>> CipherCore.java:
>>
>> 79       * update() must buffer this many bytes before before starting
>>
>> Dup "before".
>>
>> 380      AlgorithmParameters getParameters(String algName)
>>
>> The updated code does not return null anymore. Is there some other
>> reason out of this patch? The init() method below seems to support null
>> for all modes.
>>
>> 580                  } else if (key.getAlgorithm().equals("RC2")) {
>>
>> This seems a bug fix. Is there a regression test for it?
>>
>> 643, 765  arraycopy
>>
>> Maybe you can just call Arrays.copyOf()
>>
>>
>> GCMParameters.java:
>>
>> 70 +        this.tLen = gps.getTLen()/8;
>>
>> Ar you going to check if getTLen() % 8 == 0?
>>
>> I haven't read GaloisCounterMode, GSTR and GHASH yet. Guess I'll have to
>> study the NIST spec to go on.
>>
>> Thanks
>> Max
>>
>> On 11/03/2012 07:54 AM, Valerie (Yu-Ching) Peng wrote:
>>> Brad or Max,
>>>
>>> Can either of you review my changes for the following RFE?
>>> 6996769: support AEAD ciphers
>>>
>>> This is the JCE part of changes for the EFP "Support AEAD 
>>> CipherSuites".
>>>
>>> The webrev is at:
>>> http://cr.openjdk.java.net/~valeriep/6996769/webrev.00/
>>>
>>> I included IBM copyright in files where some code are adopted from 
>>> their
>>> sample impl.
>>>
>>> Thanks,
>>> Valerie




More information about the security-dev mailing list