Re: RFR 8133910: Some sun/security/tools tests failed.

2016-08-10 Thread Xuelei Fan
Looks fine. Xuelei On 8/11/2016 11:31 AM, Weijun Wang wrote: > Ping again. Thanks. --Max > > On 8/9/2016 15:37, Wang Weijun wrote: >> Please review the fix at >> >> http://cr.openjdk.java.net/~weijun/8133910/webrev.00/ >> >> Basically, "-J-Duser.language=en -J-Duser.country=US" is added to >>

Re: RFR 8133910: Some sun/security/tools tests failed.

2016-08-10 Thread Weijun Wang
Ping again. Thanks. --Max On 8/9/2016 15:37, Wang Weijun wrote: Please review the fix at http://cr.openjdk.java.net/~weijun/8133910/webrev.00/ Basically, "-J-Duser.language=en -J-Duser.country=US" is added to keytool and jarsigner calls wherever output needs to be compared to some English

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

2016-08-10 Thread Xuelei Fan
On 8/11/2016 4:15 AM, Sean Mullan wrote: > On 08/10/2016 12:39 PM, Vincent Ryan wrote: >> Yes they could be merged but the first loop iterates over all the >> certs and the second one iterates over all but the final cert. >> And the special case of a 1-cert chain also needs to be handled. I >>

Re: [9] RFR: 8162484: javax/net/ssl/Stapling/SSLSocketWithStapling.java test fails intermittently with "Address already in use" error

2016-08-10 Thread Jamil Nimeh
Hi Artem, I'm not an official reviewer but the solution for making the servers reject connections rather than stop and start looks pretty fair to me and seems like a nice way to simulate a downed OCSP responder instead of having to bounce it. A couple comments/questions: I'm a bit

[9] RFR: 8162484: javax/net/ssl/Stapling/SSLSocketWithStapling.java test fails intermittently with "Address already in use" error

2016-08-10 Thread Artem Smotrakov
Hello, Please review this update for OCSP stapling tests. The tests use test/java/security/testlibrary/SimpleOCSPServer.java which try to re-use a server port if the server restarted. Looks like sometimes it may cause "Address already in use" error. The patch updates OCSP stapling tests

Re: [9] RFR 8157579: com/sun/crypto/provider/Mac/MacClone.java failed on solaris12(sparcv9 and x86)

2016-08-10 Thread Sean Mullan
On 08/09/2016 10:00 PM, Valerie Peng wrote: OracleUcrypto provider already separates them in two classes: The cloneable libMD digest impls are encapsulated by a class which implements Cloneable and the new un-cloneable Ucrypto digest impls are in a class which does not. I wasn't sure if we can

Re: RFR [9] 8156841: sun.security.pkcs11.SunPKCS11 poller thread retains a strong reference to the context class loader

2016-08-10 Thread Valerie Peng
Changes look fine. Thanks, Valerie On 8/10/2016 8:42 AM, Chris Hegarty wrote: The SunPKCS11 poller thread has no need of any user defined class loader, so should set its context class loader to null before starting, so as to not inadvertently retain a reference to the creating thread’s context

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

2016-08-10 Thread Sean Mullan
On 08/10/2016 12:39 PM, Vincent Ryan wrote: Yes they could be merged but the first loop iterates over all the certs and the second one iterates over all but the final cert. And the special case of a 1-cert chain also needs to be handled. I think it’s a little clearer to leave them separate.

Re: RFR: 8061842: Package jurisdiction policy files as something other than JAR

2016-08-10 Thread Sean Mullan
Hi Brad, Looks pretty good. You should also send this to build-dev to review the Makefile changes. Just a few comments: - src/java.base/share/conf/security/policy/README.txt 17 contain no restrictions on cryptographic strengths, but they must s/must/must be/ 18 specifically activated by

Re: RFR [9] 8156841: sun.security.pkcs11.SunPKCS11 poller thread retains a strong reference to the context class loader

2016-08-10 Thread Jamil Nimeh
Hi Chris, I am not an official reviewer, but this looks pretty straightforward to me. --Jamil On 08/10/2016 08:42 AM, Chris Hegarty wrote: The SunPKCS11 poller thread has no need of any user defined class loader, so should set its context class loader to null before starting, so as to not

Re: [9] RFR: 8159964: Update Tests to verify JDK build for "JDK-8159488 Deprivilege java.xml.crypto"

2016-08-10 Thread Valerie Peng
Changes look fine. Thanks, Valerie On 8/10/2016 2:08 AM, Sibabrata Sahoo wrote: Hi Valerie, I have addressed the suggestion provided by Max to add "==" for the test policy files because " lib/security/default.policy" file is included by default. I am waiting for your suggestion to complete

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

2016-08-10 Thread Vincent Ryan
Yes they could be merged but the first loop iterates over all the certs and the second one iterates over all but the final cert. And the special case of a 1-cert chain also needs to be handled. I think it’s a little clearer to leave them separate. An updated webrev is at:

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

2016-08-10 Thread Seán Coffey
Looks good. Thanks. Regards, Sean. On 10/08/16 17:39, Vincent Ryan wrote: I’ve updated the webrev to include your suggestion: http://cr.openjdk.java.net/~vinnie/8163503/webrev.01/ Thanks. On 10 Aug 2016, at 10:59, Seán Coffey wrote: It would be good if we can

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

2016-08-10 Thread Vincent Ryan
You’re right. This same issue had been reported as an obscure JCK test failure. I created this new bug to clarify the issue. I’ve updated the webrev to include your suggestion: http://cr.openjdk.java.net/~vinnie/8163503/webrev.01/ Thanks. > On 10 Aug 2016, at 01:38, Weijun Wang

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

2016-08-10 Thread Vincent Ryan
I’ve updated the webrev to include your suggestion: http://cr.openjdk.java.net/~vinnie/8163503/webrev.01/ Thanks. > On 10 Aug 2016, at 10:59, Seán Coffey wrote: > > It would be good if we can print the cert class type in the new exception if > the instanceof check

Re: [9]RFR 8163435: Update issue number for SupportedDHKeys.java and UnsupportedDHKeys.java in ProblemList

2016-08-10 Thread Sean Mullan
Looks fine to me. Please add a noreg-trivial or noreg-cleanup label to the bug. --Sean On 08/09/2016 02:35 AM, John Jiang wrote: Hi, In ProblemList.txt, the tests sun/security/pkcs11/KeyAgreement/SupportedDHKeys.java and sun/security/pkcs11/KeyAgreement/UnsupportedDHKeys.java are tracked by

RFR [9] 8156841: sun.security.pkcs11.SunPKCS11 poller thread retains a strong reference to the context class loader

2016-08-10 Thread Chris Hegarty
The SunPKCS11 poller thread has no need of any user defined class loader, so should set its context class loader to null before starting, so as to not inadvertently retain a reference to the creating thread’s context class loader. In other areas that suffered from a similar issue we changed to

Re: RFR : 8147772, 8163104

2016-08-10 Thread Wang Weijun
The changes look fine. Thanks Max > On 2016年8月10日, at 下午9:48, Seán Coffey wrote: > > Looking to backport the following two bug fixes to jdk8u-dev : > > JDK-8147772 Update KerberosTicket to describe behavior if it has been > destroyed and fix NullPointerExceptions >

RFR : 8147772, 8163104

2016-08-10 Thread Seán Coffey
Looking to backport the following two bug fixes to jdk8u-dev : JDK-8147772 Update KerberosTicket to describe behavior if it has been destroyed and fix NullPointerExceptions JDK-8163104 Unexpected NPE still possible on some Kerberos ticket calls The changes are similar to JDK 9 expect that the

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

2016-08-10 Thread Seán Coffey
It would be good if we can print the cert class type in the new exception if the instanceof check fails. Regards, Sean. On 09/08/16 19:14, Vincent Ryan wrote: Please review this fix to improve the error handling for attempts to store a Certificate object in PKCS12 keystore. The PKCS12

RE: [9] RFR: 8159964: Update Tests to verify JDK build for "JDK-8159488 Deprivilege java.xml.crypto"

2016-08-10 Thread Sibabrata Sahoo
Hi Valerie, I have addressed the suggestion provided by Max to add "==" for the test policy files because " lib/security/default.policy" file is included by default. I am waiting for your suggestion to complete this review. Here is the latest webrev: