Add sni name to SSLEngine in netty transport (elastic#33144)

Tim-Brooks · Tim-Brooks · commit 48fdd654af49 · 2018-09-07T10:17:39.000-06:00
This commit is related to elastic#32517. It allows an "server_name" attribute on a DiscoveryNode to be propagated to the server using the TLS SNI extentsion. This functionality is only implemented for the netty security transport.
diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java
@@ -97,12 +97,12 @@ public class Netty4Transport extends TcpTransport {
         intSetting("transport.netty.boss_count", 1, 1, Property.NodeScope);
 
 
-    protected final RecvByteBufAllocator recvByteBufAllocator;
-    protected final int workerCount;
-    protected final ByteSizeValue receivePredictorMin;
-    protected final ByteSizeValue receivePredictorMax;
-    protected volatile Bootstrap bootstrap;
-    protected final Map<String, ServerBootstrap> serverBootstraps = newConcurrentMap();
+    private final RecvByteBufAllocator recvByteBufAllocator;
+    private final int workerCount;
+    private final ByteSizeValue receivePredictorMin;
+    private final ByteSizeValue receivePredictorMax;
+    private volatile Bootstrap clientBootstrap;
+    private final Map<String, ServerBootstrap> serverBootstraps = newConcurrentMap();
 
     public Netty4Transport(Settings settings, ThreadPool threadPool, NetworkService networkService, BigArrays bigArrays,
                            NamedWriteableRegistry namedWriteableRegistry, CircuitBreakerService circuitBreakerService) {
@@ -125,7 +125,7 @@ public Netty4Transport(Settings settings, ThreadPool threadPool, NetworkService
     protected void doStart() {
         boolean success = false;
         try {
-            bootstrap = createBootstrap();
+            clientBootstrap = createClientBootstrap();
             if (NetworkService.NETWORK_SERVER.get(settings)) {
                 for (ProfileSettings profileSettings : profileSettings) {
                     createServerBootstrap(profileSettings);
@@ -141,13 +141,11 @@ protected void doStart() {
         }
     }
 
-    private Bootstrap createBootstrap() {
+    private Bootstrap createClientBootstrap() {
         final Bootstrap bootstrap = new Bootstrap();
         bootstrap.group(new NioEventLoopGroup(workerCount, daemonThreadFactory(settings, TRANSPORT_CLIENT_BOSS_THREAD_NAME_PREFIX)));
         bootstrap.channel(NioSocketChannel.class);
 
-        bootstrap.handler(getClientChannelInitializer());
-
         bootstrap.option(ChannelOption.TCP_NODELAY, TCP_NO_DELAY.get(settings));
         bootstrap.option(ChannelOption.SO_KEEPALIVE, TCP_KEEP_ALIVE.get(settings));
 
@@ -166,8 +164,6 @@ private Bootstrap createBootstrap() {
         final boolean reuseAddress = TCP_REUSE_ADDRESS.get(settings);
         bootstrap.option(ChannelOption.SO_REUSEADDR, reuseAddress);
 
-        bootstrap.validate();
-
         return bootstrap;
     }
 
@@ -215,7 +211,7 @@ protected ChannelHandler getServerChannelInitializer(String name) {
         return new ServerChannelInitializer(name);
     }
 
-    protected ChannelHandler getClientChannelInitializer() {
+    protected ChannelHandler getClientChannelInitializer(DiscoveryNode node) {
         return new ClientChannelInitializer();
     }
 
@@ -231,7 +227,11 @@ protected final void exceptionCaught(ChannelHandlerContext ctx, Throwable cause)
     @Override
     protected NettyTcpChannel initiateChannel(DiscoveryNode node, ActionListener<Void> listener) throws IOException {
         InetSocketAddress address = node.getAddress().address();
-        ChannelFuture channelFuture = bootstrap.connect(address);
+        Bootstrap bootstrapWithHandler = clientBootstrap.clone();
+        bootstrapWithHandler.handler(getClientChannelInitializer(node));
+        bootstrapWithHandler.remoteAddress(address);
+        ChannelFuture channelFuture = bootstrapWithHandler.connect();
+
         Channel channel = channelFuture.channel();
         if (channel == null) {
             ExceptionsHelper.maybeDieOnAnotherThread(channelFuture.cause());
@@ -294,9 +294,9 @@ protected void stopInternal() {
             }
             serverBootstraps.clear();
 
-            if (bootstrap != null) {
-                bootstrap.config().group().shutdownGracefully(0, 5, TimeUnit.SECONDS).awaitUninterruptibly();
-                bootstrap = null;
+            if (clientBootstrap != null) {
+                clientBootstrap.config().group().shutdownGracefully(0, 5, TimeUnit.SECONDS).awaitUninterruptibly();
+                clientBootstrap = null;
             }
         });
     }
diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java
@@ -152,6 +152,7 @@
 import org.elasticsearch.usage.UsageService;
 import org.elasticsearch.watcher.ResourceWatcherService;
 
+import javax.net.ssl.SNIHostName;
 import java.io.BufferedWriter;
 import java.io.Closeable;
 import java.io.IOException;
@@ -211,6 +212,13 @@ public class Node implements Closeable {
                 throw new IllegalArgumentException(key + " cannot have leading or trailing whitespace " +
                     "[" + value + "]");
             }
+            if (value.length() > 0 && "node.attr.server_name".equals(key)) {
+                try {
+                    new SNIHostName(value);
+                } catch (IllegalArgumentException e) {
+                    throw new IllegalArgumentException("invalid node.attr.server_name [" + value + "]", e );
+                }
+            }
             return value;
         }, Property.NodeScope));
     public static final Setting<String> BREAKER_TYPE_KEY = new Setting<>("indices.breaker.type", "hierarchy", (s) -> {
diff --git a/server/src/test/java/org/elasticsearch/node/NodeTests.java b/server/src/test/java/org/elasticsearch/node/NodeTests.java
@@ -150,6 +150,25 @@ public void testNodeAttributes() throws IOException {
         assertSettingDeprecationsAndWarnings(new Setting<?>[] { NetworkModule.HTTP_ENABLED });
     }
 
+    public void testServerNameNodeAttribute() throws IOException {
+        String attr = "valid-hostname";
+        Settings.Builder settings = baseSettings().put(Node.NODE_ATTRIBUTES.getKey() + "server_name", attr);
+        int i = 0;
+        try (Node node = new MockNode(settings.build(), basePlugins())) {
+            final Settings nodeSettings = randomBoolean() ? node.settings() : node.getEnvironment().settings();
+            assertEquals(attr, Node.NODE_ATTRIBUTES.getAsMap(nodeSettings).get("server_name"));
+        }
+
+        // non-LDH hostname not allowed
+        attr = "invalid_hostname";
+        settings = baseSettings().put(Node.NODE_ATTRIBUTES.getKey() + "server_name", attr);
+        try (Node node = new MockNode(settings.build(), basePlugins())) {
+            fail("should not allow a server_name attribute with an underscore");
+        } catch (IllegalArgumentException e) {
+            assertEquals("invalid node.attr.server_name [invalid_hostname]", e.getMessage());
+        }
+    }
+
     private static Settings.Builder baseSettings() {
         final Path tempDir = createTempDir();
         return Settings.builder()
diff --git a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java
@@ -2689,7 +2689,7 @@ private void closeConnectionChannel(Transport.Connection connection) {
     }
 
     @SuppressForbidden(reason = "need local ephemeral port")
-    private InetSocketAddress getLocalEphemeral() throws UnknownHostException {
+    protected InetSocketAddress getLocalEphemeral() throws UnknownHostException {
         return new InetSocketAddress(InetAddress.getLocalHost(), 0);
     }
 }
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/transport/netty4/SecurityNetty4Transport.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/transport/netty4/SecurityNetty4Transport.java
@@ -12,12 +12,14 @@
 import io.netty.channel.ChannelPromise;
 import io.netty.handler.ssl.SslHandler;
 import org.apache.logging.log4j.message.ParameterizedMessage;
+import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.network.NetworkService;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.indices.breaker.CircuitBreakerService;
 import org.elasticsearch.threadpool.ThreadPool;
+import org.elasticsearch.transport.ConnectTransportException;
 import org.elasticsearch.transport.TcpChannel;
 import org.elasticsearch.transport.TcpTransport;
 import org.elasticsearch.transport.netty4.Netty4Transport;
@@ -26,7 +28,10 @@
 import org.elasticsearch.xpack.core.ssl.SSLConfiguration;
 import org.elasticsearch.xpack.core.ssl.SSLService;
 
+import javax.net.ssl.SNIHostName;
+import javax.net.ssl.SNIServerName;
 import javax.net.ssl.SSLEngine;
+import javax.net.ssl.SSLParameters;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.util.Collections;
@@ -105,8 +110,8 @@ protected ChannelHandler getNoSslChannelInitializer(final String name) {
     }
 
     @Override
-    protected ChannelHandler getClientChannelInitializer() {
-        return new SecurityClientChannelInitializer();
+    protected ChannelHandler getClientChannelInitializer(DiscoveryNode node) {
+        return new SecurityClientChannelInitializer(node);
     }
 
     @Override
@@ -166,16 +171,28 @@ protected ServerChannelInitializer getSslChannelInitializer(final String name, f
     private class SecurityClientChannelInitializer extends ClientChannelInitializer {
 
         private final boolean hostnameVerificationEnabled;
+        private final SNIHostName serverName;
 
-        SecurityClientChannelInitializer() {
+        SecurityClientChannelInitializer(DiscoveryNode node) {
             this.hostnameVerificationEnabled = sslEnabled && sslConfiguration.verificationMode().isHostnameVerificationEnabled();
+            String configuredServerName = node.getAttributes().get("server_name");
+            if (configuredServerName != null) {
+                try {
+                    serverName = new SNIHostName(configuredServerName);
+                } catch (IllegalArgumentException e) {
+                    throw new ConnectTransportException(node, "invalid DiscoveryNode server_name [" + configuredServerName + "]", e);
+                }
+            } else {
+                serverName = null;
+            }
         }
 
         @Override
         protected void initChannel(Channel ch) throws Exception {
             super.initChannel(ch);
             if (sslEnabled) {
-                ch.pipeline().addFirst(new ClientSslHandlerInitializer(sslConfiguration, sslService, hostnameVerificationEnabled));
+                ch.pipeline().addFirst(new ClientSslHandlerInitializer(sslConfiguration, sslService, hostnameVerificationEnabled,
+                    serverName));
             }
         }
     }
@@ -185,11 +202,14 @@ private static class ClientSslHandlerInitializer extends ChannelOutboundHandlerA
         private final boolean hostnameVerificationEnabled;
         private final SSLConfiguration sslConfiguration;
         private final SSLService sslService;
+        private final SNIServerName serverName;
 
-        private ClientSslHandlerInitializer(SSLConfiguration sslConfiguration, SSLService sslService, boolean hostnameVerificationEnabled) {
+        private ClientSslHandlerInitializer(SSLConfiguration sslConfiguration, SSLService sslService, boolean hostnameVerificationEnabled,
+                                            SNIServerName serverName) {
             this.sslConfiguration = sslConfiguration;
             this.hostnameVerificationEnabled = hostnameVerificationEnabled;
             this.sslService = sslService;
+            this.serverName = serverName;
         }
 
         @Override
@@ -206,6 +226,11 @@ public void connect(ChannelHandlerContext ctx, SocketAddress remoteAddress,
             }
 
             sslEngine.setUseClientMode(true);
+            if (serverName != null) {
+                SSLParameters sslParameters = sslEngine.getSSLParameters();
+                sslParameters.setServerNames(Collections.singletonList(serverName));
+                sslEngine.setSSLParameters(sslParameters);
+            }
             ctx.pipeline().replace(this, "ssl", new SslHandler(sslEngine));
             super.connect(ctx, remoteAddress, localAddress, promise);
         }
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SimpleSecurityNetty4TransportTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SimpleSecurityNetty4TransportTests.java

Original file line number	Diff line number	Diff line change
`@@ -2689,7 +2689,7 @@ private void closeConnectionChannel(Transport.Connection connection) {`
`2689`	`2689`	`}`
`2690`	`2690`
`2691`	`2691`	`@SuppressForbidden(reason = "need local ephemeral port")`
`2692`		`- private InetSocketAddress getLocalEphemeral() throws UnknownHostException {`
	`2692`	`+ protected InetSocketAddress getLocalEphemeral() throws UnknownHostException {`
`2693`	`2693`	`return new InetSocketAddress(InetAddress.getLocalHost(), 0);`
`2694`	`2694`	`}`
`2695`	`2695`	`}`