You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2019/03/09 20:47:12 UTC
[tomcat] branch 8.5.x updated: Avoid overflow with OpenSSL engine
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 <re...@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org