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 2015/10/26 15:33:49 UTC
svn commit: r1710608 - in /tomcat/trunk/java/org/apache/tomcat/util/net:
SecureNio2Channel.java openssl/OpenSSLEngine.java
Author: remm
Date: Mon Oct 26 14:33:49 2015
New Revision: 1710608
URL: http://svn.apache.org/viewvc?rev=1710608&view=rev
Log:
- Remove unwrap loop (that would have to be replicated across all unwrap codes in NIO and NIO2; I did still get some amount of timeouts in my testing - since the fix only covered non blocking unwrap, not the blocking unwrap).
- Fix the engine unwrap code to take into account that it is not possible to know if there's some plaintext to read unless trying it (this is what the "priming" read is about, but it needs to be repeated).
- Thanks to Mark for his most excellent debugging. No thanks to OpenSSL for the funny behavior.
- Testing is almost fine now with NIOx except I get an overflow with the byte counter, and the unwrap/read code doesn't handle overflows at all. Increasing the socket input buffer avoids this, although it is already at application buffer size, which is supposed to be enough. Will investigate to determine the best solution.
Modified:
tomcat/trunk/java/org/apache/tomcat/util/net/SecureNio2Channel.java
tomcat/trunk/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/SecureNio2Channel.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SecureNio2Channel.java?rev=1710608&r1=1710607&r2=1710608&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/SecureNio2Channel.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/SecureNio2Channel.java Mon Oct 26 14:33:49 2015
@@ -824,16 +824,7 @@ public class SecureNio2Channel extends N
throw new IOException(sm.getString("channel.nio.ssl.unwrapFail", unwrap.getStatus()));
}
// continue to unwrap as long as the input buffer has stuff
- // TODO: unwrap appears only to unwrap one TLS record at
- // a time even if there are multiple TLS records
- // in the input buffer. Therefore multiple calls
- // to unwrap are required to ensure that all TLS
- // records are decrypted and written to dst.
- // This may be a bug in tc-native or something
- // that is better handled at that level. For now
- // the '|| unwrap.getStatus() == Status.OK' is a
- // workaround.
- } while ((netInBuffer.position() != 0) || unwrap.getStatus() == Status.OK);
+ } while (netInBuffer.position() != 0);
// If everything is OK, so complete
handler.completed(Integer.valueOf(read), attach);
} catch (Exception e) {
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java?rev=1710608&r1=1710607&r2=1710608&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java Mon Oct 26 14:33:49 2015
@@ -559,23 +559,7 @@ public final class OpenSSLEngine extends
throw new SSLException(e);
}
if (bytesConsumed >= 0) {
- 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.
- if (lastPrimingReadResult <= 0) {
- // Check for OpenSSL errors caused by the priming read
- long error = SSL.getLastErrorNumber();
- if (error != SSL.SSL_ERROR_NONE) {
- String err = SSL.getErrorString(error);
- if (logger.isDebugEnabled()) {
- logger.debug(sm.getString("engine.readFromSSLFailed", Long.toString(error),
- Integer.toString(lastPrimingReadResult), err));
- }
- // There was an internal error -- shutdown
- shutdown();
- throw new SSLException(err);
- }
- }
+ primingSSLRead();
} else {
// Reset to 0 as -1 is used to signal that nothing was written and no priming read needs to be done
bytesConsumed = 0;
@@ -586,15 +570,15 @@ public final class OpenSSLEngine extends
// We first check handshakeFinished to eliminate the overhead of extra JNI call if possible.
int pendingApp = (handshakeFinished || SSL.isInInit(ssl) == 0) ? SSL.pendingReadableBytesInSSL(ssl) : 0;
int bytesProduced = 0;
+ int idx = offset;
- if (pendingApp > 0) {
+ while (pendingApp > 0) {
// Do we have enough room in dsts to write decrypted data?
if (capacity < pendingApp) {
return new SSLEngineResult(SSLEngineResult.Status.BUFFER_OVERFLOW, getHandshakeStatus(), bytesConsumed, 0);
}
// Write decrypted data to dsts buffers
- int idx = offset;
while (idx < endOffset) {
ByteBuffer dst = dsts[idx];
if (!dst.hasRemaining()) {
@@ -619,11 +603,16 @@ public final class OpenSSLEngine extends
bytesProduced += bytesRead;
pendingApp -= bytesRead;
+ capacity -= bytesRead;
if (!dst.hasRemaining()) {
idx++;
}
}
+ if (pendingApp == 0) {
+ primingSSLRead();
+ pendingApp = SSL.pendingReadableBytesInSSL(ssl);
+ }
}
// Check to see if we received a close_notify message from the peer
@@ -639,6 +628,27 @@ public final class OpenSSLEngine extends
}
}
+ private void primingSSLRead()
+ throws SSLException {
+ 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.
+ if (lastPrimingReadResult <= 0) {
+ // Check for OpenSSL errors caused by the priming read
+ long error = SSL.getLastErrorNumber();
+ if (error != SSL.SSL_ERROR_NONE) {
+ String err = SSL.getErrorString(error);
+ if (logger.isDebugEnabled()) {
+ logger.debug(sm.getString("engine.readFromSSLFailed", Long.toString(error),
+ Integer.toString(lastPrimingReadResult), err));
+ }
+ // There was an internal error -- shutdown
+ shutdown();
+ throw new SSLException(err);
+ }
+ }
+ }
+
@Override
public Runnable getDelegatedTask() {
// Currently, we do not delegate SSL computation tasks
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org