Merge branch 'maint-0.2.1' into release-0.2.1

changes/check-fetched-rend-desc-service-id

@@ -0,0 +1,7 @@
+  o Security fixes:
+    - When fetching a hidden service descriptor, check that it is for
+      the hidden service we were trying to connect to, in order to
+      stop a directory from pre-seeding a client with a descriptor for
+      a hidden service that they didn't want.  Bugfix on 0.0.6.
+
+

changes/check-public-key-exponents

@@ -0,0 +1,5 @@
+  o Minor bugfixes:
+    - Require that introduction point keys and onion keys have public
+      exponent 65537.  Bugfix on 0.2.0.10-alpha.
+
+

src/common/container.h

@@ -241,7 +241,7 @@ char *smartlist_join_strings2(smartlist_t *sl, const char *join,
  * Example use:
  *  SMARTLIST_FOREACH_JOIN(routerstatus_list, routerstatus_t *, rs,
  *                     routerinfo_list, routerinfo_t *, ri,
- *                     tor_memcmp(rs->identity_digest, ri->identity_digest, 20),
+ *                    tor_memcmp(rs->identity_digest, ri->identity_digest, 20),
  *                     log_info(LD_GENERAL,"No match for %s", ri->nickname)) {
  *    log_info(LD_GENERAL, "%s matches routerstatus %p", ri->nickname, rs);
  * } SMARTLIST_FOREACH_JOIN_END(rs, ri);

src/common/crypto.c

@@ -656,6 +656,18 @@ crypto_pk_key_is_private(const crypto_pk_env_t *key)
   return PRIVATE_KEY_OK(key);
 }
 
+/** Return true iff <b>env</b> contains a public key whose public exponent
+ * equals 65537.
+ */
+int
+crypto_pk_public_exponent_ok(crypto_pk_env_t *env)
+{
+  tor_assert(env);
+  tor_assert(env->key);
+
+  return BN_is_word(env->key->e, 65537);
+}
+
 /** Compare the public-key components of a and b.  Return -1 if a\<b, 0
  * if a==b, and 1 if a\>b.
  */

src/common/crypto.h

@@ -92,6 +92,7 @@ size_t crypto_pk_keysize(crypto_pk_env_t *env);
 crypto_pk_env_t *crypto_pk_dup_key(crypto_pk_env_t *orig);
 crypto_pk_env_t *crypto_pk_copy_full(crypto_pk_env_t *orig);
 int crypto_pk_key_is_private(const crypto_pk_env_t *key);
+int crypto_pk_public_exponent_ok(crypto_pk_env_t *env);
 
 int crypto_pk_public_encrypt(crypto_pk_env_t *env, char *to, size_t tolen,
                              const char *from, size_t fromlen, int padding);

src/common/di_ops.c

@@ -3,18 +3,18 @@
 
 /**
  * \file di_ops.c
- * \brief Functions for data-independent operations
+ * \brief Functions for data-independent operations.
  **/
 
 #include "orconfig.h"
 #include "di_ops.h"
 
 /**
- * Timing-safe version of memcmp.  As memcmp, compare the <b>sz</b> bytes
- * at <b>a</b> with the <b>sz</b> bytes at <b>, and returns less than 0 if the
- * bytes at <b>a</b> lexically precede those at <b>b</b>, 0 if the byte ranges
- * are equal, and greater than zero if the bytes at <b>a</b> lexically follow
- * those at <b>.
+ * Timing-safe version of memcmp.  As memcmp, compare the <b>sz</b> bytes at
+ * <b>a</b> with the <b>sz</b> bytes at <b>b</b>, and return less than 0 if
+ * the bytes at <b>a</b> lexically precede those at <b>b</b>, 0 if the byte
+ * ranges are equal, and greater than zero if the bytes at <b>a</b> lexically
+ * follow those at <b>b</b>.
  *
  * This implementation differs from memcmp in that its timing behavior is not
  * data-dependent: it should return in the same amount of time regardless of
@@ -85,7 +85,7 @@ tor_memcmp(const void *a, const void *b, size_t len)
 
 /**
  * Timing-safe memory comparison.  Return true if the <b>sz</b> bytes at
- * <b>a</b> are the same as the <b>sz</b> bytes at <b>, and 0 otherwise.
+ * <b>a</b> are the same as the <b>sz</b> bytes at <b>b</b>, and 0 otherwise.
  *
  * This implementation differs from !memcmp(a,b,sz) in that its timing
  * behavior is not data-dependent: it should return in the same amount of time

src/common/di_ops.h

@@ -28,3 +28,4 @@ int tor_memeq(const void *a, const void *b, size_t sz);
 #define fast_memneq(a,b,c) (0!=memcmp((a),(b),(c)))
 
 #endif
+

src/or/directory.c

@@ -1909,7 +1909,8 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
              (int)body_len, status_code, escaped(reason));
     switch (status_code) {
       case 200:
-        if (rend_cache_store(body, body_len, 0) < -1) {
+        if (rend_cache_store(body, body_len, 0,
+                             conn->rend_data->onion_address) < -1) {
           log_warn(LD_REND,"Failed to parse rendezvous descriptor.");
           /* Any pending rendezvous attempts will notice when
            * connection_about_to_close_connection()
@@ -3114,7 +3115,7 @@ directory_handle_command_post(dir_connection_t *conn, const char *headers,
       !strcmpstart(url,"/tor/rendezvous/publish")) {
     /* rendezvous descriptor post */
     log_info(LD_REND, "Handling rendezvous descriptor post.");
-    if (rend_cache_store(body, body_len, 1) < 0) {
+    if (rend_cache_store(body, body_len, 1, NULL) < 0) {
       log_fn(LOG_PROTOCOL_WARN, LD_DIRSERV,
              "Rejected rend descriptor (length %d) from %s.",
              (int)body_len, conn->_base.address);

src/or/dirvote.c

@@ -314,8 +314,9 @@ compare_vote_rs(const vote_routerstatus_t *a, const vote_routerstatus_t *b)
   if ((r = fast_memcmp(a->status.identity_digest, b->status.identity_digest,
                   DIGEST_LEN)))
     return r;
-  if ((r = fast_memcmp(a->status.descriptor_digest, b->status.descriptor_digest,
-                  DIGEST_LEN)))
+  if ((r = fast_memcmp(a->status.descriptor_digest,
+                       b->status.descriptor_digest,
+                       DIGEST_LEN)))
     return r;
   if ((r = (int)(b->status.published_on - a->status.published_on)))
     return r;
@@ -823,7 +824,8 @@ networkstatus_compute_consensus(smartlist_t *votes,
         if (index[v_sl_idx] < size[v_sl_idx]) {
           rs = smartlist_get(v->routerstatus_list, index[v_sl_idx]);
           if (!lowest_id ||
-              fast_memcmp(rs->status.identity_digest, lowest_id, DIGEST_LEN) < 0)
+              fast_memcmp(rs->status.identity_digest,
+                          lowest_id, DIGEST_LEN) < 0)
             lowest_id = rs->status.identity_digest;
         }
       });
@@ -881,7 +883,7 @@ networkstatus_compute_consensus(smartlist_t *votes,
        * routerinfo and its contents are. */
       rs = compute_routerstatus_consensus(matching_descs);
       /* Copy bits of that into rs_out. */
-      tor_assert(fast_memeq(lowest_id, rs->status.identity_digest, DIGEST_LEN));
+      tor_assert(fast_memeq(lowest_id, rs->status.identity_digest,DIGEST_LEN));
       memcpy(rs_out.identity_digest, lowest_id, DIGEST_LEN);
       memcpy(rs_out.descriptor_digest, rs->status.descriptor_digest,
              DIGEST_LEN);

src/or/networkstatus.c

@@ -1433,8 +1433,9 @@ networkstatus_set_current_consensus(const char *consensus, unsigned flags)
   }
 
   if (current_consensus &&
-      tor_memeq(c->networkstatus_digest, current_consensus->networkstatus_digest,
-              DIGEST_LEN)) {
+      tor_memeq(c->networkstatus_digest,
+                current_consensus->networkstatus_digest,
+                DIGEST_LEN)) {
     /* We already have this one. That's a failure. */
     log_info(LD_DIR, "Got a consensus we already have");
     goto done;

src/or/or.h

@@ -4146,7 +4146,8 @@ int rend_cache_lookup_desc(const char *query, int version, const char **desc,
 int rend_cache_lookup_entry(const char *query, int version,
                             rend_cache_entry_t **entry_out);
 int rend_cache_lookup_v2_desc_as_dir(const char *query, const char **desc);
-int rend_cache_store(const char *desc, size_t desc_len, int published);
+int rend_cache_store(const char *desc, size_t desc_len, int published,
+                     const char *service_id);
 int rend_cache_store_v2_desc_as_client(const char *desc,
                                        const rend_data_t *rend_query);
 int rend_cache_store_v2_desc_as_dir(const char *desc);

src/or/rendcommon.c

@@ -1047,9 +1047,14 @@ rend_cache_lookup_v2_desc_as_dir(const char *desc_id, const char **desc)
  *
  * The published flag tells us if we store the descriptor
  * in our role as directory (1) or if we cache it as client (0).
+ *
+ * If <b>service_id</b> is non-NULL and the descriptor is not for that
+ * service ID, reject it.  <b>service_id</b> must be specified if and
+ * only if <b>published</b> is 0 (we fetched this descriptor).
  */
 int
-rend_cache_store(const char *desc, size_t desc_len, int published)
+rend_cache_store(const char *desc, size_t desc_len, int published,
+                 const char *service_id)
 {
   rend_cache_entry_t *e;
   rend_service_descriptor_t *parsed;
@@ -1068,6 +1073,12 @@ rend_cache_store(const char *desc, size_t desc_len, int published)
     rend_service_descriptor_free(parsed);
     return -2;
   }
+  if ((service_id != NULL) && strcmp(query, service_id)) {
+    log_warn(LD_REND, "Received service descriptor for service ID %s; "
+             "expected descriptor for service ID %s.",
+             query, safe_str(service_id));
+    return -2;
+  }
   now = time(NULL);
   if (parsed->timestamp < now-REND_CACHE_MAX_AGE-REND_CACHE_MAX_SKEW) {
     log_fn(LOG_PROTOCOL_WARN, LD_REND,
@@ -1253,6 +1264,8 @@ rend_cache_store_v2_desc_as_dir(const char *desc)
  * If we have an older descriptor with the same ID, replace it.
  * If we have any v0 descriptor with the same ID, reject this one in order
  * to not get confused with having both versions for the same service.
+ * If the descriptor's service ID does not match
+ * <b>rend_query</b>-\>onion_address, reject it.
  * Return -2 if it's malformed or otherwise rejected; return -1 if we
  * already have a v0 descriptor here; return 0 if it's the same or older
  * than one we've already got; return 1 if it's novel.
@@ -1303,6 +1316,13 @@ rend_cache_store_v2_desc_as_client(const char *desc,
     retval = -2;
     goto err;
   }
+  if (strcmp(rend_query->onion_address, service_id)) {
+    log_warn(LD_REND, "Received service descriptor for service ID %s; "
+             "expected descriptor for service ID %s.",
+             service_id, safe_str(rend_query->onion_address));
+    retval = -2;
+    goto err;
+  }
   /* Decode/decrypt introduction points. */
   if (intro_content) {
     if (rend_query->auth_type != REND_NO_AUTH &&

src/or/routerlist.c

@@ -2008,8 +2008,8 @@ router_get_by_nickname(const char *nickname, int warn_if_unnamed)
       if (n_matches <= 1 || router->is_running)
         best_match = router;
     } else if (maybedigest &&
-               tor_memeq(digest, router->cache_info.identity_digest, DIGEST_LEN)
-               ) {
+               tor_memeq(digest, router->cache_info.identity_digest,
+                         DIGEST_LEN)) {
       if (router_hex_digest_matches(router, nickname))
         return router;
       /* If we reach this point, we have a ID=name syntax that matches the
@@ -4665,8 +4665,9 @@ routerinfo_incompatible_with_extrainfo(routerinfo_t *ri, extrainfo_t *ei,
 
   /* 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, ei->cache_info.identity_digest,
-             DIGEST_LEN)) {
+  if (tor_memneq(ri->cache_info.identity_digest,
+                 ei->cache_info.identity_digest,
+                 DIGEST_LEN)) {
     if (msg) *msg = "Extrainfo nickname or identity did not match routerinfo";
     goto err; /* different servers */
   }

src/or/routerparse.c

@@ -1326,6 +1326,11 @@ router_parse_entry_from_string(const char *s, const char *end,
     goto err;
 
   tok = find_by_keyword(tokens, K_ONION_KEY);
+  if (!crypto_pk_public_exponent_ok(tok->key)) {
+    log_warn(LD_DIR,
+             "Relay's onion key had invalid exponent.");
+    goto err;
+  }
   router->onion_pkey = tok->key;
   tok->key = NULL; /* Prevent free */
 
@@ -3971,10 +3976,22 @@ rend_parse_introduction_points(rend_service_descriptor_t *parsed,
     }
     /* Parse onion key. */
     tok = find_by_keyword(tokens, R_IPO_ONION_KEY);
+    if (!crypto_pk_public_exponent_ok(tok->key)) {
+      log_warn(LD_REND,
+               "Introduction point's onion key had invalid exponent.");
+      rend_intro_point_free(intro);
+      goto err;
+    }
     info->onion_key = tok->key;
     tok->key = NULL; /* Prevent free */
     /* Parse service key. */
     tok = find_by_keyword(tokens, R_IPO_SERVICE_KEY);
+    if (!crypto_pk_public_exponent_ok(tok->key)) {
+      log_warn(LD_REND,
+               "Introduction point key had invalid exponent.");
+      rend_intro_point_free(intro);
+      goto err;
+    }
     intro->intro_key = tok->key;
     tok->key = NULL; /* Prevent free */
     /* Add extend info to list of introduction points. */