From d6744d611f933e77398ed80ac45cabfec16430c1 Mon Sep 17 00:00:00 2001 From: Robert Hogan Date: Tue, 10 Aug 2010 22:07:50 +0100 Subject: Fall back to direct descriptor request to bridges when requests to authorities fail due to a network error. Bug#1138 "When a Tor client starts up using a bridge, and UpdateBridgesFromAuthority is set, Tor will go to the authority first and look up the bridge by fingerprint. If the bridge authority is filtered, Tor will never notice that the bridge authority lookup failed. So it will never fall back." Add connection_dir_bridge_routerdesc_failed(), a function for unpacking the bridge information from a failed request, and ensure connection_dir_request_failed() calls it if the failed request was for a bridge descriptor. Test: 1. for ip in `grep -iR 'router ' cached-descriptors|cut -d ' ' -f 3`; do sudo iptables -A OUTPUT -p tcp -d $ip -j DROP; done 2. remove all files from user tor directory 3. Put the following in torrc: UseBridges 1 UpdateBridgesFromAuthority 1 Bridge 85.108.88.19:443 7E1B28DB47C175392A0E8E4A287C7CB8686575B7 4. Launch tor - it should fall back to downloading descriptors directly from the bridge. Initial patch reviewed and corrected by mingw-san. --- src/or/directory.c | 64 ++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 52 insertions(+), 12 deletions(-) (limited to 'src/or') diff --git a/src/or/directory.c b/src/or/directory.c index a3e575ac9..11dce8ca7 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -67,8 +67,10 @@ static void http_set_address_origin(const char *headers, connection_t *conn); static void connection_dir_download_networkstatus_failed( dir_connection_t *conn, int status_code); static void connection_dir_download_routerdesc_failed(dir_connection_t *conn); +static void connection_dir_bridge_routerdesc_failed(dir_connection_t *conn); static void connection_dir_download_cert_failed( dir_connection_t *conn, int status_code); +static void connection_dir_retry_bridges(smartlist_t* failed, int was_ei); static void dir_networkstatus_download_failed(smartlist_t *failed, int status_code); static void dir_routerdesc_download_failed(smartlist_t *failed, @@ -592,6 +594,8 @@ connection_dir_request_failed(dir_connection_t *conn) conn->_base.purpose == DIR_PURPOSE_FETCH_EXTRAINFO) { log_info(LD_DIR, "Giving up on directory server at '%s'; retrying", conn->_base.address); + if (conn->router_purpose == ROUTER_PURPOSE_BRIDGE) + connection_dir_bridge_routerdesc_failed(conn); connection_dir_download_routerdesc_failed(conn); } else if (conn->_base.purpose == DIR_PURPOSE_FETCH_CONSENSUS) { networkstatus_consensus_download_failed(0); @@ -645,6 +649,25 @@ connection_dir_download_networkstatus_failed(dir_connection_t *conn, } } +/** Helper: Attempt to fetch directly the descriptors of each bridge + * listed in failed. + */ +static void +connection_dir_retry_bridges(smartlist_t* failed, int was_ei) +{ + char digest[DIGEST_LEN]; + tor_assert(!was_ei); /* not supported yet */ + SMARTLIST_FOREACH(failed, const char *, cp, + { + if (base16_decode(digest, DIGEST_LEN, cp, strlen(cp))<0) { + log_warn(LD_BUG, "Malformed fingerprint in list: %s", + escaped(cp)); + continue; + } + retry_bridge_descriptor_fetch_directly(digest); + }); +} + /** Called when an attempt to download one or more router descriptors * or extra-info documents on connection conn failed. */ @@ -662,6 +685,33 @@ connection_dir_download_routerdesc_failed(dir_connection_t *conn) (void) conn; } +/** Called when an attempt to download a bridge's routerdesc from + * one of the authorities failed due to a network error. If + * possible attempt to download descriptors from the bridge directly. + */ +static void +connection_dir_bridge_routerdesc_failed(dir_connection_t *conn) +{ + int was_ei; + smartlist_t *which = NULL; + + /* Requests for bridge descriptors are in the form 'fp/', so ignore + anything else. */ + if (conn->requested_resource && strcmpstart(conn->requested_resource,"fp/")) + return; + + which = smartlist_create(); + dir_split_resource_into_fingerprints(conn->requested_resource + 3, + which, NULL, 0); + + was_ei = conn->_base.purpose == DIR_PURPOSE_FETCH_EXTRAINFO; + if (smartlist_len(which)) { + connection_dir_retry_bridges(which, was_ei); + SMARTLIST_FOREACH(which, char *, cp, tor_free(cp)); + } + smartlist_free(which); +} + /** Called when an attempt to fetch a certificate fails. */ static void connection_dir_download_cert_failed(dir_connection_t *conn, int status) @@ -3498,18 +3548,8 @@ dir_routerdesc_download_failed(smartlist_t *failed, int status_code, time_t now = time(NULL); int server = directory_fetches_from_authorities(get_options()); if (!was_descriptor_digests) { - if (router_purpose == ROUTER_PURPOSE_BRIDGE) { - tor_assert(!was_extrainfo); /* not supported yet */ - SMARTLIST_FOREACH(failed, const char *, cp, - { - if (base16_decode(digest, DIGEST_LEN, cp, strlen(cp))<0) { - log_warn(LD_BUG, "Malformed fingerprint in list: %s", - escaped(cp)); - continue; - } - retry_bridge_descriptor_fetch_directly(digest); - }); - } + if (router_purpose == ROUTER_PURPOSE_BRIDGE) + connection_dir_retry_bridges(failed, was_extrainfo); return; /* FFFF should implement for other-than-router-purpose someday */ } SMARTLIST_FOREACH(failed, const char *, cp, -- cgit v1.2.3 From 2086588efe9c7b378e3813f427d72eb2d135d661 Mon Sep 17 00:00:00 2001 From: Robert Hogan Date: Tue, 31 Aug 2010 20:45:44 +0100 Subject: Amend per Sebastian's comments: - Move checks for extra_info to callers - Change argument name from failed to descs - Use strlen("fp/") instead of a magic number - I passed on the suggestion to rename functions from *_failed() to *_handle_failure(). There are a lot of these so for now just follow the house style. --- src/or/directory.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) (limited to 'src/or') diff --git a/src/or/directory.c b/src/or/directory.c index 11dce8ca7..636413637 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -70,7 +70,7 @@ static void connection_dir_download_routerdesc_failed(dir_connection_t *conn); static void connection_dir_bridge_routerdesc_failed(dir_connection_t *conn); static void connection_dir_download_cert_failed( dir_connection_t *conn, int status_code); -static void connection_dir_retry_bridges(smartlist_t* failed, int was_ei); +static void connection_dir_retry_bridges(smartlist_t* descs); static void dir_networkstatus_download_failed(smartlist_t *failed, int status_code); static void dir_routerdesc_download_failed(smartlist_t *failed, @@ -653,11 +653,10 @@ connection_dir_download_networkstatus_failed(dir_connection_t *conn, * listed in failed. */ static void -connection_dir_retry_bridges(smartlist_t* failed, int was_ei) +connection_dir_retry_bridges(smartlist_t* descs) { char digest[DIGEST_LEN]; - tor_assert(!was_ei); /* not supported yet */ - SMARTLIST_FOREACH(failed, const char *, cp, + SMARTLIST_FOREACH(descs, const char *, cp, { if (base16_decode(digest, DIGEST_LEN, cp, strlen(cp))<0) { log_warn(LD_BUG, "Malformed fingerprint in list: %s", @@ -692,21 +691,22 @@ connection_dir_download_routerdesc_failed(dir_connection_t *conn) static void connection_dir_bridge_routerdesc_failed(dir_connection_t *conn) { - int was_ei; smartlist_t *which = NULL; + tor_assert(conn->requested_resource); /* Requests for bridge descriptors are in the form 'fp/', so ignore anything else. */ if (conn->requested_resource && strcmpstart(conn->requested_resource,"fp/")) return; which = smartlist_create(); - dir_split_resource_into_fingerprints(conn->requested_resource + 3, - which, NULL, 0); + dir_split_resource_into_fingerprints(conn->requested_resource + + strlen("fp/"), + which, NULL, 0); - was_ei = conn->_base.purpose == DIR_PURPOSE_FETCH_EXTRAINFO; + tor_assert(!conn->_base.purpose == DIR_PURPOSE_FETCH_EXTRAINFO); if (smartlist_len(which)) { - connection_dir_retry_bridges(which, was_ei); + connection_dir_retry_bridges(which); SMARTLIST_FOREACH(which, char *, cp, tor_free(cp)); } smartlist_free(which); @@ -3548,8 +3548,10 @@ dir_routerdesc_download_failed(smartlist_t *failed, int status_code, time_t now = time(NULL); int server = directory_fetches_from_authorities(get_options()); if (!was_descriptor_digests) { - if (router_purpose == ROUTER_PURPOSE_BRIDGE) - connection_dir_retry_bridges(failed, was_extrainfo); + if (router_purpose == ROUTER_PURPOSE_BRIDGE) { + tor_assert(!was_extrainfo); + connection_dir_retry_bridges(failed); + } return; /* FFFF should implement for other-than-router-purpose someday */ } SMARTLIST_FOREACH(failed, const char *, cp, -- cgit v1.2.3 From 5799cdd9d326740d79ec8a30b3ace90d0bda346e Mon Sep 17 00:00:00 2001 From: Robert Hogan Date: Thu, 2 Sep 2010 22:17:43 +0100 Subject: Nick points out: tor_assert(!conn->_base.purpose == DIR_PURPOSE_FETCH_EXTRAINFO) != tor_assert(conn->_base.purpose != DIR_PURPOSE_FETCH_EXTRAINFO) !! --- src/or/directory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/or') diff --git a/src/or/directory.c b/src/or/directory.c index 636413637..de1422565 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -704,7 +704,7 @@ connection_dir_bridge_routerdesc_failed(dir_connection_t *conn) + strlen("fp/"), which, NULL, 0); - tor_assert(!conn->_base.purpose == DIR_PURPOSE_FETCH_EXTRAINFO); + tor_assert(conn->_base.purpose != DIR_PURPOSE_FETCH_EXTRAINFO); if (smartlist_len(which)) { connection_dir_retry_bridges(which); SMARTLIST_FOREACH(which, char *, cp, tor_free(cp)); -- cgit v1.2.3 From 22ab997e8319fd0287c16ab8300d1bf69689f458 Mon Sep 17 00:00:00 2001 From: Robert Hogan Date: Sun, 12 Sep 2010 14:10:16 +0100 Subject: Handle null conn->requested_resource rather than assert Per arma's comments in bug1138 --- src/or/directory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/or') diff --git a/src/or/directory.c b/src/or/directory.c index de1422565..388410c8a 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -696,7 +696,7 @@ connection_dir_bridge_routerdesc_failed(dir_connection_t *conn) tor_assert(conn->requested_resource); /* Requests for bridge descriptors are in the form 'fp/', so ignore anything else. */ - if (conn->requested_resource && strcmpstart(conn->requested_resource,"fp/")) + if (!conn->requested_resource || strcmpstart(conn->requested_resource,"fp/")) return; which = smartlist_create(); -- cgit v1.2.3 From 5634e0330283e76aa4e7411ee6d8bef6f71d01a6 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 12 Sep 2010 21:12:17 -0400 Subject: Clean up a couple more bug1138 issues mentioned by roger on code review --- src/or/directory.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'src/or') diff --git a/src/or/directory.c b/src/or/directory.c index 388410c8a..f584bc385 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -70,7 +70,7 @@ static void connection_dir_download_routerdesc_failed(dir_connection_t *conn); static void connection_dir_bridge_routerdesc_failed(dir_connection_t *conn); static void connection_dir_download_cert_failed( dir_connection_t *conn, int status_code); -static void connection_dir_retry_bridges(smartlist_t* descs); +static void connection_dir_retry_bridges(smartlist_t *descs); static void dir_networkstatus_download_failed(smartlist_t *failed, int status_code); static void dir_routerdesc_download_failed(smartlist_t *failed, @@ -653,7 +653,7 @@ connection_dir_download_networkstatus_failed(dir_connection_t *conn, * listed in failed. */ static void -connection_dir_retry_bridges(smartlist_t* descs) +connection_dir_retry_bridges(smartlist_t *descs) { char digest[DIGEST_LEN]; SMARTLIST_FOREACH(descs, const char *, cp, @@ -693,7 +693,6 @@ connection_dir_bridge_routerdesc_failed(dir_connection_t *conn) { smartlist_t *which = NULL; - tor_assert(conn->requested_resource); /* Requests for bridge descriptors are in the form 'fp/', so ignore anything else. */ if (!conn->requested_resource || strcmpstart(conn->requested_resource,"fp/")) -- cgit v1.2.3