Hi Rémy,
Thank you for your quick follow up to the issue posted by Adam. I have been
reviewing the patch from r1832519 and it appears that if a connection is
established and no bytes are sent, the socket remains open indefinitely waiting
for the handshakeReadCompletionHandler to callback. Hence it would be possible
for a malicious user to establish enough connections to match the OS file
descriptor limit and prevent Tomcat from servicing any new connections simply
by keeping the connections open and not sending any data.
In order to work around this case, I was able to include the endpoint
connection timeout in the processSNI method and pass it to the read under the
case where no bytes are available in the netInBuffer. Please see the diff below:
diff --git a/java/org/apache/tomcat/util/net/SecureNio2Channel.java
b/java/org/apache/tomcat/util/net/SecureNio2Channel.java
index b0202b706..5cf044bff 100644
--- a/java/org/apache/tomcat/util/net/SecureNio2Channel.java
+++ b/java/org/apache/tomcat/util/net/SecureNio2Channel.java
@@ -221,8 +221,9 @@ public class SecureNio2Channel extends Nio2Channel {
return 0; //we have done our initial handshake
}
+ long timeout = endpoint.getConnectionTimeout();
if (!sniComplete) {
- int sniResult = processSNI();
+ int sniResult = processSNI(timeout);
if (sniResult == 0) {
sniComplete = true;
} else {
@@ -231,7 +232,6 @@ public class SecureNio2Channel extends Nio2Channel {
}
SSLEngineResult handshake = null;
- long timeout = endpoint.getConnectionTimeout();
while (!handshakeComplete) {
switch (handshakeStatus) {
@@ -366,12 +366,13 @@ public class SecureNio2Channel extends Nio2Channel {
* present and, if it is, what host name has been requested. Based on the
* provided host name, configure the SSLEngine for this connection.
*/
- private int processSNI() throws IOException {
+ private int processSNI(long timeout) throws IOException {
// If there is no data to process, trigger a read immediately. This is
// an optimisation for the typical case so we don't create an
// SNIExtractor only to discover there is no data to process
if (netInBuffer.position() == 0) {
- sc.read(netInBuffer, socket, handshakeReadCompletionHandler);
+ sc.read(netInBuffer, Nio2Endpoint.toNio2Timeout(timeout),
+ TimeUnit.MILLISECONDS, socket,
handshakeReadCompletionHandler);
return 1;
}
I was hoping to ask for your feedback regarding whether this is a valid way to
handle the aforementioned condition. I would be happy to work on getting this
merged upstream with proper test coverage if you believe this is reasonable.
Thanks in advance,
Alex
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
