From ff2820c1ba717c11ca1a277387ec0c6ac13020ab Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 11 Oct 2007 18:01:12 +0000 Subject: [PATCH] r14892@Kushana: nickm | 2007-10-11 14:00:33 -0400 Fix a bunch of XXX020s: treat some 403s as INFO severity; remove some dead code; share the retry path for consensus routerdescs that are also listed in the v2 networkstatus; check even more aspects of votes when parsing them. svn:r11871 --- ChangeLog | 5 +++++ src/or/directory.c | 10 ++++------ src/or/dirvote.c | 8 -------- src/or/routerlist.c | 14 ++++++++++++-- src/or/routerparse.c | 28 +++++++++++++++++++--------- 5 files changed, 40 insertions(+), 25 deletions(-) diff --git a/ChangeLog b/ChangeLog index d039d250af..63b61b96e1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -27,6 +27,7 @@ Changes in version 0.2.0.8-alpha - 2007-10-12 documents. - All hosts now attempt to download and keep fresh v3 authority certificates, and re-attempt after failures. + - More internal-consistency checks for vote parsing. o Major bugfixes (performance): - Fix really bad O(n^2) performance when parsing a long list of @@ -78,6 +79,10 @@ Changes in version 0.2.0.8-alpha - 2007-10-12 - On some platforms, accept() can return a broken address. Detect this more quietly, and deal accordingly. (Fixes bug 483.) + o Minor bugfixes (usability): + - Treat some 403 responses from directory servers as INFO rather than + WARN-severity events. + o Minor bugfixes (DNS): - It's not actually an error to find a non-pending entry in the DNS cache when canceling a pending resolve. Don't log unless stuff diff --git a/src/or/directory.c b/src/or/directory.c index 4ebfa336b3..93f7be028e 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -400,7 +400,6 @@ directory_get_from_all_authorities(uint8_t dir_purpose, if (!(ds->type & V3_AUTHORITY)) continue; rs = &ds->fake_status; - /* XXXX020 should this ever tunnel via tor? */ directory_initiate_command_routerstatus(rs, dir_purpose, router_purpose, 0, resource, NULL, 0); }); @@ -1343,8 +1342,8 @@ connection_dir_client_reached_eof(dir_connection_t *conn) log_info(LD_DIR,"Received networkstatus objects (size %d) from server " "'%s:%d'",(int) body_len, conn->_base.address, conn->_base.port); if (status_code != 200) { - /* XXXX020 This warning tends to freak out clients who get a 403. */ - log_warn(LD_DIR, + int dir_okay = status_code == 403; + log_fn(dir_okay ? LOG_INFO : LOG_WARN, LD_DIR, "Received http status code %d (%s) from server " "'%s:%d' while fetching \"/tor/status/%s\". I'll try again soon.", status_code, escaped(reason), conn->_base.address, @@ -1372,7 +1371,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn) smartlist_add(which, hex); }); } else { - /* Can we even end up here? -- weasel*/ + /* XXXX Can we even end up here? -- weasel*/ source = NS_FROM_DIR_BY_FP; log_warn(LD_BUG, "We received a networkstatus but we didn't ask " "for it by fp, nor did we ask for all."); @@ -1506,11 +1505,10 @@ connection_dir_client_reached_eof(dir_connection_t *conn) n_asked_for = smartlist_len(which); } if (status_code != 200) { - int dir_okay = status_code == 404 || + int dir_okay = status_code == 404 || status_code == 403 || (status_code == 400 && !strcmp(reason, "Servers unavailable.")); /* 404 means that it didn't have them; no big deal. * Older (pre-0.1.1.8) servers said 400 Servers unavailable instead. */ - /* XXXX020 This warning tends to freak out clients who get a 403. */ log_fn(dir_okay ? LOG_INFO : LOG_WARN, LD_DIR, "Received http status code %d (%s) from server '%s:%d' " "while fetching \"/tor/server/%s\". I'll try again soon.", diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 9e048d2aa6..9625dca058 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -325,14 +325,6 @@ networkstatus_compute_consensus(smartlist_t *votes, vote_seconds = median_int(votesec_list, n_votes); dist_seconds = median_int(distsec_list, n_votes); - /* - SMARTLIST_FOREACH(va_times, int*, i, - printf("VA: %d\n", *i)); - SMARTLIST_FOREACH(fu_times, int*, i, - printf("FU: %d\n", *i)); - printf("%d..%d\n", (int)valid_after, (int)valid_until); - */ - tor_assert(valid_after+MIN_VOTE_INTERVAL <= fresh_until); tor_assert(fresh_until+MIN_VOTE_INTERVAL <= valid_until); tor_assert(vote_seconds >= MIN_VOTE_SECONDS); diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 169e0df8f9..ac779045b9 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -3826,10 +3826,20 @@ update_consensus_router_descriptor_downloads(time_t now) list_pending_descriptor_downloads(map, 0); SMARTLIST_FOREACH(consensus->routerstatus_list, routerstatus_t *, rs, { - /* ????020 need-to-mirror? */ - /* XXXX rate-limit retries. */ + routerstatus_t *lrs; if (router_get_by_descriptor_digest(rs->descriptor_digest)) continue; /* We have it already. */ + + /* XXXX020 change this once the real consensus is the canonical place + * to go for router information. */ + lrs = router_get_combined_status_by_digest(rs->identity_digest); + if (!lrs || + memcmp(lrs->descriptor_digest, rs->descriptor_digest, DIGEST_LEN)) + lrs = rs; + if (!download_status_is_ready(&lrs->dl_status, now, + MAX_ROUTERDESC_DOWNLOAD_FAILURES)) + continue; + if (authdir && dirserv_would_reject_router(rs)) continue; /* We would throw it out immediately. */ if (!dirserver && !client_would_use_router(rs, now, options)) diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 7bb3228774..b40446e35a 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -1903,7 +1903,7 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, directory_token_t *tok; int ok; struct in_addr in; - int i, inorder; + int i, inorder, n_signatures = 0; if (router_get_networkstatus_v3_hash(s, ns_digest)) { log_warn(LD_DIR, "Unable to compute digest of network-status"); @@ -2040,6 +2040,11 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, "network-status vote.", escaped(tok->args[1])); goto err; } + if (is_vote && memcmp(ns->cert->cache_info.identity_digest, + voter->identity_digest, DIGEST_LEN)) { + log_warn(LD_DIR,"Mismatch between identities in certificate and vote"); + goto err; + } voter->address = tor_strdup(tok->args[2]); if (!tor_inet_aton(tok->args[3], &in)) { log_warn(LD_DIR, "Error decoding IP address %s in network-status.", @@ -2174,13 +2179,13 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, goto err; } - if (is_vote && - memcmp(declared_identity, ns->cert->cache_info.identity_digest, - DIGEST_LEN)) { - log_warn(LD_DIR, "Digest mismatch between declared and actual on " - "network-status vote."); - goto err; - /* XXXX020 also check cert against dir-source line. */ + if (is_vote) { + if (memcmp(declared_identity, ns->cert->cache_info.identity_digest, + DIGEST_LEN)) { + log_warn(LD_DIR, "Digest mismatch between declared and actual on " + "network-status vote."); + goto err; + } } if (is_vote) { @@ -2192,8 +2197,13 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, v->signature = tor_memdup(tok->object_body, tok->object_size); v->signature_len = tok->object_size; } + ++n_signatures; }); - /* XXXX020 enforce: vote must have at least one signature. */ + + if (! n_signatures) { + log_warn(LD_DIR, "No signatures on networkstatus vote."); + goto err; + } /* XXXX020 check dates for plausibility. ??? */