From 25db0657066c163acbf2f653b6dbeb23d71bedce Mon Sep 17 00:00:00 2001 From: andreachild Date: Thu, 12 Sep 2024 12:41:29 -0700 Subject: [PATCH] Fixed failing GremlinServerSslIntegrateTest.shouldEnableSslAndFailIfCiphersDontMatch by reinstating the previous WebSocket channelizer logic that waited for the handshake to complete after the channel connects. If the handshake fails then a ConnectionException is thrown. (#2753) --- .../tinkerpop/gremlin/driver/Channelizer.java | 20 ++++++++++++++++++- .../server/GremlinServerSslIntegrateTest.java | 3 --- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Channelizer.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Channelizer.java index 8b80cc34579..43c1769c2da 100644 --- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Channelizer.java +++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Channelizer.java @@ -27,6 +27,7 @@ import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.ssl.SslContext; import io.netty.handler.ssl.SslHandler; +import org.apache.tinkerpop.gremlin.driver.exception.ConnectionException; import org.apache.tinkerpop.gremlin.driver.handler.GremlinResponseHandler; import org.apache.tinkerpop.gremlin.driver.handler.HttpContentDecompressionHandler; import org.apache.tinkerpop.gremlin.driver.handler.HttpGremlinRequestEncoder; @@ -87,6 +88,7 @@ default String getScheme(final boolean sslEnabled) { abstract class AbstractChannelizer extends ChannelInitializer implements Channelizer { protected Connection connection; protected Cluster cluster; + protected SslHandler sslHandler; private AtomicReference pending; protected static final String PIPELINE_GREMLIN_HANDLER = "gremlin-handler"; @@ -96,6 +98,10 @@ abstract class AbstractChannelizer extends ChannelInitializer imp protected static final String PIPELINE_HTTP_ENCODER = "gremlin-encoder"; protected static final String PIPELINE_HTTP_DECODER = "gremlin-decoder"; protected static final String PIPELINE_HTTP_DECOMPRESSION_HANDLER = "http-decompression-handler"; + + private static final String HANDSHAKE_ERROR = "Could not complete connection setup to the server. Ensure that SSL is correctly " + + "configured at both the client and the server. Ensure that client http handshake " + + "protocol matches the server. Ensure that the server is still reachable."; private static final SslCheckHandler sslCheckHandler = new SslCheckHandler(); @@ -136,7 +142,7 @@ protected void initChannel(final SocketChannel socketChannel) { } if (sslCtx.isPresent()) { - final SslHandler sslHandler = sslCtx.get().newHandler(socketChannel.alloc(), connection.getUri().getHost(), connection.getUri().getPort()); + sslHandler = sslCtx.get().newHandler(socketChannel.alloc(), connection.getUri().getHost(), connection.getUri().getPort()); // TINKERPOP-2814. Remove the SSL handshake timeout so that handshakes that take longer than 10000ms // (Netty default) but less than connectionSetupTimeoutMillis can succeed. This means the SSL handshake // will instead be capped by connectionSetupTimeoutMillis. @@ -149,6 +155,18 @@ protected void initChannel(final SocketChannel socketChannel) { configure(pipeline); pipeline.addLast(PIPELINE_GREMLIN_HANDLER, new GremlinResponseHandler(pending)); } + + @Override + public void connected() { + if (supportsSsl()) { + try { + // Block until the handshake is complete either successfully or with an error. + sslHandler.handshakeFuture().sync(); + } catch (Exception ex) { + throw new ConnectionException(connection.getUri(), HANDSHAKE_ERROR, ex); + } + } + } } /** diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSslIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSslIntegrateTest.java index cf2444cb022..8d97dd93fa1 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSslIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSslIntegrateTest.java @@ -30,7 +30,6 @@ import org.apache.tinkerpop.gremlin.driver.simple.SimpleClient; import org.apache.tinkerpop.gremlin.util.ExceptionHelper; import org.apache.tinkerpop.gremlin.util.message.RequestMessage; -import org.junit.Ignore; import org.junit.Test; import javax.net.ssl.SSLException; @@ -302,8 +301,6 @@ public void shouldEnableSslAndFailIfProtocolsDontMatch() { } } - // TODO: Add client-side SSL checking. - @Ignore("No client side SSL checking") @Test public void shouldEnableSslAndFailIfCiphersDontMatch() { final Cluster cluster = TestClientFactory.build().enableSsl(true).keyStore(JKS_SERVER_KEY).keyStorePassword(KEY_PASS)