r15995@catbus: nickm | 2007-10-21 00:40:46 -0400

 More fixes for bad behavior when downloading extrainfos: do not download an ei if we lack the key to verify it, and do not download it if we already got it and found (weirdly) that it didn't match the corresponding server descriptor.

svn:r12071

diff --git a/src/or/or.h b/src/or/or.h
index 167035a7c36adc77e6585dc1a0f3cfd814f093cf..53d1b6b24efccaa6cdeef089fa70ac8bd64b027c 100644 (file)
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -1109,6 +1109,9 @@ typedef struct signed_descriptor_t {
   unsigned int do_not_cache : 1;
   /* If true, this item is meant to represent an extrainfo. */
   unsigned int is_extrainfo : 1;
+  /* If true, we got an extrainfo for this item, and the digest was right,
+   * but it was incompatible. */
+  unsigned int extrainfo_is_bogus : 1;
 } signed_descriptor_t;
 
 /** Information about another onion router in the network. */

diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index ca00cefeaa0a326e5dd7d7e8a5a36d49ae6b97e5..cb78fc6f2439090f90432869d332a6d935a5df63 100644 (file)
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -3817,6 +3817,10 @@ update_extrainfo_downloads(time_t now)
         sd = &((routerinfo_t*)smartlist_get(lst, i))->cache_info;
       if (sd->is_extrainfo)
         continue; /* This should never happen. */
+      if (old_routers && !router_get_by_digest(sd->identity_digest))
+        continue; /* Couldn't check the signature if we got it. */
+      if (sd->extrainfo_is_bogus)
+        continue;
       d = sd->extra_info_digest;
       if (tor_digest_is_zero(d)) {
         ++n_no_ei;
@@ -4042,13 +4046,14 @@ router_differences_are_cosmetic(routerinfo_t *r1, routerinfo_t *r2)
  * <b>msg</b> is present, set *<b>msg</b> to a description of the
  * incompatibility (if any)
  *
- * DOCDOC sd.
+ * DOCDOC sd.  DOCDOC extrainfo_is_bogus.
  **/
 int
 routerinfo_incompatible_with_extrainfo(routerinfo_t *ri, extrainfo_t *ei,
                                        signed_descriptor_t *sd,
                                        const char **msg)
 {
+  int digest_matches, r=1;
   tor_assert(ri);
   tor_assert(ei);
   if (!sd)
@@ -4059,13 +4064,15 @@ routerinfo_incompatible_with_extrainfo(routerinfo_t *ri, extrainfo_t *ei,
     return 1;
   }
 
-  /* The nickname must match exactly to have been generated at the same time
+  digest_matches = !memcmp(ei->cache_info.signed_descriptor_digest,
+                           sd->extra_info_digest, DIGEST_LEN);
+
+  /* The identity must match exactly to have been generated at the same time
    * by the same router. */
-  if (strcmp(ri->nickname, ei->nickname) ||
-      memcmp(ri->cache_info.identity_digest, ei->cache_info.identity_digest,
+  if (memcmp(ri->cache_info.identity_digest, ei->cache_info.identity_digest,
              DIGEST_LEN)) {
     if (msg) *msg = "Extrainfo nickname or identity did not match routerinfo";
-    return 1; /* different servers */
+    goto err; /* different servers */
   }
 
   if (ei->pending_sig) {
@@ -4077,7 +4084,7 @@ routerinfo_incompatible_with_extrainfo(routerinfo_t *ri, extrainfo_t *ei,
       ei->bad_sig = 1;
       tor_free(ei->pending_sig);
       if (msg) *msg = "Extrainfo signature bad, or signed with wrong key";
-      return 1; /* Bad signature, or no match. */
+      goto err; /* Bad signature, or no match. */
     }
 
     tor_free(ei->pending_sig);
@@ -4085,19 +4092,28 @@ routerinfo_incompatible_with_extrainfo(routerinfo_t *ri, extrainfo_t *ei,
 
   if (ei->cache_info.published_on < sd->published_on) {
     if (msg) *msg = "Extrainfo published time did not match routerdesc";
-    return 1;
+    goto err;
   } else if (ei->cache_info.published_on > sd->published_on) {
     if (msg) *msg = "Extrainfo published time did not match routerdesc";
-    return -1;
+    r = -1;
+    goto err;
   }
 
-  if (memcmp(ei->cache_info.signed_descriptor_digest,
-             sd->extra_info_digest, DIGEST_LEN)) {
+  if (!digest_matches) {
     if (msg) *msg = "Extrainfo digest did not match value from routerdesc";
-    return 1; /* Digest doesn't match declared value. */
+    goto err; /* Digest doesn't match declared value. */
   }
 
   return 0;
+ err:
+  if (digest_matches) {
+    /* This signature was okay, and the digest was right: This is indeed the
+     * corresponding extrainfo.  But insanely, it doesn't match the routerinfo
+     * that lists it.  Don't try to fetch this one again. */
+    sd->extrainfo_is_bogus = 1;
+  }
+
+  return r;
 }
 
 /** Assert that the internal representation of <b>rl</b> is