commit 938623004b025c5c85745703d817e33a5308da5a Author: David Goulet dgoulet@torproject.org Date: Tue Jan 19 12:24:19 2021 -0500

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 --- src/feature/relay/relay_config.c | 25 ++++++++--- src/feature/relay/relay_config.h | 6 +++ src/test/test_config.c | 89 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 113 insertions(+), 7 deletions(-)

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));