Merge branch 'tor-gitlab/mr/84'

src/feature/relay/relay_config.c

@@ -133,6 +133,22 @@ port_warn_nonlocal_ext_orports(const smartlist_t *ports, const char *portname)
   } SMARTLIST_FOREACH_END(port);
 }
 
+/**
+ * Return a static buffer describing the port number in @a port, which may
+ * CFG_AUTO_PORT.
+ **/
+static const char *
+describe_portnum(int port)
+{
+  static char buf[16];
+  if (port == CFG_AUTO_PORT) {
+    return "auto";
+  } else {
+    tor_snprintf(buf, sizeof(buf), "%d", port);
+    return buf;
+  }
+}
+
 /** Return a static buffer containing the human readable logging string that
  * describes the given port object. */
 static const char *
@@ -166,8 +182,9 @@ describe_relay_port(const port_cfg_t *port)
     addr = "";
   }
 
-  tor_snprintf(buf, sizeof(buf), "%sPort %s%s%d",
-               type, addr, (strlen(addr) > 0) ? ":" : "", port->port);
+  tor_snprintf(buf, sizeof(buf), "%sPort %s%s%s",
+               type, addr, (strlen(addr) > 0) ? ":" : "",
+               describe_portnum(port->port));
   return buf;
 }
 
@@ -183,11 +200,17 @@ describe_relay_port(const port_cfg_t *port)
  *
  * The following is O(n^2) but it is done at bootstrap or config reload and
  * the list is not very long usually. */
-static void
+STATIC void
 remove_duplicate_orports(smartlist_t *ports)
 {
+  /* First we'll decide what to remove, then we'll remove it. */
+  bool *removing = tor_calloc(smartlist_len(ports), sizeof(bool));
+
   for (int i = 0; i < smartlist_len(ports); ++i) {
-    port_cfg_t *current = smartlist_get(ports, i);
+    const port_cfg_t *current = smartlist_get(ports, i);
+    if (removing[i]) {
+      continue;
+    }
 
     /* Skip non ORPorts. */
     if (current->type != CONN_TYPE_OR_LISTENER) {
@@ -195,26 +218,40 @@ remove_duplicate_orports(smartlist_t *ports)
     }
 
     for (int j = 0; j < smartlist_len(ports); ++j) {
-      port_cfg_t *next = smartlist_get(ports, j);
+      const port_cfg_t *next = smartlist_get(ports, j);
 
       /* Avoid comparing the same object. */
       if (current == next) {
         continue;
       }
+      if (removing[j]) {
+        continue;
+      }
       /* Same address family and same port number, we have a match. */
       if (!current->explicit_addr && next->explicit_addr &&
           tor_addr_family(&current->addr) == tor_addr_family(&next->addr) &&
           current->port == next->port) {
         /* Remove current because next is explicitly set. */
-        smartlist_del_keeporder(ports, i--);
+        removing[i] = true;
         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);
-        port_cfg_free(current);
       }
     }
   }
+
+  /* Iterate over array in reverse order to keep indices valid. */
+  for (int i = smartlist_len(ports)-1; i >= 0; --i) {
+    tor_assert(i < smartlist_len(ports));
+    if (removing[i]) {
+      port_cfg_t *current = smartlist_get(ports, i);
+      smartlist_del_keeporder(ports, i);
+      port_cfg_free(current);
+    }
+  }
+
+  tor_free(removing);
 }
 
 /** Given a list of <b>port_cfg_t</b> in <b>ports</b>, check them for internal

src/feature/relay/relay_config.h

@@ -84,6 +84,7 @@ int options_act_relay_dir(const struct or_options_t *old_options);
 
 #ifdef RELAY_CONFIG_PRIVATE
 
+STATIC void remove_duplicate_orports(struct smartlist_t *ports);
 STATIC int check_bridge_distribution_setting(const char *bd);
 STATIC int have_enough_mem_for_dircache(const struct or_options_t *options,
                                         size_t total_mem, char **msg);

src/test/test_config.c

@@ -6497,6 +6497,39 @@ test_config_getinfo_config_names(void *arg)
   tor_free(answer);
 }
 
+static void
+test_config_duplicate_orports(void *arg)
+{
+  (void)arg;
+
+  config_line_t *config_port = NULL;
+  smartlist_t *ports = smartlist_new();
+
+  // Pretend that the user has specified an implicit 0.0.0.0:9050, an implicit
+  // [::]:9050, and an explicit on [::1]:9050.
+  config_line_append(&config_port, "ORPort", "9050"); // two implicit entries.
+  config_line_append(&config_port, "ORPort", "[::1]:9050");
+
+  // Parse IPv4, then IPv6.
+  port_parse_config(ports, config_port, "OR", CONN_TYPE_OR_LISTENER, "0.0.0.0",
+                    0, CL_PORT_SERVER_OPTIONS);
+  port_parse_config(ports, config_port, "OR", CONN_TYPE_OR_LISTENER, "[::]",
+                    0, CL_PORT_SERVER_OPTIONS);
+
+  // There should be three ports at this point.
+  tt_int_op(smartlist_len(ports), OP_EQ, 3);
+
+  remove_duplicate_orports(ports);
+
+  // The explicit IPv6 port should have replaced the implicit IPv6 port.
+  tt_int_op(smartlist_len(ports), OP_EQ, 2);
+
+ done:
+  SMARTLIST_FOREACH(ports,port_cfg_t *,pf,port_cfg_free(pf));
+  smartlist_free(ports);
+  config_free_lines(config_port);
+}
+
 #ifndef COCCI
 #define CONFIG_TEST(name, flags)                          \
   { #name, test_config_ ## name, flags, NULL, NULL }
@@ -6562,5 +6595,6 @@ struct testcase_t config_tests[] = {
   CONFIG_TEST(extended_fmt, 0),
   CONFIG_TEST(kvline_parse, 0),
   CONFIG_TEST(getinfo_config_names, 0),
+  CONFIG_TEST(duplicate_orports, 0),
   END_OF_TESTCASES
 };