RFR: 8207775: Better management of CipherCore buffers

Anthony Scarpino anthony.scarpino at oracle.com
Wed Aug 1 17:41:26 UTC 2018


That looks fine to me.

Tony

On 08/01/2018 08:39 AM, Seán Coffey wrote:
> Thanks again for review Tony. I think you raise a good point and should 
> give some performance gain.
> 
> for line 676: maybe this :
> 
>                   byte[] copy = Arrays.copyOf(output, len);
>                   if (decrypting) {
>                       Arrays.fill(output, (byte) 0x00);
>                   }
>                   return copy;
> 
> 
> perhaps the same again for around line 852 :
>                   byte[] copy = Arrays.copyOf(output, len);
>                   if (decrypting) {
>                       Arrays.fill(output, (byte) 0x00);
>                   }
>                   return copy;
> 
> ==
> 
> Any other reviews/comments from folks before I submit final build/test & 
> webrev ?
> 
> Regards,
> Sean.
> 
> On 01/08/18 15:55, Anthony Scarpino wrote:
>>
>>
>> My only comment is if it makes sense to have the change at 676 to also 
>> only null out on decrypt?
>>
>> Otherwise I'm fine with the changes
>>
>> Tony
>>
>> On 07/31/2018 02:04 AM, Seán Coffey wrote:
>>> Thanks for review Tony. Comments inline..
>>>
>>> On 27/07/18 21:02, Anthony Scarpino wrote:
>>>> If we are going to add more, here are two more ton consider:
>>>>
>>>> - It looks like there is another Arrays.copyOf() on doFinal() line 851
>>> Good point.
>>>> - doFinal()
>>>> at line 897 there might be something that should be done with 
>>>> 'buffer'.  In particular as a result of line 963's arraycopy().
>>> Yes - I've identified two areas where we can be proactive about 
>>> nulling out 'buffer' contents. That's around the same time where we 
>>> reset 'buffered' to 0. See lines 777 and 967
>>>
>>> http://cr.openjdk.java.net/~coffeys/webrev.8207775.v3/webrev/
>>>
>>> regards,
>>> Sean.
>>>>
>>>> Tony
>>>>
>>>>
>>>> On 07/27/2018 08:29 AM, Seán Coffey wrote:
>>>>> Thanks Tony. If it's alright with you, I'd like to make one more 
>>>>> edit for this change.
>>>>>
>>>>> http://cr.openjdk.java.net/~coffeys/webrev.8207775.v2/webrev/
>>>>>
>>>>> There's a condition where we can null out an array early if we're 
>>>>> returning a copy. See lines 671-683
>>>>>
>>>>> Regards,
>>>>> Sean.
>>>>>
>>>>> On 26/07/18 17:42, Anthony Scarpino wrote:
>>>>>> On 07/26/2018 07:36 AM, Seán Coffey wrote:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8207775
>>>>>>>
>>>>>>> Simple enough fix to null out some internal buffers once they're 
>>>>>>> no longer required.
>>>>>>>
>>>>>>> webrev : http://cr.openjdk.java.net/~coffeys/webrev.8207775/webrev/
>>>>>>>
>>>>>>> regards,
>>>>>>> Sean.
>>>>>>>
>>>>>>
>>>>>> that looks fine..
>>>>>>
>>>>>> Tony
>>>>>
>>>>
>>>
>>
> 




More information about the security-dev mailing list