r8846@totoro: nickm | 2006-10-02 16:59:57 -0400

 Move is_local_IP to config.c; have it check for same-/24; make it used only for reachability (not for banwidth, because that is probably not what we want). Fixes an XXX.


svn:r8578

ChangeLog

@@ -51,6 +51,9 @@ Changes in version 0.1.2.2-alpha - 2006-10-??
       available in non-Exit nodes is much higher then the bandwidth available
       in Exit nodes. (Fixes bug 200.)
     - Give more meaningful errors on control authentication failure.
+    - When deciding whether an IP is "local", check for IPs on the same /24
+      as us.  This prevents some false positives during reachability
+      detection.
 
   o Security Fixes, minor:
     - If a client asked for a server by name, and we didn't have a

src/common/util.c

@@ -1572,17 +1572,6 @@ is_internal_IP(uint32_t ip, int for_listening)
   return 0;
 }
 
-/** Return true iff <b>ip</b> (in host order) is judged to be on the
- * same network as us. For now, check if it's an internal IP.
- *
- * XXX Also check if it's on the same class C network as our public IP.
- */
-int
-is_local_IP(uint32_t ip)
-{
-  return is_internal_IP(ip, 0);
-}
-
 /** Parse a string of the form "host[:port]" from <b>addrport</b>.  If
  * <b>address</b> is provided, set *<b>address</b> to a copy of the
  * host portion of the string.  If <b>addr</b> is provided, try to

src/common/util.h

@@ -188,7 +188,6 @@ int path_is_relative(const char *filename) ATTR_PURE;
 
 /* Net helpers */
 int is_internal_IP(uint32_t ip, int for_listening) ATTR_PURE;
-int is_local_IP(uint32_t ip) ATTR_PURE;
 int parse_addr_port(int severity, const char *addrport, char **address,
                     uint32_t *addr, uint16_t *port_out);
 int parse_port_range(const char *port, uint16_t *port_min_out,

src/or/config.c

@@ -1590,6 +1590,8 @@ print_usage(void)
 "See man page for options, or http://tor.eff.org/ for documentation.\n");
 }
 
+/** Last value actually set by resolve_my_address. */
+static uint32_t last_resolved_addr = 0;
 /**
  * Based on <b>options-\>Address</b>, guess our public IP address and put it
  * (in host order) into *<b>addr_out</b>. If <b>hostname_out</b> is provided,
@@ -1607,7 +1609,6 @@ resolve_my_address(int warn_severity, or_options_t *options,
   int explicit_ip=1;
   int explicit_hostname=1;
   char tmpbuf[INET_NTOA_BUF_LEN];
-  static uint32_t old_addr=0;
   const char *address = options->Address;
   int notice_severity = warn_severity <= LOG_NOTICE ?
                           LOG_NOTICE : warn_severity;
@@ -1714,18 +1715,41 @@ resolve_my_address(int warn_severity, or_options_t *options,
 
   log_debug(LD_CONFIG, "Resolved Address to '%s'.", tmpbuf);
   *addr_out = ntohl(in.s_addr);
-  if (old_addr && old_addr != *addr_out) {
+  if (last_resolved_addr && last_resolved_addr != *addr_out) {
     /* Leave this as a notice, regardless of the requested severity,
      * at least until dynamic IP address support becomes bulletproof. */
     log_notice(LD_NET, "Your IP address seems to have changed. Updating.");
     server_has_changed_ip();
   }
-  old_addr = *addr_out;
+  last_resolved_addr = *addr_out;
   if (hostname_out)
     *hostname_out = tor_strdup(hostname);
   return 0;
 }
 
+/** Return true iff <b>ip</b> (in host order) is judged to be on the
+ * same network as us, or on a private network.
+ */
+int
+is_local_IP(uint32_t ip)
+{
+  if (is_internal_IP(ip, 0))
+    return 1;
+  /* Check whether ip is on the same /24 as we are.
+   *
+   * It's possible that this next check will hit before the first time
+   * resolve_my_address actually succeeds.  (For clients, it is likely that
+   * resolve_my_address will never be called at all).  In those cases,
+   * last_resolved_addr will be 0, and so checking to see whether ip is on the
+   * same /24 as last_resolved_addr will be the same as checking whether it
+   * was on net 0, which is already done by is_internal_IP.
+   */
+  if ((last_resolved_addr & 0xffffff00ul) == (ip & 0xffffff00ul))
+    return 1;
+  return 0;
+}
+
+
 /** Called when we don't have a nickname set.  Try to guess a good nickname
  * based on the hostname, and return it in a newly allocated string. If we
  * can't, return NULL and let the caller warn if it wants to. */

src/or/connection.c

@@ -1425,7 +1425,7 @@ connection_read_to_buf(connection_t *conn, int *max_to_read)
     *max_to_read = at_most - result;
   }
 
-  if (result > 0 && !is_local_IP(conn->addr)) { /* remember it */
+  if (result > 0 && !is_internal_IP(conn->addr, 0)) { /* remember it */
     rep_hist_note_bytes_read(result, time(NULL));
     connection_read_bucket_decrement(conn, result);
   }
@@ -1601,7 +1601,7 @@ connection_handle_write(connection_t *conn)
   }
 
   if (result > 0) {
-    if (!is_local_IP(conn->addr)) { /* remember it */
+    if (!is_internal_IP(conn->addr, 0)) { /* remember it */
       rep_hist_note_bytes_written(result, time(NULL));
       global_write_bucket -= result;
     }
@@ -1646,7 +1646,7 @@ _connection_controller_force_write(control_connection_t *control_conn)
   }
 
   if (result > 0) {
-    if (!is_local_IP(conn->addr)) { /* remember it */
+    if (!is_internal_IP(conn->addr, 0)) { /* remember it */
       rep_hist_note_bytes_written(result, time(NULL));
       global_write_bucket -= result;
     }

src/or/or.h

@@ -1821,6 +1821,7 @@ int options_trial_assign(config_line_t *list, int use_defaults,
                          int clear_first, char **msg);
 int resolve_my_address(int warn_severity, or_options_t *options,
                        uint32_t *addr, char **hostname_out);
+int is_local_IP(uint32_t ip) ATTR_PURE;
 void options_init(or_options_t *options);
 int options_init_from_torrc(int argc, char **argv);
 int options_init_logs(or_options_t *options, int validate_only);