Prevent unexpected TLS handshake failures caused by errors during a previous handshake that were not correctly cleaned-up when using the NIO or NIO2 connector with the OpenSSLImplementation.

markt-asf · markt-asf · commit ebab25a3576a · 2018-04-17T13:52:22.000Z
git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1829366 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
@@ -630,6 +630,7 @@ private int pendingReadableBytesInSSL()
         // NOTE: Calling a fake read is necessary before calling pendingReadableBytesInSSL because
         // SSL_pending will return 0 if OpenSSL has not started the current TLS record
         // See https://www.openssl.org/docs/manmaster/ssl/SSL_pending.html
+        clearLastError();
         int lastPrimingReadResult = SSL.readFromSSL(ssl, EMPTY_ADDR, 0); // priming read
         // check if SSL_read returned <= 0. In this case we need to check the error and see if it was something
         // fatal.
@@ -884,6 +885,7 @@ private void beginHandshakeImplicitly() throws SSLException {
 
     private void handshake() throws SSLException {
         currentHandshake = SSL.getHandshakeCount(ssl);
+        clearLastError();
         int code = SSL.doHandshake(ssl);
         if (code <= 0) {
             checkLastError();
@@ -902,6 +904,7 @@ private void handshake() throws SSLException {
     }
 
     private synchronized void renegotiate() throws SSLException {
+        clearLastError();
         int code = SSL.renegotiate(ssl);
         if (code <= 0) {
             checkLastError();
@@ -932,6 +935,20 @@ private void checkLastError() throws SSLException {
         }
     }
 
+
+    /*
+     * Many calls to SSL methods do not check the last error. Those that do
+     * check the last error need to ensure that any previously ignored error is
+     * cleared prior to the method call else errors may be falsely reported.
+     *
+     * TODO: Check last error after every call to an SSL method and respond
+     *       appropriately.
+     */
+    private void clearLastError() {
+        SSL.getLastErrorNumber();
+    }
+
+
     private static long memoryAddress(ByteBuffer buf) {
         return Buffer.address(buf);
     }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
@@ -77,6 +77,11 @@
         increase in size to one more than the configured limit. Patch provided
         by usc. (markt)
       </fix>
+      <fix>
+        Prevent unexpected TLS handshake failures caused by errors during a
+        previous handshake that were not correctly cleaned-up when using the NIO
+        or NIO2 connector with the <code>OpenSSLImplementation</code>. (markt)
+      </fix>
     </changelog>
   </subsection>
 </section>
