commit 861337fd6d286329537e07f667602992b82e921f Author: teor <teor@torproject.org> Date: Fri Dec 20 14:30:51 2019 +1000 router: Stop advertising incorrect auto IPv6 ORPorts When IPv6 ORPorts are set to "auto", tor relays and bridges would advertise an incorrect port in their descriptor. This may be a low-severity memory safety issue, because the published port number may be derived from uninitialised or out-of-bounds memory reads. Fixes bug 32588; bugfix on 0.2.3.9-alpha. --- changes/bug32588 | 4 +++ src/feature/relay/router.c | 76 +++++++++++++++++++++++++++++----------------- src/feature/relay/router.h | 2 ++ 3 files changed, 54 insertions(+), 28 deletions(-) diff --git a/changes/bug32588 b/changes/bug32588 new file mode 100644 index 000000000..f31f2ce1a --- /dev/null +++ b/changes/bug32588 @@ -0,0 +1,4 @@ + o Minor bugfixes (relays): + - Stop advertising incorrect IPv6 ORPorts in relay and bridge descriptors, + when the IPv6 port was configured as "auto". + Fixes bug 32588; bugfix on 0.2.3.9-alpha diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index e91550a78..760daa249 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -1433,6 +1433,49 @@ router_get_advertised_or_port_by_af(const or_options_t *options, return port; } +/** As router_get_advertised_or_port(), but returns the IPv6 address and + * port in ipv6_ap_out, which must not be NULL. Returns a null address and + * zero port, if no ORPort is found. */ +void +router_get_advertised_ipv6_or_ap(const or_options_t *options, + tor_addr_port_t *ipv6_ap_out) +{ + /* Bug in calling function, we can't return a sensible result, and it + * shouldn't use the NULL pointer once we return. */ + tor_assert(ipv6_ap_out); + + /* If there is no valid IPv6 ORPort, return a null address and port. */ + tor_addr_make_null(&ipv6_ap_out->addr, AF_INET6); + ipv6_ap_out->port = 0; + + const tor_addr_t *addr = get_first_advertised_addr_by_type_af( + CONN_TYPE_OR_LISTENER, + AF_INET6); + const uint16_t port = router_get_advertised_or_port_by_af( + options, + AF_INET6); + + if (!addr || port == 0) { + log_info(LD_CONFIG, "There is no advertised IPv6 ORPort."); + return; + } + + /* Like IPv4, if the relay is configured using the default + * authorities, disallow internal IPs. Otherwise, allow them. */ + const int default_auth = using_default_dir_authorities(options); + if (! tor_addr_is_internal(addr, 0) || ! default_auth) { + tor_addr_copy(&ipv6_ap_out->addr, addr); + ipv6_ap_out->port = port; + } else { + char addrbuf[TOR_ADDR_BUF_LEN]; + log_warn(LD_CONFIG, + "Unable to use configured IPv6 address \"%s\" in a " + "descriptor. Skipping it. " + "Try specifying a globally reachable address explicitly.", + tor_addr_to_str(addrbuf, addr, sizeof(addrbuf), 1)); + } +} + /** Return the port that we should advertise as our DirPort; * this is one of three possibilities: * The one that is passed as <b>dirport</b> if the DirPort option is 0, or @@ -1848,34 +1891,11 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) sizeof(curve25519_public_key_t)); /* For now, at most one IPv6 or-address is being advertised. */ - { - const port_cfg_t *ipv6_orport = NULL; - SMARTLIST_FOREACH_BEGIN(get_configured_ports(), const port_cfg_t *, p) { - if (p->type == CONN_TYPE_OR_LISTENER && - ! p->server_cfg.no_advertise && - ! p->server_cfg.bind_ipv4_only && - tor_addr_family(&p->addr) == AF_INET6) { - /* Like IPv4, if the relay is configured using the default - * authorities, disallow internal IPs. Otherwise, allow them. */ - const int default_auth = using_default_dir_authorities(options); - if (! tor_addr_is_internal(&p->addr, 0) || ! default_auth) { - ipv6_orport = p; - break; - } else { - char addrbuf[TOR_ADDR_BUF_LEN]; - log_warn(LD_CONFIG, - "Unable to use configured IPv6 address \"%s\" in a " - "descriptor. Skipping it. " - "Try specifying a globally reachable address explicitly.", - tor_addr_to_str(addrbuf, &p->addr, sizeof(addrbuf), 1)); - } - } - } SMARTLIST_FOREACH_END(p); - if (ipv6_orport) { - tor_addr_copy(&ri->ipv6_addr, &ipv6_orport->addr); - ri->ipv6_orport = ipv6_orport->port; - } - } + tor_addr_port_t ipv6_orport; + router_get_advertised_ipv6_or_ap(options, &ipv6_orport); + /* If there is no valud IPv6 ORPort, the address and port are null. */ + tor_addr_copy(&ri->ipv6_addr, &ipv6_orport.addr); + ri->ipv6_orport = ipv6_orport.port; ri->identity_pkey = crypto_pk_dup_key(get_server_identity_key()); if (BUG(crypto_pk_get_digest(ri->identity_pkey, diff --git a/src/feature/relay/router.h b/src/feature/relay/router.h index bd6a8a012..ab1f77101 100644 --- a/src/feature/relay/router.h +++ b/src/feature/relay/router.h @@ -59,6 +59,8 @@ int init_keys_client(void); uint16_t router_get_active_listener_port_by_type_af(int listener_type, sa_family_t family); uint16_t router_get_advertised_or_port(const or_options_t *options); +void router_get_advertised_ipv6_or_ap(const or_options_t *options, + tor_addr_port_t *ipv6_ap_out); uint16_t router_get_advertised_or_port_by_af(const or_options_t *options, sa_family_t family); uint16_t router_get_advertised_dir_port(const or_options_t *options,