[tor-commits] [tor/release-0.3.2] Several attempts to diagnose ticket 25686

[nickm at torproject.org]

commit faf45800611a35be6a4e1381818e69852877f62e
Author: Nick Mathewson <nickm at torproject.org>
Date:   Mon Jun 11 16:14:57 2018 -0400

    Several attempts to diagnose ticket 25686
    
    There are a few reasons that relays might be uploading desciptors
    without saying X-Desc-Gen-Reason:
      1. They are running an old version of our software, before 0.3.2.stable.
      2. They are not running our software, but they are claiming they
         are.
      3. They are uploading through a proxy that strips X-Desc-Gen-Reason.
      4. They somehow had a bug in their software.
    
    According to the 25686 data, 1 is the most common reason.  This
    ticket is an attempt to diagnose case 4, or prove that case 4
    doesn't actually happen.
---
 changes/bug25686_diagnostic | 4 ++++
 src/or/directory.c          | 5 +++--
 src/or/router.c             | 8 +++++++-
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/changes/bug25686_diagnostic b/changes/bug25686_diagnostic
new file mode 100644
index 000000000..96323145d
--- /dev/null
+++ b/changes/bug25686_diagnostic
@@ -0,0 +1,4 @@
+  o Minor features (relay, diagnostic):
+    - Add several checks to detect whether Tor relays are uploading their
+      descriptors without specifying why they regenerated. Diagnostic for
+      ticket 25686.
diff --git a/src/or/directory.c b/src/or/directory.c
index f14bdde90..47058352f 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -1810,9 +1810,10 @@ directory_send_command(dir_connection_t *conn,
       tor_assert(payload);
       httpcommand = "POST";
       url = tor_strdup("/tor/");
-      if (why) {
-        smartlist_add_asprintf(headers, "X-Desc-Gen-Reason: %s\r\n", why);
+      if (!why) {
+        why = "for no reason at all";
       }
+      smartlist_add_asprintf(headers, "X-Desc-Gen-Reason: %s\r\n", why);
       break;
     }
     case DIR_PURPOSE_UPLOAD_VOTE:
diff --git a/src/or/router.c b/src/or/router.c
index 83e5538a5..cacd1132a 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -1866,7 +1866,7 @@ static routerinfo_t *desc_routerinfo = NULL;
 static extrainfo_t *desc_extrainfo = NULL;
 /** Why did we most recently decide to regenerate our descriptor?  Used to
  * tell the authorities why we're sending it to them. */
-static const char *desc_gen_reason = NULL;
+static const char *desc_gen_reason = "uninitialized reason";
 /** Since when has our descriptor been "clean"?  0 if we need to regenerate it
  * now. */
 static time_t desc_clean_since = 0;
@@ -2454,6 +2454,9 @@ router_rebuild_descriptor(int force)
   desc_clean_since = time(NULL);
   desc_needs_upload = 1;
   desc_gen_reason = desc_dirty_reason;
+  if (BUG(desc_gen_reason == NULL)) {
+    desc_gen_reason = "descriptor was marked dirty earlier, for no reason.";
+  }
   desc_dirty_reason = NULL;
   control_event_my_descriptor_changed();
   return 0;
@@ -2510,6 +2513,9 @@ void
 mark_my_descriptor_dirty(const char *reason)
 {
   const or_options_t *options = get_options();
+  if (BUG(reason == NULL)) {
+    reason = "marked descriptor dirty for unspecified reason";
+  }
   if (server_mode(options) && options->PublishServerDescriptor_)
     log_info(LD_OR, "Decided to publish new relay descriptor: %s", reason);
   desc_clean_since = 0;