diff --git a/src/feature/relay/relay_config.c b/src/feature/relay/relay_config.c index 9273a7cef0..d3f904d286 100644 --- a/src/feature/relay/relay_config.c +++ b/src/feature/relay/relay_config.c @@ -133,6 +133,22 @@ port_warn_nonlocal_ext_orports(const smartlist_t *ports, const char *portname) } SMARTLIST_FOREACH_END(port); } +/** + * Return a static buffer describing the port number in @a port, which may + * CFG_AUTO_PORT. + **/ +static const char * +describe_portnum(int port) +{ + static char buf[16]; + if (port == CFG_AUTO_PORT) { + return "auto"; + } else { + tor_snprintf(buf, sizeof(buf), "%d", port); + return buf; + } +} + /** Return a static buffer containing the human readable logging string that * describes the given port object. */ static const char * @@ -166,8 +182,9 @@ describe_relay_port(const port_cfg_t *port) addr = ""; } - tor_snprintf(buf, sizeof(buf), "%sPort %s%s%d", - type, addr, (strlen(addr) > 0) ? ":" : "", port->port); + tor_snprintf(buf, sizeof(buf), "%sPort %s%s%s", + type, addr, (strlen(addr) > 0) ? ":" : "", + describe_portnum(port->port)); return buf; } @@ -183,11 +200,17 @@ describe_relay_port(const port_cfg_t *port) * * 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 +STATIC void remove_duplicate_orports(smartlist_t *ports) { + /* First we'll decide what to remove, then we'll remove it. */ + bool *removing = tor_calloc(smartlist_len(ports), sizeof(bool)); + for (int i = 0; i < smartlist_len(ports); ++i) { - port_cfg_t *current = smartlist_get(ports, i); + const port_cfg_t *current = smartlist_get(ports, i); + if (removing[i]) { + continue; + } /* Skip non ORPorts. */ if (current->type != CONN_TYPE_OR_LISTENER) { @@ -195,26 +218,40 @@ remove_duplicate_orports(smartlist_t *ports) } for (int j = 0; j < smartlist_len(ports); ++j) { - port_cfg_t *next = smartlist_get(ports, j); + const port_cfg_t *next = smartlist_get(ports, j); /* Avoid comparing the same object. */ if (current == next) { continue; } + if (removing[j]) { + 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) && current->port == next->port) { /* Remove current because next is explicitly set. */ - smartlist_del_keeporder(ports, i--); + 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); - port_cfg_free(current); } } } + + /* Iterate over array in reverse order to keep indices valid. */ + for (int i = smartlist_len(ports)-1; i >= 0; --i) { + tor_assert(i < smartlist_len(ports)); + if (removing[i]) { + port_cfg_t *current = smartlist_get(ports, i); + smartlist_del_keeporder(ports, i); + port_cfg_free(current); + } + } + + tor_free(removing); } /** Given a list of port_cfg_t in ports, check them for internal diff --git a/src/feature/relay/relay_config.h b/src/feature/relay/relay_config.h index c70c322d88..671399ac0a 100644 --- a/src/feature/relay/relay_config.h +++ b/src/feature/relay/relay_config.h @@ -84,6 +84,7 @@ int options_act_relay_dir(const struct or_options_t *old_options); #ifdef RELAY_CONFIG_PRIVATE +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); diff --git a/src/test/test_config.c b/src/test/test_config.c index 9eadfeed33..c116a1bbe9 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -6497,6 +6497,39 @@ test_config_getinfo_config_names(void *arg) tor_free(answer); } +static void +test_config_duplicate_orports(void *arg) +{ + (void)arg; + + config_line_t *config_port = NULL; + smartlist_t *ports = smartlist_new(); + + // Pretend that the user has specified an implicit 0.0.0.0:9050, an implicit + // [::]:9050, and an explicit on [::1]:9050. + config_line_append(&config_port, "ORPort", "9050"); // two implicit entries. + config_line_append(&config_port, "ORPort", "[::1]:9050"); + + // 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 three ports at this point. + tt_int_op(smartlist_len(ports), OP_EQ, 3); + + remove_duplicate_orports(ports); + + // The explicit IPv6 port should have replaced the implicit IPv6 port. + tt_int_op(smartlist_len(ports), OP_EQ, 2); + + done: + SMARTLIST_FOREACH(ports,port_cfg_t *,pf,port_cfg_free(pf)); + smartlist_free(ports); + config_free_lines(config_port); +} + #ifndef COCCI #define CONFIG_TEST(name, flags) \ { #name, test_config_ ## name, flags, NULL, NULL } @@ -6562,5 +6595,6 @@ struct testcase_t config_tests[] = { CONFIG_TEST(extended_fmt, 0), CONFIG_TEST(kvline_parse, 0), CONFIG_TEST(getinfo_config_names, 0), + CONFIG_TEST(duplicate_orports, 0), END_OF_TESTCASES };