andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

SslHandler#wrap to preserve exception if SSLEngine is closed (#10327)

Browse Source 
Motivation:
SslHandler currently throws a general SSLException if a wrap attempt
fails due to the SSLEngine being closed. If writes are queued the
failure rational typically requires more investigation to track down the
original failure from a previous event. We may have more informative
rational for the failure and so we should use it.

Modifications:
- SslHandler#wrap to use failure information from the handshake or prior
transport closure if available

Result:
More informative exceptions from SslHandler#wrap if the SSLEngine has
been previously closed.
This commit is contained in:
Scott Mitchell 2020-06-02 00:40:14 -07:00 committed by GitHub
parent 0375e6e01b
commit bc943808d0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 37 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
handler/src/main/java/io/netty/handler/ssl/SslHandler.java
View File

@ -845,7 +845,15 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
if (result.getStatus() == Status.CLOSED) {
buf.release();
buf = null;
SSLException exception = new SSLException("SSLEngine closed already");
// Make a best effort to preserve any exception that way previously encountered from the handshake
// or the transport, else fallback to a general error.
Throwable exception = handshakePromise.cause();
if (exception == null) {
exception = sslClosePromise.cause();
if (exception == null) {
exception = new SSLException("SSLEngine closed already");
}
}
promise.tryFailure(exception);
promise = null;
// SSLEngine has been closed already.

35
handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java
View File

@ -40,6 +40,8 @@ import io.netty.handler.ssl.util.SimpleTrustManagerFactory;
import io.netty.util.CharsetUtil;
import io.netty.util.NetUtil;
import io.netty.util.ReferenceCountUtil;
import io.netty.util.concurrent.ImmediateEventExecutor;
import io.netty.util.concurrent.UnaryPromiseNotifier;
import io.netty.util.internal.ResourcesUtil;
import io.netty.util.concurrent.Future;
import io.netty.util.concurrent.Promise;
@ -833,21 +835,27 @@ public abstract class SSLEngineTest {
@Test
public void testClientHostnameValidationFail() throws InterruptedException, SSLException {
mySetupClientHostnameValidation(ResourcesUtil.getFile(getClass(), "notlocalhost_server.pem"),
ResourcesUtil.getFile(getClass(), "notlocalhost_server.key"),
ResourcesUtil.getFile(getClass(), "mutual_auth_ca.pem"),
true);
Future<Void> clientWriteFuture =
mySetupClientHostnameValidation(ResourcesUtil.getFile(getClass(), "notlocalhost_server.pem"),
ResourcesUtil.getFile(getClass(), "notlocalhost_server.key"),
ResourcesUtil.getFile(getClass(), "mutual_auth_ca.pem"),
true);
assertTrue(clientLatch.await(5, TimeUnit.SECONDS));
assertTrue("unexpected exception: " + clientException,
mySetupMutualAuthServerIsValidClientException(clientException));
assertTrue(serverLatch.await(5, TimeUnit.SECONDS));
assertTrue("unexpected exception: " + serverException,
mySetupMutualAuthServerIsValidServerException(serverException));
// Verify that any pending writes are failed with the cached handshake exception and not a general SSLException.
clientWriteFuture.awaitUninterruptibly();
Throwable actualCause = clientWriteFuture.cause();
assertSame(clientException, actualCause);
}
private void mySetupClientHostnameValidation(File serverCrtFile, File serverKeyFile,
File clientTrustCrtFile,
final boolean failureExpected)
private Future<Void> mySetupClientHostnameValidation(File serverCrtFile, File serverKeyFile,
File clientTrustCrtFile,
final boolean failureExpected)
throws SSLException, InterruptedException {
final String expectedHost = "localhost";
serverSslCtx = wrapContext(SslContextBuilder.forServer(serverCrtFile, serverKeyFile, null)
@ -918,6 +926,7 @@ public abstract class SSLEngineTest {
}
});
final Promise<Void> clientWritePromise = ImmediateEventExecutor.INSTANCE.newPromise();
cb.group(new NioEventLoopGroup());
cb.channel(NioSocketChannel.class);
cb.handler(new ChannelInitializer<Channel>() {
@ -941,6 +950,17 @@ public abstract class SSLEngineTest {
p.addLast(sslHandler);
p.addLast(new MessageDelegatorChannelHandler(clientReceiver, clientLatch));
p.addLast(new ChannelInboundHandlerAdapter() {
@Override
public void handlerAdded(ChannelHandlerContext ctx) {
// Only write if there is a failure expected. We don't actually care about the write going
// through we just want to verify the local failure condition. This way we don't have to worry
// about verifying the payload and releasing the content on the server side.
if (failureExpected) {
ctx.write(ctx.alloc().buffer(1).writeByte(1))
.addListener(new UnaryPromiseNotifier<Void>(clientWritePromise));
}
}
@Override
public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
if (evt == SslHandshakeCompletionEvent.SUCCESS) {
@ -974,6 +994,7 @@ public abstract class SSLEngineTest {
ChannelFuture ccf = cb.connect(new InetSocketAddress(expectedHost, port));
assertTrue(ccf.awaitUninterruptibly().isSuccess());
clientChannel = ccf.channel();
return clientWritePromise;
}
private void mySetupMutualAuth(File keyFile, File crtFile, String keyPassword)