RFR: JDK-8209129 :Further improvements to cipher buffer management

[Ivan Gerasimov]

Hi Seán!

Just a minor comment.

I don't know if it's even measurable in this context, but I was under 
impression that filling memory with zero *bytes* might be a slightly 
more efficient then filling with any other constant.

Maybe it is better to use Arrays.fill(passwd, '\0') instead of 
Arrays.fill(passwd, '0') to give the JVM a chance to optimize filling if 
it's possible?

With kind regards,

Ivan


On 8/22/18 9:25 AM, Seán Coffey wrote:
> Thanks for reviewing. comments inline..
>
> On 22/08/18 15:50, Weijun Wang wrote:
>> PBES2Core.java:
>>
>>   181         byte[] passwdBytes = key.getEncoded();
>>   182         char[] passwdChars = null;
>>   183         PBEKeySpec pbeSpec;
>>   184         try {
>>   185             if ((passwdBytes == null) ||
>>   186 !(key.getAlgorithm().regionMatches(true, 0, "PBE", 0, 3))) {
>>   187                 throw new InvalidKeyException("Missing password");
>>   188             }
>>   ....
>>   272         } finally {
>>   273             if (passwdChars != null) Arrays.fill(passwdChars, ' 
>> ');
>>   274             Arrays.fill(passwdBytes, (byte)0x00);
>>   275         }
>>
>> If passwdBytes == null, line 274 would throw an NPE.
> Good catch. Corrected.
>>
>> PBKDF2KeyImpl.java:
>>
>>    87         char[] passwd = keySpec.getPassword();
>>    88         if (passwd == null) {
>>    89             // Should allow an empty password.
>>    90             this.passwd = new char[0];
>>    91         } else {
>>    92             this.passwd = passwd.clone();
>>    93         }
>>    94         // Convert the password from char[] to byte[]
>>    95         byte[] passwdBytes = getPasswordBytes(this.passwd);
>>
>>    96         // remove local copy
>>    97         Arrays.fill(passwd, '0');
>>
>> If passwd == null, line 97 would throw an NPE.
> Another good catch!
>
> updated webrev : 
> http://cr.openjdk.java.net/~coffeys/webrev.8209129.v3/webrev/
>
> regards,
> Sean.
>
>>
>> Otherwise fine.
>>
>> Thanks
>> Max
>>
>>
>>> On Aug 17, 2018, at 12:53 AM, Seán Coffey <sean.coffey at oracle.com> 
>>> wrote:
>>>
>>> Find new webrev here Max :
>>>
>>> http://cr.openjdk.java.net/~coffeys/webrev.8209129.v2/webrev/
>>>
>>> regards :
>>>
>
>

-- 
With kind regards,
Ivan Gerasimov