This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/8.5.x by this push:
     new ef099ae  Avoid overflow with OpenSSL engine
ef099ae is described below

commit ef099aeb3a3d6e1c6664ada940e3092ed7348606
Author: remm <r...@apache.org>
AuthorDate: Sat Mar 9 21:37:11 2019 +0100

    Avoid overflow with OpenSSL engine
    
    Avoid many overflow situations with OpenSSL engine, overflow will now
    only occur if the destination buffers are all full when unwrap is
    called. Since it has an internal buffer (the bio in the native code), it
    doesn't have to write all the decrypted data available and can hold them
    until the next unwrap call. Also improve the overflow handling in the
    NIO2 code to give the right bytes produced and avoid some useless
    processing.
    Rebase on the read(ByteBuffer[]) code from 9.
    # Conflicts:
    #   java/org/apache/tomcat/util/net/SecureNio2Channel.java
---
 .../apache/tomcat/util/net/SecureNio2Channel.java  | 41 ++++++++++++++++++++--
 .../tomcat/util/net/openssl/OpenSSLEngine.java     |  2 +-
 webapps/docs/changelog.xml                         |  4 +++
 3 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/SecureNio2Channel.java 
b/java/org/apache/tomcat/util/net/SecureNio2Channel.java
index 4ae2509..6eae95d 100644
--- a/java/org/apache/tomcat/util/net/SecureNio2Channel.java
+++ b/java/org/apache/tomcat/util/net/SecureNio2Channel.java
@@ -1008,16 +1008,28 @@ public class SecureNio2Channel extends Nio2Channel  {
                         long read = 0;
                         //the SSL engine result
                         SSLEngineResult unwrap;
+                        ByteBuffer[] dsts2 = dsts;
+                        int length2 = length;
+                        boolean processOverflow = false;
                         do {
+                            boolean useOverflow = false;
+                            if (processOverflow) {
+                                useOverflow = true;
+                            }
+                            processOverflow = false;
                             //prepare the buffer
                             netInBuffer.flip();
                             //unwrap the data
-                            unwrap = sslEngine.unwrap(netInBuffer, dsts, 
offset, length);
+                            unwrap = sslEngine.unwrap(netInBuffer, dsts2, 
offset, length2);
                             //compact the buffer
                             netInBuffer.compact();
                             if (unwrap.getStatus() == Status.OK || 
unwrap.getStatus() == Status.BUFFER_UNDERFLOW) {
                                 //we did receive some data, add it to our total
                                 read += unwrap.bytesProduced();
+                                if (useOverflow) {
+                                    // Remove the data read into the overflow 
buffer
+                                    read -= dsts2[dsts.length].position();
+                                }
                                 //perform any tasks if needed
                                 if (unwrap.getHandshakeStatus() == 
HandshakeStatus.NEED_TASK)
                                     tasks();
@@ -1034,13 +1046,36 @@ public class SecureNio2Channel extends Nio2Channel  {
                                 //buffer overflow can happen, if we have read 
data, then
                                 //empty out the dst buffer before we do 
another read
                                 break;
-                            } else {
+                            } else if (unwrap.getStatus() == 
Status.BUFFER_OVERFLOW) {
                                 //here we should trap BUFFER_OVERFLOW and call 
expand on the buffer
                                 //for now, throw an exception, as we 
initialized the buffers
                                 //in the constructor
+                                ByteBuffer readBuffer = 
getBufHandler().getReadBuffer();
+                                boolean found = false;
+                                for (ByteBuffer buffer : dsts2) {
+                                    if (buffer == readBuffer) {
+                                        found = true;
+                                    }
+                                }
+                                if (found) {
+                                    throw new 
IOException(sm.getString("channel.nio.ssl.unwrapFail", unwrap.getStatus()));
+                                } else {
+                                    // Add the main read buffer in the 
destinations and try again
+                                    dsts2 = new ByteBuffer[dsts.length + 1];
+                                    for (int i = 0; i < dsts.length; i++) {
+                                        dsts2[i] = dsts[i];
+                                    }
+                                    dsts2[dsts.length] = readBuffer;
+                                    length2 = length + 1;
+                                    
getBufHandler().configureReadBufferForWrite();
+                                    processOverflow = true;
+                                }
+                            } else if (unwrap.getStatus() == Status.CLOSED) {
+                                break;
+                            } else {
                                 throw new 
IOException(sm.getString("channel.nio.ssl.unwrapFail", unwrap.getStatus()));
                             }
-                        } while (netInBuffer.position() != 0); //continue to 
unwrapping as long as the input buffer has stuff
+                        } while ((netInBuffer.position() != 0) || 
processOverflow); //continue to unwrapping as long as the input buffer has stuff
                         int capacity = 0;
                         final int endOffset = offset + length;
                         for (int i = offset; i < endOffset; i++) {
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java 
b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
index 5c085fb..241a35d 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
@@ -572,7 +572,7 @@ public final class OpenSSLEngine extends SSLEngine 
implements SSLUtil.ProtocolIn
         int bytesProduced = 0;
         int idx = offset;
         // Do we have enough room in dsts to write decrypted data?
-        if (capacity < pendingApp) {
+        if (capacity == 0) {
             return new SSLEngineResult(SSLEngineResult.Status.BUFFER_OVERFLOW, 
getHandshakeStatus(), written, 0);
         }
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3b2b00d..39df098 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -183,6 +183,10 @@
         Verify HTTP/2 stream is still writable before assuming a timeout
         occurred. (remm)
       </fix>
+      <fix>
+        Avoid some overflow cases with OpenSSL to improve efficiency, as the
+        OpenSSL engine has an internal buffer. (remm)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="WebSocket">


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to