From 8043fefcf655068c2cdc0e120354435fba6753c2 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Thu, 7 Mar 2019 09:36:28 -0700 Subject: [PATCH] Log close_notify during handshake at debug level (#39715) A TLS handshake requires exchanging multiple messages to initiate a session. If one side decides to close during the handshake, it is supposed to send a close_notify alert (similar to closing during application data exchange). The java SSLEngine engine throws an exception when this happens. We currently log this at the warn level if trace logging is not enabled. This level is too high for a valid scenario. Additionally it happens all the time in tests (quickly closing and opened transports). This commit changes this to be logged at the debug level if trace is not enabled. Additionally, it extracts the transport security exception handling to a common class. --- .../SecurityTransportExceptionHandler.java | 58 +++++++++++++++++++ .../netty4/SecurityNetty4Transport.java | 35 ++--------- .../SecurityHttpExceptionHandler.java | 2 +- .../transport/nio/SecurityNioTransport.java | 35 ++--------- 4 files changed, 67 insertions(+), 63 deletions(-) create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/transport/SecurityTransportExceptionHandler.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/transport/SecurityTransportExceptionHandler.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/transport/SecurityTransportExceptionHandler.java new file mode 100644 index 00000000000..37616d0ad76 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/transport/SecurityTransportExceptionHandler.java @@ -0,0 +1,58 @@ +/* + * 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.core.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.transport.TcpChannel; + +import java.util.function.BiConsumer; + +public final class SecurityTransportExceptionHandler implements BiConsumer { + + private final Lifecycle lifecycle; + private final Logger logger; + private final BiConsumer fallback; + + public SecurityTransportExceptionHandler(Logger logger, Lifecycle lifecycle, BiConsumer fallback) { + this.lifecycle = lifecycle; + this.logger = logger; + this.fallback = fallback; + } + + public void accept(TcpChannel channel, Exception e) { + if (!lifecycle.started()) { + // just close and ignore - we are already stopped and just need to make sure we release all resources + CloseableChannel.closeChannel(channel); + } else if (SSLExceptionHelper.isNotSslRecordException(e)) { + if (logger.isTraceEnabled()) { + logger.trace( + new ParameterizedMessage("received plaintext traffic on an encrypted channel, closing connection {}", channel), e); + } else { + logger.warn("received plaintext traffic on an encrypted channel, closing connection {}", channel); + } + CloseableChannel.closeChannel(channel); + } else if (SSLExceptionHelper.isCloseDuringHandshakeException(e)) { + if (logger.isTraceEnabled()) { + logger.trace(new ParameterizedMessage("connection {} closed during ssl handshake", channel), e); + } else { + logger.debug("connection {} closed during handshake", channel); + } + CloseableChannel.closeChannel(channel); + } else if (SSLExceptionHelper.isReceivedCertificateUnknownException(e)) { + if (logger.isTraceEnabled()) { + logger.trace(new ParameterizedMessage("client did not trust server's certificate, closing connection {}", channel), e); + } else { + logger.warn("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/core/src/main/java/org/elasticsearch/xpack/core/security/transport/netty4/SecurityNetty4Transport.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/transport/netty4/SecurityNetty4Transport.java index 80bf638d781..6e2b9c1a7ef 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/transport/netty4/SecurityNetty4Transport.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/transport/netty4/SecurityNetty4Transport.java @@ -13,11 +13,9 @@ import io.netty.channel.ChannelPromise; import io.netty.handler.ssl.SslHandler; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.Version; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; -import org.elasticsearch.common.network.CloseableChannel; import org.elasticsearch.common.network.NetworkService; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.PageCacheRecycler; @@ -28,7 +26,7 @@ import org.elasticsearch.transport.TcpChannel; import org.elasticsearch.transport.netty4.Netty4Transport; import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.security.transport.ProfileConfigurations; -import org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper; +import org.elasticsearch.xpack.core.security.transport.SecurityTransportExceptionHandler; import org.elasticsearch.xpack.core.ssl.SSLConfiguration; import org.elasticsearch.xpack.core.ssl.SSLService; @@ -49,6 +47,7 @@ import static org.elasticsearch.xpack.core.security.SecurityField.setting; public class SecurityNetty4Transport extends Netty4Transport { private static final Logger logger = LogManager.getLogger(SecurityNetty4Transport.class); + private final SecurityTransportExceptionHandler exceptionHandler; private final SSLService sslService; private final SSLConfiguration sslConfiguration; private final Map profileConfiguration; @@ -64,6 +63,7 @@ public class SecurityNetty4Transport extends Netty4Transport { final CircuitBreakerService circuitBreakerService, final SSLService sslService) { super(settings, version, threadPool, networkService, pageCacheRecycler, namedWriteableRegistry, circuitBreakerService); + this.exceptionHandler = new SecurityTransportExceptionHandler(logger, lifecycle, (c, e) -> super.onException(c, e)); this.sslService = sslService; this.sslEnabled = XPackSettings.TRANSPORT_SSL_ENABLED.get(settings); if (sslEnabled) { @@ -105,34 +105,7 @@ public class SecurityNetty4Transport extends Netty4Transport { @Override public void onException(TcpChannel channel, Exception e) { - if (!lifecycle.started()) { - // just close and ignore - we are already stopped and just need to make sure we release all resources - CloseableChannel.closeChannel(channel); - } else if (SSLExceptionHelper.isNotSslRecordException(e)) { - if (logger.isTraceEnabled()) { - logger.trace( - new ParameterizedMessage("received plaintext traffic on an encrypted channel, closing connection {}", channel), e); - } else { - logger.warn("received plaintext traffic on an encrypted channel, closing connection {}", channel); - } - CloseableChannel.closeChannel(channel); - } else if (SSLExceptionHelper.isCloseDuringHandshakeException(e)) { - if (logger.isTraceEnabled()) { - logger.trace(new ParameterizedMessage("connection {} closed during ssl handshake", channel), e); - } else { - logger.warn("connection {} closed during handshake", channel); - } - CloseableChannel.closeChannel(channel); - } else if (SSLExceptionHelper.isReceivedCertificateUnknownException(e)) { - if (logger.isTraceEnabled()) { - logger.trace(new ParameterizedMessage("client did not trust server's certificate, closing connection {}", channel), e); - } else { - logger.warn("client did not trust this server's certificate, closing connection {}", channel); - } - CloseableChannel.closeChannel(channel); - } else { - super.onException(channel, e); - } + exceptionHandler.accept(channel, e); } public class SslChannelInitializer extends ServerChannelInitializer { 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 20d2115116f..d49c05f3344 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 @@ -46,7 +46,7 @@ public final class SecurityHttpExceptionHandler implements BiConsumer profileConfiguration; @@ -80,6 +79,7 @@ public class SecurityNioTransport extends NioTransport { SSLService sslService, NioGroupFactory groupFactory) { super(settings, version, threadPool, networkService, pageCacheRecycler, namedWriteableRegistry, circuitBreakerService, groupFactory); + this.exceptionHandler = new SecurityTransportExceptionHandler(logger, lifecycle, (c, e) -> super.onException(c, e)); this.authenticator = authenticator; this.sslService = sslService; this.sslEnabled = XPackSettings.TRANSPORT_SSL_ENABLED.get(settings); @@ -102,34 +102,7 @@ public class SecurityNioTransport extends NioTransport { @Override public void onException(TcpChannel channel, Exception e) { - if (!lifecycle.started()) { - // just close and ignore - we are already stopped and just need to make sure we release all resources - CloseableChannel.closeChannel(channel); - } else if (SSLExceptionHelper.isNotSslRecordException(e)) { - if (logger.isTraceEnabled()) { - logger.trace( - new ParameterizedMessage("received plaintext traffic on an encrypted channel, closing connection {}", channel), e); - } else { - logger.warn("received plaintext traffic on an encrypted channel, closing connection {}", channel); - } - CloseableChannel.closeChannel(channel); - } else if (SSLExceptionHelper.isCloseDuringHandshakeException(e)) { - if (logger.isTraceEnabled()) { - logger.trace(new ParameterizedMessage("connection {} closed during ssl handshake", channel), e); - } else { - logger.warn("connection {} closed during handshake", channel); - } - CloseableChannel.closeChannel(channel); - } else if (SSLExceptionHelper.isReceivedCertificateUnknownException(e)) { - if (logger.isTraceEnabled()) { - logger.trace(new ParameterizedMessage("client did not trust server's certificate, closing connection {}", channel), e); - } else { - logger.warn("client did not trust this server's certificate, closing connection {}", channel); - } - CloseableChannel.closeChannel(channel); - } else { - super.onException(channel, e); - } + exceptionHandler.accept(channel, e); } @Override