From 64b948f5fa0099ddabcf0c63e437cc8e7373ef38 Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Wed, 4 May 2016 13:05:18 +1000 Subject: [PATCH 1/2] Use the consensus download schedule for authority certificates Previously, we were using the generic schedule for some downloads, and the consensus schedule for others. Resolves ticket 18816; fix on fddb814fe in 0.2.4.13-alpha. --- changes/bug18816 | 4 ++++ src/or/routerlist.c | 21 +++++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 changes/bug18816 diff --git a/changes/bug18816 b/changes/bug18816 new file mode 100644 index 0000000000..7265f5ab3f --- /dev/null +++ b/changes/bug18816 @@ -0,0 +1,4 @@ + o Minor bugfix (bootstrap): + - Consistently use the consensus download schedule for + authority certificates. + Resolves ticket 18816; fix on fddb814fe in 0.2.4.13-alpha. diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 1c275a68c3..85e9e7d834 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -148,6 +148,22 @@ get_n_authorities(dirinfo_type_t type) return n; } +/** Initialise schedule, want_authority, and increment on in the download + * status dlstatus, then call download_status_reset() on it. + * It is safe to call this function or download_status_reset() multiple times + * on a new dlstatus. But it should *not* be called after a dlstatus has been + * used to count download attempts or failures. */ +static void +download_status_cert_init(download_status_t *dlstatus) +{ + dlstatus->schedule = DL_SCHED_CONSENSUS; + dlstatus->want_authority = DL_WANT_ANY_DIRSERVER; + dlstatus->increment_on = DL_SCHED_INCREMENT_FAILURE; + + /* Use the new schedule to set next_attempt_at */ + download_status_reset(dlstatus); +} + /** Reset the download status of a specified element in a dsmap */ static void download_status_reset_by_sk_in_cl(cert_list_t *cl, const char *digest) @@ -168,6 +184,7 @@ download_status_reset_by_sk_in_cl(cert_list_t *cl, const char *digest) /* Insert before we reset */ dlstatus = tor_malloc_zero(sizeof(*dlstatus)); dsmap_set(cl->dl_status_map, digest, dlstatus); + download_status_cert_init(dlstatus); } tor_assert(dlstatus); /* Go ahead and reset it */ @@ -206,7 +223,7 @@ download_status_is_ready_by_sk_in_cl(cert_list_t *cl, * too. */ dlstatus = tor_malloc_zero(sizeof(*dlstatus)); - download_status_reset(dlstatus); + download_status_cert_init(dlstatus); dsmap_set(cl->dl_status_map, digest, dlstatus); rv = 1; } @@ -225,7 +242,7 @@ get_cert_list(const char *id_digest) cl = digestmap_get(trusted_dir_certs, id_digest); if (!cl) { cl = tor_malloc_zero(sizeof(cert_list_t)); - cl->dl_status_by_id.schedule = DL_SCHED_CONSENSUS; + download_status_cert_init(&cl->dl_status_by_id); cl->certs = smartlist_new(); cl->dl_status_map = dsmap_new(); digestmap_set(trusted_dir_certs, id_digest, cl); From 92d7ee08b8c51b4b29f68c6d00ca4aa91ea5a66b Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Wed, 4 May 2016 15:41:37 +1000 Subject: [PATCH 2/2] Switch between fallback and authority when auth cert fetch fails --- changes/bug18816 | 2 ++ src/or/routerlist.c | 14 ++++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/changes/bug18816 b/changes/bug18816 index 7265f5ab3f..05455120c0 100644 --- a/changes/bug18816 +++ b/changes/bug18816 @@ -1,4 +1,6 @@ o Minor bugfix (bootstrap): - Consistently use the consensus download schedule for authority certificates. + - When downloading authority certificates fails, switch from + using a fallback to using an authority. Resolves ticket 18816; fix on fddb814fe in 0.2.4.13-alpha. diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 85e9e7d834..31693437e1 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -912,11 +912,14 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now) } SMARTLIST_FOREACH_END(d); if (smartlist_len(fps) > 1) { + static int want_auth = 0; resource = smartlist_join_strings(fps, "", 0, NULL); - /* XXX - do we want certs from authorities or mirrors? - teor */ directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0, resource, PDS_RETRY_IF_NO_SERVERS, - DL_WANT_ANY_DIRSERVER); + want_auth ? DL_WANT_AUTHORITY + : DL_WANT_ANY_DIRSERVER); + /* on failure, swap between using fallbacks and authorities */ + want_auth = !want_auth; tor_free(resource); } /* else we didn't add any: they were all pending */ @@ -958,11 +961,14 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now) } SMARTLIST_FOREACH_END(d); if (smartlist_len(fp_pairs) > 1) { + static int want_auth = 0; resource = smartlist_join_strings(fp_pairs, "", 0, NULL); - /* XXX - do we want certs from authorities or mirrors? - teor */ directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0, resource, PDS_RETRY_IF_NO_SERVERS, - DL_WANT_ANY_DIRSERVER); + want_auth ? DL_WANT_AUTHORITY + : DL_WANT_ANY_DIRSERVER); + /* on failure, swap between using fallbacks and authorities */ + want_auth = !want_auth; tor_free(resource); } /* else they were all pending */