Use digest256map for computing microdescriptor downloads

changes/bug13399

@@ -0,0 +1,7 @@
+  o Minor bugfixes:
+    - Use a full 256 bits of the SHA256 digest of a microdescriptor when
+      computing which microdescriptors to download.  This keeps us from
+      erroneous download behavior if two microdescriptor digests ever have
+      the same first 160 bits. Fixes part of bug 13399; bugfix on
+      0.2.3.1-alpha.
+

src/or/microdesc.c

@@ -163,19 +163,18 @@ microdescs_add_to_cache(microdesc_cache_t *cache,
                       md->last_listed = listed_at);
   }
   if (requested_digests256) {
-    digestmap_t *requested; /* XXXX actually we should just use a
-                               digest256map */
-    requested = digestmap_new();
+    digest256map_t *requested;
+    requested = digest256map_new();
     /* Set requested[d] to DIGEST_REQUESTED for every md we requested. */
-    SMARTLIST_FOREACH(requested_digests256, const char *, cp,
-      digestmap_set(requested, cp, DIGEST_REQUESTED));
+    SMARTLIST_FOREACH(requested_digests256, const uint8_t *, cp,
+      digest256map_set(requested, cp, DIGEST_REQUESTED));
     /* Set requested[d] to DIGEST_INVALID for every md we requested which we
      * will never be able to parse.  Remove the ones we didn't request from
      * invalid_digests.
      */
-    SMARTLIST_FOREACH_BEGIN(invalid_digests, char *, cp) {
-      if (digestmap_get(requested, cp)) {
-        digestmap_set(requested, cp, DIGEST_INVALID);
+    SMARTLIST_FOREACH_BEGIN(invalid_digests, uint8_t *, cp) {
+      if (digest256map_get(requested, cp)) {
+        digest256map_set(requested, cp, DIGEST_INVALID);
       } else {
         tor_free(cp);
         SMARTLIST_DEL_CURRENT(invalid_digests, cp);
@@ -185,8 +184,9 @@ microdescs_add_to_cache(microdesc_cache_t *cache,
      * ones we never requested from the 'descriptors' smartlist.
      */
     SMARTLIST_FOREACH_BEGIN(descriptors, microdesc_t *, md) {
-      if (digestmap_get(requested, md->digest)) {
-        digestmap_set(requested, md->digest, DIGEST_RECEIVED);
+      if (digest256map_get(requested, (const uint8_t*)md->digest)) {
+        digest256map_set(requested, (const uint8_t*)md->digest,
+                         DIGEST_RECEIVED);
       } else {
         log_fn(LOG_PROTOCOL_WARN, LD_DIR, "Received non-requested microdesc");
         microdesc_free(md);
@@ -195,14 +195,14 @@ microdescs_add_to_cache(microdesc_cache_t *cache,
     } SMARTLIST_FOREACH_END(md);
     /* Remove the ones we got or the invalid ones from requested_digests256.
      */
-    SMARTLIST_FOREACH_BEGIN(requested_digests256, char *, cp) {
-      void *status = digestmap_get(requested, cp);
+    SMARTLIST_FOREACH_BEGIN(requested_digests256, uint8_t *, cp) {
+      void *status = digest256map_get(requested, cp);
       if (status == DIGEST_RECEIVED || status == DIGEST_INVALID) {
         tor_free(cp);
         SMARTLIST_DEL_CURRENT(requested_digests256, cp);
       }
     } SMARTLIST_FOREACH_END(cp);
-    digestmap_free(requested, NULL);
+    digest256map_free(requested, NULL);
   }
 
   /* For every requested microdescriptor that was unparseable, mark it
@@ -794,7 +794,7 @@ microdesc_average_size(microdesc_cache_t *cache)
  * smartlist.  Omit all microdescriptors whose digest appear in <b>skip</b>. */
 smartlist_t *
 microdesc_list_missing_digest256(networkstatus_t *ns, microdesc_cache_t *cache,
-                                 int downloadable_only, digestmap_t *skip)
+                                 int downloadable_only, digest256map_t *skip)
 {
   smartlist_t *result = smartlist_new();
   time_t now = time(NULL);
@@ -806,7 +806,7 @@ microdesc_list_missing_digest256(networkstatus_t *ns, microdesc_cache_t *cache,
         !download_status_is_ready(&rs->dl_status, now,
                   get_options()->TestingMicrodescMaxDownloadTries))
       continue;
-    if (skip && digestmap_get(skip, rs->descriptor_digest))
+    if (skip && digest256map_get(skip, (const uint8_t*)rs->descriptor_digest))
       continue;
     if (tor_mem_is_zero(rs->descriptor_digest, DIGEST256_LEN))
       continue;
@@ -831,7 +831,7 @@ update_microdesc_downloads(time_t now)
   const or_options_t *options = get_options();
   networkstatus_t *consensus;
   smartlist_t *missing;
-  digestmap_t *pending;
+  digest256map_t *pending;
 
   if (should_delay_dir_fetches(options, NULL))
     return;
@@ -845,14 +845,14 @@ update_microdesc_downloads(time_t now)
   if (!we_fetch_microdescriptors(options))
     return;
 
-  pending = digestmap_new();
+  pending = digest256map_new();
   list_pending_microdesc_downloads(pending);
 
   missing = microdesc_list_missing_digest256(consensus,
                                              get_microdesc_cache(),
                                              1,
                                              pending);
-  digestmap_free(pending, NULL);
+  digest256map_free(pending, NULL);
 
   launch_descriptor_downloads(DIR_PURPOSE_FETCH_MICRODESC,
                               missing, NULL, now);

src/or/microdesc.h

@@ -37,7 +37,7 @@ size_t microdesc_average_size(microdesc_cache_t *cache);
 smartlist_t *microdesc_list_missing_digest256(networkstatus_t *ns,
                                               microdesc_cache_t *cache,
                                               int downloadable_only,
-                                              digestmap_t *skip);
+                                              digest256map_t *skip);
 
 void microdesc_free_(microdesc_t *md, const char *fname, int line);
 #define microdesc_free(md) \

src/or/routerlist.c

@@ -79,6 +79,7 @@ static const char *signed_descriptor_get_body_impl(
                                               const signed_descriptor_t *desc,
                                               int with_annotations);
 static void list_pending_downloads(digestmap_t *result,
+                                   digest256map_t *result256,
                                    int purpose, const char *prefix);
 static void list_pending_fpsk_downloads(fp_pair_map_t *result);
 static void launch_dummy_descriptor_download_as_needed(time_t now,
@@ -717,7 +718,8 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now)
    * First, we get the lists of already pending downloads so we don't
    * duplicate effort.
    */
-  list_pending_downloads(pending_id, DIR_PURPOSE_FETCH_CERTIFICATE, "fp/");
+  list_pending_downloads(pending_id, NULL,
+                         DIR_PURPOSE_FETCH_CERTIFICATE, "fp/");
   list_pending_fpsk_downloads(pending_cert);
 
   /*
@@ -4268,7 +4270,7 @@ clear_dir_servers(void)
  * corresponding elements of <b>result</b> to a nonzero value.
  */
 static void
-list_pending_downloads(digestmap_t *result,
+list_pending_downloads(digestmap_t *result, digest256map_t *result256,
                        int purpose, const char *prefix)
 {
   const size_t p_len = strlen(prefix);
@@ -4278,7 +4280,7 @@ list_pending_downloads(digestmap_t *result,
   if (purpose == DIR_PURPOSE_FETCH_MICRODESC)
     flags = DSR_DIGEST256|DSR_BASE64;
 
-  tor_assert(result);
+  tor_assert(result || result256);
 
   SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn) {
     if (conn->type == CONN_TYPE_DIR &&
@@ -4291,11 +4293,19 @@ list_pending_downloads(digestmap_t *result,
     }
   } SMARTLIST_FOREACH_END(conn);
 
-  SMARTLIST_FOREACH(tmp, char *, d,
+  if (result) {
+    SMARTLIST_FOREACH(tmp, char *, d,
                     {
                       digestmap_set(result, d, (void*)1);
                       tor_free(d);
                     });
+  } else if (result256) {
+    SMARTLIST_FOREACH(tmp, uint8_t *, d,
+                    {
+                      digest256map_set(result256, d, (void*)1);
+                      tor_free(d);
+                    });
+  }
   smartlist_free(tmp);
 }
 
@@ -4307,20 +4317,16 @@ list_pending_descriptor_downloads(digestmap_t *result, int extrainfo)
 {
   int purpose =
     extrainfo ? DIR_PURPOSE_FETCH_EXTRAINFO : DIR_PURPOSE_FETCH_SERVERDESC;
-  list_pending_downloads(result, purpose, "d/");
+  list_pending_downloads(result, NULL, purpose, "d/");
 }
 
 /** For every microdescriptor we are currently downloading by descriptor
- * digest, set result[d] to (void*)1.   (Note that microdescriptor digests
- * are 256-bit, and digestmap_t only holds 160-bit digests, so we're only
- * getting the first 20 bytes of each digest here.)
- *
- * XXXX Let there be a digestmap256_t, and use that instead.
+ * digest, set result[d] to (void*)1.
  */
 void
-list_pending_microdesc_downloads(digestmap_t *result)
+list_pending_microdesc_downloads(digest256map_t *result)
 {
-  list_pending_downloads(result, DIR_PURPOSE_FETCH_MICRODESC, "d/");
+  list_pending_downloads(NULL, result, DIR_PURPOSE_FETCH_MICRODESC, "d/");
 }
 
 /** For every certificate we are currently downloading by (identity digest,

src/or/routerlist.h

@@ -196,7 +196,7 @@ int hid_serv_get_responsible_directories(smartlist_t *responsible_dirs,
 int hid_serv_acting_as_directory(void);
 int hid_serv_responsible_for_desc_id(const char *id);
 
-void list_pending_microdesc_downloads(digestmap_t *result);
+void list_pending_microdesc_downloads(digest256map_t *result);
 void launch_descriptor_downloads(int purpose,
                                  smartlist_t *downloadable,
                                  const routerstatus_t *source,