Refactor resolve_my_address() so logs are more accurate / helpful

It returns the method by which we decided our public IP address
(explicitly configured, resolved from explicit hostname, guessed from
interfaces, learned by gethostname).

Now we can provide more helpful log messages when a relay guesses its IP
address incorrectly (e.g. due to unexpected lines in /etc/hosts). Resolves
ticket 2267.

While we're at it, stop sending a stray "(null)" in some cases for the
server status "EXTERNAL_ADDRESS" controller event. Resolves bug 8200.

changes/bug8200

@@ -0,0 +1,5 @@
+  o Minor bugfix:
+    - Stop sending a stray "(null)" in some cases for the server status
+      "EXTERNAL_ADDRESS" controller event. Resolves bug 8200; bugfix
+      on 0.1.2.6-alpha.
+

changes/ticket2267

@@ -0,0 +1,8 @@
+  o Minor features:
+    - Refactor resolve_my_address() so it returns the method by which we
+      decided our public IP address (explicitly configured, resolved from
+      explicit hostname, guessed from interfaces, learned by gethostname).
+      Now we can provide more helpful log messages when a relay guesses
+      its IP address incorrectly (e.g. due to unexpected lines in
+      /etc/hosts). Resolves ticket 2267.
+

src/or/config.c

@@ -1891,20 +1891,32 @@ list_torrc_options(void)
 /** Last value actually set by resolve_my_address. */
 static uint32_t last_resolved_addr = 0;
 /**
- * Based on <b>options-\>Address</b>, guess our public IP address and put it
- * (in host order) into *<b>addr_out</b>. If <b>hostname_out</b> is provided,
- * set *<b>hostname_out</b> to a new string holding the hostname we used to
- * get the address. Return 0 if all is well, or -1 if we can't find a suitable
+ * Use <b>options-\>Address</b> to guess our public IP address.
+ *
+ * Return 0 if all is well, or -1 if we can't find a suitable
  * public IP address.
+ *
+ * If we are returning 0:
+ *   - Put our public IP address (in host order) into *<b>addr_out</b>.
+ *   - If <b>method_out</b> is non-NULL, set *<b>method_out</b> to a static
+ *     string describing how we arrived at our answer.
+ *   - If <b>hostname_out</b> is non-NULL, and we resolved a hostname to
+ *     get our address, set *<b>hostname_out</b> to a newly allocated string
+ *     holding that hostname. (If we didn't get our address by resolving a
+ *     hostname, set *<b>hostname_out</b> to NULL.)
+ *
  * XXXX ipv6
  */
 int
 resolve_my_address(int warn_severity, const or_options_t *options,
-                   uint32_t *addr_out, char **hostname_out)
+                   uint32_t *addr_out,
+                   const char **method_out, char **hostname_out)
 {
   struct in_addr in;
   uint32_t addr; /* host order */
   char hostname[256];
+  const char *method_used;
+  const char *hostname_used;
   int explicit_ip=1;
   int explicit_hostname=1;
   int from_interface=0;
@@ -1915,6 +1927,10 @@ resolve_my_address(int warn_severity, const or_options_t *options,
 
   tor_assert(addr_out);
 
+  /*
+   * Step one: Fill in 'hostname' to be our best guess.
+   */
+
   if (address && *address) {
     strlcpy(hostname, address, sizeof(hostname));
   } else { /* then we need to guess our address */
@@ -1925,10 +1941,14 @@ resolve_my_address(int warn_severity, const or_options_t *options,
       log_fn(warn_severity, LD_NET,"Error obtaining local hostname");
       return -1;
     }
-    log_debug(LD_CONFIG,"Guessed local host name as '%s'",hostname);
+    log_debug(LD_CONFIG, "Guessed local host name as '%s'", hostname);
   }
 
-  /* now we know hostname. resolve it and keep only the IP address */
+  /*
+   * Step two: Now that we know 'hostname', parse it or resolve it. If
+   * it doesn't parse or resolve, look at the interface address. Set 'addr'
+   * to be our (host-order) 32-bit answer.
+   */
 
   if (tor_inet_aton(hostname, &in) == 0) {
     /* then we have to resolve it */
@@ -1985,6 +2005,11 @@ resolve_my_address(int warn_severity, const or_options_t *options,
                               * illformed */
   }
 
+  /*
+   * Step three: Check whether 'addr' is an internal IP address, and error
+   * out if it is and we don't want that.
+   */
+
   addr_string = tor_dup_ip(addr);
   if (is_internal_IP(addr, 0)) {
     /* make sure we're ok with publishing an internal IP */
@@ -2009,37 +2034,65 @@ resolve_my_address(int warn_severity, const or_options_t *options,
     }
   }
 
-  log_debug(LD_CONFIG, "Resolved Address to '%s'.", fmt_addr32(addr));
+  /*
+   * Step four: We have a winner! 'addr' is our answer for sure, and
+   * 'addr_string' is its string form. Fill out the various fields to
+   * say how we decided it.
+   */
+
+  log_debug(LD_CONFIG, "Resolved Address to '%s'.", addr_string);
+
+  if (explicit_ip) {
+    method_used = "CONFIGURED";
+    hostname_used = NULL;
+  } else if (explicit_hostname) {
+    method_used = "RESOLVED";
+    hostname_used = hostname;
+  } else if (from_interface) {
+    method_used = "INTERFACE";
+    hostname_used = NULL;
+  } else {
+    method_used = "GETHOSTNAME";
+    hostname_used = hostname;
+  }
+
   *addr_out = addr;
+  if (method_out)
+    *method_out = method_used;
+  if (hostname_out)
+    *hostname_out = hostname_used ? tor_strdup(hostname_used) : NULL;
+
+  /*
+   * Step five: Check if the answer has changed since last time (or if
+   * there was no last time), and if so call various functions to keep
+   * us up-to-date.
+   */
+
   if (last_resolved_addr && last_resolved_addr != *addr_out) {
     /* Leave this as a notice, regardless of the requested severity,
      * at least until dynamic IP address support becomes bulletproof. */
     log_notice(LD_NET,
-               "Your IP address seems to have changed to %s. Updating.",
-               addr_string);
+               "Your IP address seems to have changed to %s "
+               "(METHOD=%s %s%s). Updating.",
+               addr_string, method_used,
+               hostname_used ? "HOSTNAME=" : "",
+               hostname_used ? hostname_used : "");
     ip_address_changed(0);
   }
+
   if (last_resolved_addr != *addr_out) {
-    const char *method;
-    const char *h = hostname;
-    if (explicit_ip) {
-      method = "CONFIGURED";
-      h = NULL;
-    } else if (explicit_hostname) {
-      method = "RESOLVED";
-    } else if (from_interface) {
-      method = "INTERFACE";
-      h = NULL;
-    } else {
-      method = "GETHOSTNAME";
-    }
     control_event_server_status(LOG_NOTICE,
                                 "EXTERNAL_ADDRESS ADDRESS=%s METHOD=%s %s%s",
-                                addr_string, method, h?"HOSTNAME=":"", h);
+                                addr_string, method_used,
+                                hostname_used ? "HOSTNAME=" : "",
+                                hostname_used ? hostname_used : "");
   }
   last_resolved_addr = *addr_out;
-  if (hostname_out)
-    *hostname_out = tor_strdup(hostname);
+
+  /*
+   * And finally, clean up and return success.
+   */
+
   tor_free(addr_string);
   return 0;
 }
@@ -2287,7 +2340,7 @@ options_validate(or_options_t *old_options, or_options_t *options,
   if (authdir_mode(options)) {
     /* confirm that our address isn't broken, so we can complain now */
     uint32_t tmp;
-    if (resolve_my_address(LOG_WARN, options, &tmp, NULL) < 0)
+    if (resolve_my_address(LOG_WARN, options, &tmp, NULL, NULL) < 0)
       REJECT("Failed to resolve/guess local address. See logs for details.");
   }
 

src/or/config.h

@@ -27,7 +27,8 @@ setopt_err_t options_trial_assign(config_line_t *list, int use_defaults,
                                   int clear_first, char **msg);
 
 int resolve_my_address(int warn_severity, const or_options_t *options,
-                       uint32_t *addr, char **hostname_out);
+                       uint32_t *addr_out,
+                       const char **method_out, char **hostname_out);
 int is_local_addr(const tor_addr_t *addr);
 void options_init(or_options_t *options);
 char *options_dump(const or_options_t *options, int minimal);

src/or/dirserv.c

@@ -2763,11 +2763,11 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_t *private_key,
   tor_assert(private_key);
   tor_assert(cert);
 
-  if (resolve_my_address(LOG_WARN, options, &addr, &hostname)<0) {
+  if (resolve_my_address(LOG_WARN, options, &addr, NULL, &hostname)<0) {
     log_warn(LD_NET, "Couldn't resolve my hostname");
     return NULL;
   }
-  if (!strchr(hostname, '.')) {
+  if (!hostname || !strchr(hostname, '.')) {
     tor_free(hostname);
     hostname = tor_dup_ip(addr);
   }
@@ -2990,10 +2990,12 @@ generate_v2_networkstatus_opinion(void)
 
   private_key = get_server_identity_key();
 
-  if (resolve_my_address(LOG_WARN, options, &addr, &hostname)<0) {
+  if (resolve_my_address(LOG_WARN, options, &addr, NULL, &hostname)<0) {
     log_warn(LD_NET, "Couldn't resolve my hostname");
     goto done;
   }
+  if (!hostname)
+    hostname = tor_dup_ip(addr);
 
   format_iso_time(published, now);
 

src/or/router.c

@@ -1712,7 +1712,7 @@ static int router_guess_address_from_dir_headers(uint32_t *guess);
 int
 router_pick_published_address(const or_options_t *options, uint32_t *addr)
 {
-  if (resolve_my_address(LOG_INFO, options, addr, NULL) < 0) {
+  if (resolve_my_address(LOG_INFO, options, addr, NULL, NULL) < 0) {
     log_info(LD_CONFIG, "Could not determine our address locally. "
              "Checking if directory headers provide any hints.");
     if (router_guess_address_from_dir_headers(addr) < 0) {
@@ -2089,6 +2089,7 @@ check_descriptor_ipaddress_changed(time_t now)
 {
   uint32_t prev, cur;
   const or_options_t *options = get_options();
+  const char *method = NULL;
   char *hostname = NULL;
 
   (void) now;
@@ -2098,7 +2099,7 @@ check_descriptor_ipaddress_changed(time_t now)
 
   /* XXXX ipv6 */
   prev = desc_routerinfo->addr;
-  if (resolve_my_address(LOG_INFO, options, &cur, &hostname) < 0) {
+  if (resolve_my_address(LOG_INFO, options, &cur, &method, &hostname) < 0) {
     log_info(LD_CONFIG,"options->Address didn't resolve into an IP.");
     return;
   }
@@ -2110,7 +2111,10 @@ check_descriptor_ipaddress_changed(time_t now)
     tor_addr_from_ipv4h(&tmp_prev, prev);
     tor_addr_from_ipv4h(&tmp_cur, cur);
 
-    tor_asprintf(&source, "resolved from %s", hostname);
+    tor_asprintf(&source, "METHOD=%s %s%s", method,
+                 hostname ? "HOSTNAME=" : "",
+                 hostname ? hostname : "");
+
     log_addr_has_changed(LOG_NOTICE, &tmp_prev, &tmp_cur, source);
     tor_free(source);
 
@@ -2151,7 +2155,7 @@ router_new_address_suggestion(const char *suggestion,
   }
 
   /* XXXX ipv6 */
-  if (resolve_my_address(LOG_INFO, options, &cur, NULL) >= 0) {
+  if (resolve_my_address(LOG_INFO, options, &cur, NULL, NULL) >= 0) {
     /* We're all set -- we already know our address. Great. */
     tor_addr_from_ipv4h(&last_guessed_ip, cur); /* store it in case we
                                                    need it later */

src/or/routerlist.c

@@ -3951,12 +3951,14 @@ trusted_dir_server_new(const char *nickname, const char *address,
   dir_server_t *result;
 
   if (!address) { /* The address is us; we should guess. */
-    if (resolve_my_address(LOG_WARN, get_options(), &a, &hostname) < 0) {
+    if (resolve_my_address(LOG_WARN, get_options(), &a, NULL, &hostname) < 0) {
       log_warn(LD_CONFIG,
                "Couldn't find a suitable address when adding ourself as a "
                "trusted directory server.");
       return NULL;
     }
+    if (!hostname)
+      hostname = tor_dup_ip(a);
   } else {
     if (tor_lookup_hostname(address, &a)) {
       log_warn(LD_CONFIG,