Work around the JVM crash that occurs when cipher suite uses GCM

Motivation:

For an unknown reason, JVM of JDK8 crashes intermittently when
SslHandler feeds a direct buffer to SSLEngine.unwrap() *and* the current
cipher suite has GCM (Galois/Counter Mode) enabled.

Modifications:

Convert the inbound network buffer to a heap buffer when the current
cipher suite is using GCM.

Result:

JVM does not crash anymore.
This commit is contained in:
Trustin Lee 2014-05-19 11:44:50 +09:00
parent a116d1205c
commit a6541e40d4
2 changed files with 62 additions and 7 deletions

View File

@ -69,14 +69,18 @@ public abstract class JdkSslContext extends SslContext {
addIfSupported( addIfSupported(
supportedCiphers, ciphers, supportedCiphers, ciphers,
// XXX: Make sure to sync this list with OpenSslEngineFactory. // XXX: Make sure to sync this list with OpenSslEngineFactory.
"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", // since JDK 8 // GCM (Galois/Counter Mode) requires JDK 8.
"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
"TLS_ECDHE_RSA_WITH_RC4_128_SHA", "TLS_ECDHE_RSA_WITH_RC4_128_SHA",
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
// AES256 requires JCE unlimited strength jurisdiction policy files.
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
"TLS_RSA_WITH_AES_128_GCM_SHA256", // since JDK 8 // GCM (Galois/Counter Mode) requires JDK 8.
"TLS_RSA_WITH_AES_128_GCM_SHA256",
"SSL_RSA_WITH_RC4_128_SHA", "SSL_RSA_WITH_RC4_128_SHA",
"SSL_RSA_WITH_RC4_128_MD5", "SSL_RSA_WITH_RC4_128_MD5",
"TLS_RSA_WITH_AES_128_CBC_SHA", "TLS_RSA_WITH_AES_128_CBC_SHA",
// AES256 requires JCE unlimited strength jurisdiction policy files.
"TLS_RSA_WITH_AES_256_CBC_SHA", "TLS_RSA_WITH_AES_256_CBC_SHA",
"SSL_RSA_WITH_DES_CBC_SHA"); "SSL_RSA_WITH_DES_CBC_SHA");

View File

@ -180,6 +180,13 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
private final SSLEngine engine; private final SSLEngine engine;
private final int maxPacketBufferSize; private final int maxPacketBufferSize;
private final Executor delegatedTaskExecutor; private final Executor delegatedTaskExecutor;
// BEGIN Platform-dependent flags
/**
* {@code trus} if and only if {@link SSLEngine} expects a direct buffer.
*/
private final boolean wantsDirectBuffer;
/** /**
* {@code true} if and only if {@link SSLEngine#wrap(ByteBuffer, ByteBuffer)} requires the output buffer * {@code true} if and only if {@link SSLEngine#wrap(ByteBuffer, ByteBuffer)} requires the output buffer
* to be always as large as {@link #maxPacketBufferSize} even if the input buffer contains small amount of data. * to be always as large as {@link #maxPacketBufferSize} even if the input buffer contains small amount of data.
@ -187,7 +194,15 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
* If this flag is {@code false}, we allocate a smaller output buffer. * If this flag is {@code false}, we allocate a smaller output buffer.
* </p> * </p>
*/ */
private final boolean needsLargeOutNetBuf; private final boolean wantsLargeOutboundNetworkBuffer;
/**
* {@code true} if and only if {@link SSLEngine#unwrap(ByteBuffer, ByteBuffer)} expects a heap buffer rather than
* a direct buffer. For an unknown reason, JDK8 SSLEngine causes JVM to crash when its cipher suite uses Galois
* Counter Mode (GCM).
*/
private boolean wantsInboundHeapBuffer;
// END Platform-dependent flags
private final boolean startTls; private final boolean startTls;
private boolean sentFirstMessage; private boolean sentFirstMessage;
@ -251,7 +266,9 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
this.delegatedTaskExecutor = delegatedTaskExecutor; this.delegatedTaskExecutor = delegatedTaskExecutor;
this.startTls = startTls; this.startTls = startTls;
maxPacketBufferSize = engine.getSession().getPacketBufferSize(); maxPacketBufferSize = engine.getSession().getPacketBufferSize();
needsLargeOutNetBuf = !(engine instanceof OpenSslEngine);
wantsDirectBuffer = engine instanceof OpenSslEngine;
wantsLargeOutboundNetworkBuffer = !(engine instanceof OpenSslEngine);
} }
public long getHandshakeTimeoutMillis() { public long getHandshakeTimeoutMillis() {
@ -578,6 +595,12 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
private SSLEngineResult wrap(SSLEngine engine, ByteBuf in, ByteBuf out) throws SSLException { private SSLEngineResult wrap(SSLEngine engine, ByteBuf in, ByteBuf out) throws SSLException {
ByteBuffer in0 = in.nioBuffer(); ByteBuffer in0 = in.nioBuffer();
if (!in0.isDirect()) {
ByteBuffer newIn0 = ByteBuffer.allocateDirect(in0.remaining());
newIn0.put(in0).flip();
in0 = newIn0;
}
for (;;) { for (;;) {
ByteBuffer out0 = out.nioBuffer(out.writerIndex(), out.writableBytes()); ByteBuffer out0 = out.nioBuffer(out.writerIndex(), out.writableBytes());
SSLEngineResult result = engine.wrap(in0, out0); SSLEngineResult result = engine.wrap(in0, out0);
@ -883,6 +906,20 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
private void unwrap( private void unwrap(
ChannelHandlerContext ctx, ByteBuffer packet, int initialOutAppBufCapacity) throws SSLException { ChannelHandlerContext ctx, ByteBuffer packet, int initialOutAppBufCapacity) throws SSLException {
// If SSLEngine expects a heap buffer for unwrapping, do the conversion.
final ByteBuffer oldPacket;
final ByteBuf newPacket;
final int oldPos = packet.position();
if (wantsInboundHeapBuffer && packet.isDirect()) {
newPacket = ctx.alloc().heapBuffer(packet.limit() - oldPos);
newPacket.writeBytes(packet);
oldPacket = packet;
packet = newPacket.nioBuffer();
} else {
oldPacket = null;
newPacket = null;
}
boolean wrapLater = false; boolean wrapLater = false;
ByteBuf decodeOut = allocate(ctx, initialOutAppBufCapacity); ByteBuf decodeOut = allocate(ctx, initialOutAppBufCapacity);
try { try {
@ -942,6 +979,13 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
setHandshakeFailure(e); setHandshakeFailure(e);
throw e; throw e;
} finally { } finally {
// If we converted packet into a heap buffer at the beginning of this method,
// we should synchronize the position of the original buffer.
if (newPacket != null) {
oldPacket.position(oldPos + packet.position());
newPacket.release();
}
if (decodeOut.isReadable()) { if (decodeOut.isReadable()) {
ctx.fireChannelRead(decodeOut); ctx.fireChannelRead(decodeOut);
} else { } else {
@ -1055,6 +1099,12 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
* Notify all the handshake futures about the successfully handshake * Notify all the handshake futures about the successfully handshake
*/ */
private void setHandshakeSuccess() { private void setHandshakeSuccess() {
// Work around the JVM crash which occurs when a cipher suite with GCM enabled.
final String cipherSuite = String.valueOf(engine.getSession().getCipherSuite());
if (!wantsDirectBuffer && (cipherSuite.contains("_GCM_") || cipherSuite.contains("-GCM-"))) {
wantsInboundHeapBuffer = true;
}
if (handshakePromise.trySuccess(ctx.channel())) { if (handshakePromise.trySuccess(ctx.channel())) {
if (logger.isDebugEnabled()) { if (logger.isDebugEnabled()) {
logger.debug(ctx.channel() + " HANDSHAKEN: " + engine.getSession().getCipherSuite()); logger.debug(ctx.channel() + " HANDSHAKEN: " + engine.getSession().getCipherSuite());
@ -1187,6 +1237,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
} }
ctx.fireChannelActive(); ctx.fireChannelActive();
} }
private void safeClose( private void safeClose(
final ChannelHandlerContext ctx, ChannelFuture flushFuture, final ChannelHandlerContext ctx, ChannelFuture flushFuture,
final ChannelPromise promise) { final ChannelPromise promise) {
@ -1230,9 +1281,9 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
* Always prefer a direct buffer when it's pooled, so that we reduce the number of memory copies * Always prefer a direct buffer when it's pooled, so that we reduce the number of memory copies
* in {@link OpenSslEngine}. * in {@link OpenSslEngine}.
*/ */
private static ByteBuf allocate(ChannelHandlerContext ctx, int capacity) { private ByteBuf allocate(ChannelHandlerContext ctx, int capacity) {
ByteBufAllocator alloc = ctx.alloc(); ByteBufAllocator alloc = ctx.alloc();
if (alloc.isDirectBufferPooled()) { if (wantsDirectBuffer) {
return alloc.directBuffer(capacity); return alloc.directBuffer(capacity);
} else { } else {
return alloc.buffer(capacity); return alloc.buffer(capacity);
@ -1244,7 +1295,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
* the specified amount of pending bytes. * the specified amount of pending bytes.
*/ */
private ByteBuf allocateOutNetBuf(ChannelHandlerContext ctx, int pendingBytes) { private ByteBuf allocateOutNetBuf(ChannelHandlerContext ctx, int pendingBytes) {
if (needsLargeOutNetBuf) { if (wantsLargeOutboundNetworkBuffer) {
return allocate(ctx, maxPacketBufferSize); return allocate(ctx, maxPacketBufferSize);
} else { } else {
return allocate(ctx, Math.min( return allocate(ctx, Math.min(