RFR 8218723: SecretKeyFactory.getInstance( algo_, provider_ ) ignores the provider argument.

[Adam Petcher]

Hi Jamil,

This looks good to me.

I read through the discussion about running the Mac in hardware, and I 
think it is fine if this is not supported by SunJCE. It doesn't look 
like the current API for Mac fully allows this, and even if it did, it 
would be simpler to stay in SunJCE once that provider is selected. If 
running the PRF in hardware is desired, then the way to accomplish that 
is for all of PBKDF2 to happen in the hardware provider.

On 3/15/2019 6:05 PM, Jamil Nimeh wrote:
> Hello all,
>
> I've updated the webrev with all of Adam's findings with one 
> exception.  I did try bringing the crypto provider code in-line. That 
> approach will work for OpenJDK where there is no 3rd party signing 
> requirement, but on Oracle JDK it will not and the provider needs to 
> be in the form of a signed jar file.  I would like this to run on both 
> JDKs, especially since the provider selection codepaths are a bit 
> different between Oracle and Open JDKs.  Also the original bug came in 
> against Oracle JDK so it would be nice to have the test run there.
>
> http://cr.openjdk.java.net/~jnimeh/reviews/8218723/webrev.02
>
> Thanks,
>
> --Jamil
>
> On 3/14/19 7:53 AM, Adam Petcher wrote:
>> The change to PBKDF2KeyImpl.java looks fine. About the test:
>>
>> *) Is it necessary to put the provider in a separate jar? It seems 
>> unnecessary because you are adding it with Security.insertProviderAt.
>>
>> *) Line 54 of the test compares the result of a constructor to null. 
>> Unless I'm missing something, this reference will always be non-null.
>>
>> *) At the end of the test, there are some methods that do conversion 
>> between hex strings and bytes. Can you use the methods in Convert (in 
>> the test list) instead? I think Convert.hexStringToByteArray is the 
>> same thing as hex2bin. You may also want to move dumpHexBytes to 
>> Convert, but it's fine either way.
>>
>> *) It looks the evilprovider source files have the wrong copyright 
>> header.
>>
>> *) There is a commented out line of code on line 16 of EvilProvider.java
>>
>> On 3/14/2019 9:34 AM, Jamil Nimeh wrote:
>>> Hello all,
>>>
>>> This review will change the SunJCE implementation of PBKDF2 so that 
>>> it always uses the SunJCE version of the PRF algorithm internally.
>>>
>>> Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8218723/webrev.01/
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8218723
>>>
>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8220531
>>>