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

Jamil Nimeh jamil.j.nimeh at oracle.com
Thu Mar 14 15:18:12 UTC 2019


Hi Adam, thanks for taking a look at this.  Comments are in-line:

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.
JN: I'm honestly not sure.  I thought if I wanted this test to run on 
both Open and Oracle JDK I thought I'd need the provider to be signed 
(which the jar file is).  I can try bringing the provider code into the 
test body itself.  If it runs fine on Oracle JDK then it would be way 
better to do that and get rid of the makefile and all the attendant 
source for rebuilding the provider jar.
>
> *) 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.
JN: That was left over from an early rev of the test where I was doing 
Security.getProvider(String) where it can return null.  I'll take that out.
>
> *) 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.
JN: I had meant to fix that before I sent it to review, I'm in the 
process of making that change now.
>
> *) It looks the evilprovider source files have the wrong copyright 
> header.
JN: Easily fixed.
>
> *) There is a commented out line of code on line 16 of EvilProvider.java
JN: Good as gone.
>
> 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
>>


More information about the security-dev mailing list