commit dc05b8549a73064337020eb3f0d8d91c8149518a Author: Nick Mathewson nickm@torproject.org Date: Fri Oct 31 11:32:32 2014 -0400

Use digest256map for computing microdescriptor downloads --- changes/bug13399 | 7 +++++++ src/or/microdesc.c | 36 ++++++++++++++++++------------------ src/or/microdesc.h | 2 +- src/or/routerlist.c | 30 ++++++++++++++++++------------ src/or/routerlist.h | 2 +- 5 files changed, 45 insertions(+), 32 deletions(-)

diff --git a/changes/bug13399 b/changes/bug13399 new file mode 100644 index 0000000..bbd0b23 --- /dev/null +++ b/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. + diff --git a/src/or/microdesc.c b/src/or/microdesc.c index e8c11a9..7b82600 100644 --- a/src/or/microdesc.c +++ b/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); diff --git a/src/or/microdesc.h b/src/or/microdesc.h index f858d9f..fdfe892 100644 --- a/src/or/microdesc.h +++ b/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) \ diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 0099486..214b7c4 100644 --- a/src/or/routerlist.c +++ b/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, diff --git a/src/or/routerlist.h b/src/or/routerlist.h index ac6f95b..c6151de 100644 --- a/src/or/routerlist.h +++ b/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,