diff --git a/changes/ticket40183 b/changes/ticket40183 new file mode 100644 index 0000000000..3c4bdf21e2 --- /dev/null +++ b/changes/ticket40183 @@ -0,0 +1,4 @@ + o Minor bugfixes (port configuration): + - Second non ORPort of a different family (ex: SocksPort [::1]:9050) was + ignored due to a logical configuration parsing error. Fixes bug 40183; + bugfix on 0.4.5.1-alpha. diff --git a/src/app/config/config.c b/src/app/config/config.c index 458067af4d..04a82a5c43 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -5934,13 +5934,12 @@ port_parse_config(smartlist_t *out, char *unix_socket_path = NULL; port_cfg_t *cfg = NULL; bool addr_is_explicit = false; - int family = -1; - - /* Parse default address. This can fail for Unix socket for instance so - * family can be -1 and the default_addr will be made UNSPEC. */ tor_addr_t default_addr = TOR_ADDR_NULL; + + /* Parse default address. This can fail for Unix socket so the default_addr + * will simply be made UNSPEC. */ if (defaultaddr) { - family = tor_addr_parse(&default_addr, defaultaddr); + tor_addr_parse(&default_addr, defaultaddr); } /* If there's no FooPort, then maybe make a default one. */ @@ -6018,7 +6017,6 @@ port_parse_config(smartlist_t *out, port = 1; } else if (!strcasecmp(addrport, "auto")) { port = CFG_AUTO_PORT; - tor_assert(family >= 0); tor_addr_copy(&addr, &default_addr); } else if (!strcasecmpend(addrport, ":auto")) { char *addrtmp = tor_strndup(addrport, strlen(addrport)-5); @@ -6035,18 +6033,12 @@ port_parse_config(smartlist_t *out, "9050" might be a valid address. */ port = (int) tor_parse_long(addrport, 10, 0, 65535, &ok, NULL); if (ok) { - tor_assert(family >= 0); tor_addr_copy(&addr, &default_addr); } else if (tor_addr_port_lookup(addrport, &addr, &ptmp) == 0) { if (ptmp == 0) { log_warn(LD_CONFIG, "%sPort line has address but no port", portname); goto err; } - if (family != -1 && tor_addr_family(&addr) != family) { - /* This means we are parsing another ORPort family but we are - * attempting to find the default address' family ORPort. */ - goto ignore; - } port = ptmp; addr_is_explicit = true; } else { diff --git a/src/feature/relay/relay_config.c b/src/feature/relay/relay_config.c index 6504c680eb..ea03f43e13 100644 --- a/src/feature/relay/relay_config.c +++ b/src/feature/relay/relay_config.c @@ -228,15 +228,16 @@ remove_duplicate_orports(smartlist_t *ports) continue; } /* Same address family and same port number, we have a match. */ - if (!current->explicit_addr && next->explicit_addr && - tor_addr_family(¤t->addr) == tor_addr_family(&next->addr) && + 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; - 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); + 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", + describe_relay_port(current), next_str); + tor_free(next_str); + } } } } diff --git a/src/test/test_config.c b/src/test/test_config.c index a0278f9422..d680ab31ef 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -6855,9 +6855,15 @@ test_config_duplicate_orports(void *arg) 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); + /* There should be 4 ports at this point that is: + * - 0.0.0.0:9050 + * - [::]:9050 + * - [::1]:9050 + * - [::1]:9050 + */ + tt_int_op(smartlist_len(ports), OP_EQ, 4); + /* This will remove the [::] and the extra [::1]. */ remove_duplicate_orports(ports); // The explicit IPv6 port should have replaced the implicit IPv6 port. @@ -6869,6 +6875,33 @@ test_config_duplicate_orports(void *arg) config_free_lines(config_port); } +static void +test_config_multifamily_port(void *arg) +{ + (void) arg; + + config_line_t *config_port = NULL; + smartlist_t *ports = smartlist_new(); + + config_line_append(&config_port, "SocksPort", "9050"); + config_line_append(&config_port, "SocksPort", "[::1]:9050"); + + // Parse IPv4, then IPv6. + port_parse_config(ports, config_port, "SOCKS", CONN_TYPE_AP_LISTENER, + "0.0.0.0", 9050, 0); + + /* There should be 2 ports at this point that is: + * - 0.0.0.0:9050 + * - [::1]:9050 + */ + tt_int_op(smartlist_len(ports), OP_EQ, 2); + + done: + SMARTLIST_FOREACH(ports, port_cfg_t *, cfg, port_cfg_free(cfg)); + smartlist_free(ports); + config_free_lines(config_port); +} + #ifndef COCCI #define CONFIG_TEST(name, flags) \ { #name, test_config_ ## name, flags, NULL, NULL } @@ -6937,5 +6970,6 @@ struct testcase_t config_tests[] = { CONFIG_TEST(kvline_parse, 0), CONFIG_TEST(getinfo_config_names, 0), CONFIG_TEST(duplicate_orports, 0), + CONFIG_TEST(multifamily_port, 0), END_OF_TESTCASES };