Append default exit policy before checking for implicit internal addresses: fix bug 129.

svn:r4201

5 files changed, 78 insertions, 35 deletions

diff --git a/src/or/config.c b/src/or/config.c
index 4ba20c602..ff4ee6e11 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -1574,6 +1574,7 @@ options_validate(or_options_t *options)
     log_fn(LOG_WARN, "Error in Exit Policy entry.");
     result = -1;
   }
+  config_append_default_exit_policy(&addr_policy);
   if (server_mode(options)) {
     exit_policy_implicitly_allows_local_networks(addr_policy, 1);
   }
@@ -2161,6 +2162,32 @@ normalize_log_options(or_options_t *options)
   return 0;
 }
 
+#define DEFAULT_EXIT_POLICY "reject 0.0.0.0/8,reject 169.254.0.0/16,reject 127.0.0.0/8,reject 192.168.0.0/16,reject 10.0.0.0/8,reject 172.16.0.0/12,reject *:25,reject *:119,reject *:135-139,reject *:445,reject *:1214,reject *:4661-4666,reject *:6346-6429,reject *:6699,reject *:6881-6999,accept *:*"
+
+void
+config_append_default_exit_policy(addr_policy_t **policy)
+{
+  struct config_line_t tmp;
+  addr_policy_t *ap;
+
+  tmp.key = NULL;
+  tmp.value = (char*)DEFAULT_EXIT_POLICY;
+  tmp.next = NULL;
+  config_parse_addr_policy(&tmp, policy);
+
+  /* Remove redundant parts, if any. */
+  for (ap=*policy; ap; ap=ap->next) {
+    if (ap->msk == 0 && ap->prt_min <= 1 && ap->prt_max >= 65535) {
+      if (ap->next) {
+        addr_policy_free(ap->next);
+        ap->next = NULL;
+      }
+      return;
+    }
+  }
+}
+
+
 /**
  * Given a linked list of config lines containing "allow" and "deny" tokens,
  * parse them and append the result to <b>dest</b>.  Return -1 if any tokens
diff --git a/src/or/or.h b/src/or/or.h
index 8a19b7359..08282f1d5 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -1255,6 +1255,7 @@ int init_from_config(int argc, char **argv);
 int config_init_logs(or_options_t *options, int validate_only);
 int config_parse_addr_policy(struct config_line_t *cfg,
                              addr_policy_t **dest);
+void config_append_default_exit_policy(addr_policy_t **policy);
 void addr_policy_free(addr_policy_t *p);
 int config_option_is_recognized(const char *key);
 struct config_line_t *config_get_assigned_option(or_options_t *options,
diff --git a/src/or/router.c b/src/or/router.c
index f97e30f46..503178635 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -577,31 +577,6 @@ void router_upload_dir_desc_to_dirservers(int force) {
   directory_post_to_dirservers(DIR_PURPOSE_UPLOAD_DIR, s, strlen(s));
 }
 
-#define DEFAULT_EXIT_POLICY "reject 0.0.0.0/8,reject 169.254.0.0/16,reject 127.0.0.0/8,reject 192.168.0.0/16,reject 10.0.0.0/8,reject 172.16.0.0/12,reject *:25,reject *:119,reject *:135-139,reject *:445,reject *:1214,reject *:4661-4666,reject *:6346-6429,reject *:6699,reject *:6881-6999,accept *:*"
-
-/** Set the exit policy on <b>router</b> to match the exit policy in the
- * current configuration file.  If the exit policy doesn't have a catch-all
- * rule, then append the default exit policy as well.
- */
-static void router_add_exit_policy_from_config(routerinfo_t *router) {
-  addr_policy_t *ep;
-  struct config_line_t default_policy;
-  config_parse_addr_policy(get_options()->ExitPolicy, &router->exit_policy);
-
-  for (ep = router->exit_policy; ep; ep = ep->next) {
-    if (ep->msk == 0 && ep->prt_min <= 1 && ep->prt_max >= 65535) {
-      /* if exitpolicy includes a *:* line, then we're done. */
-      return;
-    }
-  }
-
-  /* Else, append the default exitpolicy. */
-  default_policy.key = NULL;
-  default_policy.value = (char*)DEFAULT_EXIT_POLICY;
-  default_policy.next = NULL;
-  config_parse_addr_policy(&default_policy, &router->exit_policy);
-}
-
 /** OR only: Check whether my exit policy says to allow connection to
  * conn.  Return false if we accept; true if we reject.
  */
@@ -702,7 +677,9 @@ int router_rebuild_descriptor(int force) {
   if (options->BandwidthRate > options->MaxAdvertisedBandwidth)
     ri->bandwidthrate = (int)options->MaxAdvertisedBandwidth;
 
-  router_add_exit_policy_from_config(ri);
+  config_parse_addr_policy(get_options()->ExitPolicy, &ri->exit_policy);
+  config_append_default_exit_policy(&ri->exit_policy);
+
   if (desc_routerinfo) /* inherit values */
     ri->is_verified = desc_routerinfo->is_verified;
   if (options->MyFamily) {
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index c7a1c5c7e..1e5e9e8bc 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1171,8 +1171,9 @@ policy_includes_addr_mask_implicitly(addr_policy_t *policy,
      * its value, and every free bit set to 1.  So if addr and addr2 are
      * both in the policy, the range is covered by the policy.
      */
-    if ((policy->addr & policy->msk) == (addr & policy->msk) &&
-        (policy->addr & policy->msk) == (addr2 & policy->msk) &&
+    uint32_t p_addr = policy->addr & policy->msk;
+    if (p_addr == (addr & policy->msk) &&
+        p_addr == (addr2 & policy->msk) &&
         (policy->prt_min <= 1 && policy->prt_max == 65535)) {
       return 0;
     }
@@ -1216,14 +1217,15 @@ exit_policy_implicitly_allows_local_networks(addr_policy_t *policy,
   };
   for (i=0; private_networks[i].addr; ++i) {
     p = NULL;
-      if (policy_includes_addr_mask_implicitly(
+    /* log_fn(LOG_INFO,"Checking network %s", private_networks[i].network); */
+    if (policy_includes_addr_mask_implicitly(
               policy, private_networks[i].addr, private_networks[i].mask, &p)) {
-        if (warn)
-          log_fn(LOG_WARN, "Exit policy %s implicitly accepts %s",
-                 p?p->string:"(default)",
-                 private_networks[i].network);
-        r = 1;
-      }
+      if (warn)
+        log_fn(LOG_WARN, "Exit policy %s implicitly accepts %s",
+               p?p->string:"(default)",
+               private_networks[i].network);
+      r = 1;
+    }
   }
 
   return r;
diff --git a/src/or/test.c b/src/or/test.c
index e4b000b96..d9dea858c 100644
--- a/src/or/test.c
+++ b/src/or/test.c
@@ -1316,6 +1316,40 @@ test_dir_format(void)
 }
 
 static void
+test_exit_policies(void)
+{
+  addr_policy_t *policy;
+
+  policy = router_parse_addr_policy_from_string("reject 192.168.0.0/16:*");
+  test_eq(NULL, policy->next);
+  test_eq(ADDR_POLICY_REJECT, policy->policy_type);
+  test_eq(0xc0a80000u, policy->addr);
+  test_eq(0xffff0000u, policy->msk);
+  test_eq(1, policy->prt_min);
+  test_eq(65535, policy->prt_max);
+  test_streq("reject 192.168.0.0/16:*", policy->string);
+
+  test_assert(exit_policy_implicitly_allows_local_networks(policy, 0));
+  test_eq(ADDR_POLICY_ACCEPTED,
+          router_compare_addr_to_addr_policy(0x01020304u, 2, policy));
+  test_eq(ADDR_POLICY_PROBABLY_ACCEPTED,
+          router_compare_addr_to_addr_policy(0, 2, policy));
+  test_eq(ADDR_POLICY_REJECTED,
+          router_compare_addr_to_addr_policy(0xc0a80102, 2, policy));
+
+  addr_policy_free(policy);
+
+  /* Copied from router.c */
+  policy = NULL;
+  config_append_default_exit_policy(&policy);
+  test_assert(policy);
+  test_assert(!exit_policy_implicitly_allows_local_networks(policy, 1));
+
+  addr_policy_free(policy);
+
+}
+
+static void
 test_rend_fns(void)
 {
   char address1[] = "fooaddress.onion";
@@ -1386,6 +1420,8 @@ main(int c, char**v) {
   test_onion_handshake();
   puts("\n========================= Directory Formats ===============");
   test_dir_format();
+  puts("\n========================= Exit policies ===================");
+  test_exit_policies();
   puts("\n========================= Rendezvous functionality ========");
   test_rend_fns();
   puts("");