[tor-commits] [tor/master] relay: Don't flag that we published if descriptor build fails

[nickm at torproject.org]

commit 743a5ef2b3781b593f5299758d0a7fc78b5816c4
Author: David Goulet <dgoulet at torproject.org>
Date:   Thu Jan 14 09:42:56 2021 -0500

    relay: Don't flag that we published if descriptor build fails
    
    In case building the descriptor would fail, we could still flag that we did in
    fact publish the descriptors leading to no more attempt at publishing it which
    in turn makes the relay silent for some hours and not try to rebuild the
    descriptor later.
    
    This has been spotted with #40231 because the operator used a localhost
    address for the ORPort and "AssumeReachable 1" leading to this code path where
    the descriptor failed to build but all conditions to "can I publish" were met.
    
    Related to #40231
    
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 src/feature/relay/relay_periodic.c |  2 +-
 src/feature/relay/router.c         | 18 +++++++++---------
 src/feature/relay/router.h         |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/feature/relay/relay_periodic.c b/src/feature/relay/relay_periodic.c
index a857ea8d92..a917d90f1a 100644
--- a/src/feature/relay/relay_periodic.c
+++ b/src/feature/relay/relay_periodic.c
@@ -104,7 +104,7 @@ rotate_onion_key_callback(time_t now, const or_options_t *options)
     log_info(LD_GENERAL,"Rotating onion key.");
     rotate_onion_key();
     cpuworkers_rotate_keyinfo();
-    if (router_rebuild_descriptor(1)<0) {
+    if (!router_rebuild_descriptor(1)) {
       log_info(LD_CONFIG, "Couldn't rebuild router descriptor");
     }
     if (advertised_server_mode() && !net_is_disabled())
diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c
index e018561556..eb1d5a63f1 100644
--- a/src/feature/relay/router.c
+++ b/src/feature/relay/router.c
@@ -1427,10 +1427,9 @@ consider_publishable_server(int force)
     return;
 
   rebuilt = router_rebuild_descriptor(0);
-  if (decide_if_publishable_server()) {
+  if (rebuilt && decide_if_publishable_server()) {
     set_server_advertised(1);
-    if (rebuilt == 0)
-      router_upload_dir_desc_to_dirservers(force);
+    router_upload_dir_desc_to_dirservers(force);
   } else {
     set_server_advertised(0);
   }
@@ -1817,7 +1816,7 @@ router_get_my_extrainfo(void)
 {
   if (!server_mode(get_options()))
     return NULL;
-  if (router_rebuild_descriptor(0))
+  if (!router_rebuild_descriptor(0))
     return NULL;
   return desc_extrainfo;
 }
@@ -2414,9 +2413,10 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
 
 /** If <b>force</b> is true, or our descriptor is out-of-date, rebuild a fresh
  * routerinfo, signed server descriptor, and extra-info document for this OR.
- * Return 0 on success, -1 on temporary error.
+ *
+ * Return true on success, else false on temporary error.
  */
-int
+bool
 router_rebuild_descriptor(int force)
 {
   int err = 0;
@@ -2424,13 +2424,13 @@ router_rebuild_descriptor(int force)
   extrainfo_t *ei;
 
   if (desc_clean_since && !force)
-    return 0;
+    return true;
 
   log_info(LD_OR, "Rebuilding relay descriptor%s", force ? " (forced)" : "");
 
   err = router_build_fresh_descriptor(&ri, &ei);
   if (err < 0) {
-    return err;
+    return false;
   }
 
   routerinfo_free(desc_routerinfo);
@@ -2446,7 +2446,7 @@ router_rebuild_descriptor(int force)
   }
   desc_dirty_reason = NULL;
   control_event_my_descriptor_changed();
-  return 0;
+  return true;
 }
 
 /** Called when we have a new set of consensus parameters. */
diff --git a/src/feature/relay/router.h b/src/feature/relay/router.h
index 2648bb5112..aa03c27142 100644
--- a/src/feature/relay/router.h
+++ b/src/feature/relay/router.h
@@ -102,7 +102,7 @@ int router_extrainfo_digest_is_me(const char *digest);
 int router_is_me(const routerinfo_t *router);
 bool router_addr_is_my_published_addr(const tor_addr_t *addr);
 int router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e);
-int router_rebuild_descriptor(int force);
+bool router_rebuild_descriptor(int force);
 char *router_dump_router_to_string(routerinfo_t *router,
                              const crypto_pk_t *ident_key,
                              const crypto_pk_t *tap_key,