diff --git a/core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java b/core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java index dd60a2487fb..babeffffdcf 100644 --- a/core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java +++ b/core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java @@ -1184,9 +1184,9 @@ public enum DefaultDriverOption implements DriverOption { * system_views.clients} on Cassandra 4.1+) so operators can inspect driver settings while * investigating incidents. It describes the effective configuration of the driver's default * execution profile (connection/socket settings, timeouts, retry/reconnection/ - * speculative-execution/load-balancing policies, connection pooling, query defaults, and TLS). - * Only the control connection sends it, since it describes the whole session. When {@code false}, - * {@code DRIVER_CONFIG} is not sent. + * speculative-execution/load-balancing policies, connection pooling, and query defaults), plus + * the effective TLS state of the control connection carrying it. Only the control connection + * sends it. When {@code false}, {@code DRIVER_CONFIG} is not sent. * *

This option governs {@code DRIVER_CONFIG} only. The {@code SESSION_ID} startup option, which * lets the server group all of a session's connections, is an innate driver behavior: it is sent diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java b/core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java index dd7630a6530..d7e9bd06c1d 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java @@ -35,6 +35,7 @@ import com.datastax.oss.driver.api.core.metadata.EndPoint; import com.datastax.oss.driver.api.core.type.codec.TypeCodecs; import com.datastax.oss.driver.internal.core.DefaultProtocolFeature; +import com.datastax.oss.driver.internal.core.context.DriverConfigReporter.TlsInfo; import com.datastax.oss.driver.internal.core.context.InternalDriverContext; import com.datastax.oss.driver.internal.core.protocol.BytesToSegmentDecoder; import com.datastax.oss.driver.internal.core.protocol.FrameDecoder; @@ -62,8 +63,10 @@ import com.datastax.oss.protocol.internal.response.Supported; import com.datastax.oss.protocol.internal.response.result.Rows; import com.datastax.oss.protocol.internal.response.result.SetKeyspace; +import io.netty.channel.Channel; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPipeline; +import io.netty.handler.ssl.SslHandler; import java.nio.ByteBuffer; import java.util.HashMap; import java.util.List; @@ -192,12 +195,14 @@ Message getRequest() { case STARTUP: Map startupOptions = new HashMap<>(context.getStartupOptions()); featureStore.populateStartupOptions(startupOptions); - // The DRIVER_CONFIG blob describes the whole session, so only the control connection - // carries it (options.reportConfig); the other connections are correlated to it by the - // SESSION_ID that every connection already carries from context.getStartupOptions(). - // No-op when driver config reporting is disabled. + // Most of DRIVER_CONFIG describes the whole session; its TLS group describes the control + // connection carrying it. Other connections are correlated to it by the SESSION_ID that + // every connection already carries from context.getStartupOptions(). No-op when driver + // config reporting is disabled. if (options.reportConfig) { - context.getDriverConfigReporter().populateControlConnectionOptions(startupOptions); + context + .getDriverConfigReporter() + .populateControlConnectionOptions(startupOptions, currentTlsInfo()); } return request = new Startup(startupOptions); case GET_CLUSTER_NAME: @@ -213,6 +218,21 @@ Message getRequest() { } } + private TlsInfo currentTlsInfo() { + try { + return tlsInfo(channel); + } catch (RuntimeException e) { + // Configuration reporting is best-effort and must never prevent a connection. TLS presence + // is still assumed because the failure came while inspecting its handler, but the schema + // permits hostname-verification to be omitted when unknown. + LOG.warn( + "[{}] Could not inspect hostname verification on the active SSL engine; omitting it", + logPrefix, + e); + return TlsInfo.enabledWithUnknownHostnameVerification(); + } + } + @Override void send() { stepNumber++; @@ -416,6 +436,21 @@ public String toString() { } } + static TlsInfo tlsInfo(Channel channel) { + // SslHandlerFactory always returns SslHandler, and this reads the pipeline after + // NettyOptions.afterChannelInitialized() has had a chance to add, replace, remove, or + // reconfigure it. A hook that implements encryption with a handler unrelated to SslHandler + // cannot be identified generically and is therefore reported as TLS-disabled. + SslHandler sslHandler = channel.pipeline().get(SslHandler.class); + if (sslHandler == null) { + return TlsInfo.disabled(); + } + String endpointIdentificationAlgorithm = + sslHandler.engine().getSSLParameters().getEndpointIdentificationAlgorithm(); + return TlsInfo.enabled( + endpointIdentificationAlgorithm != null && !endpointIdentificationAlgorithm.isEmpty()); + } + /** * Conditionally rebuilds pipeline. * diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java b/core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java index f48d686573c..d90a2e7fea0 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java @@ -26,8 +26,6 @@ import com.datastax.oss.driver.api.core.loadbalancing.LoadBalancingPolicy; import com.datastax.oss.driver.api.core.retry.RetryPolicy; import com.datastax.oss.driver.api.core.specex.SpeculativeExecutionPolicy; -import com.datastax.oss.driver.api.core.ssl.ProgrammaticSslEngineFactory; -import com.datastax.oss.driver.api.core.ssl.SslEngineFactory; import com.datastax.oss.driver.api.core.time.TimestampGenerator; import com.datastax.oss.driver.internal.core.channel.ChannelFactory; import com.datastax.oss.driver.internal.core.connection.ConstantReconnectionPolicy; @@ -39,10 +37,6 @@ import com.datastax.oss.driver.internal.core.retry.DefaultRetryPolicy; import com.datastax.oss.driver.internal.core.specex.ConstantSpeculativeExecutionPolicy; import com.datastax.oss.driver.internal.core.specex.NoSpeculativeExecutionPolicy; -import com.datastax.oss.driver.internal.core.ssl.DefaultSslEngineFactory; -import com.datastax.oss.driver.internal.core.ssl.JdkSslHandlerFactory; -import com.datastax.oss.driver.internal.core.ssl.SniSslEngineFactory; -import com.datastax.oss.driver.internal.core.ssl.SslHandlerFactory; import com.datastax.oss.driver.internal.core.time.AtomicTimestampGenerator; import com.datastax.oss.driver.internal.core.time.ServerSideTimestampGenerator; import com.datastax.oss.driver.internal.core.time.ThreadLocalTimestampGenerator; @@ -50,6 +44,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; +import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; import java.nio.charset.StandardCharsets; import java.time.Duration; @@ -75,8 +70,8 @@ * {@code connection.reconnection.policy}, {@code query.speculative-execution.policy}, {@code * query.load-balancing.policy.adaptive-ordering}, the first term of {@code * fallback-to-non-preferred-nodes}, both {@code node-preference} groups, and {@code - * connection.tls.hostname-verification} off the active handler factory rather than the configured - * engine factory. + * connection.tls.hostname-verification} from an immutable snapshot of the active control-channel + * engine rather than the configured engine factory. * *

Anything added here owes the same question: find what consumes the option, and if it stores * the value rather than re-reading it, expose an accessor and read that. Every field that got this @@ -104,13 +99,10 @@ * basic.request.serial-consistency} outside the schema's two serial levels and the like are omitted * rather than emitted as a value the schema rejects. Two optional booleans are omitted for a third * reason — the answer is genuinely unknown, which is the only thing the schema lets their absence - * mean: {@code connection.tls.hostname-verification} when the SSL handler or engine factory in - * force is not one this class recognizes, and {@code query.defaults.client-timestamps} when the - * timestamp generator is not (see {@link #hostnameValidation} and {@link #clientTimestamps}). - * Guessing a boolean there would describe a security control, or a write-timestamp source, that may - * well be the opposite — which is also why neither is asked of the SPI itself: an accessor on - * {@link SslEngineFactory} or {@link TimestampGenerator} would have needed a default, and a default - * answer is exactly the guess being avoided. + * mean: {@code connection.tls.hostname-verification} when the active SSL engine could not expose + * it, and {@code query.defaults.client-timestamps} when the timestamp generator is not one this + * class recognizes (see {@link #tls} and {@link #clientTimestamps}). Guessing a boolean there would + * describe a security control, or a write-timestamp source, that may well be the opposite. * *

A new field owes three checks, each of which this class has already got wrong once and * each of which is cheap to run before review does it for you: @@ -189,8 +181,8 @@ * cross-driver schema doesn't define; this is a known gap, not an oversight. * *

Thread safety: this class is safe to use as shipped, and holds no mutable state. Note - * that {@code buildJson()} runs on every control-connection (re)initialization, and may be called - * concurrently with a reconnect racing a fresh session start. + * that {@code buildJson(DriverConfigReporter.TlsInfo)} runs on every control-connection + * (re)initialization, and may be called concurrently with a reconnect racing a fresh session start. */ @ThreadSafe public class DefaultDriverConfigReporter implements DriverConfigReporter { @@ -231,7 +223,8 @@ public DefaultDriverConfigReporter(InternalDriverContext context) { } @Override - public void populateControlConnectionOptions(Map startupOptions) { + public void populateControlConnectionOptions( + @NonNull Map startupOptions, @NonNull TlsInfo tlsInfo) { // Configuration reporting is a best-effort diagnostic aid: it runs on the connection // initialization path, so any failure here (a bad config read, a misbehaving policy while // introspecting, a serialization error) must be swallowed rather than allowed to break the @@ -247,7 +240,7 @@ public void populateControlConnectionOptions(Map startupOptions) if (!isEnabled()) { return; } - String json = buildJson(); + String json = buildJson(tlsInfo); if (json == null) { return; } @@ -288,22 +281,20 @@ private boolean isEnabled() { * class's to enforce: a future change to session bootstrap that dropped one of those from the * eager list would quietly reintroduce that. * - *

The configured SSL engine factory is deliberately not among them: {@link #tls()} - * reads the engine factory held by the {@code JdkSslHandlerFactory} in force rather than the one - * behind {@code getSslEngineFactory()}. Those can differ — a context that overrides {@code - * buildSslHandlerFactory()} may wrap an engine factory of its own — and going through the context - * would both describe an engine nothing on the connection path uses and risk being the first - * caller to resolve it, which for the built-in factory means reading keystore/truststore files on - * a Netty event-loop thread (and failing the whole report if that throws). + *

The configured SSL engine factory is deliberately not among them: {@link + * #tls(DriverConfigReporter.TlsInfo)} reads the immutable snapshot captured from the active + * control-channel handler. Resolving the configured factory here could describe an engine the + * connection does not use and, for the built-in factory, read keystore/truststore files on a + * Netty event-loop thread. * * @return the report, or {@code null} if it could not be serialized — in which case {@code * DRIVER_CONFIG} is skipped rather than the connection failed. */ @Nullable - String buildJson() { + String buildJson(TlsInfo tlsInfo) { ObjectNode root = OBJECT_MAPPER.createObjectNode(); root.put("version", SCHEMA_VERSION); - populateConfig(root, context.getConfig().getDefaultProfile()); + populateConfig(root, context.getConfig().getDefaultProfile(), tlsInfo); try { return OBJECT_MAPPER.writeValueAsString(root); } catch (JsonProcessingException e) { @@ -318,14 +309,14 @@ String buildJson() { * plus the context's policies. Each group follows the cross-driver schema; a key the Java driver * has no equivalent for is omitted rather than reported as {@code null}. */ - private void populateConfig(ObjectNode root, DriverExecutionProfile config) { + private void populateConfig(ObjectNode root, DriverExecutionProfile config, TlsInfo tlsInfo) { // Resolved once and shared: the load balancing policy decides both its own group and the // node-location preferences reported under two different parents, and resolving it twice would // mean a second SPI lookup on the Netty event-loop thread that is building STARTUP. LoadBalancingPolicy loadBalancingPolicy = context.getLoadBalancingPolicy(DriverExecutionProfile.DEFAULT_NAME); NodeLocation nodeLocation = nodeLocation(config, loadBalancingPolicy); - root.set("connection", connection(config, nodeLocation)); + root.set("connection", connection(config, nodeLocation, tlsInfo)); root.set("control-plane", controlPlane(config)); root.set("query", query(config, loadBalancingPolicy, nodeLocation)); } @@ -335,7 +326,7 @@ private void populateConfig(ObjectNode root, DriverExecutionProfile config) { * top of it, how it is re-established, and which part of the cluster gets one at all. */ private ObjectNode connection( - DriverExecutionProfile config, @Nullable NodeLocation nodeLocation) { + DriverExecutionProfile config, @Nullable NodeLocation nodeLocation, TlsInfo tlsInfo) { ObjectNode n = connectionTimeouts(config); n.set("socket", socket(config)); ObjectNode reconnection = OBJECT_MAPPER.createObjectNode(); @@ -343,7 +334,7 @@ private ObjectNode connection( n.set("reconnection", reconnection); // Optional, and absent rather than false when off: presence of the group is what says TLS is // enabled, since the schema dropped the boolean that used to carry it. - ObjectNode tls = tls(); + ObjectNode tls = tls(tlsInfo); if (tls != null) { n.set("tls", tls); } @@ -1059,9 +1050,9 @@ private ObjectNode queryDefaults(DriverExecutionProfile config) { * which both of them extend, is package-private, so nothing outside its own package can inherit * its behavior without going through one of these two. * - *

{@code instanceof}, not the exact-class checks the policy branches use, for the same reason - * as in {@link #hostnameValidation}: this reads a property the generator has rather than deciding - * which built-in is in force, and a subclass inherits the {@code next()} that supplies it. + *

{@code instanceof}, not the exact-class checks the policy branches use: this reads a + * property the generator has rather than deciding which built-in is in force, and a subclass + * inherits the {@code next()} that supplies it. */ private static Optional clientTimestamps(TimestampGenerator generator) { if (generator instanceof AtomicTimestampGenerator @@ -1078,73 +1069,18 @@ private static Optional clientTimestamps(TimestampGenerator generator) * so presence of the group is what reports that it is on. */ @Nullable - private ObjectNode tls() { - // TLS is on exactly when the channel pipeline gets an SSL handler, which ChannelFactory decides - // from the low-level SslHandlerFactory. Deliberately not getSslEngineFactory(): that is only - // the public JDK-based path that DefaultDriverContext.buildSslHandlerFactory() wraps, and an - // override of that method (the documented expert extension point, e.g. Netty's native OpenSSL) - // supplies a handler factory with no engine factory at all — a session that is encrypted all - // the same. - Optional handlerFactory = context.getSslHandlerFactory(); - if (!handlerFactory.isPresent()) { + private ObjectNode tls(TlsInfo tlsInfo) { + // The snapshot is captured from the active pipeline immediately before STARTUP, after the + // per-channel customization hook has run. Deliberately not getSslEngineFactory(): that is + // configuration intent and may describe an engine this connection does not use. + if (!tlsInfo.isEnabled()) { return null; } ObjectNode n = OBJECT_MAPPER.createObjectNode(); - // Host name validation, on the other hand, is a property of the JDK SSLEngine that the engine - // factory configures, so it can only be read on the JDK path — when the handler factory in - // force is the JdkSslHandlerFactory that buildSslHandlerFactory() wraps an engine factory in — - // and read off that handler rather than through the context (see #buildJson for why the two can - // disagree, and why resolving the context's is worse). Anything else (a native-OpenSSL handler, - // a bespoke one) leaves it unknown, and the schema's field is optional precisely so that - // unknown can be said by omission: reporting false would claim a session is not checking host - // names when it may well be. Exact-class check, like the policy branches above: - // JdkSslHandlerFactory is not final, and a subclass need not use the engine it was given. - // - // Note this is the factory's own state, not the SSL_HOSTNAME_VALIDATION config option: that - // option only governs the built-in DefaultSslEngineFactory. A factory supplied via - // SessionBuilder.withSslContext(...) (ProgrammaticSslEngineFactory) validates only if - // explicitly asked to (default off) regardless of that option, so reading the option here would - // falsely report validation as on when it isn't. - SslHandlerFactory factory = handlerFactory.get(); - if (factory.getClass() == JdkSslHandlerFactory.class) { - SslEngineFactory engineFactory = ((JdkSslHandlerFactory) factory).getSslEngineFactory(); - hostnameValidation(engineFactory).ifPresent(v -> n.put("hostname-verification", v)); - } + tlsInfo.getHostnameVerification().ifPresent(v -> n.put("hostname-verification", v)); return n; } - /** - * Whether the engine factory in force validates host names, or {@link Optional#empty()} when it - * is not one this class recognizes. - * - *

Read by naming the driver's own factories rather than through an accessor on {@link - * SslEngineFactory}, deliberately: the interface obliges nobody to answer, so a default answer - * there would have described a security control on behalf of every implementation that never - * considered the question — including the ones that misdescribe it. Unknown is instead said by - * omission, which is what the schema's optional field is for. - * - *

{@code instanceof}, not the exact-class checks the policy branches use, and for the same - * reason as {@link #nodeLocation}: those decide which built-in is in force and must not - * be fooled by a subclass, whereas this one reads a value the factory already holds, and a - * subclass inherits it along with the {@code newSslEngine} that acts on it. A subclass that - * overrides {@code newSslEngine} to configure the engine differently — the only way to break that - * — reports its parent's answer; extending one of these factories is documented as a way to reuse - * it, not to invert it. - */ - private static Optional hostnameValidation(@Nullable SslEngineFactory engineFactory) { - if (engineFactory instanceof DefaultSslEngineFactory) { - return Optional.of(((DefaultSslEngineFactory) engineFactory).isHostnameValidationRequired()); - } else if (engineFactory instanceof ProgrammaticSslEngineFactory) { - return Optional.of( - ((ProgrammaticSslEngineFactory) engineFactory).isHostnameValidationRequired()); - } else if (engineFactory instanceof SniSslEngineFactory) { - // No accessor to read: SniSslEngineFactory sets the "HTTPS" endpoint identification algorithm - // on every engine it builds, unconditionally. - return Optional.of(true); - } - return Optional.empty(); - } - /** * A duration in milliseconds, floored at 1 for any strictly positive duration, and 0 for a zero * or negative one. diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/context/DriverConfigReporter.java b/core/src/main/java/com/datastax/oss/driver/internal/core/context/DriverConfigReporter.java index bbabe2c8b3f..c4783e7c916 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/context/DriverConfigReporter.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/context/DriverConfigReporter.java @@ -17,22 +17,63 @@ */ package com.datastax.oss.driver.internal.core.context; +import edu.umd.cs.findbugs.annotations.NonNull; import java.util.Map; +import java.util.Optional; /** * Adds the {@code DRIVER_CONFIG} entry to the control connection's CQL {@code STARTUP} options, so * ScyllaDB can store it in {@code system.clients.client_options} and operators can inspect the * driver's effective settings while investigating incidents. * - *

The blob describes the whole session, so only the control connection carries it — pooled - * connections are correlated back to it through the {@link StartupOptionsBuilder#SESSION_ID_KEY - * SESSION_ID} startup option, which the driver sends on every connection unconditionally and - * independently of this reporter. + *

Most of the blob describes the whole session; its TLS group describes the control connection + * carrying it. Pooled connections are correlated back to that control connection through the {@link + * StartupOptionsBuilder#SESSION_ID_KEY SESSION_ID} startup option, which the driver sends on every + * connection unconditionally and independently of this reporter. * *

Governed by {@code advanced.driver-config-reporting.enabled} (enabled by default). */ public interface DriverConfigReporter { + /** Immutable snapshot of the effective TLS state of the reporting control connection. */ + final class TlsInfo { + private static final TlsInfo DISABLED = new TlsInfo(false, Optional.empty()); + private static final TlsInfo ENABLED_UNKNOWN = new TlsInfo(true, Optional.empty()); + + private final boolean enabled; + private final Optional hostnameVerification; + + private TlsInfo(boolean enabled, Optional hostnameVerification) { + this.enabled = enabled; + this.hostnameVerification = hostnameVerification; + } + + /** Returns a snapshot for a connection without a Netty {@code SslHandler}. */ + public static TlsInfo disabled() { + return DISABLED; + } + + /** Returns a snapshot for a TLS connection whose hostname-verification state is known. */ + public static TlsInfo enabled(boolean hostnameVerification) { + return new TlsInfo(true, Optional.of(hostnameVerification)); + } + + /** Returns a snapshot for a TLS connection whose hostname-verification state is unknown. */ + public static TlsInfo enabledWithUnknownHostnameVerification() { + return ENABLED_UNKNOWN; + } + + public boolean isEnabled() { + return enabled; + } + + /** Empty when TLS is disabled or its active engine could not expose this state. */ + @NonNull + public Optional getHostnameVerification() { + return hostnameVerification; + } + } + /** * Adds the {@code DRIVER_CONFIG} blob to the given startup options, unless configuration * reporting is disabled. @@ -43,8 +84,13 @@ public interface DriverConfigReporter { * failure to build the report must be swallowed (and logged) rather than propagated, otherwise it * would prevent the session from establishing or reconnecting. * - *

The report describes the driver's own configuration only, so nothing here depends on which - * backend answered: it can be built before the connection learns anything about its peer. + *

The report describes the driver's own configuration and the supplied effective TLS state of + * the control connection. The caller must capture that state after channel customization and + * immediately before this method is invoked. + * + * @param startupOptions startup options to add the report to + * @param tlsInfo immutable snapshot of the control connection's effective TLS state */ - void populateControlConnectionOptions(Map startupOptions); + void populateControlConnectionOptions( + @NonNull Map startupOptions, @NonNull TlsInfo tlsInfo); } diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/context/NoopDriverConfigReporter.java b/core/src/main/java/com/datastax/oss/driver/internal/core/context/NoopDriverConfigReporter.java index 213c3657585..eed8b28b00a 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/context/NoopDriverConfigReporter.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/context/NoopDriverConfigReporter.java @@ -17,6 +17,7 @@ */ package com.datastax.oss.driver.internal.core.context; +import edu.umd.cs.findbugs.annotations.NonNull; import java.util.Map; import net.jcip.annotations.ThreadSafe; @@ -41,7 +42,8 @@ public class NoopDriverConfigReporter implements DriverConfigReporter { @Override - public void populateControlConnectionOptions(Map startupOptions) { + public void populateControlConnectionOptions( + @NonNull Map startupOptions, @NonNull TlsInfo tlsInfo) { // nothing to do } } diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryPipelineTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryPipelineTest.java new file mode 100644 index 00000000000..ba2ef2bb52b --- /dev/null +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryPipelineTest.java @@ -0,0 +1,120 @@ +/* + * 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 com.datastax.oss.driver.internal.core.channel; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.datastax.oss.driver.api.core.DefaultProtocolVersion; +import com.datastax.oss.driver.api.core.config.DefaultDriverOption; +import com.datastax.oss.driver.api.core.config.DriverConfig; +import com.datastax.oss.driver.api.core.config.DriverExecutionProfile; +import com.datastax.oss.driver.internal.core.context.InternalDriverContext; +import com.datastax.oss.driver.internal.core.context.NettyOptions; +import com.datastax.oss.driver.internal.core.metrics.MetricsFactory; +import com.datastax.oss.driver.internal.core.metrics.NoopNodeMetricUpdater; +import com.datastax.oss.driver.internal.core.metrics.NoopSessionMetricUpdater; +import com.datastax.oss.driver.internal.core.ssl.SslHandlerFactory; +import com.datastax.oss.protocol.internal.FrameCodec; +import io.netty.buffer.ByteBuf; +import io.netty.channel.Channel; +import io.netty.channel.ChannelInitializer; +import io.netty.channel.embedded.EmbeddedChannel; +import io.netty.handler.ssl.SslHandler; +import java.time.Duration; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicBoolean; +import org.junit.Test; + +public class ChannelFactoryPipelineTest { + + @Test + @SuppressWarnings("unchecked") + public void should_install_ssl_before_protocol_init_and_channel_customization() { + InternalDriverContext context = mock(InternalDriverContext.class); + DriverConfig config = mock(DriverConfig.class); + DriverExecutionProfile profile = mock(DriverExecutionProfile.class); + NettyOptions nettyOptions = mock(NettyOptions.class); + MetricsFactory metricsFactory = mock(MetricsFactory.class); + SslHandlerFactory sslHandlerFactory = mock(SslHandlerFactory.class); + SslHandler sslHandler = mock(SslHandler.class); + FrameCodec frameCodec = mock(FrameCodec.class); + + when(context.getSessionName()).thenReturn("test"); + when(context.getConfig()).thenReturn(config); + when(config.getDefaultProfile()).thenReturn(profile); + when(profile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(profile.getDuration(DefaultDriverOption.CONNECTION_SET_KEYSPACE_TIMEOUT)) + .thenReturn(Duration.ofSeconds(1)); + when(profile.getDuration(DefaultDriverOption.CONNECTION_INIT_QUERY_TIMEOUT)) + .thenReturn(Duration.ofSeconds(1)); + when(profile.getDuration(DefaultDriverOption.HEARTBEAT_INTERVAL)) + .thenReturn(Duration.ofSeconds(30)); + when(profile.getBytes(DefaultDriverOption.PROTOCOL_MAX_FRAME_LENGTH)).thenReturn(1024L); + when(profile.getInt(DefaultDriverOption.CONNECTION_MAX_REQUESTS)).thenReturn(128); + when(profile.getInt(DefaultDriverOption.CONNECTION_MAX_ORPHAN_REQUESTS)).thenReturn(32); + when(context.getSslHandlerFactory()).thenReturn(Optional.of(sslHandlerFactory)); + when(sslHandlerFactory.newSslHandler(any(), eq(ChannelFactoryTestBase.SERVER_ADDRESS))) + .thenReturn(sslHandler); + when(context.getMetricsFactory()).thenReturn(metricsFactory); + when(metricsFactory.getSessionUpdater()).thenReturn(NoopSessionMetricUpdater.INSTANCE); + when(context.getFrameCodec()).thenReturn(frameCodec); + when(context.getNettyOptions()).thenReturn(nettyOptions); + + AtomicBoolean observedCompletePipeline = new AtomicBoolean(); + doAnswer( + invocation -> { + Channel channel = invocation.getArgument(0); + List names = channel.pipeline().names(); + assertThat(channel.pipeline().get(ChannelFactory.SSL_HANDLER_NAME)) + .isSameAs(sslHandler); + assertThat(names.indexOf(ChannelFactory.SSL_HANDLER_NAME)) + .isLessThan(names.indexOf(ChannelFactory.INIT_HANDLER_NAME)); + observedCompletePipeline.set(true); + return null; + }) + .when(nettyOptions) + .afterChannelInitialized(any()); + + ChannelFactory factory = new ChannelFactory(context); + CompletableFuture resultFuture = new CompletableFuture<>(); + ChannelInitializer initializer = + factory.initializer( + ChannelFactoryTestBase.SERVER_ADDRESS, + DefaultProtocolVersion.V4, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE, + resultFuture); + + EmbeddedChannel channel = new EmbeddedChannel(initializer); + try { + assertThat(observedCompletePipeline).isTrue(); + assertThat(resultFuture).isNotCompletedExceptionally(); + verify(nettyOptions).afterChannelInitialized(channel); + } finally { + channel.finishAndReleaseAll(); + } + } +} diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java index ed6668a6c83..0bf4b762aa9 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java @@ -142,7 +142,7 @@ public void setup() throws InterruptedException { when(context.getWriteCoalescer()).thenReturn(new PassThroughWriteCoalescer(null)); when(context.getCompressor()).thenReturn(compressor); // The init handler consults the config reporter for the control connection; default to a no-op. - when(context.getDriverConfigReporter()).thenReturn(startupOptions -> {}); + when(context.getDriverConfigReporter()).thenReturn((startupOptions, tlsInfo) -> {}); // Start local server ServerBootstrap serverBootstrap = diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandlerTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandlerTest.java index 682caac198d..30ac4d2cbc5 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandlerTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandlerTest.java @@ -19,6 +19,7 @@ import static com.datastax.oss.driver.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -43,6 +44,7 @@ import com.datastax.oss.driver.internal.core.TestResponses; import com.datastax.oss.driver.internal.core.context.DefaultDriverConfigReporter; import com.datastax.oss.driver.internal.core.context.DriverConfigReporter; +import com.datastax.oss.driver.internal.core.context.DriverConfigReporter.TlsInfo; import com.datastax.oss.driver.internal.core.context.InternalDriverContext; import com.datastax.oss.driver.internal.core.context.StartupOptionsBuilder; import com.datastax.oss.driver.internal.core.metadata.TestNodeFactory; @@ -64,6 +66,8 @@ import com.datastax.oss.protocol.internal.response.result.SetKeyspace; import com.datastax.oss.protocol.internal.util.Bytes; import io.netty.channel.ChannelFuture; +import io.netty.channel.embedded.EmbeddedChannel; +import io.netty.handler.ssl.SslHandler; import java.io.IOException; import java.net.InetSocketAddress; import java.time.Duration; @@ -72,8 +76,12 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.TimeUnit; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLParameters; import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.slf4j.LoggerFactory; @@ -107,7 +115,8 @@ public void setup() { .thenReturn(Duration.ofSeconds(30)); when(internalDriverContext.getProtocolVersionRegistry()).thenReturn(protocolVersionRegistry); // The init handler consults the config reporter for the control connection; default to a no-op. - when(internalDriverContext.getDriverConfigReporter()).thenReturn(startupOptions -> {}); + when(internalDriverContext.getDriverConfigReporter()) + .thenReturn((startupOptions, tlsInfo) -> {}); channel .pipeline() @@ -160,17 +169,24 @@ public void should_initialize() { } // Mirrors the real reporter, which only ever sees the control connection. - private void stubConfigReporter() { - when(internalDriverContext.getDriverConfigReporter()) - .thenReturn( - startupOptions -> - startupOptions.put( - DefaultDriverConfigReporter.DRIVER_CONFIG_KEY, "{\"version\":1}")); + private DriverConfigReporter stubConfigReporter() { + DriverConfigReporter reporter = mock(DriverConfigReporter.class); + doAnswer( + invocation -> { + invocation + .>getArgument(0) + .put(DefaultDriverConfigReporter.DRIVER_CONFIG_KEY, "{\"version\":1}"); + return null; + }) + .when(reporter) + .populateControlConnectionOptions(any(), any()); + when(internalDriverContext.getDriverConfigReporter()).thenReturn(reporter); + return reporter; } @Test public void should_report_driver_config_on_control_connection() { - stubConfigReporter(); + DriverConfigReporter reporter = stubConfigReporter(); channel .pipeline() .addLast( @@ -190,6 +206,41 @@ public void should_report_driver_config_on_control_connection() { assertThat(requestFrame.message).isInstanceOf(Startup.class); Startup startup = (Startup) requestFrame.message; assertThat(startup.options).containsKey(DefaultDriverConfigReporter.DRIVER_CONFIG_KEY); + + ArgumentCaptor tlsInfo = ArgumentCaptor.forClass(TlsInfo.class); + verify(reporter).populateControlConnectionOptions(any(), tlsInfo.capture()); + assertThat(tlsInfo.getValue().isEnabled()).isFalse(); + } + + @Test + public void should_read_tls_from_the_replacement_ssl_engine() throws Exception { + EmbeddedChannel sslChannel = new EmbeddedChannel(); + try { + sslChannel + .pipeline() + .addLast(ChannelFactory.SSL_HANDLER_NAME, sslHandler(/* hostnameVerification= */ true)); + sslChannel + .pipeline() + .replace( + ChannelFactory.SSL_HANDLER_NAME, + ChannelFactory.SSL_HANDLER_NAME, + sslHandler(/* hostnameVerification= */ false)); + + TlsInfo tlsInfo = ProtocolInitHandler.tlsInfo(sslChannel); + assertThat(tlsInfo.isEnabled()).isTrue(); + assertThat(tlsInfo.getHostnameVerification()).contains(false); + } finally { + sslChannel.finishAndReleaseAll(); + } + } + + private static SslHandler sslHandler(boolean hostnameVerification) throws Exception { + SSLEngine engine = SSLContext.getDefault().createSSLEngine(); + engine.setUseClientMode(true); + SSLParameters parameters = engine.getSSLParameters(); + parameters.setEndpointIdentificationAlgorithm(hostnameVerification ? "HTTPS" : null); + engine.setSSLParameters(parameters); + return new SslHandler(engine, true); } @Test @@ -216,7 +267,7 @@ public void should_not_consult_the_config_reporter_on_pool_connection() { assertThat(requestFrame.message).isInstanceOf(Startup.class); Startup startup = (Startup) requestFrame.message; assertThat(startup.options).doesNotContainKey(DefaultDriverConfigReporter.DRIVER_CONFIG_KEY); - verify(reporter, never()).populateControlConnectionOptions(any()); + verify(reporter, never()).populateControlConnectionOptions(any(), any()); } @Test diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java index 4122f82a399..f707cebaecf 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java @@ -42,6 +42,7 @@ import com.datastax.oss.driver.api.core.time.TimestampGenerator; import com.datastax.oss.driver.internal.core.connection.ConstantReconnectionPolicy; import com.datastax.oss.driver.internal.core.connection.ExponentialReconnectionPolicy; +import com.datastax.oss.driver.internal.core.context.DriverConfigReporter.TlsInfo; import com.datastax.oss.driver.internal.core.loadbalancing.BasicLoadBalancingPolicy; import com.datastax.oss.driver.internal.core.loadbalancing.DcInferringLoadBalancingPolicy; import com.datastax.oss.driver.internal.core.loadbalancing.DefaultLoadBalancingPolicy; @@ -49,9 +50,7 @@ import com.datastax.oss.driver.internal.core.retry.DefaultRetryPolicy; import com.datastax.oss.driver.internal.core.specex.ConstantSpeculativeExecutionPolicy; import com.datastax.oss.driver.internal.core.specex.NoSpeculativeExecutionPolicy; -import com.datastax.oss.driver.internal.core.ssl.DefaultSslEngineFactory; import com.datastax.oss.driver.internal.core.ssl.JdkSslHandlerFactory; -import com.datastax.oss.driver.internal.core.ssl.SniSslEngineFactory; import com.datastax.oss.driver.internal.core.ssl.SslHandlerFactory; import com.datastax.oss.driver.internal.core.time.AtomicTimestampGenerator; import com.datastax.oss.driver.internal.core.time.ServerSideTimestampGenerator; @@ -127,7 +126,7 @@ private void enableReporting(boolean enabled) { private DefaultDriverConfigReporter reporterReporting(Supplier json) { return new DefaultDriverConfigReporter(mockContext) { @Override - String buildJson() { + String buildJson(TlsInfo tlsInfo) { return json.get(); } }; @@ -142,7 +141,8 @@ String buildJson() { public void should_add_driver_config_when_enabled() { enableReporting(true); Map options = new HashMap<>(); - reporterReporting(() -> "{\"version\":1}").populateControlConnectionOptions(options); + reporterReporting(() -> "{\"version\":1}") + .populateControlConnectionOptions(options, TlsInfo.disabled()); assertThat(options) .hasSize(1) .containsEntry(DefaultDriverConfigReporter.DRIVER_CONFIG_KEY, "{\"version\":1}"); @@ -152,7 +152,7 @@ public void should_add_driver_config_when_enabled() { public void should_add_nothing_when_disabled() { enableReporting(false); Map options = new HashMap<>(); - reporter.populateControlConnectionOptions(options); + reporter.populateControlConnectionOptions(options, TlsInfo.disabled()); assertThat(options).isEmpty(); } @@ -163,7 +163,7 @@ public void should_add_driver_config_when_the_option_is_not_defined() { // getBoolean(), ignoring the fallback that is under test here. Map options = new HashMap<>(); defaultsReporter(map -> map.remove(TypedDriverOption.DRIVER_CONFIG_REPORTING_ENABLED)) - .populateControlConnectionOptions(options); + .populateControlConnectionOptions(options, TlsInfo.disabled()); assertThat(options).containsKey(DefaultDriverConfigReporter.DRIVER_CONFIG_KEY); } @@ -176,7 +176,7 @@ public void should_report_nothing_at_all_without_jackson() { // in-process; what is checked here is that the substitute contributes nothing and, in // particular, does not need a context to say so. Map options = new HashMap<>(); - new NoopDriverConfigReporter().populateControlConnectionOptions(options); + new NoopDriverConfigReporter().populateControlConnectionOptions(options, TlsInfo.disabled()); assertThat(options).isEmpty(); } @@ -187,7 +187,7 @@ public void should_not_throw_when_reading_the_flag_fails() { when(mockProfile.getBoolean(DefaultDriverOption.DRIVER_CONFIG_REPORTING_ENABLED, true)) .thenThrow(new IllegalStateException("config blew up")); Map options = new HashMap<>(); - reporter.populateControlConnectionOptions(options); // must not throw + reporter.populateControlConnectionOptions(options, TlsInfo.disabled()); // must not throw assertThat(options).isEmpty(); } @@ -199,7 +199,7 @@ public void should_skip_driver_config_when_building_fails() { () -> { throw new IllegalStateException("introspection blew up"); }) - .populateControlConnectionOptions(options); // must not throw + .populateControlConnectionOptions(options, TlsInfo.disabled()); // must not throw assertThat(options).isEmpty(); } @@ -208,7 +208,7 @@ public void should_skip_driver_config_when_serialization_fails() { // buildJson() returns null when Jackson fails to serialize the node tree. enableReporting(true); Map options = new HashMap<>(); - reporterReporting(() -> null).populateControlConnectionOptions(options); + reporterReporting(() -> null).populateControlConnectionOptions(options, TlsInfo.disabled()); assertThat(options).isEmpty(); } @@ -285,7 +285,7 @@ public void should_skip_driver_config_when_it_exceeds_the_size_limit() { enableReporting(true); Map options = new HashMap<>(); reporterReporting(() -> oversizedReport()) - .populateControlConnectionOptions(options); // must not throw + .populateControlConnectionOptions(options, TlsInfo.disabled()); // must not throw assertThat(options).isEmpty(); } @@ -294,7 +294,7 @@ public void should_add_driver_config_that_is_just_within_the_size_limit() { enableReporting(true); Map options = new HashMap<>(); String atLimit = padTo(DefaultDriverConfigReporter.MAX_DRIVER_CONFIG_LENGTH); - reporterReporting(() -> atLimit).populateControlConnectionOptions(options); + reporterReporting(() -> atLimit).populateControlConnectionOptions(options, TlsInfo.disabled()); assertThat(options).containsEntry(DefaultDriverConfigReporter.DRIVER_CONFIG_KEY, atLimit); } @@ -314,13 +314,13 @@ public void should_skip_a_report_a_configuration_pushes_over_the_size_limit() th // Built, well-formed and over the limit: it is dropped for its size, not because building it // failed. Reporting is left at the shipped default here, since defaultsReporter() reads a real // profile rather than the bare mock the tests above use. - String json = reporter.buildJson(); + String json = reporter.buildJson(TlsInfo.disabled()); assertConformsToSchema(MAPPER.readTree(json)); assertThat(json.getBytes(StandardCharsets.UTF_8).length) .isGreaterThan(DefaultDriverConfigReporter.MAX_DRIVER_CONFIG_LENGTH); Map options = new HashMap<>(); - reporter.populateControlConnectionOptions(options); + reporter.populateControlConnectionOptions(options, TlsInfo.disabled()); assertThat(options).isEmpty(); } @@ -1432,10 +1432,6 @@ private Boolean clientTimestampsOf(TimestampGenerator generator) throws Exceptio @Test public void should_report_tls_enabled_with_hostname_verification() throws Exception { - // hostname-verification comes from the factory's own state, not the config option. - SslEngineFactory factory = - new ProgrammaticSslEngineFactory( - SSLContext.getDefault(), null, /* requireHostnameValidation= */ true); DefaultDriverConfigReporter r = reporterWith( defaults(map -> {}), @@ -1444,21 +1440,16 @@ public void should_report_tls_enabled_with_hostname_verification() throws Except mock(NoSpeculativeExecutionPolicy.class), loadBalancing(DefaultLoadBalancingPolicy.class), clientSideGenerator(), - Optional.of(factory)); - JsonNode connection = report(r).get("connection"); + Optional.empty()); + JsonNode connection = report(r, TlsInfo.enabled(true)).get("connection"); // Presence of the group is what reports TLS as on: the schema dropped the "enabled" boolean. assertThat(connection.has("tls")).isTrue(); assertThat(connection.get("tls").get("hostname-verification").asBoolean()).isTrue(); } @Test - public void should_report_hostname_verification_from_factory_not_config_option() + public void should_report_hostname_verification_from_active_tls_snapshot_not_config_option() throws Exception { - // Regression for the false-report bug: a ProgrammaticSslEngineFactory (as built by - // SessionBuilder.withSslContext(...)) does NO hostname validation by default and ignores the - // SSL_HOSTNAME_VALIDATION config option. The report must reflect the factory's real state - // (false), not the config option (true here) — otherwise it falsely claims validation is on. - SslEngineFactory programmatic = new ProgrammaticSslEngineFactory(SSLContext.getDefault()); DefaultDriverConfigReporter r = reporterWith( defaults(map -> map.put(TypedDriverOption.SSL_HOSTNAME_VALIDATION, true)), @@ -1467,50 +1458,32 @@ public void should_report_hostname_verification_from_factory_not_config_option() mock(NoSpeculativeExecutionPolicy.class), loadBalancing(DefaultLoadBalancingPolicy.class), clientSideGenerator(), - Optional.of(programmatic)); - JsonNode connection = report(r).get("connection"); - // Presence of the group is what reports TLS as on: the schema dropped the "enabled" boolean. + Optional.empty()); + JsonNode connection = report(r, TlsInfo.enabled(false)).get("connection"); assertThat(connection.has("tls")).isTrue(); assertThat(connection.get("tls").get("hostname-verification").asBoolean()).isFalse(); } @Test - public void should_report_tls_enabled_for_a_custom_ssl_handler_factory() throws Exception { - // Overriding DefaultDriverContext.buildSslHandlerFactory() is the driver's documented low-level - // SSL extension point (e.g. Netty's native OpenSSL), and such an override supplies no - // SslEngineFactory at all. That session is still encrypted, so TLS must be read from the - // handler - // factory — the same reference ChannelFactory installs the SSL handler from — and not from - // getSslEngineFactory(). Host name validation is a property of the JDK SSLEngine and cannot be - // read on this path at all, so it is omitted rather than guessed in either direction. + public void should_omit_hostname_verification_when_active_engine_state_is_unknown() + throws Exception { DefaultDriverConfigReporter r = reporterWith( - defaults(map -> map.put(TypedDriverOption.SSL_HOSTNAME_VALIDATION, true)), + defaults(map -> {}), exponentialReconnection(), mock(DefaultRetryPolicy.class), mock(NoSpeculativeExecutionPolicy.class), loadBalancing(DefaultLoadBalancingPolicy.class), clientSideGenerator(), - /* ssl= */ Optional.empty(), - Optional.of(mock(SslHandlerFactory.class)), - /* programmaticLocalDc= */ null); - JsonNode report = report(r); - JsonNode connection = report.get("connection"); - // Presence of the group is what reports TLS as on: the schema dropped the "enabled" boolean. - assertThat(connection.has("tls")).isTrue(); - assertThat(connection.get("tls").has("hostname-verification")).isFalse(); - // An empty tls group is a valid document: the schema made hostname-verification optional so - // that "unknown" has a representation. + Optional.empty()); + JsonNode report = report(r, TlsInfo.enabledWithUnknownHostnameVerification()); + assertThat(report.get("connection").get("tls").has("hostname-verification")).isFalse(); assertConformsToSchema(report); } @Test - public void should_omit_hostname_verification_for_an_unrecognized_engine_factory() - throws Exception { - // The JDK path, but with a custom engine factory that is none of the driver's own, so its host - // name handling is unknown. Guessing false here would report a session as not checking host - // names when its factory may well be doing exactly that. - SslEngineFactory unrecognized = mock(SslEngineFactory.class); + public void should_not_report_tls_when_the_configured_ssl_handler_was_removed() throws Exception { + SslEngineFactory factory = new ProgrammaticSslEngineFactory(SSLContext.getDefault()); DefaultDriverConfigReporter r = reporterWith( defaults(map -> {}), @@ -1519,33 +1492,34 @@ public void should_omit_hostname_verification_for_an_unrecognized_engine_factory mock(NoSpeculativeExecutionPolicy.class), loadBalancing(DefaultLoadBalancingPolicy.class), clientSideGenerator(), - Optional.of(unrecognized)); - JsonNode report = report(r); + Optional.of(factory)); + + assertThat(report(r).get("connection").has("tls")).isFalse(); + } + + @Test + public void should_report_tls_when_a_pipeline_hook_added_an_ssl_handler() throws Exception { + DefaultDriverConfigReporter r = + reporterWith( + defaults(map -> {}), + exponentialReconnection(), + mock(DefaultRetryPolicy.class), + mock(NoSpeculativeExecutionPolicy.class), + loadBalancing(DefaultLoadBalancingPolicy.class), + clientSideGenerator(), + Optional.empty()); + + JsonNode report = report(r, TlsInfo.enabled(false)); JsonNode tls = report.get("connection").get("tls"); assertThat(tls).isNotNull(); - assertThat(tls.has("hostname-verification")).isFalse(); + assertThat(tls.get("hostname-verification").asBoolean()).isFalse(); assertConformsToSchema(report); } @Test - public void should_report_every_built_in_engine_factory() throws Exception { - // None of the built-ins is ever reported as unknown. Real instances rather than mocks, so that - // the branches are pinned to the classes the driver actually instantiates — and, for the - // configured one, to the whole option-to-field-to-report chain. - assertThat(hostnameVerificationOf(new DefaultSslEngineFactory(policyConstructionContext()))) - .isTrue(); - assertThat(hostnameVerificationOf(new SniSslEngineFactory(SSLContext.getDefault()))).isTrue(); - assertThat(hostnameVerificationOf(new ProgrammaticSslEngineFactory(SSLContext.getDefault()))) - .isFalse(); - assertThat( - hostnameVerificationOf( - new ProgrammaticSslEngineFactory( - SSLContext.getDefault(), null, /* requireHostnameValidation= */ true))) - .isTrue(); - } - - /** The {@code connection.tls.hostname-verification} a report built over this factory carries. */ - private Boolean hostnameVerificationOf(SslEngineFactory factory) throws Exception { + public void should_report_active_hostname_verification_for_an_unrecognized_engine_factory() + throws Exception { + SslEngineFactory unrecognized = mock(SslEngineFactory.class); DefaultDriverConfigReporter r = reporterWith( defaults(map -> {}), @@ -1554,24 +1528,18 @@ private Boolean hostnameVerificationOf(SslEngineFactory factory) throws Exceptio mock(NoSpeculativeExecutionPolicy.class), loadBalancing(DefaultLoadBalancingPolicy.class), clientSideGenerator(), - Optional.of(factory)); - JsonNode verification = report(r).get("connection").get("tls").get("hostname-verification"); - assertThat(verification).isNotNull(); - return verification.asBoolean(); + Optional.of(unrecognized)); + JsonNode report = report(r, TlsInfo.enabled(true)); + JsonNode tls = report.get("connection").get("tls"); + assertThat(tls).isNotNull(); + assertThat(tls.get("hostname-verification").asBoolean()).isTrue(); + assertConformsToSchema(report); } @Test - public void should_report_hostname_verification_from_the_engine_the_handler_actually_wraps() + public void should_report_active_hostname_verification_not_the_configured_factory() throws Exception { - // The configured engine factory and the one the active handler wraps can be different objects: - // a context that overrides buildSslHandlerFactory() may pass an engine factory of its own while - // advanced.ssl-engine-factory.class still names another. The report has to describe the engine - // that actually builds the connection's SSLEngine, so the wrapped one wins. - SslEngineFactory wrapped = - new ProgrammaticSslEngineFactory( - SSLContext.getDefault(), null, /* requireHostnameValidation= */ true); - SslEngineFactory configuredButUnused = - new ProgrammaticSslEngineFactory(SSLContext.getDefault()); + SslEngineFactory configuredButUnused = mock(SslEngineFactory.class); DefaultDriverConfigReporter r = reporterWith( defaults(map -> {}), @@ -1581,10 +1549,10 @@ public void should_report_hostname_verification_from_the_engine_the_handler_actu loadBalancing(DefaultLoadBalancingPolicy.class), clientSideGenerator(), Optional.of(configuredButUnused), - Optional.of(new JdkSslHandlerFactory(wrapped)), + Optional.of(mock(SslHandlerFactory.class)), /* programmaticLocalDc= */ null); - JsonNode connection = report(r).get("connection"); - assertThat(connection.get("tls").get("hostname-verification").asBoolean()).isTrue(); + JsonNode connection = report(r, TlsInfo.enabled(false)).get("connection"); + assertThat(connection.get("tls").get("hostname-verification").asBoolean()).isFalse(); } @Test @@ -1597,10 +1565,6 @@ public void should_not_resolve_the_configured_engine_factory_at_all() throws Exc // Mockito cannot have a when(...) open while another begins. ReconnectionPolicy reconnection = exponentialReconnection(); TimestampGenerator timestamps = clientSideGenerator(); - SslEngineFactory wrapped = - new ProgrammaticSslEngineFactory( - SSLContext.getDefault(), null, /* requireHostnameValidation= */ true); - SslHandlerFactory handlerFactory = new JdkSslHandlerFactory(wrapped); // Built before the stubbing chain below: the helper stubs the policy itself, and Mockito // rejects a nested when() inside an unfinished one. LoadBalancingPolicy policy = loadBalancing(DefaultLoadBalancingPolicy.class); @@ -1616,26 +1580,19 @@ public void should_not_resolve_the_configured_engine_factory_at_all() throws Exc .thenReturn(mock(NoSpeculativeExecutionPolicy.class)); when(ctx.getLoadBalancingPolicy(DriverExecutionProfile.DEFAULT_NAME)).thenReturn(policy); when(ctx.getTimestampGenerator()).thenReturn(timestamps); - when(ctx.getSslHandlerFactory()).thenReturn(Optional.of(handlerFactory)); + when(ctx.getSslHandlerFactory()).thenThrow(new AssertionError("must not be resolved")); when(ctx.getSslEngineFactory()) .thenThrow(new AssertionError("the configured engine factory must not be resolved")); - JsonNode report = MAPPER.readTree(new DefaultDriverConfigReporter(ctx).buildJson()); - // The group is built from the wrapped engine factory alone; getSslEngineFactory() throwing - // proves it was never consulted. + JsonNode report = + MAPPER.readTree(new DefaultDriverConfigReporter(ctx).buildJson(TlsInfo.enabled(true))); assertThat(report.get("connection").get("tls").get("hostname-verification").asBoolean()) .isTrue(); } @Test - public void should_not_report_hostname_verification_from_an_unused_engine_factory() + public void should_not_report_hostname_verification_from_an_unused_engine_factory_as_enabled() throws Exception { - // The handler factory and the engine factory are independent: a context can override - // buildSslHandlerFactory() (so the pipeline gets a handler the driver knows nothing about) and - // still have advanced.ssl-engine-factory.class configured, leaving a fully built engine factory - // that nothing on the connection path ever consults. Reading it would claim host name - // validation the custom handler does not perform, so hostname-verification is omitted unless - // the handler in force is the driver's own JdkSslHandlerFactory. SslEngineFactory validating = new ProgrammaticSslEngineFactory( SSLContext.getDefault(), null, /* requireHostnameValidation= */ true); @@ -1650,10 +1607,9 @@ public void should_not_report_hostname_verification_from_an_unused_engine_factor Optional.of(validating), Optional.of(mock(SslHandlerFactory.class)), /* programmaticLocalDc= */ null); - JsonNode connection = report(r).get("connection"); - // Presence of the group is what reports TLS as on: the schema dropped the "enabled" boolean. + JsonNode connection = report(r, TlsInfo.enabled(false)).get("connection"); assertThat(connection.has("tls")).isTrue(); - assertThat(connection.get("tls").has("hostname-verification")).isFalse(); + assertThat(connection.get("tls").get("hostname-verification").asBoolean()).isFalse(); } @Test @@ -2595,7 +2551,11 @@ private static TimestampGenerator clientSideGenerator() { } private JsonNode report(DefaultDriverConfigReporter reporter) throws Exception { - return MAPPER.readTree(reporter.buildJson()); + return report(reporter, TlsInfo.disabled()); + } + + private JsonNode report(DefaultDriverConfigReporter reporter, TlsInfo tlsInfo) throws Exception { + return MAPPER.readTree(reporter.buildJson(tlsInfo)); } /** A real default execution profile with the given customizations applied. */ @@ -2639,8 +2599,8 @@ private DefaultDriverConfigReporter reporterWith( TimestampGenerator timestamps, Optional ssl, String programmaticLocalDc) { - // tls.enabled reads the low-level handler factory, which DefaultDriverContext derives from the - // engine factory when SSL was configured through the public API; mirror that wrapping here. + // DefaultDriverContext derives the low-level handler factory from the public engine factory; + // mirror the context shape even though the reporter now reads neither one. return reporterWith( profile, reconnection, diff --git a/upgrade_guide/README.md b/upgrade_guide/README.md index 83963f94775..211f3580d92 100644 --- a/upgrade_guide/README.md +++ b/upgrade_guide/README.md @@ -32,8 +32,9 @@ a client's connections and inspect its driver settings while investigating an in turn it off. It is not derived from `CLIENT_ID`, which remains user-settable and unchanged. * `DRIVER_CONFIG` — a compact JSON description of the effective configuration of the session's default execution profile (connection/socket settings, timeouts, - retry/reconnection/speculative-execution/load-balancing policies, connection pooling, query - defaults, and TLS). Only the control connection sends it, since it describes the whole session. + retry/reconnection/speculative-execution/load-balancing policies, connection pooling, and query + defaults), plus the effective TLS state of the control connection carrying it. Only the control + connection sends it. It reports settings only — never credentials, statements or data — and identifies non-built-in policies by class name: the simple name, or the fully-qualified name when the policy is an anonymous class (which has no simple name). Reporting it is best-effort: if the report cannot be