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