From dc05b8549a73064337020eb3f0d8d91c8149518a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 31 Oct 2014 11:32:32 -0400 Subject: 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(-) create mode 100644 changes/bug13399 diff --git a/changes/bug13399 b/changes/bug13399 new file mode 100644 index 0000000000..bbd0b2376b --- /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 e8c11a90c2..7b826008b5 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 skip. */ 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 f858d9ffd7..fdfe8922ab 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 00994863f0..214b7c483f 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 result 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 ac6f95b3ef..c6151deb49 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, -- cgit v1.2.3-54-g00ecf From efd5001c3b94c633850d7ae9b9f5b6e1ee45ab4e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 31 Oct 2014 11:36:31 -0400 Subject: Use digest256_len in networkstatus_copy_old_consensus_info() Now, if a router ever changes its microdescriptor, but the new microdescriptor SHA256 hash has the same 160-bit prefix as the old one, we treat it as a new microdescriptor when deciding whether to copy status information. (This function also is used to compare SHA1 digests of router descriptors, but don't worry: the descriptor_digest field either holds a SHA256 hash, or a SHA1 hash padded with 0 bytes.) --- changes/bug13399 | 5 +++++ src/or/networkstatus.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/changes/bug13399 b/changes/bug13399 index bbd0b2376b..fcaf58a53c 100644 --- a/changes/bug13399 +++ b/changes/bug13399 @@ -5,3 +5,8 @@ the same first 160 bits. Fixes part of bug 13399; bugfix on 0.2.3.1-alpha. + - Reset a router's status if its microdescriptor digest changes, + even if the first 160 bits remain the same. Fixes part of bug + 13399; bugfix on 0.2.3.1-alpha. + + diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 351b20c3af..21efdd129d 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1123,7 +1123,7 @@ networkstatus_copy_old_consensus_info(networkstatus_t *new_c, rs_new->last_dir_503_at = rs_old->last_dir_503_at; if (tor_memeq(rs_old->descriptor_digest, rs_new->descriptor_digest, - DIGEST_LEN)) { /* XXXX Change this to digest256_len */ + DIGEST256_LEN)) { /* And the same descriptor too! */ memcpy(&rs_new->dl_status, &rs_old->dl_status,sizeof(download_status_t)); } -- cgit v1.2.3-54-g00ecf