clean up the Reachable*Addresses changes

svn:r6041

3 files changed, 48 insertions, 46 deletions

diff --git a/src/or/config.c b/src/or/config.c
index 47920aca40..e9449c0d42 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -198,8 +198,8 @@ static config_var_t _option_vars[] = {
   VAR("PidFile",             STRING,   PidFile,              NULL),
   VAR("ProtocolWarnings",    BOOL,     ProtocolWarnings,     "0"),
   VAR("ReachableAddresses",  LINELIST, ReachableAddresses,   NULL),
-  VAR("ReachableORAddresses",LINELIST, ReachableORAddresses,   NULL),
-  VAR("ReachableDirAddresses",LINELIST, ReachableDirAddresses,   NULL),
+  VAR("ReachableDirAddresses",LINELIST,ReachableDirAddresses,NULL),
+  VAR("ReachableORAddresses",LINELIST, ReachableORAddresses, NULL),
   VAR("RecommendedVersions", LINELIST, RecommendedVersions,  NULL),
   VAR("RecommendedClientVersions", LINELIST, RecommendedClientVersions,  NULL),
   VAR("RecommendedServerVersions", LINELIST, RecommendedServerVersions,  NULL),
@@ -1839,34 +1839,39 @@ parse_reachable_addresses(void)
 
   if (options->ReachableDirAddresses &&
       options->ReachableORAddresses &&
-      options->ReachableAddresses)
-    log_warn(LD_CONFIG, "Both ReachableDirAddresses and ReachableORAddresses are set. "
-                        "ReachableAddresses setting will be ignored.");
-
+      options->ReachableAddresses) {
+    log_warn(LD_CONFIG,
+             "Both ReachableDirAddresses and ReachableORAddresses are set. "
+             "ReachableAddresses setting will be ignored.");
+  }
   addr_policy_free(reachable_or_addr_policy);
   reachable_or_addr_policy = NULL;
   if (!options->ReachableORAddresses && options->ReachableAddresses)
-    log_notice(LD_CONFIG, "Using ReachableAddresses for "
-                               "ReachableORAddresses");
+    log_info(LD_CONFIG,
+             "Using ReachableAddresses as ReachableORAddresses.");
   if (config_parse_addr_policy(options->ReachableORAddresses ?
-                               options->ReachableORAddresses :
-                               options->ReachableAddresses,
+                                 options->ReachableORAddresses :
+                                 options->ReachableAddresses,
                                &reachable_or_addr_policy,
                                ADDR_POLICY_ACCEPT)) {
-    log_warn(LD_CONFIG, "Error in ReachableORAddresses entry; ignoring.");
+    log_warn(LD_CONFIG,
+             "Error parsing Reachable%sAddresses entry; ignoring.",
+             options->ReachableORAddresses ? "OR" : "");
   }
 
   addr_policy_free(reachable_dir_addr_policy);
   reachable_dir_addr_policy = NULL;
   if (!options->ReachableDirAddresses && options->ReachableAddresses)
-    log_notice(LD_CONFIG, "Using ReachableAddresses for "
-                               "ReachableDirAddresses");
+    log_info(LD_CONFIG,
+             "Using ReachableAddresses as ReachableDirAddresses");
   if (config_parse_addr_policy(options->ReachableDirAddresses ?
                                options->ReachableDirAddresses :
                                options->ReachableAddresses,
                                &reachable_dir_addr_policy,
                                ADDR_POLICY_ACCEPT)) {
-    log_warn(LD_CONFIG, "Error in ReachableDirAddresses entry; ignoring.");
+    if (options->ReachableDirAddresses)
+      log_warn(LD_CONFIG,
+               "Error parsing ReachableDirAddresses entry; ignoring.");
   }
 }
 
@@ -1879,22 +1884,16 @@ firewall_is_fascist_or(void)
   return !!reachable_or_addr_policy;
 }
 
-/** Return true iff we are configured to think that the local fascist
- * firewall (if any) will allow a connection to <b>addr</b>:<b>port</b>.
- *
- * If dir_or_or is 1 then it consults ReachableDirAddresses,
- * if it is 2, then ReachableORAddresses are consulted.
- * */
-int
-_fascist_firewall_allows_address(uint32_t addr, uint16_t port, int dir_or_or)
+/** Return true iff <b>policy</b> (possibly NULL) will allow a
+ * connection to <b>addr</b>:<b>port</b>.
+ */
+static int
+_fascist_firewall_allows_address(uint32_t addr, uint16_t port,
+                                 addr_policy_t *policy)
 {
   addr_policy_result_t p;
 
-  assert(dir_or_or == 1 || dir_or_or == 2);
-  p = router_compare_addr_to_addr_policy(
-               addr, port, dir_or_or == 1 ?
-                           reachable_dir_addr_policy :
-                           reachable_or_addr_policy);
+  p = router_compare_addr_to_addr_policy(addr, port, policy);
 
   switch (p) {
     case ADDR_POLICY_PROBABLY_ACCEPTED:
@@ -1912,13 +1911,15 @@ _fascist_firewall_allows_address(uint32_t addr, uint16_t port, int dir_or_or)
 int
 fascist_firewall_allows_address_or(uint32_t addr, uint16_t port)
 {
-  return _fascist_firewall_allows_address(addr, port, 2);
+  return _fascist_firewall_allows_address(addr, port,
+                                          reachable_or_addr_policy);
 }
 
 int
 fascist_firewall_allows_address_dir(uint32_t addr, uint16_t port)
 {
-  return _fascist_firewall_allows_address(addr, port, 1);
+  return _fascist_firewall_allows_address(addr, port,
+                                          reachable_dir_addr_policy);
 }
 
 /** Return 0 if every setting in <b>options</b> is reasonable.  Else
@@ -2119,8 +2120,8 @@ options_validate(or_options_t *old_options, or_options_t *options,
   if (options->FascistFirewall && !options->ReachableAddresses) {
     if (smartlist_len(options->FirewallPorts)) {
       /* We already have firewall ports set, so migrate them to
-       * ReachableAddresses, which will set ReachableOR and ReachableDir-
-       * Addresses if they aren't set otherwise*/
+       * ReachableAddresses, which will set ReachableORAddresses and
+       * ReachableDirAddresses if they aren't set explicitly. */
       smartlist_t *instead = smartlist_create();
       config_line_t *new_line = tor_malloc_zero(sizeof(config_line_t));
       new_line->key = tor_strdup("ReachableAddresses");
@@ -2137,15 +2138,16 @@ options_validate(or_options_t *old_options, or_options_t *options,
       });
       new_line->value = smartlist_join_strings(instead,",",0,NULL);
       /* These have been deprecated since 0.1.1.5-alpha-cvs */
-      log(LOG_NOTICE, LD_CONFIG, "Converting FascistFirewall and FirewallPorts "
+      log(LOG_NOTICE, LD_CONFIG,
+          "Converting FascistFirewall and FirewallPorts "
           "config options to new format: \"ReachableAddresses %s\"",
           new_line->value);
       options->ReachableAddresses = new_line;
       SMARTLIST_FOREACH(instead, char *, cp, tor_free(cp));
       smartlist_free(instead);
     } else {
-      /* We do not have FirewallPorts set, so add 80 to ReachableDir-,
-       * and 443 to ReachableORAddresses */
+      /* We do not have FirewallPorts set, so add 80 to
+       * ReachableDirAddresses, and 443 to ReachableORAddresses. */
       if (!options->ReachableDirAddresses) {
         config_line_t *new_line = tor_malloc_zero(sizeof(config_line_t));
         new_line->key = tor_strdup("ReachableDirAddresses");
@@ -2165,11 +2167,11 @@ options_validate(or_options_t *old_options, or_options_t *options,
     }
   }
 
-  for (i=0; i<3; i++){
-    config_line_t **linep = 
+  for (i=0; i<3; i++) {
+    config_line_t **linep =
       (i==0) ? &options->ReachableAddresses :
-      (i==1) ? &options->ReachableORAddresses :
-               &options->ReachableDirAddresses;
+        (i==1) ? &options->ReachableORAddresses :
+                 &options->ReachableDirAddresses;
     if (!*linep)
       continue;
     /* We need to end with a reject *:*, not an implicit accept *:* */
@@ -2179,7 +2181,10 @@ options_validate(or_options_t *old_options, or_options_t *options,
       linep = &((*linep)->next);
       if (!*linep) {
         *linep = tor_malloc_zero(sizeof(config_line_t));
-        (*linep)->key = tor_strdup("ReachableAddresses");
+        (*linep)->key = tor_strdup(
+          (i==0) ?  "ReachableAddresses" :
+            (i==1) ? "ReachableORAddresses" :
+                     "ReachableDirAddresses");
         (*linep)->value = tor_strdup("reject *:*");
         break;
       }
diff --git a/src/or/directory.c b/src/or/directory.c
index 48c202edc3..7543144f0d 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -146,7 +146,7 @@ directory_post_to_dirservers(uint8_t purpose, const char *payload,
       if (post_to_v1_only && !ds->is_v1_authority)
         continue;
       post_via_tor = purpose_is_private(purpose) ||
-                     !fascist_firewall_allows_address_dir(ds->addr,ds->dir_port);
+              !fascist_firewall_allows_address_dir(ds->addr, ds->dir_port);
       directory_initiate_command_routerstatus(rs, purpose, post_via_tor,
                                               NULL, payload, payload_len);
     });
diff --git a/src/or/or.h b/src/or/or.h
index c9c3ca46b0..189f39a681 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -1276,12 +1276,9 @@ typedef struct {
   int FascistFirewall; /**< Whether to prefer ORs reachable on open ports. */
   smartlist_t *FirewallPorts; /**< Which ports our firewall allows
                                * (strings). */
-  config_line_t *ReachableAddresses; /**< Which IP:ports our firewall allows
-                                      * (exit policy.) */
-  config_line_t *ReachableORAddresses; /**< Which IP:ports our firewall allows
-                                        * (exit policy.) */
-  config_line_t *ReachableDirAddresses; /**< Which IP:ports our firewall allows
-                                         * (exit policy.) */
+  config_line_t *ReachableAddresses; /**< IP:ports our firewall allows. */
+  config_line_t *ReachableORAddresses; /**< IP:ports for OR conns. */
+  config_line_t *ReachableDirAddresses; /**< IP:ports for Dir conns. */
 
   /** Application ports that require all nodes in circ to have sufficient
    * uptime. */