This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.0.x by this push: new 2b9d155 Fix BZ 65776. Reduce false positives detecting duplicate accept bug 2b9d155 is described below commit 2b9d1552ca23fbd197cec5803561ecb7f10649e0 Author: Mark Thomas <ma...@apache.org> AuthorDate: Thu Feb 10 16:02:05 2022 +0000 Fix BZ 65776. Reduce false positives detecting duplicate accept bug https://bz.apache.org/bugzilla/show_bug.cgi?id=65776 --- java/org/apache/tomcat/util/net/AprEndpoint.java | 22 ++++++++++++++++------ java/org/apache/tomcat/util/net/Nio2Endpoint.java | 18 +++++++++++++----- java/org/apache/tomcat/util/net/NioEndpoint.java | 18 +++++++++++++----- webapps/docs/changelog.xml | 4 ++++ 4 files changed, 46 insertions(+), 16 deletions(-) diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java index ef6dcdf..ea025f4 100644 --- a/java/org/apache/tomcat/util/net/AprEndpoint.java +++ b/java/org/apache/tomcat/util/net/AprEndpoint.java @@ -62,6 +62,7 @@ import org.apache.tomcat.jni.Status; import org.apache.tomcat.util.ExceptionUtils; import org.apache.tomcat.util.buf.ByteBufferUtils; import org.apache.tomcat.util.collections.SynchronizedStack; +import org.apache.tomcat.util.compat.JrePlatform; import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState; import org.apache.tomcat.util.net.Acceptor.AcceptorState; import org.apache.tomcat.util.net.openssl.OpenSSLContext; @@ -118,6 +119,8 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB private int previousAcceptedPort = -1; private String previousAcceptedAddress = null; + private long previouspreviousAcceptedSocketNanoTime = 0; + // ------------------------------------------------------------ Constructor @@ -129,8 +132,8 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB setUseAsyncIO(false); } - // ------------------------------------------------------------- Properties + // ------------------------------------------------------------- Properties /** * Defer accept. @@ -810,13 +813,20 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB // Do the duplicate accept check here rather than in serverSocketaccept() // so we can cache the results in the SocketWrapper AprSocketWrapper wrapper = new AprSocketWrapper(socket, this); - if (wrapper.getRemotePort() == previousAcceptedPort) { - if (wrapper.getRemoteAddr().equals(previousAcceptedAddress)) { - throw new IOException(sm.getString("endpoint.err.duplicateAccept")); + // Bug does not affect Windows. Skip the check on that platform. + if (!JrePlatform.IS_WINDOWS) { + long currentNanoTime = System.nanoTime(); + if (wrapper.getRemotePort() == previousAcceptedPort) { + if (wrapper.getRemoteAddr().equals(previousAcceptedAddress)) { + if (currentNanoTime - previouspreviousAcceptedSocketNanoTime < 1000) { + throw new IOException(sm.getString("endpoint.err.duplicateAccept")); + } + } } + previousAcceptedPort = wrapper.getRemotePort(); + previousAcceptedAddress = wrapper.getRemoteAddr(); + previouspreviousAcceptedSocketNanoTime = currentNanoTime; } - previousAcceptedPort = wrapper.getRemotePort(); - previousAcceptedAddress = wrapper.getRemoteAddr(); connections.put(socket, wrapper); wrapper.setKeepAliveLeft(getMaxKeepAliveRequests()); diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java b/java/org/apache/tomcat/util/net/Nio2Endpoint.java index 00719a6..23b9434 100644 --- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java +++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java @@ -44,6 +44,7 @@ import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.util.ExceptionUtils; import org.apache.tomcat.util.collections.SynchronizedStack; +import org.apache.tomcat.util.compat.JrePlatform; import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState; import org.apache.tomcat.util.net.Acceptor.AcceptorState; import org.apache.tomcat.util.net.jsse.JSSESupport; @@ -85,9 +86,10 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS private SynchronizedStack<Nio2Channel> nioChannels; private SocketAddress previousAcceptedSocketRemoteAddress = null; + private long previouspreviousAcceptedSocketNanoTime = 0; - // --------------------------------------------------------- Public Methods + // --------------------------------------------------------- Public Methods /** * Number of keep-alive sockets. @@ -362,11 +364,17 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS protected AsynchronousSocketChannel serverSocketAccept() throws Exception { AsynchronousSocketChannel result = serverSock.accept().get(); - SocketAddress currentRemoteAddress = result.getRemoteAddress(); - if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress)) { - throw new IOException(sm.getString("endpoint.err.duplicateAccept")); + // Bug does not affect Windows. Skip the check on that platform. + if (!JrePlatform.IS_WINDOWS) { + SocketAddress currentRemoteAddress = result.getRemoteAddress(); + long currentNanoTime = System.nanoTime(); + if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) && + currentNanoTime - previouspreviousAcceptedSocketNanoTime < 1000) { + throw new IOException(sm.getString("endpoint.err.duplicateAccept")); + } + previousAcceptedSocketRemoteAddress = currentRemoteAddress; + previouspreviousAcceptedSocketNanoTime = currentNanoTime; } - previousAcceptedSocketRemoteAddress = currentRemoteAddress; return result; } diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java index 7ef9f4a..f350d04 100644 --- a/java/org/apache/tomcat/util/net/NioEndpoint.java +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java @@ -58,6 +58,7 @@ import org.apache.tomcat.util.ExceptionUtils; import org.apache.tomcat.util.collections.SynchronizedQueue; import org.apache.tomcat.util.collections.SynchronizedStack; import org.apache.tomcat.util.compat.JreCompat; +import org.apache.tomcat.util.compat.JrePlatform; import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState; import org.apache.tomcat.util.net.Acceptor.AcceptorState; import org.apache.tomcat.util.net.jsse.JSSESupport; @@ -109,11 +110,11 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> private SynchronizedStack<NioChannel> nioChannels; private SocketAddress previousAcceptedSocketRemoteAddress = null; + private long previouspreviousAcceptedSocketNanoTime = 0; // ------------------------------------------------------------- Properties - /** * Use System.inheritableChannel to obtain channel from stdin/stdout. */ @@ -516,11 +517,18 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> @Override protected SocketChannel serverSocketAccept() throws Exception { SocketChannel result = serverSock.accept(); - SocketAddress currentRemoteAddress = result.getRemoteAddress(); - if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress)) { - throw new IOException(sm.getString("endpoint.err.duplicateAccept")); + + // Bug does not affect Windows. Skip the check on that platform. + if (!JrePlatform.IS_WINDOWS) { + SocketAddress currentRemoteAddress = result.getRemoteAddress(); + long currentNanoTime = System.nanoTime(); + if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress) && + currentNanoTime - previouspreviousAcceptedSocketNanoTime < 1000) { + throw new IOException(sm.getString("endpoint.err.duplicateAccept")); + } + previousAcceptedSocketRemoteAddress = currentRemoteAddress; + previouspreviousAcceptedSocketNanoTime = currentNanoTime; } - previousAcceptedSocketRemoteAddress = currentRemoteAddress; return result; } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 3ad0ad2..cf4b3c8 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -143,6 +143,10 @@ ignored when the Connector used an internal executor. (markt) </fix> <fix> + <bug>65776</bug>: Improve the detection of the Linux duplicate accept + bug and reduce (hopefully avoid) instances of false positives. (markt) + </fix> + <fix> <bug>65848</bug>: Revert the change that attempted to align the behaviour of client certificate authentication with NIO or NIO2 with OpenSSL for TLS between MacOS and Linux/Windows as the root cause was --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org