SSL certificates are not checked during the handshake when possible

[kruton]

In Java 7 the X509ExtendedTrustManager was introduced which closes the gap between the time when SSLSocket#startHandshake is called and when the certificates returned by the server are verified. Using that along with SSLParameters#setEndpointIdentificationAlgorithm allows the server certificate chain to be verified during the SSL handshake.

This is because the X509ExtendedTrustManager has newer methods which take SSLSocket or SSLEngine instances. It can then use either of these to get the SSLParameters and check if the endpoint identification algorithm is set (e.g., to "HTTPS").

A few things that need to happen:

• Tests need to be updated to extend X509ExtendedTrustManager for the RecordingTrustManager
• The given trust manager must be checked to see whether it's an X509ExtendedTrustManager
• SSLParameters#setEndpointIdentificationAlgorithm needs to be called before startHandshake (i.e., in Platform#enableTlsExtensions)
• If the trust manager is a X509ExtendedTrustManager then the subsequent verifier check in RealConnection can be skipped to avoid double-checking the certificates.

[swankjesse]

Just to be explicit: what's the benefit? Fail faster?

[kruton]

In days past the SSL stack didn't know a priori what endpoint validation to use for the given protocol running over TLS (e.g., we might be running over USB, bluetooth, etc, where hostnames don't make sense), so it's been up to the caller to validate the endpoint which okhttp does with the DNS hostname HostnameVerifier. However, there are things that might act on the SSL connection such as a HandshakeCompletedListener or the SSL stack itself might want to know when a session is actually valid before caching it (e.g., for use with session tickets). For HTTPS this is difficult for the SSL stack to know when to cache without having the hostname.

In Android we've worked around it by defining our own TrustManagerImpl that takes an extra argument for the hostname and using duck-typing to call that method. But the newer classes are an official way of closing this gap and making sure bad sessions (e.g., valid certificate chain and mismatching host) do not make it out of the startHandshake() call.

[swankjesse]

I looked into this and the best way forward is to wait until the API that does this is in all releases we support. Otherwise we risk either doing hostname verification 2x (inefficient) or not at all (very dangerous!).

#2672