[tor-commits] [tor/release-0.4.5] config: Do not compare for duplicate ORPorts with different addresses

[dgoulet@xxxxxxxxxxxxxx]

commit dfcb050bbf424b6e72acb1bccd2dd99e8c96cb8c
Author: David Goulet <dgoulet@xxxxxxxxxxxxxx>
Date:   Thu Feb 11 16:14:56 2021 -0500

    config: Do not compare for duplicate ORPorts with different addresses
    
    We were just looking at the family which is not correct because it is possible
    to have two explicit ORPort for the same family but different addresses. One
    example is:
    
      ORPort 127.0.0.1:9001 NoAdvertise
      ORPort 1.2.3.4:9001 NoListen
    
    Thus, this patch now ignores ports that have different addresses iff they are
    both explicits. That is, if we have this example, also two different
    addresses:
    
      ORPort 9001
      ORPort 127.0.0.1:9001 NoAdvertise
    
    The first one is implicit and second one is explicit and thus we have to
    consider them for removal which in this case would remove the "ORPort 9001" in
    favor of the second port.
    
    Fixes #40289
    
    Signe-off-by: David Goulet <dgoulet@xxxxxxxxxxxxxx>
---
 changes/ticket40289              |  6 ++++
 src/feature/relay/relay_config.c | 69 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/changes/ticket40289 b/changes/ticket40289
new file mode 100644
index 0000000000..cdb36825b0
--- /dev/null
+++ b/changes/ticket40289
@@ -0,0 +1,6 @@
+  o Minor bugfixes (relay, config):
+    - Fix a problem in the removal of duplicate ORPort from the internal port
+      list when loading config file. We were removing wrong ports breaking valid
+      torrc uses cases for multiple ORPorts of the same address family. Fixes
+      bug 40289; bugfix on 0.4.5.1-alpha.
+
diff --git a/src/feature/relay/relay_config.c b/src/feature/relay/relay_config.c
index e007a3bd0d..c4a5d7f572 100644
--- a/src/feature/relay/relay_config.c
+++ b/src/feature/relay/relay_config.c
@@ -188,6 +188,41 @@ describe_relay_port(const port_cfg_t *port)
   return buf;
 }
 
+/** Return true iff port p1 is equal to p2.
+ *
+ * This does a field by field comparaison. */
+static bool
+port_cfg_eq(const port_cfg_t *p1, const port_cfg_t *p2)
+{
+  bool ret = true;
+
+  tor_assert(p1);
+  tor_assert(p2);
+
+  /* Address, port and type. */
+  ret &= tor_addr_eq(&p1->addr, &p2->addr);
+  ret &= (p1->port == p2->port);
+  ret &= (p1->type == p2->type);
+
+  /* Mode. */
+  ret &= (p1->is_unix_addr == p2->is_unix_addr);
+  ret &= (p1->is_group_writable == p2->is_group_writable);
+  ret &= (p1->is_world_writable == p2->is_world_writable);
+  ret &= (p1->relax_dirmode_check == p2->relax_dirmode_check);
+  ret &= (p1->explicit_addr == p2->explicit_addr);
+
+  /* Entry config flags. */
+  ret &= tor_memeq(&p1->entry_cfg, &p2->entry_cfg,
+                    sizeof(entry_port_cfg_t));
+  /* Server config flags. */
+  ret &= tor_memeq(&p1->server_cfg, &p2->server_cfg,
+                    sizeof(server_port_cfg_t));
+  /* Unix address path if any. */
+  ret &= !strcmp(p1->unix_addr, p2->unix_addr);
+
+  return ret;
+}
+
 /** Attempt to find duplicate ORPort that would be superseded by another and
  * remove them from the given ports list. This is possible if we have for
  * instance:
@@ -241,20 +276,42 @@ remove_duplicate_orports(smartlist_t *ports)
       if (next->type != CONN_TYPE_OR_LISTENER) {
         continue;
       }
+      /* Remove duplicates. */
+      if (port_cfg_eq(current, next)) {
+        removing[j] = true;
+        continue;
+      }
       /* Don't compare addresses of different family. */
       if (tor_addr_family(&current->addr) != tor_addr_family(&next->addr)) {
         continue;
       }
+      /* At this point, we have a port of the same type and same address
+       * family. Now, we want to avoid comparing addresses that are different
+       * but are both explicit. As an example, these are not duplicates:
+       *
+       *    ORPort 127.0.0.:9001 NoAdvertise
+       *    ORPort 1.2.3.4:9001 NoListen
+       *
+       * Any implicit address must be considered for removal since an explicit
+       * one will always supersedes it. */
+      if (!tor_addr_eq(&current->addr, &next->addr) &&
+          current->explicit_addr && next->explicit_addr) {
+        continue;
+      }
 
-      /* Same port, we keep the explicit one. */
+      /* Port value is the same so we either have a duplicate or a port that
+       * supersedes another. */
       if (current->port == next->port) {
-        removing[j] = true;
+        /* Do not remove the explicit address. As stated before above, we keep
+         * explicit addresses which supersedes implicit ones. */
         if (!current->explicit_addr && next->explicit_addr) {
-          char *next_str = tor_strdup(describe_relay_port(next));
-          log_warn(LD_CONFIG, "Configuration port %s superseded by %s",
-                   describe_relay_port(current), next_str);
-          tor_free(next_str);
+          continue;
         }
+        removing[j] = true;
+        char *next_str = tor_strdup(describe_relay_port(next));
+        log_warn(LD_CONFIG, "Configuration port %s superseded by %s",
+                 next_str, describe_relay_port(current));
+        tor_free(next_str);
       }
     }
   }



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits