[CALCITE-7796] Avatica: hostname_verification=NONE has no effect with Apache HttpClient 5.6+

core/src/main/java/org/apache/calcite/avatica/remote/CommonsHttpClientPoolCache.java

@@ -21,6 +21,7 @@
 import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
 import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
 import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
+import org.apache.hc.client5.http.ssl.HostnameVerificationPolicy;
 import org.apache.hc.client5.http.ssl.HttpsSupport;
 import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
 import org.apache.hc.client5.http.ssl.TlsSocketStrategy;
@@ -34,7 +35,6 @@
 import java.nio.file.Files;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.concurrent.ConcurrentHashMap;
-import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLContext;
 
 /**
@@ -82,8 +82,7 @@ private static PoolingHttpClientConnectionManager setupPool(ConnectionConfig con
 
   private static TlsSocketStrategy createTlsSocketStrategy(ConnectionConfig config) {
     try {
-      return new DefaultClientTlsStrategy(getSSLContext(config),
-          getHostnameVerifier(config.hostnameVerification()));
+      return createTlsSocketStrategy(getSSLContext(config), config.hostnameVerification());
     } catch (Exception e) {
       LOG.error("HTTPS TlsSocketStrategy configuration failed");
       throw new RuntimeException(e);
@@ -120,15 +119,16 @@ private static void loadTrustStore(SSLContextBuilder sslContextBuilder, Connecti
   }
 
   /**
-   * Creates the {@code HostnameVerifier} given the provided {@code verification}.
+   * Creates the {@code TlsSocketStrategy} given the provided {@code verification}.
    *
+   * @param sslContext The SSL context to use for the connections.
    * @param verification The intended hostname verification action.
-   * @return A verifier for the request verification.
+   * @return A strategy performing the requested verification.
    * @throws IllegalArgumentException if the provided verification cannot be
    *                                  handled.
    */
   @SuppressWarnings("deprecation")
-  private static HostnameVerifier getHostnameVerifier(
+  private static TlsSocketStrategy createTlsSocketStrategy(SSLContext sslContext,
       org.apache.calcite.avatica.remote.
           HostnameVerificationConfigurable.HostnameVerification verification) {
     // Normally, the configuration logic would give us a default of STRICT if it was
@@ -139,9 +139,12 @@ private static HostnameVerifier getHostnameVerifier(
     }
     switch (verification) {
     case STRICT:
-      return HttpsSupport.getDefaultHostnameVerifier();
+      return new DefaultClientTlsStrategy(sslContext, HostnameVerificationPolicy.BOTH,
+          HttpsSupport.getDefaultHostnameVerifier());
     case NONE:
-      return NoopHostnameVerifier.INSTANCE;
+      // CLIENT leaves verification to the supplied verifier (which does nothing)
+      return new DefaultClientTlsStrategy(sslContext, HostnameVerificationPolicy.CLIENT,
+          NoopHostnameVerifier.INSTANCE);
     default:
       throw new IllegalArgumentException("Unhandled HostnameVerification: " + verification.name());
     }

server/src/test/java/org/apache/calcite/avatica/HttpBaseTest.java

@@ -28,6 +28,8 @@
 import org.bouncycastle.asn1.x500.style.RFC4519Style;
 import org.bouncycastle.asn1.x509.BasicConstraints;
 import org.bouncycastle.asn1.x509.Extension;
+import org.bouncycastle.asn1.x509.GeneralName;
+import org.bouncycastle.asn1.x509.GeneralNames;
 import org.bouncycastle.asn1.x509.KeyUsage;
 import org.bouncycastle.cert.X509CertificateHolder;
 import org.bouncycastle.cert.jcajce.JcaX509CertificateConverter;
@@ -86,15 +88,8 @@ public abstract class HttpBaseTest {
 
   public static void setupClass() throws SQLException {
     // Create a self-signed cert
-    if (KEYSTORE.isFile()) {
-      assertTrue("Failed to delete keystore: " + KEYSTORE, KEYSTORE.delete());
-    }
-    new CertTool().createSelfSignedCert(KEYSTORE, "avatica", KEYSTORE_PASSWORD);
-
-    if (EMPTY_PW_KEYSTORE.isFile()) {
-      assertTrue("Failed to delete keystore: " + EMPTY_PW_KEYSTORE, EMPTY_PW_KEYSTORE.delete());
-    }
-    new CertTool().createSelfSignedCert(EMPTY_PW_KEYSTORE, "avatica", KEYSTORE_EMPTY_PASSWORD);
+    createSelfSignedKeyStore(KEYSTORE, "localhost", KEYSTORE_PASSWORD);
+    createSelfSignedKeyStore(EMPTY_PW_KEYSTORE, "localhost", KEYSTORE_EMPTY_PASSWORD);
 
     // Create a LocalService around HSQLDB
     JdbcMeta jdbcMeta;
@@ -103,6 +98,17 @@ public static void setupClass() throws SQLException {
     localService = new LocalService(jdbcMeta);
   }
 
+  /**
+   * Creates a keystore holding a self-signed certificate for {@code hostname}.
+   */
+  protected static void createSelfSignedKeyStore(File keystore, String hostname,
+      String keystorePassword) {
+    if (keystore.isFile()) {
+      assertTrue("Failed to delete keystore: " + keystore, keystore.delete());
+    }
+    new CertTool().createSelfSignedCert(keystore, "avatica", keystorePassword, hostname);
+  }
+
   @AfterClass public static void stopServers() throws KrbException {
     for (HttpServer server : SERVERS_TO_STOP) {
       server.stop();
@@ -131,15 +137,15 @@ private static class CertTool {
     }
 
     private void createSelfSignedCert(File targetKeystore, String keyName,
-        String keystorePassword) {
+        String keystorePassword, String hostname) {
       if (targetKeystore.exists()) {
         throw new RuntimeException("Keystore already exists: " + targetKeystore);
       }
 
       try {
         KeyPair kp = generateKeyPair();
 
-        X509Certificate cert = generateCert(keyName, kp, true, kp.getPublic(),
+        X509Certificate cert = generateCert(hostname, kp, true, kp.getPublic(),
             kp.getPrivate());
 
         char[] password = keystorePassword.toCharArray();
@@ -161,7 +167,7 @@ private KeyPair generateKeyPair() throws NoSuchAlgorithmException {
       return gen.generateKeyPair();
     }
 
-    private X509Certificate generateCert(String keyName, KeyPair kp, boolean isCertAuthority,
+    private X509Certificate generateCert(String hostname, KeyPair kp, boolean isCertAuthority,
                                          PublicKey signerPublicKey, PrivateKey signerPrivateKey)
         throws IOException, OperatorCreationException, CertificateException,
         NoSuchAlgorithmException {
@@ -171,14 +177,16 @@ private X509Certificate generateCert(String keyName, KeyPair kp, boolean isCertA
 
       BigInteger serialNumber = BigInteger.valueOf(startDate.getTimeInMillis());
       X500Name issuer = new X500Name(
-          IETFUtils.rDNsFromString("cn=localhost", RFC4519Style.INSTANCE));
+          IETFUtils.rDNsFromString("cn=" + hostname, RFC4519Style.INSTANCE));
       JcaX509v3CertificateBuilder certGen = new JcaX509v3CertificateBuilder(issuer,
           serialNumber, startDate.getTime(), endDate.getTime(), issuer, kp.getPublic());
       JcaX509ExtensionUtils extensionUtils = new JcaX509ExtensionUtils();
       certGen.addExtension(Extension.subjectKeyIdentifier, false,
           extensionUtils.createSubjectKeyIdentifier(kp.getPublic()));
       certGen.addExtension(Extension.basicConstraints, false,
           new BasicConstraints(isCertAuthority));
+      certGen.addExtension(Extension.subjectAlternativeName, false,
+          new GeneralNames(new GeneralName(GeneralName.dNSName, hostname)));
       certGen.addExtension(Extension.authorityKeyIdentifier, false,
           extensionUtils.createAuthorityKeyIdentifier(signerPublicKey));
       if (isCertAuthority) {

server/src/test/java/org/apache/calcite/avatica/SslHostnameVerificationTest.java

@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.avatica;
+
+import org.apache.calcite.avatica.remote.Driver.Serialization;
+import org.apache.calcite.avatica.server.HttpServer;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.io.File;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.Statement;
+import javax.net.ssl.SSLException;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeFalse;
+
+/**
+ * Test case for the {@code hostname_verification} connection property of the remote driver.
+ */
+public class SslHostnameVerificationTest extends HttpBaseTest {
+
+  private static final File MISMATCHED_KEYSTORE =
+      new File(TARGET_DIR, "avatica-test-mismatched-hostname.jks");
+
+  private static String url;
+
+  public SslHostnameVerificationTest() {
+    super(url);
+  }
+
+  @BeforeClass
+  public static void setup() throws Exception {
+    // Skip TLS testing on IBM Java, see SslDriverTest
+    assumeFalse(
+        "Skip TLS testing on IBM Java due eclipse/jetty.project#2807",
+        System.getProperty("java.vendor").contains("IBM")
+    );
+
+    setupClass();
+    // The certificate is issued to a different hostname than the one we connect to
+    createSelfSignedKeyStore(MISMATCHED_KEYSTORE, "not-localhost.test", KEYSTORE_PASSWORD);
+
+    HttpServer httpServer = new HttpServer.Builder()
+        .withPort(0)
+        .withTLS(MISMATCHED_KEYSTORE, KEYSTORE_PASSWORD, MISMATCHED_KEYSTORE, KEYSTORE_PASSWORD)
+        .withHandler(localService, Serialization.PROTOBUF)
+        .build();
+    httpServer.start();
+    SERVERS_TO_STOP.add(httpServer);
+
+    url = "jdbc:avatica:remote:url=https://localhost:" + httpServer.getPort()
+        + ";serialization=PROTOBUF;truststore=" + MISMATCHED_KEYSTORE.getAbsolutePath()
+        + ";truststore_password=" + KEYSTORE_PASSWORD;
+    LOG.info("JDBC URL {}", url);
+  }
+
+  @Test
+  public void testHostnameVerificationNone() throws Exception {
+    String urlWithNone = jdbcUrl + ";hostname_verification=NONE";
+    try (Connection conn = DriverManager.getConnection(urlWithNone);
+        Statement stmt = conn.createStatement()) {
+      // Just make sure that the https connection is established
+      assertTrue(stmt.execute("SELECT * FROM EMP"));
+    }
+  }
+
+  @Test
+  public void testHostnameVerificationStrict() throws Exception {
+    String urlWithStrict = jdbcUrl + ";hostname_verification=STRICT";
+    try (Connection conn = DriverManager.getConnection(urlWithStrict);
+        Statement stmt = conn.createStatement()) {
+      stmt.execute("SELECT * FROM EMP");
+      fail("Exception should have been thrown for the url: " + urlWithStrict);
+    } catch (Exception e) {
+      assertTrue("Expected an SSLException as cause of: " + e,
+          e.getCause() instanceof SSLException);
+    }
+  }
+}