[tor-commits] [tor/maint-0.3.0] Mark descriptors as undownloadable when dirserv_add_() rejects them

nickm at torproject.org nickm at torproject.org
Mon Oct 23 12:46:42 UTC 2017


commit f367453cb5a9ea5cb0d5664425256d7abbcbb407
Author: Nick Mathewson <nickm at torproject.org>
Date:   Tue Jun 27 12:01:46 2017 -0400

    Mark descriptors as undownloadable when dirserv_add_() rejects them
    
    As of ac2f6b608a18a8595f62384788196d7c3f2875fd in 0.2.1.19-alpha,
    Sebastian fixed bug 888 by marking descriptors as "impossible" by
    digest if they got rejected during the
    router_load_routers_from_string() phase. This fix stopped clients
    and relays from downloading the same thing over and over.
    
    But we never made the same change for descriptors rejected during
    dirserv_add_{descriptor,extrainfo}.  Instead, we tried to notice in
    advance that we'd reject them with dirserv_would_reject().
    
    This notice-in-advance check stopped working once we added
    key-pinning and didn't make a corresponding key-pinning change to
    dirserv_would_reject() [since a routerstatus_t doesn't include an
    ed25519 key].
    
    So as a fix, let's make the dirserv_add_*() functions mark digests
    as undownloadable when they are rejected.
    
    Fixes bug 22349; I am calling this a fix on 0.2.1.19-alpha, though
    you could also argue for it being a fix on 0.2.7.2-alpha.
---
 changes/bug22349 |  9 +++++++++
 src/or/dirserv.c | 56 ++++++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/changes/bug22349 b/changes/bug22349
new file mode 100644
index 000000000..bb43404bf
--- /dev/null
+++ b/changes/bug22349
@@ -0,0 +1,9 @@
+  o Minor bugfixes (directory authority):
+    - When a directory authority rejects a descriptor or extrainfo with
+      a given digest, mark that digest as undownloadable, so that we
+      do not attempt to download it again over and over. We previously
+      tried to avoid downloading such descriptors by other means, but
+      we didn't notice if we accidentally downloaded one anyway. This
+      behavior became problematic in 0.2.7.2-alpha, when authorities
+      began pinning Ed25519 keys. Fixes ticket
+      22349; bugfix on 0.2.1.19-alpha.
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index 72441081c..da34c196f 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -658,8 +658,8 @@ dirserv_add_descriptor(routerinfo_t *ri, const char **msg, const char *source)
     control_event_or_authdir_new_descriptor("REJECTED",
                ri->cache_info.signed_descriptor_body,
                                             desclen, *msg);
-    routerinfo_free(ri);
-    return ROUTER_AUTHDIR_REJECTS;
+    r = ROUTER_AUTHDIR_REJECTS;
+    goto fail;
   }
 
   /* Check whether this descriptor is semantically identical to the last one
@@ -679,8 +679,8 @@ dirserv_add_descriptor(routerinfo_t *ri, const char **msg, const char *source)
     control_event_or_authdir_new_descriptor("DROPPED",
                          ri->cache_info.signed_descriptor_body,
                                             desclen, *msg);
-    routerinfo_free(ri);
-    return ROUTER_IS_ALREADY_KNOWN;
+    r = ROUTER_IS_ALREADY_KNOWN;
+    goto fail;
   }
 
   /* Do keypinning again ... this time, to add the pin if appropriate */
@@ -703,8 +703,8 @@ dirserv_add_descriptor(routerinfo_t *ri, const char **msg, const char *source)
              "its key did not match an older RSA/Ed25519 keypair",
              router_describe(ri), source);
     *msg = "Looks like your keypair does not match its older value.";
-    routerinfo_free(ri);
-    return ROUTER_AUTHDIR_REJECTS;
+    r = ROUTER_AUTHDIR_REJECTS;
+    goto fail;
   }
 
   /* Make a copy of desc, since router_add_to_routerlist might free
@@ -742,6 +742,20 @@ dirserv_add_descriptor(routerinfo_t *ri, const char **msg, const char *source)
   tor_free(desc);
   tor_free(nickname);
   return r;
+ fail:
+  {
+    const char *desc_digest = ri->cache_info.signed_descriptor_digest;
+    download_status_t *dls =
+      router_get_dl_status_by_descriptor_digest(desc_digest);
+    if (dls) {
+      log_info(LD_GENERAL, "Marking router with descriptor %s as rejected, "
+               "and therefore undownloadable",
+               hex_str(desc_digest, DIGEST_LEN));
+      download_status_mark_impossible(dls);
+    }
+    routerinfo_free(ri);
+  }
+  return r;
 }
 
 /** As dirserv_add_descriptor, but for an extrainfo_t <b>ei</b>. */
@@ -750,6 +764,7 @@ dirserv_add_extrainfo(extrainfo_t *ei, const char **msg)
 {
   routerinfo_t *ri;
   int r;
+  was_router_added_t rv;
   tor_assert(msg);
   *msg = NULL;
 
@@ -758,8 +773,8 @@ dirserv_add_extrainfo(extrainfo_t *ei, const char **msg)
   ri = router_get_mutable_by_digest(ei->cache_info.identity_digest);
   if (!ri) {
     *msg = "No corresponding router descriptor for extra-info descriptor";
-    extrainfo_free(ei);
-    return ROUTER_BAD_EI;
+    rv = ROUTER_BAD_EI;
+    goto fail;
   }
 
   /* If it's too big, refuse it now. Otherwise we'll cache it all over the
@@ -771,17 +786,34 @@ dirserv_add_extrainfo(extrainfo_t *ei, const char **msg)
                (int)ei->cache_info.signed_descriptor_len,
                MAX_EXTRAINFO_UPLOAD_SIZE);
     *msg = "Extrainfo document was too large";
-    extrainfo_free(ei);
-    return ROUTER_BAD_EI;
+    rv = ROUTER_BAD_EI;
+    goto fail;
   }
 
   if ((r = routerinfo_incompatible_with_extrainfo(ri->identity_pkey, ei,
                                                   &ri->cache_info, msg))) {
-    extrainfo_free(ei);
-    return r < 0 ? ROUTER_IS_ALREADY_KNOWN : ROUTER_BAD_EI;
+    if (r<0) {
+      extrainfo_free(ei);
+      return ROUTER_IS_ALREADY_KNOWN;
+    }
+    rv = ROUTER_BAD_EI;
+    goto fail;
   }
   router_add_extrainfo_to_routerlist(ei, msg, 0, 0);
   return ROUTER_ADDED_SUCCESSFULLY;
+ fail:
+  {
+    const char *d = ei->cache_info.signed_descriptor_digest;
+    signed_descriptor_t *sd = router_get_by_extrainfo_digest((char*)d);
+    if (sd) {
+      log_info(LD_GENERAL, "Marking extrainfo with descriptor %s as "
+               "rejected, and therefore undownloadable",
+               hex_str((char*)d,DIGEST_LEN));
+      download_status_mark_impossible(&sd->ei_dl_status);
+    }
+    extrainfo_free(ei);
+  }
+  return rv;
 }
 
 /** Remove all descriptors whose nicknames or fingerprints no longer





More information about the tor-commits mailing list