Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suppress noisy SSL exceptions #61359

Conversation

DaveCTurner
Copy link
Contributor

If a TLS-protected connection closes unexpectedly then today we often
emit a WARN log, typically one of the following:

io.netty.handler.codec.DecoderException: javax.net.ssl.SSLHandshakeException: Insufficient buffer remaining for AEAD cipher fragment (2). Needs to be more than tag size (16)

io.netty.handler.codec.DecoderException: javax.net.ssl.SSLException: Received close_notify during handshake

We typically only report unexpectedly-closed connections at DEBUG
level, but these two messages don't follow that rule and generate a lot
of noise as a result. This commit adjusts the logging to report these
two exceptions at DEBUG level only.

If a TLS-protected connection closes unexpectedly then today we often
emit a `WARN` log, typically one of the following:

    io.netty.handler.codec.DecoderException: javax.net.ssl.SSLHandshakeException: Insufficient buffer remaining for AEAD cipher fragment (2). Needs to be more than tag size (16)

    io.netty.handler.codec.DecoderException: javax.net.ssl.SSLException: Received close_notify during handshake

We typically only report unexpectedly-closed connections at `DEBUG`
level, but these two messages don't follow that rule and generate a lot
of noise as a result. This commit adjusts the logging to report these
two exceptions at `DEBUG` level only.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (:Security/Network)

@elasticmachine elasticmachine added the Team:Security Meta label for security team label Aug 20, 2020
@DaveCTurner
Copy link
Contributor Author

I could do with guidance on testing this change, I couldn't find a pre-existing test suite for this area, nor do I know how to trigger these exceptions synthetically.

@@ -22,6 +24,11 @@ public static boolean isNotSslRecordException(Throwable e) {
}

public static boolean isCloseDuringHandshakeException(Throwable e) {
return isCloseDuringHandshakeSSLException(e)
|| isCloseDuringHandshakeSSLException(e.getCause());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were already checking for Received close_notify during handshake, just not within a DecoderException. Not sure whether this was a mistake or whether we do sometimes see this exception unwrapped too.

Also not sure what the implications for the nio transport since the DecoderException wrapper is Netty-specific.

Copy link
Contributor

@Tim-Brooks Tim-Brooks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DaveCTurner DaveCTurner merged commit e7e6859 into elastic:master Aug 27, 2020
@DaveCTurner DaveCTurner deleted the 2020-08-20-suppress-noisy-ssl-exceptions branch August 27, 2020 09:59
@DaveCTurner
Copy link
Contributor Author

Thanks Tim!

DaveCTurner added a commit that referenced this pull request Aug 27, 2020
If a TLS-protected connection closes unexpectedly then today we often
emit a `WARN` log, typically one of the following:

    io.netty.handler.codec.DecoderException: javax.net.ssl.SSLHandshakeException: Insufficient buffer remaining for AEAD cipher fragment (2). Needs to be more than tag size (16)

    io.netty.handler.codec.DecoderException: javax.net.ssl.SSLException: Received close_notify during handshake

We typically only report unexpectedly-closed connections at `DEBUG`
level, but these two messages don't follow that rule and generate a lot
of noise as a result. This commit adjusts the logging to report these
two exceptions at `DEBUG` level only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Security/TLS SSL/TLS, Certificates Team:Security Meta label for security team v7.10.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants