From e5c608e535ef9a4c4fe951a277e3891c77de4908 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 30 Nov 2016 11:06:36 +1100 Subject: [PATCH 1/5] Stop discarding consensus flavors and descriptors we wanted to fetch Instead, fetch and store consensus flavors and descriptors we wanted to fetch. And serve them if we are a directory cache (or authority). --- src/or/control.c | 2 +- src/or/dirserv.c | 5 ++++- src/or/networkstatus.c | 15 +++++++++------ src/or/networkstatus.h | 1 + src/or/routerlist.c | 2 +- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/or/control.c b/src/or/control.c index f0cb435845..a22113174a 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -2029,7 +2029,7 @@ getinfo_helper_dir(control_connection_t *control_conn, } else if (!strcmpstart(question, "dir/status/")) { *answer = tor_strdup(""); } else if (!strcmp(question, "dir/status-vote/current/consensus")) { /* v3 */ - if (directory_caches_dir_info(get_options())) { + if (we_want_to_fetch_flavor(get_options(), FLAV_NS)) { const cached_dir_t *consensus = dirserv_get_consensus("ns"); if (consensus) *answer = tor_strdup(consensus->dir); diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 7ff37b4c62..57ef1b1937 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1078,11 +1078,14 @@ directory_caches_unknown_auth_certs(const or_options_t *options) return dir_server_mode(options) || options->BridgeRelay; } -/** Return 1 if we want to keep descriptors, networkstatuses, etc around. +/** Return 1 if we want to fetch and serve descriptors, networkstatuses, etc * Else return 0. * Check options->DirPort_set and directory_permits_begindir_requests() * to see if we are willing to serve these directory documents to others via * the DirPort and begindir-over-ORPort, respectively. + * + * To check if we should fetch documents, use we_want_to_fetch_flavor and + * we_want_to_fetch_unknown_auth_certs instead of this function. */ int directory_caches_dir_info(const or_options_t *options) diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 316ce48387..6cbcaf8c01 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -814,8 +814,11 @@ networkstatus_nickname_is_unnamed(const char *nickname) #define NONAUTHORITY_NS_CACHE_INTERVAL (60*60) /** Return true iff, given the options listed in options, flavor - * is the flavor of a consensus networkstatus that we would like to fetch. */ -static int + * is the flavor of a consensus networkstatus that we would like to fetch. + * + * For certificate fetches, use we_want_to_fetch_unknown_auth_certs, and + * for serving fetched documents, use directory_caches_dir_info. */ +int we_want_to_fetch_flavor(const or_options_t *options, int flavor) { if (flavor < 0 || flavor > N_CONSENSUS_FLAVORS) { @@ -1728,9 +1731,9 @@ networkstatus_set_current_consensus(const char *consensus, } if (flav != usable_consensus_flavor() && - !directory_caches_dir_info(options)) { - /* This consensus is totally boring to us: we won't use it, and we won't - * serve it. Drop it. */ + !we_want_to_fetch_flavor(options, flav)) { + /* This consensus is totally boring to us: we won't use it, we didn't want + * it, and we won't serve it. Drop it. */ goto done; } @@ -1932,7 +1935,7 @@ networkstatus_set_current_consensus(const char *consensus, download_status_failed(&consensus_dl_status[flav], 0); } - if (directory_caches_dir_info(options)) { + if (we_want_to_fetch_flavor(options, flav)) { dirserv_set_cached_consensus_networkstatus(consensus, flavor, &c->digests, diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h index 71f36b69ed..e23eeef70f 100644 --- a/src/or/networkstatus.h +++ b/src/or/networkstatus.h @@ -66,6 +66,7 @@ const routerstatus_t *router_get_consensus_status_by_nickname( int warn_if_unnamed); const char *networkstatus_get_router_digest_by_nickname(const char *nickname); int networkstatus_nickname_is_unnamed(const char *nickname); +int we_want_to_fetch_flavor(const or_options_t *options, int flavor); void networkstatus_consensus_download_failed(int status_code, const char *flavname); void update_consensus_networkstatus_fetch_time(time_t now); diff --git a/src/or/routerlist.c b/src/or/routerlist.c index c99d22ed41..7508c4b1a6 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -5154,7 +5154,7 @@ update_consensus_router_descriptor_downloads(time_t now, int is_vote, ++n_would_reject; continue; /* We would throw it out immediately. */ } - if (!directory_caches_dir_info(options) && + if (!we_want_to_fetch_flavor(options, consensus->flavor) && !client_would_use_router(rs, now, options)) { ++n_wouldnt_use; continue; /* We would never use it ourself. */ From 65d793fab2bf0a58d00080df8320693cb3a2dbeb Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 30 Nov 2016 11:07:48 +1100 Subject: [PATCH 2/5] Fetch unknown certificates if FetchUselessDescriptors is true --- src/or/dirserv.c | 4 +++- src/or/networkstatus.c | 23 +++++++++++++++++++++++ src/or/networkstatus.h | 1 + src/or/routerlist.c | 9 +++++---- 4 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 57ef1b1937..e2a6943708 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1069,8 +1069,10 @@ directory_fetches_dir_info_later(const or_options_t *options) return options->UseBridges != 0; } -/** Return true iff we want to fetch and keep certificates for authorities +/** Return true iff we want to serve certificates for authorities * that we don't acknowledge as authorities ourself. + * Use we_want_to_fetch_unknown_auth_certs to check if we want to fetch + * and keep these certificates. */ int directory_caches_unknown_auth_certs(const or_options_t *options) diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 6cbcaf8c01..10157c6170 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -840,6 +840,29 @@ we_want_to_fetch_flavor(const or_options_t *options, int flavor) return flavor == usable_consensus_flavor(); } +/** Return true iff, given the options listed in options, we would like + * to fetch and store unknown authority certificates. + * + * For consensus and descriptor fetches, use we_want_to_fetch_flavor, and + * for serving fetched certificates, use directory_caches_unknown_auth_certs. + */ +int +we_want_to_fetch_unknown_auth_certs(const or_options_t *options) +{ + if (authdir_mode_v3(options) || + directory_caches_unknown_auth_certs((options))) { + /* We want to serve all certs to others, regardless if we would use + * them ourselves. */ + return 1; + } + if (options->FetchUselessDescriptors) { + /* Unknown certificates are definitely useless. */ + return 1; + } + /* Otherwise, don't fetch unknown certificates. */ + return 0; +} + /** How long will we hang onto a possibly live consensus for which we're * fetching certs before we check whether there is a better one? */ #define DELAY_WHILE_FETCHING_CERTS (20*60) diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h index e23eeef70f..454356e0bb 100644 --- a/src/or/networkstatus.h +++ b/src/or/networkstatus.h @@ -67,6 +67,7 @@ const routerstatus_t *router_get_consensus_status_by_nickname( const char *networkstatus_get_router_digest_by_nickname(const char *nickname); int networkstatus_nickname_is_unnamed(const char *nickname); int we_want_to_fetch_flavor(const or_options_t *options, int flavor); +int we_want_to_fetch_unknown_auth_certs(const or_options_t *options); void networkstatus_consensus_download_failed(int status_code, const char *flavname); void update_consensus_networkstatus_fetch_time(time_t now); diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 7508c4b1a6..9d5b74d8b6 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -586,7 +586,7 @@ trusted_dirs_load_certs_from_string(const char *contents, int source, "signing key %s", from_store ? "cached" : "downloaded", ds->nickname, hex_str(cert->signing_key_digest,DIGEST_LEN)); } else { - int adding = directory_caches_unknown_auth_certs(get_options()); + int adding = we_want_to_fetch_unknown_auth_certs(get_options()); log_info(LD_DIR, "%s %s certificate for unrecognized directory " "authority with signing key %s", adding ? "Adding" : "Not adding", @@ -1012,7 +1012,7 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, char *resource = NULL; cert_list_t *cl; const or_options_t *options = get_options(); - const int cache = directory_caches_unknown_auth_certs(options); + const int keep_unknown = we_want_to_fetch_unknown_auth_certs(options); fp_pair_t *fp_tmp = NULL; char id_digest_str[2*DIGEST_LEN+1]; char sk_digest_str[2*DIGEST_LEN+1]; @@ -1084,9 +1084,10 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, if (!smartlist_len(voter->sigs)) continue; /* This authority never signed this consensus, so don't * go looking for a cert with key digest 0000000000. */ - if (!cache && + if (!keep_unknown && !trusteddirserver_get_by_v3_auth_digest(voter->identity_digest)) - continue; /* We are not a cache, and we don't know this authority.*/ + continue; /* We don't want unknown certs, and we don't know this + * authority.*/ /* * If we don't know *any* cert for this authority, and a download by ID From 24e293c3c6228c0038360471e9bbd4b4b4daed05 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 30 Nov 2016 11:18:38 +1100 Subject: [PATCH 3/5] Man page update and changes file for 20667 --- changes/bug20667 | 5 +++++ doc/tor.1.txt | 12 +++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 changes/bug20667 diff --git a/changes/bug20667 b/changes/bug20667 new file mode 100644 index 0000000000..93b293a4e6 --- /dev/null +++ b/changes/bug20667 @@ -0,0 +1,5 @@ + o Minor bugfixes (directory downloads): + - Download all consensus flavors, descriptors, and authority certificates + when FetchUselessDescriptors is set, regardless of whether tor is a + directory cache or not. + Fixes bug 20667; bugfix on all recent tor versions. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 670c75d214..aa3859e0fe 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -504,11 +504,13 @@ GENERAL OPTIONS (Default: 1) [[FetchUselessDescriptors]] **FetchUselessDescriptors** **0**|**1**:: - If set to 1, Tor will fetch every non-obsolete descriptor from the - authorities that it hears about. Otherwise, it will avoid fetching useless - descriptors, for example for routers that are not running. This option is - useful if you're using the contributed "exitlist" script to enumerate Tor - nodes that exit to certain addresses. (Default: 0) + If set to 1, Tor will fetch every consensus flavor, descriptor, and + certificate that it hears about. Otherwise, it will avoid fetching useless + descriptors: flavors that it is not using to build circuits, and authority + certificates it does not trust. This option is useful if you're using a + tor client with an external parser that uses a full consensus. + This option fetches all documents, **DirCache** fetches and serves + all documents. (Default: 0) [[HTTPProxy]] **HTTPProxy** __host__[:__port__]:: Tor will make all its directory requests through this host:port (or host:80 From e061cf4d1d7e9d8533cde936f0904d4f7b877746 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 30 Nov 2016 11:43:04 +1100 Subject: [PATCH 4/5] Remove an unneccessary FetchUselessDescriptors check in client_would_use_router --- src/or/networkstatus.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 10157c6170..bfb36413ce 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -2407,9 +2407,9 @@ int client_would_use_router(const routerstatus_t *rs, time_t now, const or_options_t *options) { - if (!rs->is_flagged_running && !options->FetchUselessDescriptors) { + if (!rs->is_flagged_running) { /* If we had this router descriptor, we wouldn't even bother using it. - * But, if we want to have a complete list, fetch it anyway. */ + * (Fetching and storing depends on by we_want_to_fetch_flavor().) */ return 0; } if (rs->published_on + options->TestingEstimatedDescriptorPropagationTime From 04f794489107bab6316d60ade7228d4443beb3a6 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 30 Nov 2016 12:20:28 +1100 Subject: [PATCH 5/5] Stop discarding downloaded full descriptors when using microdescs for circuits This affects clients with FetchUselessDescriptors 1. It might also cause subtle bugs on directory mirrors and authorities, causing them to consider all full descriptors as failed or old. --- changes/bug20839 | 5 +++++ src/or/routerlist.c | 9 +++++---- 2 files changed, 10 insertions(+), 4 deletions(-) create mode 100644 changes/bug20839 diff --git a/changes/bug20839 b/changes/bug20839 new file mode 100644 index 0000000000..c290097d80 --- /dev/null +++ b/changes/bug20839 @@ -0,0 +1,5 @@ + o Minor bugfixes (descriptors): + - Correctly recognise downloaded full descriptors as valid, even when + using microdescriptors as circuits. This affects clients with + FetchUselessDescriptors set, and may affect directory authorities. + Fixes bug 20839; bugfix on commit 6083276 in 0.2.3.2-alpha. diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 9d5b74d8b6..9bcca76b63 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -3896,7 +3896,7 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg, router_describe(router)); *msg = "Router descriptor is not referenced by any network-status."; - /* Only journal this desc if we'll be serving it. */ + /* Only journal this desc if we want to keep old descriptors */ if (!from_cache && should_cache_old_descriptors()) signed_desc_append_to_journal(&router->cache_info, &routerlist->desc_store); @@ -4526,13 +4526,14 @@ router_load_extrainfo_from_string(const char *s, const char *eos, smartlist_free(extrainfo_list); } -/** Return true iff any networkstatus includes a descriptor whose digest - * is that of desc. */ +/** Return true iff the latest ns-flavored consensus includes a descriptor + * whose digest is that of desc. */ static int signed_desc_digest_is_recognized(signed_descriptor_t *desc) { const routerstatus_t *rs; - networkstatus_t *consensus = networkstatus_get_latest_consensus(); + networkstatus_t *consensus = networkstatus_get_latest_consensus_by_flavor( + FLAV_NS); if (consensus) { rs = networkstatus_vote_find_entry(consensus, desc->identity_digest);