Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-27 Thread Sean Mullan
On 5/22/20 6:38 PM, Xuelei Fan wrote: On 5/22/2020 11:17 AM, Sean Mullan wrote: On 5/22/20 1:55 PM, Xuelei Fan wrote: * test/jdk/sun/security/ssl/X509TrustManagerImpl/TooMuchCAs.java Will this test FAIL if we ever exceed the maximum number of CAs? I think it is important that it does FAIL,

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-22 Thread Xuelei Fan
On 5/22/2020 11:17 AM, Sean Mullan wrote: On 5/22/20 1:55 PM, Xuelei Fan wrote: * test/jdk/sun/security/ssl/X509TrustManagerImpl/TooMuchCAs.java Will this test FAIL if we ever exceed the maximum number of CAs? I think it is important that it does FAIL, as the extension is effectively not

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-22 Thread Sean Mullan
On 5/22/20 1:55 PM, Xuelei Fan wrote: * test/jdk/sun/security/ssl/X509TrustManagerImpl/TooMuchCAs.java Will this test FAIL if we ever exceed the maximum number of CAs? I think it is important that it does FAIL, as the extension is effectively not working anymore and could cause compatibility

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-22 Thread Xuelei Fan
All good comments. I updated the code and CSR accordingly. http://cr.openjdk.java.net/~xuelei/8206925/webrev.05/ On 5/22/2020 8:41 AM, Sean Mullan wrote: On 5/15/20 6:11 PM, Xuelei Fan wrote: New webrev: http://cr.openjdk.java.net/~xuelei/8206925/webrev.04/ *

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-22 Thread Sean Mullan
On 5/15/20 6:11 PM, Xuelei Fan wrote: New webrev: http://cr.openjdk.java.net/~xuelei/8206925/webrev.04/ * src/java.base/share/classes/sun/security/ssl/CertificateAuthoritiesExtension.java 212 SSLLogger.warning( 213 "Too much certificate

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-15 Thread Xuelei Fan
New webrev: http://cr.openjdk.java.net/~xuelei/8206925/webrev.04/ On 5/15/2020 5:27 AM, Sean Mullan wrote: On 5/15/20 1:22 AM, Xuelei Fan wrote: Alexey has some good point about the size limit of the extension.  I added more comments about the compatibility impact and interop impact when

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-15 Thread Sean Mullan
On 5/15/20 1:22 AM, Xuelei Fan wrote: Alexey has some good point about the size limit of the extension.  I added more comments about the compatibility impact and interop impact when there is too much CAs to meet the size limits in CSR, source code and release notes. New webrev:

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-14 Thread Xuelei Fan
Alexey has some good point about the size limit of the extension. I added more comments about the compatibility impact and interop impact when there is too much CAs to meet the size limits in CSR, source code and release notes. New webrev: https://bugs.openjdk.java.net/browse/JDK-8244460 I

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-14 Thread Xuelei Fan
Hi Alexey, Thanks for the reproducer. Would you mind add it to JDK-8206925 for further testing? I think more about if a control number could be helpful. If the certificate authorities can not be fully listed, it cannot be used to indicate the peer certificate selection accuracy. For

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-14 Thread Sean Mullan
For the CSR, why did you check the binary and behavioral boxes for compatibility risk? Otherwise it looks good, and I added my name as Reviewer. I will review the updated webrev later. Please file and add a link to a docs issue to document the new system property. --Sean On 5/13/20 5:20

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-14 Thread Alexey Bakhtin
Just fix a missprint: It should be -Djdk.tls.client.enableCAExtension=true in the reproducer: $JAVA_HOME/bin/java -Djdk.tls.client.enableCAExtension=true -Djavax.net.ssl.trustStore=./cacerts -Djavax.net.ssl.trustStorePassword=changeit HttpsClient https://www.google.com > On 14 May 2020, at

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-14 Thread Alexey Bakhtin
Hello Xuelei, I’ve posted a reproducer for described issue: http://cr.openjdk.java.net/~abakhtin/8206925/ The test passed and returns code=200 from the server in case of CA extension disabled on the client side: $JAVA_HOME/bin/java -Djavax.net.ssl.trustStore=./cacerts

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-13 Thread Xuelei Fan
On 5/13/2020 2:11 PM, Sean Mullan wrote: It is not expected to use this extension regularly. Please let me know if you still prefer to use "enableCAExtension". Also, it is a bit unfortunate that we have to have a system property to enable it. Can we not enable it based on whether the

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-13 Thread Sean Mullan
On 5/13/20 4:16 PM, Xuelei Fan wrote: Updated webrev: http://cr.openjdk.java.net/~xuelei/8206925/webrev.02/ The CSR and release note were updated accordingly, to use the new system property. On 5/13/2020 6:38 AM, Sean Mullan wrote: On 5/12/20 5:43 PM, Xuelei Fan wrote: Updated webrev:

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-13 Thread Xuelei Fan
Hi Alexey, It a good information for interop. Thank you! I will see how to workaround it. Thanks, Xuelei On 5/13/2020 3:00 AM, Alexey Bakhtin wrote: Hello Xuelei, I’m not a reviewer but I have some comment which could be helpful for your implementation. We’ve developed CA Extension in

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-13 Thread Xuelei Fan
Updated webrev: http://cr.openjdk.java.net/~xuelei/8206925/webrev.02/ The CSR and release note were updated accordingly, to use the new system property. On 5/13/2020 6:38 AM, Sean Mullan wrote: On 5/12/20 5:43 PM, Xuelei Fan wrote: Updated webrev:

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-13 Thread Sean Mullan
On 5/12/20 5:43 PM, Xuelei Fan wrote: Updated webrev: http://cr.openjdk.java.net/~xuelei/8206925/webrev.01/ On 5/12/2020 12:40 PM, Sean Mullan wrote: On 5/5/20 2:29 PM, Xuelei Fan wrote: Hi, Could I get the following update reviewed? RFE: https://bugs.openjdk.java.net/browse/JDK-8206925

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-13 Thread Alexey Bakhtin
Hello Xuelei, I’m not a reviewer but I have some comment which could be helpful for your implementation. We’ve developed CA Extension in the OpenJSSE provider [1] and found an issue with a third party server implementations. According to RFC-8446 specification [2] the maximum size of the CA

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-12 Thread Xuelei Fan
Updated webrev: http://cr.openjdk.java.net/~xuelei/8206925/webrev.01/ On 5/12/2020 12:40 PM, Sean Mullan wrote: On 5/5/20 2:29 PM, Xuelei Fan wrote: Hi, Could I get the following update reviewed? RFE: https://bugs.openjdk.java.net/browse/JDK-8206925 CSR:

Re: RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-12 Thread Sean Mullan
On 5/5/20 2:29 PM, Xuelei Fan wrote: Hi, Could I get the following update reviewed? RFE: https://bugs.openjdk.java.net/browse/JDK-8206925 CSR: https://bugs.openjdk.java.net/browse/JDK-821 We have previously used the syntax "enable[Extension]" when naming system properties that enable

RFR JDK-8206925,,Support the certificate_authorities extension

2020-05-05 Thread Xuelei Fan
Hi, Could I get the following update reviewed? RFE: https://bugs.openjdk.java.net/browse/JDK-8206925 CSR: https://bugs.openjdk.java.net/browse/JDK-821 Release-note: https://bugs.openjdk.java.net/browse/JDK-8244460 webrev: http://cr.openjdk.java.net/~xuelei/8206925/webrev.00/ The