Browse Source

Improve API of routerinfo_incompatible_with_extrainfo()

This API change makes it so that routerinfo_incompatible...() no
longer takes a routerinfo_t, so that it's obvious that it should
only look at fields from the signed_descriptor_t.

This change should prevent a recurrence of #17150.
Nick Mathewson 8 years ago
parent
commit
00f74e0372
4 changed files with 19 additions and 16 deletions
  1. 6 3
      src/or/dirserv.c
  2. 2 1
      src/or/router.c
  3. 10 11
      src/or/routerlist.c
  4. 1 1
      src/or/routerlist.h

+ 6 - 3
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;
   }

+ 2 - 1
src/or/router.c

@@ -2072,7 +2072,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;

+ 10 - 11
src/or/routerlist.c

@@ -2900,7 +2900,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) ?
@@ -4901,9 +4901,9 @@ router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2)
 /** Check whether <b>sd</b> describes a router descriptor compatible with the
  * extrainfo document <b>ei</b>.
  *
- * <b>ri</b> (which must also be provided) is the full routerinfo corresponding
- * to the same router -- but note that it might not refer to the same specific
- * descriptor as sd.
+ * <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
@@ -4915,16 +4915,15 @@ router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2)
  * 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.";
@@ -4942,7 +4941,7 @@ routerinfo_incompatible_with_extrainfo(const routerinfo_t *ri,
 
   /* 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";
@@ -4956,7 +4955,7 @@ routerinfo_incompatible_with_extrainfo(const routerinfo_t *ri,
 
   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,
@@ -4967,7 +4966,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);
   }
 

+ 1 - 1
src/or/routerlist.h

@@ -187,7 +187,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);