Merge remote-tracking branch 'karsten/bug5053'

changes/bug5053

@@ -0,0 +1,5 @@
+  o Minor bugfixes:
+    - Resolve IPv6 addresses in bridge and entry statistics to country code
+      "??" which means we at least count them. Fixes bug 5053; bugfix on
+      0.2.3.9-alpha.
+

src/or/connection_or.c

@@ -1713,11 +1713,8 @@ connection_or_set_state_open(or_connection_t *conn)
   } else {
     /* only report it to the geoip module if it's not a known router */
     if (!router_get_by_id_digest(conn->identity_digest)) {
-      if (tor_addr_family(&TO_CONN(conn)->addr) == AF_INET) {
-        /*XXXX IP6 support ipv6 geoip.*/
-        uint32_t a = tor_addr_to_ipv4h(&TO_CONN(conn)->addr);
-        geoip_note_client_seen(GEOIP_CLIENT_CONNECT, a, now);
-      }
+      geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &TO_CONN(conn)->addr,
+                             now);
     }
   }
 

src/or/directory.c

@@ -2845,8 +2845,10 @@ directory_handle_command_get(dir_connection_t *conn, const char *headers,
 
     {
       struct in_addr in;
+      tor_addr_t addr;
       if (tor_inet_aton((TO_CONN(conn))->address, &in)) {
-        geoip_note_client_seen(act, ntohl(in.s_addr), time(NULL));
+        tor_addr_from_ipv4h(&addr, ntohl(in.s_addr));
+        geoip_note_client_seen(act, &addr, time(NULL));
         geoip_note_ns_response(act, GEOIP_SUCCESS);
         /* Note that a request for a network status has started, so that we
          * can measure the download time later on. */

src/or/geoip.c

@@ -261,6 +261,21 @@ geoip_get_country_by_ip(uint32_t ipaddr)
   return ent ? (int)ent->country : 0;
 }
 
+/** Given an IP address, return a number representing the country to which
+ * that address belongs, -1 for "No geoip information available", or 0 for
+ * the 'unknown country'.  The return value will always be less than
+ * geoip_get_n_countries().  To decode it, call geoip_get_country_name().
+ */
+int
+geoip_get_country_by_addr(const tor_addr_t *addr)
+{
+  if (tor_addr_family(addr) != AF_INET) {
+    /*XXXX IP6 support ipv6 geoip.*/
+    return -1;
+  }
+  return geoip_get_country_by_ip(tor_addr_to_ipv4h(addr));
+}
+
 /** Return the number of countries recognized by the GeoIP database. */
 int
 geoip_get_n_countries(void)
@@ -303,7 +318,7 @@ geoip_db_digest(void)
  * countries have them blocked. */
 typedef struct clientmap_entry_t {
   HT_ENTRY(clientmap_entry_t) node;
-  uint32_t ipaddr;
+  tor_addr_t addr;
   /** Time when we last saw this IP address, in MINUTES since the epoch.
    *
    * (This will run out of space around 4011 CE.  If Tor is still in use around
@@ -325,13 +340,14 @@ static HT_HEAD(clientmap, clientmap_entry_t) client_history =
 static INLINE unsigned
 clientmap_entry_hash(const clientmap_entry_t *a)
 {
-  return ht_improve_hash((unsigned) a->ipaddr);
+  return ht_improve_hash(tor_addr_hash(&a->addr));
 }
 /** Hashtable helper: compare two clientmap_entry_t values for equality. */
 static INLINE int
 clientmap_entries_eq(const clientmap_entry_t *a, const clientmap_entry_t *b)
 {
-  return a->ipaddr == b->ipaddr && a->action == b->action;
+  return !tor_addr_compare(&a->addr, &b->addr, CMP_EXACT) &&
+         a->action == b->action;
 }
 
 HT_PROTOTYPE(clientmap, clientmap_entry_t, node, clientmap_entry_hash,
@@ -417,12 +433,12 @@ geoip_get_mean_shares(time_t now, double *v2_share_out,
   return 0;
 }
 
-/** Note that we've seen a client connect from the IP <b>addr</b> (host order)
+/** Note that we've seen a client connect from the IP <b>addr</b>
  * at time <b>now</b>. Ignored by all but bridges and directories if
  * configured accordingly. */
 void
 geoip_note_client_seen(geoip_client_action_t action,
-                       uint32_t addr, time_t now)
+                       const tor_addr_t *addr, time_t now)
 {
   const or_options_t *options = get_options();
   clientmap_entry_t lookup, *ent;
@@ -437,12 +453,12 @@ geoip_note_client_seen(geoip_client_action_t action,
       return;
   }
 
-  lookup.ipaddr = addr;
+  tor_addr_copy(&lookup.addr, addr);
   lookup.action = (int)action;
   ent = HT_FIND(clientmap, &client_history, &lookup);
   if (! ent) {
     ent = tor_malloc_zero(sizeof(clientmap_entry_t));
-    ent->ipaddr = addr;
+    tor_addr_copy(&ent->addr, addr);
     ent->action = (int)action;
     HT_INSERT(clientmap, &client_history, ent);
   }
@@ -453,7 +469,7 @@ geoip_note_client_seen(geoip_client_action_t action,
 
   if (action == GEOIP_CLIENT_NETWORKSTATUS ||
       action == GEOIP_CLIENT_NETWORKSTATUS_V2) {
-    int country_idx = geoip_get_country_by_ip(addr);
+    int country_idx = geoip_get_country_by_addr(addr);
     if (country_idx < 0)
       country_idx = 0; /** unresolved requests are stored at index 0. */
     if (country_idx >= 0 && country_idx < smartlist_len(geoip_countries)) {
@@ -823,7 +839,7 @@ geoip_get_client_history(geoip_client_action_t action)
     int country;
     if ((*ent)->action != (int)action)
       continue;
-    country = geoip_get_country_by_ip((*ent)->ipaddr);
+    country = geoip_get_country_by_addr(&(*ent)->addr);
     if (country < 0)
       country = 0; /** unresolved requests are stored at index 0. */
     tor_assert(0 <= country && country < n_countries);

src/or/geoip.h

@@ -18,6 +18,7 @@ int geoip_parse_entry(const char *line);
 int should_record_bridge_info(const or_options_t *options);
 int geoip_load_file(const char *filename, const or_options_t *options);
 int geoip_get_country_by_ip(uint32_t ipaddr);
+int geoip_get_country_by_addr(const tor_addr_t *addr);
 int geoip_get_n_countries(void);
 const char *geoip_get_country_name(country_t num);
 int geoip_is_loaded(void);
@@ -25,7 +26,7 @@ const char *geoip_db_digest(void);
 country_t geoip_get_country(const char *countrycode);
 
 void geoip_note_client_seen(geoip_client_action_t action,
-                            uint32_t addr, time_t now);
+                            const tor_addr_t *addr, time_t now);
 void geoip_remove_old_clients(time_t cutoff);
 
 void geoip_note_ns_response(geoip_client_action_t action,

src/test/test.c

@@ -1452,6 +1452,7 @@ test_geoip(void)
   *entry_stats_2 =
       "entry-stats-end 2010-08-12 13:27:30 (86400 s)\n"
       "entry-ips \n";
+  tor_addr_t addr;
 
   /* Populate the DB a bit.  Add these in order, since we can't do the final
    * 'sort' step.  These aren't very good IP addresses, but they're perfectly
@@ -1480,16 +1481,23 @@ test_geoip(void)
   get_options_mutable()->BridgeRelay = 1;
   get_options_mutable()->BridgeRecordUsageByCountry = 1;
   /* Put 9 observations in AB... */
-  for (i=32; i < 40; ++i)
-    geoip_note_client_seen(GEOIP_CLIENT_CONNECT, i, now-7200);
-  geoip_note_client_seen(GEOIP_CLIENT_CONNECT, 225, now-7200);
+  for (i=32; i < 40; ++i) {
+    tor_addr_from_ipv4h(&addr, (uint32_t) i);
+    geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &addr, now-7200);
+  }
+  tor_addr_from_ipv4h(&addr, (uint32_t) 225);
+  geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &addr, now-7200);
   /* and 3 observations in XY, several times. */
   for (j=0; j < 10; ++j)
-    for (i=52; i < 55; ++i)
-      geoip_note_client_seen(GEOIP_CLIENT_CONNECT, i, now-3600);
+    for (i=52; i < 55; ++i) {
+      tor_addr_from_ipv4h(&addr, (uint32_t) i);
+      geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &addr, now-3600);
+    }
   /* and 17 observations in ZZ... */
-  for (i=110; i < 127; ++i)
-    geoip_note_client_seen(GEOIP_CLIENT_CONNECT, i, now);
+  for (i=110; i < 127; ++i) {
+    tor_addr_from_ipv4h(&addr, (uint32_t) i);
+    geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &addr, now);
+  }
   s = geoip_get_client_history(GEOIP_CLIENT_CONNECT);
   test_assert(s);
   test_streq("zz=24,ab=16,xy=8", s);
@@ -1528,14 +1536,16 @@ test_geoip(void)
 
   /* Start testing dirreq statistics by making sure that we don't collect
    * dirreq stats without initializing them. */
-  geoip_note_client_seen(GEOIP_CLIENT_NETWORKSTATUS, 100, now);
+  tor_addr_from_ipv4h(&addr, (uint32_t) 100);
+  geoip_note_client_seen(GEOIP_CLIENT_NETWORKSTATUS, &addr, now);
   s = geoip_format_dirreq_stats(now + 86400);
   test_assert(!s);
 
   /* Initialize stats, note one connecting client, and generate the
    * dirreq-stats history string. */
   geoip_dirreq_stats_init(now);
-  geoip_note_client_seen(GEOIP_CLIENT_NETWORKSTATUS, 100, now);
+  tor_addr_from_ipv4h(&addr, (uint32_t) 100);
+  geoip_note_client_seen(GEOIP_CLIENT_NETWORKSTATUS, &addr, now);
   s = geoip_format_dirreq_stats(now + 86400);
   test_streq(dirreq_stats_1, s);
   tor_free(s);
@@ -1543,14 +1553,16 @@ test_geoip(void)
   /* Stop collecting stats, add another connecting client, and ensure we
    * don't generate a history string. */
   geoip_dirreq_stats_term();
-  geoip_note_client_seen(GEOIP_CLIENT_NETWORKSTATUS, 101, now);
+  tor_addr_from_ipv4h(&addr, (uint32_t) 101);
+  geoip_note_client_seen(GEOIP_CLIENT_NETWORKSTATUS, &addr, now);
   s = geoip_format_dirreq_stats(now + 86400);
   test_assert(!s);
 
   /* Re-start stats, add a connecting client, reset stats, and make sure
    * that we get an all empty history string. */
   geoip_dirreq_stats_init(now);
-  geoip_note_client_seen(GEOIP_CLIENT_NETWORKSTATUS, 100, now);
+  tor_addr_from_ipv4h(&addr, (uint32_t) 100);
+  geoip_note_client_seen(GEOIP_CLIENT_NETWORKSTATUS, &addr, now);
   geoip_reset_dirreq_stats(now);
   s = geoip_format_dirreq_stats(now + 86400);
   test_streq(dirreq_stats_2, s);
@@ -1577,14 +1589,16 @@ test_geoip(void)
 
   /* Start testing entry statistics by making sure that we don't collect
    * anything without initializing entry stats. */
-  geoip_note_client_seen(GEOIP_CLIENT_CONNECT, 100, now);
+  tor_addr_from_ipv4h(&addr, (uint32_t) 100);
+  geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &addr, now);
   s = geoip_format_entry_stats(now + 86400);
   test_assert(!s);
 
   /* Initialize stats, note one connecting client, and generate the
    * entry-stats history string. */
   geoip_entry_stats_init(now);
-  geoip_note_client_seen(GEOIP_CLIENT_CONNECT, 100, now);
+  tor_addr_from_ipv4h(&addr, (uint32_t) 100);
+  geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &addr, now);
   s = geoip_format_entry_stats(now + 86400);
   test_streq(entry_stats_1, s);
   tor_free(s);
@@ -1592,14 +1606,16 @@ test_geoip(void)
   /* Stop collecting stats, add another connecting client, and ensure we
    * don't generate a history string. */
   geoip_entry_stats_term();
-  geoip_note_client_seen(GEOIP_CLIENT_CONNECT, 101, now);
+  tor_addr_from_ipv4h(&addr, (uint32_t) 101);
+  geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &addr, now);
   s = geoip_format_entry_stats(now + 86400);
   test_assert(!s);
 
   /* Re-start stats, add a connecting client, reset stats, and make sure
    * that we get an all empty history string. */
   geoip_entry_stats_init(now);
-  geoip_note_client_seen(GEOIP_CLIENT_CONNECT, 100, now);
+  tor_addr_from_ipv4h(&addr, (uint32_t) 100);
+  geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &addr, now);
   geoip_reset_entry_stats(now);
   s = geoip_format_entry_stats(now + 86400);
   test_streq(entry_stats_2, s);