Re: [14] RFR JDK-8229243 "SunPKCS11-Solaris provider tests failing on Solaris 11.4"

[Valerie Peng]

Great, thanks for the review and feedback~

Valerie

On 9/24/2019 9:20 PM, Xuelei Fan wrote:

I looked at the latest webrev:
  http://cr.openjdk.java.net/~valeriep/8229243/webrev.01/

Which is good to me.

Thanks,
Xuelei

On 9/19/2019 5:46 PM, Valerie Peng wrote:


I am not on the PKCS#11 committee and not sure about the plan.
As for which one is right, I am more inclined to the "spec is right" 
side which is also what NSS picked.
Comparing between spec and header, shouldn't the former get more 
eyeballs in terms of review?
The header file also has a deprecated structure CK_AES_GCM_PARAMS 
which contains both ulIvLen and ulIvBits fields.
As ulIvBits and ulIvLen are essentially same in terms of meaning 
except bytes vs bits. Having both just creates confusion and 
potential inconsistency and makes not much sense to me.


Valerie

- Original Message -
From: xuelei@oracle.com
To: valerie.p...@oracle.com, security-dev@openjdk.java.net
Sent: Thursday, September 19, 2019 7:47:43 AM GMT -08:00 US/Canada 
Pacific
Subject: Re: [14] RFR JDK-8229243 "SunPKCS11-Solaris provider tests 
failing on Solaris 11.4"


Will the inconsistency structure be continue?  I was just wondering if
OpenHSM2/Solaris/NSS will fix the bug and use one structure in the
future, then we may not need to workaround the issue in the calling 
side.


I had a quick look the PKCS#11 3.0 draft, there is no update of the
structure yet.

Xuelei

On 9/18/2019 6:01 PM, Valerie Peng wrote:

Ping?

Can someone help take a look?

Thanks,

Valerie

On 8/15/2019 4:49 PM, Valerie Peng wrote:


Anyone has time to help review this fix? PKCS#11 v2.40 has
inconsistent definition for CK_GCM_PARAMS struct. The mechanism spec
(sec 2..12.3) has:

 typedef struct CK_GCM_PARAMS {
     CK_BYTE_PTR   pIv;
     CK_ULONG  ulIvLen;
     CK_BYTE_PTR   pAAD;
     CK_ULONG  ulAADLen;
     CK_ULONG  ulTagBits;
 } CK_GCM_PARAMS;

However, the header file pkcs11t.h has an extra "ulIvBits" field:

 typedef struct CK_GCM_PARAMS {
     CK_BYTE_PTR   pIv;
     CK_ULONG  ulIvLen;
 *    CK_ULONG  ulIvBits;*
     CK_BYTE_PTR   pAAD;
     CK_ULONG  ulAADLen;
     CK_ULONG  ulTagBits;
 } CK_GCM_PARAMS;

Some vendors such OpenHSM2 and Solaris go with the header file while
some vendor such as NSS go with the mech spec. Current SunPKCS11
provider impl works with NSS but not with other vendors whose
CK_GCM_PARAMS struct contains ulIvBits field. Based on testing
results, OpenHSM2 and Solaris error out at init time when the
parameter length does not have their expected value. Thus, one way to
workaround this inconsistency is to re-try with a different structure
for AES GCM when the init failed. In addition, given the parameters
contains Iv and AAD which some vendor may use during subsequent
enc/dec operations, I changed to use the same model as RSASSA-PSS to
defer the freeing after the enc/dec operation is finished. Verified
the changes on Solaris 11.4 against existing GCM regression tests.

Bug: https://bugs.openjdk.java.net/browse/JDK-8229243

Webrev: http://cr.openjdk.java.net/~valeriep/8229243/webrev.00/

Thanks,
Valerie

Re: RFR (XS) JDK-8231387 "java.security.Provider.getService returns random result due to race condition with mutating methods in the same class"

[Valerie Peng]

Great, the update looks good.

Thanks,
Valerie
On 9/24/2019 5:49 PM, Hohensee, Paul wrote:


Thank you, Valerie. :)

The patch needs a test, however, so I added a slightly modified 
version of Tianmin’s reproducer. New webrev at


http://cr.openjdk.java.net/~phh/8231387/webrev.01/

The test fails without the fix.

*From: *Valerie Peng 
*Organization: *Oracle Corporation
*Date: *Tuesday, September 24, 2019 at 3:03 PM
*To: *"Hohensee, Paul" , 
"security-dev@openjdk.java.net" 
*Subject: *Re: RFR (XS) JDK-8231387 "java.security.Provider.getService 
returns random result due to race condition with mutating methods in 
the same class"


I am the security reviewer that you need. One should be enough. ;)

Valerie

On 9/24/2019 2:37 PM, Hohensee, Paul wrote:

Yes, I’ll sponsor. Though it looks ok to me, I’m not a security
expert, so do we need another security reviewer?

*From: *security-dev 
 on behalf of
Valerie Peng 

*Organization: *Oracle Corporation
*Date: *Tuesday, September 24, 2019 at 2:17 PM
*To: *"security-dev@openjdk.java.net"

 
*Subject: *Re: RFR (XS) JDK-8231387
"java.security.Provider.getService returns random result due to
race condition with mutating methods in the same class"

Changes look fine. I suppose Paul will help sponsoring the fix as
he is listed as the RE for 8231387?

Thanks,
Valerie

On 9/24/2019 12:36 PM, Shi, Tianmin wrote:

Hi

Can someone help reviewing this fix?

Bug: https://bugs.openjdk.java.net/browse/JDK-8231387

Webrev: http://cr.openjdk.java.net/~phh/8231387/webrev.00/

Thank you,

Tianmin Shi

Re: [14] RFR JDK-8229243 "SunPKCS11-Solaris provider tests failing on Solaris 11.4"

[Xuelei Fan]

I looked at the latest webrev:
  http://cr.openjdk.java.net/~valeriep/8229243/webrev.01/

Which is good to me.

Thanks,
Xuelei

On 9/19/2019 5:46 PM, Valerie Peng wrote:


I am not on the PKCS#11 committee and not sure about the plan.
As for which one is right, I am more inclined to the "spec is right" side which 
is also what NSS picked.
Comparing between spec and header, shouldn't the former get more eyeballs in 
terms of review?
The header file also has a deprecated structure CK_AES_GCM_PARAMS which 
contains both ulIvLen and ulIvBits fields.
As ulIvBits and ulIvLen are essentially same in terms of meaning except bytes 
vs bits. Having both just creates confusion and potential inconsistency and 
makes not much sense to me.

Valerie

- Original Message -
From: xuelei@oracle.com
To: valerie.p...@oracle.com, security-dev@openjdk.java.net
Sent: Thursday, September 19, 2019 7:47:43 AM GMT -08:00 US/Canada Pacific
Subject: Re: [14] RFR JDK-8229243 "SunPKCS11-Solaris provider tests failing on 
Solaris 11.4"

Will the inconsistency structure be continue?  I was just wondering if
OpenHSM2/Solaris/NSS will fix the bug and use one structure in the
future, then we may not need to workaround the issue in the calling side.

I had a quick look the PKCS#11 3.0 draft, there is no update of the
structure yet.

Xuelei

On 9/18/2019 6:01 PM, Valerie Peng wrote:

Ping?

Can someone help take a look?

Thanks,

Valerie

On 8/15/2019 4:49 PM, Valerie Peng wrote:


Anyone has time to help review this fix? PKCS#11 v2.40 has
inconsistent definition for CK_GCM_PARAMS struct. The mechanism spec
(sec 2..12.3) has:

 typedef struct CK_GCM_PARAMS {
     CK_BYTE_PTR   pIv;
     CK_ULONG  ulIvLen;
     CK_BYTE_PTR   pAAD;
     CK_ULONG  ulAADLen;
     CK_ULONG  ulTagBits;
 } CK_GCM_PARAMS;

However, the header file pkcs11t.h has an extra "ulIvBits" field:

 typedef struct CK_GCM_PARAMS {
     CK_BYTE_PTR   pIv;
     CK_ULONG  ulIvLen;
 *    CK_ULONG  ulIvBits;*
     CK_BYTE_PTR   pAAD;
     CK_ULONG  ulAADLen;
     CK_ULONG  ulTagBits;
 } CK_GCM_PARAMS;

Some vendors such OpenHSM2 and Solaris go with the header file while
some vendor such as NSS go with the mech spec. Current SunPKCS11
provider impl works with NSS but not with other vendors whose
CK_GCM_PARAMS struct contains ulIvBits field. Based on testing
results, OpenHSM2 and Solaris error out at init time when the
parameter length does not have their expected value. Thus, one way to
workaround this inconsistency is to re-try with a different structure
for AES GCM when the init failed. In addition, given the parameters
contains Iv and AAD which some vendor may use during subsequent
enc/dec operations, I changed to use the same model as RSASSA-PSS to
defer the freeing after the enc/dec operation is finished. Verified
the changes on Solaris 11.4 against existing GCM regression tests.

Bug: https://bugs.openjdk.java.net/browse/JDK-8229243

Webrev: http://cr.openjdk.java.net/~valeriep/8229243/webrev.00/

Thanks,
Valerie

Re: RFR: 8231351: Add notes for PKCS11 tests in the test doc

[Jia Huang]

Thanks Erik.


在 2019年09月24日 23:19, Erik Joelsson 写道:

Sure, will do.

/Erik 

Re: RFR (XS) JDK-8231387 "java.security.Provider.getService returns random result due to race condition with mutating methods in the same class"

[Hohensee, Paul]

Thank you, Valerie. :)

The patch needs a test, however, so I added a slightly modified version of 
Tianmin’s reproducer. New webrev at

http://cr.openjdk.java.net/~phh/8231387/webrev.01/

The test fails without the fix.

From: Valerie Peng 
Organization: Oracle Corporation
Date: Tuesday, September 24, 2019 at 3:03 PM
To: "Hohensee, Paul" , "security-dev@openjdk.java.net" 

Subject: Re: RFR (XS) JDK-8231387 "java.security.Provider.getService returns 
random result due to race condition with mutating methods in the same class"


I am the security reviewer that you need. One should be enough. ;)

Valerie
On 9/24/2019 2:37 PM, Hohensee, Paul wrote:
Yes, I’ll sponsor. Though it looks ok to me, I’m not a security expert, so do 
we need another security reviewer?

From: security-dev 

 on behalf of Valerie Peng 

Organization: Oracle Corporation
Date: Tuesday, September 24, 2019 at 2:17 PM
To: "security-dev@openjdk.java.net" 

Subject: Re: RFR (XS) JDK-8231387 "java.security.Provider.getService returns 
random result due to race condition with mutating methods in the same class"




Changes look fine. I suppose Paul will help sponsoring the fix as he is listed 
as the RE for 8231387?
Thanks,
Valerie
On 9/24/2019 12:36 PM, Shi, Tianmin wrote:

Hi



Can someone help reviewing this fix?


Bug: https://bugs.openjdk.java.net/browse/JDK-8231387
Webrev: http://cr.openjdk.java.net/~phh/8231387/webrev.00/



Thank you,
Tianmin Shi

Re: RFR (XS) JDK-8231387 "java.security.Provider.getService returns random result due to race condition with mutating methods in the same class"

[Valerie Peng]

I am the security reviewer that you need. One should be enough. ;)

Valerie

On 9/24/2019 2:37 PM, Hohensee, Paul wrote:


Yes, I’ll sponsor. Though it looks ok to me, I’m not a security 
expert, so do we need another security reviewer?


*From: *security-dev  on behalf 
of Valerie Peng 

*Organization: *Oracle Corporation
*Date: *Tuesday, September 24, 2019 at 2:17 PM
*To: *"security-dev@openjdk.java.net" 
*Subject: *Re: RFR (XS) JDK-8231387 "java.security.Provider.getService 
returns random result due to race condition with mutating methods in 
the same class"


Changes look fine. I suppose Paul will help sponsoring the fix as he 
is listed as the RE for 8231387?


Thanks,
Valerie

On 9/24/2019 12:36 PM, Shi, Tianmin wrote:

Hi

Can someone help reviewing this fix?

Bug: https://bugs.openjdk.java.net/browse/JDK-8231387

Webrev: http://cr.openjdk.java.net/~phh/8231387/webrev.00/

Thank you,

Tianmin Shi

Re: RFR (XS) JDK-8231387 "java.security.Provider.getService returns random result due to race condition with mutating methods in the same class"

[Hohensee, Paul]

Yes, I’ll sponsor. Though it looks ok to me, I’m not a security expert, so do 
we need another security reviewer?

From: security-dev  on behalf of Valerie 
Peng 
Organization: Oracle Corporation
Date: Tuesday, September 24, 2019 at 2:17 PM
To: "security-dev@openjdk.java.net" 
Subject: Re: RFR (XS) JDK-8231387 "java.security.Provider.getService returns 
random result due to race condition with mutating methods in the same class"




Changes look fine. I suppose Paul will help sponsoring the fix as he is listed 
as the RE for 8231387?
Thanks,
Valerie
On 9/24/2019 12:36 PM, Shi, Tianmin wrote:

Hi



Can someone help reviewing this fix?


Bug: https://bugs.openjdk.java.net/browse/JDK-8231387
Webrev: http://cr.openjdk.java.net/~phh/8231387/webrev.00/



Thank you,
Tianmin Shi

Re: RFR [XS] 8231357: sun/security/pkcs11/Cipher/TestKATForGCM.java fails on SLES11 using mozilla-nss-3.14

[Valerie Peng]

Hi Matthias,

The output on line 324 may report inconsistent info - the provider in 
use may not be NSS but yet you put NSS version with provider name?


Since you are updating the code here, how about re-org the code a bit 
and first check for whether NSS provider is used then do the various 
version-specific handling.


Regards,

Valerie


On 9/24/2019 5:39 AM, Langer, Christoph wrote:


Hi Matthias,

looks good to me.

One minor style nit: You could close the else case in line 335 with 
“}” and then have just one throw e; after the whole if else block.


Best regards

Christoph

*From:*Baesken, Matthias 
*Sent:* Dienstag, 24. September 2019 10:30
*To:* Langer, Christoph ; 
security-dev@openjdk.java.net

*Cc:* Zeller, Arno 
*Subject:* RE: RFR [XS] 8231357: 
sun/security/pkcs11/Cipher/TestKATForGCM.java fails on SLES11 using 
mozilla-nss-3.14


New webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8231357.1/

I adjusted the imports and  now output a warning .

Best regards, Matthias

*From:*Baesken, Matthias
*Sent:* Montag, 23. September 2019 16:07
*To:* Langer, Christoph >; security-dev@openjdk.java.net 


*Cc:* Zeller, Arno mailto:arno.zel...@sap.com>>
*Subject:* RE: RFR [XS] 8231357: 
sun/security/pkcs11/Cipher/TestKATForGCM.java fails on SLES11 using 
mozilla-nss-3.14


Hi Christoph,   I am okay  with your suggestion to   just  print  a  
message in  case that  an  older nss lib  < 3.15 is found ; however   
let’s  see what others think .


(but for Solaris  we let the test pass when   noticing  old nss 
versions , so I thought  it might make some sense to behave on Linux 
in the same way )


Best regards, Matthias

*From:*Langer, Christoph >

*Sent:* Montag, 23. September 2019 15:53
*To:* Baesken, Matthias >; security-dev@openjdk.java.net 


*Cc:* Zeller, Arno mailto:arno.zel...@sap.com>>
*Subject:* RE: RFR [XS] 8231357: 
sun/security/pkcs11/Cipher/TestKATForGCM.java fails on SLES11 using 
mozilla-nss-3.14


Hi Matthias,

generally I agree that it’s a good idea to have more diagnostic output 
in case of failures in this test.


But, given that there’s an upgrade path even on our old Linux SLES 
11.3 system to a newer libnss that has a fix for the bug that we 
observe, I suggest that the test should still fail with libnss 3.14. 
So I suggest you only add the line


System.out.println("Exception occured using " + p.getName() + " 
version " + ver);


and maybe a comment stating that libnss 3.14 on Linux isn’t good for 
this test.


BTW, if you want to clean up the testcase a bit, you might remove line 
36, import java.math.*; because it’s not needed. Or replace all the 
imports with:


import java.security.GeneralSecurityException;

import java.security.Provider;

import java.util.Arrays;

import javax.crypto.Cipher;

import javax.crypto.SecretKey;

import javax.crypto.spec.GCMParameterSpec;

import javax.crypto.spec.SecretKeySpec;

Thanks

Christoph

*From:*Baesken, Matthias >

*Sent:* Montag, 23. September 2019 15:16
*To:* security-dev@openjdk.java.net 
*Cc:* Langer, Christoph >; Zeller, Arno >
*Subject:* RFR [XS] 8231357: 
sun/security/pkcs11/Cipher/TestKATForGCM.java fails on SLES11 using 
mozilla-nss-3.14


Hello,  please review  this small test related change .

We  noticed that  on our SLES (SuSE Linux) 11   test  machines, the test

sun/security/pkcs11/Cipher/TestKATForGCM.java

fails when older nss versions  are used on the system , especially 
 nss 3.14 .



The used package is named   mozilla-nss-3.14 .
Upgrading to newer versions (e.g. 3.20)   makes the test succeed , so 
it might be helpful
to add a check in the test like it is done already for old nss 
versions on Solaris.


(nss  3.15 contains a couple  of  AES cipher with GCM   related fixes, 
those might be the ones needed to run the test successfully ).


Bug/webrev :

https://bugs.openjdk.java.net/browse/JDK-8231357

http://cr.openjdk.java.net/~mbaesken/webrevs/8231357.0/

Thanks, Matthias

Re: RFR (XS) JDK-8231387 "java.security.Provider.getService returns random result due to race condition with mutating methods in the same class"

[Valerie Peng]

Changes look fine. I suppose Paul will help sponsoring the fix as he is 
listed as the RE for 8231387?


Thanks,
Valerie
On 9/24/2019 12:36 PM, Shi, Tianmin wrote:

Hi
Can someone help reviewing this fix?

Bug: https://bugs.openjdk.java.net/browse/JDK-8231387

Webrev: http://cr.openjdk.java.net/~phh/8231387/webrev.00/

Thank you,

Tianmin Shi

RFR (XS) JDK-8231387 "java.security.Provider.getService returns random result due to race condition with mutating methods in the same class"

[Shi, Tianmin]

Hi



Can someone help reviewing this fix?


Bug: https://bugs.openjdk.java.net/browse/JDK-8231387
Webrev: http://cr.openjdk.java.net/~phh/8231387/webrev.00/



Thank you,
Tianmin Shi

Re: JDK 14 RFR of JDK-8231368: Suppress warnings on non-serializable non-transient instance fields in java.security.jgss

[Seán Coffey]

Looks fine Joe.

regards,
Sean.

On 23/09/2019 17:15, Joe Darcy wrote:

Hello,

Another module, another review request as part of making serial 
warnings more robust:


    JDK-8231368: Suppress warnings on non-serializable non-transient 
instance fields in java.security.jgss

    http://cr.openjdk.java.net/~darcy/8231368.0/

(Related earlier review 
https://mail.openjdk.java.net/pipermail/security-dev/2019-September/020672.html.)


In this latest review, I included a comment in KRBError.java that its 
writeObject method uses a different encoding scheme.


Thanks,

-Joe

Re: RFR: 8231351: Add notes for PKCS11 tests in the test doc

[Erik Joelsson]

Sure, will do.

/Erik

On 2019-09-23 20:57, Jia Huang wrote:

Hi Erik,

Thank you for your review and valuable comments.

Updated: http://cr.openjdk.java.net/~jiefu/8231351-huangjia/webrev.01/
 - The reference to the pkcs11 README and the reviewers had been added.

Please note the user in the patch.
Hope you wouldn't mind it. Thanks.

Could you please sponsor it?

Thanks a lot.
Best regards,
Jia

在 2019年09月23日 23:18, Erik Joelsson 写道:
I think this type of comment fits well in the top level test doc. It 
just provides basic instructions for setting up these tests so that 
they pass without going into too much detail. Perhaps a reference to 
the pkcs11 README for more details would be a good idea.


Looks good to me.

/Erik

On 2019-09-23 05:54, sha.ji...@oracle.com wrote:

Hi Jia,
I think this isn't a general testing problem.
It may not worthy of highlighting this point in the JDK testing doc.
In fact, PKCS11 tests have their own doc at: 
test/jdk/sun/security/pkcs11/README


Best regards,
John Jiang

On 2019/9/23 18:04, Jia Huang wrote:

Hi all,

JBS:    https://bugs.openjdk.java.net/browse/JDK-8231351
Webrev: http://cr.openjdk.java.net/~jiefu/8231351-huangjia/webrev.00/

sun/security/pkcs11/Secmod/AddTrustedCert.java failed on Ubuntu 18.04.
According to the comments in JDK-8231338, it was caused by the 
improper NSS libs of the system.


These failures are confusing and hard to diagnose.
It might be better to add some notes for the pkcs11 tests.

Thanks a lot.

Best regards,
Jia

RE: RFR [XS] 8231357: sun/security/pkcs11/Cipher/TestKATForGCM.java fails on SLES11 using mozilla-nss-3.14

[Langer, Christoph]

Hi Matthias,

looks good to me.

One minor style nit: You could close the else case in line 335 with "}" and 
then have just one throw e; after the whole if else block.

Best regards
Christoph

From: Baesken, Matthias 
Sent: Dienstag, 24. September 2019 10:30
To: Langer, Christoph ; security-dev@openjdk.java.net
Cc: Zeller, Arno 
Subject: RE: RFR [XS] 8231357: sun/security/pkcs11/Cipher/TestKATForGCM.java 
fails on SLES11 using mozilla-nss-3.14

New webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8231357.1/

I adjusted the imports and  now output a warning .

Best regards, Matthias

From: Baesken, Matthias
Sent: Montag, 23. September 2019 16:07
To: Langer, Christoph 
mailto:christoph.lan...@sap.com>>; 
security-dev@openjdk.java.net
Cc: Zeller, Arno mailto:arno.zel...@sap.com>>
Subject: RE: RFR [XS] 8231357: sun/security/pkcs11/Cipher/TestKATForGCM.java 
fails on SLES11 using mozilla-nss-3.14

Hi Christoph,   I am okay  with your suggestion to   just  print  a  message  
in  case that  an  older nss lib  < 3.15 is found ;  however   let's  see what 
others think .
(but for Solaris  we let the test pass when   noticing  old nss versions ,  so 
I thought  it might make some sense to behave on Linux in the same way )


Best regards, Matthias



From: Langer, Christoph 
mailto:christoph.lan...@sap.com>>
Sent: Montag, 23. September 2019 15:53
To: Baesken, Matthias 
mailto:matthias.baes...@sap.com>>; 
security-dev@openjdk.java.net
Cc: Zeller, Arno mailto:arno.zel...@sap.com>>
Subject: RE: RFR [XS] 8231357: sun/security/pkcs11/Cipher/TestKATForGCM.java 
fails on SLES11 using mozilla-nss-3.14

Hi Matthias,

generally I agree that it's a good idea to have more diagnostic output in case 
of failures in this test.

But, given that there's an upgrade path even on our old Linux SLES 11.3 system 
to a newer libnss that has a fix for the bug that we observe, I suggest that 
the test should still fail with libnss 3.14. So I suggest you only add the line
System.out.println("Exception occured using " + p.getName() + " version " + 
ver);
and maybe a comment stating that libnss 3.14 on Linux isn't good for this test.

BTW, if you want to clean up the testcase a bit, you might remove line 36, 
import java.math.*; because it's not needed. Or replace all the imports with:

import java.security.GeneralSecurityException;
import java.security.Provider;
import java.util.Arrays;

import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.spec.GCMParameterSpec;
import javax.crypto.spec.SecretKeySpec;

Thanks
Christoph


From: Baesken, Matthias 
mailto:matthias.baes...@sap.com>>
Sent: Montag, 23. September 2019 15:16
To: security-dev@openjdk.java.net
Cc: Langer, Christoph 
mailto:christoph.lan...@sap.com>>; Zeller, Arno 
mailto:arno.zel...@sap.com>>
Subject: RFR [XS] 8231357: sun/security/pkcs11/Cipher/TestKATForGCM.java fails 
on SLES11 using mozilla-nss-3.14

Hello,  please review  this small test related change .

We  noticed that  on our SLES (SuSE Linux) 11   test  machines, the test

sun/security/pkcs11/Cipher/TestKATForGCM.java

fails when older nss versions  are used on the system , especially  nss 3.14 .

The used package is named   mozilla-nss-3.14 .
Upgrading to newer versions (e.g. 3.20)   makes the test succeed , so it might 
be helpful
to add a check in the test like it is done already for old nss versions on 
Solaris.

(nss  3.15  contains a couple  of  AES cipher with GCMrelated fixes, those 
might be the ones needed to run the test successfully ).



Bug/webrev :

https://bugs.openjdk.java.net/browse/JDK-8231357

http://cr.openjdk.java.net/~mbaesken/webrevs/8231357.0/


Thanks, Matthias

Re: [14] RFR 8228659: Record which Java methods are called by native codes in JGSS and JAAS

[Sean Mullan]

Just a couple of minor comments:

1. For comments like this:

// Warning: Used by NativeCreds.c and nativeccache.c

I think not capitalizing "Used" would be better. Also, would "called by" 
be more appropriate? Result:


// Warning: called by NativeCreds.c and nativeccache.c

2. Sometimes you end a comment with a period, and other times you don't:

// Warning: The following 4 fields are used by Unix.c.

I think removing the period reads better:

// Warning: The following 4 fields are used by Unix.c


On 8/19/19 10:35 AM, Weijun Wang wrote:

Ping again.


On Jul 26, 2019, at 11:24 PM, Weijun Wang  wrote:

Please review the change at

   http://cr.openjdk.java.net/~weijun/8228659/webrev.00/

Most of the change is new comments on internal Java methods called by native 
code. I also take this chance to simply Ticket creation in nativeccache.c and 
NativeCreds.c. There is no need to call `new Ticket(new DerValue(byteArray))` 
which is identical to `new Ticket(byteArray)`.

I added the label noreg-doc but does not feel really comfortable. There is no 
public document here. noreg-comment or noreg-clarification would be better, if 
they exists. Please advise.


You have changed more than comments so noreg-doc is not appropriate. How 
about noreg-cleanup?


--Sean

Re: RFR: 8231351: Add notes for PKCS11 tests in the test doc

[Jie Fu]

Hi John Jiang,

I don't care which NSS you have skipped.
I am just curious about the root cause for the failure on Ubuntu.

It seems more reasonable to close JDK-8231338 as won't fix.
But you just closed it as can't reproduce.

Thanks.
Best regards,
Jie

On 2019/9/24 下午4:01, sha.ji...@oracle.com wrote:

Hi Jie,
IIRC, this test passed on your Ubuntu 18.04 with a new built NSS 3.35 
libs.
So, I suspected your Linux or the system built-in NSS libs had 
something wrong.
Now that this test passed with NSS 3.35 on others' Linux, including 
mine, it may not make sense that this test is skipped for NSS 3.35 on 
all platforms (even Linux only).

Certainly, you are always welcomed to re-open JDK-8231338.

A bit furthermore...
Some different PKCS11 test cases have been skipped due to the bugs on 
different NSS releases or platforms.
It would be better not to make the codes more complicated to deal with 
this scenario.
The alternative NSS libs could be specified (via system property 
test.nss.lib.paths) for PKCS11 tests.

It doesn't have to depend on the system built-in NSS libs.
Different people can use different NSS versions based on their 
requirements.


Best regards,
John Jiang

On 2019/9/24 15:03, Jie Fu wrote:
I can't understand why JDK-8231338 was closed as "Cannot Reproduce" 
since it can always be reproduced on Ubuntu 18.04.2 LTS.


Reproduce on Ubuntu 18.04 is very simple:
-
make test TEST="jtreg:sun/security/pkcs11/Secmod/AddTrustedCert.java" 
CONF=re

-

In the fix of JDK-8180837, the NSS-3.35 was assumed to work with 
AddTrustedCert.java, but it failed on Ubuntu 18.04.
I wonder whether there is some other reasons which may lead to the 
failure.

So I filed JDK-8231338 hoping to find the root cause of the failure.

But I was disappointed since the author of JDK-8180837 just told me:
 "it's hard to say what's the problem, Linux, NSS build or others, on 
this test case."


In fact, the root cause for the failure on Ubuntu 18.04 still remains 
unknown.


Thanks a lot.
Best regards,
Jie

On 2019/9/24 下午2:01, Xuelei Fan wrote:
I may be a little bit hesitate to add such words, "highly 
recommended to use the latest NSS version ...", in the general TOP 
doc.  There are a few issues that I wary about:


It is not always expected that all PKCS 11 test should be run on 
latest NSS version.  Otherwise, there might be compatibility issues 
that we did not handle properly.  The JDK is expected to work with 
as much NSS versions as possible, not just the latest version.  It 
is good to know which version does not really work because of a 
specific bug.


The note should be added as close as to the place where the issue 
happens, for maintaining and searching.  I think the best place 
could be the test code where the failure occurs 
(test/jdk/sun/security/pkcs11/Secmod/AddTrustedCert.java?).


However, I'm still not sure if we really want this note.

JDK-8231338 is reported with NSS 3.35.  Per the comment, the test 
could pass with NSS 3.35 on Debian and Ubuntu, and the bug submitter 
could pass the test with a proper build of NSS 3.35. And then the 
bug was closed as "Cannot Reproduce".  I think we are done with the 
bug.  It might not be necessary to add a note any more.


Just my $.02.

Xuelei

On 9/23/2019 9:06 PM, Jia Huang wrote:

Hi John Jiang,

Thank you for your review.

在 2019年09月23日 20:54, sha.ji...@oracle.com 写道:
In fact, PKCS11 tests have their own doc at: 
test/jdk/sun/security/pkcs11/README 
I'm afraid most people wouldn't see 
test/jdk/sun/security/pkcs11/README at all.

So it makes very little sense to add the notes in it.
I still prefer doc/testing.md.

A reference to test/jdk/sun/security/pkcs11/README had been added 
in [1].


Thanks a lot.
Best regards,
Jia

[1] http://cr.openjdk.java.net/~jiefu/8231351-huangjia/webrev.01/

RE: RFR [XS] 8231357: sun/security/pkcs11/Cipher/TestKATForGCM.java fails on SLES11 using mozilla-nss-3.14

[Baesken, Matthias]

New webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8231357.1/

I adjusted the imports and  now output a warning .

Best regards, Matthias

From: Baesken, Matthias
Sent: Montag, 23. September 2019 16:07
To: Langer, Christoph ; security-dev@openjdk.java.net
Cc: Zeller, Arno 
Subject: RE: RFR [XS] 8231357: sun/security/pkcs11/Cipher/TestKATForGCM.java 
fails on SLES11 using mozilla-nss-3.14

Hi Christoph,   I am okay  with your suggestion to   just  print  a  message  
in  case that  an  older nss lib  < 3.15 is found ;  however   let's  see what 
others think .
(but for Solaris  we let the test pass when   noticing  old nss versions ,  so 
I thought  it might make some sense to behave on Linux in the same way )


Best regards, Matthias



From: Langer, Christoph 
mailto:christoph.lan...@sap.com>>
Sent: Montag, 23. September 2019 15:53
To: Baesken, Matthias 
mailto:matthias.baes...@sap.com>>; 
security-dev@openjdk.java.net
Cc: Zeller, Arno mailto:arno.zel...@sap.com>>
Subject: RE: RFR [XS] 8231357: sun/security/pkcs11/Cipher/TestKATForGCM.java 
fails on SLES11 using mozilla-nss-3.14

Hi Matthias,

generally I agree that it's a good idea to have more diagnostic output in case 
of failures in this test.

But, given that there's an upgrade path even on our old Linux SLES 11.3 system 
to a newer libnss that has a fix for the bug that we observe, I suggest that 
the test should still fail with libnss 3.14. So I suggest you only add the line
System.out.println("Exception occured using " + p.getName() + " version " + 
ver);
and maybe a comment stating that libnss 3.14 on Linux isn't good for this test.

BTW, if you want to clean up the testcase a bit, you might remove line 36, 
import java.math.*; because it's not needed. Or replace all the imports with:

import java.security.GeneralSecurityException;
import java.security.Provider;
import java.util.Arrays;

import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.spec.GCMParameterSpec;
import javax.crypto.spec.SecretKeySpec;

Thanks
Christoph


From: Baesken, Matthias 
mailto:matthias.baes...@sap.com>>
Sent: Montag, 23. September 2019 15:16
To: security-dev@openjdk.java.net
Cc: Langer, Christoph 
mailto:christoph.lan...@sap.com>>; Zeller, Arno 
mailto:arno.zel...@sap.com>>
Subject: RFR [XS] 8231357: sun/security/pkcs11/Cipher/TestKATForGCM.java fails 
on SLES11 using mozilla-nss-3.14

Hello,  please review  this small test related change .

We  noticed that  on our SLES (SuSE Linux) 11   test  machines, the test

sun/security/pkcs11/Cipher/TestKATForGCM.java

fails when older nss versions  are used on the system , especially  nss 3.14 .

The used package is named   mozilla-nss-3.14 .
Upgrading to newer versions (e.g. 3.20)   makes the test succeed , so it might 
be helpful
to add a check in the test like it is done already for old nss versions on 
Solaris.

(nss  3.15  contains a couple  of  AES cipher with GCMrelated fixes, those 
might be the ones needed to run the test successfully ).



Bug/webrev :

https://bugs.openjdk.java.net/browse/JDK-8231357

http://cr.openjdk.java.net/~mbaesken/webrevs/8231357.0/


Thanks, Matthias

Re: RFR: 8231351: Add notes for PKCS11 tests in the test doc

[sha . jiang]

Hi Jie,
IIRC, this test passed on your Ubuntu 18.04 with a new built NSS 3.35 libs.
So, I suspected your Linux or the system built-in NSS libs had something 
wrong.
Now that this test passed with NSS 3.35 on others' Linux, including 
mine, it may not make sense that this test is skipped for NSS 3.35 on 
all platforms (even Linux only).

Certainly, you are always welcomed to re-open JDK-8231338.

A bit furthermore...
Some different PKCS11 test cases have been skipped due to the bugs on 
different NSS releases or platforms.
It would be better not to make the codes more complicated to deal with 
this scenario.
The alternative NSS libs could be specified (via system property 
test.nss.lib.paths) for PKCS11 tests.

It doesn't have to depend on the system built-in NSS libs.
Different people can use different NSS versions based on their requirements.

Best regards,
John Jiang

On 2019/9/24 15:03, Jie Fu wrote:
I can't understand why JDK-8231338 was closed as "Cannot Reproduce" 
since it can always be reproduced on Ubuntu 18.04.2 LTS.


Reproduce on Ubuntu 18.04 is very simple:
-
make test TEST="jtreg:sun/security/pkcs11/Secmod/AddTrustedCert.java" 
CONF=re

-

In the fix of JDK-8180837, the NSS-3.35 was assumed to work with 
AddTrustedCert.java, but it failed on Ubuntu 18.04.
I wonder whether there is some other reasons which may lead to the 
failure.

So I filed JDK-8231338 hoping to find the root cause of the failure.

But I was disappointed since the author of JDK-8180837 just told me:
 "it's hard to say what's the problem, Linux, NSS build or others, on 
this test case."


In fact, the root cause for the failure on Ubuntu 18.04 still remains 
unknown.


Thanks a lot.
Best regards,
Jie

On 2019/9/24 下午2:01, Xuelei Fan wrote:
I may be a little bit hesitate to add such words, "highly recommended 
to use the latest NSS version ...", in the general TOP doc.  There 
are a few issues that I wary about:


It is not always expected that all PKCS 11 test should be run on 
latest NSS version.  Otherwise, there might be compatibility issues 
that we did not handle properly.  The JDK is expected to work with as 
much NSS versions as possible, not just the latest version.  It is 
good to know which version does not really work because of a specific 
bug.


The note should be added as close as to the place where the issue 
happens, for maintaining and searching.  I think the best place could 
be the test code where the failure occurs 
(test/jdk/sun/security/pkcs11/Secmod/AddTrustedCert.java?).


However, I'm still not sure if we really want this note.

JDK-8231338 is reported with NSS 3.35.  Per the comment, the test 
could pass with NSS 3.35 on Debian and Ubuntu, and the bug submitter 
could pass the test with a proper build of NSS 3.35. And then the bug 
was closed as "Cannot Reproduce".  I think we are done with the bug.  
It might not be necessary to add a note any more.


Just my $.02.

Xuelei

On 9/23/2019 9:06 PM, Jia Huang wrote:

Hi John Jiang,

Thank you for your review.

在 2019年09月23日 20:54, sha.ji...@oracle.com 写道:
In fact, PKCS11 tests have their own doc at: 
test/jdk/sun/security/pkcs11/README 
I'm afraid most people wouldn't see 
test/jdk/sun/security/pkcs11/README at all.

So it makes very little sense to add the notes in it.
I still prefer doc/testing.md.

A reference to test/jdk/sun/security/pkcs11/README had been added in 
[1].


Thanks a lot.
Best regards,
Jia

[1] http://cr.openjdk.java.net/~jiefu/8231351-huangjia/webrev.01/

Re: RFR: 8231351: Add notes for PKCS11 tests in the test doc

[Jie Fu]

I can't understand why JDK-8231338 was closed as "Cannot Reproduce" 
since it can always be reproduced on Ubuntu 18.04.2 LTS.


Reproduce on Ubuntu 18.04 is very simple:
-
make test TEST="jtreg:sun/security/pkcs11/Secmod/AddTrustedCert.java" 
CONF=re

-

In the fix of JDK-8180837, the NSS-3.35 was assumed to work with 
AddTrustedCert.java, but it failed on Ubuntu 18.04.

I wonder whether there is some other reasons which may lead to the failure.
So I filed JDK-8231338 hoping to find the root cause of the failure.

But I was disappointed since the author of JDK-8180837 just told me:
 "it's hard to say what's the problem, Linux, NSS build or others, on 
this test case."


In fact, the root cause for the failure on Ubuntu 18.04 still remains 
unknown.


Thanks a lot.
Best regards,
Jie

On 2019/9/24 下午2:01, Xuelei Fan wrote:
I may be a little bit hesitate to add such words, "highly recommended 
to use the latest NSS version ...", in the general TOP doc.  There are 
a few issues that I wary about:


It is not always expected that all PKCS 11 test should be run on 
latest NSS version.  Otherwise, there might be compatibility issues 
that we did not handle properly.  The JDK is expected to work with as 
much NSS versions as possible, not just the latest version.  It is 
good to know which version does not really work because of a specific 
bug.


The note should be added as close as to the place where the issue 
happens, for maintaining and searching.  I think the best place could 
be the test code where the failure occurs 
(test/jdk/sun/security/pkcs11/Secmod/AddTrustedCert.java?).


However, I'm still not sure if we really want this note.

JDK-8231338 is reported with NSS 3.35.  Per the comment, the test 
could pass with NSS 3.35 on Debian and Ubuntu, and the bug submitter 
could pass the test with a proper build of NSS 3.35. And then the bug 
was closed as "Cannot Reproduce".  I think we are done with the bug.  
It might not be necessary to add a note any more.


Just my $.02.

Xuelei

On 9/23/2019 9:06 PM, Jia Huang wrote:

Hi John Jiang,

Thank you for your review.

在 2019年09月23日 20:54, sha.ji...@oracle.com 写道:
In fact, PKCS11 tests have their own doc at: 
test/jdk/sun/security/pkcs11/README 
I'm afraid most people wouldn't see 
test/jdk/sun/security/pkcs11/README at all.

So it makes very little sense to add the notes in it.
I still prefer doc/testing.md.

A reference to test/jdk/sun/security/pkcs11/README had been added in 
[1].


Thanks a lot.
Best regards,
Jia

[1] http://cr.openjdk.java.net/~jiefu/8231351-huangjia/webrev.01/