Merge remote-tracking branch 'tor-gitlab/mr/266' into maint-0.4.5

4 files changed, 125 insertions, 11 deletions

diff --git a/src/app/config/config.c b/src/app/config/config.c
index d8bc5f6025..7db5e5cfa8 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -6651,20 +6651,28 @@ get_first_listener_addrport_string(int listener_type)
 static const port_cfg_t *
 portconf_get_first_advertised(int listener_type, int address_family)
 {
+  const port_cfg_t *first_port = NULL;
+  const port_cfg_t *first_port_explicit_addr = NULL;
+
   if (address_family == AF_UNSPEC)
     return NULL;
 
   const smartlist_t *conf_ports = get_configured_ports();
   SMARTLIST_FOREACH_BEGIN(conf_ports, const port_cfg_t *, cfg) {
-    if (cfg->type == listener_type &&
-        !cfg->server_cfg.no_advertise) {
+    if (cfg->type == listener_type && !cfg->server_cfg.no_advertise) {
       if ((address_family == AF_INET && port_binds_ipv4(cfg)) ||
           (address_family == AF_INET6 && port_binds_ipv6(cfg))) {
-        return cfg;
+        if (cfg->explicit_addr && !first_port_explicit_addr) {
+          first_port_explicit_addr = cfg;
+        } else if (!first_port) {
+          first_port = cfg;
+        }
       }
     }
   } SMARTLIST_FOREACH_END(cfg);
-  return NULL;
+
+  /* Prefer the port with the explicit address if any. */
+  return (first_port_explicit_addr) ? first_port_explicit_addr : first_port;
 }
 
 /** Return the first advertised port of type <b>listener_type</b> in
diff --git a/src/feature/relay/relay_config.c b/src/feature/relay/relay_config.c
index e8c29fa7ed..e007a3bd0d 100644
--- a/src/feature/relay/relay_config.c
+++ b/src/feature/relay/relay_config.c
@@ -151,7 +151,7 @@ describe_portnum(int port)
 
 /** Return a static buffer containing the human readable logging string that
  * describes the given port object. */
-static const char *
+STATIC const char *
 describe_relay_port(const port_cfg_t *port)
 {
   IF_BUG_ONCE(!port) {
@@ -198,6 +198,16 @@ describe_relay_port(const port_cfg_t *port)
  * First one binds to both v4 and v6 address but second one is specific to an
  * address superseding the global bind one.
  *
+ * Another example is this one:
+ *
+ *    ORPort 9001
+ *    ORPort [4242::1]:9002
+ *    ORPort [4242::2]:9003
+ *
+ * In this case, all IPv4 and IPv6 are kept since we do allow multiple ORPorts
+ * but the published port will be the first explicit one if any to be
+ * published or else the implicit.
+ *
  * 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
@@ -231,11 +241,14 @@ remove_duplicate_orports(smartlist_t *ports)
       if (next->type != CONN_TYPE_OR_LISTENER) {
         continue;
       }
-      /* Same address family and same port number, we have a match. */
-      if (tor_addr_family(&current->addr) == tor_addr_family(&next->addr) &&
-          current->port == next->port) {
-        /* Remove current because next is explicitly set. */
-        removing[i] = true;
+      /* Don't compare addresses of different family. */
+      if (tor_addr_family(&current->addr) != tor_addr_family(&next->addr)) {
+        continue;
+      }
+
+      /* Same port, we keep the explicit one. */
+      if (current->port == next->port) {
+        removing[j] = true;
         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",
diff --git a/src/feature/relay/relay_config.h b/src/feature/relay/relay_config.h
index 671399ac0a..d36863a1a1 100644
--- a/src/feature/relay/relay_config.h
+++ b/src/feature/relay/relay_config.h
@@ -88,6 +88,12 @@ 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);
+#ifdef TOR_UNIT_TESTS
+
+struct port_cfg_t;
+STATIC const char *describe_relay_port(const struct port_cfg_t *port);
+
+#endif /* TOR_UNIT_TESTS */
 
 #endif /* defined(RELAY_CONFIG_PRIVATE) */
 
diff --git a/src/test/test_config.c b/src/test/test_config.c
index 49d7b87410..4eb4ac9cf5 100644
--- a/src/test/test_config.c
+++ b/src/test/test_config.c
@@ -6868,8 +6868,95 @@ test_config_duplicate_orports(void *arg)
   /* This will remove the [::] and the extra [::1]. */
   remove_duplicate_orports(ports);
 
-  // The explicit IPv6 port should have replaced the implicit IPv6 port.
   tt_int_op(smartlist_len(ports), OP_EQ, 2);
+  tt_str_op(describe_relay_port(smartlist_get(ports, 0)), OP_EQ,
+            "ORPort 9050");
+  tt_str_op(describe_relay_port(smartlist_get(ports, 1)), OP_EQ,
+            "ORPort [::1]:9050");
+
+  /* Reset. Test different ORPort value. */
+  SMARTLIST_FOREACH(ports, port_cfg_t *, p, port_cfg_free(p));
+  smartlist_free(ports);
+  config_free_lines(config_port);
+  config_port = NULL;
+  ports = smartlist_new();
+
+  /* Implicit port and then specific IPv6 addresses but more than one. */
+  config_line_append(&config_port, "ORPort", "9050"); // two implicit entries.
+  config_line_append(&config_port, "ORPort", "[4242::1]:9051");
+  config_line_append(&config_port, "ORPort", "[4242::2]:9051");
+
+  // 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 6 ports at this point that is:
+   *    - 0.0.0.0:9050
+   *    - [::]:9050
+   *    - [4242::1]:9051
+   *    - [4242::1]:9051
+   *    - [4242::2]:9051
+   *    - [4242::2]:9051
+   */
+  tt_int_op(smartlist_len(ports), OP_EQ, 6);
+
+  /* This will remove the [::] and the duplicates. */
+  remove_duplicate_orports(ports);
+
+  /* We have four address here, 1 IPv4 on 9050, IPv6 on 9050, IPv6 on 9051 and
+   * a different IPv6 on 9051. */
+  tt_int_op(smartlist_len(ports), OP_EQ, 3);
+  tt_str_op(describe_relay_port(smartlist_get(ports, 0)), OP_EQ,
+            "ORPort 9050");
+  tt_str_op(describe_relay_port(smartlist_get(ports, 1)), OP_EQ,
+            "ORPort [4242::1]:9051");
+  tt_str_op(describe_relay_port(smartlist_get(ports, 2)), OP_EQ,
+            "ORPort 9050");
+
+  /* Reset. Test different ORPort value. */
+  SMARTLIST_FOREACH(ports, port_cfg_t *, p, port_cfg_free(p));
+  smartlist_free(ports);
+  config_free_lines(config_port);
+  config_port = NULL;
+  ports = smartlist_new();
+
+  /* Three different ports. */
+  config_line_append(&config_port, "ORPort", "9050"); // two implicit entries.
+  config_line_append(&config_port, "ORPort", "[4242::1]:9051");
+  config_line_append(&config_port, "ORPort", "[4242::2]:9052");
+
+  // 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 6 ports at this point that is:
+   *    - 0.0.0.0:9050
+   *    - [::]:9050
+   *    - [4242::1]:9051
+   *    - [4242::1]:9051
+   *    - [4242::2]:9052
+   *    - [4242::2]:9052
+   */
+  tt_int_op(smartlist_len(ports), OP_EQ, 6);
+
+  /* This will remove the [::] and the duplicates. */
+  remove_duplicate_orports(ports);
+
+  /* We have four address here, 1 IPv4 on 9050, IPv6 on 9050, IPv6 on 9051 and
+   * IPv6 on 9052. */
+  tt_int_op(smartlist_len(ports), OP_EQ, 4);
+  tt_str_op(describe_relay_port(smartlist_get(ports, 0)), OP_EQ,
+            "ORPort 9050");
+  tt_str_op(describe_relay_port(smartlist_get(ports, 1)), OP_EQ,
+            "ORPort [4242::1]:9051");
+  tt_str_op(describe_relay_port(smartlist_get(ports, 2)), OP_EQ,
+            "ORPort [4242::2]:9052");
+  tt_str_op(describe_relay_port(smartlist_get(ports, 3)), OP_EQ,
+            "ORPort 9050");
 
  done:
   SMARTLIST_FOREACH(ports,port_cfg_t *,pf,port_cfg_free(pf));