apply a variant of rovv's bug 872 patch, and document that we want a better solution for 0.2.2.x.

svn:r17924

3 files changed, 24 insertions, 5 deletions

diff --git a/ChangeLog b/ChangeLog
index 160acb9350..7ae496eaec 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -51,6 +51,11 @@ Changes in version 0.2.1.10-alpha - 2009-01-??
     - Resume reporting accurate "stream end" reasons to the local control
       port. They were lost in the changes for Proposal 148. Bugfix on
       0.2.1.9-alpha.
+    - When an exit resolves an address to a local IP, do not just keep
+      retrying that same exit over and over.   Instead, just close
+      the connection.  Addresses bug 872.  Patch from rovv.
+    - If a hidden service sends us an END cell, do not consider
+      retrying the connection.  Patch from rovv.
 
   o Deprecated and removed features:
     - The old "tor --version --version" command, which would spit out the
diff --git a/doc/TODO.future b/doc/TODO.future
index eaebde5940..8b5decd0d0 100644
--- a/doc/TODO.future
+++ b/doc/TODO.future
@@ -49,6 +49,13 @@ Later, unless people want to implement them now:
   - Make the timestamp granularity on logs configurable, with default
     of "1 second".  This might make some kinds of after-the-fact attack harder.
 
+  - We should get smarter about handkling address resolve failures, or
+    addresses that resolve to local IPs.  It would be neat to retry
+    them, since right now we just close the stream.  But we need to
+    make sure we don't retry them on the same exit as before.  But if
+    we mark the circuit, then any user who types "localhost" will
+    cycle through circuits till they run out of retries.  See bug 872.
+
 Can anybody remember why we wanted to do this and/or what it means?
   - config option __ControllerLimit that hangs up if there are a limit
     of controller connections already.
diff --git a/src/or/relay.c b/src/or/relay.c
index bc1b2a50ed..0c607f04f7 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -659,7 +659,9 @@ connection_ap_process_end_not_open(
   int control_reason = reason | END_STREAM_REASON_FLAG_REMOTE;
   (void) layer_hint; /* unused */
 
-  if (rh->length > 0 && edge_reason_is_retriable(reason)) {
+  if (rh->length > 0 && edge_reason_is_retriable(reason) &&
+      !connection_edge_is_rendezvous_stream(conn)  /* avoid retry if rend */
+      ) {
     log_info(LD_APP,"Address '%s' refused due to '%s'. Considering retrying.",
              safe_str(conn->socks_request->address),
              stream_end_reason_to_string(reason));
@@ -681,10 +683,15 @@ connection_ap_process_end_not_open(
           else
             ttl = -1;
 
-          if (!(get_options()->ClientDNSRejectInternalAddresses &&
-                                           is_internal_IP(addr, 0)))
-            client_dns_set_addressmap(conn->socks_request->address, addr,
-                                      conn->chosen_exit_name, ttl);
+          if (get_options()->ClientDNSRejectInternalAddresses &&
+              is_internal_IP(addr, 0)) {
+            log_info(LD_APP,"Address '%s' resolved to internal. Closing,",
+                     safe_str(conn->socks_request->address));
+            connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL);
+            return 0;
+          }
+          client_dns_set_addressmap(conn->socks_request->address, addr,
+                                    conn->chosen_exit_name, ttl);
         }
         /* check if he *ought* to have allowed it */
         if (exitrouter &&