RFR: 8208583: Better management of internal KeyStore buffers

[Seán Coffey]

Thanks for the comments Max. Comments inline..


On 01/08/2018 07:59, Weijun Wang wrote:
> Some comments:
>
> 1. Is it possible to let rename PBEKey::clearKey to PNEKey::destroy?
Sure.
>
> 2. I am not sure if the newly added "Arrays.fill(thatEncoded, (byte)0x00)" line in PBEKey::equals is safe. What if that key does not return a cloned copy when getEncode() is called? This is possible if the class is only used internally and never escape.
This is not new code. I just refactored the java.util.Arrays.fill call 
to Arrays.fill.
>
> 3. You would need to modify the spec of s.s.p.KeyProtector::<init> since password is of a new type. In fact, is this change necessary? Just to prevent the duplication of code in convertToBytes()?
Yes, I can add edits to that effect. The class is package private and I 
only found 2 usages of it. The main advantage is not having to copy or 
create an extra buffer in this code. The calling code now has full 
control over when it can null out the bytes in use. That should be a 
help here and also means we don't place unnecessary load on GC/Cleaner.
>
> 4. I found the last line of PBEKey::<init> might be modified to
>
>     CleanerFactory.cleaner().register(this, this::clearKey);
Sure - I can modify that.

regards,
Sean.
>
> I have never been a lambda expert so forgive me if this is not correct.
>
> Thanks
> Max
>
>> On Aug 1, 2018, at 3:11 AM, Seán Coffey <sean.coffey at oracle.com> wrote:
>>
>> Looking to improve management of internal buffers in KeyStore. The com.sun.crypto.provider.KeyProtector class uses the PBEKey class to protect some keys. Buffers can be freed up earlier if the caller takes responsibility for lifecycle of PBEKey object. (hence no reliance on Cleaner). Some other minor improvements made while visiting this area.
>>
>> Other improvements in sun.security.provider.KeyProtector where I believe the password buffer can be managed by the caller.  I only found 2 instances of where this class is used.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8208583
>> http://cr.openjdk.java.net/~coffeys/webrev.8208583.v1/webrev/index.html
>>
>> regards,
>> Sean.
>>