Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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. */
Expand All @@ -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
Expand All @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't dawn on me at the time that SniSslEngineFactory does not access DriverContext so we can't make use of it unless programatically (outside of CloudConfigFactory), but I suppose if someone wants to use it separately, they can programmatically and in which case this adds some functionality 👍

this.sslContext = sslContext;
this.allowDnsReverseLookupSan = allowDnsReverseLookupSan;
}

@NonNull
Expand Down Expand Up @@ -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)));
Expand Down
6 changes: 6 additions & 0 deletions core/src/main/resources/reference.conf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
}
}