[tor-commits] [tor/master] Do not even try to keep going on a socket with socklen==0

[nickm at torproject.org]

commit b5e6bbc01dc5d89285aba8a1440a2f5833e531a5
Author: Nick Mathewson <nickm at torproject.org>
Date:   Mon Dec 19 14:37:52 2011 -0500

    Do not even try to keep going on a socket with socklen==0
    
    Back in #1240, r1eo linked to information about how this could happen
    with older Linux kernels in response to nmap.  Bugs #4545 and #4547
    are about how our approach to trying to deal with this condition was
    broken and stupid.  Thanks to wanoskarnet for reminding us about #1240.
    
    This is a fix for the abovementioned bugs, and is a bugfix on
    0.1.0.3-rc.
---
 changes/bug1240     |    8 ++++++++
 src/or/connection.c |   27 ++++++++++-----------------
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/changes/bug1240 b/changes/bug1240
new file mode 100644
index 0000000..6570664
--- /dev/null
+++ b/changes/bug1240
@@ -0,0 +1,8 @@
+  o Minor bugfixes:
+    - When running with an older Linux kernel that erroneously responds
+      to strange nmap behavior by having accept() return successfully
+      with a zero-length socket, just close the connection. Previously,
+      we would try harder to learn the remote address: but there was no
+      such remote address to learn, and our method for trying to learn
+      it was incorrect. Fixes bugs #1240, #4745, and #4747. Bugfix on
+      0.1.0.3-rc. Reported and diagnosed by "r1eo".
diff --git a/src/or/connection.c b/src/or/connection.c
index 2049f42..d36a37a 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -1076,7 +1076,12 @@ connection_create_listener(const struct sockaddr *listensockaddr,
 }
 
 /** Do basic sanity checking on a newly received socket. Return 0
- * if it looks ok, else return -1. */
+ * if it looks ok, else return -1.
+ *
+ * Notably, some TCP stacks can erroneously have accept() return successfully
+ * with socklen 0, when the client sends an RST before the accept call (as
+ * nmap does).  We want to detect that, and not go on with the connection.
+ */
 static int
 check_sockaddr(struct sockaddr *sa, int len, int level)
 {
@@ -1142,7 +1147,7 @@ connection_handle_listener_read(connection_t *conn, int new_type)
   tor_socket_t news; /* the new socket */
   connection_t *newconn;
   /* information about the remote peer when connecting to other routers */
-  char addrbuf[256];
+  char addrbuf[256]; /*XXX023 use sockaddr_storage instead*/
   struct sockaddr *remote = (struct sockaddr*)addrbuf;
   /* length of the remote address. Must be whatever accept() needs. */
   socklen_t remotelen = (socklen_t)sizeof(addrbuf);
@@ -1186,21 +1191,9 @@ connection_handle_listener_read(connection_t *conn, int new_type)
     uint16_t port;
     if (check_sockaddr(remote, remotelen, LOG_INFO)<0) {
       log_info(LD_NET,
-               "accept() returned a strange address; trying getsockname().");
-      remotelen=sizeof(addrbuf);
-      memset(addrbuf, 0, sizeof(addrbuf));
-      if (getsockname(news, remote, &remotelen)<0) {
-        int e = tor_socket_errno(news);
-        log_warn(LD_NET, "getsockname() for new connection failed: %s",
-                 tor_socket_strerror(e));
-      } else {
-        if (check_sockaddr((struct sockaddr*)addrbuf, remotelen,
-                              LOG_WARN) < 0) {
-          log_warn(LD_NET,"Something's wrong with this conn. Closing it.");
-          tor_close_socket(news);
-          return 0;
-        }
-      }
+               "accept() returned a strange address; closing connection.");
+      tor_close_socket(news);
+      return 0;
     }
 
     if (check_sockaddr_family_match(remote->sa_family, conn) < 0) {