Code Review Request for 6996769: support AEAD ciphers

Weijun Wang weijun.wang at oracle.com
Wed Dec 12 18:48:34 PST 2012


Hi Valerie

The code changes work. However, I'm only not quite comfortable with the 
passKeyAndIvCheck flag. It sounds like it should be only required for 
GCM/encoding but now it's used everywhere. I especially find this line 
in doFinal() confusing:

    passKeyAndIvCheck = (cipherMode != GCM_MODE);

Maybe you can use a negative value called "requireInit"?

Also, there seems no need to update the flag in the following lines and 
a local variable is ok:

     // check key+iv for encryption in GCM mode
     passKeyAndIvCheck =
         !Arrays.equals(ivBytes, lastEncIv) ||
         !Arrays.equals(keyBytes, lastEncKey);

The flag will be set at the end of the method when real init is fine.

Thanks
Max

On 12/08/2012 09:23 AM, Valerie (Yu-Ching) Peng wrote:
> Max,
>
> The webrev has been updated so that different key + iv values have to be
> used for AES/GCM encryption.
> Latest version at: http://cr.openjdk.java.net/~valeriep/6996769/webrev.03/
>
> Please review and send me comments.
> Thanks!
> Valerie
>
> On 11/07/12 21:50, Valerie (Yu-Ching) Peng wrote:
>> Max,
>>
>> Update: I removed the block (starting line 580 in CipherCore.java) for
>> handling RC2 since it's never used.
>>
>> It turns out that the current impl in RC2Cipher always convert the
>> AlgorithmParameters object to RC2ParameterSpec and only uses
>> CipherCore.init(..., AlgorithmParameterSpec,...) method. Thus, I won't
>> be adding a regression test, but rather simply document the current
>> RC2Cipher behavior in CipherCore.java to clarify things up.
>>
>> The updated webrev is at:
>> http://cr.openjdk.java.net/~valeriep/6996769/webrev.01/
>>
>> Xuelei brought up the issue of enforcing (Key+IV) uniqueness for GCM
>> mode during this afternoon's meeting.
>> I think more changes may be made after we decide what to do.
>> So, there may be a webrev.02 coming... Just a heads up.
>>
>> Thanks!
>> Valerie
>>
>> On 11/07/12 14:48, Valerie (Yu-Ching) Peng wrote:
>>>> 580                  } else if (key.getAlgorithm().equals("RC2")) {
>>>>
>>>> This seems a bug fix. Is there a regression test for it?
>>> I just noticed this problem when make the enhancement for GCM mode.
>>> I will add a regression test for this.
>


More information about the security-dev mailing list