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

5 files changed, 40 insertions, 25 deletions

diff --git a/ChangeLog b/ChangeLog
index d039d250a..63b61b96e 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 4ebfa336b..93f7be028 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 9e048d2aa..9625dca05 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 169e0df8f..ac779045b 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 7bb322877..b40446e35 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.  ??? */