Skip to content

Commit

Permalink
Fix event loop hang in SniHandler (#9933)
Browse files Browse the repository at this point in the history
Motivation

A bug was introduced in #9806 which looks likely to be the cause of
#9919. SniHandler will enter an infinite loop if an SSL record is
received with SSL major version byte != 3 (i.e. something other than TLS
or SSL3.0)

Modifications

- Follow default path as intended  for majorVersion != 3 in
AbstractSniHandler#decode(...)
- Add unit test to reproduce the hang

Result

Fixes #9919
  • Loading branch information
njhill authored and normanmaurer committed Jan 10, 2020
1 parent b82258b commit 41c47b4
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,9 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) t
select(ctx, extractSniHostname(handshakeBuffer, 0, handshakeLength));
return;
}
break;
}
break;
// fall-through
default:
// not tls, ssl or application data, do not try sni
select(ctx, null);
Expand Down
39 changes: 38 additions & 1 deletion handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ public void testFallbackToDefaultContext() throws Exception {
ch.writeInbound(Unpooled.wrappedBuffer(message));
// TODO(scott): This should fail because the engine should reject zero length records during handshake.
// See https://github.com/netty/netty/issues/6348.
// fail();
fail();
} catch (Exception e) {
// expected
}
Expand All @@ -344,6 +344,43 @@ public void testFallbackToDefaultContext() throws Exception {
}
}

@Test(timeout = 10000)
public void testMajorVersionNot3() throws Exception {
SslContext nettyContext = makeSslContext(provider, false);

try {
DomainNameMapping<SslContext> mapping = new DomainNameMappingBuilder<SslContext>(nettyContext).build();

SniHandler handler = new SniHandler(mapping);
EmbeddedChannel ch = new EmbeddedChannel(handler);

// invalid
byte[] message = {22, 2, 0, 0, 0};
try {
// Push the handshake message.
ch.writeInbound(Unpooled.wrappedBuffer(message));
fail();
} catch (Exception e) {
// expected
}

ch.close();

// When the channel is closed the SslHandler will write an empty buffer to the channel.
ByteBuf buf = ch.readOutbound();
if (buf != null) {
assertFalse(buf.isReadable());
buf.release();
}

assertThat(ch.finish(), is(false));
assertThat(handler.hostname(), nullValue());
assertThat(handler.sslContext(), is(nettyContext));
} finally {
releaseAll(nettyContext);
}
}

@Test
public void testSniWithApnHandler() throws Exception {
SslContext nettyContext = makeSslContext(provider, true);
Expand Down

0 comments on commit 41c47b4

Please sign in to comment.