RFR: 8207775: Better management of CipherCore buffers
Seán Coffey
sean.coffey at oracle.com
Thu Aug 2 12:15:01 UTC 2018
Thanks Tony. I was asked off thread to add some comments to help code
maintenance also.
webrev updated. Hope we're nearly ready to push now.
http://cr.openjdk.java.net/~coffeys/webrev.8207775.v4/webrev/
regards,
Sean.
On 01/08/2018 18:41, Anthony Scarpino wrote:
> 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