[netty#4235] Ensure OpenSslEngine.unwrap(...) / wrap(...) correctly r…

…eturn HandshakeStatus.FINISHED

Motivation:

OpenSslEngine.unwrap(...) / wrap(...) must return HandhsakeStatus.FINISHED if an unwrap or wrap finishes a handshake to behave like descripted in the SSLEngine docs.

Modifications:

- Ensure we return HandshakeStatus.FINISHED

Result:

Behave correctly.

handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java

@@ -401,16 +401,17 @@ private int readEncryptedData(final ByteBuffer dst, final int pending) {
         return bioRead;
     }
 
-    private SSLEngineResult readPendingBytesFromBIO(ByteBuffer dst, int bytesConsumed, int bytesProduced)
-            throws SSLException {
+    private SSLEngineResult readPendingBytesFromBIO(
+            ByteBuffer dst, int bytesConsumed, int bytesProduced, HandshakeStatus status) throws SSLException {
         // Check to see if the engine wrote data into the network BIO
         int pendingNet = SSL.pendingWrittenBytesInBIO(networkBIO);
         if (pendingNet > 0) {
 
             // Do we have enough room in dst to write encrypted data?
             int capacity = dst.remaining();
             if (capacity < pendingNet) {
-                return new SSLEngineResult(BUFFER_OVERFLOW, mayFinishHandshake(getHandshakeStatus(pendingNet)),
+                return new SSLEngineResult(BUFFER_OVERFLOW,
+                        mayFinishHandshake(status != FINISHED ? getHandshakeStatus(pendingNet) : status),
                                            bytesConsumed, bytesProduced);
             }
 
@@ -432,7 +433,8 @@ private SSLEngineResult readPendingBytesFromBIO(ByteBuffer dst, int bytesConsume
                 shutdown();
             }
 
-            return new SSLEngineResult(getEngineStatus(), mayFinishHandshake(getHandshakeStatus(pendingNet)),
+            return new SSLEngineResult(getEngineStatus(),
+                                       mayFinishHandshake(status != FINISHED ? getHandshakeStatus(pendingNet) : status),
                                        bytesConsumed, bytesProduced);
         }
         return null;
@@ -465,14 +467,15 @@ public synchronized SSLEngineResult wrap(
             throw new ReadOnlyBufferException();
         }
 
+        HandshakeStatus status = NOT_HANDSHAKING;
         // Prepare OpenSSL to work in server mode and receive handshake
         if (handshakeState != HandshakeState.FINISHED) {
             if (handshakeState != HandshakeState.STARTED_EXPLICITLY) {
                 // Update accepted so we know we triggered the handshake via wrap
                 handshakeState = HandshakeState.STARTED_IMPLICITLY;
             }
 
-            HandshakeStatus status = handshake();
+            status = handshake();
             if (status == NEED_UNWRAP) {
                 return NEED_UNWRAP_OK;
             }
@@ -517,7 +520,7 @@ public synchronized SSLEngineResult wrap(
                     }
                 }
 
-                SSLEngineResult pendingNetResult = readPendingBytesFromBIO(dst, bytesConsumed, bytesProduced);
+                SSLEngineResult pendingNetResult = readPendingBytesFromBIO(dst, bytesConsumed, bytesProduced, status);
                 if (pendingNetResult != null) {
                     return pendingNetResult;
                 }
@@ -526,13 +529,13 @@ public synchronized SSLEngineResult wrap(
         // We need to check if pendingWrittenBytesInBIO was checked yet, as we may not checked if the srcs was empty,
         // or only contained empty buffers.
         if (bytesConsumed == 0) {
-            SSLEngineResult pendingNetResult = readPendingBytesFromBIO(dst, 0, bytesProduced);
+            SSLEngineResult pendingNetResult = readPendingBytesFromBIO(dst, 0, bytesProduced, status);
             if (pendingNetResult != null) {
                 return pendingNetResult;
             }
         }
 
-        return newResult(bytesConsumed, bytesProduced);
+        return newResult(bytesConsumed, bytesProduced, status);
     }
 
     private void checkPendingHandshakeException() throws SSLHandshakeException {
@@ -605,14 +608,15 @@ public synchronized SSLEngineResult unwrap(
             capacity += dst.remaining();
         }
 
+        HandshakeStatus status = NOT_HANDSHAKING;
         // Prepare OpenSSL to work in server mode and receive handshake
         if (handshakeState != HandshakeState.FINISHED) {
             if (handshakeState != HandshakeState.STARTED_EXPLICITLY) {
                 // Update accepted so we know we triggered the handshake via wrap
                 handshakeState = HandshakeState.STARTED_IMPLICITLY;
             }
 
-            HandshakeStatus status = handshake();
+            status = handshake();
             if (status == NEED_WRAP) {
                 return NEED_WRAP_OK;
             }
@@ -703,7 +707,7 @@ public synchronized SSLEngineResult unwrap(
                         idx ++;
                     } else {
                         // We read everything return now.
-                        return newResult(bytesConsumed, bytesProduced);
+                        return newResult(bytesConsumed, bytesProduced, status);
                     }
                 } else {
                     int sslError = SSL.getError(ssl, bytesRead);
@@ -717,7 +721,7 @@ public synchronized SSLEngineResult unwrap(
                     case SSL.SSL_ERROR_WANT_READ:
                     case SSL.SSL_ERROR_WANT_WRITE:
                         // break to the outer loop
-                        return newResult(bytesConsumed, bytesProduced);
+                        return newResult(bytesConsumed, bytesProduced, status);
                     default:
                         // Everything else is considered as error so shutdown and throw an exceptions
                         shutdownWithError("SSL_read");
@@ -738,15 +742,16 @@ public synchronized SSLEngineResult unwrap(
         if (pendingAppData() > 0) {
             // We filled all buffers but there is still some data pending in the BIO buffer, return BUFFER_OVERFLOW.
             return new SSLEngineResult(
-                    BUFFER_OVERFLOW, mayFinishHandshake(getHandshakeStatus()), bytesConsumed, bytesProduced);
+                    BUFFER_OVERFLOW, mayFinishHandshake(status != FINISHED ? getHandshakeStatus(): status),
+                    bytesConsumed, bytesProduced);
         }
 
         // Check to see if we received a close_notify message from the peer.
         if (!receivedShutdown && (SSL.getShutdown(ssl) & SSL.SSL_RECEIVED_SHUTDOWN) == SSL.SSL_RECEIVED_SHUTDOWN) {
             closeAll();
         }
 
-        return newResult(bytesConsumed, bytesProduced);
+        return newResult(bytesConsumed, bytesProduced, status);
     }
 
     private int pendingAppData() {
@@ -755,9 +760,11 @@ private int pendingAppData() {
         return handshakeState == HandshakeState.FINISHED ? SSL.pendingReadableBytesInSSL(ssl) : 0;
     }
 
-    private SSLEngineResult newResult(int bytesConsumed, int bytesProduced) throws SSLException {
+    private SSLEngineResult newResult(
+            int bytesConsumed, int bytesProduced, HandshakeStatus status) throws SSLException {
        return new SSLEngineResult(
-               getEngineStatus(), mayFinishHandshake(getHandshakeStatus()), bytesConsumed, bytesProduced);
+               getEngineStatus(), mayFinishHandshake(status != FINISHED ? getHandshakeStatus() : status)
+               , bytesConsumed, bytesProduced);
     }
 
     private void closeAll() throws SSLException {