From 596ccbf8396b10fb404594c201bd04ef135b330f Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Fri, 1 Jul 2016 09:35:27 +1000 Subject: Refactor authority_certs_fetch_missing to call get_options once --- src/or/routerlist.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'src/or/routerlist.c') diff --git a/src/or/routerlist.c b/src/or/routerlist.c index d8319afe19..296dbd8b0e 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -865,12 +865,13 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, smartlist_t *missing_cert_digests, *missing_id_digests; char *resource = NULL; cert_list_t *cl; - const int cache = directory_caches_unknown_auth_certs(get_options()); + const or_options_t *options = get_options(); + const int cache = directory_caches_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]; - if (should_delay_dir_fetches(get_options(), NULL)) + if (should_delay_dir_fetches(options, NULL)) return; pending_cert = fp_pair_map_new(); @@ -911,7 +912,7 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, } SMARTLIST_FOREACH_END(cert); if (!found && download_status_is_ready(&(cl->dl_status_by_id), now, - get_options()->TestingCertMaxDownloadTries) && + options->TestingCertMaxDownloadTries) && !digestmap_get(pending_id, ds->v3_identity_digest)) { log_info(LD_DIR, "No current certificate known for authority %s " @@ -973,7 +974,7 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, } if (download_status_is_ready_by_sk_in_cl( cl, sig->signing_key_digest, - now, get_options()->TestingCertMaxDownloadTries) && + now, options->TestingCertMaxDownloadTries) && !fp_pair_map_get_by_digests(pending_cert, voter->identity_digest, sig->signing_key_digest)) { @@ -1075,7 +1076,7 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, * directory */ if (rs) { /* Certificate fetches are one-hop, unless AllDirActionsPrivate is 1 */ - int get_via_tor = get_options()->AllDirActionsPrivate; + int get_via_tor = options->AllDirActionsPrivate; const dir_indirection_t indirection = get_via_tor ? DIRIND_ANONYMOUS : DIRIND_ONEHOP; directory_initiate_command_routerstatus(rs, @@ -1136,7 +1137,7 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, * directory */ if (rs) { /* Certificate fetches are one-hop, unless AllDirActionsPrivate is 1 */ - int get_via_tor = get_options()->AllDirActionsPrivate; + int get_via_tor = options->AllDirActionsPrivate; const dir_indirection_t indirection = get_via_tor ? DIRIND_ANONYMOUS : DIRIND_ONEHOP; directory_initiate_command_routerstatus(rs, -- cgit v1.2.3-54-g00ecf From b4dcf56768e4b8c27fdade8f5f936dd7c651aeb0 Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Fri, 1 Jul 2016 09:38:41 +1000 Subject: Hex-encode raw digest before printing in authority_certs_fetch_missing --- src/or/routerlist.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/or/routerlist.c') diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 296dbd8b0e..6ad276a913 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -1038,7 +1038,8 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, if (!rs) { log_warn(LD_BUG, "Directory %s delivered a consensus, but a " - "routerstatus could not be found for it.", dir_hint); + "routerstatus could not be found for it.", + hex_str(dir_hint, DIGEST_LEN)); } } -- cgit v1.2.3-54-g00ecf From d3ca6fe475ab5e0cc9c80e0ff984bebe4160f689 Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Fri, 1 Jul 2016 10:08:38 +1000 Subject: Call purpose_needs_anonymity in authority_certs_fetch_missing --- src/or/directory.c | 2 +- src/or/directory.h | 4 ++-- src/or/routerlist.c | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) (limited to 'src/or/routerlist.c') diff --git a/src/or/directory.c b/src/or/directory.c index 1fe4d6804d..68ec3b7503 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -122,7 +122,7 @@ static void connection_dir_close_consensus_fetches( /** Return true iff the directory purpose dir_purpose (and if it's * fetching descriptors, it's fetching them for router_purpose) * must use an anonymous connection to a directory. */ -STATIC int +int purpose_needs_anonymity(uint8_t dir_purpose, uint8_t router_purpose) { if (get_options()->AllDirActionsPrivate) diff --git a/src/or/directory.h b/src/or/directory.h index afa3bcc611..f04e7ab315 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -132,12 +132,12 @@ int download_status_get_n_failures(const download_status_t *dls); int download_status_get_n_attempts(const download_status_t *dls); time_t download_status_get_next_attempt_at(const download_status_t *dls); +int purpose_needs_anonymity(uint8_t dir_purpose, uint8_t router_purpose); + #ifdef TOR_UNIT_TESTS /* Used only by directory.c and test_dir.c */ STATIC int parse_http_url(const char *headers, char **url); -STATIC int purpose_needs_anonymity(uint8_t dir_purpose, - uint8_t router_purpose); STATIC dirinfo_type_t dir_fetch_type(int dir_purpose, int router_purpose, const char *resource); STATIC int directory_handle_command_get(dir_connection_t *conn, diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 6ad276a913..cae0241343 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -1076,8 +1076,8 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, /* If we've just downloaded a consensus from a directory, re-use that * directory */ if (rs) { - /* Certificate fetches are one-hop, unless AllDirActionsPrivate is 1 */ - int get_via_tor = options->AllDirActionsPrivate; + int get_via_tor = purpose_needs_anonymity( + DIR_PURPOSE_FETCH_CERTIFICATE, 0); const dir_indirection_t indirection = get_via_tor ? DIRIND_ANONYMOUS : DIRIND_ONEHOP; directory_initiate_command_routerstatus(rs, @@ -1137,8 +1137,8 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, /* If we've just downloaded a consensus from a directory, re-use that * directory */ if (rs) { - /* Certificate fetches are one-hop, unless AllDirActionsPrivate is 1 */ - int get_via_tor = options->AllDirActionsPrivate; + int get_via_tor = purpose_needs_anonymity( + DIR_PURPOSE_FETCH_CERTIFICATE, 0); const dir_indirection_t indirection = get_via_tor ? DIRIND_ANONYMOUS : DIRIND_ONEHOP; directory_initiate_command_routerstatus(rs, -- cgit v1.2.3-54-g00ecf From f90bfaae8dabb6acf7656b20f1bc0513a683a0dd Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Fri, 1 Jul 2016 10:18:28 +1000 Subject: Refactor duplicate code in authority_certs_fetch_missing --- src/or/routerlist.c | 69 ++++++++++++++++++++++++----------------------------- 1 file changed, 31 insertions(+), 38 deletions(-) (limited to 'src/or/routerlist.c') diff --git a/src/or/routerlist.c b/src/or/routerlist.c index cae0241343..c08d1a33f3 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -834,6 +834,33 @@ authority_cert_dl_looks_uncertain(const char *id_digest) return n_failures >= N_AUTH_CERT_DL_FAILURES_TO_BUG_USER; } +/* Fetch the authority certificates specified in resource. + * If rs is not NULL, fetch from rs, otherwise, fetch from a random directory + * mirror. */ +static void +authority_certs_fetch_resource_impl(const char *resource, + const routerstatus_t *rs) +{ + if (rs) { + /* If we've just downloaded a consensus from a directory, re-use that + * directory */ + int get_via_tor = purpose_needs_anonymity( + DIR_PURPOSE_FETCH_CERTIFICATE, 0); + const dir_indirection_t indirection = get_via_tor ? DIRIND_ANONYMOUS + : DIRIND_ONEHOP; + directory_initiate_command_routerstatus(rs, + DIR_PURPOSE_FETCH_CERTIFICATE, + 0, indirection, resource, NULL, + 0, 0); + } else { + /* Otherwise, we want certs from a random fallback or directory + * mirror, because they will almost always succeed. */ + directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0, + resource, PDS_RETRY_IF_NO_SERVERS, + DL_WANT_ANY_DIRSERVER); + } +} + /** Try to download any v3 authority certificates that we may be missing. If * status is provided, try to get all the ones that were used to sign * status. Additionally, try to have a non-expired certificate for @@ -1072,25 +1099,8 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, if (smartlist_len(fps) > 1) { resource = smartlist_join_strings(fps, "", 0, NULL); - - /* If we've just downloaded a consensus from a directory, re-use that - * directory */ - if (rs) { - int get_via_tor = purpose_needs_anonymity( - DIR_PURPOSE_FETCH_CERTIFICATE, 0); - const dir_indirection_t indirection = get_via_tor ? DIRIND_ANONYMOUS - : DIRIND_ONEHOP; - directory_initiate_command_routerstatus(rs, - DIR_PURPOSE_FETCH_CERTIFICATE, - 0, indirection, resource, NULL, - 0, 0); - } else { - /* Otherwise, we want certs from a random fallback or directory - * mirror, because they will almost always succeed. */ - directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0, - resource, PDS_RETRY_IF_NO_SERVERS, - DL_WANT_ANY_DIRSERVER); - } + /* rs is the directory that just gave us a consensus or certificates */ + authority_certs_fetch_resource_impl(resource, rs); tor_free(resource); } /* else we didn't add any: they were all pending */ @@ -1133,25 +1143,8 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, if (smartlist_len(fp_pairs) > 1) { resource = smartlist_join_strings(fp_pairs, "", 0, NULL); - - /* If we've just downloaded a consensus from a directory, re-use that - * directory */ - if (rs) { - int get_via_tor = purpose_needs_anonymity( - DIR_PURPOSE_FETCH_CERTIFICATE, 0); - const dir_indirection_t indirection = get_via_tor ? DIRIND_ANONYMOUS - : DIRIND_ONEHOP; - directory_initiate_command_routerstatus(rs, - DIR_PURPOSE_FETCH_CERTIFICATE, - 0, indirection, resource, NULL, - 0, 0); - } else { - /* Otherwise, we want certs from a random fallback or directory - * mirror, because they will almost always succeed. */ - directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0, - resource, PDS_RETRY_IF_NO_SERVERS, - DL_WANT_ANY_DIRSERVER); - } + /* rs is the directory that just gave us a consensus or certificates */ + authority_certs_fetch_resource_impl(resource, rs); tor_free(resource); } /* else they were all pending */ -- cgit v1.2.3-54-g00ecf From 516c02b178cf1fd5745280f9979ac67314857ecb Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Fri, 1 Jul 2016 14:01:25 +1000 Subject: Make authority_certs_fetch_missing support bridge hints This also fixes an issue where bridge clients may have found a routerstatus for a directory mirror, and connected to it directly. --- src/or/routerlist.c | 93 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 69 insertions(+), 24 deletions(-) (limited to 'src/or/routerlist.c') diff --git a/src/or/routerlist.c b/src/or/routerlist.c index c08d1a33f3..d2068a2d9f 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -835,30 +835,65 @@ authority_cert_dl_looks_uncertain(const char *id_digest) } /* Fetch the authority certificates specified in resource. - * If rs is not NULL, fetch from rs, otherwise, fetch from a random directory - * mirror. */ + * If we are a bridge client, and node is a configured bridge, fetch from node + * using dir_hint as the fingerprint. Otherwise, if rs is not NULL, fetch from + * rs. Otherwise, fetch from a random directory mirror. */ static void authority_certs_fetch_resource_impl(const char *resource, + const char *dir_hint, + const node_t *node, const routerstatus_t *rs) { + const or_options_t *options = get_options(); + int get_via_tor = purpose_needs_anonymity(DIR_PURPOSE_FETCH_CERTIFICATE, 0); + + /* Make sure bridge clients never connect to anything but a bridge */ + if (options->UseBridges) { + if (node && !node_is_a_configured_bridge(node)) { + /* If we're using bridges, and node is not a bridge, use a 3-hop path. */ + get_via_tor = 1; + } else if (!node) { + /* If we're using bridges, and there's no node, use a 3-hop path. */ + get_via_tor = 1; + } + } + + const dir_indirection_t indirection = get_via_tor ? DIRIND_ANONYMOUS + : DIRIND_ONEHOP; + + /* If we've just downloaded a consensus from a bridge, re-use that + * bridge */ + if (options->UseBridges && node && !get_via_tor) { + /* clients always make OR connections to bridges */ + tor_addr_port_t or_ap; + /* we are willing to use a non-preferred address if we need to */ + fascist_firewall_choose_address_node(node, FIREWALL_OR_CONNECTION, 0, + &or_ap); + directory_initiate_command(&or_ap.addr, or_ap.port, + NULL, 0, /*no dirport*/ + dir_hint, + DIR_PURPOSE_FETCH_CERTIFICATE, + 0, + indirection, + resource, NULL, 0, 0); + return; + } + if (rs) { /* If we've just downloaded a consensus from a directory, re-use that * directory */ - int get_via_tor = purpose_needs_anonymity( - DIR_PURPOSE_FETCH_CERTIFICATE, 0); - const dir_indirection_t indirection = get_via_tor ? DIRIND_ANONYMOUS - : DIRIND_ONEHOP; directory_initiate_command_routerstatus(rs, DIR_PURPOSE_FETCH_CERTIFICATE, 0, indirection, resource, NULL, 0, 0); - } else { - /* Otherwise, we want certs from a random fallback or directory - * mirror, because they will almost always succeed. */ - directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0, - resource, PDS_RETRY_IF_NO_SERVERS, - DL_WANT_ANY_DIRSERVER); + return; } + + /* Otherwise, we want certs from a random fallback or directory + * mirror, because they will almost always succeed. */ + directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0, + resource, PDS_RETRY_IF_NO_SERVERS, + DL_WANT_ANY_DIRSERVER); } /** Try to download any v3 authority certificates that we may be missing. If @@ -1038,7 +1073,10 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, } SMARTLIST_FOREACH_END(voter); } - /* Look up the routerstatus for the dir_hint */ + /* Bridge clients look up the node for the dir_hint */ + const node_t *node = NULL; + /* All clients, including bridge clients, look up the routerstatus for the + * dir_hint */ const routerstatus_t *rs = NULL; /* If we still need certificates, try the directory that just successfully @@ -1046,12 +1084,16 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, * As soon as the directory fails to provide additional certificates, we try * another, randomly selected directory. This avoids continual retries. * (We only ever have one outstanding request per certificate.) - * - * Bridge clients won't find their bridges using this hint, so they will - * fall back to using directory_get_from_dirserver, which selects a bridge. */ if (dir_hint) { - /* First try the consensus routerstatus, then the fallback + if (options->UseBridges) { + /* Bridge clients try the nodelist. If the dir_hint is from an authority, + * or something else fetched over tor, we won't find the node here, but + * we will find the rs. */ + node = node_get_by_id(dir_hint); + } + + /* All clients try the consensus routerstatus, then the fallback * routerstatus */ rs = router_get_consensus_status_by_id(dir_hint); if (!rs) { @@ -1063,9 +1105,10 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, } } - if (!rs) { - log_warn(LD_BUG, "Directory %s delivered a consensus, but a " - "routerstatus could not be found for it.", + if (!node && !rs) { + log_warn(LD_BUG, "Directory %s delivered a consensus, but %s" + "no routerstatus could be found for it.", + options->UseBridges ? "no node and " : "", hex_str(dir_hint, DIGEST_LEN)); } } @@ -1099,8 +1142,9 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, if (smartlist_len(fps) > 1) { resource = smartlist_join_strings(fps, "", 0, NULL); - /* rs is the directory that just gave us a consensus or certificates */ - authority_certs_fetch_resource_impl(resource, rs); + /* node and rs are directories that just gave us a consensus or + * certificates */ + authority_certs_fetch_resource_impl(resource, dir_hint, node, rs); tor_free(resource); } /* else we didn't add any: they were all pending */ @@ -1143,8 +1187,9 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now, if (smartlist_len(fp_pairs) > 1) { resource = smartlist_join_strings(fp_pairs, "", 0, NULL); - /* rs is the directory that just gave us a consensus or certificates */ - authority_certs_fetch_resource_impl(resource, rs); + /* node and rs are directories that just gave us a consensus or + * certificates */ + authority_certs_fetch_resource_impl(resource, dir_hint, node, rs); tor_free(resource); } /* else they were all pending */ -- cgit v1.2.3-54-g00ecf