tor-commits May 2013

tor-commits@lists.torproject.org

20 participants
1497 discussions

[tor/release-0.2.4] Merge branch 'maint-0.2.4' into release-0.2.4
by arma＠torproject.org 25 May '13

25 May '13

1 0

[tor/release-0.2.4] Downgrade the unexpected sendme cell warnings for 0.2.4
by arma＠torproject.org 25 May '13

25 May '13

commit 30c06c187a0b649bd2d30edc0b16e2376f74ad9a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue May 21 13:45:21 2013 -0400 Downgrade the unexpected sendme cell warnings for 0.2.4 See discussion on #8093 --- changes/bug8093.part1 | 3 +++ src/or/relay.c | 10 ++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/changes/bug8093.part1 b/changes/bug8093.part1 new file mode 100644 index 0000000..2450794 --- /dev/null +++ b/changes/bug8093.part1 @@ -0,0 +1,3 @@ + o Minor features: + - Downgrade "unexpected SENDME" warnings to protocol-warn for 0.2.4, + for bug 8093. diff --git a/src/or/relay.c b/src/or/relay.c index 52ff32f..d57ceaa 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -1494,9 +1494,8 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, if (layer_hint) { if (layer_hint->package_window + CIRCWINDOW_INCREMENT > CIRCWINDOW_START_MAX) { - /*XXXX024: Downgrade this back to LOG_PROTOCOL_WARN after a while*/ - log_fn(LOG_WARN, LD_PROTOCOL, - "Bug/attack: unexpected sendme cell from exit relay. " + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Unexpected sendme cell from exit relay. " "Closing circ."); return -END_CIRC_REASON_TORPROTOCOL; } @@ -1507,9 +1506,8 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, } else { if (circ->package_window + CIRCWINDOW_INCREMENT > CIRCWINDOW_START_MAX) { - /*XXXX024: Downgrade this back to LOG_PROTOCOL_WARN after a while*/ - log_fn(LOG_WARN, LD_PROTOCOL, - "Bug/attack: unexpected sendme cell from client. " + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Unexpected sendme cell from client. " "Closing circ (window %d).", circ->package_window); return -END_CIRC_REASON_TORPROTOCOL;

1 0

[tor/release-0.2.4] Fix bug 8846: better log message on IP version confusion
by arma＠torproject.org 25 May '13

25 May '13

commit bc56918e5abb48f67f31012838c689e9572eec69 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri May 17 14:50:45 2013 -0400 Fix bug 8846: better log message on IP version confusion --- changes/bug8846 | 4 ++++ src/or/connection_edge.c | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/changes/bug8846 b/changes/bug8846 new file mode 100644 index 0000000..377cc37 --- /dev/null +++ b/changes/bug8846 @@ -0,0 +1,4 @@ + o Minor bugfixes: + - Give a less useless error message when the user asks for an IPv4 + address on an IPv6-only port, or vice versa. Fixes bug 8846; bugfix + on 0.2.4.7-alpha. diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 926fcab..4d317d0 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1769,9 +1769,10 @@ connection_ap_get_begincell_flags(entry_connection_t *ap_conn) } if (flags == BEGIN_FLAG_IPV4_NOT_OK) { - log_warn(LD_BUG, "Hey; I'm about to ask a node for a connection that I " + log_warn(LD_EDGE, "I'm about to ask a node for a connection that I " "am telling it to fulfil with neither IPv4 nor IPv6. That's " - "probably not going to work."); + "not going to work. Did you perhaps ask for an IPv6 address " + "on an IPv4Only port, or vice versa?"); } return flags;

1 0

[tor/release-0.2.4] Merge remote-tracking branch 'origin/maint-0.2.3' into maint-0.2.4
by arma＠torproject.org 25 May '13

25 May '13

commit bae5dd6c8d4535360d471932b87431f54b515567 Merge: 7d3fd85 96d3219 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 9 13:14:53 2013 -0400 Merge remote-tracking branch 'origin/maint-0.2.3' into maint-0.2.4 changes/bug8844 | 6 ++++++ src/or/buffers.c | 3 ++- src/test/test.c | 12 ++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-)

1 0

[tor/release-0.2.4] Implement fp_pair_map_t
by arma＠torproject.org 25 May '13

25 May '13

commit d5bd4a4763dfa74572ce6ed0b565315c43ff9f87 Author: Andrea Shepard <andrea(a)torproject.org> Date: Fri Mar 22 12:13:25 2013 -0700 Implement fp_pair_map_t --- src/or/Makefile.am | 2 + src/or/Makefile.nmake | 2 +- src/or/fp_pair.c | 308 +++++++++++++++++++++++++++++++++++++++++++++++++ src/or/fp_pair.h | 45 ++++++++ 4 files changed, 356 insertions(+), 1 deletion(-) diff --git a/src/or/Makefile.am b/src/or/Makefile.am index 3cc789a..813227f 100644 --- a/src/or/Makefile.am +++ b/src/or/Makefile.am @@ -32,6 +32,7 @@ libtor_a_SOURCES = \ dirvote.c \ dns.c \ dnsserv.c \ + fp_pair.c \ geoip.c \ hibernate.c \ main.c \ @@ -96,6 +97,7 @@ noinst_HEADERS = \ dnsserv.h \ eventdns.h \ eventdns_tor.h \ + fp_pair.h \ geoip.h \ hibernate.h \ main.h \ diff --git a/src/or/Makefile.nmake b/src/or/Makefile.nmake index 3181e79..146866e 100644 --- a/src/or/Makefile.nmake +++ b/src/or/Makefile.nmake @@ -11,7 +11,7 @@ LIBS = ..\..\..\build-alpha\lib\libevent.a \ LIBTOR_OBJECTS = buffers.obj circuitbuild.obj circuitlist.obj circuituse.obj \ command.obj config.obj connection.obj connection_edge.obj \ connection_or.obj control.obj cpuworker.obj directory.obj \ - dirserv.obj dirvote.obj dns.obj dnsserv.obj geoip.obj \ + dirserv.obj dirvote.obj dns.obj dnsserv.obj fp_pair.obj geoip.obj \ hibernate.obj main.obj microdesc.obj networkstatus.obj \ nodelist.obj onion.obj policies.obj reasons.obj relay.obj \ rendclient.obj rendcommon.obj rendmid.obj rendservice.obj \ diff --git a/src/or/fp_pair.c b/src/or/fp_pair.c new file mode 100644 index 0000000..4d8a835 --- /dev/null +++ b/src/or/fp_pair.c @@ -0,0 +1,308 @@ +/* Copyright (c) 2013, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#include "or.h" +#include "fp_pair.h" + +/* Define fp_pair_map_t structures */ + +struct fp_pair_map_entry_s { + HT_ENTRY(fp_pair_map_entry_s) node; + void *val; + fp_pair_t key; +}; + +struct fp_pair_map_s { + HT_HEAD(fp_pair_map_impl, fp_pair_map_entry_s) head; +}; + +/* + * Hash function and equality checker for fp_pair_map_t + */ + +/** Compare fp_pair_entry_t objects by key value. */ +static INLINE int +fp_pair_map_entries_eq(const fp_pair_map_entry_t *a, + const fp_pair_map_entry_t *b) +{ + return tor_memeq(&(a->key), &(b->key), sizeof(fp_pair_t)); +} + +/** Return a hash value for an fp_pair_entry_t. */ +static INLINE unsigned int +fp_pair_map_entry_hash(const fp_pair_map_entry_t *a) +{ + const uint32_t *p; + unsigned int hash; + + p = (const uint32_t *)(a->key.first); + /* Hashes are 20 bytes long, so 5 times uint32_t */ + hash = p[0] ^ p[1] ^ p[2] ^ p[3] ^ p[4]; + /* Now XOR in the second fingerprint */ + p = (const uint32_t *)(a->key.second); + hash ^= p[0] ^ p[1] ^ p[2] ^ p[3] ^ p[4]; + + return hash; +} + +/* + * Hash table functions for fp_pair_map_t + */ + +HT_PROTOTYPE(fp_pair_map_impl, fp_pair_map_entry_s, node, + fp_pair_map_entry_hash, fp_pair_map_entries_eq) +HT_GENERATE(fp_pair_map_impl, fp_pair_map_entry_s, node, + fp_pair_map_entry_hash, fp_pair_map_entries_eq, + 0.6, tor_malloc, tor_realloc, tor_free) + +/** Constructor to create a new empty map from fp_pair_t to void * + */ + +fp_pair_map_t * +fp_pair_map_new(void) +{ + fp_pair_map_t *result; + + result = tor_malloc(sizeof(fp_pair_map_t)); + HT_INIT(fp_pair_map_impl, &result->head); + return result; +} + +/** Set the current value for key to val; returns the previous + * value for key if one was set, or NULL if one was not. + */ + +void * +fp_pair_map_set(fp_pair_map_t *map, const fp_pair_t *key, void *val) +{ + fp_pair_map_entry_t *resolve; + fp_pair_map_entry_t search; + void *oldval; + + tor_assert(map); + tor_assert(key); + tor_assert(val); + + memcpy(&(search.key), key, sizeof(*key)); + resolve = HT_FIND(fp_pair_map_impl, &(map->head), &search); + if (resolve) { + oldval = resolve->val; + resolve->val = val; + } else { + resolve = tor_malloc_zero(sizeof(fp_pair_map_entry_t)); + memcpy(&(resolve->key), key, sizeof(*key)); + resolve->val = val; + HT_INSERT(fp_pair_map_impl, &(map->head), resolve); + oldval = NULL; + } + + return oldval; +} + +/** Set the current value for the key (first, second) to val; returns + * the previous value for key if one was set, or NULL if one was not. + */ + +void * +fp_pair_map_set_by_digests(fp_pair_map_t *map, + const char *first, const char *second, + void *val) +{ + fp_pair_t k; + + tor_assert(first); + tor_assert(second); + + memcpy(k.first, first, DIGEST_LEN); + memcpy(k.second, second, DIGEST_LEN); + + return fp_pair_map_set(map, &k, val); +} + +/** Return the current value associated with key, or NULL if no value is set. + */ + +void * +fp_pair_map_get(const fp_pair_map_t *map, const fp_pair_t *key) +{ + fp_pair_map_entry_t *resolve; + fp_pair_map_entry_t search; + void *val = NULL; + + tor_assert(map); + tor_assert(key); + + memcpy(&(search.key), key, sizeof(*key)); + resolve = HT_FIND(fp_pair_map_impl, &(map->head), &search); + if (resolve) val = resolve->val; + + return val; +} + +/** Return the current value associated the key (first, second), or + * NULL if no value is set. + */ + +void * +fp_pair_map_get_by_digests(const fp_pair_map_t *map, + const char *first, const char *second) +{ + fp_pair_t k; + + tor_assert(first); + tor_assert(second); + + memcpy(k.first, first, DIGEST_LEN); + memcpy(k.second, second, DIGEST_LEN); + + return fp_pair_map_get(map, &k); +} + +/** Remove the value currently associated with key from the map. + * Return the value if one was set, or NULL if there was no entry for + * key. The caller must free any storage associated with the + * returned value. + */ + +void * +fp_pair_map_remove(fp_pair_map_t *map, const fp_pair_t *key) +{ + fp_pair_map_entry_t *resolve; + fp_pair_map_entry_t search; + void *val = NULL; + + tor_assert(map); + tor_assert(key); + + memcpy(&(search.key), key, sizeof(*key)); + resolve = HT_REMOVE(fp_pair_map_impl, &(map->head), &search); + if (resolve) { + val = resolve->val; + tor_free(resolve); + } + + return val; +} + +/** Remove all entries from map, and deallocate storage for those entries. + * If free_val is provided, it is invoked on every value in map. + */ + +void +fp_pair_map_free(fp_pair_map_t *map, void (*free_val)(void*)) +{ + fp_pair_map_entry_t **ent, **next, *this; + + if (map) { + for (ent = HT_START(fp_pair_map_impl, &(map->head)); + ent != NULL; ent = next) { + this = *ent; + next = HT_NEXT_RMV(fp_pair_map_impl, &(map->head), ent); + if (free_val) free_val(this->val); + tor_free(this); + } + tor_assert(HT_EMPTY(&(map->head))); + HT_CLEAR(fp_pair_map_impl, &(map->head)); + tor_free(map); + } +} + +/** Return true iff map has no entries. + */ + +int +fp_pair_map_isempty(const fp_pair_map_t *map) +{ + tor_assert(map); + + return HT_EMPTY(&(map->head)); +} + +/** Return the number of items in map. + */ + +int +fp_pair_map_size(const fp_pair_map_t *map) +{ + tor_assert(map); + + return HT_SIZE(&(map->head)); +} + +/** return an iterator pointing to the start of map. + */ + +fp_pair_map_iter_t * +fp_pair_map_iter_init(fp_pair_map_t *map) +{ + tor_assert(map); + + return HT_START(fp_pair_map_impl, &(map->head)); +} + +/** Advance iter a single step to the next entry of map, and return + * its new value. + */ + +fp_pair_map_iter_t * +fp_pair_map_iter_next(fp_pair_map_t *map, fp_pair_map_iter_t *iter) +{ + tor_assert(map); + tor_assert(iter); + + return HT_NEXT(fp_pair_map_impl, &(map->head), iter); +} + +/** Advance iter a single step to the next entry of map, removing the current + * entry, and return its new value. + */ + +fp_pair_map_iter_t * +fp_pair_map_iter_next_rmv(fp_pair_map_t *map, fp_pair_map_iter_t *iter) +{ + fp_pair_map_entry_t *rmv; + + tor_assert(map); + tor_assert(iter); + tor_assert(*iter); + + rmv = *iter; + iter = HT_NEXT_RMV(fp_pair_map_impl, &(map->head), iter); + tor_free(rmv); + + return iter; +} + +/** Set *key_out and *val_out to the current entry pointed to by iter. + */ + +void +fp_pair_map_iter_get(fp_pair_map_iter_t *iter, + fp_pair_t *key_out, void **val_out) +{ + tor_assert(iter); + tor_assert(*iter); + + if (key_out) memcpy(key_out, &((*iter)->key), sizeof(fp_pair_t)); + if (val_out) *val_out = (*iter)->val; +} + +/** Return true iff iter has advanced past the last entry of its map. + */ + +int +fp_pair_map_iter_done(fp_pair_map_iter_t *iter) +{ + return (iter == NULL); +} + +/** Assert if anything has gone wrong with the internal + * representation of map. + */ + +void +fp_pair_map_assert_ok(const fp_pair_map_t *map) +{ + tor_assert(!fp_pair_map_impl_HT_REP_IS_BAD_(&(map->head))); +} + diff --git a/src/or/fp_pair.h b/src/or/fp_pair.h new file mode 100644 index 0000000..89f664a --- /dev/null +++ b/src/or/fp_pair.h @@ -0,0 +1,45 @@ +/* Copyright (c) 2013, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file fp_pair.h + * \brief Header file for fp_pair.c. + **/ + +#ifndef _TOR_FP_PAIR_H +#define _TOR_FP_PAIR_H + +/* + * Declare fp_pair_map_t functions and structs + */ + +typedef struct fp_pair_map_entry_s fp_pair_map_entry_t; +typedef struct fp_pair_map_s fp_pair_map_t; +typedef fp_pair_map_entry_t *fp_pair_map_iter_t; + +fp_pair_map_t * fp_pair_map_new(void); +void * fp_pair_map_set(fp_pair_map_t *map, const fp_pair_t *key, void *val); +void * fp_pair_map_set_by_digests(fp_pair_map_t *map, + const char *first, const char *second, + void *val); +void * fp_pair_map_get(const fp_pair_map_t *map, const fp_pair_t *key); +void * fp_pair_map_get_by_digests(const fp_pair_map_t *map, + const char *first, const char *second); +void * fp_pair_map_remove(fp_pair_map_t *map, const fp_pair_t *key); +void fp_pair_map_free(fp_pair_map_t *map, void (*free_val)(void*)); +int fp_pair_map_isempty(const fp_pair_map_t *map); +int fp_pair_map_size(const fp_pair_map_t *map); +fp_pair_map_iter_t * fp_pair_map_iter_init(fp_pair_map_t *map); +fp_pair_map_iter_t * fp_pair_map_iter_next(fp_pair_map_t *map, + fp_pair_map_iter_t *iter); +fp_pair_map_iter_t * fp_pair_map_iter_next_rmv(fp_pair_map_t *map, + fp_pair_map_iter_t *iter); +void fp_pair_map_iter_get(fp_pair_map_iter_t *iter, + fp_pair_t *key_out, void **val_out); +int fp_pair_map_iter_done(fp_pair_map_iter_t *iter); +void fp_pair_map_assert_ok(const fp_pair_map_t *map); + +#undef DECLARE_MAP_FNS + +#endif +

1 0

[tor/release-0.2.4] Use tor_asprintf() and clean up string handling in authority_certs_fetch_missing()
by arma＠torproject.org 25 May '13

25 May '13

commit 2824bf3445448fee55a0f302d7ec85a5915e8f18 Author: Andrea Shepard <andrea(a)torproject.org> Date: Thu May 9 09:31:39 2013 -0700 Use tor_asprintf() and clean up string handling in authority_certs_fetch_missing() --- src/or/routerlist.c | 58 ++++++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 15a71e4..8677400 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -791,23 +791,33 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now) /* Do downloads by identity digest */ if (smartlist_len(missing_id_digests) > 0) { + int need_plus = 0; smartlist_t *fps = smartlist_new(); + smartlist_add(fps, tor_strdup("fp/")); - SMARTLIST_FOREACH(missing_id_digests, const char *, d, { - char *fp; + SMARTLIST_FOREACH_BEGIN(missing_id_digests, const char *, d) { + char *fp = NULL; + if (digestmap_get(pending_id, d)) continue; - fp = tor_malloc(HEX_DIGEST_LEN+2); - base16_encode(fp, HEX_DIGEST_LEN+1, d, DIGEST_LEN); - fp[HEX_DIGEST_LEN] = '+'; - fp[HEX_DIGEST_LEN+1] = '\0'; + + base16_encode(id_digest_str, sizeof(id_digest_str), + d, DIGEST_LEN); + + if (need_plus) { + tor_asprintf(&fp, "+%s", id_digest_str); + } else { + /* No need for tor_asprintf() in this case; first one gets no '+' */ + fp = tor_strdup(id_digest_str); + need_plus = 1; + } + smartlist_add(fps, fp); - }); + } SMARTLIST_FOREACH_END(d); if (smartlist_len(fps) > 1) { resource = smartlist_join_strings(fps, "", 0, NULL); - resource[strlen(resource)-1] = '\0'; directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0, resource, PDS_RETRY_IF_NO_SERVERS); tor_free(resource); @@ -820,46 +830,38 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now) /* Do downloads by identity digest/signing key pair */ if (smartlist_len(missing_cert_digests) > 0) { + int need_plus = 0; smartlist_t *fp_pairs = smartlist_new(); - int need_plus = 0, offset = 0; smartlist_add(fp_pairs, tor_strdup("fp-sk/")); SMARTLIST_FOREACH_BEGIN(missing_cert_digests, const fp_pair_t *, d) { - char *fp_pair; + char *fp_pair = NULL; if (fp_pair_map_get(pending_cert, d)) continue; - fp_pair = tor_malloc(2*HEX_DIGEST_LEN+3); - offset = 0; + /* Construct string encodings of the digests */ + base16_encode(id_digest_str, sizeof(id_digest_str), + d->first, DIGEST_LEN); + base16_encode(sk_digest_str, sizeof(sk_digest_str), + d->second, DIGEST_LEN); + + /* Now tor_asprintf() */ if (need_plus) { - fp_pair[offset++] = '+'; + tor_asprintf(&fp_pair, "+%s-%s", id_digest_str, sk_digest_str); } else { - /* Prepend a '+' to all but the first in the list */ + /* First one in the list doesn't get a '+' */ + tor_asprintf(&fp_pair, "%s-%s", id_digest_str, sk_digest_str); need_plus = 1; } - /* Encode the first fingerprint */ - base16_encode(fp_pair + offset, HEX_DIGEST_LEN+1, - d->first, DIGEST_LEN); - offset += HEX_DIGEST_LEN; - /* Add a '-' to separate them */ - fp_pair[offset++] = '-'; - /* Encode the second fingerprint */ - base16_encode(fp_pair + offset, HEX_DIGEST_LEN+1, - d->second, DIGEST_LEN); - offset += HEX_DIGEST_LEN; - /* Add a NUL */ - fp_pair[offset++] = '\0'; - /* Add it to the list of pairs to request */ smartlist_add(fp_pairs, fp_pair); } SMARTLIST_FOREACH_END(d); if (smartlist_len(fp_pairs) > 1) { resource = smartlist_join_strings(fp_pairs, "", 0, NULL); - resource[strlen(resource)-1] = '\0'; directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0, resource, PDS_RETRY_IF_NO_SERVERS); tor_free(resource);

1 0

[tor/release-0.2.4] Make warning in authority_cert_dl_failed() LD_BUG per NickM code review
by arma＠torproject.org 25 May '13

25 May '13

commit 17692b2fe2f9fd7c33461c981b8d2eb511976758 Author: Andrea Shepard <andrea(a)torproject.org> Date: Thu May 9 09:33:32 2013 -0700 Make warning in authority_cert_dl_failed() LD_BUG per NickM code review --- src/or/routerlist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 8677400..14c44ec 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -593,7 +593,7 @@ authority_cert_dl_failed(const char *id_digest, id_digest, DIGEST_LEN); base16_encode(sk_digest_str, sizeof(sk_digest_str), signing_key_digest, DIGEST_LEN); - log_warn(LD_DIR, + log_warn(LD_BUG, "Got failure for cert fetch with (fp,sk) = (%s,%s), with " "status %d, but knew nothing about the download.", id_digest_str, sk_digest_str, status);

1 0

[tor/release-0.2.4] When downloading certificates, distinguish requesting by identity digest from requesting by ID digest, signing key pair; fixes bug 5595
by arma＠torproject.org 25 May '13

25 May '13

commit fddb814feaa3d0091df03b26fa709cfba55312ed Author: Andrea Shepard <andrea(a)torproject.org> Date: Thu May 9 04:56:54 2013 -0700 When downloading certificates, distinguish requesting by identity digest from requesting by ID digest, signing key pair; fixes bug 5595 --- changes/bug5595 | 8 + src/or/directory.c | 69 +++++++-- src/or/dirvote.c | 2 +- src/or/router.c | 3 +- src/or/routerlist.c | 427 ++++++++++++++++++++++++++++++++++++++++++--------- src/or/routerlist.h | 18 ++- 6 files changed, 436 insertions(+), 91 deletions(-) diff --git a/changes/bug5595 b/changes/bug5595 new file mode 100644 index 0000000..31f4b84 --- /dev/null +++ b/changes/bug5595 @@ -0,0 +1,8 @@ + o Critical bugfixes: + - Distinguish downloading an authority certificate by identity digest from + downloading one by identity digest/signing key digest pair; formerly we + always request them only by identity digest and get the newest one even + when we wanted one with a different signing key. Then we would complain + about being given a certificate we already had, and never get the one we + really wanted. Now we use the "fp-sk/" resource as well as the "fp/" + resource to request the one we want. Fixes bug 5595. diff --git a/src/or/directory.c b/src/or/directory.c index f235bf3..8dd5a7c 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -809,13 +809,34 @@ connection_dir_download_cert_failed(dir_connection_t *conn, int status) if (!conn->requested_resource) return; failed = smartlist_new(); - dir_split_resource_into_fingerprints(conn->requested_resource+3, - failed, NULL, DSR_HEX); - SMARTLIST_FOREACH(failed, char *, cp, - { - authority_cert_dl_failed(cp, status); - tor_free(cp); - }); + /* + * We have two cases download by fingerprint (resource starts + * with "fp/") or download by fingerprint/signing key pair + * (resource starts with "fp-sk/"). + */ + if (!strcmpstart(conn->requested_resource, "fp/")) { + /* Download by fingerprint case */ + dir_split_resource_into_fingerprints(conn->requested_resource + 3, + failed, NULL, DSR_HEX); + SMARTLIST_FOREACH_BEGIN(failed, char *, cp) { + /* Null signing key digest indicates download by fp only */ + authority_cert_dl_failed(cp, NULL, status); + tor_free(cp); + } SMARTLIST_FOREACH_END(cp); + } else if (!strcmpstart(conn->requested_resource, "fp-sk/")) { + /* Download by (fp,sk) pairs */ + dir_split_resource_into_fingerprint_pairs(conn->requested_resource + 5, + failed); + SMARTLIST_FOREACH_BEGIN(failed, fp_pair_t *, cp) { + authority_cert_dl_failed(cp->first, cp->second, status); + tor_free(cp); + } SMARTLIST_FOREACH_END(cp); + } else { + log_warn(LD_DIR, + "Don't know what to do with failure for cert fetch %s", + conn->requested_resource); + } + smartlist_free(failed); update_certificate_downloads(time(NULL)); @@ -1589,6 +1610,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn) conn->_base.purpose == DIR_PURPOSE_FETCH_MICRODESC); int was_compressed=0; time_t now = time(NULL); + int src_code; switch (connection_fetch_from_buf_http(TO_CONN(conn), &headers, MAX_HEADERS_SIZE, @@ -1857,14 +1879,33 @@ connection_dir_client_reached_eof(dir_connection_t *conn) } log_info(LD_DIR,"Received authority certificates (size %d) from server " "'%s:%d'", (int)body_len, conn->_base.address, conn->_base.port); - if (trusted_dirs_load_certs_from_string(body, 0, 1)<0) { - log_warn(LD_DIR, "Unable to parse fetched certificates"); - /* if we fetched more than one and only some failed, the successful - * ones got flushed to disk so it's safe to call this on them */ - connection_dir_download_cert_failed(conn, status_code); + /* + * Tell trusted_dirs_load_certs_from_string() whether it was by fp + * or fp-sk pair. + */ + src_code = -1; + if (!strcmpstart(conn->requested_resource, "fp/")) { + src_code = TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST; + } else if (!strcmpstart(conn->requested_resource, "fp-sk/")) { + src_code = TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_SK_DIGEST; + } + + if (src_code != -1) { + if (trusted_dirs_load_certs_from_string(body, src_code, 1)<0) { + log_warn(LD_DIR, "Unable to parse fetched certificates"); + /* if we fetched more than one and only some failed, the successful + * ones got flushed to disk so it's safe to call this on them */ + connection_dir_download_cert_failed(conn, status_code); + } else { + directory_info_has_arrived(now, 0); + log_info(LD_DIR, "Successfully loaded certificates from fetch."); + } } else { - directory_info_has_arrived(now, 0); - log_info(LD_DIR, "Successfully loaded certificates from fetch."); + log_warn(LD_DIR, + "Couldn't figure out what to do with fetched certificates for " + "unknown resource %s", + conn->requested_resource); + connection_dir_download_cert_failed(conn, status_code); } } if (conn->_base.purpose == DIR_PURPOSE_FETCH_STATUS_VOTE) { diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 144859a..959e05a 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -2947,7 +2947,7 @@ dirvote_add_vote(const char *vote_body, const char **msg_out, int *status_out) /* Hey, it's a new cert! */ trusted_dirs_load_certs_from_string( vote->cert->cache_info.signed_descriptor_body, - 0 /* from_store */, 1 /*flush*/); + TRUSTED_DIRS_CERTS_SRC_FROM_VOTE, 1 /*flush*/); if (!authority_cert_get_by_digests(vote->cert->cache_info.identity_digest, vote->cert->signing_key_digest)) { log_warn(LD_BUG, "We added a cert, but still couldn't find it."); diff --git a/src/or/router.c b/src/or/router.c index 1ace8e2..c68a309 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -758,7 +758,8 @@ init_keys(void) if (cert) { /* add my own cert to the list of known certs */ log_info(LD_DIR, "adding my own v3 cert"); if (trusted_dirs_load_certs_from_string( - cert->cache_info.signed_descriptor_body, 0, 0)<0) { + cert->cache_info.signed_descriptor_body, + TRUSTED_DIRS_CERTS_SRC_SELF, 0)<0) { log_warn(LD_DIR, "Unable to parse my own v3 cert! Failing."); return -1; } diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 3c39e36..20d5110 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -19,6 +19,7 @@ #include "directory.h" #include "dirserv.h" #include "dirvote.h" +#include "fp_pair.h" #include "geoip.h" #include "hibernate.h" #include "main.h" @@ -38,6 +39,24 @@ /****************************************************************************/ +DECLARE_TYPED_DIGESTMAP_FNS(sdmap_, digest_sd_map_t, signed_descriptor_t) +DECLARE_TYPED_DIGESTMAP_FNS(rimap_, digest_ri_map_t, routerinfo_t) +DECLARE_TYPED_DIGESTMAP_FNS(eimap_, digest_ei_map_t, extrainfo_t) +DECLARE_TYPED_DIGESTMAP_FNS(dsmap_, digest_ds_map_t, download_status_t) +#define SDMAP_FOREACH(map, keyvar, valvar) \ + DIGESTMAP_FOREACH(sdmap_to_digestmap(map), keyvar, signed_descriptor_t *, \ + valvar) +#define RIMAP_FOREACH(map, keyvar, valvar) \ + DIGESTMAP_FOREACH(rimap_to_digestmap(map), keyvar, routerinfo_t *, valvar) +#define EIMAP_FOREACH(map, keyvar, valvar) \ + DIGESTMAP_FOREACH(eimap_to_digestmap(map), keyvar, extrainfo_t *, valvar) +#define DSMAP_FOREACH(map, keyvar, valvar) \ + DIGESTMAP_FOREACH(dsmap_to_digestmap(map), keyvar, download_status_t *, \ + valvar) + +/* Forward declaration for cert_list_t */ +typedef struct cert_list_t cert_list_t; + /* static function prototypes */ static const routerstatus_t *router_pick_directory_server_impl( dirinfo_type_t auth, int flags); @@ -56,19 +75,14 @@ static const char *signed_descriptor_get_body_impl( int with_annotations); static void list_pending_downloads(digestmap_t *result, 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, const or_options_t *options); - -DECLARE_TYPED_DIGESTMAP_FNS(sdmap_, digest_sd_map_t, signed_descriptor_t) -DECLARE_TYPED_DIGESTMAP_FNS(rimap_, digest_ri_map_t, routerinfo_t) -DECLARE_TYPED_DIGESTMAP_FNS(eimap_, digest_ei_map_t, extrainfo_t) -#define SDMAP_FOREACH(map, keyvar, valvar) \ - DIGESTMAP_FOREACH(sdmap_to_digestmap(map), keyvar, signed_descriptor_t *, \ - valvar) -#define RIMAP_FOREACH(map, keyvar, valvar) \ - DIGESTMAP_FOREACH(rimap_to_digestmap(map), keyvar, routerinfo_t *, valvar) -#define EIMAP_FOREACH(map, keyvar, valvar) \ - DIGESTMAP_FOREACH(eimap_to_digestmap(map), keyvar, extrainfo_t *, valvar) +static void download_status_reset_by_sk_in_cl(cert_list_t *cl, + const char *digest); +static int download_status_is_ready_by_sk_in_cl(cert_list_t *cl, + const char *digest, + time_t now, int max_failures); /****************************************************************************/ @@ -78,10 +92,17 @@ static smartlist_t *trusted_dir_servers = NULL; /** List of for a given authority, and download status for latest certificate. */ -typedef struct cert_list_t { - download_status_t dl_status; +struct cert_list_t { + /* + * The keys of download status map are cert->signing_key_digest for pending + * downloads by (identity digest/signing key digest) pair; functions such + * as authority_cert_get_by_digest() already assume these are unique. + */ + struct digest_ds_map_t *dl_status_map; + /* There is also a dlstatus for the download by identity key only */ + download_status_t dl_status_by_id; smartlist_t *certs; -} cert_list_t; +}; /** Map from v3 identity key digest to cert_list_t. */ static digestmap_t *trusted_dir_certs = NULL; /** True iff any key certificate in at least one member of @@ -125,6 +146,72 @@ get_n_authorities(dirinfo_type_t type) return n; } +/** 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) +{ + download_status_t *dlstatus = NULL; + + tor_assert(cl); + tor_assert(digest); + + /* Make sure we have a dsmap */ + if (!(cl->dl_status_map)) { + cl->dl_status_map = dsmap_new(); + } + /* Look for a download_status_t in the map with this digest */ + dlstatus = dsmap_get(cl->dl_status_map, digest); + /* Got one? */ + if (!dlstatus) { + /* Insert before we reset */ + dlstatus = tor_malloc_zero(sizeof(*dlstatus)); + dsmap_set(cl->dl_status_map, digest, dlstatus); + } + tor_assert(dlstatus); + /* Go ahead and reset it */ + download_status_reset(dlstatus); +} + +/** + * Return true if the download for this signing key digest in cl is ready + * to be re-attempted. + */ +static int +download_status_is_ready_by_sk_in_cl(cert_list_t *cl, + const char *digest, + time_t now, int max_failures) +{ + int rv = 0; + download_status_t *dlstatus = NULL; + + tor_assert(cl); + tor_assert(digest); + + /* Make sure we have a dsmap */ + if (!(cl->dl_status_map)) { + cl->dl_status_map = dsmap_new(); + } + /* Look for a download_status_t in the map with this digest */ + dlstatus = dsmap_get(cl->dl_status_map, digest); + /* Got one? */ + if (dlstatus) { + /* Use download_status_is_ready() */ + rv = download_status_is_ready(dlstatus, now, max_failures); + } else { + /* + * If we don't know anything about it, return 1, since we haven't + * tried this one before. We need to create a new entry here, + * too. + */ + dlstatus = tor_malloc_zero(sizeof(*dlstatus)); + download_status_reset(dlstatus); + dsmap_set(cl->dl_status_map, digest, dlstatus); + rv = 1; + } + + return rv; +} + #define get_n_v2_authorities() get_n_authorities(V2_DIRINFO) /** Helper: Return the cert_list_t for an authority whose authority ID is @@ -138,8 +225,9 @@ 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.schedule = DL_SCHED_CONSENSUS; + cl->dl_status_by_id.schedule = DL_SCHED_CONSENSUS; cl->certs = smartlist_new(); + cl->dl_status_map = dsmap_new(); digestmap_set(trusted_dir_certs, id_digest, cl); } return cl; @@ -159,7 +247,9 @@ trusted_dirs_reload_certs(void) tor_free(filename); if (!contents) return 0; - r = trusted_dirs_load_certs_from_string(contents, 1, 1); + r = trusted_dirs_load_certs_from_string( + contents, + TRUSTED_DIRS_CERTS_SRC_FROM_STORE, 1); tor_free(contents); return r; } @@ -182,17 +272,23 @@ already_have_cert(authority_cert_t *cert) } /** Load a bunch of new key certificates from the string contents. If - * from_store is true, the certificates are from the cache, and we - * don't need to flush them to disk. If flush is true, we need - * to flush any changed certificates to disk now. Return 0 on success, -1 - * if any certs fail to parse. */ + * source is TRUSTED_DIRS_CERTS_SRC_FROM_STORE, the certificates are + * from the cache, and we don't need to flush them to disk. If we are a + * dirauth loading our own cert, source is TRUSTED_DIRS_CERTS_SRC_SELF. + * Otherwise, source is download type: TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST + * or TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_SK_DIGEST. If flush is true, we + * need to flush any changed certificates to disk now. Return 0 on success, + * -1 if any certs fail to parse. + */ + int -trusted_dirs_load_certs_from_string(const char *contents, int from_store, +trusted_dirs_load_certs_from_string(const char *contents, int source, int flush) { trusted_dir_server_t *ds; const char *s, *eos; int failure_code = 0; + int from_store = (source == TRUSTED_DIRS_CERTS_SRC_FROM_STORE); for (s = contents; *s; s = eos) { authority_cert_t *cert = authority_cert_parse_from_string(s, &eos); @@ -229,7 +325,18 @@ trusted_dirs_load_certs_from_string(const char *contents, int from_store, ds ? ds->nickname : "an old or new authority"); } - authority_cert_dl_failed(cert->cache_info.identity_digest, 404); + /* + * This is where we care about the source; authority_cert_dl_failed() + * needs to know whether the download was by fp or (fp,sk) pair to + * twiddle the right bit in the download map. + */ + if (source == TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST) { + authority_cert_dl_failed(cert->cache_info.identity_digest, + NULL, 404); + } else if (source == TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_SK_DIGEST) { + authority_cert_dl_failed(cert->cache_info.identity_digest, + cert->signing_key_digest, 404); + } } authority_cert_free(cert); @@ -445,17 +552,53 @@ authority_cert_get_all(smartlist_t *certs_out) } /** Called when an attempt to download a certificate with the authority with - * ID id_digest fails with HTTP response code status: remember - * the failure, so we don't try again immediately. */ + * ID id_digest and, if not NULL, signed with key signing_key_digest + * fails with HTTP response code status: remember the failure, so we + * don't try again immediately. */ void -authority_cert_dl_failed(const char *id_digest, int status) +authority_cert_dl_failed(const char *id_digest, + const char *signing_key_digest, int status) { cert_list_t *cl; + download_status_t *dlstatus = NULL; + char id_digest_str[2*DIGEST_LEN+1]; + char sk_digest_str[2*DIGEST_LEN+1]; + if (!trusted_dir_certs || !(cl = digestmap_get(trusted_dir_certs, id_digest))) return; - download_status_failed(&cl->dl_status, status); + /* + * Are we noting a failed download of the latest cert for the id digest, + * or of a download by (id, signing key) digest pair? + */ + if (!signing_key_digest) { + /* Just by id digest */ + download_status_failed(&cl->dl_status_by_id, status); + } else { + /* Reset by (id, signing key) digest pair + * + * Look for a download_status_t in the map with this digest + */ + dlstatus = dsmap_get(cl->dl_status_map, signing_key_digest); + /* Got one? */ + if (dlstatus) { + download_status_failed(dlstatus, status); + } else { + /* + * Do this rather than hex_str(), since hex_str clobbers + * old results and we call twice in the param list. + */ + base16_encode(id_digest_str, sizeof(id_digest_str), + id_digest, DIGEST_LEN); + base16_encode(sk_digest_str, sizeof(sk_digest_str), + signing_key_digest, DIGEST_LEN); + log_warn(LD_DIR, + "Got failure for cert fetch with (fp,sk) = (%s,%s), with " + "status %d, but knew nothing about the download.", + id_digest_str, sk_digest_str, status); + } + } } /** Return true iff when we've been getting enough failures when trying to @@ -471,7 +614,7 @@ authority_cert_dl_looks_uncertain(const char *id_digest) !(cl = digestmap_get(trusted_dir_certs, id_digest))) return 0; - n_failures = download_status_get_n_failures(&cl->dl_status); + n_failures = download_status_get_n_failures(&cl->dl_status_by_id); return n_failures >= N_AUTH_CERT_DL_FAILURES_TO_BUG_USER; } @@ -487,20 +630,38 @@ authority_cert_dl_looks_uncertain(const char *id_digest) void authority_certs_fetch_missing(networkstatus_t *status, time_t now) { - digestmap_t *pending; + /* + * The pending_id digestmap tracks pending certificate downloads by + * identity digest; the pending_cert digestmap tracks pending downloads + * by (identity digest, signing key digest) pairs. + */ + digestmap_t *pending_id; + fp_pair_map_t *pending_cert; authority_cert_t *cert; - smartlist_t *missing_digests; + /* + * The missing_id_digests smartlist will hold a list of id digests + * we want to fetch the newest cert for; the missing_cert_digests + * smartlist will hold a list of fp_pair_t with an identity and + * signing key digest. + */ + 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()); + 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())) return; - pending = digestmap_new(); - missing_digests = smartlist_new(); + pending_cert = fp_pair_map_new(); + pending_id = digestmap_new(); + missing_cert_digests = smartlist_new(); + missing_id_digests = smartlist_new(); - list_pending_downloads(pending, DIR_PURPOSE_FETCH_CERTIFICATE, "fp/"); + list_pending_downloads(pending_id, DIR_PURPOSE_FETCH_CERTIFICATE, "fp/"); + list_pending_fpsk_downloads(pending_cert); if (status) { SMARTLIST_FOREACH_BEGIN(status->voters, networkstatus_voter_info_t *, voter) { @@ -516,16 +677,43 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now) sig->signing_key_digest); if (cert) { if (now < cert->expires) - download_status_reset(&cl->dl_status); + download_status_reset_by_sk_in_cl(cl, sig->signing_key_digest); continue; } - if (download_status_is_ready(&cl->dl_status, now, - MAX_CERT_DL_FAILURES) && - !digestmap_get(pending, voter->identity_digest)) { - log_info(LD_DIR, "We're missing a certificate from authority " - "with signing key %s: launching request.", - hex_str(sig->signing_key_digest, DIGEST_LEN)); - smartlist_add(missing_digests, sig->identity_digest); + if (download_status_is_ready_by_sk_in_cl( + cl, sig->signing_key_digest, + now, MAX_CERT_DL_FAILURES) && + !fp_pair_map_get_by_digests(pending_cert, + voter->identity_digest, + sig->signing_key_digest)) { + /* + * Do this rather than hex_str(), since hex_str clobbers + * old results and we call twice in the param list. + */ + base16_encode(id_digest_str, sizeof(id_digest_str), + voter->identity_digest, DIGEST_LEN); + base16_encode(sk_digest_str, sizeof(sk_digest_str), + sig->signing_key_digest, DIGEST_LEN); + + if (voter->nickname) { + log_info(LD_DIR, + "We're missing a certificate from authority %s " + "(ID digest %s) with signing key %s: " + "launching request.", + voter->nickname, id_digest_str, sk_digest_str); + } else { + log_info(LD_DIR, + "We're missing a certificate from authority ID digest " + "%s with signing key %s: launching request.", + id_digest_str, sk_digest_str); + } + + /* Allocate a new fp_pair_t to append */ + fp_tmp = tor_malloc(sizeof(*fp_tmp)); + memcpy(fp_tmp->first, voter->identity_digest, sizeof(fp_tmp->first)); + memcpy(fp_tmp->second, sig->signing_key_digest, + sizeof(fp_tmp->second)); + smartlist_add(missing_cert_digests, fp_tmp); } } SMARTLIST_FOREACH_END(sig); } SMARTLIST_FOREACH_END(voter); @@ -534,60 +722,118 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now) int found = 0; if (!(ds->type & V3_DIRINFO)) continue; - if (smartlist_digest_isin(missing_digests, ds->v3_identity_digest)) + if (smartlist_digest_isin(missing_id_digests, ds->v3_identity_digest)) continue; cl = get_cert_list(ds->v3_identity_digest); SMARTLIST_FOREACH(cl->certs, authority_cert_t *, cert, { if (now < cert->expires) { /* It's not expired, and we weren't looking for something to * verify a consensus with. Call it done. */ - download_status_reset(&cl->dl_status); + download_status_reset(&(cl->dl_status_by_id)); + /* No sense trying to download it specifically by signing key hash */ + download_status_reset_by_sk_in_cl(cl, cert->signing_key_digest); found = 1; break; } }); if (!found && - download_status_is_ready(&cl->dl_status, now,MAX_CERT_DL_FAILURES) && - !digestmap_get(pending, ds->v3_identity_digest)) { - log_info(LD_DIR, "No current certificate known for authority %s; " - "launching request.", ds->nickname); - smartlist_add(missing_digests, ds->v3_identity_digest); + download_status_is_ready(&(cl->dl_status_by_id), now, + MAX_CERT_DL_FAILURES) && + !digestmap_get(pending_id, ds->v3_identity_digest)) { + log_info(LD_DIR, + "No current certificate known for authority %s " + "(ID digest %s); launching request.", + ds->nickname, hex_str(ds->v3_identity_digest, DIGEST_LEN)); + smartlist_add(missing_id_digests, ds->v3_identity_digest); } } SMARTLIST_FOREACH_END(ds); - if (!smartlist_len(missing_digests)) { - goto done; - } else { + /* Do downloads by identity digest */ + if (smartlist_len(missing_id_digests) > 0) { smartlist_t *fps = smartlist_new(); smartlist_add(fps, tor_strdup("fp/")); - SMARTLIST_FOREACH(missing_digests, const char *, d, { - char *fp; - if (digestmap_get(pending, d)) - continue; - fp = tor_malloc(HEX_DIGEST_LEN+2); - base16_encode(fp, HEX_DIGEST_LEN+1, d, DIGEST_LEN); - fp[HEX_DIGEST_LEN] = '+'; - fp[HEX_DIGEST_LEN+1] = '\0'; - smartlist_add(fps, fp); - }); - if (smartlist_len(fps) == 1) { - /* we didn't add any: they were all pending */ - SMARTLIST_FOREACH(fps, char *, cp, tor_free(cp)); - smartlist_free(fps); - goto done; + + SMARTLIST_FOREACH(missing_id_digests, const char *, d, { + char *fp; + if (digestmap_get(pending_id, d)) + continue; + fp = tor_malloc(HEX_DIGEST_LEN+2); + base16_encode(fp, HEX_DIGEST_LEN+1, d, DIGEST_LEN); + fp[HEX_DIGEST_LEN] = '+'; + fp[HEX_DIGEST_LEN+1] = '\0'; + smartlist_add(fps, fp); + }); + + if (smartlist_len(fps) > 1) { + resource = smartlist_join_strings(fps, "", 0, NULL); + resource[strlen(resource)-1] = '\0'; + directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0, + resource, PDS_RETRY_IF_NO_SERVERS); + tor_free(resource); } - resource = smartlist_join_strings(fps, "", 0, NULL); - resource[strlen(resource)-1] = '\0'; + /* else we didn't add any: they were all pending */ + SMARTLIST_FOREACH(fps, char *, cp, tor_free(cp)); smartlist_free(fps); } - directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0, - resource, PDS_RETRY_IF_NO_SERVERS); - done: - tor_free(resource); - smartlist_free(missing_digests); - digestmap_free(pending, NULL); + /* Do downloads by identity digest/signing key pair */ + if (smartlist_len(missing_cert_digests) > 0) { + smartlist_t *fp_pairs = smartlist_new(); + int need_plus = 0, offset = 0; + + smartlist_add(fp_pairs, tor_strdup("fp-sk/")); + + SMARTLIST_FOREACH_BEGIN(missing_cert_digests, const fp_pair_t *, d) { + char *fp_pair; + + if (fp_pair_map_get(pending_cert, d)) + continue; + + fp_pair = tor_malloc(2*HEX_DIGEST_LEN+3); + offset = 0; + if (need_plus) { + fp_pair[offset++] = '+'; + } else { + /* Prepend a '+' to all but the first in the list */ + need_plus = 1; + } + + /* Encode the first fingerprint */ + base16_encode(fp_pair + offset, HEX_DIGEST_LEN+1, + d->first, DIGEST_LEN); + offset += HEX_DIGEST_LEN; + /* Add a '-' to separate them */ + fp_pair[offset++] = '-'; + /* Encode the second fingerprint */ + base16_encode(fp_pair + offset, HEX_DIGEST_LEN+1, + d->second, DIGEST_LEN); + offset += HEX_DIGEST_LEN; + /* Add a NUL */ + fp_pair[offset++] = '\0'; + + /* Add it to the list of pairs to request */ + smartlist_add(fp_pairs, fp_pair); + } SMARTLIST_FOREACH_END(d); + + if (smartlist_len(fp_pairs) > 1) { + resource = smartlist_join_strings(fp_pairs, "", 0, NULL); + resource[strlen(resource)-1] = '\0'; + directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0, + resource, PDS_RETRY_IF_NO_SERVERS); + tor_free(resource); + } + /* else they were all pending */ + + SMARTLIST_FOREACH(fp_pairs, char *, p, tor_free(p)); + smartlist_free(fp_pairs); + } + + smartlist_free(missing_id_digests); + SMARTLIST_FOREACH(missing_cert_digests, fp_pair_t *, p, tor_free(p)); + smartlist_free(missing_cert_digests); + digestmap_free(pending_id, NULL); + fp_pair_map_free(pending_cert, NULL); } /* Router descriptor storage. @@ -4283,6 +4529,41 @@ list_pending_microdesc_downloads(digestmap_t *result) list_pending_downloads(result, DIR_PURPOSE_FETCH_MICRODESC, "d/"); } +/** For every certificate we are currently downloading by (identity digest, + * signing key digest) pair, set result[fp_pair] to (void *1). + */ +static void +list_pending_fpsk_downloads(fp_pair_map_t *result) +{ + const char *pfx = "fp-sk/"; + smartlist_t *tmp; + smartlist_t *conns; + const char *resource; + + tor_assert(result); + + tmp = smartlist_new(); + conns = get_connection_array(); + + SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn) { + if (conn->type == CONN_TYPE_DIR && + conn->purpose == DIR_PURPOSE_FETCH_CERTIFICATE && + !conn->marked_for_close) { + resource = TO_DIR_CONN(conn)->requested_resource; + if (!strcmpstart(resource, pfx)) + dir_split_resource_into_fingerprint_pairs(resource + strlen(pfx), + tmp); + } + } SMARTLIST_FOREACH_END(conn); + + SMARTLIST_FOREACH_BEGIN(tmp, fp_pair_t *, fp) { + fp_pair_map_set(result, fp, (void*)1); + tor_free(fp); + } SMARTLIST_FOREACH_END(fp); + + smartlist_free(tmp); +} + /** Launch downloads for all the descriptors whose digests or digests256 * are listed as digests[i] for lo <= i < hi. (Lo and hi may be out of * range.) If source is given, download from source; diff --git a/src/or/routerlist.h b/src/or/routerlist.h index 8dcc6eb..ba6f930 100644 --- a/src/or/routerlist.h +++ b/src/or/routerlist.h @@ -13,7 +13,20 @@ int get_n_authorities(dirinfo_type_t type); int trusted_dirs_reload_certs(void); -int trusted_dirs_load_certs_from_string(const char *contents, int from_store, + +/* + * Pass one of these as source to trusted_dirs_load_certs_from_string() + * to indicate whence string originates; this controls error handling + * behavior such as marking downloads as failed. + */ + +#define TRUSTED_DIRS_CERTS_SRC_SELF 0 +#define TRUSTED_DIRS_CERTS_SRC_FROM_STORE 1 +#define TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST 2 +#define TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_SK_DIGEST 3 +#define TRUSTED_DIRS_CERTS_SRC_FROM_VOTE 4 + +int trusted_dirs_load_certs_from_string(const char *contents, int source, int flush); void trusted_dirs_flush_certs_to_disk(void); authority_cert_t *authority_cert_get_newest_by_id(const char *id_digest); @@ -21,7 +34,8 @@ authority_cert_t *authority_cert_get_by_sk_digest(const char *sk_digest); authority_cert_t *authority_cert_get_by_digests(const char *id_digest, const char *sk_digest); void authority_cert_get_all(smartlist_t *certs_out); -void authority_cert_dl_failed(const char *id_digest, int status); +void authority_cert_dl_failed(const char *id_digest, + const char *signing_key_digest, int status); void authority_certs_fetch_missing(networkstatus_t *status, time_t now); int router_reload_router_list(void); int authority_cert_dl_looks_uncertain(const char *id_digest);

1 0

[tor/release-0.2.4] Avoid duplicate downloads by (fp, sk) and by fp for authority certs when bootstrapping
by arma＠torproject.org 25 May '13

25 May '13

commit 7b6ee54bdc96faf1cc72445a6dd7a1130148129c Author: Andrea Shepard <andrea(a)torproject.org> Date: Thu May 9 08:19:48 2013 -0700 Avoid duplicate downloads by (fp,sk) and by fp for authority certs when bootstrapping --- src/or/routerlist.c | 99 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 70 insertions(+), 29 deletions(-) diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 20d5110..15a71e4 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -660,8 +660,57 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now) missing_cert_digests = smartlist_new(); missing_id_digests = smartlist_new(); + /* + * 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_fpsk_downloads(pending_cert); + + /* + * Now, we download any trusted authority certs we don't have by + * identity digest only. This gets the latest cert for that authority. + */ + SMARTLIST_FOREACH_BEGIN(trusted_dir_servers, trusted_dir_server_t *, ds) { + int found = 0; + if (!(ds->type & V3_DIRINFO)) + continue; + if (smartlist_digest_isin(missing_id_digests, ds->v3_identity_digest)) + continue; + cl = get_cert_list(ds->v3_identity_digest); + SMARTLIST_FOREACH_BEGIN(cl->certs, authority_cert_t *, cert) { + if (now < cert->expires) { + /* It's not expired, and we weren't looking for something to + * verify a consensus with. Call it done. */ + download_status_reset(&(cl->dl_status_by_id)); + /* No sense trying to download it specifically by signing key hash */ + download_status_reset_by_sk_in_cl(cl, cert->signing_key_digest); + found = 1; + break; + } + } SMARTLIST_FOREACH_END(cert); + if (!found && + download_status_is_ready(&(cl->dl_status_by_id), now, + MAX_CERT_DL_FAILURES) && + !digestmap_get(pending_id, ds->v3_identity_digest)) { + log_info(LD_DIR, + "No current certificate known for authority %s " + "(ID digest %s); launching request.", + ds->nickname, hex_str(ds->v3_identity_digest, DIGEST_LEN)); + smartlist_add(missing_id_digests, ds->v3_identity_digest); + } + } SMARTLIST_FOREACH_END(ds); + + /* + * Next, if we have a consensus, scan through it and look for anything + * signed with a key from a cert we don't have. Those get downloaded + * by (fp,sk) pair, but if we don't know any certs at all for the fp + * (identity digest), and it's one of the trusted dir server certs + * we started off above or a pending download in pending_id, don't + * try to get it yet. Most likely, the one we'll get for that will + * have the right signing key too, and we'd just be downloading + * redundantly. + */ if (status) { SMARTLIST_FOREACH_BEGIN(status->voters, networkstatus_voter_info_t *, voter) { @@ -671,7 +720,28 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now) if (!cache && !trusteddirserver_get_by_v3_auth_digest(voter->identity_digest)) continue; /* We are not a cache, and we don't know this authority.*/ + + /* + * If we don't know *any* cert for this authority, and a download by ID + * is pending or we added it to missing_id_digests above, skip this + * one for now to avoid duplicate downloads. + */ cl = get_cert_list(voter->identity_digest); + if (smartlist_len(cl->certs) == 0) { + /* We have no certs at all for this one */ + + /* Do we have a download of one pending? */ + if (digestmap_get(pending_id, voter->identity_digest)) + continue; + + /* + * Are we about to launch a download of one due to the trusted + * dir server check above? + */ + if (smartlist_digest_isin(missing_id_digests, voter->identity_digest)) + continue; + } + SMARTLIST_FOREACH_BEGIN(voter->sigs, document_signature_t *, sig) { cert = authority_cert_get_by_digests(voter->identity_digest, sig->signing_key_digest); @@ -718,35 +788,6 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now) } SMARTLIST_FOREACH_END(sig); } SMARTLIST_FOREACH_END(voter); } - SMARTLIST_FOREACH_BEGIN(trusted_dir_servers, trusted_dir_server_t *, ds) { - int found = 0; - if (!(ds->type & V3_DIRINFO)) - continue; - if (smartlist_digest_isin(missing_id_digests, ds->v3_identity_digest)) - continue; - cl = get_cert_list(ds->v3_identity_digest); - SMARTLIST_FOREACH(cl->certs, authority_cert_t *, cert, { - if (now < cert->expires) { - /* It's not expired, and we weren't looking for something to - * verify a consensus with. Call it done. */ - download_status_reset(&(cl->dl_status_by_id)); - /* No sense trying to download it specifically by signing key hash */ - download_status_reset_by_sk_in_cl(cl, cert->signing_key_digest); - found = 1; - break; - } - }); - if (!found && - download_status_is_ready(&(cl->dl_status_by_id), now, - MAX_CERT_DL_FAILURES) && - !digestmap_get(pending_id, ds->v3_identity_digest)) { - log_info(LD_DIR, - "No current certificate known for authority %s " - "(ID digest %s); launching request.", - ds->nickname, hex_str(ds->v3_identity_digest, DIGEST_LEN)); - smartlist_add(missing_id_digests, ds->v3_identity_digest); - } - } SMARTLIST_FOREACH_END(ds); /* Do downloads by identity digest */ if (smartlist_len(missing_id_digests) > 0) {

1 0

[tor/release-0.2.4] Clean up ugly constants in connection_dir_download_cert_failed(), and fix a broken one
by arma＠torproject.org 25 May '13

25 May '13

commit c0d96bae666c1dc0c16b4df69190fa126131aa65 Author: Andrea Shepard <andrea(a)torproject.org> Date: Thu May 9 08:23:53 2013 -0700 Clean up ugly constants in connection_dir_download_cert_failed(), and fix a broken one --- src/or/directory.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 8dd5a7c..f65ac87 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -803,6 +803,8 @@ connection_dir_bridge_routerdesc_failed(dir_connection_t *conn) static void connection_dir_download_cert_failed(dir_connection_t *conn, int status) { + const char *fp_pfx = "fp/"; + const char *fpsk_pfx = "fp-sk/"; smartlist_t *failed; tor_assert(conn->_base.purpose == DIR_PURPOSE_FETCH_CERTIFICATE); @@ -814,19 +816,20 @@ connection_dir_download_cert_failed(dir_connection_t *conn, int status) * with "fp/") or download by fingerprint/signing key pair * (resource starts with "fp-sk/"). */ - if (!strcmpstart(conn->requested_resource, "fp/")) { + if (!strcmpstart(conn->requested_resource, fp_pfx)) { /* Download by fingerprint case */ - dir_split_resource_into_fingerprints(conn->requested_resource + 3, + dir_split_resource_into_fingerprints(conn->requested_resource + + strlen(fp_pfx), failed, NULL, DSR_HEX); SMARTLIST_FOREACH_BEGIN(failed, char *, cp) { /* Null signing key digest indicates download by fp only */ authority_cert_dl_failed(cp, NULL, status); tor_free(cp); } SMARTLIST_FOREACH_END(cp); - } else if (!strcmpstart(conn->requested_resource, "fp-sk/")) { + } else if (!strcmpstart(conn->requested_resource, fpsk_pfx)) { /* Download by (fp,sk) pairs */ - dir_split_resource_into_fingerprint_pairs(conn->requested_resource + 5, - failed); + dir_split_resource_into_fingerprint_pairs(conn->requested_resource + + strlen(fpsk_pfx), failed); SMARTLIST_FOREACH_BEGIN(failed, fp_pair_t *, cp) { authority_cert_dl_failed(cp->first, cp->second, status); tor_free(cp);

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

tor-commits May 2013