RE: RFR: 8209011: [TESTBUG] AArch64: sun/security/pkcs11/Secmod/TestNssDbSqlite.java fails in aarch64 platforms

2018-08-07 Thread Yang Zhang
Hi Weijun Thanks for your review and help of pushing the change for me. I have updated the reviewer. http://cr.openjdk.java.net/~yzhang/8209011/webrev.01/ 8209011: [TESTBUG] AArch64: sun/security/pkcs11/Secmod/TestNssDbSqlite.java fails in aarch64 platforms Summary: Add the path of libnss3.so fo

Re: RFR: 8209011: [TESTBUG] AArch64: sun/security/pkcs11/Secmod/TestNssDbSqlite.java fails in aarch64 platforms

2018-08-07 Thread Weijun Wang
Change looks good. Do you need me to push the change for you? If yes, show me what changeset comment you'd like. Thanks Max > On Aug 8, 2018, at 12:18 PM, Yang Zhang wrote: > > Hi, > > Could you please help review the small fix to jtreg test case in jdk > "sun/security/pkcs11/Secmod/TestNs

RFR: 8209011: [TESTBUG] AArch64: sun/security/pkcs11/Secmod/TestNssDbSqlite.java fails in aarch64 platforms

2018-08-07 Thread Yang Zhang
Hi, Could you please help review the small fix to jtreg test case in jdk "sun/security/pkcs11/Secmod/TestNssDbSqlite.java" ? The failure happens in aarch64 platforms. The path of libnss3.so isn't set correctly. JBS: https://bugs.openjdk.java.net/browse/JDK-8209011 Webrev: http://cr.openjd

Re: RFR 8201290: keytool importcert fails with CertificateParsingException if unknown certificate algorithms should be imported

2018-08-07 Thread Weijun Wang
> On Aug 8, 2018, at 12:58 AM, Sean Mullan wrote: > > On 8/6/18 8:20 PM, Weijun Wang wrote: >>> I'm not seeing how this would be a behavior change if it is a new option, >>> can you add more details on that? If I specify -providerName, intuitively I >>> would expect it would be used, at leas

Exception Reporting Difference

2018-08-07 Thread Bradford Wetmore
Hi Xuelei, We've noticed a significant difference in the way exceptions are being reported/consumed post TLSv1.3 integration. In the original SSLEngine API/implementation, my design was to fail-fast and let the application know of problems immediately, then the app could recover and close b

Re: RFR 6913047: SunPKCS11 memory leak

2018-08-07 Thread Valerie Peng
Hi Martin, Thanks for the update, I will resume the review on this one with your latest webrev. BTW, there is no webrev.07 for your other fix, i.e. JDK-8029661, right? Just checking. Regards, Valerie On 8/3/2018 2:13 PM, Martin Balao wrote: Hi Valerie,  * http://cr.openjdk.java.net/~mba

Re: Code Review Request, JDK-8207009 SSLEngine#closeInbound mentions SSLException when no close_notify is received

2018-08-07 Thread Xuelei Fan
Hi Jamil, Thanks for comments. Here is the updated webrev: http://cr.openjdk.java.net/~xuelei/8207009/webrev.04/ Thanks, Xuelei On 8/7/2018 3:12 PM, Jamil Nimeh wrote: Hi Xuelei, mostly small stuff: * SSLEngineImpl.java o 717: Nit, inbout --> inbound * SSLEngineOutputRecord.java

Re: CSR Review: 8208641: SSLSocket should throw an exception when configuring DTLS

2018-08-07 Thread Xuelei Fan
Hi Tony, The Specification section looks more like the implementation details. We may change the implementation details again in the future. It may be more suitable to move it to the Solution section, or just remove it. In the Specification section, I may just say something like, "No APIs c

Re: CSR Review: 8208641: SSLSocket should throw an exception when configuring DTLS

2018-08-07 Thread Anthony Scarpino
Hi Xuelei, I have updated the csr and I believe I have addressed your comments. thanks Tony On 08/07/2018 01:43 PM, Xuelei Fan wrote: Hi Tony, Would you mind make it clear that this impact the JDK JSSE provider only?  Third party's provider may be able to support DTLS with SSLSocket. I th

Re: Code Review Request, JDK-8207009 SSLEngine#closeInbound mentions SSLException when no close_notify is received

2018-08-07 Thread Jamil Nimeh
Hi Xuelei, mostly small stuff: * SSLEngineImpl.java o 717: Nit, inbout --> inbound * SSLEngineOutputRecord.java o 162, 169: Nit, applicatoin --> application o Same section: It looks like the "if" and "else if" clauses take the same actions with the same message.  Maybe jus

Re: CSR Review: 8208641: SSLSocket should throw an exception when configuring DTLS

2018-08-07 Thread Xuelei Fan
Hi Tony, Would you mind make it clear that this impact the JDK JSSE provider only? Third party's provider may be able to support DTLS with SSLSocket. I think there may be no specification change. The SSLContext.getServerSocketFactory() and SSLContext.getSocketFactory() defines the spec if

Re: CSR Review: 8208641: SSLSocket should throw an exception when configuring DTLS

2018-08-07 Thread Sean Mullan
There are a few typos in the CSR and an unfinished sentence in the Summary. I think the Solution or Specification should say what standard APIs the underlying impl will be throwing an UnsupportedOperationException, i.e. SSLContextSpi.engineGetSocketFactory and engineGetServerSocketFactory. It'

CSR Review: 8208641: SSLSocket should throw an exception when configuring DTLS

2018-08-07 Thread Anthony Scarpino
I need a review of a CSR for SSLSocket should throw an exception when configuring DTLS. We are targeting this for 12 right now. https://bugs.openjdk.java.net/browse/JDK-8209031 thanks Tony

Re: RFR 8201290: keytool importcert fails with CertificateParsingException if unknown certificate algorithms should be imported

2018-08-07 Thread Sean Mullan
On 8/6/18 8:20 PM, Weijun Wang wrote: I'm not seeing how this would be a behavior change if it is a new option, can you add more details on that? If I specify -providerName, intuitively I would expect it would be used, at least as the first one. Before this change when "keytool -importcert" is

JDK-6782021

2018-08-07 Thread Oddbjørn Kvalsund
Hi, I was just bit by this issue [JDK-6782021] It is not possible to read local computer certificates with the SunMSCAPI provider and from StackOverflow I notice that several other people (see [1][2][3]) have come across the same problem. Coming u

Re: [12] RFR 8193859: Allow user provided ObjectInputFilter in SealedObject and SignedObject

2018-08-07 Thread Roger Riggs
Hi Max, It may be useful to include in the descriptions a reminder that if no ObjectInputFilter is supplied the global filter is used.  Details in ObjectInputStream. Typically, the @throws clauses that are not full sentences do not include a final period "." For consistency with the existing

Re: Inconsistent SSLEngine behavior for closing outbound while in handshake in 11ea22

2018-08-07 Thread Xuelei Fan
Hi Tim, Thank you very much for the test code. I can play with it. I made an update accordingly in the new webrev patch: http://cr.openjdk.java.net/~xuelei/8207009/webrev.03/ Would you mind test if the patch works for you? Thanks & Regards, Xuelei On 8/6/2018 5:01 PM, Tim Brooks wrote: Hi X

Re: Code Review Request, JDK-8207009 SSLEngine#closeInbound mentions SSLException when no close_notify is received

2018-08-07 Thread Xuelei Fan
New webrev: http://cr.openjdk.java.net/~xuelei/8207009/webrev.03/ Thanks for a find of Tim Brooks, that the SSLEngine inbound/outbound status is incorrect if closing during handshake. The above webrev is trying to fix the problems. See more in the OpenJDK thread: http://mail.openjdk.java.net

Re: [11] RFR: 8208691: Tighten up jdk.includeInExceptions security property

2018-08-07 Thread Roger Riggs
+1 On 8/7/18 9:09 AM, Sean Mullan wrote: On 8/7/18 3:09 AM, Alan Bateman wrote: On 06/08/2018 20:23, Sean Mullan wrote: After further evaluation of the new jdk.includeInExceptions security property originally introduced in JDK-8204233 [1] and further generalized in JDK-8207846 [2], I felt tha

Re: [11] RFR: 8208691: Tighten up jdk.includeInExceptions security property

2018-08-07 Thread Sean Mullan
On 8/7/18 3:09 AM, Alan Bateman wrote: On 06/08/2018 20:23, Sean Mullan wrote: After further evaluation of the new jdk.includeInExceptions security property originally introduced in JDK-8204233 [1] and further generalized in JDK-8207846 [2], I felt that a stronger warning should be added to th

Re: [11] RFR: 8208691: Tighten up jdk.includeInExceptions security property

2018-08-07 Thread Alan Bateman
On 06/08/2018 20:23, Sean Mullan wrote: After further evaluation of the new jdk.includeInExceptions security property originally introduced in JDK-8204233 [1] and further generalized in JDK-8207846 [2], I felt that a stronger warning should be added to the description of the property alerting t