Mark descriptors as undownloadable when dirserv_add_() rejects them

As of ac2f6b608a 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

@@ -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.

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