Add a new config option ExitPolicyRejectPrivate which defaults to 1.

This means all exit policies will begin with rejecting private addresses,
unless the server operator explicitly turns it off.

Also, make our code to remove redundancies in the exit policy smarter,
so it can detect "reject foo, reject bar, reject *" patterns.

Lastly, we can get rid of the "exit policy implicitly accepts" code,
since we make everything more explicit now.


svn:r5888

5 files changed, 78 insertions, 33 deletions

diff --git a/src/or/config.c b/src/or/config.c
index d56f34937..39df0715b 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -149,6 +149,7 @@ static config_var_t _option_vars[] = {
   VAR("ExcludeNodes",        STRING,   ExcludeNodes,         NULL),
   VAR("ExitNodes",           STRING,   ExitNodes,            NULL),
   VAR("ExitPolicy",          LINELIST, ExitPolicy,           NULL),
+  VAR("ExitPolicyRejectPrivate", BOOL, ExitPolicyRejectPrivate, "1"),
   VAR("FascistFirewall",     BOOL,     FascistFirewall,      "0"),
   VAR("FirewallPorts",       CSV,      FirewallPorts,        ""),
   VAR("FastFirstHopPK",      BOOL,     FastFirstHopPK,       "1"),
@@ -2214,13 +2215,10 @@ options_validate(or_options_t *old_options, or_options_t *options,
       result = -1;
   }
 
-  if (config_parse_addr_policy(options->ExitPolicy, &addr_policy, -1))
-    REJECT("Error in Exit Policy entry.");
+  if (config_parse_exit_policy(options->ExitPolicy, &addr_policy,
+                               options->ExitPolicyRejectPrivate))
+    REJECT("Error in ExitPolicy entry.");
 
-  options_append_default_exit_policy(&addr_policy);
-  if (server_mode(options)) {
-    exit_policy_implicitly_allows_local_networks(addr_policy, 1);
-  }
   /* The rest of these calls *append* to addr_policy. So don't actually
    * use the results for anything other than checking if they parse! */
   if (config_parse_addr_policy(options->DirPolicy, &addr_policy, -1))
@@ -2916,34 +2914,17 @@ normalize_log_options(or_options_t *options)
   return 0;
 }
 
-#define DEFAULT_EXIT_POLICY                                                  \
-  "reject private:*,reject *:25,reject *:119,reject *:135-139,reject *:445," \
-  "reject *:465,reject *:587,reject *:1214,reject *:4661-4666,"              \
-  "reject *:6346-6429,reject *:6699,reject *:6881-6999,accept *:*"
-
-/** Add the default exit policy entries to <b>policy</b>
+/** Add the exit policy described by <b>more</b> to <b>policy</b>.
  */
-void
-options_append_default_exit_policy(addr_policy_t **policy)
+static void
+options_append_exit_policy_string(addr_policy_t **policy, char *more)
 {
   config_line_t tmp;
-  addr_policy_t *ap;
 
   tmp.key = NULL;
-  tmp.value = (char*)DEFAULT_EXIT_POLICY;
+  tmp.value = more;
   tmp.next = NULL;
   config_parse_addr_policy(&tmp, policy, -1);
-
-  /* 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;
-    }
-  }
 }
 
 static int
@@ -3005,6 +2986,65 @@ config_expand_exit_policy_aliases(smartlist_t *entries, int assume_action)
   return expanded_any;
 }
 
+/** Detect and excise "dead code" from the policy *<b>dest</b>. */
+static void
+config_exit_policy_remove_redundancies(addr_policy_t **dest)
+{
+  addr_policy_t *ap, *tmp;
+  int have_seen_accept=0;
+
+  for (ap=*dest; ap; ap=ap->next) {
+    if (ap->policy_type == ADDR_POLICY_ACCEPT)
+      have_seen_accept=1;
+    if (ap->msk == 0 && ap->prt_min <= 1 && ap->prt_max >= 65535) {
+      /* This is a catch-all line -- later lines are unreachable. */
+      if (ap->next) {
+        addr_policy_free(ap->next);
+        ap->next = NULL;
+      }
+      if (ap->policy_type == ADDR_POLICY_REJECT &&
+          ap != *dest && !have_seen_accept) {
+        /* This is a "reject *:*" and all previous entries were
+         * "reject something". Throw out the previous entries. */
+        for (tmp=*dest; tmp; tmp=tmp->next) {
+          if (tmp->next == ap) {
+            tmp->next = NULL;
+            addr_policy_free(*dest);
+            *dest = ap;
+            break;
+          }
+        }
+      }
+    }
+  }
+}
+
+#define DEFAULT_EXIT_POLICY                                         \
+  "reject *:25,reject *:119,reject *:135-139,reject *:445,"         \
+  "reject *:465,reject *:587,reject *:1214,reject *:4661-4666,"     \
+  "reject *:6346-6429,reject *:6699,reject *:6881-6999,accept *:*"
+
+/** Parse the exit policy <b>cfg</b> into the linked list *<b>dest</b>. If
+ * cfg doesn't end in an absolute accept or reject, add the default exit
+ * policy afterwards. If <b>rejectprivate</b> is true, prepend
+ * "reject private:*" to the policy. Return -1 if we can't parse cfg,
+ * else return 0.
+ *
+ */
+int
+config_parse_exit_policy(config_line_t *cfg, addr_policy_t **dest,
+                         int rejectprivate)
+{
+  if (rejectprivate)
+    options_append_exit_policy_string(dest, "reject private:*");
+  if (config_parse_addr_policy(cfg, dest, -1))
+    return -1;
+  options_append_exit_policy_string(dest, DEFAULT_EXIT_POLICY);
+
+  config_exit_policy_remove_redundancies(dest);
+  return 0;
+}
+
 /**
  * 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 3ab5826f6..9032e9ed2 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -1230,6 +1230,7 @@ typedef struct {
   smartlist_t *AllowUnverifiedNodes; /**< List of "entry", "middle", "exit" */
   int _AllowUnverified; /**< Bitmask; derived from AllowUnverifiedNodes; */
   config_line_t *ExitPolicy; /**< Lists of exit policy components. */
+  int ExitPolicyRejectPrivate; /**< Should we not exit to local addresses? */
   config_line_t *SocksPolicy; /**< Lists of socks policy components */
   config_line_t *DirPolicy; /**< Lists of dir policy components */
   /** Addresses to bind for listening for SOCKS connections. */
@@ -1585,11 +1586,13 @@ int resolve_my_address(or_options_t *options, uint32_t *addr,
 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);
+int config_parse_exit_policy(config_line_t *cfg,
+                             addr_policy_t **dest,
+                             int rejectprivate);
 int config_parse_addr_policy(config_line_t *cfg,
                              addr_policy_t **dest,
                              int assume_action);
 int config_cmp_addr_policies(addr_policy_t *a, addr_policy_t *b);
-void options_append_default_exit_policy(addr_policy_t **policy);
 void addr_policy_free(addr_policy_t *p);
 int option_is_recognized(const char *key);
 const char *option_get_canonical_name(const char *key);
@@ -2260,8 +2263,6 @@ void add_nickname_list_to_smartlist(smartlist_t *sl, const char *list,
                                     int must_be_running,
                                     int warn_if_down, int warn_if_unnamed);
 routerinfo_t *routerlist_find_my_routerinfo(void);
-int exit_policy_implicitly_allows_local_networks(addr_policy_t *policy,
-                                                 int warn);
 routerinfo_t *router_find_exact_exit_enclave(const char *address,
                                              uint16_t port);
 
diff --git a/src/or/router.c b/src/or/router.c
index 9775a6317..754f6c228 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -820,8 +820,8 @@ router_rebuild_descriptor(int force)
   if (options->BandwidthRate > options->MaxAdvertisedBandwidth)
     ri->bandwidthrate = (int)options->MaxAdvertisedBandwidth;
 
-  config_parse_addr_policy(get_options()->ExitPolicy, &ri->exit_policy, -1);
-  options_append_default_exit_policy(&ri->exit_policy);
+  config_parse_exit_policy(options->ExitPolicy, &ri->exit_policy,
+                           options->ExitPolicyRejectPrivate);
 
   if (desc_routerinfo) { /* inherit values */
     ri->is_verified = desc_routerinfo->is_verified;
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 575301e70..66dbf8c93 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -2465,6 +2465,7 @@ router_exit_policy_all_routers_reject(uint32_t addr, uint16_t port,
   return 1; /* all will reject. */
 }
 
+#if 0
 /**
  * If <b>policy</b> implicitly allows connections to any port in the
  * IP set <b>addr</b>/<b>mask</b>, then set *<b>policy_out</b> to the
@@ -2553,6 +2554,7 @@ exit_policy_implicitly_allows_local_networks(addr_policy_t *policy,
   return r;
 }
 
+#endif
 /** Return true iff <b>router</b> does not permit exit streams.
  */
 int
diff --git a/src/or/test.c b/src/or/test.c
index 866d97aa3..053a3ba15 100644
--- a/src/or/test.c
+++ b/src/or/test.c
@@ -1438,7 +1438,7 @@ test_exit_policies(void)
   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_assert(exit_policy_implicitly_allows_local_networks(policy, 0));
   test_assert(ADDR_POLICY_ACCEPTED ==
           router_compare_addr_to_addr_policy(0x01020304u, 2, policy));
   test_assert(ADDR_POLICY_PROBABLY_ACCEPTED ==
@@ -1448,6 +1448,7 @@ test_exit_policies(void)
 
   addr_policy_free(policy);
 
+#if 0
   /* Copied from router.c */
   policy = NULL;
   options_append_default_exit_policy(&policy);
@@ -1455,6 +1456,7 @@ test_exit_policies(void)
   test_assert(!exit_policy_implicitly_allows_local_networks(policy, 1));
 
   addr_policy_free(policy);
+#endif
 
 }