relay: Keep all ORPorts that are on different ports

We used to actually discard ORPorts that were the same port and same family
but they could have different address.

Instead, we need to keep all different ORPorts so we can bind a listener on
each of them. We will publish only one of these in our descriptor though.

Related to #40246

Signed-off-by: David Goulet <dgoulet@torproject.org>

diff --git a/src/feature/relay/relay_config.c b/src/feature/relay/relay_config.c
index e8c29fa7ed9f958aff2ef255334b37236a309e0d..e007a3bd0deff64b72fc6d3fe86c5e45737dd0db 100644 (file)
--- 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 671399ac0afaebdd6a43ca6ae206d636ff582abb..d36863a1a1370784e785ad7a39aa917487fffea6 100644 (file)
--- 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 49d7b874105d39112331e43c00749bcf560db244..4eb4ac9cf566310dd37fdd8f1eb7b1672de85b70 100644 (file)
--- 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));