RFR: 8208583: Better management of internal KeyStore buffers

[Weijun Wang]

com/sun/crypto/provider/KeyProtector.java:

 187             Key k = kFac.generatePrivate(new PKCS8EncodedKeySpec(plain));
 188             return k;

 No need to create a local variable "k" anymore.

JavaKeyStore.java:

 128         byte[] passwordBytes = null;

 No need to define so early and assigned null.

No other comments.

> On Aug 3, 2018, at 12:37 AM, Seán Coffey <sean.coffey at oracle.com> wrote:
> 
> All valid comments Max. Changes made. Webrev at
> 
> http://cr.openjdk.java.net/~coffeys/webrev.8208583.v4/webrev/index.html
> 
>> I wonder why DestroyedFailedException was a checked exception, what can we do if it's thrown?
> I guess we could log a message, but given the limited usage case here, I don't see a need.

I was not clear, I meant I cannot find a proper action to do in this case, thus it might be better to defined as an unchecked exception.

Thanks
Max

> 
> regards,
> Sean.
> 
> 
> On 02/08/2018 17:05, Weijun Wang wrote:
>> KeyProtector.java:
>> 
>>  113         pbeKeySpec.clearPassword();
>> 
>>  You can also put this into the finally block.
>> 
>>  189             Arrays.fill(plain, (byte) 0x00);
>> 
>>  Can this be in finally?
>> 
>> JavaKeyStore.java:
>> 
>>  149         Arrays.fill(passwordBytes, (byte) 0x00);
>> 
>>  In other cases, you call it in a finally block. Unnecessary here?
>> 
>>  (Oops, every comment is about finally.)
>> 
>> KeyProtector.java:
>> 
>>  123     public KeyProtector(byte[] password)
>> 
>>  How about just "public KeyProtector(byte[] passwordBytes)"?
>> 
>>> On Aug 2, 2018, at 7:38 PM, Seán Coffey <sean.coffey at oracle.com> wrote:
>>> 
>>> No - no problem at all. Some extra exception handling but probably best for the long run.
>> I wonder why DestroyedFailedException was a checked exception, what can we do if it's thrown?
>> 
>> Thanks
>> Max
>> 
>>> http://cr.openjdk.java.net/~coffeys/webrev.8208583.v3/webrev/index.html
>>> 
>>> regards,
>>> Sean.
>>> 
>>> On 02/08/2018 02:13, Weijun Wang wrote:
>>>>> 1.
>>>>> 
>>>>> I wasn't able to rename to destroy since that method is reserved for the Destroyable interface. I've gone with destroyKey.
>>>>> 
>>>> Sorry I wasn't clear but this is exactly what I meant. SecretKey implements Destroyable so you don't need to define sKey as PBEKey. Does it make any problem?
>>>> 
>>>> 
>