This is an automated email from the ASF dual-hosted git repository.
zaynt4606 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/celeborn.git
The following commit(s) were added to refs/heads/main by this push:
new 9ae2834ee0 [CELEBORN-2386] Pass peer hostname to SSLEngine for SNI
support
9ae2834ee0 is described below
commit 9ae2834ee0cdb211687e2dbd0753b3c6417a1a26
Author: hzheng182 <[email protected]>
AuthorDate: Sat Oct 31 18:29:12 2026 +0800
[CELEBORN-2386] Pass peer hostname to SSLEngine for SNI support
### What changes were proposed in this pull request?
Thread peer hostname/port from
`TransportClientFactory.internalCreateClient()` through
`TransportContext.initializePipeline()` to `SSLFactory.createSSLEngine()`.
`SSLFactory.createEngine()` now calls `jdkSslContext.createSSLEngine(peerHost,
peerPort)` for client connections, which makes the JDK include the SNI
extension in the TLS ClientHello. Existing server-side and no-arg callers use
backward-compatible overloads.
### Why are the changes needed?
`SSLFactory.createEngine()` called `jdkSslContext.createSSLEngine()` with
no arguments. The JDK only includes the SNI extension in the TLS ClientHello
when a peer hostname is provided via `createSSLEngine(host, port)`. Without
SNI, TLS termination/routing that depends on server name (e.g. an SNI-based
ingress/gateway such as Istio TLS PASSTHROUGH or SIMPLE mode) cannot route or
differentiate connections by hostname. This fix is a prerequisite for
shared-port multi-tenant routing throu [...]
### Does this PR introduce _any_ user-facing change?
No. Client `SSLEngine` creation now includes SNI; server-side and existing
no-arg callers are unaffected.
### How was this patch tested?
- `SSLFactorySuiteJ`: client engines send SNI for a peer host name, and
omit it without a peer host or port, for IP literals, for server engines, and
through the existing no-arg overload.
- `SslConnectivitySuiteJ.testSniHostNameReachesServer`: connects through
`TransportClientFactory` to a host name (`sni.localhost`) with pooled and
unmanaged clients over TLS 1.2 and TLS 1.3, and asserts the server observes it
via `ExtendedSSLSession.getRequestedServerNames()`. This fails if the peer host
or port is dropped anywhere along `TransportClientFactory` ->
`TransportContext` -> `SSLFactory`. The JDK sends SNI only for host names
containing a dot, hence a `.localhost` name; th [...]
Closes #3764 from shellfish007/upstream-pr/ssl-sni-support.
Authored-by: hzheng182 <[email protected]>
Signed-off-by: zhengtao <[email protected]>
AI-Contributed/Feature: 0/49
AI-Contributed/UT: 0/191
---
.../celeborn/common/network/TransportContext.java | 23 ++++-
.../network/client/TransportClientFactory.java | 4 +-
.../celeborn/common/network/ssl/SSLFactory.java | 22 +++-
.../common/network/ssl/SSLFactorySuiteJ.java | 112 +++++++++++++++++++++
.../common/network/ssl/SslConnectivitySuiteJ.java | 79 +++++++++++++++
5 files changed, 236 insertions(+), 4 deletions(-)
diff --git
a/common/src/main/java/org/apache/celeborn/common/network/TransportContext.java
b/common/src/main/java/org/apache/celeborn/common/network/TransportContext.java
index 464e9d1e62..7145bdbaac 100644
---
a/common/src/main/java/org/apache/celeborn/common/network/TransportContext.java
+++
b/common/src/main/java/org/apache/celeborn/common/network/TransportContext.java
@@ -177,6 +177,15 @@ public class TransportContext implements Closeable {
return initializePipeline(channel, decoder, msgHandler, isClient);
}
+ public TransportChannelHandler initializePipeline(
+ SocketChannel channel,
+ ChannelInboundHandlerAdapter decoder,
+ boolean isClient,
+ String peerHost,
+ int peerPort) {
+ return initializePipeline(channel, decoder, msgHandler, isClient,
peerHost, peerPort);
+ }
+
public TransportChannelHandler initializePipeline(
SocketChannel channel, BaseMessageHandler resolvedMsgHandler, boolean
isClient) {
return initializePipeline(channel, new TransportFrameDecoder(),
resolvedMsgHandler, isClient);
@@ -187,6 +196,16 @@ public class TransportContext implements Closeable {
ChannelInboundHandlerAdapter decoder,
BaseMessageHandler resolvedMsgHandler,
boolean isClient) {
+ return initializePipeline(channel, decoder, resolvedMsgHandler, isClient,
null, -1);
+ }
+
+ public TransportChannelHandler initializePipeline(
+ SocketChannel channel,
+ ChannelInboundHandlerAdapter decoder,
+ BaseMessageHandler resolvedMsgHandler,
+ boolean isClient,
+ String peerHost,
+ int peerPort) {
try {
ChannelPipeline pipeline = channel.pipeline();
if (nettyLogger.getLoggingHandler() != null) {
@@ -199,7 +218,9 @@ public class TransportContext implements Closeable {
SslHandler sslHandler;
try {
- sslHandler = new SslHandler(sslFactory.createSSLEngine(isClient,
channel.alloc()));
+ sslHandler =
+ new SslHandler(
+ sslFactory.createSSLEngine(isClient, channel.alloc(),
peerHost, peerPort));
sslHandler.setHandshakeTimeoutMillis(conf.sslHandshakeTimeoutMs());
} catch (Exception e) {
throw new IllegalStateException("Error creating Netty SslHandler",
e);
diff --git
a/common/src/main/java/org/apache/celeborn/common/network/client/TransportClientFactory.java
b/common/src/main/java/org/apache/celeborn/common/network/client/TransportClientFactory.java
index b79383277a..f96ddceb96 100644
---
a/common/src/main/java/org/apache/celeborn/common/network/client/TransportClientFactory.java
+++
b/common/src/main/java/org/apache/celeborn/common/network/client/TransportClientFactory.java
@@ -309,7 +309,9 @@ public class TransportClientFactory implements Closeable {
new ChannelInitializer<SocketChannel>() {
@Override
public void initChannel(SocketChannel ch) {
- TransportChannelHandler clientHandler =
context.initializePipeline(ch, decoder, true);
+ TransportChannelHandler clientHandler =
+ context.initializePipeline(
+ ch, decoder, true, address.getHostString(),
address.getPort());
clientRef.set(clientHandler.getClient());
}
});
diff --git
a/common/src/main/java/org/apache/celeborn/common/network/ssl/SSLFactory.java
b/common/src/main/java/org/apache/celeborn/common/network/ssl/SSLFactory.java
index a0eb106bf2..c3d872186f 100644
---
a/common/src/main/java/org/apache/celeborn/common/network/ssl/SSLFactory.java
+++
b/common/src/main/java/org/apache/celeborn/common/network/ssl/SSLFactory.java
@@ -284,7 +284,21 @@ public class SSLFactory {
* @return A valid {@link SSLEngine}.
*/
public SSLEngine createSSLEngine(boolean isClient, ByteBufAllocator
allocator) {
- SSLEngine engine = createEngine(isClient, allocator);
+ return createSSLEngine(isClient, allocator, null, -1);
+ }
+
+ /**
+ * Creates a new {@link SSLEngine} with peer hostname for SNI support.
+ *
+ * @param isClient Whether the engine is used in a client context
+ * @param allocator The {@link ByteBufAllocator to use}
+ * @param peerHost The peer hostname (used for SNI in client mode)
+ * @param peerPort The peer port
+ * @return A valid {@link SSLEngine}.
+ */
+ public SSLEngine createSSLEngine(
+ boolean isClient, ByteBufAllocator allocator, String peerHost, int
peerPort) {
+ SSLEngine engine = createEngine(isClient, allocator, peerHost, peerPort);
engine.setUseClientMode(isClient);
engine.setWantClientAuth(true);
engine.setEnabledProtocols(enabledProtocols(engine, requestedProtocol));
@@ -292,7 +306,11 @@ public class SSLFactory {
return engine;
}
- private SSLEngine createEngine(boolean isClient, ByteBufAllocator allocator)
{
+ private SSLEngine createEngine(
+ boolean isClient, ByteBufAllocator allocator, String peerHost, int
peerPort) {
+ if (isClient && peerHost != null && peerPort > 0) {
+ return jdkSslContext.createSSLEngine(peerHost, peerPort);
+ }
return jdkSslContext.createSSLEngine();
}
diff --git
a/common/src/test/java/org/apache/celeborn/common/network/ssl/SSLFactorySuiteJ.java
b/common/src/test/java/org/apache/celeborn/common/network/ssl/SSLFactorySuiteJ.java
new file mode 100644
index 0000000000..859fc3cb3a
--- /dev/null
+++
b/common/src/test/java/org/apache/celeborn/common/network/ssl/SSLFactorySuiteJ.java
@@ -0,0 +1,112 @@
+/*
+ * 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.celeborn.common.network.ssl;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import java.util.List;
+
+import javax.net.ssl.SNIHostName;
+import javax.net.ssl.SNIServerName;
+import javax.net.ssl.SSLEngine;
+
+import io.netty.buffer.ByteBufAllocator;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.celeborn.common.CelebornConf;
+import org.apache.celeborn.common.network.TestHelper;
+import org.apache.celeborn.common.network.util.TransportConf;
+
+/** Tests that client engines carry the peer hostname so the ClientHello
includes SNI. */
+public class SSLFactorySuiteJ {
+
+ private static final String TEST_MODULE = "rpc";
+ // SNI carries DNS names only, so this must be multi-label and not an IP
literal.
+ private static final String PEER_HOST = "celeborn-master-1.example.com";
+ private static final int PEER_PORT = 9097;
+
+ private SSLFactory factory;
+
+ @Before
+ public void setUp() {
+ CelebornConf celebornConf = new CelebornConf();
+ TestHelper.updateCelebornConfWithMap(
+ celebornConf,
SslSampleConfigs.createDefaultConfigMapForModule(TEST_MODULE));
+ factory = SSLFactory.createSslFactory(new TransportConf(TEST_MODULE,
celebornConf));
+ assertNotNull(factory);
+ }
+
+ @After
+ public void tearDown() {
+ if (factory != null) {
+ factory.destroy();
+ factory = null;
+ }
+ }
+
+ private static void assertSniHost(SSLEngine engine, String expectedHost) {
+ List<SNIServerName> serverNames =
engine.getSSLParameters().getServerNames();
+ assertNotNull("expected an SNI server_name entry", serverNames);
+ assertEquals(1, serverNames.size());
+ assertEquals(new SNIHostName(expectedHost), serverNames.get(0));
+ }
+
+ private static void assertNoSni(SSLEngine engine) {
+ List<SNIServerName> serverNames =
engine.getSSLParameters().getServerNames();
+ assertTrue("expected no SNI server_name entry", serverNames == null ||
serverNames.isEmpty());
+ }
+
+ @Test
+ public void testClientEngineSendsSniForPeerHost() {
+ SSLEngine engine =
+ factory.createSSLEngine(true, ByteBufAllocator.DEFAULT, PEER_HOST,
PEER_PORT);
+ assertEquals(PEER_HOST, engine.getPeerHost());
+ assertEquals(PEER_PORT, engine.getPeerPort());
+ assertSniHost(engine, PEER_HOST);
+ }
+
+ @Test
+ public void testClientEngineWithoutPeerHostOmitsSni() {
+ assertNoSni(factory.createSSLEngine(true, ByteBufAllocator.DEFAULT, null,
PEER_PORT));
+ }
+
+ @Test
+ public void testClientEngineWithoutPeerPortOmitsSni() {
+ assertNoSni(factory.createSSLEngine(true, ByteBufAllocator.DEFAULT,
PEER_HOST, -1));
+ }
+
+ @Test
+ public void testClientEngineOmitsSniForIpLiteral() {
+ // RFC 6066 disallows IP literals in server_name, so the JDK drops the
extension.
+ assertNoSni(factory.createSSLEngine(true, ByteBufAllocator.DEFAULT,
"127.0.0.1", PEER_PORT));
+ }
+
+ @Test
+ public void testServerEngineOmitsSni() {
+ assertNoSni(factory.createSSLEngine(false, ByteBufAllocator.DEFAULT,
PEER_HOST, PEER_PORT));
+ }
+
+ @Test
+ public void testExistingOverloadOmitsSni() {
+ assertNoSni(factory.createSSLEngine(true, ByteBufAllocator.DEFAULT));
+ }
+}
diff --git
a/common/src/test/java/org/apache/celeborn/common/network/ssl/SslConnectivitySuiteJ.java
b/common/src/test/java/org/apache/celeborn/common/network/ssl/SslConnectivitySuiteJ.java
index 52fadb69c4..3e942e7574 100644
---
a/common/src/test/java/org/apache/celeborn/common/network/ssl/SslConnectivitySuiteJ.java
+++
b/common/src/test/java/org/apache/celeborn/common/network/ssl/SslConnectivitySuiteJ.java
@@ -19,16 +19,27 @@ package org.apache.celeborn.common.network.ssl;
import static org.apache.celeborn.common.util.JavaUtils.getLocalHost;
import static org.junit.Assert.*;
+import static org.junit.Assume.assumeTrue;
import java.io.Closeable;
import java.io.IOException;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
import java.nio.ByteBuffer;
+import java.util.List;
import java.util.Map;
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
+import java.util.stream.Collectors;
+import javax.net.ssl.ExtendedSSLSession;
+import javax.net.ssl.SNIHostName;
+import javax.net.ssl.SNIServerName;
+import javax.net.ssl.SSLSession;
+
+import io.netty.handler.ssl.SslHandler;
import org.apache.commons.lang3.tuple.Pair;
import org.junit.Test;
@@ -105,6 +116,13 @@ public class SslConnectivitySuiteJ {
return clientFactory.createClient(getLocalHost(), server.getPort());
}
+ TransportClient createClient(String host, boolean unmanaged)
+ throws IOException, InterruptedException {
+ return unmanaged
+ ? clientFactory.createUnmanagedClient(host, server.getPort())
+ : clientFactory.createClient(host, server.getPort());
+ }
+
@Override
public void close() throws IOException {
JavaUtils.closeQuietly(server);
@@ -134,6 +152,28 @@ public class SslConnectivitySuiteJ {
}
}
+ // Replies with the SNI host names the server observed on the client's
connection.
+ private static class SniEchoMessageHandler extends BaseMessageHandler {
+
+ @Override
+ public void receive(
+ TransportClient client, RequestMessage requestMessage,
RpcResponseCallback callback) {
+ SSLSession session =
+
client.getChannel().pipeline().get(SslHandler.class).engine().getSession();
+ List<SNIServerName> serverNames = ((ExtendedSSLSession)
session).getRequestedServerNames();
+ String response =
+ serverNames.stream()
+ .map(name -> ((SNIHostName) name).getAsciiName())
+ .collect(Collectors.joining(","));
+ callback.onSuccess(JavaUtils.stringToBytes(response));
+ }
+
+ @Override
+ public boolean checkRegistered() {
+ return true;
+ }
+ }
+
// Pair<Success, Failure> ... should be an Either actually.
private Pair<String, String> sendRPC(TransportClient client, String message,
boolean canTimeout)
@@ -394,6 +434,45 @@ public class SslConnectivitySuiteJ {
}
}
+ // Connects through TransportClientFactory using a host name, and verifies
the server observes
+ // it as SNI - i.e. the peer host and port reach the client SSLEngine.
+ @Test
+ public void testSniHostNameReachesServer() throws Exception {
+ // The JDK sends SNI only for host names containing a dot, so "localhost"
is not enough.
+ // Names under ".localhost" resolve to loopback (RFC 6761) on most
resolvers.
+ final String sniHost = "sni.localhost";
+ assumeTrue(sniHost + " does not resolve", resolves(sniHost));
+
+ for (String protocol : new String[] {"TLSv1.2", "TLSv1.3"}) {
+ final Function<CelebornConf, CelebornConf> updateConf =
+ conf -> {
+ conf.set("celeborn.ssl." + TEST_MODULE + ".protocol", protocol);
+ return conf;
+ };
+ for (boolean unmanaged : new boolean[] {false, true}) {
+ try (TestTransportState state =
+ new TestTransportState(
+ new SniEchoMessageHandler(),
+ createTransportConf(TEST_MODULE, true, true, updateConf),
+ createTransportConf(TEST_MODULE, true, true, updateConf));
+ TransportClient client = state.createClient(sniHost, unmanaged)) {
+ Pair<String, String> response = sendRPC(client, "sni", false);
+ assertNull(response.getRight());
+ assertEquals(protocol + ", unmanaged=" + unmanaged, sniHost,
response.getLeft());
+ }
+ }
+ }
+ }
+
+ private static boolean resolves(String host) {
+ try {
+ InetAddress.getByName(host);
+ return true;
+ } catch (UnknownHostException e) {
+ return false;
+ }
+ }
+
@Test
public void testAutoSslConnectivity() throws Exception {
