Repository: netty/netty
Issue: java.lang.NullPointerException: ssl at ReferenceCountedOpenSslEngine.rejectRemoteInitiatedRenegotiation
Labels: defect

### Expected behavior
No NPE

### Actual behavior
java.lang.NullPointerException: ssl (see stracktrace below) at ReferenceCountedOpenSslEngine.rejectRemoteInitiatedRenegotiation

### Steps to reproduce
Happens randomly

### Minimal yet complete reproducer code (or URL to code)

### Netty version
4.1.13.Final with tcnative 2.0.5.Final and OpenSSL 1.0.2l  25 May 2017

### JVM version (e.g. `java -version`)
java version "1.8.0_121"
Java(TM) SE Runtime Environment (build 1.8.0_121-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.121-b13, mixed mode)

### OS version (e.g. `uname -a`)
Darwin MacBook-Pro.local 14.5.0 Darwin Kernel Version 14.5.0: Sun Jun  4 21:40:08 PDT 2017; root:xnu-2782.70.3~1/RELEASE_X86_64 x86_64

<pre>
io.netty.handler.codec.DecoderException: java.lang.NullPointerException: ssl
	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:459) ~[netty-codec-4.1.13.Final.jar:4.1.13.Final]
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:265) ~[netty-codec-4.1.13.Final.jar:4.1.13.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1334) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:926) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
	at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:134) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
	at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:644) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
	at io.netty.channel.nio.NioEventLoop.processSelectedKeysPlain(NioEventLoop.java:544) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
	at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:498) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:458) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
	at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:858) [netty-common-4.1.13.Final.jar:4.1.13.Final]
	at java.lang.Thread.run(Thread.java:745) [?:1.8.0_121]
Caused by: java.lang.NullPointerException: ssl
	at io.netty.internal.tcnative.SSL.getHandshakeCount(Native Method) ~[?:?]
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.rejectRemoteInitiatedRenegotiation(ReferenceCountedOpenSslEngine.java:985) ~[?:?]
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:948) ~[?:?]
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:994) ~[?:?]
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:1037) ~[?:?]
	at io.netty.handler.ssl.SslHandler$SslEngineType$1.unwrap(SslHandler.java:206) ~[?:?]
	at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1162) ~[?:?]
	at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1084) ~[?:?]
	at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:489) ~[?:?]
	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:428) ~[?:?]
</pre>

Issue comments:
Will have a look tomorrow

> Am 31.10.2017 um 17:07 schrieb Search Guard <notifications@github.com>:
> 
> Expected behavior
> 
> No NPE
> 
> Actual behavior
> 
> java.lang.NullPointerException: ssl (see stracktrace below) at ReferenceCountedOpenSslEngine.rejectRemoteInitiatedRenegotiation
> 
> Steps to reproduce
> 
> Happens randomly
> 
> Minimal yet complete reproducer code (or URL to code)
> 
> Netty version
> 
> 4.1.13.Final with tcnative 2.0.5.Final and OpenSSL 1.0.2l 25 May 2017
> 
> JVM version (e.g. java -version)
> 
> java version "1.8.0_121"
> Java(TM) SE Runtime Environment (build 1.8.0_121-b13)
> Java HotSpot(TM) 64-Bit Server VM (build 25.121-b13, mixed mode)
> 
> OS version (e.g. uname -a)
> 
> Darwin MacBook-Pro.local 14.5.0 Darwin Kernel Version 14.5.0: Sun Jun 4 21:40:08 PDT 2017; root:xnu-2782.70.3~1/RELEASE_X86_64 x86_64
> 
> io.netty.handler.codec.DecoderException: java.lang.NullPointerException: ssl
> 	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:459) ~[netty-codec-4.1.13.Final.jar:4.1.13.Final]
> 	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:265) ~[netty-codec-4.1.13.Final.jar:4.1.13.Final]
> 	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
> 	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
> 	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
> 	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1334) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
> 	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
> 	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
> 	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:926) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
> 	at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:134) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
> 	at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:644) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
> 	at io.netty.channel.nio.NioEventLoop.processSelectedKeysPlain(NioEventLoop.java:544) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
> 	at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:498) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
> 	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:458) [netty-transport-4.1.13.Final.jar:4.1.13.Final]
> 	at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:858) [netty-common-4.1.13.Final.jar:4.1.13.Final]
> 	at java.lang.Thread.run(Thread.java:745) [?:1.8.0_121]
> Caused by: java.lang.NullPointerException: ssl
> 	at io.netty.internal.tcnative.SSL.getHandshakeCount(Native Method) ~[?:?]
> 	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.rejectRemoteInitiatedRenegotiation(ReferenceCountedOpenSslEngine.java:985) ~[?:?]
> 	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:948) ~[?:?]
> 	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:994) ~[?:?]
> 	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:1037) ~[?:?]
> 	at io.netty.handler.ssl.SslHandler$SslEngineType$1.unwrap(SslHandler.java:206) ~[?:?]
> 	at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1162) ~[?:?]
> 	at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1084) ~[?:?]
> 	at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:489) ~[?:?]
> 	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:428) ~[?:?]
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 

@floragunncom thanks for reporting. Fixed in https://github.com/netty/netty/pull/7357
Fixed by https://github.com/netty/netty/pull/7357
@normanmaurer must be close to release day? 😎 
Yes

> Am 03.11.2017 um 07:57 schrieb Johno Crawford <notifications@github.com>:
> 
> @normanmaurer must be close to release day? 😎
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 


Pull request review comments:

Patch that resolved the issue:
diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java
index 0c4bf5c2d47..5b2848e2373 100644
--- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java
+++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java
@@ -1115,7 +1115,10 @@ private void closeAll() throws SSLException {
     }
 
     private void rejectRemoteInitiatedRenegotiation() throws SSLHandshakeException {
-        if (rejectRemoteInitiatedRenegotiation && SSL.getHandshakeCount(ssl) > 1) {
+        // As rejectRemoteInitiatedRenegotiation() is called in a finally block we also need to check if we shutdown
+        // the engine before as otherwise SSL.getHandshakeCount(ssl) will throw an NPE if the passed in ssl is 0.
+        // See https://github.com/netty/netty/issues/7353
+        if (rejectRemoteInitiatedRenegotiation && !isDestroyed() && SSL.getHandshakeCount(ssl) > 1) {
             // TODO: In future versions me may also want to send a fatal_alert to the client and so notify it
             // that the renegotiation failed.
             shutdown();


Test patch: