Clients avoid choosing nodes that can't do ntor

If we know a node's version, and it can't do ntor, consider it not running.
If we have a node's descriptor, and it doesn't have a valid ntor key,
consider it not running.

Refactor these checks so they're consistent between authorities and clients.

diff --git a/changes/reject-tap b/changes/reject-tap
index 77ca63b46e26067c62e00f6086c0707a546ccc17..75800184fd3c764d396f0bcddddfbd16fe88ba7f 100644 (file)
--- a/changes/reject-tap
+++ b/changes/reject-tap
@@ -7,4 +7,7 @@
       instead, they check specifically for an ntor key.
     - Clients avoid downloading a descriptor if the relay version is
       too old to support ntor.
+    - Client code ignores nodes without ntor keys: they will not be
+      selected during circuit-building, or as guards, or as directory
+      mirrors, or as introduction or rendezvous points.
       Fixes bug 19163; bugfix on 0.2.4.18-rc.

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 13cc16670ce7a3890e41f623d2c710efaf512a3a..d6c5c85f2ca18d458f5dbaf5a48cda7218c47b4d 100644 (file)
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -820,8 +820,8 @@ circuit_pick_create_handshake(uint8_t *cell_type_out,
 
 /** Decide whether to use a TAP or ntor handshake for connecting to <b>ei</b>
  * directly, and set *<b>handshake_type_out</b> accordingly. Decide whether,
- * in extending through <b>node</b> to do so, we should use an EXTEND2 or an
- * EXTEND cell to do so, and set *<b>cell_type_out</b> and
+ * in extending through <b>node_prev</b> to do so, we should use an EXTEND2 or
+ * an EXTEND cell to do so, and set *<b>cell_type_out</b> and
  * *<b>create_cell_type_out</b> accordingly. */
 static void
 circuit_pick_extend_handshake(uint8_t *cell_type_out,
@@ -837,7 +837,8 @@ circuit_pick_extend_handshake(uint8_t *cell_type_out,
   if (node_prev &&
       *handshake_type_out != ONION_HANDSHAKE_TYPE_TAP &&
       (node_has_curve25519_onion_key(node_prev) ||
-       (node_prev->rs && node_prev->rs->version_supports_extend2_cells))) {
+       (node_prev->rs &&
+        routerstatus_version_supports_ntor(node_prev->rs, 0)))) {
     *cell_type_out = RELAY_COMMAND_EXTEND2;
     *create_cell_type_out = CELL_CREATE2;
   } else {

diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index ef3a3058951ea466f7321c5f0dfad08de45a9728..ff50ca44176ab54e2abc56bd6226b753c2cf6e4d 100644 (file)
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -260,9 +260,7 @@ dirserv_router_get_status(const routerinfo_t *router, const char **msg,
    * But just in case a relay doesn't provide or lies about its version, or
    * doesn't include an ntor key in its descriptor, check that it exists,
    * and is non-zero (clients check that it's non-zero before using it). */
-  if (router->onion_curve25519_pkey == NULL ||
-      tor_mem_is_zero((const char*)router->onion_curve25519_pkey->public_key,
-                      CURVE25519_PUBKEY_LEN)) {
+  if (!routerinfo_has_curve25519_onion_key(router)) {
     log_fn(severity, LD_DIR,
            "Descriptor from router %s is missing an ntor curve25519 onion "
            "key.", router_describe(router));

diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 61753e5da19494a5e76fa1352f99e9ce1f87567e..1223788df7d62a4c8d14feadecb9fee1d9ea9c62 100644 (file)
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -2272,8 +2272,10 @@ client_would_use_router(const routerstatus_t *rs, time_t now,
     /* We'd drop it immediately for being too old. */
     return 0;
   }
-  if (rs->version_known && !rs->version_supports_extend2_cells) {
-    /* We'd ignore it because it doesn't support ntor. */
+  if (!routerstatus_version_supports_ntor(rs, 1)) {
+    /* We'd ignore it because it doesn't support ntor.
+     * If we don't know the version, download the descriptor so we can
+     * check if it supports ntor. */
     return 0;
   }
   return 1;

diff --git a/src/or/nodelist.c b/src/or/nodelist.c
index a49bf03f61d6d9588e0f7b530fcbcafa4b3860b5..a888ebefbda0d7974f34528986c6f62b93727638 100644 (file)
--- a/src/or/nodelist.c
+++ b/src/or/nodelist.c
@@ -1171,14 +1171,35 @@ node_get_pref_ipv6_dirport(const node_t *node, tor_addr_port_t *ap_out)
   }
 }
 
+/** Return true iff <b>md</b> has a curve25519 onion key.
+ * Use node_has_curve25519_onion_key() instead of calling this directly. */
+static int
+microdesc_has_curve25519_onion_key(const microdesc_t *md)
+{
+  if (!md) {
+    return 0;
+  }
+
+  if (!md->onion_curve25519_pkey) {
+    return 0;
+  }
+
+  if (tor_mem_is_zero((const char*)md->onion_curve25519_pkey->public_key,
+                      CURVE25519_PUBKEY_LEN)) {
+    return 0;
+  }
+
+  return 1;
+}
+
 /** Return true iff <b>node</b> has a curve25519 onion key. */
 int
 node_has_curve25519_onion_key(const node_t *node)
 {
   if (node->ri)
-    return node->ri->onion_curve25519_pkey != NULL;
+    return routerinfo_has_curve25519_onion_key(node->ri);
   else if (node->md)
-    return node->md->onion_curve25519_pkey != NULL;
+    return microdesc_has_curve25519_onion_key(node->md);
   else
     return 0;
 }

diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 6ea9d8b0d17138a86609d26d1bba9055530c73c4..08015038fa7efe86e4d1b9a62bba15da322f3e1a 100644 (file)
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -2260,6 +2260,11 @@ router_add_running_nodes_to_smartlist(smartlist_t *sl, int allow_invalid,
       continue;
     if (node_is_unreliable(node, need_uptime, need_capacity, need_guard))
       continue;
+    /* Don't choose nodes if we are certain they can't do ntor */
+    if (node->rs && !routerstatus_version_supports_ntor(node->rs, 1))
+      continue;
+    if ((node->ri || node->md) && !node_has_curve25519_onion_key(node))
+      continue;
     /* Choose a node with an OR address that matches the firewall rules */
     if (check_reach && !fascist_firewall_allows_node(node,
                                                      FIREWALL_OR_CONNECTION,
@@ -5488,6 +5493,45 @@ routerinfo_incompatible_with_extrainfo(const crypto_pk_t *identity_pkey,
   return r;
 }
 
+/* Does ri have a valid ntor onion key?
+ * Valid ntor onion keys exist and have at least one non-zero byte. */
+int
+routerinfo_has_curve25519_onion_key(const routerinfo_t *ri)
+{
+  if (!ri) {
+    return 0;
+  }
+
+  if (!ri->onion_curve25519_pkey) {
+    return 0;
+  }
+
+  if (tor_mem_is_zero((const char*)ri->onion_curve25519_pkey->public_key,
+                      CURVE25519_PUBKEY_LEN)) {
+    return 0;
+  }
+
+  return 1;
+}
+
+/* Is rs running a tor version known to support ntor?
+ * If allow_unknown_versions is true, return true if the version is unknown.
+ * Otherwise, return false if the version is unknown. */
+int
+routerstatus_version_supports_ntor(const routerstatus_t *rs,
+                                   int allow_unknown_versions)
+{
+  if (!rs) {
+    return allow_unknown_versions;
+  }
+
+  if (!rs->version_known) {
+    return allow_unknown_versions;
+  }
+
+  return rs->version_supports_extend2_cells;
+}
+
 /** Assert that the internal representation of <b>rl</b> is
  * self-consistent. */
 void

diff --git a/src/or/routerlist.h b/src/or/routerlist.h
index 01f7644535272e9baa786702b387cc01177724ae..4041a38168d81b167a1f1ea1fd4bd00553dfbf19 100644 (file)
--- a/src/or/routerlist.h
+++ b/src/or/routerlist.h
@@ -205,6 +205,9 @@ int routerinfo_incompatible_with_extrainfo(const crypto_pk_t *ri,
                                            extrainfo_t *ei,
                                            signed_descriptor_t *sd,
                                            const char **msg);
+int routerinfo_has_curve25519_onion_key(const routerinfo_t *ri);
+int routerstatus_version_supports_ntor(const routerstatus_t *rs,
+                                       int allow_unknown_versions);
 
 void routerlist_assert_ok(const routerlist_t *rl);
 const char *esc_router_info(const routerinfo_t *router);