Browse Source

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

svn:r17924
Nick Mathewson 17 years ago
parent
commit
f772fc0c36
3 changed files with 24 additions and 5 deletions
  1. 5 0
      ChangeLog
  2. 7 0
      doc/TODO.future
  3. 12 5
      src/or/relay.c

+ 5 - 0
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
     - Resume reporting accurate "stream end" reasons to the local control
       port. They were lost in the changes for Proposal 148. Bugfix on
       port. They were lost in the changes for Proposal 148. Bugfix on
       0.2.1.9-alpha.
       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:
   o Deprecated and removed features:
     - The old "tor --version --version" command, which would spit out the
     - The old "tor --version --version" command, which would spit out the

+ 7 - 0
doc/TODO.future

@@ -49,6 +49,13 @@ Later, unless people want to implement them now:
   - Make the timestamp granularity on logs configurable, with default
   - Make the timestamp granularity on logs configurable, with default
     of "1 second".  This might make some kinds of after-the-fact attack harder.
     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?
 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
   - config option __ControllerLimit that hangs up if there are a limit
     of controller connections already.
     of controller connections already.

+ 12 - 5
src/or/relay.c

@@ -659,7 +659,9 @@ connection_ap_process_end_not_open(
   int control_reason = reason | END_STREAM_REASON_FLAG_REMOTE;
   int control_reason = reason | END_STREAM_REASON_FLAG_REMOTE;
   (void) layer_hint; /* unused */
   (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.",
     log_info(LD_APP,"Address '%s' refused due to '%s'. Considering retrying.",
              safe_str(conn->socks_request->address),
              safe_str(conn->socks_request->address),
              stream_end_reason_to_string(reason));
              stream_end_reason_to_string(reason));
@@ -681,10 +683,15 @@ connection_ap_process_end_not_open(
           else
           else
             ttl = -1;
             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 */
         /* check if he *ought* to have allowed it */
         if (exitrouter &&
         if (exitrouter &&