Mark,
Apologies for top-posting but this might get lost in the code below.
This patch appears to:
1. Get the address from the just-accepted socket
2. Compare the address to the most-recently-accepted socket address (see #3)
2a. Throw an error if the current and previous address are the same
3. Save a reference to the just-accepted socket address
Won't this be a problem if there is only one connection being handled,
going in and out of the poll/accept cycle?
I haven't looked at the implementation of SocketAddress.equals and I may
be confusing things a bit, but I would imagine on a mostly-idle server,
this might cause an error:
1. Client connects from e.g. 127.0.0.1
2. Server accepts, stores 127.0.0.1
3. Client disconnects
4. Client connects from 127.0.0.1
2. Server accepts, throws error that 127.0.0.1 == 127.0.0.1
This would happen pretty frequently for connections through a reverse-proxy.
Is there a way to differentiate between "I've seen this address before"
and "this is actually a duplicate?"
-chris
On 11/17/21 13:49, ma...@apache.org wrote:
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new d03cfcf Protect against a known OS bug
d03cfcf is described below
commit d03cfcf3b0d6639acb2884f1bbea5f2f29b95d91
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Nov 17 18:48:33 2021 +0000
Protect against a known OS bug
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1924298
---
java/org/apache/tomcat/util/net/LocalStrings.properties | 1 +
java/org/apache/tomcat/util/net/Nio2Endpoint.java | 12 +++++++++++-
java/org/apache/tomcat/util/net/NioEndpoint.java | 11 ++++++++++-
webapps/docs/changelog.xml | 6 ++++++
4 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/LocalStrings.properties
b/java/org/apache/tomcat/util/net/LocalStrings.properties
index 69d2dc4..3389f01 100644
--- a/java/org/apache/tomcat/util/net/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/net/LocalStrings.properties
@@ -66,6 +66,7 @@ endpoint.debug.unlock.localFail=Unable to determine local
address for [{0}]
endpoint.debug.unlock.localNone=Failed to unlock acceptor for [{0}] because
the local address was not available
endpoint.duplicateSslHostName=Multiple SSLHostConfig elements were provided
for the host name [{0}]. Host names must be unique.
endpoint.err.close=Caught exception trying to close socket
+endpoint.err.duplicateAccept=Duplicate accept detected. This is a known OS
bug. Please consider reporting that you are affected:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1924298
endpoint.err.handshake=Handshake failed
endpoint.err.unexpected=Unexpected error processing socket
endpoint.executor.fail=Executor rejected socket [{0}] for processing
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 5fb0477..47825f0 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -84,6 +84,8 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint<Nio2Channel,AsynchronousS
*/
private SynchronizedStack<Nio2Channel> nioChannels;
+ private SocketAddress previousAcceptedSocketRemoteAddress = null;
+
// --------------------------------------------------------- Public
Methods
@@ -355,7 +357,15 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS
@Override
protected AsynchronousSocketChannel serverSocketAccept() throws Exception
{
- return serverSock.accept().get();
+ AsynchronousSocketChannel result = serverSock.accept().get();
+
+ SocketAddress currentRemoteAddress = result.getRemoteAddress();
+ if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress)) {
+ throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
+ }
+ 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 81be955..34acce6 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -108,6 +108,8 @@ public class NioEndpoint extends
AbstractJsseEndpoint<NioChannel,SocketChannel>
*/
private SynchronizedStack<NioChannel> nioChannels;
+ private SocketAddress previousAcceptedSocketRemoteAddress = null;
+
// ------------------------------------------------------------- Properties
@@ -510,7 +512,14 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
@Override
protected SocketChannel serverSocketAccept() throws Exception {
- return serverSock.accept();
+ SocketChannel result = serverSock.accept();
+ SocketAddress currentRemoteAddress = result.getRemoteAddress();
+ if (currentRemoteAddress.equals(previousAcceptedSocketRemoteAddress)) {
+ throw new
IOException(sm.getString("endpoint.err.duplicateAccept"));
+ }
+ previousAcceptedSocketRemoteAddress = currentRemoteAddress;
+
+ return result;
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a6825c3..ef6b181 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -119,6 +119,12 @@
Use an implicit scope to tie the cleanup of OpenSSL memory to the Java
GC. (remm)
</add>
+ <add>
+ Provide protection against a known <a
+ href="https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1924298";>OS
+ bug</a> that causes the acceptor to report an incoming connection more
+ than once. (markt)
+ </add>
</changelog>
</subsection>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
