[tor-commits] [tor/master] Fix a bug in policy_is_reject_star() that was making IPv4 exits break

[nickm at torproject.org]

commit bb2145b45ba5992eae6d647b946b430dd2367375
Author: Nick Mathewson <nickm at torproject.org>
Date:   Wed Nov 14 20:51:41 2012 -0500

    Fix a bug in policy_is_reject_star() that was making IPv4 exits break
    
    IPv4-only exits have an implicit "reject [::]/0", which was making
    policy_is_reject_star() return 1 for them, making us refuse to do
    hostname lookups.
    
    This fix chanes policy_is_reject_star() to ask about which family we meant.
---
 src/or/dns.c         |    6 ++----
 src/or/policies.c    |   19 ++++++++++++-------
 src/or/policies.h    |    2 +-
 src/or/router.c      |    3 ++-
 src/or/routerparse.c |    7 +++++--
 src/test/test.c      |    6 +++---
 6 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/src/or/dns.c b/src/or/dns.c
index 6090c45..976d188 100644
--- a/src/or/dns.c
+++ b/src/or/dns.c
@@ -799,7 +799,6 @@ dns_resolve_impl(edge_connection_t *exitconn, int is_resolve,
   cached_resolve_t search;
   pending_connection_t *pending_connection;
   int is_reverse = 0;
-  const routerinfo_t *me;
   tor_addr_t addr;
   time_t now = time(NULL);
   int r;
@@ -824,10 +823,9 @@ dns_resolve_impl(edge_connection_t *exitconn, int is_resolve,
   }
 
   /* If we're a non-exit, don't even do DNS lookups. */
-  if (!(me = router_get_my_routerinfo()) ||
-      policy_is_reject_star(me->exit_policy)) {
+  if (router_my_exit_policy_is_reject_star())
     return -1;
-  }
+
   if (address_is_invalid_destination(exitconn->base_.address, 0)) {
     log(LOG_PROTOCOL_WARN, LD_EXIT,
         "Rejecting invalid destination address %s",
diff --git a/src/or/policies.c b/src/or/policies.c
index f9646f8..dd7de70 100644
--- a/src/or/policies.c
+++ b/src/or/policies.c
@@ -1049,18 +1049,23 @@ exit_policy_is_general_exit(smartlist_t *policy)
 /** Return false if <b>policy</b> might permit access to some addr:port;
  * otherwise if we are certain it rejects everything, return true. */
 int
-policy_is_reject_star(const smartlist_t *policy)
+policy_is_reject_star(const smartlist_t *policy, sa_family_t family)
 {
   if (!policy) /*XXXX disallow NULL policies? */
     return 1;
-  SMARTLIST_FOREACH(policy, addr_policy_t *, p, {
-    if (p->policy_type == ADDR_POLICY_ACCEPT)
+  SMARTLIST_FOREACH_BEGIN(policy, addr_policy_t *, p) {
+    if (p->policy_type == ADDR_POLICY_ACCEPT &&
+        (tor_addr_family(&p->addr) == family ||
+         tor_addr_family(&p->addr) == AF_UNSPEC)) {
       return 0;
-    else if (p->policy_type == ADDR_POLICY_REJECT &&
-             p->prt_min <= 1 && p->prt_max == 65535 &&
-             p->maskbits == 0)
+    } else if (p->policy_type == ADDR_POLICY_REJECT &&
+               p->prt_min <= 1 && p->prt_max == 65535 &&
+               p->maskbits == 0 &&
+               (tor_addr_family(&p->addr) == family ||
+                tor_addr_family(&p->addr) == AF_UNSPEC)) {
       return 1;
-  });
+    }
+  } SMARTLIST_FOREACH_END(p);
   return 1;
 }
 
diff --git a/src/or/policies.h b/src/or/policies.h
index f8de911..d9983e8 100644
--- a/src/or/policies.h
+++ b/src/or/policies.h
@@ -49,7 +49,7 @@ int policies_parse_exit_policy(config_line_t *cfg, smartlist_t **dest,
 void policies_exit_policy_append_reject_star(smartlist_t **dest);
 void policies_set_node_exitpolicy_to_reject_all(node_t *exitrouter);
 int exit_policy_is_general_exit(smartlist_t *policy);
-int policy_is_reject_star(const smartlist_t *policy);
+int policy_is_reject_star(const smartlist_t *policy, sa_family_t family);
 int getinfo_helper_policies(control_connection_t *conn,
                             const char *question, char **answer,
                             const char **errmsg);
diff --git a/src/or/router.c b/src/or/router.c
index f51b74e..d5ffb36 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -1621,7 +1621,8 @@ router_rebuild_descriptor(int force)
                                ri->address, !options->BridgeRelay);
   }
   ri->policy_is_reject_star =
-    policy_is_reject_star(ri->exit_policy);
+    policy_is_reject_star(ri->exit_policy, AF_INET) &&
+    policy_is_reject_star(ri->exit_policy, AF_INET6);
 
   if (options->IPv6Exit) {
     char *p_tmp = policy_summarize(ri->exit_policy, AF_INET6);
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 5607479..82c062c 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -1574,8 +1574,6 @@ router_parse_entry_from_string(const char *s, const char *end,
                       goto err;
                     });
   policy_expand_private(&router->exit_policy);
-  if (policy_is_reject_star(router->exit_policy))
-    router->policy_is_reject_star = 1;
 
   if ((tok = find_opt_by_keyword(tokens, K_IPV6_POLICY)) && tok->n_args) {
     router->ipv6_exit_policy = parse_short_policy(tok->args[0]);
@@ -1585,6 +1583,11 @@ router_parse_entry_from_string(const char *s, const char *end,
     }
   }
 
+  if (policy_is_reject_star(router->exit_policy, AF_INET) &&
+      (!router->ipv6_exit_policy ||
+       short_policy_is_reject_star(router->ipv6_exit_policy)))
+    router->policy_is_reject_star = 1;
+
   if ((tok = find_opt_by_keyword(tokens, K_FAMILY)) && tok->n_args) {
     int i;
     router->declared_family = smartlist_new();
diff --git a/src/test/test.c b/src/test/test.c
index d70f255..c96aeb7 100644
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -1176,9 +1176,9 @@ test_policies(void)
   test_assert(!cmp_addr_policies(policy2, policy2));
   test_assert(!cmp_addr_policies(NULL, NULL));
 
-  test_assert(!policy_is_reject_star(policy2));
-  test_assert(policy_is_reject_star(policy));
-  test_assert(policy_is_reject_star(NULL));
+  test_assert(!policy_is_reject_star(policy2, AF_INET));
+  test_assert(policy_is_reject_star(policy, AF_INET));
+  test_assert(policy_is_reject_star(NULL, AF_INET));
 
   addr_policy_list_free(policy);
   policy = NULL;