commit b5e6bbc01dc5d89285aba8a1440a2f5833e531a5 Author: Nick Mathewson nickm@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) {
tor-commits@lists.torproject.org