Do not issue a handshake when an SSLEngine is closing/closed

Motivation:

We sometimes get the following exception:

javax.net.ssl.SSLException: SSLEngine is closing/closed
  at sun.security.ssl.SSLEngineImpl.kickstartHandshake(SSLEngineImpl.java:692)
  at sun.security.ssl.SSLEngineImpl.beginHandshake(SSLEngineImpl.java:734)
  at org.jboss.netty.handler.ssl.SslHandler.handshake(SslHandler.java:433)
  at org.jboss.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1233)
  at org.jboss.netty.handler.ssl.SslHandler.channelDisconnected(SslHandler.java:668)

.. which is triggered when we attempt to issue a handshake even if the
SSLEngine is closed (or being closed).

We don't really need to initiate handshake if:

- SSLEngine is closed already.
- Connection is closed already

Modifications:

Add a boolean parameter to unwrap() and unwrapNonApp() to suppress the
invocation of handshake() when SSLEngine or connection is closed
already.
This commit is contained in:
Trustin Lee 2014-08-05 11:44:15 -07:00
parent ba84e44660
commit 9efe48fc3a

View File

@ -645,7 +645,7 @@ public class SslHandler extends FrameDecoder
try { try {
super.channelDisconnected(ctx, e); super.channelDisconnected(ctx, e);
} finally { } finally {
unwrapNonAppData(ctx, e.getChannel()); unwrapNonAppData(ctx, e.getChannel(), false);
closeEngine(); closeEngine();
} }
} }
@ -914,7 +914,7 @@ public class SslHandler extends FrameDecoder
// See https://github.com/netty/netty/issues/1534 // See https://github.com/netty/netty/issues/1534
final ByteBuffer inNetBuf = in.toByteBuffer(in.readerIndex(), totalLength); final ByteBuffer inNetBuf = in.toByteBuffer(in.readerIndex(), totalLength);
unwrapped = unwrap(ctx, channel, in, inNetBuf, totalLength); unwrapped = unwrap(ctx, channel, in, inNetBuf, totalLength, true);
assert !inNetBuf.hasRemaining() || engine.isInboundDone(); assert !inNetBuf.hasRemaining() || engine.isInboundDone();
} }
@ -1101,7 +1101,7 @@ public class SslHandler extends FrameDecoder
} }
if (needsUnwrap) { if (needsUnwrap) {
unwrapNonAppData(ctx, channel); unwrapNonAppData(ctx, channel, true);
} }
} }
@ -1191,7 +1191,7 @@ public class SslHandler extends FrameDecoder
// unwrap shouldn't be called when this method was // unwrap shouldn't be called when this method was
// called by unwrap - unwrap will keep running after // called by unwrap - unwrap will keep running after
// this method returns. // this method returns.
unwrapNonAppData(ctx, channel); unwrapNonAppData(ctx, channel, true);
} }
break; break;
case NOT_HANDSHAKING: case NOT_HANDSHAKING:
@ -1227,8 +1227,9 @@ public class SslHandler extends FrameDecoder
/** /**
* Calls {@link SSLEngine#unwrap(ByteBuffer, ByteBuffer)} with an empty buffer to handle handshakes, etc. * Calls {@link SSLEngine#unwrap(ByteBuffer, ByteBuffer)} with an empty buffer to handle handshakes, etc.
*/ */
private void unwrapNonAppData(ChannelHandlerContext ctx, Channel channel) throws SSLException { private void unwrapNonAppData(
unwrap(ctx, channel, ChannelBuffers.EMPTY_BUFFER, EMPTY_BUFFER, -1); ChannelHandlerContext ctx, Channel channel, boolean mightNeedHandshake) throws SSLException {
unwrap(ctx, channel, ChannelBuffers.EMPTY_BUFFER, EMPTY_BUFFER, -1, mightNeedHandshake);
} }
/** /**
@ -1237,7 +1238,7 @@ public class SslHandler extends FrameDecoder
private ChannelBuffer unwrap( private ChannelBuffer unwrap(
ChannelHandlerContext ctx, Channel channel, ChannelHandlerContext ctx, Channel channel,
ChannelBuffer nettyInNetBuf, ByteBuffer nioInNetBuf, ChannelBuffer nettyInNetBuf, ByteBuffer nioInNetBuf,
int initialNettyOutAppBufCapacity) throws SSLException { int initialNettyOutAppBufCapacity, boolean mightNeedHandshake) throws SSLException {
final int nettyInNetBufStartOffset = nettyInNetBuf.readerIndex(); final int nettyInNetBufStartOffset = nettyInNetBuf.readerIndex();
final int nioInNetBufStartOffset = nioInNetBuf.position(); final int nioInNetBufStartOffset = nioInNetBuf.position();
@ -1250,6 +1251,7 @@ public class SslHandler extends FrameDecoder
for (;;) { for (;;) {
SSLEngineResult result; SSLEngineResult result;
boolean needsHandshake = false; boolean needsHandshake = false;
if (mightNeedHandshake) {
synchronized (handshakeLock) { synchronized (handshakeLock) {
if (!handshaken && !handshaking && if (!handshaken && !handshaking &&
!engine.getUseClientMode() && !engine.getUseClientMode() &&
@ -1257,6 +1259,7 @@ public class SslHandler extends FrameDecoder
needsHandshake = true; needsHandshake = true;
} }
} }
}
if (needsHandshake) { if (needsHandshake) {
handshake(); handshake();
@ -1592,7 +1595,7 @@ public class SslHandler extends FrameDecoder
boolean passthrough = true; boolean passthrough = true;
try { try {
try { try {
unwrapNonAppData(ctx, e.getChannel()); unwrapNonAppData(ctx, e.getChannel(), false);
} catch (SSLException ex) { } catch (SSLException ex) {
if (logger.isDebugEnabled()) { if (logger.isDebugEnabled()) {
logger.debug("Failed to unwrap before sending a close_notify message", ex); logger.debug("Failed to unwrap before sending a close_notify message", ex);