Yes, latest webrev looks fine to me.
Thanks for the update,
Valerie
On 9/26/2019 2:30 AM, Baesken, Matthias wrote:
Hi Christoph, you are right - I should better use getVersionStr()
, I’ll replace it .
Valerie are you fine with the latest webrev ?
Best regards, Matthias
*From:*Langer, Christoph <christoph.lan...@sap.com>
*Sent:* Mittwoch, 25. September 2019 16:50
*To:* Baesken, Matthias <matthias.baes...@sap.com>; Valerie Peng
<valerie.p...@oracle.com>; security-dev@openjdk.java.net
*Subject:* RE: Re: RFR [XS] 8231357:
sun/security/pkcs11/Cipher/TestKATForGCM.java fails on SLES11 using
mozilla-nss-3.14
Hi Matthias,
looks good.
One minor thing I just recognized:
java.security.Provider::getVersion() is deprecated. Can you please use
p.getVersionStr in line 323? But no need to see another webrev 😊
Best regards
Christoph
*From:*Baesken, Matthias <matthias.baes...@sap.com
<mailto:matthias.baes...@sap.com>>
*Sent:* Mittwoch, 25. September 2019 16:34
*To:* Valerie Peng <valerie.p...@oracle.com
<mailto:valerie.p...@oracle.com>>; security-dev@openjdk.java.net
<mailto:security-dev@openjdk.java.net>
*Cc:* Langer, Christoph <christoph.lan...@sap.com
<mailto:christoph.lan...@sap.com>>
*Subject:* RE: Re: RFR [XS] 8231357:
sun/security/pkcs11/Cipher/TestKATForGCM.java fails on SLES11 using
mozilla-nss-3.14
Hi Valerie / Christoph ,
New webrev :
http://cr.openjdk.java.net/~mbaesken/webrevs/8231357.2/
* Followed the idea of Christoph : “One minor style nit: You could
close the else case in line 335 with “}” and then have just one
throw e;”
* Moved the isNSS(Provider) check in front of the version
checking and potential output
Best regards, Matthias
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 <matthias.baes...@sap.com>
<mailto:matthias.baes...@sap.com>
*Sent:* Dienstag, 24. September 2019 10:30
*To:* Langer, Christoph <christoph.lan...@sap.com>
<mailto:christoph.lan...@sap.com>; security-dev@openjdk.java.net
<mailto:security-dev@openjdk.java.net>
*Cc:* Zeller, Arno <arno.zel...@sap.com> <mailto:arno.zel...@sap.com>
*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 <christoph.lan...@sap.com
<mailto:christoph.lan...@sap.com>>; security-dev@openjdk.java.net
<mailto:security-dev@openjdk.java.net>
*Cc:* Zeller, Arno <arno.zel...@sap.com <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 <christoph.lan...@sap.com
<mailto:christoph.lan...@sap.com>>
*Sent:* Montag, 23. September 2019 15:53
*To:* Baesken, Matthias <matthias.baes...@sap.com
<mailto:matthias.baes...@sap.com>>; security-dev@openjdk.java.net
<mailto:security-dev@openjdk.java.net>
*Cc:* Zeller, Arno <arno.zel...@sap.com <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 <matthias.baes...@sap.com
<mailto:matthias.baes...@sap.com>>
*Sent:* Montag, 23. September 2019 15:16
*To:* security-dev@openjdk.java.net <mailto:security-dev@openjdk.java.net>
*Cc:* Langer, Christoph <christoph.lan...@sap.com
<mailto:christoph.lan...@sap.com>>; Zeller, Arno <arno.zel...@sap.com
<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 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
