SslHandler should fail handshake / close promise and notify pipeline on removal (#10161)

Motivation:

If the SslHandler is removed from the pipeline we also need to ensure we fail the handshake / close promise if it was not notified before as otherwise we may never do so.

Modifications:

- Correctly fail promise and notify pipeline if handshake was not done yet when the SslHandler is removed
- Add unit test

Result:

Fix https://github.com/netty/netty/issues/10158
This commit is contained in:
Norman Maurer 2020-04-03 08:45:48 +02:00
parent 492ecae4cf
commit 0368da9e3c
2 changed files with 56 additions and 0 deletions

View File

@ -66,6 +66,7 @@ import javax.net.ssl.SSLEngineResult;
import javax.net.ssl.SSLEngineResult.HandshakeStatus;
import javax.net.ssl.SSLEngineResult.Status;
import javax.net.ssl.SSLException;
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.SSLSession;
import static io.netty.buffer.ByteBufUtil.ensureWritableSuccess;
@ -688,6 +689,24 @@ public class SslHandler extends ByteToMessageDecoder {
new ChannelException("Pending write on removal of SslHandler"));
}
pendingUnencryptedWrites = null;
SSLHandshakeException cause = null;
// If the handshake is not done yet we should fail the handshake promise and notify the rest of the
// pipeline.
if (!handshakePromise.isDone()) {
cause = new SSLHandshakeException("SslHandler removed before handshake completed");
if (handshakePromise.tryFailure(cause)) {
ctx.fireUserEventTriggered(new SslHandshakeCompletionEvent(cause));
}
}
if (!sslClosePromise.isDone()) {
if (cause == null) {
cause = new SSLHandshakeException("SslHandler removed before handshake completed");
}
notifyClosePromise(cause);
}
if (engine instanceof ReferenceCounted) {
((ReferenceCounted) engine).release();
}

View File

@ -249,6 +249,43 @@ public class SslHandlerTest {
}
}
@Test(timeout = 5000L)
public void testHandshakeAndClosePromiseFailedOnRemoval() throws Exception {
SSLEngine engine = SSLContext.getDefault().createSSLEngine();
engine.setUseClientMode(true);
SslHandler handler = new SslHandler(engine);
final AtomicReference<Throwable> handshakeRef = new AtomicReference<>();
final AtomicReference<Throwable> closeRef = new AtomicReference<>();
EmbeddedChannel ch = new EmbeddedChannel(handler, new ChannelHandler() {
@Override
public void userEventTriggered(ChannelHandlerContext ctx, Object evt) {
if (evt instanceof SslHandshakeCompletionEvent) {
handshakeRef.set(((SslHandshakeCompletionEvent) evt).cause());
} else if (evt instanceof SslCloseCompletionEvent) {
closeRef.set(((SslCloseCompletionEvent) evt).cause());
}
}
});
assertFalse(handler.handshakeFuture().isDone());
assertFalse(handler.sslCloseFuture().isDone());
ch.pipeline().remove(handler);
try {
while (!handler.handshakeFuture().isDone() || handshakeRef.get() == null
|| !handler.sslCloseFuture().isDone() || closeRef.get() == null) {
Thread.sleep(10);
// Continue running all pending tasks until we notified for everything.
ch.runPendingTasks();
}
assertSame(handler.handshakeFuture().cause(), handshakeRef.get());
assertSame(handler.sslCloseFuture().cause(), closeRef.get());
} finally {
ch.finishAndReleaseAll();
}
}
@Test
public void testTruncatedPacket() throws Exception {
SSLEngine engine = newServerModeSSLEngine();