CASSJAVA-80: Support configuration to disable DNS reverse-lookups for SAN validation

patch by Abe Ratnofsky; reviewed by Alexandre Dutra, Andy Tolbert, and Francisco Guerrero for CASSJAVA-80

Author: aratno

core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java

@@ -243,6 +243,12 @@ public enum DefaultDriverOption implements DriverOption {
    * <p>Value-type: boolean
    */
   SSL_HOSTNAME_VALIDATION("advanced.ssl-engine-factory.hostname-validation"),
+  /**
+   * Whether or not to do a DNS reverse-lookup of provided server addresses for SAN addresses.
+   *
+   * <p>Value-type: boolean
+   */
+  SSL_ALLOW_DNS_REVERSE_LOOKUP_SAN("advanced.ssl-engine-factory.allow-dns-reverse-lookup-san"),
   /**
    * The location of the keystore file.
    *

core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java

@@ -229,6 +229,10 @@ public String toString() {
    */
   public static final TypedDriverOption<Boolean> SSL_HOSTNAME_VALIDATION =
       new TypedDriverOption<>(DefaultDriverOption.SSL_HOSTNAME_VALIDATION, GenericType.BOOLEAN);
+
+  public static final TypedDriverOption<Boolean> SSL_ALLOW_DNS_REVERSE_LOOKUP_SAN =
+      new TypedDriverOption<>(
+          DefaultDriverOption.SSL_ALLOW_DNS_REVERSE_LOOKUP_SAN, GenericType.BOOLEAN);
   /** The location of the keystore file. */
   public static final TypedDriverOption<String> SSL_KEYSTORE_PATH =
       new TypedDriverOption<>(DefaultDriverOption.SSL_KEYSTORE_PATH, GenericType.STRING);

core/src/main/java/com/datastax/oss/driver/api/core/ssl/ProgrammaticSslEngineFactory.java

@@ -45,6 +45,7 @@ public class ProgrammaticSslEngineFactory implements SslEngineFactory {
   protected final SSLContext sslContext;
   protected final String[] cipherSuites;
   protected final boolean requireHostnameValidation;
+  protected final boolean allowDnsReverseLookupSan;
 
   /**
    * Creates an instance with the given {@link SSLContext}, default cipher suites and no host name
@@ -80,9 +81,28 @@ public ProgrammaticSslEngineFactory(
       @NonNull SSLContext sslContext,
       @Nullable String[] cipherSuites,
       boolean requireHostnameValidation) {
+    this(sslContext, cipherSuites, requireHostnameValidation, true);
+  }
+
+  /**
+   * Creates an instance with the given {@link SSLContext}, cipher suites and host name validation.
+   *
+   * @param sslContext the {@link SSLContext} to use.
+   * @param cipherSuites the cipher suites to use, or null to use the default ones.
+   * @param requireHostnameValidation whether to enable host name validation. If enabled, host name
+   *     validation will be done using HTTPS algorithm.
+   * @param allowDnsReverseLookupSan whether to allow raw server IPs to be DNS reverse-resolved to
+   *     choose the appropriate Subject Alternative Name.
+   */
+  public ProgrammaticSslEngineFactory(
+      @NonNull SSLContext sslContext,
+      @Nullable String[] cipherSuites,
+      boolean requireHostnameValidation,
+      boolean allowDnsReverseLookupSan) {
     this.sslContext = sslContext;
     this.cipherSuites = cipherSuites;
     this.requireHostnameValidation = requireHostnameValidation;
+    this.allowDnsReverseLookupSan = allowDnsReverseLookupSan;
   }
 
   @NonNull
@@ -92,7 +112,12 @@ public SSLEngine newSslEngine(@NonNull EndPoint remoteEndpoint) {
     SocketAddress remoteAddress = remoteEndpoint.resolve();
     if (remoteAddress instanceof InetSocketAddress) {
       InetSocketAddress socketAddress = (InetSocketAddress) remoteAddress;
-      engine = sslContext.createSSLEngine(socketAddress.getHostName(), socketAddress.getPort());
+      engine =
+          sslContext.createSSLEngine(
+              allowDnsReverseLookupSan
+                  ? socketAddress.getHostName()
+                  : socketAddress.getHostString(),
+              socketAddress.getPort());
     } else {
       engine = sslContext.createSSLEngine();
     }

core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java

@@ -22,6 +22,7 @@
 import com.datastax.oss.driver.api.core.context.DriverContext;
 import com.datastax.oss.driver.api.core.metadata.EndPoint;
 import com.datastax.oss.driver.api.core.ssl.SslEngineFactory;
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import java.io.InputStream;
 import java.net.InetSocketAddress;
@@ -69,6 +70,7 @@ public class DefaultSslEngineFactory implements SslEngineFactory {
   private final SSLContext sslContext;
   private final String[] cipherSuites;
   private final boolean requireHostnameValidation;
+  private final boolean allowDnsReverseLookupSan;
   private ReloadingKeyManagerFactory kmf;
 
   /** Builds a new instance from the driver configuration. */
@@ -88,6 +90,28 @@ public DefaultSslEngineFactory(DriverContext driverContext) {
     }
     this.requireHostnameValidation =
         config.getBoolean(DefaultDriverOption.SSL_HOSTNAME_VALIDATION, true);
+    this.allowDnsReverseLookupSan =
+        config.getBoolean(DefaultDriverOption.SSL_ALLOW_DNS_REVERSE_LOOKUP_SAN, true);
+  }
+
+  @VisibleForTesting
+  protected String hostname(InetSocketAddress addr) {
+    return allowDnsReverseLookupSan ? hostMaybeFromDnsReverseLookup(addr) : hostNoLookup(addr);
+  }
+
+  @VisibleForTesting
+  protected String hostMaybeFromDnsReverseLookup(InetSocketAddress addr) {
+    // See java.net.InetSocketAddress.getHostName:
+    // "This method may trigger a name service reverse lookup if the address was created with a
+    // literal IP address."
+    return addr.getHostName();
+  }
+
+  @VisibleForTesting
+  protected String hostNoLookup(InetSocketAddress addr) {
+    // See java.net.InetSocketAddress.getHostString:
+    // "This has the benefit of not attempting a reverse lookup"
+    return addr.getHostString();
   }
 
   @NonNull
@@ -97,7 +121,7 @@ public SSLEngine newSslEngine(@NonNull EndPoint remoteEndpoint) {
     SocketAddress remoteAddress = remoteEndpoint.resolve();
     if (remoteAddress instanceof InetSocketAddress) {
       InetSocketAddress socketAddress = (InetSocketAddress) remoteAddress;
-      engine = sslContext.createSSLEngine(socketAddress.getHostName(), socketAddress.getPort());
+      engine = sslContext.createSSLEngine(hostname(socketAddress), socketAddress.getPort());
     } else {
       engine = sslContext.createSSLEngine();
     }

core/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.java

@@ -38,9 +38,15 @@ public class SniSslEngineFactory implements SslEngineFactory {
 
   private final SSLContext sslContext;
   private final CopyOnWriteArrayList<String> fakePorts = new CopyOnWriteArrayList<>();
+  private final boolean allowDnsReverseLookupSan;
 
   public SniSslEngineFactory(SSLContext sslContext) {
+    this(sslContext, true);
+  }
+
+  public SniSslEngineFactory(SSLContext sslContext, boolean allowDnsReverseLookupSan) {
     this.sslContext = sslContext;
+    this.allowDnsReverseLookupSan = allowDnsReverseLookupSan;
   }
 
   @NonNull
@@ -71,8 +77,8 @@ public SSLEngine newSslEngine(@NonNull EndPoint remoteEndpoint) {
     // To avoid that, we create a unique "fake" port for every node. We still get session reuse for
     // a given node, but not across nodes. This is safe because the advisory port is only used for
     // session caching.
-    SSLEngine engine =
-        sslContext.createSSLEngine(address.getHostName(), getFakePort(sniServerName));
+    String peerHost = allowDnsReverseLookupSan ? address.getHostName() : address.getHostString();
+    SSLEngine engine = sslContext.createSSLEngine(peerHost, getFakePort(sniServerName));
     engine.setUseClientMode(true);
     SSLParameters parameters = engine.getSSLParameters();
     parameters.setServerNames(ImmutableList.of(new SNIHostName(sniServerName)));

core/src/main/resources/reference.conf

@@ -789,6 +789,12 @@ datastax-java-driver {
     # name matches the hostname of the server being connected to. If not set, defaults to true.
     // hostname-validation = true
 
+    # Whether or not to allow a DNS reverse-lookup of provided server addresses for SAN addresses,
+    # if cluster endpoints are specified as literal IPs.
+    # This is left as true for compatibility, but in most environments a DNS reverse-lookup should
+    # not be necessary to get an address that matches the server certificate SANs.
+    // allow-dns-reverse-lookup-san = true
+
     # The locations and passwords used to access truststore and keystore contents.
     # These properties are optional. If either truststore-path or keystore-path are specified,
     # the driver builds an SSLContext from these files.  If neither option is specified, the

integration-tests/src/test/java/com/datastax/oss/driver/core/ssl/DefaultSslEngineFactoryIT.java

@@ -21,10 +21,13 @@
 import com.datastax.oss.driver.api.core.CqlSession;
 import com.datastax.oss.driver.api.core.config.DefaultDriverOption;
 import com.datastax.oss.driver.api.core.config.DriverConfigLoader;
+import com.datastax.oss.driver.api.core.context.DriverContext;
 import com.datastax.oss.driver.api.testinfra.ccm.CcmBridge;
 import com.datastax.oss.driver.api.testinfra.ccm.CustomCcmRule;
 import com.datastax.oss.driver.api.testinfra.session.SessionUtils;
+import com.datastax.oss.driver.assertions.Assertions;
 import com.datastax.oss.driver.internal.core.ssl.DefaultSslEngineFactory;
+import java.net.InetSocketAddress;
 import org.junit.ClassRule;
 import org.junit.Test;
 
@@ -88,4 +91,67 @@ public void should_not_connect_if_not_using_ssl() {
       session.execute("select * from system.local");
     }
   }
+
+  public static class InstrumentedSslEngineFactory extends DefaultSslEngineFactory {
+    int countReverseLookups = 0;
+    int countNoLookups = 0;
+
+    public InstrumentedSslEngineFactory(DriverContext driverContext) {
+      super(driverContext);
+    }
+
+    @Override
+    protected String hostMaybeFromDnsReverseLookup(InetSocketAddress addr) {
+      countReverseLookups++;
+      return super.hostMaybeFromDnsReverseLookup(addr);
+    }
+
+    @Override
+    protected String hostNoLookup(InetSocketAddress addr) {
+      countNoLookups++;
+      return super.hostNoLookup(addr);
+    }
+  };
+
+  @Test
+  public void should_respect_config_for_san_resolution() {
+    DriverConfigLoader loader =
+        SessionUtils.configLoaderBuilder()
+            .withClass(
+                DefaultDriverOption.SSL_ENGINE_FACTORY_CLASS, InstrumentedSslEngineFactory.class)
+            .withBoolean(DefaultDriverOption.SSL_HOSTNAME_VALIDATION, false)
+            .withString(
+                DefaultDriverOption.SSL_TRUSTSTORE_PATH,
+                CcmBridge.DEFAULT_CLIENT_TRUSTSTORE_FILE.getAbsolutePath())
+            .withString(
+                DefaultDriverOption.SSL_TRUSTSTORE_PASSWORD,
+                CcmBridge.DEFAULT_CLIENT_TRUSTSTORE_PASSWORD)
+            .build();
+    try (CqlSession session = SessionUtils.newSession(CCM_RULE, loader)) {
+      InstrumentedSslEngineFactory ssl =
+          (InstrumentedSslEngineFactory) session.getContext().getSslEngineFactory().get();
+      Assertions.assertThat(ssl.countReverseLookups).isGreaterThan(0);
+      Assertions.assertThat(ssl.countNoLookups).isEqualTo(0);
+    }
+
+    loader =
+        SessionUtils.configLoaderBuilder()
+            .withClass(
+                DefaultDriverOption.SSL_ENGINE_FACTORY_CLASS, InstrumentedSslEngineFactory.class)
+            .withBoolean(DefaultDriverOption.SSL_HOSTNAME_VALIDATION, false)
+            .withString(
+                DefaultDriverOption.SSL_TRUSTSTORE_PATH,
+                CcmBridge.DEFAULT_CLIENT_TRUSTSTORE_FILE.getAbsolutePath())
+            .withString(
+                DefaultDriverOption.SSL_TRUSTSTORE_PASSWORD,
+                CcmBridge.DEFAULT_CLIENT_TRUSTSTORE_PASSWORD)
+            .withBoolean(DefaultDriverOption.SSL_ALLOW_DNS_REVERSE_LOOKUP_SAN, false)
+            .build();
+    try (CqlSession session = SessionUtils.newSession(CCM_RULE, loader)) {
+      InstrumentedSslEngineFactory ssl =
+          (InstrumentedSslEngineFactory) session.getContext().getSslEngineFactory().get();
+      Assertions.assertThat(ssl.countReverseLookups).isEqualTo(0);
+      Assertions.assertThat(ssl.countNoLookups).isGreaterThan(0);
+    }
+  }
 }