finagle-core: Stop Using Sun's HostnameChecker Directly

Summary: Problem

`HostnameVerifier` uses Sun's `HostnameChecker` directly for hostname
verification. This is no longer allowed in JDK 9 and will result in an
`IllegalAccessException`.

Solution

Change Finagle's SSL/TLS configurations to use `HostnameChecker` indirectly
by setting `SSLParameters#setEndpointIdentificationAlgorithm` to 'HTTPS'
when a hostname has been included as part of an `SslClientConfiguration`.

Result

Hostname verification continues to work and an impediment for moving to JDK
9 has been removed.

JIRA Issues: CSL-2956

Differential Revision: https://phabricator.twitter.biz/D144149

CHANGES

@@ -7,13 +7,27 @@ Note that ``PHAB_ID=#`` and ``RB_ID=#`` correspond to associated messages in com
 Unreleased
 ----------
 
+Breaking API Changes
+~~~~~~~~~~~~~~~~~~~~
+
+  * finagle-core: `c.t.f.ssl.client.HostnameVerifier` has been removed since it was using
+    `sun.security.util.HostnameChecker` which is no longer accessible in JDK 9.
+    ``PHAB_ID=D144149``
+
 Runtime Behavior Changes
 ~~~~~~~~~~~~~~~~~~~~~~~~
 
   * finagle-core: `c.t.f.filter.NackAdmissionFilter` no longer takes effect when
     the client's request rate is too low to accurately update the EMA value or
     drop requests. ``PHAB_ID=D143996``
 
+  * finagle-core: SSL/TLS client hostname verification is no longer performed by
+    `c.t.f.ssl.client.HostnameVerifier`. The same underlying library
+    `sun.security.util.HostnameChecker` is used to perform the hostname verification.
+    However it now occurs before the SSL/TLS handshake has been completed, and the
+    exception on failure has changes from a `c.t.f.SslHostVerificationException` to a
+    `javax.net.ssl.CertificateException`. ``PHAB_ID=D144149``
+
 18.3.0
 -------
 

finagle-core/src/main/scala/com/twitter/finagle/ssl/SslConfigurations.scala

@@ -115,6 +115,25 @@ private[ssl] object SslConfigurations {
         sslEngine.setEnabledProtocols(protocols.toArray)
     }
 
+  /**
+   * If a non-empty hostname is supplied, the engine is set to use
+   * "HTTPS"-style hostname verification.
+   *
+   * See: `javax.net.ssl.SSLParameters#setEndpointIdentificationAlgorithm` and
+   * https://docs.oracle.com/javase/7/docs/technotes/guides/security/StandardNames.html
+   * for more details.
+   */
+  def configureHostnameVerification(
+    sslEngine: SSLEngine,
+    hostname: Option[String]
+  ): Unit = hostname match {
+    case Some(_) =>
+      val sslParameters = sslEngine.getSSLParameters()
+      sslParameters.setEndpointIdentificationAlgorithm("HTTPS")
+      sslEngine.setSSLParameters(sslParameters)
+    case None => // do nothing
+  }
+
   /**
    * Guard method for failing fast inside of a factory's apply method when
    * [[KeyCredentials]] are not supported.

finagle-core/src/main/scala/com/twitter/finagle/ssl/client/SslClientEngineFactory.scala

@@ -57,6 +57,7 @@ object SslClientEngineFactory {
 
     SslConfigurations.configureProtocols(sslEngine, config.protocols)
     SslConfigurations.configureCipherSuites(sslEngine, config.cipherSuites)
+    SslConfigurations.configureHostnameVerification(sslEngine, config.hostname)
   }
 
   /**

finagle-core/src/main/scala/com/twitter/finagle/ssl/client/SslClientSessionVerifier.scala

@@ -33,15 +33,15 @@ object SslClientSessionVerifier {
    * whether the SSL/TLS connection should be used. It is an additional level
    * of verification beyond the standard certificate chain verification checking.
    *
-   * @note By default a `HostnameVerifier` will be used if this
+   * @note By default sessions will be seen as `AlwaysValid` if this
    * param is not configured.
    */
   case class Param(verifier: SslClientSessionVerifier) {
     def mk(): (Param, Stack.Param[Param]) =
       (this, Param.param)
   }
   object Param {
-    implicit val param = Stack.Param(Param(HostnameVerifier))
+    implicit val param = Stack.Param(Param(AlwaysValid))
   }
 
   /**

finagle-core/src/test/scala/com/twitter/finagle/ssl/SslConfigurationsTest.scala

@@ -1,11 +1,8 @@
 package com.twitter.finagle.ssl
 
 import javax.net.ssl.{SSLContext, SSLEngine}
-import org.junit.runner.RunWith
 import org.scalatest.FunSuite
-import org.scalatest.junit.JUnitRunner
 
-@RunWith(classOf[JUnitRunner])
 class SslConfigurationsTest extends FunSuite {
 
   private[this] def createTestEngine(): SSLEngine = {
@@ -61,6 +58,26 @@ class SslConfigurationsTest extends FunSuite {
     }
   }
 
+  test("configureHostnameVerifier turns on endpoint identification when hostname is set") {
+    val sslEngine = createTestEngine()
+    val sslParametersBefore = sslEngine.getSSLParameters
+    assert(sslParametersBefore.getEndpointIdentificationAlgorithm == null)
+
+    SslConfigurations.configureHostnameVerification(sslEngine, Some("twitter.com"))
+    val sslParametersAfter = sslEngine.getSSLParameters
+    assert(sslParametersAfter.getEndpointIdentificationAlgorithm == "HTTPS")
+  }
+
+  test("configureHostnameVerifier leaves endpoint identification off when hostname is empty") {
+    val sslEngine = createTestEngine()
+    val sslParametersBefore = sslEngine.getSSLParameters
+    assert(sslParametersBefore.getEndpointIdentificationAlgorithm == null)
+
+    SslConfigurations.configureHostnameVerification(sslEngine, None)
+    val sslParametersAfter = sslEngine.getSSLParameters
+    assert(sslParametersAfter.getEndpointIdentificationAlgorithm == null)
+  }
+
   test("checkKeyCredentialsNotSupported does nothing for Unspecified") {
     val keyCredentials = KeyCredentials.Unspecified
     SslConfigurations.checkKeyCredentialsNotSupported("TestFactory", keyCredentials)