From 26ea13ee8c399d2725c78834462287fbb4dd9f9f Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Thu, 12 Jul 2018 15:03:05 -0600 Subject: [PATCH 1/7] WIP --- .../http/nio/HttpReadWriteHandler.java | 4 +- .../http/nio/NioHttpChannel.java | 2 +- .../http/nio/NioHttpServerChannel.java | 2 +- .../http/nio/NioHttpServerTransport.java | 28 +-- .../SecurityNetty4HttpServerTransport.java | 1 - .../security/transport/nio/NioIPFilter.java | 32 ++++ .../nio/SecurityNioHttpServerTransport.java | 171 ++++++++++++++++++ .../transport/nio/SecurityNioTransport.java | 19 +- 8 files changed, 229 insertions(+), 30 deletions(-) create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/NioIPFilter.java create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java diff --git a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/HttpReadWriteHandler.java b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/HttpReadWriteHandler.java index ad81719ebcbb9..3dcd59cf8e28c 100644 --- a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/HttpReadWriteHandler.java +++ b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/HttpReadWriteHandler.java @@ -51,8 +51,8 @@ public class HttpReadWriteHandler implements ReadWriteHandler { private final NioHttpChannel nioHttpChannel; private final NioHttpServerTransport transport; - HttpReadWriteHandler(NioHttpChannel nioHttpChannel, NioHttpServerTransport transport, HttpHandlingSettings settings, - NioCorsConfig corsConfig) { + public HttpReadWriteHandler(NioHttpChannel nioHttpChannel, NioHttpServerTransport transport, HttpHandlingSettings settings, + NioCorsConfig corsConfig) { this.nioHttpChannel = nioHttpChannel; this.transport = transport; diff --git a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpChannel.java b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpChannel.java index 0a797a5687ec7..1a4c5f14c91da 100644 --- a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpChannel.java +++ b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpChannel.java @@ -28,7 +28,7 @@ public class NioHttpChannel extends NioSocketChannel implements HttpChannel { - NioHttpChannel(SocketChannel socketChannel) { + public NioHttpChannel(SocketChannel socketChannel) { super(socketChannel); } diff --git a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerChannel.java b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerChannel.java index 2674d38dc490e..520704c6bf441 100644 --- a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerChannel.java +++ b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerChannel.java @@ -28,7 +28,7 @@ public class NioHttpServerChannel extends NioServerSocketChannel implements HttpServerChannel { - NioHttpServerChannel(ServerSocketChannel serverSocketChannel) throws IOException { + public NioHttpServerChannel(ServerSocketChannel serverSocketChannel) { super(serverSocketChannel); } diff --git a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerTransport.java b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerTransport.java index b80778e964293..6a5ce713f3665 100644 --- a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerTransport.java +++ b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerTransport.java @@ -49,6 +49,8 @@ import org.elasticsearch.nio.SocketChannelContext; import org.elasticsearch.rest.RestUtils; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.TcpTransport; +import org.elasticsearch.transport.nio.NioTransport; import java.io.IOException; import java.net.InetSocketAddress; @@ -87,21 +89,21 @@ public class NioHttpServerTransport extends AbstractHttpServerTransport { (s) -> Integer.toString(EsExecutors.numberOfProcessors(s) * 2), (s) -> Setting.parseInt(s, 1, "http.nio.worker_count"), Setting.Property.NodeScope); - private final PageCacheRecycler pageCacheRecycler; + protected final PageCacheRecycler pageCacheRecycler; + protected final NioCorsConfig corsConfig; - private final boolean tcpNoDelay; - private final boolean tcpKeepAlive; - private final boolean reuseAddress; - private final int tcpSendBufferSize; - private final int tcpReceiveBufferSize; + protected final boolean tcpNoDelay; + protected final boolean tcpKeepAlive; + protected final boolean reuseAddress; + protected final int tcpSendBufferSize; + protected final int tcpReceiveBufferSize; private NioGroup nioGroup; - private HttpChannelFactory channelFactory; - private final NioCorsConfig corsConfig; + private ChannelFactory channelFactory; public NioHttpServerTransport(Settings settings, NetworkService networkService, BigArrays bigArrays, PageCacheRecycler pageCacheRecycler, ThreadPool threadPool, NamedXContentRegistry xContentRegistry, - HttpServerTransport.Dispatcher dispatcher) { + Dispatcher dispatcher) { super(settings, networkService, bigArrays, threadPool, xContentRegistry, dispatcher); this.pageCacheRecycler = pageCacheRecycler; @@ -136,7 +138,7 @@ protected void doStart() { nioGroup = new NioGroup(daemonThreadFactory(this.settings, HTTP_SERVER_ACCEPTOR_THREAD_NAME_PREFIX), acceptorCount, daemonThreadFactory(this.settings, HTTP_SERVER_WORKER_THREAD_NAME_PREFIX), workerCount, (s) -> new EventHandler(this::onNonChannelException, s)); - channelFactory = new HttpChannelFactory(); + channelFactory = channelFactory(); bindServer(); success = true; } catch (IOException e) { @@ -162,6 +164,10 @@ protected HttpServerChannel bind(InetSocketAddress socketAddress) throws IOExcep return nioGroup.bindServerChannel(socketAddress, channelFactory); } + protected ChannelFactory channelFactory() { + return new HttpChannelFactory(); + } + static NioCorsConfig buildCorsConfig(Settings settings) { if (SETTING_CORS_ENABLED.get(settings) == false) { return NioCorsConfigBuilder.forOrigins().disable().build(); @@ -194,7 +200,7 @@ static NioCorsConfig buildCorsConfig(Settings settings) { .build(); } - private void acceptChannel(NioSocketChannel socketChannel) { + protected void acceptChannel(NioSocketChannel socketChannel) { super.serverAcceptedChannel((HttpChannel) socketChannel); } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransport.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransport.java index 9667ca675b4c1..4bc9430468781 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransport.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransport.java @@ -53,7 +53,6 @@ public SecurityNetty4HttpServerTransport(Settings settings, NetworkService netwo } else { this.sslConfiguration = null; } - } @Override diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/NioIPFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/NioIPFilter.java new file mode 100644 index 0000000000000..54f3de5b4a685 --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/NioIPFilter.java @@ -0,0 +1,32 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.security.transport.nio; + +import org.elasticsearch.common.Nullable; +import org.elasticsearch.nio.NioSocketChannel; +import org.elasticsearch.xpack.security.transport.filter.IPFilter; + +import java.util.function.Predicate; + +public class NioIPFilter implements Predicate { + + private final IPFilter filter; + private final String profile; + + NioIPFilter(@Nullable IPFilter filter, String profile) { + this.filter = filter; + this.profile = profile; + } + + @Override + public boolean test(NioSocketChannel nioChannel) { + if (filter != null) { + return filter.accept(profile, nioChannel.getRemoteAddress()); + } else { + return true; + } + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java new file mode 100644 index 0000000000000..27bc1961afa3f --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java @@ -0,0 +1,171 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.security.transport.nio; + +import org.apache.logging.log4j.message.ParameterizedMessage; +import org.elasticsearch.common.network.CloseableChannel; +import org.elasticsearch.common.network.NetworkService; +import org.elasticsearch.common.recycler.Recycler; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.BigArrays; +import org.elasticsearch.common.util.PageCacheRecycler; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.http.HttpChannel; +import org.elasticsearch.http.nio.HttpReadWriteHandler; +import org.elasticsearch.http.nio.NioHttpChannel; +import org.elasticsearch.http.nio.NioHttpServerChannel; +import org.elasticsearch.http.nio.NioHttpServerTransport; +import org.elasticsearch.nio.BytesChannelContext; +import org.elasticsearch.nio.ChannelFactory; +import org.elasticsearch.nio.InboundChannelBuffer; +import org.elasticsearch.nio.NioSelector; +import org.elasticsearch.nio.NioSocketChannel; +import org.elasticsearch.nio.ServerChannelContext; +import org.elasticsearch.nio.SocketChannelContext; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.core.ssl.SSLConfiguration; +import org.elasticsearch.xpack.core.ssl.SSLService; +import org.elasticsearch.xpack.security.transport.filter.IPFilter; + +import javax.net.ssl.SSLEngine; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.nio.ByteBuffer; +import java.nio.channels.ServerSocketChannel; +import java.nio.channels.SocketChannel; +import java.util.function.Consumer; +import java.util.function.Supplier; + +import static org.elasticsearch.xpack.core.XPackSettings.HTTP_SSL_ENABLED; +import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isCloseDuringHandshakeException; +import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isNotSslRecordException; +import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isReceivedCertificateUnknownException; + +public class SecurityNioHttpServerTransport extends NioHttpServerTransport { + + private final IPFilter ipFilter; + private final NioIPFilter nioIpFilter; + private final Settings sslSettings; + private final SSLService sslService; + private final SSLConfiguration sslConfiguration; + private final boolean sslEnabled; + + public SecurityNioHttpServerTransport(Settings settings, NetworkService networkService, BigArrays bigArrays, + PageCacheRecycler pageCacheRecycler, ThreadPool threadPool, + NamedXContentRegistry xContentRegistry, Dispatcher dispatcher, IPFilter ipFilter, + SSLService sslService) { + super(settings, networkService, bigArrays, pageCacheRecycler, threadPool, xContentRegistry, dispatcher); + this.ipFilter = ipFilter; + this.nioIpFilter = new NioIPFilter(ipFilter, IPFilter.HTTP_PROFILE_NAME); + sslEnabled = HTTP_SSL_ENABLED.get(settings); + this.sslSettings = SSLService.getHttpTransportSSLSettings(settings); + this.sslService = sslService; + if (sslEnabled) { + this.sslConfiguration = sslService.sslConfiguration(sslSettings, Settings.EMPTY); + if (sslService.isConfigurationValidForServerUsage(sslConfiguration) == false) { + throw new IllegalArgumentException("a key must be provided to run as a server. the key should be configured using the " + + "[xpack.security.http.ssl.key] or [xpack.security.http.ssl.keystore.path] setting"); + } + } else { + this.sslConfiguration = null; + } + } + + @Override + protected void onException(HttpChannel channel, Exception e) { + if (!lifecycle.started()) { + return; + } + + if (isNotSslRecordException(e)) { + if (logger.isTraceEnabled()) { + logger.trace(new ParameterizedMessage("received plaintext http traffic on a https channel, closing connection {}", + channel), e); + } else { + logger.warn("received plaintext http traffic on a https channel, closing connection {}", channel); + } + CloseableChannel.closeChannel(channel); + } else if (isCloseDuringHandshakeException(e)) { + if (logger.isTraceEnabled()) { + logger.trace(new ParameterizedMessage("connection {} closed during ssl handshake", channel), e); + } else { + logger.warn("connection {} closed during ssl handshake", channel); + } + CloseableChannel.closeChannel(channel); + } else if (isReceivedCertificateUnknownException(e)) { + if (logger.isTraceEnabled()) { + logger.trace(new ParameterizedMessage("http client did not trust server's certificate, closing connection {}", + channel), e); + } else { + logger.warn("http client did not trust this server's certificate, closing connection {}", channel); + } + CloseableChannel.closeChannel(channel); + } else { + super.onException(channel, e); + } + } + + @Override + protected void doStart() { + super.doStart(); + ipFilter.setBoundHttpTransportAddress(this.boundAddress()); + } + + protected SecurityHttpChannelFactory channelFactory() { + return new SecurityHttpChannelFactory(); + } + + private class SecurityHttpChannelFactory extends ChannelFactory { + + private SecurityHttpChannelFactory() { + super(new RawChannelFactory(tcpNoDelay, tcpKeepAlive, reuseAddress, tcpSendBufferSize, tcpReceiveBufferSize)); + } + + @Override + public NioHttpChannel createChannel(NioSelector selector, SocketChannel channel) throws IOException { + NioHttpChannel httpChannel = new NioHttpChannel(channel); + Supplier pageSupplier = () -> { + Recycler.V bytes = pageCacheRecycler.bytePage(false); + return new InboundChannelBuffer.Page(ByteBuffer.wrap(bytes.v()), bytes::close); + }; + HttpReadWriteHandler httpHandler = new HttpReadWriteHandler(httpChannel,SecurityNioHttpServerTransport.this, + handlingSettings, corsConfig); + InboundChannelBuffer buffer = new InboundChannelBuffer(pageSupplier); + Consumer exceptionHandler = (e) -> onException(httpChannel, e); + + SocketChannelContext context; + if (sslEnabled) { + SSLEngine sslEngine; + boolean hostnameVerificationEnabled = sslConfiguration.verificationMode().isHostnameVerificationEnabled(); + if (hostnameVerificationEnabled) { + InetSocketAddress address = (InetSocketAddress) channel.getRemoteAddress(); + // we create the socket based on the name given. don't reverse DNS + sslEngine = sslService.createSSLEngine(sslConfiguration, address.getHostString(), address.getPort()); + } else { + sslEngine = sslService.createSSLEngine(sslConfiguration, null, -1); + } + SSLDriver sslDriver = new SSLDriver(sslEngine, false); + context = new SSLChannelContext(httpChannel, selector, exceptionHandler, sslDriver, httpHandler, buffer, nioIpFilter); + } else { + context = new BytesChannelContext(httpChannel, selector, exceptionHandler, httpHandler, buffer, nioIpFilter); + } + httpChannel.setContext(context); + + return httpChannel; + } + + @Override + public NioHttpServerChannel createServerChannel(NioSelector selector, ServerSocketChannel channel) throws IOException { + NioHttpServerChannel httpServerChannel = new NioHttpServerChannel(channel); + Consumer exceptionHandler = (e) -> onServerException(httpServerChannel, e); + Consumer acceptor = SecurityNioHttpServerTransport.this::acceptChannel; + ServerChannelContext context = new ServerChannelContext(httpServerChannel, this, selector, acceptor, exceptionHandler); + httpServerChannel.setContext(context); + + return httpServerChannel; + } + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioTransport.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioTransport.java index 1e00019793025..8bc92a11ca634 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioTransport.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioTransport.java @@ -45,7 +45,6 @@ import java.util.HashMap; import java.util.Map; import java.util.function.Consumer; -import java.util.function.Predicate; import java.util.function.Supplier; import static org.elasticsearch.xpack.core.security.SecurityField.setting; @@ -140,19 +139,11 @@ protected TcpChannelFactory channelFactory(ProfileSettings profileSettings, bool return new SecurityTcpChannelFactory(profileSettings, isClient); } - private boolean validateChannel(NioSocketChannel channel) { - if (authenticator != null) { - NioTcpChannel nioTcpChannel = (NioTcpChannel) channel; - return authenticator.accept(nioTcpChannel.getProfile(), nioTcpChannel.getRemoteAddress()); - } else { - return true; - } - } - private class SecurityTcpChannelFactory extends TcpChannelFactory { private final String profileName; private final boolean isClient; + private final NioIPFilter ipFilter; private SecurityTcpChannelFactory(ProfileSettings profileSettings, boolean isClient) { super(new RawChannelFactory(profileSettings.tcpNoDelay, @@ -162,12 +153,12 @@ private SecurityTcpChannelFactory(ProfileSettings profileSettings, boolean isCli Math.toIntExact(profileSettings.receiveBufferSize.getBytes()))); this.profileName = profileSettings.profileName; this.isClient = isClient; + this.ipFilter = new NioIPFilter(authenticator, profileName); } @Override public NioTcpChannel createChannel(NioSelector selector, SocketChannel channel) throws IOException { NioTcpChannel nioChannel = new NioTcpChannel(profileName, channel); - SocketChannelContext context; Supplier pageSupplier = () -> { Recycler.V bytes = pageCacheRecycler.bytePage(false); return new InboundChannelBuffer.Page(ByteBuffer.wrap(bytes.v()), bytes::close); @@ -175,8 +166,8 @@ public NioTcpChannel createChannel(NioSelector selector, SocketChannel channel) TcpReadWriteHandler readWriteHandler = new TcpReadWriteHandler(nioChannel, SecurityNioTransport.this); InboundChannelBuffer buffer = new InboundChannelBuffer(pageSupplier); Consumer exceptionHandler = (e) -> onException(nioChannel, e); - Predicate filter = SecurityNioTransport.this::validateChannel; + SocketChannelContext context; if (sslEnabled) { SSLEngine sslEngine; SSLConfiguration defaultConfig = profileConfiguration.get(TcpTransport.DEFAULT_PROFILE); @@ -190,9 +181,9 @@ public NioTcpChannel createChannel(NioSelector selector, SocketChannel channel) sslEngine = sslService.createSSLEngine(sslConfig, null, -1); } SSLDriver sslDriver = new SSLDriver(sslEngine, isClient); - context = new SSLChannelContext(nioChannel, selector, exceptionHandler, sslDriver, readWriteHandler, buffer, filter); + context = new SSLChannelContext(nioChannel, selector, exceptionHandler, sslDriver, readWriteHandler, buffer, ipFilter); } else { - context = new BytesChannelContext(nioChannel, selector, exceptionHandler, readWriteHandler, buffer, filter); + context = new BytesChannelContext(nioChannel, selector, exceptionHandler, readWriteHandler, buffer, ipFilter); } nioChannel.setContext(context); From 3f0fee03e34dc726e7b3e333d942a4324d0ac943 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Thu, 12 Jul 2018 15:28:02 -0600 Subject: [PATCH 2/7] Extract exception handler --- .../xpack/security/Security.java | 11 +++- .../transport/HttpExceptionHandler.java | 64 +++++++++++++++++++ .../SecurityNetty4HttpServerTransport.java | 34 ++-------- .../nio/SecurityNioHttpServerTransport.java | 51 ++------------- 4 files changed, 83 insertions(+), 77 deletions(-) create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/HttpExceptionHandler.java diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 596acaeeac6a0..993caf0243a52 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -203,6 +203,7 @@ import org.elasticsearch.xpack.security.transport.netty4.SecurityNetty4HttpServerTransport; import org.elasticsearch.xpack.security.transport.netty4.SecurityNetty4ServerTransport; import org.elasticsearch.xpack.core.template.TemplateUtils; +import org.elasticsearch.xpack.security.transport.nio.SecurityNioHttpServerTransport; import org.elasticsearch.xpack.security.transport.nio.SecurityNioTransport; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; @@ -867,8 +868,14 @@ public Map> getHttpTransports(Settings set if (enabled == false) { // don't register anything if we are not enabled return Collections.emptyMap(); } - return Collections.singletonMap(SecurityField.NAME4, () -> new SecurityNetty4HttpServerTransport(settings, - networkService, bigArrays, ipFilter.get(), getSslService(), threadPool, xContentRegistry, dispatcher)); + + Map> httpTransports = new HashMap<>(); + httpTransports.put(SecurityField.NAME4, () -> new SecurityNetty4HttpServerTransport(settings, networkService, bigArrays, + ipFilter.get(), getSslService(), threadPool, xContentRegistry, dispatcher)); + httpTransports.put(SecurityField.NIO, () -> new SecurityNioHttpServerTransport(settings, networkService, bigArrays, + pageCacheRecycler, threadPool, xContentRegistry, dispatcher, ipFilter.get(), getSslService())); + + return httpTransports; } @Override diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/HttpExceptionHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/HttpExceptionHandler.java new file mode 100644 index 0000000000000..6a24a956f1900 --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/HttpExceptionHandler.java @@ -0,0 +1,64 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.security.transport; + +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; +import org.elasticsearch.common.component.Lifecycle; +import org.elasticsearch.common.network.CloseableChannel; +import org.elasticsearch.http.HttpChannel; + +import java.util.function.BiConsumer; + +import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isCloseDuringHandshakeException; +import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isNotSslRecordException; +import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isReceivedCertificateUnknownException; + +public class HttpExceptionHandler implements BiConsumer { + + private final Lifecycle lifecycle; + private final Logger logger; + private final BiConsumer fallback; + + public HttpExceptionHandler(Logger logger, Lifecycle lifecycle, BiConsumer fallback) { + this.lifecycle = lifecycle; + this.logger = logger; + this.fallback = fallback; + } + + public void accept(HttpChannel channel, Exception e) { + if (!lifecycle.started()) { + return; + } + + if (isNotSslRecordException(e)) { + if (logger.isTraceEnabled()) { + logger.trace(new ParameterizedMessage("received plaintext http traffic on a https channel, closing connection {}", + channel), e); + } else { + logger.warn("received plaintext http traffic on a https channel, closing connection {}", channel); + } + CloseableChannel.closeChannel(channel); + } else if (isCloseDuringHandshakeException(e)) { + if (logger.isTraceEnabled()) { + logger.trace(new ParameterizedMessage("connection {} closed during ssl handshake", channel), e); + } else { + logger.warn("connection {} closed during ssl handshake", channel); + } + CloseableChannel.closeChannel(channel); + } else if (isReceivedCertificateUnknownException(e)) { + if (logger.isTraceEnabled()) { + logger.trace(new ParameterizedMessage("http client did not trust server's certificate, closing connection {}", + channel), e); + } else { + logger.warn("http client did not trust this server's certificate, closing connection {}", channel); + } + CloseableChannel.closeChannel(channel); + } else { + fallback.accept(channel, e); + } + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransport.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransport.java index 4bc9430468781..4a74eeb0cd182 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransport.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransport.java @@ -19,6 +19,7 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.core.ssl.SSLConfiguration; import org.elasticsearch.xpack.core.ssl.SSLService; +import org.elasticsearch.xpack.security.transport.HttpExceptionHandler; import org.elasticsearch.xpack.security.transport.filter.IPFilter; import javax.net.ssl.SSLEngine; @@ -31,6 +32,7 @@ public class SecurityNetty4HttpServerTransport extends Netty4HttpServerTransport { + private final HttpExceptionHandler exceptionHandler; private final IPFilter ipFilter; private final Settings sslSettings; private final SSLService sslService; @@ -40,6 +42,7 @@ public SecurityNetty4HttpServerTransport(Settings settings, NetworkService netwo SSLService sslService, ThreadPool threadPool, NamedXContentRegistry xContentRegistry, Dispatcher dispatcher) { super(settings, networkService, bigArrays, threadPool, xContentRegistry, dispatcher); + this.exceptionHandler = new HttpExceptionHandler(logger, lifecycle, (c, e) -> super.onException(c, e)); this.ipFilter = ipFilter; final boolean ssl = HTTP_SSL_ENABLED.get(settings); this.sslSettings = SSLService.getHttpTransportSSLSettings(settings); @@ -57,36 +60,7 @@ public SecurityNetty4HttpServerTransport(Settings settings, NetworkService netwo @Override protected void onException(HttpChannel channel, Exception e) { - if (!lifecycle.started()) { - return; - } - - if (isNotSslRecordException(e)) { - if (logger.isTraceEnabled()) { - logger.trace(new ParameterizedMessage("received plaintext http traffic on a https channel, closing connection {}", - channel), e); - } else { - logger.warn("received plaintext http traffic on a https channel, closing connection {}", channel); - } - CloseableChannel.closeChannel(channel); - } else if (isCloseDuringHandshakeException(e)) { - if (logger.isTraceEnabled()) { - logger.trace(new ParameterizedMessage("connection {} closed during ssl handshake", channel), e); - } else { - logger.warn("connection {} closed during ssl handshake", channel); - } - CloseableChannel.closeChannel(channel); - } else if (isReceivedCertificateUnknownException(e)) { - if (logger.isTraceEnabled()) { - logger.trace(new ParameterizedMessage("http client did not trust server's certificate, closing connection {}", - channel), e); - } else { - logger.warn("http client did not trust this server's certificate, closing connection {}", channel); - } - CloseableChannel.closeChannel(channel); - } else { - super.onException(channel, e); - } + exceptionHandler.accept(channel, e); } @Override diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java index 27bc1961afa3f..baa0070587fe4 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java @@ -5,15 +5,12 @@ */ package org.elasticsearch.xpack.security.transport.nio; -import org.apache.logging.log4j.message.ParameterizedMessage; -import org.elasticsearch.common.network.CloseableChannel; import org.elasticsearch.common.network.NetworkService; import org.elasticsearch.common.recycler.Recycler; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.common.util.PageCacheRecycler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.http.HttpChannel; import org.elasticsearch.http.nio.HttpReadWriteHandler; import org.elasticsearch.http.nio.NioHttpChannel; import org.elasticsearch.http.nio.NioHttpServerChannel; @@ -28,6 +25,7 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.core.ssl.SSLConfiguration; import org.elasticsearch.xpack.core.ssl.SSLService; +import org.elasticsearch.xpack.security.transport.HttpExceptionHandler; import org.elasticsearch.xpack.security.transport.filter.IPFilter; import javax.net.ssl.SSLEngine; @@ -40,15 +38,12 @@ import java.util.function.Supplier; import static org.elasticsearch.xpack.core.XPackSettings.HTTP_SSL_ENABLED; -import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isCloseDuringHandshakeException; -import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isNotSslRecordException; -import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isReceivedCertificateUnknownException; public class SecurityNioHttpServerTransport extends NioHttpServerTransport { + private final HttpExceptionHandler exceptionHandler; private final IPFilter ipFilter; private final NioIPFilter nioIpFilter; - private final Settings sslSettings; private final SSLService sslService; private final SSLConfiguration sslConfiguration; private final boolean sslEnabled; @@ -58,13 +53,13 @@ public SecurityNioHttpServerTransport(Settings settings, NetworkService networkS NamedXContentRegistry xContentRegistry, Dispatcher dispatcher, IPFilter ipFilter, SSLService sslService) { super(settings, networkService, bigArrays, pageCacheRecycler, threadPool, xContentRegistry, dispatcher); + this.exceptionHandler = new HttpExceptionHandler(logger, lifecycle, (c, e) -> super.onException(c, e)); this.ipFilter = ipFilter; this.nioIpFilter = new NioIPFilter(ipFilter, IPFilter.HTTP_PROFILE_NAME); - sslEnabled = HTTP_SSL_ENABLED.get(settings); - this.sslSettings = SSLService.getHttpTransportSSLSettings(settings); + this.sslEnabled = HTTP_SSL_ENABLED.get(settings); this.sslService = sslService; if (sslEnabled) { - this.sslConfiguration = sslService.sslConfiguration(sslSettings, Settings.EMPTY); + this.sslConfiguration = sslService.sslConfiguration(SSLService.getHttpTransportSSLSettings(settings), Settings.EMPTY); if (sslService.isConfigurationValidForServerUsage(sslConfiguration) == false) { throw new IllegalArgumentException("a key must be provided to run as a server. the key should be configured using the " + "[xpack.security.http.ssl.key] or [xpack.security.http.ssl.keystore.path] setting"); @@ -74,40 +69,6 @@ public SecurityNioHttpServerTransport(Settings settings, NetworkService networkS } } - @Override - protected void onException(HttpChannel channel, Exception e) { - if (!lifecycle.started()) { - return; - } - - if (isNotSslRecordException(e)) { - if (logger.isTraceEnabled()) { - logger.trace(new ParameterizedMessage("received plaintext http traffic on a https channel, closing connection {}", - channel), e); - } else { - logger.warn("received plaintext http traffic on a https channel, closing connection {}", channel); - } - CloseableChannel.closeChannel(channel); - } else if (isCloseDuringHandshakeException(e)) { - if (logger.isTraceEnabled()) { - logger.trace(new ParameterizedMessage("connection {} closed during ssl handshake", channel), e); - } else { - logger.warn("connection {} closed during ssl handshake", channel); - } - CloseableChannel.closeChannel(channel); - } else if (isReceivedCertificateUnknownException(e)) { - if (logger.isTraceEnabled()) { - logger.trace(new ParameterizedMessage("http client did not trust server's certificate, closing connection {}", - channel), e); - } else { - logger.warn("http client did not trust this server's certificate, closing connection {}", channel); - } - CloseableChannel.closeChannel(channel); - } else { - super.onException(channel, e); - } - } - @Override protected void doStart() { super.doStart(); @@ -134,7 +95,7 @@ public NioHttpChannel createChannel(NioSelector selector, SocketChannel channel) HttpReadWriteHandler httpHandler = new HttpReadWriteHandler(httpChannel,SecurityNioHttpServerTransport.this, handlingSettings, corsConfig); InboundChannelBuffer buffer = new InboundChannelBuffer(pageSupplier); - Consumer exceptionHandler = (e) -> onException(httpChannel, e); + Consumer exceptionHandler = (e) -> SecurityNioHttpServerTransport.this.exceptionHandler.accept(httpChannel, e); SocketChannelContext context; if (sslEnabled) { From b8b84d7262d26a270bc21043899b472a9ebde7c4 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Thu, 12 Jul 2018 17:33:41 -0600 Subject: [PATCH 3/7] WIP --- .../transport/netty4/Netty4TcpChannel.java | 2 +- .../xpack/security/Security.java | 10 +- .../security/rest/SecurityRestFilter.java | 9 +- .../security/transport/SSLEngineUtils.java | 52 +++++++++++ ...java => SecurityHttpExceptionHandler.java} | 4 +- .../transport/SecurityHttpSettings.java | 22 +++++ .../transport/ServerTransportFilter.java | 19 ++-- .../SecurityNetty4HttpServerTransport.java | 20 +--- .../transport/nio/SSLChannelContext.java | 5 + .../security/transport/nio/SSLDriver.java | 4 + .../nio/SecurityNioHttpServerTransport.java | 10 +- .../test/SecurityIntegTestCase.java | 1 + .../test/SecuritySettingsSource.java | 1 + .../transport/SecurityHttpSettingsTests.java | 44 +++++++++ ...ecurityNetty4HttpServerTransportTests.java | 28 ------ .../transport/nio/NioIPFilterTests.java | 91 +++++++++++++++++++ 16 files changed, 254 insertions(+), 68 deletions(-) create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SSLEngineUtils.java rename x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/{HttpExceptionHandler.java => SecurityHttpExceptionHandler.java} (92%) create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityHttpSettings.java create mode 100644 x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/SecurityHttpSettingsTests.java create mode 100644 x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/NioIPFilterTests.java diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4TcpChannel.java b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4TcpChannel.java index 78a1425500072..51821c73329ca 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4TcpChannel.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4TcpChannel.java @@ -112,7 +112,7 @@ public void sendMessage(BytesReference reference, ActionListener listener) } } - public Channel getLowLevelChannel() { + public Channel getNettyChannel() { return channel; } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 993caf0243a52..3710e1b4162b7 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -198,6 +198,7 @@ import org.elasticsearch.xpack.security.rest.action.user.RestPutUserAction; import org.elasticsearch.xpack.security.rest.action.user.RestSetEnabledAction; import org.elasticsearch.xpack.security.support.SecurityIndexManager; +import org.elasticsearch.xpack.security.transport.SecurityHttpSettings; import org.elasticsearch.xpack.security.transport.SecurityServerTransportInterceptor; import org.elasticsearch.xpack.security.transport.filter.IPFilter; import org.elasticsearch.xpack.security.transport.netty4.SecurityNetty4HttpServerTransport; @@ -510,21 +511,22 @@ static Settings additionalSettings(final Settings settings, final boolean enable if (NetworkModule.HTTP_TYPE_SETTING.exists(settings)) { final String httpType = NetworkModule.HTTP_TYPE_SETTING.get(settings); - if (httpType.equals(SecurityField.NAME4)) { - SecurityNetty4HttpServerTransport.overrideSettings(builder, settings); + if (httpType.equals(SecurityField.NAME4) || httpType.equals(SecurityField.NIO)) { + SecurityHttpSettings.overrideSettings(builder, settings); } else { final String message = String.format( Locale.ROOT, - "http type setting [%s] must be [%s] but is [%s]", + "http type setting [%s] must be [%s] or [%s] but is [%s]", NetworkModule.HTTP_TYPE_KEY, SecurityField.NAME4, + SecurityField.NIO, httpType); throw new IllegalArgumentException(message); } } else { // default to security4 builder.put(NetworkModule.HTTP_TYPE_KEY, SecurityField.NAME4); - SecurityNetty4HttpServerTransport.overrideSettings(builder, settings); + SecurityHttpSettings.overrideSettings(builder, settings); } builder.put(SecuritySettings.addUserSettings(settings)); return builder.build(); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java index 9109bb37e8c41..0061338d27c41 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java @@ -24,8 +24,10 @@ import org.elasticsearch.rest.RestRequest.Method; import org.elasticsearch.xpack.core.security.rest.RestRequestFilter; import org.elasticsearch.xpack.security.authc.AuthenticationService; +import org.elasticsearch.xpack.security.transport.SSLEngineUtils; import org.elasticsearch.xpack.security.transport.ServerTransportFilter; +import javax.net.ssl.SSLEngine; import java.io.IOException; public class SecurityRestFilter implements RestHandler { @@ -53,10 +55,9 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c // CORS - allow for preflight unauthenticated OPTIONS request if (extractClientCertificate) { HttpChannel httpChannel = request.getHttpChannel(); - Channel nettyChannel = ((Netty4HttpChannel) httpChannel).getNettyChannel(); - SslHandler handler = nettyChannel.pipeline().get(SslHandler.class); - assert handler != null; - ServerTransportFilter.extractClientCertificates(logger, threadContext, handler.engine(), nettyChannel); + SSLEngine sslEngine = SSLEngineUtils.getSSLEngine(httpChannel); + + ServerTransportFilter.extractClientCertificates(logger, threadContext, sslEngine, httpChannel); } service.authenticate(maybeWrapRestRequest(request), ActionListener.wrap( authentication -> { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SSLEngineUtils.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SSLEngineUtils.java new file mode 100644 index 0000000000000..559f8d814baa7 --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SSLEngineUtils.java @@ -0,0 +1,52 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.security.transport; + +import io.netty.channel.Channel; +import io.netty.handler.ssl.SslHandler; +import org.elasticsearch.http.HttpChannel; +import org.elasticsearch.http.netty4.Netty4HttpChannel; +import org.elasticsearch.http.nio.NioHttpChannel; +import org.elasticsearch.nio.SocketChannelContext; +import org.elasticsearch.transport.TcpChannel; +import org.elasticsearch.transport.netty4.Netty4TcpChannel; +import org.elasticsearch.transport.nio.NioTcpChannel; +import org.elasticsearch.xpack.security.transport.nio.SSLChannelContext; + +import javax.net.ssl.SSLEngine; + +public class SSLEngineUtils { + + public static SSLEngine getSSLEngine(HttpChannel httpChannel) { + if (httpChannel instanceof Netty4HttpChannel) { + Channel nettyChannel = ((Netty4HttpChannel) httpChannel).getNettyChannel(); + SslHandler handler = nettyChannel.pipeline().get(SslHandler.class); + assert handler != null : "Must have SslHandler"; + return handler.engine(); + } else if (httpChannel instanceof NioHttpChannel) { + SocketChannelContext context = ((NioHttpChannel) httpChannel).getContext(); + assert context instanceof SSLChannelContext : "Must be SSLChannelContext.class, found: " + context.getClass(); + return ((SSLChannelContext) context).getSSLEngine(); + } else { + throw new AssertionError("Unknown channel class type: " + httpChannel.getClass()); + } + } + + public static SSLEngine getSSLEngine(TcpChannel tcpChannel) { + if (tcpChannel instanceof Netty4TcpChannel) { + Channel nettyChannel = ((Netty4TcpChannel) tcpChannel).getNettyChannel(); + SslHandler handler = nettyChannel.pipeline().get(SslHandler.class); + assert handler != null : "Must have SslHandler"; + return handler.engine(); + } else if (tcpChannel instanceof NioTcpChannel) { + SocketChannelContext context = ((NioTcpChannel) tcpChannel).getContext(); + assert context instanceof SSLChannelContext : "Must be SSLChannelContext.class, found: " + context.getClass(); + return ((SSLChannelContext) context).getSSLEngine(); + } else { + throw new AssertionError("Unknown channel class type: " + tcpChannel.getClass()); + } + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/HttpExceptionHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityHttpExceptionHandler.java similarity index 92% rename from x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/HttpExceptionHandler.java rename to x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityHttpExceptionHandler.java index 6a24a956f1900..310b78378e376 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/HttpExceptionHandler.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityHttpExceptionHandler.java @@ -17,13 +17,13 @@ import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isNotSslRecordException; import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isReceivedCertificateUnknownException; -public class HttpExceptionHandler implements BiConsumer { +public class SecurityHttpExceptionHandler implements BiConsumer { private final Lifecycle lifecycle; private final Logger logger; private final BiConsumer fallback; - public HttpExceptionHandler(Logger logger, Lifecycle lifecycle, BiConsumer fallback) { + public SecurityHttpExceptionHandler(Logger logger, Lifecycle lifecycle, BiConsumer fallback) { this.lifecycle = lifecycle; this.logger = logger; this.fallback = fallback; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityHttpSettings.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityHttpSettings.java new file mode 100644 index 0000000000000..f8079535acf99 --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityHttpSettings.java @@ -0,0 +1,22 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.security.transport; + +import org.elasticsearch.common.settings.Settings; + +import static org.elasticsearch.http.HttpTransportSettings.SETTING_HTTP_COMPRESSION; +import static org.elasticsearch.xpack.core.XPackSettings.HTTP_SSL_ENABLED; + +public final class SecurityHttpSettings { + + private SecurityHttpSettings() {} + + public static void overrideSettings(Settings.Builder settingsBuilder, Settings settings) { + if (HTTP_SSL_ENABLED.get(settings) && SETTING_HTTP_COMPRESSION.exists(settings) == false) { + settingsBuilder.put(SETTING_HTTP_COMPRESSION.getKey(), false); + } + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java index 9427812ba1349..e97fa34c02dea 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java @@ -20,11 +20,13 @@ import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.transport.TaskTransportChannel; +import org.elasticsearch.transport.TcpChannel; import org.elasticsearch.transport.TcpTransportChannel; import org.elasticsearch.transport.TransportChannel; import org.elasticsearch.transport.TransportRequest; import org.elasticsearch.transport.TransportService; import org.elasticsearch.transport.netty4.Netty4TcpChannel; +import org.elasticsearch.transport.nio.NioTcpChannel; import org.elasticsearch.xpack.core.security.SecurityContext; import org.elasticsearch.xpack.core.security.authc.Authentication; import org.elasticsearch.xpack.core.security.user.KibanaUser; @@ -115,13 +117,13 @@ requests from all the nodes are attached with a user (either a serialize unwrappedChannel = ((TaskTransportChannel) unwrappedChannel).getChannel(); } - if (extractClientCert && (unwrappedChannel instanceof TcpTransportChannel) && - ((TcpTransportChannel) unwrappedChannel).getChannel() instanceof Netty4TcpChannel) { - Channel channel = ((Netty4TcpChannel) ((TcpTransportChannel) unwrappedChannel).getChannel()).getLowLevelChannel(); - SslHandler sslHandler = channel.pipeline().get(SslHandler.class); - if (channel.isOpen()) { - assert sslHandler != null : "channel [" + channel + "] did not have a ssl handler. pipeline " + channel.pipeline(); - extractClientCertificates(logger, threadContext, sslHandler.engine(), channel); + if (extractClientCert && (unwrappedChannel instanceof TcpTransportChannel)) { + TcpChannel tcpChannel = ((TcpTransportChannel) unwrappedChannel).getChannel(); + if (tcpChannel instanceof Netty4TcpChannel || tcpChannel instanceof NioTcpChannel) { + SSLEngine sslEngine = SSLEngineUtils.getSSLEngine(tcpChannel); + if (tcpChannel.isOpen()) { + extractClientCertificates(logger, threadContext, sslEngine, tcpChannel); + } } } @@ -172,7 +174,8 @@ private void executeAsCurrentVersionKibanaUser(String securityAction, TransportR } } - static void extractClientCertificates(Logger logger, ThreadContext threadContext, SSLEngine sslEngine, Channel channel) { + // Channel is only used for logging, which is why it is okay to just be of type Object + static void extractClientCertificates(Logger logger, ThreadContext threadContext, SSLEngine sslEngine, Object channel) { try { Certificate[] certs = sslEngine.getSession().getPeerCertificates(); if (certs instanceof X509Certificate[]) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransport.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransport.java index 4a74eeb0cd182..e94c9652ded1e 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransport.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransport.java @@ -8,8 +8,6 @@ import io.netty.channel.Channel; import io.netty.channel.ChannelHandler; import io.netty.handler.ssl.SslHandler; -import org.apache.logging.log4j.message.ParameterizedMessage; -import org.elasticsearch.common.network.CloseableChannel; import org.elasticsearch.common.network.NetworkService; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.BigArrays; @@ -19,20 +17,16 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.core.ssl.SSLConfiguration; import org.elasticsearch.xpack.core.ssl.SSLService; -import org.elasticsearch.xpack.security.transport.HttpExceptionHandler; +import org.elasticsearch.xpack.security.transport.SecurityHttpExceptionHandler; import org.elasticsearch.xpack.security.transport.filter.IPFilter; import javax.net.ssl.SSLEngine; -import static org.elasticsearch.http.HttpTransportSettings.SETTING_HTTP_COMPRESSION; import static org.elasticsearch.xpack.core.XPackSettings.HTTP_SSL_ENABLED; -import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isCloseDuringHandshakeException; -import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isNotSslRecordException; -import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isReceivedCertificateUnknownException; public class SecurityNetty4HttpServerTransport extends Netty4HttpServerTransport { - private final HttpExceptionHandler exceptionHandler; + private final SecurityHttpExceptionHandler securityExceptionHandler; private final IPFilter ipFilter; private final Settings sslSettings; private final SSLService sslService; @@ -42,7 +36,7 @@ public SecurityNetty4HttpServerTransport(Settings settings, NetworkService netwo SSLService sslService, ThreadPool threadPool, NamedXContentRegistry xContentRegistry, Dispatcher dispatcher) { super(settings, networkService, bigArrays, threadPool, xContentRegistry, dispatcher); - this.exceptionHandler = new HttpExceptionHandler(logger, lifecycle, (c, e) -> super.onException(c, e)); + this.securityExceptionHandler = new SecurityHttpExceptionHandler(logger, lifecycle, (c, e) -> super.onException(c, e)); this.ipFilter = ipFilter; final boolean ssl = HTTP_SSL_ENABLED.get(settings); this.sslSettings = SSLService.getHttpTransportSSLSettings(settings); @@ -60,7 +54,7 @@ public SecurityNetty4HttpServerTransport(Settings settings, NetworkService netwo @Override protected void onException(HttpChannel channel, Exception e) { - exceptionHandler.accept(channel, e); + securityExceptionHandler.accept(channel, e); } @Override @@ -90,10 +84,4 @@ protected void initChannel(Channel ch) throws Exception { ch.pipeline().addFirst("ip_filter", new IpFilterRemoteAddressFilter(ipFilter, IPFilter.HTTP_PROFILE_NAME)); } } - - public static void overrideSettings(Settings.Builder settingsBuilder, Settings settings) { - if (HTTP_SSL_ENABLED.get(settings) && SETTING_HTTP_COMPRESSION.exists(settings) == false) { - settingsBuilder.put(SETTING_HTTP_COMPRESSION.getKey(), false); - } - } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SSLChannelContext.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SSLChannelContext.java index da348ea1f78e1..c83bd16ca95e1 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SSLChannelContext.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SSLChannelContext.java @@ -14,6 +14,7 @@ import org.elasticsearch.nio.NioSelector; import org.elasticsearch.nio.WriteOperation; +import javax.net.ssl.SSLEngine; import java.io.IOException; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -164,6 +165,10 @@ public void closeFromSelector() throws IOException { } } + public SSLEngine getSSLEngine() { + return sslDriver.getSSLEngine(); + } + private static class CloseNotifyOperation implements WriteOperation { private static final BiConsumer LISTENER = (v, t) -> {}; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SSLDriver.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SSLDriver.java index 4080574713cce..382230684c77f 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SSLDriver.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SSLDriver.java @@ -96,6 +96,10 @@ public void renegotiate() throws SSLException { } } + public SSLEngine getSSLEngine() { + return engine; + } + public boolean hasFlushPending() { return networkWriteBuffer.hasRemaining(); } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java index baa0070587fe4..761a2bf1dc489 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java @@ -25,7 +25,7 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.core.ssl.SSLConfiguration; import org.elasticsearch.xpack.core.ssl.SSLService; -import org.elasticsearch.xpack.security.transport.HttpExceptionHandler; +import org.elasticsearch.xpack.security.transport.SecurityHttpExceptionHandler; import org.elasticsearch.xpack.security.transport.filter.IPFilter; import javax.net.ssl.SSLEngine; @@ -41,7 +41,7 @@ public class SecurityNioHttpServerTransport extends NioHttpServerTransport { - private final HttpExceptionHandler exceptionHandler; + private final SecurityHttpExceptionHandler securityExceptionHandler; private final IPFilter ipFilter; private final NioIPFilter nioIpFilter; private final SSLService sslService; @@ -53,7 +53,7 @@ public SecurityNioHttpServerTransport(Settings settings, NetworkService networkS NamedXContentRegistry xContentRegistry, Dispatcher dispatcher, IPFilter ipFilter, SSLService sslService) { super(settings, networkService, bigArrays, pageCacheRecycler, threadPool, xContentRegistry, dispatcher); - this.exceptionHandler = new HttpExceptionHandler(logger, lifecycle, (c, e) -> super.onException(c, e)); + this.securityExceptionHandler = new SecurityHttpExceptionHandler(logger, lifecycle, (c, e) -> super.onException(c, e)); this.ipFilter = ipFilter; this.nioIpFilter = new NioIPFilter(ipFilter, IPFilter.HTTP_PROFILE_NAME); this.sslEnabled = HTTP_SSL_ENABLED.get(settings); @@ -95,7 +95,7 @@ public NioHttpChannel createChannel(NioSelector selector, SocketChannel channel) HttpReadWriteHandler httpHandler = new HttpReadWriteHandler(httpChannel,SecurityNioHttpServerTransport.this, handlingSettings, corsConfig); InboundChannelBuffer buffer = new InboundChannelBuffer(pageSupplier); - Consumer exceptionHandler = (e) -> SecurityNioHttpServerTransport.this.exceptionHandler.accept(httpChannel, e); + Consumer exceptionHandler = (e) -> securityExceptionHandler.accept(httpChannel, e); SocketChannelContext context; if (sslEnabled) { @@ -119,7 +119,7 @@ public NioHttpChannel createChannel(NioSelector selector, SocketChannel channel) } @Override - public NioHttpServerChannel createServerChannel(NioSelector selector, ServerSocketChannel channel) throws IOException { + public NioHttpServerChannel createServerChannel(NioSelector selector, ServerSocketChannel channel) { NioHttpServerChannel httpServerChannel = new NioHttpServerChannel(channel); Consumer exceptionHandler = (e) -> onServerException(httpServerChannel, e); Consumer acceptor = SecurityNioHttpServerTransport.this::acceptChannel; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java index e6db3407496eb..9bb0e44eb664c 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java @@ -244,6 +244,7 @@ protected Settings nodeSettings(int nodeOrdinal) { builder.put(customSettings, false); // handle secure settings separately builder.put(LicenseService.SELF_GENERATED_LICENSE_TYPE.getKey(), "trial"); builder.put(NetworkModule.TRANSPORT_TYPE_KEY, randomBoolean() ? SecurityField.NAME4 : SecurityField.NIO); + builder.put(NetworkModule.HTTP_TYPE_KEY, randomBoolean() ? SecurityField.NAME4 : SecurityField.NIO); Settings.Builder customBuilder = Settings.builder().put(customSettings); if (customBuilder.getSecureSettings() != null) { SecuritySettingsSource.addSecureSettings(builder, secureSettings -> diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java index 2e0662264a248..df1456c3790b2 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java @@ -126,6 +126,7 @@ public Settings nodeSettings(int nodeOrdinal) { Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)) .put(XPackSettings.SECURITY_ENABLED.getKey(), true) .put(NetworkModule.TRANSPORT_TYPE_KEY, randomBoolean() ? SecurityField.NAME4 : SecurityField.NIO) + .put(NetworkModule.HTTP_TYPE_KEY, randomBoolean() ? SecurityField.NAME4 : SecurityField.NIO) //TODO: for now isolate security tests from watcher & monitoring (randomize this later) .put(XPackSettings.WATCHER_ENABLED.getKey(), false) .put(XPackSettings.MONITORING_ENABLED.getKey(), false) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/SecurityHttpSettingsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/SecurityHttpSettingsTests.java new file mode 100644 index 0000000000000..56c79a4c12791 --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/SecurityHttpSettingsTests.java @@ -0,0 +1,44 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.security.transport; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.http.HttpTransportSettings; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.core.XPackSettings; + +import static org.hamcrest.Matchers.is; + +public class SecurityHttpSettingsTests extends ESTestCase { + + public void testDisablesCompressionByDefaultForSsl() { + Settings settings = Settings.builder() + .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), true).build(); + + Settings.Builder pluginSettingsBuilder = Settings.builder(); + SecurityHttpSettings.overrideSettings(pluginSettingsBuilder, settings); + assertThat(HttpTransportSettings.SETTING_HTTP_COMPRESSION.get(pluginSettingsBuilder.build()), is(false)); + } + + public void testLeavesCompressionOnIfNotSsl() { + Settings settings = Settings.builder() + .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), false).build(); + Settings.Builder pluginSettingsBuilder = Settings.builder(); + SecurityHttpSettings.overrideSettings(pluginSettingsBuilder, settings); + assertThat(pluginSettingsBuilder.build().isEmpty(), is(true)); + } + + public void testDoesNotChangeExplicitlySetCompression() { + Settings settings = Settings.builder() + .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), true) + .put(HttpTransportSettings.SETTING_HTTP_COMPRESSION.getKey(), true) + .build(); + + Settings.Builder pluginSettingsBuilder = Settings.builder(); + SecurityHttpSettings.overrideSettings(pluginSettingsBuilder, settings); + assertThat(pluginSettingsBuilder.build().isEmpty(), is(true)); + } +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java index ec925f43abe79..e0d1e5a282bc7 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java @@ -144,34 +144,6 @@ public void testCustomSSLConfiguration() throws Exception { assertThat(customEngine.getEnabledProtocols(), not(equalTo(defaultEngine.getEnabledProtocols()))); } - public void testDisablesCompressionByDefaultForSsl() throws Exception { - Settings settings = Settings.builder() - .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), true).build(); - - Settings.Builder pluginSettingsBuilder = Settings.builder(); - SecurityNetty4HttpServerTransport.overrideSettings(pluginSettingsBuilder, settings); - assertThat(HttpTransportSettings.SETTING_HTTP_COMPRESSION.get(pluginSettingsBuilder.build()), is(false)); - } - - public void testLeavesCompressionOnIfNotSsl() throws Exception { - Settings settings = Settings.builder() - .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), false).build(); - Settings.Builder pluginSettingsBuilder = Settings.builder(); - SecurityNetty4HttpServerTransport.overrideSettings(pluginSettingsBuilder, settings); - assertThat(pluginSettingsBuilder.build().isEmpty(), is(true)); - } - - public void testDoesNotChangeExplicitlySetCompression() throws Exception { - Settings settings = Settings.builder() - .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), true) - .put(HttpTransportSettings.SETTING_HTTP_COMPRESSION.getKey(), true) - .build(); - - Settings.Builder pluginSettingsBuilder = Settings.builder(); - SecurityNetty4HttpServerTransport.overrideSettings(pluginSettingsBuilder, settings); - assertThat(pluginSettingsBuilder.build().isEmpty(), is(true)); - } - public void testThatExceptionIsThrownWhenConfiguredWithoutSslKey() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.ssl.truststore.secure_password", "testnode"); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/NioIPFilterTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/NioIPFilterTests.java new file mode 100644 index 0000000000000..1832669fce144 --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/NioIPFilterTests.java @@ -0,0 +1,91 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.security.transport.nio; + +import org.elasticsearch.common.component.Lifecycle; +import org.elasticsearch.common.network.InetAddresses; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.transport.BoundTransportAddress; +import org.elasticsearch.common.transport.TransportAddress; +import org.elasticsearch.http.HttpServerTransport; +import org.elasticsearch.license.XPackLicenseState; +import org.elasticsearch.nio.NioSocketChannel; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.transport.Transport; +import org.elasticsearch.xpack.security.audit.AuditTrailService; +import org.elasticsearch.xpack.security.transport.filter.IPFilter; +import org.junit.Before; + +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; + +import static org.hamcrest.Matchers.is; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class NioIPFilterTests extends ESTestCase { + + private NioIPFilter nioIPFilter; + + @Before + public void init() throws Exception { + Settings settings = Settings.builder() + .put("xpack.security.transport.filter.allow", "127.0.0.1") + .put("xpack.security.transport.filter.deny", "10.0.0.0/8") + .build(); + + boolean isHttpEnabled = randomBoolean(); + + Transport transport = mock(Transport.class); + TransportAddress address = new TransportAddress(InetAddress.getLoopbackAddress(), 9300); + when(transport.boundAddress()).thenReturn(new BoundTransportAddress(new TransportAddress[] { address }, address)); + when(transport.lifecycleState()).thenReturn(Lifecycle.State.STARTED); + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList( + IPFilter.HTTP_FILTER_ALLOW_SETTING, + IPFilter.HTTP_FILTER_DENY_SETTING, + IPFilter.IP_FILTER_ENABLED_HTTP_SETTING, + IPFilter.IP_FILTER_ENABLED_SETTING, + IPFilter.TRANSPORT_FILTER_ALLOW_SETTING, + IPFilter.TRANSPORT_FILTER_DENY_SETTING, + IPFilter.PROFILE_FILTER_ALLOW_SETTING, + IPFilter.PROFILE_FILTER_DENY_SETTING))); + XPackLicenseState licenseState = mock(XPackLicenseState.class); + when(licenseState.isIpFilteringAllowed()).thenReturn(true); + when(licenseState.isSecurityEnabled()).thenReturn(true); + AuditTrailService auditTrailService = new AuditTrailService(settings, Collections.emptyList(), licenseState); + IPFilter ipFilter = new IPFilter(settings, auditTrailService, clusterSettings, licenseState); + ipFilter.setBoundTransportAddress(transport.boundAddress(), transport.profileBoundAddresses()); + if (isHttpEnabled) { + HttpServerTransport httpTransport = mock(HttpServerTransport.class); + TransportAddress httpAddress = new TransportAddress(InetAddress.getLoopbackAddress(), 9200); + when(httpTransport.boundAddress()).thenReturn(new BoundTransportAddress(new TransportAddress[] { httpAddress }, httpAddress)); + when(httpTransport.lifecycleState()).thenReturn(Lifecycle.State.STARTED); + ipFilter.setBoundHttpTransportAddress(httpTransport.boundAddress()); + } + + if (isHttpEnabled) { + nioIPFilter = new NioIPFilter(ipFilter, IPFilter.HTTP_PROFILE_NAME); + } else { + nioIPFilter = new NioIPFilter(ipFilter, "default"); + } + } + + public void testThatFilteringWorksByIp() throws Exception { + InetSocketAddress localhostAddr = new InetSocketAddress(InetAddresses.forString("127.0.0.1"), 12345); + NioSocketChannel channel1 = mock(NioSocketChannel.class); + when(channel1.getRemoteAddress()).thenReturn(localhostAddr); + assertThat(nioIPFilter.test(channel1), is(true)); + + InetSocketAddress remoteAddr = new InetSocketAddress(InetAddresses.forString("10.0.0.8"), 12345); + NioSocketChannel channel2 = mock(NioSocketChannel.class); + when(channel2.getRemoteAddress()).thenReturn(remoteAddr); + assertThat(nioIPFilter.test(channel2), is(false)); + } +} From 9d5887ae40495e261a2ea7ba7734c5efb4e90b58 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Thu, 12 Jul 2018 17:59:55 -0600 Subject: [PATCH 4/7] Enable tests --- .../security/rest/SecurityRestFilter.java | 3 - .../transport/ServerTransportFilter.java | 3 - .../nio/SecurityNioHttpServerTransport.java | 2 +- .../SecurityNioHttpServerTransportTests.java | 205 ++++++++++++++++++ 4 files changed, 206 insertions(+), 7 deletions(-) create mode 100644 x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransportTests.java diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java index 0061338d27c41..d204586ed1ea4 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java @@ -5,8 +5,6 @@ */ package org.elasticsearch.xpack.security.rest; -import io.netty.channel.Channel; -import io.netty.handler.ssl.SslHandler; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; @@ -15,7 +13,6 @@ import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.http.HttpChannel; -import org.elasticsearch.http.netty4.Netty4HttpChannel; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.rest.BytesRestResponse; import org.elasticsearch.rest.RestChannel; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java index e97fa34c02dea..118bd4ea44db7 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java @@ -5,8 +5,6 @@ */ package org.elasticsearch.xpack.security.transport; -import io.netty.channel.Channel; -import io.netty.handler.ssl.SslHandler; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; @@ -40,7 +38,6 @@ import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLPeerUnverifiedException; - import java.io.IOException; import java.security.cert.Certificate; import java.security.cert.X509Certificate; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java index 761a2bf1dc489..006c78b4ae0de 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java @@ -79,7 +79,7 @@ protected SecurityHttpChannelFactory channelFactory() { return new SecurityHttpChannelFactory(); } - private class SecurityHttpChannelFactory extends ChannelFactory { + class SecurityHttpChannelFactory extends ChannelFactory { private SecurityHttpChannelFactory() { super(new RawChannelFactory(tcpNoDelay, tcpKeepAlive, reuseAddress, tcpSendBufferSize, tcpReceiveBufferSize)); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransportTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransportTests.java new file mode 100644 index 0000000000000..e782d8032a538 --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransportTests.java @@ -0,0 +1,205 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.security.transport.nio; + +import org.elasticsearch.common.network.NetworkService; +import org.elasticsearch.common.settings.MockSecureSettings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.BigArrays; +import org.elasticsearch.common.util.PageCacheRecycler; +import org.elasticsearch.env.Environment; +import org.elasticsearch.env.TestEnvironment; +import org.elasticsearch.http.NullDispatcher; +import org.elasticsearch.http.nio.NioHttpChannel; +import org.elasticsearch.nio.NioSelector; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.core.XPackSettings; +import org.elasticsearch.xpack.core.ssl.SSLClientAuth; +import org.elasticsearch.xpack.core.ssl.SSLService; +import org.elasticsearch.xpack.security.transport.SSLEngineUtils; +import org.elasticsearch.xpack.security.transport.filter.IPFilter; +import org.junit.Before; + +import javax.net.ssl.SSLEngine; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.nio.channels.SocketChannel; +import java.nio.file.Path; +import java.util.Collections; +import java.util.Locale; + +import static org.hamcrest.Matchers.arrayContaining; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class SecurityNioHttpServerTransportTests extends ESTestCase { + + private SSLService sslService; + private Environment env; + + @Before + public void createSSLService() { + Path testNodeStore = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks"); + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString("xpack.ssl.keystore.secure_password", "testnode"); + Settings settings = Settings.builder() + .put("xpack.ssl.keystore.path", testNodeStore) + .put("path.home", createTempDir()) + .setSecureSettings(secureSettings) + .build(); + env = TestEnvironment.newEnvironment(settings); + sslService = new SSLService(settings, env); + } + + public void testDefaultClientAuth() throws IOException { + Settings settings = Settings.builder() + .put(env.settings()) + .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), true).build(); + sslService = new SSLService(settings, env); + SecurityNioHttpServerTransport transport = new SecurityNioHttpServerTransport(settings, + new NetworkService(Collections.emptyList()), mock(BigArrays.class), mock(PageCacheRecycler.class), mock(ThreadPool.class), + xContentRegistry(), new NullDispatcher(), mock(IPFilter.class), sslService); + SecurityNioHttpServerTransport.SecurityHttpChannelFactory factory = transport.channelFactory(); + SocketChannel socketChannel = mock(SocketChannel.class); + when(socketChannel.getRemoteAddress()).thenReturn(new InetSocketAddress("localhost", 0)); + NioHttpChannel channel = factory.createChannel(mock(NioSelector.class), socketChannel); + SSLEngine engine = SSLEngineUtils.getSSLEngine(channel); + + assertThat(engine.getNeedClientAuth(), is(false)); + assertThat(engine.getWantClientAuth(), is(false)); + } + + public void testOptionalClientAuth() throws IOException { + String value = randomFrom(SSLClientAuth.OPTIONAL.name(), SSLClientAuth.OPTIONAL.name().toLowerCase(Locale.ROOT)); + Settings settings = Settings.builder() + .put(env.settings()) + .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), true) + .put("xpack.security.http.ssl.client_authentication", value).build(); + sslService = new SSLService(settings, env); + SecurityNioHttpServerTransport transport = new SecurityNioHttpServerTransport(settings, + new NetworkService(Collections.emptyList()), mock(BigArrays.class), mock(PageCacheRecycler.class), mock(ThreadPool.class), + xContentRegistry(), new NullDispatcher(), mock(IPFilter.class), sslService); + + SecurityNioHttpServerTransport.SecurityHttpChannelFactory factory = transport.channelFactory(); + SocketChannel socketChannel = mock(SocketChannel.class); + when(socketChannel.getRemoteAddress()).thenReturn(new InetSocketAddress("localhost", 0)); + NioHttpChannel channel = factory.createChannel(mock(NioSelector.class), socketChannel); + SSLEngine engine = SSLEngineUtils.getSSLEngine(channel); + assertThat(engine.getNeedClientAuth(), is(false)); + assertThat(engine.getWantClientAuth(), is(true)); + } + + public void testRequiredClientAuth() throws IOException { + String value = randomFrom(SSLClientAuth.REQUIRED.name(), SSLClientAuth.REQUIRED.name().toLowerCase(Locale.ROOT)); + Settings settings = Settings.builder() + .put(env.settings()) + .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), true) + .put("xpack.security.http.ssl.client_authentication", value).build(); + sslService = new SSLService(settings, env); + SecurityNioHttpServerTransport transport = new SecurityNioHttpServerTransport(settings, + new NetworkService(Collections.emptyList()), mock(BigArrays.class), mock(PageCacheRecycler.class), mock(ThreadPool.class), + xContentRegistry(), new NullDispatcher(), mock(IPFilter.class), sslService); + + SecurityNioHttpServerTransport.SecurityHttpChannelFactory factory = transport.channelFactory(); + SocketChannel socketChannel = mock(SocketChannel.class); + when(socketChannel.getRemoteAddress()).thenReturn(new InetSocketAddress("localhost", 0)); + NioHttpChannel channel = factory.createChannel(mock(NioSelector.class), socketChannel); + SSLEngine engine = SSLEngineUtils.getSSLEngine(channel); + assertThat(engine.getNeedClientAuth(), is(true)); + assertThat(engine.getWantClientAuth(), is(false)); + } + + public void testNoClientAuth() throws IOException { + String value = randomFrom(SSLClientAuth.NONE.name(), SSLClientAuth.NONE.name().toLowerCase(Locale.ROOT)); + Settings settings = Settings.builder() + .put(env.settings()) + .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), true) + .put("xpack.security.http.ssl.client_authentication", value).build(); + sslService = new SSLService(settings, env); + SecurityNioHttpServerTransport transport = new SecurityNioHttpServerTransport(settings, + new NetworkService(Collections.emptyList()), mock(BigArrays.class), mock(PageCacheRecycler.class), mock(ThreadPool.class), + xContentRegistry(), new NullDispatcher(), mock(IPFilter.class), sslService); + + SecurityNioHttpServerTransport.SecurityHttpChannelFactory factory = transport.channelFactory(); + SocketChannel socketChannel = mock(SocketChannel.class); + when(socketChannel.getRemoteAddress()).thenReturn(new InetSocketAddress("localhost", 0)); + NioHttpChannel channel = factory.createChannel(mock(NioSelector.class), socketChannel); + SSLEngine engine = SSLEngineUtils.getSSLEngine(channel); + assertThat(engine.getNeedClientAuth(), is(false)); + assertThat(engine.getWantClientAuth(), is(false)); + } + + public void testCustomSSLConfiguration() throws IOException { + Settings settings = Settings.builder() + .put(env.settings()) + .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), true).build(); + sslService = new SSLService(settings, env); + SecurityNioHttpServerTransport transport = new SecurityNioHttpServerTransport(settings, + new NetworkService(Collections.emptyList()), mock(BigArrays.class), mock(PageCacheRecycler.class), mock(ThreadPool.class), + xContentRegistry(), new NullDispatcher(), mock(IPFilter.class), sslService); + SecurityNioHttpServerTransport.SecurityHttpChannelFactory factory = transport.channelFactory(); + SocketChannel socketChannel = mock(SocketChannel.class); + when(socketChannel.getRemoteAddress()).thenReturn(new InetSocketAddress("localhost", 0)); + NioHttpChannel channel = factory.createChannel(mock(NioSelector.class), socketChannel); + SSLEngine defaultEngine = SSLEngineUtils.getSSLEngine(channel); + + settings = Settings.builder() + .put(env.settings()) + .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), true) + .put("xpack.security.http.ssl.supported_protocols", "TLSv1.2") + .build(); + sslService = new SSLService(settings, TestEnvironment.newEnvironment(settings)); + transport = new SecurityNioHttpServerTransport(settings, + new NetworkService(Collections.emptyList()), mock(BigArrays.class), mock(PageCacheRecycler.class), mock(ThreadPool.class), + xContentRegistry(), new NullDispatcher(), mock(IPFilter.class), sslService); + factory = transport.channelFactory(); + channel = factory.createChannel(mock(NioSelector.class), socketChannel); + SSLEngine customEngine = SSLEngineUtils.getSSLEngine(channel); + assertThat(customEngine.getEnabledProtocols(), arrayContaining("TLSv1.2")); + assertThat(customEngine.getEnabledProtocols(), not(equalTo(defaultEngine.getEnabledProtocols()))); + } + + public void testThatExceptionIsThrownWhenConfiguredWithoutSslKey() { + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString("xpack.ssl.truststore.secure_password", "testnode"); + Settings settings = Settings.builder() + .put("xpack.ssl.truststore.path", + getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks")) + .setSecureSettings(secureSettings) + .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), true) + .put("path.home", createTempDir()) + .build(); + env = TestEnvironment.newEnvironment(settings); + sslService = new SSLService(settings, env); + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> new SecurityNioHttpServerTransport(settings, + new NetworkService(Collections.emptyList()), mock(BigArrays.class), mock(PageCacheRecycler.class), mock(ThreadPool.class), + xContentRegistry(), new NullDispatcher(), mock(IPFilter.class), sslService)); + assertThat(e.getMessage(), containsString("key must be provided")); + } + + public void testNoExceptionWhenConfiguredWithoutSslKeySSLDisabled() { + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString("xpack.ssl.truststore.secure_password", "testnode"); + Settings settings = Settings.builder() + .put("xpack.ssl.truststore.path", + getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks")) + .setSecureSettings(secureSettings) + .put("path.home", createTempDir()) + .build(); + env = TestEnvironment.newEnvironment(settings); + sslService = new SSLService(settings, env); + SecurityNioHttpServerTransport transport = new SecurityNioHttpServerTransport(settings, + new NetworkService(Collections.emptyList()), mock(BigArrays.class), mock(PageCacheRecycler.class), mock(ThreadPool.class), + xContentRegistry(), new NullDispatcher(), mock(IPFilter.class), sslService); + } +} From 9977af6841a110fa1850d052962805daeb1e7564 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Thu, 12 Jul 2018 18:43:47 -0600 Subject: [PATCH 5/7] Fix checkstyle --- .../java/org/elasticsearch/http/nio/NioHttpServerChannel.java | 1 - .../org/elasticsearch/http/nio/NioHttpServerTransport.java | 3 --- .../netty4/SecurityNetty4HttpServerTransportTests.java | 1 - 3 files changed, 5 deletions(-) diff --git a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerChannel.java b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerChannel.java index 520704c6bf441..d72376da5c03b 100644 --- a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerChannel.java +++ b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerChannel.java @@ -23,7 +23,6 @@ import org.elasticsearch.http.HttpServerChannel; import org.elasticsearch.nio.NioServerSocketChannel; -import java.io.IOException; import java.nio.channels.ServerSocketChannel; public class NioHttpServerChannel extends NioServerSocketChannel implements HttpServerChannel { diff --git a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerTransport.java b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerTransport.java index 6a5ce713f3665..9c672c1caf15a 100644 --- a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerTransport.java +++ b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerTransport.java @@ -35,7 +35,6 @@ import org.elasticsearch.http.AbstractHttpServerTransport; import org.elasticsearch.http.HttpChannel; import org.elasticsearch.http.HttpServerChannel; -import org.elasticsearch.http.HttpServerTransport; import org.elasticsearch.http.nio.cors.NioCorsConfig; import org.elasticsearch.http.nio.cors.NioCorsConfigBuilder; import org.elasticsearch.nio.BytesChannelContext; @@ -49,8 +48,6 @@ import org.elasticsearch.nio.SocketChannelContext; import org.elasticsearch.rest.RestUtils; import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.transport.TcpTransport; -import org.elasticsearch.transport.nio.NioTransport; import java.io.IOException; import java.net.InetSocketAddress; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java index e0d1e5a282bc7..ad64dea79a587 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java @@ -14,7 +14,6 @@ import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; -import org.elasticsearch.http.HttpTransportSettings; import org.elasticsearch.http.NullDispatcher; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; From a0fbd07f6fe55b01f11cc47500bc98f35728176a Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Thu, 12 Jul 2018 20:59:30 -0600 Subject: [PATCH 6/7] Fix forbidden --- .../nio/SecurityNioHttpServerTransportTests.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransportTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransportTests.java index e782d8032a538..b5d84d459160f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransportTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransportTests.java @@ -26,6 +26,7 @@ import javax.net.ssl.SSLEngine; import java.io.IOException; +import java.net.InetAddress; import java.net.InetSocketAddress; import java.nio.channels.SocketChannel; import java.nio.file.Path; @@ -44,6 +45,7 @@ public class SecurityNioHttpServerTransportTests extends ESTestCase { private SSLService sslService; private Environment env; + private InetSocketAddress address = new InetSocketAddress(InetAddress.getLoopbackAddress(), 0); @Before public void createSSLService() { @@ -69,7 +71,7 @@ public void testDefaultClientAuth() throws IOException { xContentRegistry(), new NullDispatcher(), mock(IPFilter.class), sslService); SecurityNioHttpServerTransport.SecurityHttpChannelFactory factory = transport.channelFactory(); SocketChannel socketChannel = mock(SocketChannel.class); - when(socketChannel.getRemoteAddress()).thenReturn(new InetSocketAddress("localhost", 0)); + when(socketChannel.getRemoteAddress()).thenReturn(address); NioHttpChannel channel = factory.createChannel(mock(NioSelector.class), socketChannel); SSLEngine engine = SSLEngineUtils.getSSLEngine(channel); @@ -90,7 +92,7 @@ public void testOptionalClientAuth() throws IOException { SecurityNioHttpServerTransport.SecurityHttpChannelFactory factory = transport.channelFactory(); SocketChannel socketChannel = mock(SocketChannel.class); - when(socketChannel.getRemoteAddress()).thenReturn(new InetSocketAddress("localhost", 0)); + when(socketChannel.getRemoteAddress()).thenReturn(address); NioHttpChannel channel = factory.createChannel(mock(NioSelector.class), socketChannel); SSLEngine engine = SSLEngineUtils.getSSLEngine(channel); assertThat(engine.getNeedClientAuth(), is(false)); @@ -110,7 +112,7 @@ public void testRequiredClientAuth() throws IOException { SecurityNioHttpServerTransport.SecurityHttpChannelFactory factory = transport.channelFactory(); SocketChannel socketChannel = mock(SocketChannel.class); - when(socketChannel.getRemoteAddress()).thenReturn(new InetSocketAddress("localhost", 0)); + when(socketChannel.getRemoteAddress()).thenReturn(address); NioHttpChannel channel = factory.createChannel(mock(NioSelector.class), socketChannel); SSLEngine engine = SSLEngineUtils.getSSLEngine(channel); assertThat(engine.getNeedClientAuth(), is(true)); @@ -130,7 +132,7 @@ public void testNoClientAuth() throws IOException { SecurityNioHttpServerTransport.SecurityHttpChannelFactory factory = transport.channelFactory(); SocketChannel socketChannel = mock(SocketChannel.class); - when(socketChannel.getRemoteAddress()).thenReturn(new InetSocketAddress("localhost", 0)); + when(socketChannel.getRemoteAddress()).thenReturn(address); NioHttpChannel channel = factory.createChannel(mock(NioSelector.class), socketChannel); SSLEngine engine = SSLEngineUtils.getSSLEngine(channel); assertThat(engine.getNeedClientAuth(), is(false)); @@ -147,7 +149,7 @@ public void testCustomSSLConfiguration() throws IOException { xContentRegistry(), new NullDispatcher(), mock(IPFilter.class), sslService); SecurityNioHttpServerTransport.SecurityHttpChannelFactory factory = transport.channelFactory(); SocketChannel socketChannel = mock(SocketChannel.class); - when(socketChannel.getRemoteAddress()).thenReturn(new InetSocketAddress("localhost", 0)); + when(socketChannel.getRemoteAddress()).thenReturn(address); NioHttpChannel channel = factory.createChannel(mock(NioSelector.class), socketChannel); SSLEngine defaultEngine = SSLEngineUtils.getSSLEngine(channel); From 63ae74f3f511c7a9635394847fa50933ee78b591 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Fri, 13 Jul 2018 11:42:44 -0600 Subject: [PATCH 7/7] Issue from review --- .../security/rest/SecurityRestFilter.java | 6 +-- .../security/transport/SSLEngineUtils.java | 41 +++++++++++++++++++ .../SecurityHttpExceptionHandler.java | 2 +- .../transport/ServerTransportFilter.java | 32 +-------------- .../security/transport/nio/NioIPFilter.java | 2 +- 5 files changed, 45 insertions(+), 38 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java index d204586ed1ea4..8d304302e03ee 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java @@ -22,9 +22,7 @@ import org.elasticsearch.xpack.core.security.rest.RestRequestFilter; import org.elasticsearch.xpack.security.authc.AuthenticationService; import org.elasticsearch.xpack.security.transport.SSLEngineUtils; -import org.elasticsearch.xpack.security.transport.ServerTransportFilter; -import javax.net.ssl.SSLEngine; import java.io.IOException; public class SecurityRestFilter implements RestHandler { @@ -52,9 +50,7 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c // CORS - allow for preflight unauthenticated OPTIONS request if (extractClientCertificate) { HttpChannel httpChannel = request.getHttpChannel(); - SSLEngine sslEngine = SSLEngineUtils.getSSLEngine(httpChannel); - - ServerTransportFilter.extractClientCertificates(logger, threadContext, sslEngine, httpChannel); + SSLEngineUtils.extractClientCertificates(logger, threadContext, httpChannel); } service.authenticate(maybeWrapRestRequest(request), ActionListener.wrap( authentication -> { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SSLEngineUtils.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SSLEngineUtils.java index 559f8d814baa7..5bbcbaa050917 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SSLEngineUtils.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SSLEngineUtils.java @@ -7,6 +7,10 @@ import io.netty.channel.Channel; import io.netty.handler.ssl.SslHandler; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; +import org.apache.logging.log4j.util.Supplier; +import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.http.HttpChannel; import org.elasticsearch.http.netty4.Netty4HttpChannel; import org.elasticsearch.http.nio.NioHttpChannel; @@ -14,12 +18,28 @@ import org.elasticsearch.transport.TcpChannel; import org.elasticsearch.transport.netty4.Netty4TcpChannel; import org.elasticsearch.transport.nio.NioTcpChannel; +import org.elasticsearch.xpack.security.authc.pki.PkiRealm; import org.elasticsearch.xpack.security.transport.nio.SSLChannelContext; import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLPeerUnverifiedException; +import java.security.cert.Certificate; +import java.security.cert.X509Certificate; public class SSLEngineUtils { + private SSLEngineUtils() {} + + public static void extractClientCertificates(Logger logger, ThreadContext threadContext, HttpChannel httpChannel) { + SSLEngine sslEngine = getSSLEngine(httpChannel); + extract(logger, threadContext, sslEngine, httpChannel); + } + + public static void extractClientCertificates(Logger logger, ThreadContext threadContext, TcpChannel tcpChannel) { + SSLEngine sslEngine = getSSLEngine(tcpChannel); + extract(logger, threadContext, sslEngine, tcpChannel); + } + public static SSLEngine getSSLEngine(HttpChannel httpChannel) { if (httpChannel instanceof Netty4HttpChannel) { Channel nettyChannel = ((Netty4HttpChannel) httpChannel).getNettyChannel(); @@ -49,4 +69,25 @@ public static SSLEngine getSSLEngine(TcpChannel tcpChannel) { throw new AssertionError("Unknown channel class type: " + tcpChannel.getClass()); } } + + private static void extract(Logger logger, ThreadContext threadContext, SSLEngine sslEngine, Object channel) { + try { + Certificate[] certs = sslEngine.getSession().getPeerCertificates(); + if (certs instanceof X509Certificate[]) { + threadContext.putTransient(PkiRealm.PKI_CERT_HEADER_NAME, certs); + } + } catch (SSLPeerUnverifiedException e) { + // this happens when client authentication is optional and the client does not provide credentials. If client + // authentication was required then this connection should be closed before ever getting into this class + assert sslEngine.getNeedClientAuth() == false; + assert sslEngine.getWantClientAuth(); + if (logger.isTraceEnabled()) { + logger.trace( + (Supplier) () -> new ParameterizedMessage( + "SSL Peer did not present a certificate on channel [{}]", channel), e); + } else if (logger.isDebugEnabled()) { + logger.debug("SSL Peer did not present a certificate on channel [{}]", channel); + } + } + } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityHttpExceptionHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityHttpExceptionHandler.java index 310b78378e376..c1999c5ddfba2 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityHttpExceptionHandler.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityHttpExceptionHandler.java @@ -17,7 +17,7 @@ import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isNotSslRecordException; import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isReceivedCertificateUnknownException; -public class SecurityHttpExceptionHandler implements BiConsumer { +public final class SecurityHttpExceptionHandler implements BiConsumer { private final Lifecycle lifecycle; private final Logger logger; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java index 118bd4ea44db7..2f0c40c1fdd16 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java @@ -6,8 +6,6 @@ package org.elasticsearch.xpack.security.transport; import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.message.ParameterizedMessage; -import org.apache.logging.log4j.util.Supplier; import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.IndicesRequest; @@ -32,15 +30,10 @@ import org.elasticsearch.xpack.core.security.user.User; import org.elasticsearch.xpack.security.action.SecurityActionMapper; import org.elasticsearch.xpack.security.authc.AuthenticationService; -import org.elasticsearch.xpack.security.authc.pki.PkiRealm; import org.elasticsearch.xpack.security.authz.AuthorizationService; import org.elasticsearch.xpack.security.authz.AuthorizationUtils; -import javax.net.ssl.SSLEngine; -import javax.net.ssl.SSLPeerUnverifiedException; import java.io.IOException; -import java.security.cert.Certificate; -import java.security.cert.X509Certificate; import static org.elasticsearch.xpack.core.security.support.Exceptions.authenticationError; @@ -117,9 +110,8 @@ requests from all the nodes are attached with a user (either a serialize if (extractClientCert && (unwrappedChannel instanceof TcpTransportChannel)) { TcpChannel tcpChannel = ((TcpTransportChannel) unwrappedChannel).getChannel(); if (tcpChannel instanceof Netty4TcpChannel || tcpChannel instanceof NioTcpChannel) { - SSLEngine sslEngine = SSLEngineUtils.getSSLEngine(tcpChannel); if (tcpChannel.isOpen()) { - extractClientCertificates(logger, threadContext, sslEngine, tcpChannel); + SSLEngineUtils.extractClientCertificates(logger, threadContext, tcpChannel); } } } @@ -171,28 +163,6 @@ private void executeAsCurrentVersionKibanaUser(String securityAction, TransportR } } - // Channel is only used for logging, which is why it is okay to just be of type Object - static void extractClientCertificates(Logger logger, ThreadContext threadContext, SSLEngine sslEngine, Object channel) { - try { - Certificate[] certs = sslEngine.getSession().getPeerCertificates(); - if (certs instanceof X509Certificate[]) { - threadContext.putTransient(PkiRealm.PKI_CERT_HEADER_NAME, certs); - } - } catch (SSLPeerUnverifiedException e) { - // this happens when client authentication is optional and the client does not provide credentials. If client - // authentication was required then this connection should be closed before ever getting into this class - assert sslEngine.getNeedClientAuth() == false; - assert sslEngine.getWantClientAuth(); - if (logger.isTraceEnabled()) { - logger.trace( - (Supplier) () -> new ParameterizedMessage( - "SSL Peer did not present a certificate on channel [{}]", channel), e); - } else if (logger.isDebugEnabled()) { - logger.debug("SSL Peer did not present a certificate on channel [{}]", channel); - } - } - } - /** * A server transport filter rejects internal calls, which should be used on connections * where only clients connect to. This ensures that no client can send any internal actions diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/NioIPFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/NioIPFilter.java index 54f3de5b4a685..afb13ceff2edd 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/NioIPFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/NioIPFilter.java @@ -11,7 +11,7 @@ import java.util.function.Predicate; -public class NioIPFilter implements Predicate { +public final class NioIPFilter implements Predicate { private final IPFilter filter; private final String profile;