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
