More precise close_notify handling

This commit is contained in:
Trustin Lee 2013-02-09 18:41:00 +09:00
parent 139b1b8382
commit 779870321c

View File

@ -149,7 +149,7 @@ public class SslHandler
InternalLoggerFactory.getInstance(SslHandler.class); InternalLoggerFactory.getInstance(SslHandler.class);
private static final Pattern IGNORABLE_CLASS_IN_STACK = Pattern.compile( private static final Pattern IGNORABLE_CLASS_IN_STACK = Pattern.compile(
"^.*(Socket|DatagramChannel|SctpChannel).*$"); "^.*(?:Socket|Datagram|Sctp)Channel.*$");
private static final Pattern IGNORABLE_ERROR_MESSAGE = Pattern.compile( private static final Pattern IGNORABLE_ERROR_MESSAGE = Pattern.compile(
"^.*(?:connection.*reset|connection.*closed|broken.*pipe).*$", "^.*(?:connection.*reset|connection.*closed|broken.*pipe).*$",
Pattern.CASE_INSENSITIVE); Pattern.CASE_INSENSITIVE);
@ -164,6 +164,7 @@ public class SslHandler
private final Queue<ChannelPromise> handshakePromises = new ArrayDeque<ChannelPromise>(); private final Queue<ChannelPromise> handshakePromises = new ArrayDeque<ChannelPromise>();
private final SSLEngineInboundCloseFuture sslCloseFuture = new SSLEngineInboundCloseFuture(); private final SSLEngineInboundCloseFuture sslCloseFuture = new SSLEngineInboundCloseFuture();
private final CloseNotifyListener closeNotifyWriteListener = new CloseNotifyListener();
private volatile long handshakeTimeoutMillis = 10000; private volatile long handshakeTimeoutMillis = 10000;
private volatile long closeNotifyTimeoutMillis = 3000; private volatile long closeNotifyTimeoutMillis = 3000;
@ -349,7 +350,14 @@ public class SslHandler
@Override @Override
public void run() { public void run() {
engine.closeOutbound(); engine.closeOutbound();
ctx.flush(future); future.addListener(closeNotifyWriteListener);
try {
flush(ctx, future);
} catch (Exception e) {
if (!future.tryFailure(e)) {
logger.warn("flush() raised a masked exception.", e);
}
}
} }
}); });
@ -572,14 +580,6 @@ public class SslHandler
try { try {
inboundBufferUpdated(ctx); inboundBufferUpdated(ctx);
} finally { } finally {
engine.closeOutbound();
try {
engine.closeInbound();
} catch (SSLException ex) {
if (logger.isDebugEnabled()) {
logger.debug("Failed to clean up SSLEngine.", ex);
}
}
ctx.fireChannelInactive(); ctx.fireChannelInactive();
} }
} }
@ -588,12 +588,11 @@ public class SslHandler
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
if (ignoreException(cause)) { if (ignoreException(cause)) {
// It is safe to ignore the 'connection reset by peer' or // It is safe to ignore the 'connection reset by peer' or
// 'broken pipe' error after sending closure_notify. // 'broken pipe' error after sending close_notify.
if (logger.isDebugEnabled()) { if (logger.isDebugEnabled()) {
logger.debug( logger.debug(
"Swallowing a 'connection reset by peer / " + "Swallowing a harmless 'connection reset by peer / broken pipe' error that occurred " +
"broken pipe' error occurred while writing " + "while writing close_notify in response to the peer's close_notify", cause);
"'closure_notify'", cause);
} }
// Close the connection explicitly just in case the transport // Close the connection explicitly just in case the transport
@ -601,9 +600,9 @@ public class SslHandler
if (ctx.channel().isActive()) { if (ctx.channel().isActive()) {
ctx.close(); ctx.close();
} }
return; } else {
ctx.fireExceptionCaught(cause);
} }
super.exceptionCaught(ctx, cause);
} }
/** /**
@ -616,7 +615,7 @@ public class SslHandler
* *
*/ */
private boolean ignoreException(Throwable t) { private boolean ignoreException(Throwable t) {
if (!(t instanceof SSLException) && t instanceof IOException && engine.isOutboundDone()) { if (!(t instanceof SSLException) && t instanceof IOException && sslCloseFuture.isDone()) {
String message = String.valueOf(t.getMessage()).toLowerCase(); String message = String.valueOf(t.getMessage()).toLowerCase();
// first try to match connection reset / broke peer based on the regex. This is the fastest way // first try to match connection reset / broke peer based on the regex. This is the fastest way
@ -898,29 +897,37 @@ public class SslHandler
* Notify all the handshake futures about the failure during the handshake. * Notify all the handshake futures about the failure during the handshake.
*/ */
private void setHandshakeFailure(Throwable cause) { private void setHandshakeFailure(Throwable cause) {
// Release all resources such as internal buffers that SSLEngine // Release all resources such as internal buffers that SSLEngine
// is managing. // is managing.
engine.closeOutbound(); engine.closeOutbound();
final boolean disconnected = cause == null || cause instanceof ClosedChannelException;
try { try {
engine.closeInbound(); engine.closeInbound();
} catch (SSLException e) { } catch (SSLException e) {
if (logger.isDebugEnabled()) { if (!disconnected) {
logger.debug( logger.warn("SSLEngine.closeInbound() raised an exception after a handshake failure.", e);
"SSLEngine.closeInbound() raised an exception after " + } else if (!closeNotifyWriteListener.done) {
"a handshake failure.", e); logger.warn("SSLEngine.closeInbound() raised an exception due to closed connection.", e);
} else {
// cause == null && sentCloseNotify
// closeInbound() will raise an exception with bogus truncation attack warning.
} }
} }
if (cause == null) { if (!handshakePromises.isEmpty()) {
cause = new ClosedChannelException(); if (cause == null) {
} cause = new ClosedChannelException();
}
for (;;) {
ChannelPromise p = handshakePromises.poll(); for (;;) {
if (p == null) { ChannelPromise p = handshakePromises.poll();
break; if (p == null) {
break;
}
p.setFailure(cause);
} }
p.setFailure(cause);
} }
flush0(ctx, 0, cause); flush0(ctx, 0, cause);
@ -939,7 +946,7 @@ public class SslHandler
engine.closeOutbound(); engine.closeOutbound();
ChannelPromise closeNotifyFuture = ctx.newPromise(); ChannelPromise closeNotifyFuture = ctx.newPromise().addListener(closeNotifyWriteListener);
flush0(ctx, closeNotifyFuture, true); flush0(ctx, closeNotifyFuture, true);
safeClose(ctx, closeNotifyFuture, promise); safeClose(ctx, closeNotifyFuture, promise);
} }
@ -1022,6 +1029,20 @@ public class SslHandler
}); });
} }
private static final class CloseNotifyListener implements ChannelFutureListener {
volatile boolean done;
@Override
public void operationComplete(ChannelFuture future) throws Exception {
if (future.isSuccess()) {
if (done) {
throw new IllegalStateException("notified twice");
}
done = true;
}
}
}
private final class SSLEngineInboundCloseFuture extends DefaultChannelPromise { private final class SSLEngineInboundCloseFuture extends DefaultChannelPromise {
public SSLEngineInboundCloseFuture() { public SSLEngineInboundCloseFuture() {
super(null); super(null);