[or-cvs] [tor/master 1/4] Don't assign Exit flag incorrectly

[nickm at torproject.org]

Author: Sebastian Hahn <sebastian at torproject.org>
Date: Tue, 2 Feb 2010 14:51:51 +0100
Subject: Don't assign Exit flag incorrectly
Commit: 03bd98b3b1f92954c286f9d4dd5a2dd5f34a7e8b

exit_policy_is_general_exit() assumed that there are no redundancies
in the passed policy, in the sense that we actively combine entries
in the policy to really get rid of any redundancy. Since we cannot
do that without massively rewriting the policy lines the relay
operators set, fix exit_policy_is_general_exit().

Fixes bug 1238, discovered by Martin Kowalczyk.
---
 ChangeLog         |    6 ++++++
 src/or/policies.c |   51 ++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 78e4039..2cf7776 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,10 @@
 Changes in version 0.2.2.9-alpha - 2010-??-??
+  o Major bugfixes:
+    - Authorities could be tricked into giving out the Exit flag to relays
+      that didn't allow exiting to any ports. This could screw with load
+      balancing and stats. Bugfix on 0.1.1.6-alpha, fixes bug 1238. Bug
+      discovered by Martin Kowalczyk.
+
   o Minor bugfixes:
     - When deciding whether to use strange flags to turn TLS renegotiation
       on, detect the OpenSSL version at run-time, not compile time.  We
diff --git a/src/or/policies.c b/src/or/policies.c
index 453138e..ab308f7 100644
--- a/src/or/policies.c
+++ b/src/or/policies.c
@@ -865,6 +865,43 @@ policies_set_router_exitpolicy_to_reject_all(routerinfo_t *r)
   smartlist_add(r->exit_policy, item);
 }
 
+/** Return 1 if there is at least one /8 subnet in <b>policy</b> that
+ * allows exiting to <b>port</b>. */
+static int
+exit_policy_is_general_exit_helper(smartlist_t *policy, int port)
+{
+  uint32_t j;
+  /* Is this /8 rejected (1), or undecided (0)? */
+  char subnet_status[256];
+
+  memset(subnet_status, 0, sizeof(subnet_status));
+  SMARTLIST_FOREACH(policy, addr_policy_t *, p, {
+    if (p->prt_min > port || p->prt_max < port)
+      continue; /* Doesn't cover our port. */
+    for (j = 0; j < 256; ++j) {
+      tor_addr_t addr;
+      if (subnet_status[j] != 0)
+        continue; /* We already reject some part of this /8 */
+      tor_addr_from_ipv4h(&addr, j<<24);
+      if (tor_addr_is_internal(&addr, 1)) /* 1 because * = 0.0.0.0 */
+        continue; /* Local or non-routable addresses */
+      if (tor_addr_compare_masked(&addr, &p->addr, p->maskbits,
+                                  CMP_EXACT) == 0) {
+        if (p->policy_type == ADDR_POLICY_ACCEPT) {
+          if (p->maskbits > 8)
+            continue; /* Narrower than a /8. */
+          /* We found an allowed subnet of at least size /8. Done
+           * for this port! */
+          return 1;
+        } else if (p->policy_type == ADDR_POLICY_REJECT) {
+          subnet_status[j] = 1;
+        }
+      }
+    }
+  });
+  return 0;
+}
+
 /** Return true iff <b>ri</b> is "useful as an exit node", meaning
  * it allows exit to at least one /8 address space for at least
  * two of ports 80, 443, and 6667. */
@@ -878,19 +915,7 @@ exit_policy_is_general_exit(smartlist_t *policy)
     return 0;
 
   for (i = 0; i < 3; ++i) {
-    SMARTLIST_FOREACH(policy, addr_policy_t *, p, {
-      if (p->prt_min > ports[i] || p->prt_max < ports[i])
-        continue; /* Doesn't cover our port. */
-      if (p->maskbits > 8)
-        continue; /* Narrower than a /8. */
-      if (tor_addr_is_loopback(&p->addr))
-        continue; /* 127.x or ::1. */
-      /* We have a match that is at least a /8. */
-      if (p->policy_type == ADDR_POLICY_ACCEPT) {
-        ++n_allowed;
-        break; /* stop considering this port */
-      }
-    });
+    n_allowed += exit_policy_is_general_exit_helper(policy, ports[i]);
   }
   return n_allowed >= 2;
 }
-- 
1.6.5