Code review request: JDK-8046295 - Support Trusted CA Indication extension

Xuelei Fan xuelei.fan at oracle.com
Fri Jun 9 03:36:19 UTC 2017


On 6/8/2017 3:09 PM, Martin Balao wrote:
> Hi Xuelei,
> 
> Thanks for your time reviewing this implementation.
> 
> You have a point in interoperability considerations. A client may decide 
> not to send this extension to avoid an error on the server: either 
> because it does not know the server implementation and assumes that it 
> won't support the extension or because it knows that server does not 
> support this extension. Given that the extension is not widely 
> implemented (as far as I know OpenSSL does not support it [1]), I 
> imagine that the first use cases are going to be in scenarios where the 
> user has control of both the client and the server. In fact, this may be 
> a good reason to choose Java on both the server and the client -and thus 
> the value of implementing both sides at the same time-.
> 
> Your design concerns were also mine. I'll try to explain why I went that 
> way, and I'm open to discuss and re-work all of them.
> 
> 1.
> 
> I understand your point and agree that TrustedManager and KeyManager are 
> better to remain immutable. The reason why I decided to go for a change 
> in X509KeyManager/X509TrustManager API (instead of a change in the 
> factory API/constructor) was to minimize the scope of the change as this 
> is X509 oriented. I agree with you that if multiple threads use the same 
> trust objects, any change will impact the others. If they require 
> isolation, they would need to instantiate different factories and get 
> different trust objects.
> 
> 2.
> 
> As it was implemented now, it'd be necessesary to instantiate a new 
> KeyManagerFactory/TrustedManagerFactory to have a different 
> configuration regarding pre-agreed certificates or the use of Trusted CA 
> Indication extension. Once the factory is instantiated, it will return 
> the same TrustManager/KeyManager object.
> 
> My first implementation was aligned to have this information more 
> tighted to the connection. But I faced a few issues with that:
> 
>   1. When you create a SSLSocket from the client side, the socket 
> automatically connects to the server. So, the use of Trusted CA 
> Indication needs to be there at the very beginning -if not, the 
> extension won't be used-. Getting a socket and setting SSLParameters is 
> not useful in this case.
> 
I did not get the point why setting SSLParametes is not useful.

When an SSLSocket in created, the SSL connection has not established. 
Creating a SSLSocket, and then setting the SSLParameters, and then 
negotiating the SSL connection (handshake).  That's the general routines 
to use SSLParameters.

        // connect
        SSLSocket sslSocket = sslSocketFactory.createSocket(...);

        // configure
        SSLParameters sslParameters = ...
        sslSocket.setParameters(sslParameters);

        // negotiate
        sslSocket.startHandshake()/sslSocket I/O.

I think if trusted ca indication is configured with SSLParameters, it 
should can be used.

>   2. Setting this information through the SSLContext would be a hard 
> interface change -init method?-, but we can explore that path.
> 
> The fact that the information that is going to be used (when the 
> extension is enabled) is inside the TrustManager is what made me decide 
> to go for the option I implemented. Both the "switch" to use it and the 
> information are together. This is related to #3.
> 
> 3.
> 
> The client sends all the certificates in the TrustManager as trusted CA 
> indication data because those are the certificates in which it trusts in 
> order to validate the connection. These certificates are not necessarily 
> the same that the certificates that the server has in its KeyManager. If 
> the client previously knows that the server has the same certificates, I 
> agree with you that it makes no sense to use this extension. That is 
> going to depend on each use-case.
> 
> A different alternative for #1 and #2 would be to set this information 
> through SSLServerSocketFactory (SSLSocketImpl) and SSLEngine 
> (SSLEngineImpl).
> 
> Do you have any preference or any other idea regarding the interface 
> design for enabling the extension?
> 
Per my understanding, the significant benefit of this extension is to 
help the server choose the right server cert if the server supports 
multiple certificates.  For example, the server has 10 RSA certs issued 
by 8 CAs, while the client only trust one CA.

I may add a pair of SSLParametersmethod, or define a system property:
      public boolean getUseTrustedIndication();
      public void setUseTrustedIndication(boolean useTrustedIndication);

or define a system property:
      jdk.tls.useTrustedIndication = true/false.

or use system property:
      jdk.tls.extensions = +/-trusted_ca_keys

As this is a mandatory TLS Extensions of NIST SP 800-52, more 
flexibility may be not desired.  I would prefer to use one system 
property only.

The trusted authorities can be get from client trust manager.  Server 
can choose the best matching of server certificate of the client 
requested trusted authorities.

In JSSE, the benefits pre_agreed option can get by customizing the 
key/trust manager, so I did not see too much benefits to support this 
option in JDK.  The values of other three options can get from the 
key/trust manager certificates.

What do you think?  Can it meet your requirements?

> BTW, there is an additional issue I've recently noticed related to the 
> aliases retrieval in the server side that I'll fix in the next Webrev. 
> To briefly summarize, getServerAliases method (invoked from 
> ServerHanshaker.java) is not enough when the KeyManager is implemented 
> by X509KeyManagerImpl.java (instead of SunX509KeyManagerImpl.java). The 
> reason is that chooseServerAlias does a lot more stuff in this 
> implementation than just getting the alias from a cache (i.e.: use of 
> server name indications).
> Please feel free to file a bug or submit the change.

Thanks & Regards,
Xuelei

> Kind regards,
> Martin.-
> 
> --
> [1] - https://github.com/openssl/openssl/issues/3029
> 
> On Thu, Jun 8, 2017 at 1:35 PM, Xuelei Fan <xuelei.fan at oracle.com 
> <mailto:xuelei.fan at oracle.com>> wrote:
> 
>     Hi Martin,
> 
>     Thanks for the contribution of the extension implementation.
> 
>     Do you know the real requirements or user cases for this extension?
> 
> 
>     There is a vague point for this extension (Section 6, RFC 6066):
> 
>         Servers that receive a client hello containing the "trusted_ca_keys"
>         extension MAY use the information contained in the extension to
>     guide
>         their selection of an appropriate certificate chain to return to the
>         client.
> 
>     For better interop, a vendor may not use this extension if no
>     appropriate certificate matching the trusted CA indication.  This
>     could make the benefits of this extension discounted a lot.
> 
>     I have some concerns about the design:
>     1. The trust manager and key manager should be immutable, otherwise
>     there are multiple threading issues.  It implies that the set
>     methods should not be used for the trust/key managers.
> 
>     2. The trusted ca indication is connection-oriented attribute, while
>     trust/key managers are context wild configuration.  So it may be not
>     the right place to configure the trusted ca indication in trust/key
>     manager.  For example, every connection may have its own pre-agreed
>     indication. While the pre-agreed configuration in key manager means
>     all connections in the context have the same pre-agreed indication.
> 
>     3. In the implementation, if the extension is enabled, the client
>     will use all trusted certificates as the trusted ca indication.  As
>     all trusted CA are indicated, the extension is actually redundant. 
>     Use it or not, no impact on the final handshaking result.  I would
>     think it twice to use this extension if there is no significant
>     benefits.
> 
>     Thanks & Regards,
>     Xuelei
> 
> 
>     On 6/7/2017 6:37 AM, Martin Balao wrote:
> 
>         Hi,
> 
>         I'd like to propose a patch for bug ID JDK-8046295 - Support
>         Trusted CA Indication extension [1] and ask for reviews. I have
>         the OCA signed since I'm a Red Hat employee and this is part of
>         my work [2].
> 
>         Webrev (jdk10 jdk10+10 based):
> 
>            *
>         http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-8046295/webrev.00/
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-8046295/webrev.00/>
>         (browse online)
>            *
>         http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-8046295/webrev.00/8046295.webrev.00.zip
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-8046295/webrev.00/8046295.webrev.00.zip>
>         (zip download)
> 
>         Trusted CA Indication TLS extension (for TLS v1.2+) has been
>         implemented on both client and server sides, according to RFC
>         6066 [3]. Implementation assumes the use of X.509 certificates.
> 
>         Client side
> 
>            * The extension can be enabled by invoking
>         'setUseTrustedCAIndication' method on the 'X509TrustManager'
>         instance used for establishing a TLS channel.
>            * When enabled, a SHA-1 hash for each certificate managed by
>         the TrustManager instance is sent to the server as a "Trusted CA
>         Indication" data element. This happens during the Client Hello
>         stage of the TLS Handshake.
>             * Note: SHA-1 key hash, Distinguished Name and pre-agreed
>         methods specified by RFC 6066 to identify a certificate were not
>         implemented on the client side.
> 
>         Server side
> 
>            * The extension is always enabled on the server side.
>            * When a client sends Trusted CA Indication data elements
>         during the Client Hello stage (TLS Handshake), the server tries
>         to choose a certificate from its 'X509KeyManager' instance based
>         on that information. If a certificate is not found, the TLS
>         channel cannot be established.
>            * A certificate chain on a 'X509KeyManager' instance can be
>         set as 'pre-agreed' trusted (see RFC 6066) invoking the
>         'setPreAgreedCertificate' method
>            * This is the procedure through which the server chooses a
>         certificate:
>             * Cipher suites iterated as usual (in preferred order)
>             * If the client has sent Trusted CA Indication data elements:
>              * All the certificate chains for the chosen cipher suite
>         algorithm are retrieved from the 'X509KeyManager' instance and
>         iterated
>               * For each certificate on a chain (starting from root):
>                * For each Trusted CA Indication data element:
>                 * If there is a match between the Trusted CA Indication
>         data element and the certificate in the server's chain, the
>         certificate chain is chosen.
>                 * If Trusted CA Indication data element is of
>         "pre-agreed" type and the certificate chain was set as
>         "pre-agreed", the certificate chain is chosen.
>            * As a consequence of the previous procedure, a client may
>         trust in an intermediate certificate and the server will be able
>         to choose a certificate chain that contains that intermediate
>         certificate.
>            * SHA-1 certificate hash, SHA-1 key hash, Distinguished Name
>         and pre-agreed methods specified by RFC 6066 are supported.
>         Test cases implemented for both client and server sides.
> 
>         Thanks in advanced,
>         Martin Balao.-
> 
>         --
>         [1] - https://bugs.openjdk.java.net/browse/JDK-8046295
>         <https://bugs.openjdk.java.net/browse/JDK-8046295>
>         [2] -
>         http://www.oracle.com/technetwork/community/oca-486395.html#r
>         <http://www.oracle.com/technetwork/community/oca-486395.html#r>
>         [3] - https://tools.ietf.org/html/rfc6066#page-12
>         <https://tools.ietf.org/html/rfc6066#page-12>
> 
> 



More information about the security-dev mailing list