[tor-commits] [tor/master] addr: Continue discovery if Address exits but not for wanted family

[nickm at torproject.org]

commit c18e52af7ce178e0dc78830e41948a9298e6d314
Author: David Goulet <dgoulet at torproject.org>
Date:   Mon Jul 20 14:20:53 2020 -0400

    addr: Continue discovery if Address exits but not for wanted family
    
    Commit b14b1f2b1d9 was a mistake.
    
    In case an Address statement is missing for the wanted family but another one
    exists for another family, simply continue the address discovery.
    
    It is not a mistake to be missing an Address statement for a family because
    the address could simply be discovered by the next methods. Not all address
    family requires a specific Address statement.
    
    However, we do bail if we couldn't find any valid address for the requested
    family _and_ a resolve failed meaning we had a hostname but couldn't resolve
    it. In that case, we can't know if that hostname would have been for v4 or v6
    thus we can't continue the address discovery properly.
    
    Couple unit tests case were removed to match this reality.
    
    Related #40025
    
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 src/app/config/resolve_addr.c | 20 ++++++++++--------
 src/test/test_config.c        | 47 -------------------------------------------
 2 files changed, 12 insertions(+), 55 deletions(-)

diff --git a/src/app/config/resolve_addr.c b/src/app/config/resolve_addr.c
index 7ec5ae565a..7368b019d6 100644
--- a/src/app/config/resolve_addr.c
+++ b/src/app/config/resolve_addr.c
@@ -194,7 +194,7 @@ get_address_from_config(const or_options_t *options, int warn_severity,
                         char **hostname_out, tor_addr_t *addr_out)
 {
   int ret;
-  bool explicit_ip = false;
+  bool explicit_ip = false, resolve_failure = false;
   int num_valid_addr = 0;
 
   tor_assert(options);
@@ -246,6 +246,7 @@ get_address_from_config(const or_options_t *options, int warn_severity,
       continue;
     } else {
       /* Hostname that can't be resolved, this is a fatal error. */
+      resolve_failure = true;
       log_fn(warn_severity, LD_CONFIG,
              "Could not resolve local Address '%s'. Failing.", cfg->value);
       continue;
@@ -253,13 +254,16 @@ get_address_from_config(const or_options_t *options, int warn_severity,
   }
 
   if (!num_valid_addr) {
-    log_fn(warn_severity, LD_CONFIG,
-           "No Address option found for family %s in configuration.",
-           fmt_af_family(family));
-    /* No Address statement for family but one exists since Address is not
-     * NULL thus we have to stop now and not attempt to send back a guessed
-     * address. */
-    return FN_RET_BAIL;
+    if (resolve_failure) {
+      /* We found no address but we got a resolution failure. This means we
+       * can know if the hostname given was v4 or v6 so we can't continue. */
+      return FN_RET_BAIL;
+    }
+    log_info(LD_CONFIG,
+             "No Address option found for family %s in configuration.",
+             fmt_af_family(family));
+    /* No Address statement for family so move on to try next method. */
+    return FN_RET_NEXT;
   }
 
   if (num_valid_addr >= MAX_CONFIG_ADDRESS) {
diff --git a/src/test/test_config.c b/src/test/test_config.c
index cdf668be49..a50e6ac927 100644
--- a/src/test/test_config.c
+++ b/src/test/test_config.c
@@ -1290,10 +1290,6 @@ test_config_find_my_address_mixed(void *arg)
                      "2a01:4f8:fff0:4f:266:37ff:fe2c:5d19");
   tor_addr_parse(&test_addr, "2a01:4f8:fff0:4f:266:37ff:fe2c:5d19");
 
-  /* IPv4 address not guessed since one Address statement exists. */
-  retval = find_my_address(options, AF_INET, LOG_NOTICE, &resolved_addr,
-                           &method_used, &hostname_out);
-  VALIDATE_FOUND_ADDRESS(false, NULL, NULL);
   /* IPv6 address should be found and considered configured. */
   retval = find_my_address(options, AF_INET6, LOG_NOTICE, &resolved_addr,
                            &method_used, &hostname_out);
@@ -1542,49 +1538,6 @@ test_config_find_my_address(void *arg)
   VALIDATE_FOUND_ADDRESS(false, NULL, NULL);
   CLEANUP_FOUND_ADDRESS;
 
-  /*
-   * Case 6: Another address family is configured. Expected to fail.
-   */
-  if (p->family == AF_INET) {
-    config_line_append(&options->Address, "Address", "4242::4242");
-  } else {
-    config_line_append(&options->Address, "Address", "1.1.1.1");
-  }
-
-  setup_full_capture_of_logs(LOG_NOTICE);
-
-  retval = find_my_address(options, p->family, LOG_NOTICE, &resolved_addr,
-                           &method_used, &hostname_out);
-
-  expect_log_msg_containing("No Address option found for family");
-  teardown_capture_of_logs();
-
-  VALIDATE_FOUND_ADDRESS(false, NULL, NULL);
-  CLEANUP_FOUND_ADDRESS;
-
-  /*
-   * Case 7: Address is a non resolvable hostname. Expected to fail.
-   */
-  MOCK(tor_addr_lookup, tor_addr_lookup_failure);
-
-  config_line_append(&options->Address, "Address", "www.torproject.org");
-  prev_n_hostname_failure = n_hostname_failure;
-
-  setup_full_capture_of_logs(LOG_NOTICE);
-
-  retval = find_my_address(options, p->family, LOG_NOTICE, &resolved_addr,
-                           &method_used, &hostname_out);
-
-  expect_log_msg_containing("Could not resolve local Address "
-                            "'www.torproject.org'. Failing.");
-  teardown_capture_of_logs();
-
-  tt_int_op(n_hostname_failure, OP_EQ, ++prev_n_hostname_failure);
-  VALIDATE_FOUND_ADDRESS(false, NULL, NULL);
-  CLEANUP_FOUND_ADDRESS;
-
-  UNMOCK(tor_addr_lookup);
-
   /*
    * Case 8:
    *    1. Address is NULL