RFR [13] 8220165: Encryption using GCM results in RuntimeException- input length out of bound

Anthony Scarpino anthony.scarpino at oracle.com
Thu Mar 7 18:25:46 UTC 2019


On 3/6/19 6:52 PM, Valerie Peng wrote:
> Hi Tony,
> 
> Source change looks fine.
> 
> For the regression test GCMLargeDataKAT.java
> 1) typo on line 42, "has" -> "hash".
> 2) Line 128, we should also check result.length to be the expected 
> value, i.e. data.length - GCM_TAG_LENGTH.

I had intentionally left a length check out figuring that a nulled 
plaintext would have to be the proper length, but I guess it worth to 
check the array was returned property

> 3) Currently, the test continues even if enc check failed (line149-155). 
> If the enc result is incorrect, why proceed with decryption? Perhaps, 
> testing against the next key (i.e. data length) after line 155?

Yeah, that's true.

> 4) Remove line 184-189 as they are not used?

It's used to create new data lengths if passed a length argument.  As I 
was able to do with your point #6.  In jtreg operation it is not used.

> 5) line 195, no need for this being public?

yep

> 6) Are 65536, 65537 and 67584 input sizes add additional coverage? For 
> data sizes 102400 to 102416, they can easily be tested with two byte[]s, 
> sizes 102400 and 16.

I chose them when I was debugging the problem and left them there.  I 
added a few more lengths around the AES block size.  64k is when the 
intrinsic warmup code starts, so anything above 65535 is equally 
relevant to test.  I used 102400 through 102416 just to cover every byte.

I also changed the plaintext to use one byte array instead of
encrypt(new byte[inLen])

new webrev:
http://cr.openjdk.java.net/~ascarpino/8220165/webrev.01/


> Thanks,
> Valerie
> On 3/6/2019 3:04 PM, Anthony Scarpino wrote:
>> Hi
>>
>> Can I get a review of a simple fix to the previous GCM change that got 
>> the lengths wrong on large data sizes.  I thought I covered this in my 
>> original testing, but I guess not.  So with this I created a test to 
>> check larger data sizes and different byte lengths
>>
>> http://cr.openjdk.java.net/~ascarpino/8220165/
>>
>> Tony
>>



More information about the security-dev mailing list