Merge branch 'bug30721_squashed'

2 files changed, 296 insertions, 153 deletions

diff --git a/src/lib/net/address.c b/src/lib/net/address.c
index 546af800a9..0a2c84caf2 100644
--- a/src/lib/net/address.c
+++ b/src/lib/net/address.c
@@ -373,7 +373,8 @@ tor_addr_to_str(char *dest, const tor_addr_t *addr, size_t len, int decorate)
  *
  * If <b>accept_regular</b> is set and the address is in neither recognized
  * reverse lookup hostname format, try parsing the address as a regular
- * IPv4 or IPv6 address too.
+ * IPv4 or IPv6 address too. This mode will accept IPv6 addresses with or
+ * without square brackets.
  */
 int
 tor_addr_parse_PTR_name(tor_addr_t *result, const char *address,
@@ -1187,17 +1188,22 @@ fmt_addr32(uint32_t addr)
 }
 
 /** Convert the string in <b>src</b> to a tor_addr_t <b>addr</b>.  The string
- * may be an IPv4 address, an IPv6 address, or an IPv6 address surrounded by
- * square brackets.
+ * may be an IPv4 address, or an IPv6 address surrounded by square brackets.
  *
- *  Return an address family on success, or -1 if an invalid address string is
- *  provided. */
-int
-tor_addr_parse(tor_addr_t *addr, const char *src)
+ * If <b>allow_ipv6_without_brackets</b> is true, also allow IPv6 addresses
+ * without brackets.
+ *
+ * Always rejects IPv4 addresses with brackets.
+ *
+ * Returns an address family on success, or -1 if an invalid address string is
+ * provided. */
+static int
+tor_addr_parse_impl(tor_addr_t *addr, const char *src,
+                    bool allow_ipv6_without_brackets)
 {
   /* Holds substring of IPv6 address after removing square brackets */
   char *tmp = NULL;
-  int result;
+  int result = -1;
   struct in_addr in_tmp;
   struct in6_addr in6_tmp;
   int brackets_detected = 0;
@@ -1211,21 +1217,46 @@ tor_addr_parse(tor_addr_t *addr, const char *src)
     src = tmp = tor_strndup(src+1, strlen(src)-2);
   }
 
-  if (tor_inet_pton(AF_INET6, src, &in6_tmp) > 0) {
-    result = AF_INET6;
-    tor_addr_from_in6(addr, &in6_tmp);
-  } else if (!brackets_detected &&
-             tor_inet_pton(AF_INET, src, &in_tmp) > 0) {
-    result = AF_INET;
-    tor_addr_from_in(addr, &in_tmp);
-  } else {
-    result = -1;
+  /* Try to parse an IPv6 address if it has brackets, or if IPv6 addresses
+   * without brackets are allowed */
+  if (brackets_detected || allow_ipv6_without_brackets) {
+    if (tor_inet_pton(AF_INET6, src, &in6_tmp) > 0) {
+      result = AF_INET6;
+      tor_addr_from_in6(addr, &in6_tmp);
+    }
+  }
+
+  /* Try to parse an IPv4 address without brackets */
+  if (!brackets_detected) {
+    if (tor_inet_pton(AF_INET, src, &in_tmp) > 0) {
+      result = AF_INET;
+      tor_addr_from_in(addr, &in_tmp);
+    }
+  }
+
+  /* Clear the address on error, to avoid returning uninitialised or partly
+   * parsed data.
+   */
+  if (result == -1) {
+    memset(addr, 0, sizeof(tor_addr_t));
   }
 
   tor_free(tmp);
   return result;
 }
 
+/** Convert the string in <b>src</b> to a tor_addr_t <b>addr</b>.  The string
+ * may be an IPv4 address, an IPv6 address, or an IPv6 address surrounded by
+ * square brackets.
+ *
+ * Returns an address family on success, or -1 if an invalid address string is
+ * provided. */
+int
+tor_addr_parse(tor_addr_t *addr, const char *src)
+{
+  return tor_addr_parse_impl(addr, src, 1);
+}
+
 #ifdef HAVE_IFADDRS_TO_SMARTLIST
 /*
  * Convert a linked list consisting of <b>ifaddrs</b> structures
@@ -1718,6 +1749,11 @@ get_interface_address6_list,(int severity,
  * form "ip" or "ip:0".  Otherwise, accept those forms, and set
  * *<b>port_out</b> to <b>default_port</b>.
  *
+ * This function accepts:
+ *  - IPv6 address and port, when the IPv6 address is in square brackets,
+ *  - IPv6 address with square brackets,
+ *  - IPv6 address without square brackets.
+ *
  * Return 0 on success, -1 on failure. */
 int
 tor_addr_port_parse(int severity, const char *addrport,
@@ -1727,6 +1763,7 @@ tor_addr_port_parse(int severity, const char *addrport,
   int retval = -1;
   int r;
   char *addr_tmp = NULL;
+  bool has_port;
 
   tor_assert(addrport);
   tor_assert(address_out);
@@ -1736,28 +1773,47 @@ tor_addr_port_parse(int severity, const char *addrport,
   if (r < 0)
     goto done;
 
-  if (!*port_out) {
+  has_port = !! *port_out;
+  /* If there's no port, use the default port, or fail if there is no default
+   */
+  if (!has_port) {
     if (default_port >= 0)
       *port_out = default_port;
     else
       goto done;
   }
 
-  /* make sure that address_out is an IP address */
-  if (tor_addr_parse(address_out, addr_tmp) < 0)
+  /* Make sure that address_out is an IP address.
+   * If there is no port in addrport, allow IPv6 addresses without brackets. */
+  if (tor_addr_parse_impl(address_out, addr_tmp, !has_port) < 0)
     goto done;
 
   retval = 0;
 
  done:
+  /* Clear the address and port on error, to avoid returning uninitialised or
+   * partly parsed data.
+   */
+  if (retval == -1) {
+    memset(address_out, 0, sizeof(tor_addr_t));
+    *port_out = 0;
+  }
   tor_free(addr_tmp);
   return retval;
 }
 
 /** Given an address of the form "host[:port]", try to divide it into its host
- * and port portions, setting *<b>address_out</b> to a newly allocated string
- * holding the address portion and *<b>port_out</b> to the port (or 0 if no
- * port is given).  Return 0 on success, -1 on failure. */
+ * and port portions.
+ *
+ * Like tor_addr_port_parse(), this function accepts:
+ *  - IPv6 address and port, when the IPv6 address is in square brackets,
+ *  - IPv6 address with square brackets,
+ *  - IPv6 address without square brackets.
+ *
+ * Sets *<b>address_out</b> to a newly allocated string holding the address
+ * portion, and *<b>port_out</b> to the port (or 0 if no port is given).
+ *
+ * Return 0 on success, -1 on failure. */
 int
 tor_addr_port_split(int severity, const char *addrport,
                     char **address_out, uint16_t *port_out)
@@ -1766,8 +1822,11 @@ tor_addr_port_split(int severity, const char *addrport,
   tor_assert(addrport);
   tor_assert(address_out);
   tor_assert(port_out);
+
   /* We need to check for IPv6 manually because the logic below doesn't
-   * do a good job on IPv6 addresses that lack a port. */
+   * do a good job on IPv6 addresses that lack a port.
+   * If an IPv6 address without square brackets is ambiguous, it gets parsed
+   * here as an address, rather than address:port. */
   if (tor_addr_parse(&a_tmp, addrport) == AF_INET6) {
     *port_out = 0;
     *address_out = tor_strdup(addrport);
@@ -1807,8 +1866,7 @@ tor_addr_port_split(int severity, const char *addrport,
     tor_free(address_);
   }
 
-  if (port_out)
-    *port_out = ok ? ((uint16_t) port_) : 0;
+  *port_out = ok ? ((uint16_t) port_) : 0;
 
   return ok ? 0 : -1;
 }
diff --git a/src/lib/net/resolve.c b/src/lib/net/resolve.c
index 2dda491d14..e8d7d0d94d 100644
--- a/src/lib/net/resolve.c
+++ b/src/lib/net/resolve.c
@@ -35,6 +35,8 @@
  * *<b>addr</b> to the proper IP address, in host byte order.  Returns 0
  * on success, -1 on failure; 1 on transient failure.
  *
+ * This function only accepts IPv4 addresses.
+ *
  * (This function exists because standard windows gethostbyname
  * doesn't treat raw IP addresses properly.)
  */
@@ -45,6 +47,11 @@ tor_lookup_hostname,(const char *name, uint32_t *addr))
   tor_addr_t myaddr;
   int ret;
 
+  if (BUG(!addr))
+    return -1;
+
+  *addr = 0;
+
   if ((ret = tor_addr_lookup(name, AF_INET, &myaddr)))
     return ret;
 
@@ -56,12 +63,125 @@ tor_lookup_hostname,(const char *name, uint32_t *addr))
   return -1;
 }
 
+#ifdef HAVE_GETADDRINFO
+
+/* Host lookup helper for tor_addr_lookup(), when getaddrinfo() is
+ * available on this system.
+ *
+ * See tor_addr_lookup() for details.
+ */
+static int
+tor_addr_lookup_host_getaddrinfo(const char *name,
+                                 uint16_t family,
+                                 tor_addr_t *addr)
+{
+  int err;
+  struct addrinfo *res=NULL, *res_p;
+  struct addrinfo *best=NULL;
+  struct addrinfo hints;
+  int result = -1;
+  memset(&hints, 0, sizeof(hints));
+  hints.ai_family = family;
+  hints.ai_socktype = SOCK_STREAM;
+  err = tor_getaddrinfo(name, NULL, &hints, &res);
+  /* The check for 'res' here shouldn't be necessary, but it makes static
+   * analysis tools happy. */
+  if (!err && res) {
+    best = NULL;
+    for (res_p = res; res_p; res_p = res_p->ai_next) {
+      if (family == AF_UNSPEC) {
+        if (res_p->ai_family == AF_INET) {
+          best = res_p;
+          break;
+        } else if (res_p->ai_family == AF_INET6 && !best) {
+          best = res_p;
+        }
+      } else if (family == res_p->ai_family) {
+        best = res_p;
+        break;
+      }
+    }
+    if (!best)
+      best = res;
+    if (best->ai_family == AF_INET) {
+      tor_addr_from_in(addr,
+                       &((struct sockaddr_in*)best->ai_addr)->sin_addr);
+      result = 0;
+    } else if (best->ai_family == AF_INET6) {
+      tor_addr_from_in6(addr,
+                        &((struct sockaddr_in6*)best->ai_addr)->sin6_addr);
+      result = 0;
+    }
+    tor_freeaddrinfo(res);
+    return result;
+  }
+  return (err == EAI_AGAIN) ? 1 : -1;
+}
+
+#else /* !(defined(HAVE_GETADDRINFO)) */
+
+/* Host lookup helper for tor_addr_lookup(), which calls getaddrinfo().
+ * Used when gethostbyname() is not available on this system.
+ *
+ * See tor_addr_lookup() for details.
+ */
+static int
+tor_addr_lookup_host_gethostbyname(const char *name,
+                                   tor_addr_t *addr)
+{
+  struct hostent *ent;
+  int err;
+#ifdef HAVE_GETHOSTBYNAME_R_6_ARG
+  char buf[2048];
+  struct hostent hostent;
+  int r;
+  r = gethostbyname_r(name, &hostent, buf, sizeof(buf), &ent, &err);
+#elif defined(HAVE_GETHOSTBYNAME_R_5_ARG)
+  char buf[2048];
+  struct hostent hostent;
+  ent = gethostbyname_r(name, &hostent, buf, sizeof(buf), &err);
+#elif defined(HAVE_GETHOSTBYNAME_R_3_ARG)
+  struct hostent_data data;
+  struct hostent hent;
+  memset(&data, 0, sizeof(data));
+  err = gethostbyname_r(name, &hent, &data);
+  ent = err ? NULL : &hent;
+#else
+  ent = gethostbyname(name);
+#ifdef _WIN32
+  err = WSAGetLastError();
+#else
+  err = h_errno;
+#endif /* defined(_WIN32) */
+#endif /* defined(HAVE_GETHOSTBYNAME_R_6_ARG) || ... */
+  if (ent) {
+    if (ent->h_addrtype == AF_INET) {
+      tor_addr_from_in(addr, (struct in_addr*) ent->h_addr);
+    } else if (ent->h_addrtype == AF_INET6) {
+      tor_addr_from_in6(addr, (struct in6_addr*) ent->h_addr);
+    } else {
+      tor_assert(0); // LCOV_EXCL_LINE: gethostbyname() returned bizarre type
+    }
+    return 0;
+  }
+#ifdef _WIN32
+  return (err == WSATRY_AGAIN) ? 1 : -1;
+#else
+  return (err == TRY_AGAIN) ? 1 : -1;
+#endif
+}
+
+#endif /* defined(HAVE_GETADDRINFO) */
+
 /** Similar behavior to Unix gethostbyname: resolve <b>name</b>, and set
  * *<b>addr</b> to the proper IP address and family. The <b>family</b>
  * argument (which must be AF_INET, AF_INET6, or AF_UNSPEC) declares a
  * <i>preferred</i> family, though another one may be returned if only one
  * family is implemented for this address.
  *
+ * Like tor_addr_parse(), this function accepts IPv6 addresses with or without
+ * square brackets.
+ *
  * Return 0 on success, -1 on failure; 1 on transient failure.
  */
 MOCK_IMPL(int,
@@ -70,169 +190,134 @@ tor_addr_lookup,(const char *name, uint16_t family, tor_addr_t *addr))
   /* Perhaps eventually this should be replaced by a tor_getaddrinfo or
    * something.
    */
-  struct in_addr iaddr;
-  struct in6_addr iaddr6;
+  int parsed_family = 0;
+  int result = -1;
+
   tor_assert(name);
   tor_assert(addr);
   tor_assert(family == AF_INET || family == AF_INET6 || family == AF_UNSPEC);
+
   if (!*name) {
     /* Empty address is an error. */
-    return -1;
-  } else if (tor_inet_pton(AF_INET, name, &iaddr)) {
-    /* It's an IPv4 IP. */
-    if (family == AF_INET6)
-      return -1;
-    tor_addr_from_in(addr, &iaddr);
-    return 0;
-  } else if (tor_inet_pton(AF_INET6, name, &iaddr6)) {
-    if (family == AF_INET)
-      return -1;
-    tor_addr_from_in6(addr, &iaddr6);
-    return 0;
+    goto permfail;
+  }
+
+  /* Is it an IP address? */
+  parsed_family = tor_addr_parse(addr, name);
+
+  if (parsed_family >= 0) {
+    /* If the IP address family matches, or was unspecified */
+    if (parsed_family == family || family == AF_UNSPEC) {
+      goto success;
+    } else {
+      goto permfail;
+    }
   } else {
+    /* Clear the address after a failed tor_addr_parse(). */
+    memset(addr, 0, sizeof(tor_addr_t));
 #ifdef HAVE_GETADDRINFO
-    int err;
-    struct addrinfo *res=NULL, *res_p;
-    struct addrinfo *best=NULL;
-    struct addrinfo hints;
-    int result = -1;
-    memset(&hints, 0, sizeof(hints));
-    hints.ai_family = family;
-    hints.ai_socktype = SOCK_STREAM;
-    err = tor_getaddrinfo(name, NULL, &hints, &res);
-    /* The check for 'res' here shouldn't be necessary, but it makes static
-     * analysis tools happy. */
-    if (!err && res) {
-      best = NULL;
-      for (res_p = res; res_p; res_p = res_p->ai_next) {
-        if (family == AF_UNSPEC) {
-          if (res_p->ai_family == AF_INET) {
-            best = res_p;
-            break;
-          } else if (res_p->ai_family == AF_INET6 && !best) {
-            best = res_p;
-          }
-        } else if (family == res_p->ai_family) {
-          best = res_p;
-          break;
-        }
-      }
-      if (!best)
-        best = res;
-      if (best->ai_family == AF_INET) {
-        tor_addr_from_in(addr,
-                         &((struct sockaddr_in*)best->ai_addr)->sin_addr);
-        result = 0;
-      } else if (best->ai_family == AF_INET6) {
-        tor_addr_from_in6(addr,
-                          &((struct sockaddr_in6*)best->ai_addr)->sin6_addr);
-        result = 0;
-      }
-      tor_freeaddrinfo(res);
-      return result;
-    }
-    return (err == EAI_AGAIN) ? 1 : -1;
+    result = tor_addr_lookup_host_getaddrinfo(name, family, addr);
+    goto done;
 #else /* !(defined(HAVE_GETADDRINFO)) */
-    struct hostent *ent;
-    int err;
-#ifdef HAVE_GETHOSTBYNAME_R_6_ARG
-    char buf[2048];
-    struct hostent hostent;
-    int r;
-    r = gethostbyname_r(name, &hostent, buf, sizeof(buf), &ent, &err);
-#elif defined(HAVE_GETHOSTBYNAME_R_5_ARG)
-    char buf[2048];
-    struct hostent hostent;
-    ent = gethostbyname_r(name, &hostent, buf, sizeof(buf), &err);
-#elif defined(HAVE_GETHOSTBYNAME_R_3_ARG)
-    struct hostent_data data;
-    struct hostent hent;
-    memset(&data, 0, sizeof(data));
-    err = gethostbyname_r(name, &hent, &data);
-    ent = err ? NULL : &hent;
-#else
-    ent = gethostbyname(name);
-#ifdef _WIN32
-    err = WSAGetLastError();
-#else
-    err = h_errno;
-#endif
-#endif /* defined(HAVE_GETHOSTBYNAME_R_6_ARG) || ... */
-    if (ent) {
-      if (ent->h_addrtype == AF_INET) {
-        tor_addr_from_in(addr, (struct in_addr*) ent->h_addr);
-      } else if (ent->h_addrtype == AF_INET6) {
-        tor_addr_from_in6(addr, (struct in6_addr*) ent->h_addr);
-      } else {
-        tor_assert(0); // LCOV_EXCL_LINE: gethostbyname() returned bizarre type
-      }
-      return 0;
-    }
-#ifdef _WIN32
-    return (err == WSATRY_AGAIN) ? 1 : -1;
-#else
-    return (err == TRY_AGAIN) ? 1 : -1;
-#endif
+    result = tor_addr_lookup_host_gethostbyname(name, addr);
+    goto done;
 #endif /* defined(HAVE_GETADDRINFO) */
   }
+
+ /* If we weren't successful, and haven't already set the result,
+  * assume it's a permanent failure */
+ permfail:
+  result = -1;
+  goto done;
+ success:
+  result = 0;
+
+ /* We have set the result, now it's time to clean up */
+ done:
+  if (result) {
+    /* Clear the address on error */
+    memset(addr, 0, sizeof(tor_addr_t));
+  }
+  return result;
 }
 
 /** Parse an address or address-port combination from <b>s</b>, resolve the
  * address as needed, and put the result in <b>addr_out</b> and (optionally)
- * <b>port_out</b>.  Return 0 on success, negative on failure. */
+ * <b>port_out</b>.
+ *
+ * Like tor_addr_port_parse(), this function accepts:
+ *  - IPv6 address and port, when the IPv6 address is in square brackets,
+ *  - IPv6 address with square brackets,
+ *  - IPv6 address without square brackets.
+ *
+ * Return 0 on success, negative on failure. */
 int
 tor_addr_port_lookup(const char *s, tor_addr_t *addr_out, uint16_t *port_out)
 {
-  const char *port;
   tor_addr_t addr;
-  uint16_t portval;
+  uint16_t portval = 0;
   char *tmp = NULL;
+  int rv = 0;
+  int result;
 
   tor_assert(s);
   tor_assert(addr_out);
 
   s = eat_whitespace(s);
 
-  if (*s == '[') {
-    port = strstr(s, "]");
-    if (!port)
-      goto err;
-    tmp = tor_strndup(s+1, port-(s+1));
-    port = port+1;
-    if (*port == ':')
-      port++;
-    else
-      port = NULL;
-  } else {
-    port = strchr(s, ':');
-    if (port)
-      tmp = tor_strndup(s, port-s);
-    else
-      tmp = tor_strdup(s);
-    if (port)
-      ++port;
+  /* Try parsing s as an address:port first, so we don't have to duplicate
+   * the logic that rejects IPv6:Port with no square brackets. */
+  rv = tor_addr_port_parse(LOG_WARN, s, &addr, &portval, 0);
+  /* That was easy, no DNS required. */
+  if (rv == 0)
+    goto success;
+
+  /* Now let's check for malformed IPv6 addresses and ports:
+   * tor_addr_port_parse() requires squared brackes if there is a port,
+   * and we want tor_addr_port_lookup() to have the same requirement.
+   * But we strip the port using tor_addr_port_split(), so tor_addr_lookup()
+   * only sees the address, and will accept it without square brackets. */
+  int family = tor_addr_parse(&addr, s);
+  /* If tor_addr_parse() succeeds where tor_addr_port_parse() failed, we need
+   * to reject this address as malformed. */
+  if (family >= 0) {
+    /* Double-check it's an IPv6 address. If not, we have a parsing bug.
+     */
+    tor_assertf_nonfatal(family == AF_INET6,
+                         "Wrong family: %d (should be IPv6: %d) which "
+                         "failed IP:port parsing, but passed IP parsing. "
+                         "input string: '%s'; parsed address: '%s'.",
+                         family, AF_INET6, s, fmt_addr(&addr));
+    goto err;
   }
 
-  if (tor_addr_lookup(tmp, AF_UNSPEC, &addr) != 0)
+  /* Now we have a hostname. Let's split off the port, if any. */
+  rv = tor_addr_port_split(LOG_WARN, s, &tmp, &portval);
+  if (rv < 0)
     goto err;
-  tor_free(tmp);
 
-  if (port) {
-    portval = (int) tor_parse_long(port, 10, 1, 65535, NULL, NULL);
-    if (!portval)
-      goto err;
-  } else {
-    portval = 0;
-  }
+  /* And feed the hostname to the lookup function. */
+  if (tor_addr_lookup(tmp, AF_UNSPEC, &addr) != 0)
+    goto err;
 
+ success:
   if (port_out)
     *port_out = portval;
   tor_addr_copy(addr_out, &addr);
+  result = 0;
+  goto done;
 
-  return 0;
  err:
+  /* Clear the address and port on error */
+  memset(addr_out, 0, sizeof(tor_addr_t));
+  if (port_out)
+    *port_out = 0;
+  result = -1;
+
+ /* We have set the result, now it's time to clean up */
+ done:
   tor_free(tmp);
-  return -1;
+  return result;
 }
 
 #ifdef USE_SANDBOX_GETADDRINFO