Skip to content

Commit

Permalink
Add StacklessSSLHandshakeException for ClosedChannelException (#1…
Browse files Browse the repository at this point in the history
…3099)

Motivation:
When a channel is closed during the SSL handshake then `java.nio.channels.ClosedChannelException` is thrown. This is correct when we see it from a low-level transport perspective but from a high-level, this error is not detailed enough to debug. And when we log `ClosedChannelException`, we get this line: `java.nio.channels.ClosedChannelException: null` in the stack trace.

Modification:
To combat this, we should throw `StacklessSSLHandshakeException` with the message `"Connection closed while SSL/TLS handshake was in progress"` as suppressed with`ClosedChannelException`. This will explicitly declare that the handshake failed due to connection closure.

Result:
Fixes #12000

Co-authored-by: Idel Pivnitskiy <idel.pivnitskiy@apple.com>
  • Loading branch information
2 people authored and normanmaurer committed Jan 24, 2023
1 parent b8d8ee1 commit bd7b70d
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 4 deletions.
8 changes: 8 additions & 0 deletions handler/src/main/java/io/netty5/handler/ssl/SslHandler.java
Expand Up @@ -898,7 +898,15 @@ private SSLEngineResult wrap(SSLEngine engine, Buffer in, Buffer out)
public void channelInactive(ChannelHandlerContext ctx) throws Exception {
boolean handshakeFailed = handshakePromise.isFailed();

// Channel closed, we will generate 'ClosedChannelException' now.
ClosedChannelException exception = new ClosedChannelException();

// Add a supressed exception if the handshake was not completed yet.
if (isStateSet(STATE_HANDSHAKE_STARTED) && !handshakePromise.isDone()) {
exception.addSuppressed(
new StacklessSSLHandshakeException("Connection closed while SSL/TLS handshake was in progress"));
}

// Make sure to release SSLEngine,
// and notify the handshake future if the connection has been closed during handshake.
setHandshakeFailure(ctx, exception, !isStateSet(STATE_OUTBOUND_CLOSED), isStateSet(STATE_HANDSHAKE_STARTED),
Expand Down
@@ -0,0 +1,43 @@
/*
* Copyright 2023 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package io.netty5.handler.ssl;

import javax.net.ssl.SSLHandshakeException;

/**
* A {@link SSLHandshakeException} that does not fill in the stack trace.
*/
final class StacklessSSLHandshakeException extends SSLHandshakeException {

private static final long serialVersionUID = -1244781947804415549L;

/**
* Constructs an exception reporting an error found by
* an SSL subsystem during handshaking.
*
* @param reason describes the problem.
*/
StacklessSSLHandshakeException(String reason) {
super(reason);
}

@Override
public Throwable fillInStackTrace() {
// This is a performance optimization to not fill in the
// stack trace as this is a stackless exception.
return this;
}
}
Expand Up @@ -560,8 +560,8 @@ public void testCloseFutureNotified() throws Exception {

assertFalse(ch.finishAndReleaseAll());

assertTrue(handler.handshakeFuture().cause() instanceof ClosedChannelException);
assertTrue(handler.sslCloseFuture().cause() instanceof ClosedChannelException);
assertThat(handler.handshakeFuture().cause(), instanceOf(ClosedChannelException.class));
assertThat(handler.sslCloseFuture().cause(), instanceOf(ClosedChannelException.class));
}

@Test
Expand All @@ -582,11 +582,11 @@ public void channelInboundEvent(ChannelHandlerContext ctx, Object evt) throws Ex

SslCompletionEvent evt = events.take();
assertTrue(evt instanceof SslHandshakeCompletionEvent);
assertTrue(evt.cause() instanceof ClosedChannelException);
assertThat(evt.cause(), instanceOf(ClosedChannelException.class));

evt = events.take();
assertTrue(evt instanceof SslCloseCompletionEvent);
assertTrue(evt.cause() instanceof ClosedChannelException);
assertThat(evt.cause(), instanceOf(ClosedChannelException.class));
assertTrue(events.isEmpty());
}

Expand Down

0 comments on commit bd7b70d

Please sign in to comment.