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.
This commit is contained in:
Tim Brooks 2019-03-07 09:36:28 -07:00
parent b9586f62cc
commit 8043fefcf6
No known key found for this signature in database
GPG Key ID: C2AA3BB91A889E77
4 changed files with 67 additions and 63 deletions

View File

@ -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<TcpChannel, Exception> {
private final Lifecycle lifecycle;
private final Logger logger;
private final BiConsumer<TcpChannel, Exception> fallback;
public SecurityTransportExceptionHandler(Logger logger, Lifecycle lifecycle, BiConsumer<TcpChannel, Exception> 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);
}
}
}

View File

@ -13,11 +13,9 @@ import io.netty.channel.ChannelPromise;
import io.netty.handler.ssl.SslHandler; import io.netty.handler.ssl.SslHandler;
import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.elasticsearch.Version; import org.elasticsearch.Version;
import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.network.CloseableChannel;
import org.elasticsearch.common.network.NetworkService; import org.elasticsearch.common.network.NetworkService;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.PageCacheRecycler; import org.elasticsearch.common.util.PageCacheRecycler;
@ -28,7 +26,7 @@ import org.elasticsearch.transport.TcpChannel;
import org.elasticsearch.transport.netty4.Netty4Transport; import org.elasticsearch.transport.netty4.Netty4Transport;
import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.core.security.transport.ProfileConfigurations; 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.SSLConfiguration;
import org.elasticsearch.xpack.core.ssl.SSLService; 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 { public class SecurityNetty4Transport extends Netty4Transport {
private static final Logger logger = LogManager.getLogger(SecurityNetty4Transport.class); private static final Logger logger = LogManager.getLogger(SecurityNetty4Transport.class);
private final SecurityTransportExceptionHandler exceptionHandler;
private final SSLService sslService; private final SSLService sslService;
private final SSLConfiguration sslConfiguration; private final SSLConfiguration sslConfiguration;
private final Map<String, SSLConfiguration> profileConfiguration; private final Map<String, SSLConfiguration> profileConfiguration;
@ -64,6 +63,7 @@ public class SecurityNetty4Transport extends Netty4Transport {
final CircuitBreakerService circuitBreakerService, final CircuitBreakerService circuitBreakerService,
final SSLService sslService) { final SSLService sslService) {
super(settings, version, threadPool, networkService, pageCacheRecycler, namedWriteableRegistry, circuitBreakerService); super(settings, version, threadPool, networkService, pageCacheRecycler, namedWriteableRegistry, circuitBreakerService);
this.exceptionHandler = new SecurityTransportExceptionHandler(logger, lifecycle, (c, e) -> super.onException(c, e));
this.sslService = sslService; this.sslService = sslService;
this.sslEnabled = XPackSettings.TRANSPORT_SSL_ENABLED.get(settings); this.sslEnabled = XPackSettings.TRANSPORT_SSL_ENABLED.get(settings);
if (sslEnabled) { if (sslEnabled) {
@ -105,34 +105,7 @@ public class SecurityNetty4Transport extends Netty4Transport {
@Override @Override
public void onException(TcpChannel channel, Exception e) { public void onException(TcpChannel channel, Exception e) {
if (!lifecycle.started()) { exceptionHandler.accept(channel, e);
// 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);
}
} }
public class SslChannelInitializer extends ServerChannelInitializer { public class SslChannelInitializer extends ServerChannelInitializer {

View File

@ -46,7 +46,7 @@ public final class SecurityHttpExceptionHandler implements BiConsumer<HttpChanne
if (logger.isTraceEnabled()) { if (logger.isTraceEnabled()) {
logger.trace(new ParameterizedMessage("connection {} closed during ssl handshake", channel), e); logger.trace(new ParameterizedMessage("connection {} closed during ssl handshake", channel), e);
} else { } else {
logger.warn("connection {} closed during ssl handshake", channel); logger.debug("connection {} closed during ssl handshake", channel);
} }
CloseableChannel.closeChannel(channel); CloseableChannel.closeChannel(channel);
} else if (isReceivedCertificateUnknownException(e)) { } else if (isReceivedCertificateUnknownException(e)) {

View File

@ -7,12 +7,10 @@ package org.elasticsearch.xpack.security.transport.nio;
import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.elasticsearch.Version; import org.elasticsearch.Version;
import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.network.CloseableChannel;
import org.elasticsearch.common.network.NetworkService; import org.elasticsearch.common.network.NetworkService;
import org.elasticsearch.common.recycler.Recycler; import org.elasticsearch.common.recycler.Recycler;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
@ -36,7 +34,7 @@ import org.elasticsearch.transport.nio.NioTransport;
import org.elasticsearch.transport.nio.TcpReadWriteHandler; import org.elasticsearch.transport.nio.TcpReadWriteHandler;
import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.core.security.transport.ProfileConfigurations; 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.SSLConfiguration;
import org.elasticsearch.xpack.core.ssl.SSLService; import org.elasticsearch.xpack.core.ssl.SSLService;
import org.elasticsearch.xpack.security.transport.filter.IPFilter; import org.elasticsearch.xpack.security.transport.filter.IPFilter;
@ -69,6 +67,7 @@ import static org.elasticsearch.xpack.core.security.SecurityField.setting;
public class SecurityNioTransport extends NioTransport { public class SecurityNioTransport extends NioTransport {
private static final Logger logger = LogManager.getLogger(SecurityNioTransport.class); private static final Logger logger = LogManager.getLogger(SecurityNioTransport.class);
private final SecurityTransportExceptionHandler exceptionHandler;
private final IPFilter authenticator; private final IPFilter authenticator;
private final SSLService sslService; private final SSLService sslService;
private final Map<String, SSLConfiguration> profileConfiguration; private final Map<String, SSLConfiguration> profileConfiguration;
@ -80,6 +79,7 @@ public class SecurityNioTransport extends NioTransport {
SSLService sslService, NioGroupFactory groupFactory) { SSLService sslService, NioGroupFactory groupFactory) {
super(settings, version, threadPool, networkService, pageCacheRecycler, namedWriteableRegistry, circuitBreakerService, super(settings, version, threadPool, networkService, pageCacheRecycler, namedWriteableRegistry, circuitBreakerService,
groupFactory); groupFactory);
this.exceptionHandler = new SecurityTransportExceptionHandler(logger, lifecycle, (c, e) -> super.onException(c, e));
this.authenticator = authenticator; this.authenticator = authenticator;
this.sslService = sslService; this.sslService = sslService;
this.sslEnabled = XPackSettings.TRANSPORT_SSL_ENABLED.get(settings); this.sslEnabled = XPackSettings.TRANSPORT_SSL_ENABLED.get(settings);
@ -102,34 +102,7 @@ public class SecurityNioTransport extends NioTransport {
@Override @Override
public void onException(TcpChannel channel, Exception e) { public void onException(TcpChannel channel, Exception e) {
if (!lifecycle.started()) { exceptionHandler.accept(channel, e);
// 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);
}
} }
@Override @Override