diff --git a/src/app/config/config.c b/src/app/config/config.c index bf2f49ead4..4c0c57af47 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -6674,20 +6674,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 listener_type 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(¤t->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(¤t->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));