Merge branch 'bug17150_027_extra' into maint-0.2.8

7 files changed, 56 insertions, 27 deletions

diff --git a/changes/bug17150 b/changes/bug17150
new file mode 100644
index 0000000000..686cc34296
--- /dev/null
+++ b/changes/bug17150
@@ -0,0 +1,7 @@
+  o Minor bugfixes (directory warnings):
+    - When fetching extrainfo documents, compare their SHA256 digests
+      and Ed25519 signing key certificates
+      with the routerinfo that led us to fetch them, rather than
+      with the most recent routerinfo. Otherwise we generate many
+      spurious warnings about mismatches. Fixes bug 17150; bugfix
+      on 0.2.7.2-alpha.
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index d38a024e14..7549795817 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -691,12 +691,14 @@ dirserv_add_descriptor(routerinfo_t *ri, const char **msg, const char *source)
 static was_router_added_t
 dirserv_add_extrainfo(extrainfo_t *ei, const char **msg)
 {
-  const routerinfo_t *ri;
+  routerinfo_t *ri;
   int r;
   tor_assert(msg);
   *msg = NULL;
 
-  ri = router_get_by_id_digest(ei->cache_info.identity_digest);
+  /* Needs to be mutable so routerinfo_incompatible_with_extrainfo
+   * can mess with some of the flags in ri->cache_info. */
+  ri = router_get_mutable_by_digest(ei->cache_info.identity_digest);
   if (!ri) {
     *msg = "No corresponding router descriptor for extra-info descriptor";
     extrainfo_free(ei);
@@ -716,7 +718,8 @@ dirserv_add_extrainfo(extrainfo_t *ei, const char **msg)
     return ROUTER_BAD_EI;
   }
 
-  if ((r = routerinfo_incompatible_with_extrainfo(ri, ei, NULL, msg))) {
+  if ((r = routerinfo_incompatible_with_extrainfo(ri->identity_pkey, ei,
+                                                  &ri->cache_info, msg))) {
     extrainfo_free(ei);
     return r < 0 ? ROUTER_IS_ALREADY_KNOWN : ROUTER_BAD_EI;
   }
diff --git a/src/or/or.h b/src/or/or.h
index 6694bb4ece..aa93c1c030 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2065,6 +2065,10 @@ typedef struct signed_descriptor_t {
   time_t published_on;
   /** For routerdescs only: digest of the corresponding extrainfo. */
   char extra_info_digest[DIGEST_LEN];
+  /** For routerdescs only: A SHA256-digest of the extrainfo (if any) */
+  char extra_info_digest256[DIGEST256_LEN];
+  /** Certificate for ed25519 signing key. */
+  struct tor_cert_st *signing_key_cert;
   /** For routerdescs only: Status of downloading the corresponding
    * extrainfo. */
   download_status_t ei_dl_status;
@@ -2096,8 +2100,6 @@ typedef int16_t country_t;
 /** Information about another onion router in the network. */
 typedef struct {
   signed_descriptor_t cache_info;
-  /** A SHA256-digest of the extrainfo (if any) */
-  char extra_info_digest256[DIGEST256_LEN];
   char *nickname; /**< Human-readable OR name. */
 
   uint32_t addr; /**< IPv4 address of OR, in host order. */
@@ -2115,7 +2117,8 @@ typedef struct {
   crypto_pk_t *identity_pkey;  /**< Public RSA key for signing. */
   /** Public curve25519 key for onions */
   curve25519_public_key_t *onion_curve25519_pkey;
-  /** Certificate for ed25519 signing key */
+  /** Certificate for ed25519 signing key
+   * (XXXX duplicated in cache_info.) */
   struct tor_cert_st *signing_key_cert;
   /** What's the earliest expiration time on all the certs in this
    * routerinfo? */
@@ -2192,7 +2195,8 @@ typedef struct extrainfo_t {
   uint8_t digest256[DIGEST256_LEN];
   /** The router's nickname. */
   char nickname[MAX_NICKNAME_LEN+1];
-  /** Certificate for ed25519 signing key */
+  /** Certificate for ed25519 signing key
+   * (XXXX duplicated in cache_info.) */
   struct tor_cert_st *signing_key_cert;
   /** True iff we found the right key for this extra-info, verified the
    * signature, and found it to be bad. */
diff --git a/src/or/router.c b/src/or/router.c
index d70756fdbf..cd8437d787 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -2037,6 +2037,7 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
     return -1;
   }
   ri->signing_key_cert = tor_cert_dup(get_master_signing_key_cert());
+  ri->cache_info.signing_key_cert = tor_cert_dup(get_master_signing_key_cert());
 
   get_platform_str(platform, sizeof(platform));
   ri->platform = tor_strdup(platform);
@@ -2129,6 +2130,8 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
   strlcpy(ei->nickname, get_options()->Nickname, sizeof(ei->nickname));
   ei->cache_info.published_on = ri->cache_info.published_on;
   ei->signing_key_cert = tor_cert_dup(get_master_signing_key_cert());
+  ei->cache_info.signing_key_cert = tor_cert_dup(get_master_signing_key_cert());
+
   memcpy(ei->cache_info.identity_digest, ri->cache_info.identity_digest,
          DIGEST_LEN);
   if (extrainfo_dump_to_string(&ei->cache_info.signed_descriptor_body,
@@ -2154,7 +2157,7 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
     memcpy(ri->cache_info.extra_info_digest,
            ei->cache_info.signed_descriptor_digest,
            DIGEST_LEN);
-    memcpy(ri->extra_info_digest256,
+    memcpy(ri->cache_info.extra_info_digest256,
            ei->digest256,
            DIGEST256_LEN);
   } else {
@@ -2195,7 +2198,8 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
                          ri->cache_info.signed_descriptor_digest);
 
   if (ei) {
-    tor_assert(! routerinfo_incompatible_with_extrainfo(ri, ei, NULL, NULL));
+    tor_assert(! routerinfo_incompatible_with_extrainfo(ri->identity_pkey, ei,
+                                                        &ri->cache_info, NULL));
   }
 
   *r = ri;
@@ -2669,9 +2673,9 @@ router_dump_router_to_string(routerinfo_t *router,
     char extra_info_digest[HEX_DIGEST_LEN+1];
     base16_encode(extra_info_digest, sizeof(extra_info_digest),
                   router->cache_info.extra_info_digest, DIGEST_LEN);
-    if (!tor_digest256_is_zero(router->extra_info_digest256)) {
+    if (!tor_digest256_is_zero(router->cache_info.extra_info_digest256)) {
       char d256_64[BASE64_DIGEST256_LEN+1];
-      digest256_to_base64(d256_64, router->extra_info_digest256);
+      digest256_to_base64(d256_64, router->cache_info.extra_info_digest256);
       tor_asprintf(&extra_info_line, "extra-info-digest %s %s\n",
                    extra_info_digest, d256_64);
     } else {
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index d40d704a1d..045d50c3db 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -2898,6 +2898,7 @@ routerinfo_free(routerinfo_t *router)
   if (router->identity_pkey)
     crypto_pk_free(router->identity_pkey);
   tor_cert_free(router->signing_key_cert);
+  tor_cert_free(router->cache_info.signing_key_cert);
   if (router->declared_family) {
     SMARTLIST_FOREACH(router->declared_family, char *, s, tor_free(s));
     smartlist_free(router->declared_family);
@@ -2917,6 +2918,7 @@ extrainfo_free(extrainfo_t *extrainfo)
   if (!extrainfo)
     return;
   tor_cert_free(extrainfo->signing_key_cert);
+  tor_cert_free(extrainfo->cache_info.signing_key_cert);
   tor_free(extrainfo->cache_info.signed_descriptor_body);
   tor_free(extrainfo->pending_sig);
 
@@ -3126,7 +3128,7 @@ extrainfo_insert,(routerlist_t *rl, extrainfo_t *ei, int warn_if_incompatible))
                      "Mismatch in digest in extrainfo map.");
     goto done;
   }
-  if (routerinfo_incompatible_with_extrainfo(ri, ei, sd,
+  if (routerinfo_incompatible_with_extrainfo(ri->identity_pkey, ei, sd,
                                              &compatibility_error_msg)) {
     char d1[HEX_DIGEST_LEN+1], d2[HEX_DIGEST_LEN+1];
     r = (ri->cache_info.extrainfo_is_bogus) ?
@@ -5165,25 +5167,32 @@ router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2)
   return 1;
 }
 
-/** Check whether <b>ri</b> (a.k.a. sd) is a router compatible with the
- * extrainfo document
- * <b>ei</b>.  If no router is compatible with <b>ei</b>, <b>ei</b> should be
+/** Check whether <b>sd</b> describes a router descriptor compatible with the
+ * extrainfo document <b>ei</b>.
+ *
+ * <b>identity_pkey</b> (which must also be provided) is RSA1024 identity key
+ * for the router. We use it to check the signature of the extrainfo document,
+ * if it has not already been checked.
+ *
+ * If no router is compatible with <b>ei</b>, <b>ei</b> should be
  * dropped.  Return 0 for "compatible", return 1 for "reject, and inform
  * whoever uploaded <b>ei</b>, and return -1 for "reject silently.".  If
  * <b>msg</b> is present, set *<b>msg</b> to a description of the
  * incompatibility (if any).
+ *
+ * Set the extrainfo_is_bogus field in <b>sd</b> if the digests matched
+ * but the extrainfo was nonetheless incompatible.
  **/
 int
-routerinfo_incompatible_with_extrainfo(const routerinfo_t *ri,
+routerinfo_incompatible_with_extrainfo(const crypto_pk_t *identity_pkey,
                                        extrainfo_t *ei,
                                        signed_descriptor_t *sd,
                                        const char **msg)
 {
   int digest_matches, digest256_matches, r=1;
-  tor_assert(ri);
+  tor_assert(identity_pkey);
+  tor_assert(sd);
   tor_assert(ei);
-  if (!sd)
-    sd = (signed_descriptor_t*)&ri->cache_info;
 
   if (ei->bad_sig) {
     if (msg) *msg = "Extrainfo signature was bad, or signed with wrong key.";
@@ -5195,27 +5204,27 @@ routerinfo_incompatible_with_extrainfo(const routerinfo_t *ri,
   /* Set digest256_matches to 1 if the digest is correct, or if no
    * digest256 was in the ri. */
   digest256_matches = tor_memeq(ei->digest256,
-                                ri->extra_info_digest256, DIGEST256_LEN);
+                                sd->extra_info_digest256, DIGEST256_LEN);
   digest256_matches |=
-    tor_mem_is_zero(ri->extra_info_digest256, DIGEST256_LEN);
+    tor_mem_is_zero(sd->extra_info_digest256, DIGEST256_LEN);
 
   /* The identity must match exactly to have been generated at the same time
    * by the same router. */
-  if (tor_memneq(ri->cache_info.identity_digest,
+  if (tor_memneq(sd->identity_digest,
                  ei->cache_info.identity_digest,
                  DIGEST_LEN)) {
     if (msg) *msg = "Extrainfo nickname or identity did not match routerinfo";
     goto err; /* different servers */
   }
 
-  if (! tor_cert_opt_eq(ri->signing_key_cert, ei->signing_key_cert)) {
+  if (! tor_cert_opt_eq(sd->signing_key_cert, ei->signing_key_cert)) {
     if (msg) *msg = "Extrainfo signing key cert didn't match routerinfo";
     goto err; /* different servers */
   }
 
   if (ei->pending_sig) {
     char signed_digest[128];
-    if (crypto_pk_public_checksig(ri->identity_pkey,
+    if (crypto_pk_public_checksig(identity_pkey,
                        signed_digest, sizeof(signed_digest),
                        ei->pending_sig, ei->pending_sig_len) != DIGEST_LEN ||
         tor_memneq(signed_digest, ei->cache_info.signed_descriptor_digest,
@@ -5226,7 +5235,7 @@ routerinfo_incompatible_with_extrainfo(const routerinfo_t *ri,
       goto err; /* Bad signature, or no match. */
     }
 
-    ei->cache_info.send_unencrypted = ri->cache_info.send_unencrypted;
+    ei->cache_info.send_unencrypted = sd->send_unencrypted;
     tor_free(ei->pending_sig);
   }
 
diff --git a/src/or/routerlist.h b/src/or/routerlist.h
index bc48c2087c..67cc253c5a 100644
--- a/src/or/routerlist.h
+++ b/src/or/routerlist.h
@@ -191,7 +191,7 @@ void update_extrainfo_downloads(time_t now);
 void router_reset_descriptor_download_failures(void);
 int router_differences_are_cosmetic(const routerinfo_t *r1,
                                     const routerinfo_t *r2);
-int routerinfo_incompatible_with_extrainfo(const routerinfo_t *ri,
+int routerinfo_incompatible_with_extrainfo(const crypto_pk_t *ri,
                                            extrainfo_t *ei,
                                            signed_descriptor_t *sd,
                                            const char **msg);
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index cec10c8f24..b108dd71a1 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -1406,6 +1406,7 @@ router_parse_entry_from_string(const char *s, const char *end,
         goto err;
       }
       router->signing_key_cert = cert; /* makes sure it gets freed. */
+      router->cache_info.signing_key_cert = tor_cert_dup(cert);
 
       if (cert->cert_type != CERT_TYPE_ID_SIGNING ||
           ! cert->signing_key_included) {
@@ -1600,7 +1601,7 @@ router_parse_entry_from_string(const char *s, const char *end,
     }
 
     if (tok->n_args >= 2) {
-      if (digest256_from_base64(router->extra_info_digest256, tok->args[1])
+      if (digest256_from_base64(router->cache_info.extra_info_digest256, tok->args[1])
           < 0) {
         log_warn(LD_DIR, "Invalid extra info digest256 %s",
                  escaped(tok->args[1]));
@@ -1787,6 +1788,7 @@ extrainfo_parse_entry_from_string(const char *s, const char *end,
         goto err;
       }
       extrainfo->signing_key_cert = cert; /* makes sure it gets freed. */
+      extrainfo->cache_info.signing_key_cert = tor_cert_dup(cert);
       if (cert->cert_type != CERT_TYPE_ID_SIGNING ||
           ! cert->signing_key_included) {
         log_warn(LD_DIR, "Invalid form for ed25519 cert");