[tor-commits] [tor/maint-0.3.5] router: Stop advertising incorrect auto IPv6 ORPorts

[nickm at torproject.org]

commit 861337fd6d286329537e07f667602992b82e921f
Author: teor <teor at 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 0000000000..f31f2ce1ad
--- /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 e91550a78c..760daa249e 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 bd6a8a012e..ab1f771017 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,