Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

On Wed, Sep 19, 2018 at 1:52 AM, Valerie Peng wrote: > Test update looks fine and regression test run is clear. I have no more > comments. > Thanks, > Valerie > Submit-repository tests passed. Integrated to baseline then: * http://hg.openjdk.java.net/jdk/jdk/rev/bccd9966f1ed Thanks Valerie f

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Test update looks fine and regression test run is clear. I have no more comments. Thanks, Valerie On 9/12/2018 4:22 AM, Martin Balao wrote: Hi Valerie, Thanks for your answer. Webrev.09:  * http://cr.openjdk.java.net/~mbalao/webrevs/8029661/8029661.webrev.09/

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Valerie, Thanks for your answer. Webrev.09: * http://cr.openjdk.java.net/~mbalao/webrevs/8029661/8029661.webrev.09/ * http://cr.openjdk.java.net/~mbalao/webrevs/8029661/8029661.webrev.09.zip In TestTLS12.java, we now capture any exception during initialization phase and skip test execution

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi, Martin, I am ok with your option#1. Note that your test fails at different places of the code, so you will need to check and skip test execution before those exception are thrown. Valerie On 9/11/2018 7:54 AM, Martin Balao wrote: Hi Valerie, On Fri, Aug 31, 2018 at 9:16 PM, Valerie Peng

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Valerie, On Fri, Aug 31, 2018 at 9:16 PM, Valerie Peng wrote: > Hi Martin, > > In TestTLS12.java, you call the initSecmod() inside initialize() and when > initSecmod() returns false, you return from initialize() and continue down > the main(). Is this intentional? Other tests seems to be skip

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Martin, In TestTLS12.java, you call the initSecmod() inside initialize() and when initSecmod() returns false, you return from initialize() and continue down the main(). Is this intentional? Other tests seems to be skipping execution when initSecmod() return false. Changes in webrev.08 res

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Valerie, Thanks for your feedback. Webrev.08: * http://cr.openjdk.java.net/~mbalao/webrevs/8029661/8029661.webrev.08 * http://cr.openjdk.java.net/~mbalao/webrevs/8029661/8029661.webrev.08.zip New in Webrev.08: * Rebased to latest JDK (rev fa378e035b81) * Test max lines length is now 80

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Martin, I still observe the TestTLS12 regression test failure with your webrev.07. Judging from the test failure log, it seems that the test fails when run on a machine whose NSS library does not support the TLS v1.2 mechanisms. Generally, the test should check and skip if the to-be-tested

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Valerie, Here it is Webrev.07: * http://cr.openjdk.java.net/~mbalao/webrevs/8029661/8029661.webrev.07/ * http://cr.openjdk.java.net/~mbalao/webrevs/8029661/8029661.webrev.07.zip * p11_convert.c: * L530 and 834: masterKeyDeriveParamToCKMasterKeyDeriveParam and keyMatParamToCKKeyMatParam

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Martin, I imported your webrev.06 and sun/security/pkcs11/tls/TestTLS12.java still fails, but with a different reason (see below). jib > STDERR: jib > java.lang.RuntimeException: unable to expand property pkcs11test.nss.libdir jib > at jdk.crypto.cryptoki/sun.security.pkcs11.

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

I sent this email a bit prematurely. I still need to review TestTLS12.java. All else is done. Valerie On 7/31/2018 4:26 PM, Valerie Peng wrote: Hi Martin, Here are the review comments for the remaining files: - Always check non-null result for JNIEnv FindClass(...) calls, e.g. line 530

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Martin, Here are the review comments for the remaining files: - Always check non-null result for JNIEnv FindClass(...) calls, e.g. line 530 and 802 - Sometimes the check is there but is after the return value has been used. These checks should be moved up, i.e. right after the FindClas

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi, That's correct: TestTLS12.java was introduced by this patch and is checking that the new feature (TLS 1.2 + SunPKCS11) is working correctly. If the PKCS11 library does not support TLS 1.2 mechanisms, the test must fail. This test should be skipped on those configurations. Kind regards, Martin

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Update: I submitted your webrev.05 through Mach5, there are one test failure observed on 4 configurations, all are due to the regression test TestTLS12.java. It looks like the test fails when the underlying PKCS11 library does not support the corresponding TLS 12 mechanisms (stacktrace includ

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Valerie, Thanks for your feedback! CKM_TLS12_MAC looks like it's not in use. Authentication codes are calculated through CKM_TLS12_KEY_AND_MAC_DERIVE mechanism. Do you know of a library supporting CKM_TLS12_MAC but not CKM_TLS12_KEY_AND_MAC_DERIVE? I've been testing this with NSS software toke

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Martin, Sorry for the delay, I am able to resume on reviewing this now. Here are some initial comments. What about CKM_TLS12_MAC? I see that it's defined under TLS 1.2 mechanisms, but not much other details for it. - Is this change still necessary? I didn't notice any new import stateme

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Valerie, Webrev 05: * http://cr.openjdk.java.net/~mbalao/webrevs/8029661/8029661.webrev.05/ * http://cr.openjdk.java.net/~mbalao/webrevs/8029661/8029661.webrev.05.zip New in Webrev 05: * Explicitly casted prfHashMechanism to CK_MECHANISM_TYPE type to avoid building warning on some compile

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Thanks for updating the webrev, I will take a look. Valerie On 7/10/2018 10:18 AM, Martin Balao wrote: Hi, Webrev 04 for JDK-8029661 is ready:  * http://cr.openjdk.java.net/~mbalao/webrevs/8029661/8029661.webrev.04.zip

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi, Webrev 04 for JDK-8029661 is ready: * http://cr.openjdk.java.net/~mbalao/webrevs/8029661/8029661.webrev.04.zip * http://cr.openjdk.java.net/~mbalao/webrevs/8029661/8029661.webrev.04/ New: * Rebased to latest JDK revision (after TLS 1.3 merge) * Rev 1acfd2f56d72 * ProtocolVersion depen

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Valerie, Webrev 03 with changes to TestTLS12.java is here: * http://cr.openjdk.java.net/~mbalao/webrevs/8029661/8029661.webrev.03 * http://cr.openjdk.java.net/~mbalao/webrevs/8029661/8029661.webrev.03.zip The reason why we need a dependency to sun.security.pkcs11.SunPKCS11 is to instance su

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Jack, Sorry for the late response, I have not been able to resume working on your patch for JDK-8029661 "Support TLS v1.2 algorithm in SunPKCS11 provider" as I have to work on a critical project for past few months. Now that my work for this critical project work is winding down, I will ci

RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Valerie, I have an important customer awaiting this change. Can you please provide a status update as to the progress of this effort and any information available as to when it may be completed. Thanks, Jack

RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Valerie, I have an important customer awaiting this change. Can you please provide a status update as to the progress of this effort and any information available as to when it may be completed. Thanks, Jack

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Any updates on this? Thanks, Martin.- On Thu, Feb 15, 2018 at 9:53 PM, Valerie Peng wrote: > > I am ok with the idea of putting the public API refactoring under a > separate RFE. > Let me apply your updated patch below and run existing regressions again. > Thanks, > Valerie > > > On 2/5/2018 5:

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

I am ok with the idea of putting the public API refactoring under a separate RFE. Let me apply your updated patch below and run existing regressions again. Thanks, Valerie On 2/5/2018 5:51 AM, Martin Balao wrote: Hi Valerie, Thanks for your review. Here it's the new webrev updated to the ne

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Valerie, Thanks for your review. Here it's the new webrev updated to the new repository structure. I've also updated the testcase to use the new @module jtreg feature: * http://cr.openjdk.java.net/~akasko/mbalao/jdk_8029661_tls_12_sunpkcs11/2018_02_02/8029661.webrev.02 (online) * http://cr.

Re: RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi Martin, Thanks for providing us with the patch. Recently, JDK workspace has been restructured a bit, do you have an updated webrev? Your changes mostly look fine, but I think we should allow 3rd party providers to have similar support. For this, we need to have more public APIs such as th

RFR JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider

Hi, I would like to propose a patch for JDK-8029661: JDK-Support TLS v1.2 algorithm in SunPKCS11 provider [1]. * http://cr.openjdk.java.net/~akasko/mbalao/jdk_8029661_tls_12_sunpkcs11/2017_11_09/8029661.webrev.01/ (browse online) * http://cr.openjdk.java.net/~akasko/mbalao/jdk_8029661_tls_12_su