From ed54ab034dcc444dffbcaaa5675c39abc0cfbff3 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Tue, 15 May 2018 19:44:02 +0200
Subject: [PATCH] Correctly clear the error stack in all cases when using
 ReferenceCountedOpenSslEngine. (#7941)

Motivation:

We missed to correctly clear the error stack in one case when using the ReferenceCountedOpenSslEngine. Because of this it was possible to pick up an error on an unrelated operation.

Modifications:

- Correctly clear the stack
- Add verification of empty error stack to tests.

Result:

Not possible to observe unrelated errors.
---
 .../netty/handler/ssl/ReferenceCountedOpenSslEngine.java | 9 +++++----
 .../java/io/netty/handler/ssl/OpenSslEngineTest.java     | 8 ++++++++
 2 files changed, 13 insertions(+), 4 deletions(-)

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 c0eb141a5f..aa03cf0a27 100644
--- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java
+++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java
@@ -1093,8 +1093,6 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
     }
 
     private SSLEngineResult sslReadErrorResult(int err, int bytesConsumed, int bytesProduced) throws SSLException {
-        String errStr = SSL.getErrorString(err);
-
         // Check if we have a pending handshakeException and if so see if we need to consume all pending data from the
         // BIO first or can just shutdown and throw it now.
         // This is needed so we ensure close_notify etc is correctly send to the remote peer.
@@ -1103,11 +1101,14 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
             if (handshakeException == null && handshakeState != HandshakeState.FINISHED) {
                 // we seems to have data left that needs to be transfered and so the user needs
                 // call wrap(...). Store the error so we can pick it up later.
-                handshakeException = new SSLHandshakeException(errStr);
+                handshakeException = new SSLHandshakeException(SSL.getErrorString(err));
             }
+            // We need to clear all errors so we not pick up anything that was left on the stack on the next
+            // operation. Note that shutdownWithError(...) will cleanup the stack as well so its only needed here.
+            SSL.clearError();
             return new SSLEngineResult(OK, NEED_WRAP, bytesConsumed, bytesProduced);
         }
-        throw shutdownWithError("SSL_read", errStr);
+        throw shutdownWithError("SSL_read", SSL.getErrorString(err));
     }
 
     private void closeAll() throws SSLException {
diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java
index 9972b2d5f4..f193468d1b 100644
--- a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java
+++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java
@@ -21,7 +21,9 @@ import io.netty.handler.ssl.ApplicationProtocolConfig.SelectedListenerFailureBeh
 import io.netty.handler.ssl.ApplicationProtocolConfig.SelectorFailureBehavior;
 import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
 import io.netty.handler.ssl.util.SelfSignedCertificate;
+import io.netty.internal.tcnative.SSL;
 import io.netty.util.internal.PlatformDependent;
+import org.junit.Assert;
 import org.junit.Assume;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -82,6 +84,12 @@ public class OpenSslEngineTest extends SSLEngineTest {
         assumeTrue(OpenSsl.isAvailable());
     }
 
+    @Override
+    public void tearDown() throws InterruptedException {
+        super.tearDown();
+        Assert.assertEquals("SSL error stack not correctly consumed", 0, SSL.getLastErrorNumber());
+    }
+
     @Override
     @Test
     public void testMutualAuthInvalidIntermediateCASucceedWithOptionalClientAuth() throws Exception {