<div dir="ltr">Hi Xuelei,<div><br></div><div>Thanks for your notification.</div><div><br></div><div>This is on my backlog again but don't have an ETA yet.</div><div><br></div><div>Kind regards,</div><div>Martin.-</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 28, 2018 at 1:14 PM, Xuelei Fan <span dir="ltr"><<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Martin,<br>
<br>
The TLS 1.3 implementation was integrated into the mainline.<br>
<br>
I know you have multiple contributions were pending because of the re-org of the JSSE implementation.  Would you mind to check if your design and implementation need some adjustment?<br>
<br>
I may not reply for your contributions in other email threads.  Please trigger the code review again if the code is ready for the new JSSE implementation.<br>
<br>
Thanks & Regards,<br>
Xuelei<div class="HOEnZb"><div class="h5"><br>
<br>
On 8/11/2017 8:24 AM, Xuelei Fan wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Martin,<br>
<br>
Sorry for the delay.<br>
<br>
I'd like to wait for finalization of TLS 1.3 specification, so that we can get a stable specification of the Certificate Authorities extension.<br>
<br>
For the current design, I did not see much benefit to add a new CertificateAuthority API.  The CertificateAuthority.implies() may not yet reach the threshold to be a public API.  A trust/key manager can easily matching the distinguished name with the target certificate.  And I did not see the cert matching behavior differences between different providers and trust/key managers.<br>
<br>
For the specification documentation part, I may suggest reword them a little bit so that those developers who are not TLS specification experts can easily catch the purpose or benefits of the API.<br>
<br>
Xuelei<br>
<br>
On 7/21/2017 7:20 AM, Martin Balao wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Webrev has been uploaded to <a href="http://cr.openjdk.java.net" rel="noreferrer" target="_blank">cr.openjdk.java.net</a> <<a href="http://cr.openjdk.java.net" rel="noreferrer" target="_blank">http://cr.openjdk.java.net</a>>:<br>
<br>
  * <a href="http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-8046295/webrev.03/" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~sg<wbr>ehwolf/webrevs/mbalaoal/JDK-80<wbr>46295/webrev.03/</a> <br>
  * <a href="http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-8046295/webrev.03/8046295.webrev.03.zip" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~sg<wbr>ehwolf/webrevs/mbalaoal/JDK-80<wbr>46295/webrev.03/8046295.webrev<wbr>.03.zip</a> <br>
<br>
Kind regards,<br>
Martin.-<br>
<br>
On Tue, Jul 18, 2017 at 2:12 PM, Martin Balao <<a href="mailto:mbalao@redhat.com" target="_blank">mbalao@redhat.com</a> <mailto:<a href="mailto:mbalao@redhat.com" target="_blank">mbalao@redhat.com</a>>> wrote:<br>
<br>
    Hi,<br>
<br>
    Given that 1) Trusted CA Indication TLS Extension does not appear to<br>
    be widely implemented today and 2) it will be replaced by<br>
    Certificate Authorities TLS extension in TLS 1.3, it looks more<br>
    beneficial to me supporting only the latter -and avoid paying the<br>
    cost for a larger code-base-.<br>
<br>
    Here it is my proposal for Certificate Authorities TLS extension:<br>
<br>
      *<br>
    <a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_07_18/8046295.webrev.03/" rel="noreferrer" target="_blank">http://people.redhat.com/mbala<wbr>oal/webrevs/jdk_8046295_truste<wbr>d_ca/2017_07_18/8046295.webrev<wbr>.03/</a> <br>
    <<a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_07_18/8046295.webrev.03/" rel="noreferrer" target="_blank">http://people.redhat.com/mbal<wbr>aoal/webrevs/jdk_8046295_trust<wbr>ed_ca/2017_07_18/8046295.webre<wbr>v.03/</a>> <br>
    (browse online)<br>
      *<br>
    <a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_07_18/8046295.webrev.03.zip" rel="noreferrer" target="_blank">http://people.redhat.com/mbala<wbr>oal/webrevs/jdk_8046295_truste<wbr>d_ca/2017_07_18/8046295.webrev<wbr>.03.zip</a> <br>
    <<a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_07_18/8046295.webrev.03.zip" rel="noreferrer" target="_blank">http://people.redhat.com/mbal<wbr>aoal/webrevs/jdk_8046295_trust<wbr>ed_ca/2017_07_18/8046295.webre<wbr>v.03.zip</a>> <br>
    (download)<br>
<br>
    Implementation based on TLS 1.3 - draft 20 [1]. Patch based on<br>
    JDK-10 (rev 76ff72bb6a4a).<br>
<br>
    Pending / blocked (in descending priority order):<br>
<br>
    High<br>
<br>
      * The extension applies only to TLSv1.3+<br>
       * Blocked because TLSv1.3 is still not supported in JSSE.<br>
       * Impact: the extension cannot be used in TLS 1.2 (high impact on<br>
    the client-side).<br>
       * Action: replace "useTLS12PlusSpec" with "useTLS13PlusSpec" in<br>
    the patch when available.<br>
<br>
    Medium<br>
<br>
      * Server can send the CertificateAuthorities extension to the<br>
    client in a CertificateRequest message (feature)<br>
       * Blocked by: Server is still not able to send<br>
    EncryptedExtensions message in CertificateRequest<br>
       * Impact: feature not supported on the server side. The extension<br>
    can still work in production environments. (medium).<br>
       * Action: implement EncryptedExtensions message in<br>
    CertificateRequest and then implement this feature.<br>
<br>
    Low<br>
<br>
      * Update documentation to refer the final TLS 1.3 RFC (draft 20 is<br>
    currently referred)<br>
       * Blocked by: publication of the final TLS 1.3 RFC<br>
       * Impact: documentation is not accurate. (low)<br>
       * Action: replace<br>
    "<a href="https://tools.ietf.org/html/draft-ietf-tls-tls13-20#section-4.2.4" rel="noreferrer" target="_blank">https://tools.ietf.org/html/d<wbr>raft-ietf-tls-tls13-20#section<wbr>-4.2.4</a><br>
    <<a href="https://tools.ietf.org/html/draft-ietf-tls-tls13-20#section-4.2.4" rel="noreferrer" target="_blank">https://tools.ietf.org/html/d<wbr>raft-ietf-tls-tls13-20#section<wbr>-4.2.4</a>>"<br>
    with the final link in the patch.<br>
<br>
      * Update bug id in "CertificateAuthoritiesClientT<wbr>est.java" and<br>
    "CertificateAuthoritiesServerT<wbr>est.java"<br>
       * Blocked by: there is no bug id for Certificate Authorities TLS<br>
    extension<br>
       * Impact: internal tests (very low).<br>
       * Action: replace "@bug 8046295" with the new bug id in the<br>
    patch. Open a new bug id for Certificate Authorities TLS extension.<br>
    Look forward to your comments.<br>
<br>
    Kind regards,<br>
    Martin.-<br>
<br>
    --<br>
    [1] -<br>
    <a href="https://tools.ietf.org/html/draft-ietf-tls-tls13-20#section-4.2.4" rel="noreferrer" target="_blank">https://tools.ietf.org/html/dr<wbr>aft-ietf-tls-tls13-20#section-<wbr>4.2.4</a><br>
    <<a href="https://tools.ietf.org/html/draft-ietf-tls-tls13-20#section-4.2.4" rel="noreferrer" target="_blank">https://tools.ietf.org/html/d<wbr>raft-ietf-tls-tls13-20#section<wbr>-4.2.4</a>><br>
<br>
    On Tue, Jun 20, 2017 at 9:33 PM, Xuelei Fan <<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a><br>
    <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>><wbr>> wrote:<br>
<br>
        Hi Martin,<br>
<br>
        The TLS 1.3 spec is replacing the Trusted CA Indication<br>
        (trusted_ca_keys) extension with a new Certificate Authorities<br>
        (certificate_authorities) extension.  See more details about the<br>
        specification in the TLS 1.3 draft:<br>
        <a href="https://tools.ietf.org/html/draft-ietf-tls-tls13-20#section-4.2.4" rel="noreferrer" target="_blank">https://tools.ietf.org/html/dr<wbr>aft-ietf-tls-tls13-20#section-<wbr>4.2.4</a> <<a href="https://tools.ietf.org/html/draft-ietf-tls-tls13-20#section-4.2.4" rel="noreferrer" target="_blank">https://tools.ietf.org/html/d<wbr>raft-ietf-tls-tls13-20#section<wbr>-4.2.4</a>><br>
<br>
        Both serves a similar purpose, but the trusted_ca_keys extension<br>
        will not be used in TLS 1.3 any more.  The "trusted_ca_keys"<br>
        extension will only be used for legacy protocol versions (TLS<br>
        1.2/1.1/1.0).<br>
<br>
        There are two options to me:<br>
        1. Supports the certificate_authorities, but not trusted_ca_keys<br>
        extension.<br>
        It is acceptable to me as trusted_ca_keys is for legacy use only<br>
        and the certificate_authorities extension is the future.  Plus,<br>
        the certificate_authorities extension can also be used for TLS<br>
        1.2 and previous versions.<br>
<br>
        2. Supports both the certificate_authorities and trusted_ca_keys<br>
        extensions.<br>
        As far as I know, I did not see much benefit of this option<br>
        unless the trusted_ca_keys extension is widely used in practice.<br>
<br>
        If I did not miss something, the APIs you designed can still be<br>
        used for the certificate_authorities extension, with a little<br>
        bit update.<br>
<br>
        What do you think?<br>
<br>
        Thanks & Regards,<br>
        Xuelei<br>
<br>
<br>
        On 6/15/2017 12:05 PM, Martin Balao wrote:<br>
<br>
            Hi Xuelei,<br>
<br>
            The new webrev.02 is ready:<br>
<br>
               *<br>
            <a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_15/8046295.webrev.02/" rel="noreferrer" target="_blank">http://people.redhat.com/mbala<wbr>oal/webrevs/jdk_8046295_truste<wbr>d_ca/2017_06_15/8046295.webrev<wbr>.02/</a> <br>
            <<a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_15/8046295.webrev.02/" rel="noreferrer" target="_blank">http://people.redhat.com/mbal<wbr>aoal/webrevs/jdk_8046295_trust<wbr>ed_ca/2017_06_15/8046295.webre<wbr>v.02/</a>> <br>
            (browse online)<br>
               *<br>
            <a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_15/8046295.webrev.02.zip" rel="noreferrer" target="_blank">http://people.redhat.com/mbala<wbr>oal/webrevs/jdk_8046295_truste<wbr>d_ca/2017_06_15/8046295.webrev<wbr>.02.zip</a> <br>
            <<a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_15/8046295.webrev.02.zip" rel="noreferrer" target="_blank">http://people.redhat.com/mbal<wbr>aoal/webrevs/jdk_8046295_trust<wbr>ed_ca/2017_06_15/8046295.webre<wbr>v.02.zip</a>> <br>
            (zip, download)<br>
<br>
            The following changes have been implemented since the<br>
            previous webrev.01:<br>
<br>
               * s/getUseTrustedCAIndication() methods in<br>
            SSLEngine/SSLSocket and in SSLEngineImpl/SSLSocketImpl<br>
            removed. s/getSSLParameters is now the only way to set or<br>
            get the use of the Trusted CA Indication extension. An<br>
            exception is no longer thrown if trying to disable the<br>
            extension for a server, but the change takes no effect as<br>
            the extension is mandatory for servers. X509KeyManagerImpl<br>
            modified to use SSLParameters to get information regarding<br>
            if Trusted CA Indication is enabled and should guide the<br>
            certificate choice.<br>
<br>
               * TrustedAuthorityIndicator.Iden<wbr>tifierType has been moved<br>
            from enum to String, to follow JSSE conventions. I<br>
            understand how important is to be consistent. However, I<br>
            still believe that an enum is a better fit for this value<br>
            and does not prevent from future extension. We are choosing<br>
            from a closed set (strictly defined by the RFC) and that's<br>
            what enum allows to express. From the client point of<br>
            view/API, it's very handy that the type gives you<br>
            information regarding the allowed choices for the parameter.<br>
            You don't necessarily have to look for implementation<br>
            details or documentation but you can just leverage on the<br>
            strongly typed language. It's also likely that enums are<br>
            faster for comparisons than strings, but that's not the main<br>
            point here.<br>
<br>
               * Removed X509Certificate from TrustedAuthorityIndicator<br>
            class (method and property). It was there for informational<br>
            purposes (when TrustedAuthorityIndicator was built from a<br>
            certificate by a client and the whole extension indicators<br>
            converted to String).<br>
<br>
               * "equals" and "hashCode" methods moved from<br>
            TrustedAuthorityIndicator to TrustedAuthorityIndicatorImpl<br>
            class.<br>
<br>
               * "getLength" method removed from<br>
            TrustedAuthorityIndicator class. It's possible to get the<br>
            encoded buffer and the length from there.<br>
<br>
               * "getData" method renamed to "getEncoded" in<br>
            TrustedAuthorityIndicator class.<br>
<br>
               * "trustedAuthorityEncodedData" renamed to "encodedData"<br>
            in TrustedAuthorityIndicator and<br>
            TrustedAuthorityIndicatorImpl classes<br>
<br>
               * "identifier" and "encodedData" instance variables moved<br>
            from TrustedAuthorityIndicator to<br>
            TrustedAuthorityIndicatorImpl class.<br>
<br>
               * "getEncoded" and "getIdentifier" are now abstract<br>
            methods in TrustedAuthorityIndicator, and their<br>
            implementation is in TrustedAuthorityIndicatorImpl class.<br>
<br>
               * "getIdentifier" method renamed to "getType" in<br>
            TrustedAuthorityIndicator and TrustedAuthorityIndicatorImpl<br>
            classes ("identifier" instance variable and parameter in<br>
            TrustedAuthorityIndicatorImpl class renamed to "type").<br>
<br>
               * Test cases (server and client) updated to reflect the<br>
            new interface (enabling the use of the extension through<br>
            SSLParameters)<br>
<br>
            However, some changes are still not implemented and I have<br>
            some concerns:<br>
<br>
            1) I still believe that identifier type information has to<br>
            be on TrustedAuthorityIndicator class somehow, and<br>
            implementations restricted on what they can return as part<br>
            of "getType" method. This is strictly specified by the RFC<br>
            TrustedAuthorityIndicator class represents, and I find<br>
            desirable that any implementation is enforced to be<br>
            compliant to that. If we remove all of that (including the<br>
            enum), TrustedAuthorityIndicator looks too generic and does<br>
            not reflect (in my opinion) what it really is. It'd also be<br>
            chaotic if different implementations call pre-agreed type as<br>
            "preagreed", "pre-agreed", "PRE_AGREED", etc. I prefer<br>
            stricter and more explicit interfaces.<br>
<br>
            2) I agree that type mappings can be seen as part of an<br>
            implementation, but they were in TrustedAuthorityIndicator<br>
            (as protected) because every implementation is highly likely<br>
            to need them and we can avoid the necessity for repeated<br>
            code/mappings. The same for "type" and "encodedData"<br>
            variables or even "hashCode" and "equals" methods. That's<br>
            why I was thinking more of an abstract class and not an<br>
            interface, as it happens (in example) with SNIServerName.<br>
<br>
            3) I think that "implies" method on<br>
            TrustedAuthorityIndicator should be also part of the<br>
            class/interface, because that's the whole point of a Trusted<br>
            Authority Information: to allow queries for a given<br>
            certificate. This is, in fact, the only thing a server wants<br>
            from one of these objects. My concern is that if we remove<br>
            this requirement for an implementation, the interface looks<br>
            more like a byte buffer holder.<br>
<br>
            I'd appreciate if you can re-consider these items.<br>
<br>
            Thanks,<br>
            Martin.-<br>
<br>
            On Wed, Jun 14, 2017 at 7:17 PM, Xuelei Fan<br>
            <<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a> <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>><br>
            <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a><br>
            <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>><wbr>>> wrote:<br>
<br>
                 Hi Martin,<br>
<br>
                 The big picture of the design looks pretty good to me,<br>
            except a few<br>
                 comment about the JSSE conventions.  I appreciate it<br>
            very much.  By<br>
                 the way, I need more time to look into the details of the<br>
                 specification and implementation.<br>
<br>
<br>
                 In order to keep the APIs simple and small,<br>
            SSLParameters is<br>
                 preferred as the only configuration port for common<br>
            cases.   I may<br>
                 suggest to remove the s/getUseTrustedCAIndication()<br>
            methods in<br>
                 SSLEngine/SSLSocket.<br>
<br>
                 The identify type is defined as an enum<br>
                 TrustedAuthorityIndicator.Iden<wbr>tifierType.  In the<br>
            future, if more<br>
                 type is added, we need to update the specification by<br>
            adding a new<br>
                 enum item.  Enum is preferred in JDK, but for good<br>
            extensibility, in<br>
                 general JSSE does not use enum in public APIs for<br>
            extensible<br>
                 properties.  I may suggest to use String (or<br>
            integer/byte, I prefer<br>
                 to use String) as the type.  The standard trusted authority<br>
                 indicator algorithm (identifier) can be documented in<br>
            the "Java<br>
                 Cryptography Architecture Standard Algorithm Name<br>
            Documentation"[1].<br>
<br>
                 In TrustedAuthorityIndicator class, some methods, like<br>
                 getIdentifierTypeFromCode(),<br>
            getCodeFromIdentifierType(), implies(),<br>
                 getLength(), equals() and hashCode() look more like<br>
            implementation<br>
                 logic.  I may suggest remove them from public APIs.<br>
<br>
                 I did not see the benefit to have X509Certificate in the<br>
                 TrustedAuthorityIndicator class.  The class is mainly<br>
            used for<br>
                 server side certificate selection.  X509Certificate<br>
            could be unknown<br>
                 for a indicator.  I may suggestion remove the related<br>
            methods and<br>
                 properties.<br>
<br>
                 After that, as there is no requirement to instantiate<br>
                 TrustedAuthorityIndicator class in application code,<br>
            looks like it<br>
                 may be enough to use an interface to represent a<br>
            trusted authorities:<br>
                     public interface TrustedAuthorityIndicator {<br>
                          // identifier type, standard algorithm name<br>
                          String/int/Byte getType();<br>
<br>
                          // identifier<br>
                          byte[] getEncoded();<br>
                     }<br>
<br>
                 What do you think?<br>
<br>
<br>
                 Thanks & Regards,<br>
                 Xuelei<br>
<br>
                 [1]<br>
            <a href="https://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html" rel="noreferrer" target="_blank">https://docs.oracle.com/javase<wbr>/8/docs/technotes/guides/<wbr>security/StandardNames.html</a> <br>
            <<a href="https://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html" rel="noreferrer" target="_blank">https://docs.oracle.com/javas<wbr>e/8/docs/technotes/guides/<wbr>security/StandardNames.html</a>> <br>
            <<a href="https://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html" rel="noreferrer" target="_blank">https://docs.oracle.com/javas<wbr>e/8/docs/technotes/guides/<wbr>security/StandardNames.html</a> <br>
            <<a href="https://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html" rel="noreferrer" target="_blank">https://docs.oracle.com/javas<wbr>e/8/docs/technotes/guides/<wbr>security/StandardNames.html</a>>> <br>
<br>
<br>
                 On 6/13/2017 3:41 PM, Martin Balao wrote:<br>
<br>
                     Hi Xuelei,<br>
<br>
                     The new webrev.01 is ready:<br>
<br>
                        *<br>
            <a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_13/8046295.webrev.01/" rel="noreferrer" target="_blank">http://people.redhat.com/mbala<wbr>oal/webrevs/jdk_8046295_truste<wbr>d_ca/2017_06_13/8046295.webrev<wbr>.01/</a> <br>
            <<a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_13/8046295.webrev.01/" rel="noreferrer" target="_blank">http://people.redhat.com/mbal<wbr>aoal/webrevs/jdk_8046295_trust<wbr>ed_ca/2017_06_13/8046295.webre<wbr>v.01/</a>> <br>
            <<a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_13/8046295.webrev.01/" rel="noreferrer" target="_blank">http://people.redhat.com/mbal<wbr>aoal/webrevs/jdk_8046295_trust<wbr>ed_ca/2017_06_13/8046295.webre<wbr>v.01/</a> <br>
            <<a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_13/8046295.webrev.01/" rel="noreferrer" target="_blank">http://people.redhat.com/mbal<wbr>aoal/webrevs/jdk_8046295_trust<wbr>ed_ca/2017_06_13/8046295.webre<wbr>v.01/</a>>> <br>
                     (browse online)<br>
                        *<br>
            <a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_13/8046295.webrev.01.zip" rel="noreferrer" target="_blank">http://people.redhat.com/mbala<wbr>oal/webrevs/jdk_8046295_truste<wbr>d_ca/2017_06_13/8046295.webrev<wbr>.01.zip</a> <br>
            <<a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_13/8046295.webrev.01.zip" rel="noreferrer" target="_blank">http://people.redhat.com/mbal<wbr>aoal/webrevs/jdk_8046295_trust<wbr>ed_ca/2017_06_13/8046295.webre<wbr>v.01.zip</a>> <br>
            <<a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_13/8046295.webrev.01.zip" rel="noreferrer" target="_blank">http://people.redhat.com/mbal<wbr>aoal/webrevs/jdk_8046295_trust<wbr>ed_ca/2017_06_13/8046295.webre<wbr>v.01.zip</a> <br>
            <<a href="http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_13/8046295.webrev.01.zip" rel="noreferrer" target="_blank">http://people.redhat.com/mbal<wbr>aoal/webrevs/jdk_8046295_trust<wbr>ed_ca/2017_06_13/8046295.webre<wbr>v.01.zip</a>>> <br>
                     (zip, download)<br>
<br>
                     The following changes have been implemented since<br>
            the previous<br>
                     webrev.00:<br>
<br>
                        * Pre-agreed support removed from server-side<br>
                         * Unnecessary overhead and minium benefits for<br>
            JSSE.<br>
<br>
                        * Enabling the use of Trusted CA Indication<br>
            extension for<br>
                     clients through TrustManager objects was reverted.<br>
            Trusted CA<br>
                     Indication extension can now be enabled through: 1)<br>
            SSLEngine,<br>
                     2) SSLSocket, or 3) SSLParameters (which can be<br>
            applied to both<br>
                     SSLEngine and SSLSocket objects). Trusted CA Indication<br>
                     extension is mandatory for servers.<br>
<br>
                        * SunX509KeyManagerImpl old key manager<br>
            ("SunX509" algorithm)<br>
                     is now out of scope. This key manager does not<br>
            support other TLS<br>
                     extensions as Server Name Indication (SNI), which<br>
            is far more<br>
                     relevant than Trusted CA Indication. The new<br>
            X509KeyManagerImpl<br>
                     key manager ("PKIX" algorithm) is now in scope.<br>
<br>
                        * Client requested indications are now an<br>
            ExtendedSSLSession<br>
                     attribute. ServerHandshaker gets the information<br>
            from the Client<br>
                     Hello message (now parsed by<br>
            TrustedCAIndicationExtension class<br>
                     instead of TrustedAuthorityIndicator) and sets it<br>
            in the<br>
                     ExtendedSSLSession (SSLSessionImpl object). The key<br>
            manager<br>
                     (i.e.: X509KeyManagerImpl), when choosing a server<br>
            alias, may<br>
                     now get the information from the ExtendedSSLSession<br>
            object and<br>
                     guide the certificate selection based on it.<br>
                         * In order to allow multiple key managers to<br>
            use Trusted<br>
                     Authority Indicators information and to allow<br>
            multiple Trusted<br>
                     Authority Indicators implementations,<br>
            TrustedAuthorityIndicator<br>
                     has now been split in an abstract class<br>
                     (TrustedAuthorityIndicator, located in<br>
            javax.net.ssl) and an<br>
                     implementation class<br>
            (TrustedAuthorityIndicatorImpl<wbr>, located in<br>
                     sun.security.ssl). No coupling was added between<br>
            javax.net.ssl<br>
                     and sun.security.ssl packages.<br>
<br>
                        * Documentation extended and improved.<br>
                        * Test cases (server and client) updated to<br>
            reflect the new<br>
                     interface and supported key manager.<br>
<br>
                     Look forward to your new review!<br>
<br>
                     Kind regards,<br>
                     Martin.-<br>
<br>
<br>
<br>
                     On Fri, Jun 9, 2017 at 6:15 PM, Xuelei Fan<br>
                     <<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a><br>
            <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>> <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a><br>
            <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>><wbr>><br>
                     <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a><br>
            <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>> <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a><br>
            <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>><wbr>>>><br>
                     wrote:<br>
<br>
                          I'm OK to use SSLParameters.  Thank you very<br>
            much for<br>
                     considering a<br>
                          new design.<br>
<br>
                          Xuelei<br>
<br>
                          On 6/9/2017 1:10 PM, Martin Balao wrote:<br>
<br>
                              Hi Xuelei,<br>
<br>
                              I didn't notice that some of the SSLSocket<br>
            contructors<br>
                     did not<br>
                              establish the connection, so SSLParameters<br>
            can be<br>
                     effective for<br>
                              Trusted CA Indication. This was an invalid<br>
            argument on<br>
                     my side,<br>
                              sorry.<br>
<br>
                              As for the configuration to enable the<br>
            extension, it's<br>
                     probably<br>
                              not necessary on the Server side because<br>
            -as you<br>
                     mentioned- it<br>
                              is mandatory and there is no harm in<br>
            supporting it.<br>
                     However, it<br>
                              has to be configurable on the Client side<br>
            because -as we<br>
                              previously discussed- the client may cause<br>
            a handshake<br>
                     failure<br>
                              if the server does not support the<br>
            extension. I'd<br>
                     prefer the<br>
                              Client configuring the SSLSocket through<br>
            SSLParameters<br>
                     instead<br>
                              of a system-wide property -which has even<br>
            more impact<br>
                     than the<br>
                              TrustManager approach-. Would this work<br>
            for you?<br>
<br>
                              <wbr>  > In JSSE, the benefits pre_agreed<br>
            option can get by<br>
                              customizing the key/trust manager, so I<br>
            did not see too<br>
                     much<br>
                              benefits to support this option in JDK<br>
<br>
                              I understand your point and will remove<br>
            support for<br>
                     "pre_agreed".<br>
<br>
<br>
                              On Fri, Jun 9, 2017 at 1:37 AM, Xuelei Fan<br>
                              <<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a><br>
            <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>> <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a><br>
            <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>><wbr>><br>
                     <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a><br>
            <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>> <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a><br>
            <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>><wbr>>><br>
                              <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a><br>
            <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>><br>
                     <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a><br>
            <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>><wbr>><br>
            <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a> <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>><br>
                     <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a><br>
            <mailto:<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>><wbr>>>>><br>
                              wrote:<br>
<br>
<br>
<br>
                              <wbr>     On 6/8/2017 8:36 PM, Xuelei Fan wrote:<br>
<br>
                              <wbr>         The trusted authorities can be<br>
            get from client<br>
                     trust<br>
                              manager.         Server can choose the<br>
            best matching of<br>
                     server<br>
                              certificate of the<br>
                              <wbr>         client requested trusted authorities.<br>
<br>
                              <wbr>     ><br>
                              <wbr>     I missed the point that the key<br>
            manager need to<br>
                     know the client<br>
                              <wbr>     requested trusted authorities for the<br>
            choosing.         So may<br>
                              need a new<br>
                              <wbr>     SSLSession attribute (See similar<br>
            method in<br>
                              ExtendedSSLSession).<br>
<br>
                              <wbr>     Xuelei<br>
<br>
<br>
<br>
                              Yes, an attribute on SSLSession may do the<br>
            job (both<br>
                     when Key<br>
                              Manager receives a SSLSocket and a SSLEngine).<br>
<br>
                              Kind regards,<br>
                              Martin.-<br>
<br>
<br>
<br>
<br>
<br>
</blockquote></blockquote>
</div></div></blockquote></div><br></div>