Merge branch 'bug25008'

changes/bug25008

@@ -0,0 +1,9 @@
+  o Minor bugfixes (performance):
+    - Avoid calling protocol_list_supports_protocol() from inside tight loops
+      when running with cached routerinfo_t objects.  Instead,
+      summarize the relevant protocols as flags in the routerinfo_t, as we do
+      for routerstatus_t objects.  This change simplifies our code a little,
+      and saves a large amount of short-term memory allocation operations.
+      Fixes bug 25008; bugfix on 0.2.9.4-alpha.
+
+

changes/bug25105

@@ -0,0 +1,5 @@
+  o Minor bugfixes (v3 onion services):
+    - Look at the "HSRend" protocol version, not the "HSDir" protocol
+      version, when deciding whether a consensus entry can support
+      the v3 onion service protocol as a rendezvous point.
+      Fixes bug 25105; bugfix on 0.3.2.1-alpha.

src/or/nodelist.c

@@ -447,7 +447,7 @@ nodelist_set_routerinfo(routerinfo_t *ri, routerinfo_t **ri_old_out)
   /* Setting the HSDir index requires the ed25519 identity key which can
    * only be found either in the ri or md. This is why this is called here.
    * Only nodes supporting HSDir=2 protocol version needs this index. */
-  if (node->rs && node->rs->supports_v3_hsdir) {
+  if (node->rs && node->rs->pv.supports_v3_hsdir) {
     node_set_hsdir_index(node,
                          networkstatus_get_latest_consensus());
   }
@@ -487,7 +487,7 @@ nodelist_add_microdesc(microdesc_t *md)
     /* Setting the HSDir index requires the ed25519 identity key which can
      * only be found either in the ri or md. This is why this is called here.
      * Only nodes supporting HSDir=2 protocol version needs this index. */
-    if (rs->supports_v3_hsdir) {
+    if (rs->pv.supports_v3_hsdir) {
       node_set_hsdir_index(node, ns);
     }
     node_add_to_ed25519_map(node);
@@ -531,7 +531,7 @@ nodelist_set_consensus(networkstatus_t *ns)
       }
     }
 
-    if (rs->supports_v3_hsdir) {
+    if (rs->pv.supports_v3_hsdir) {
       node_set_hsdir_index(node, ns);
     }
     node_set_country(node);
@@ -959,6 +959,30 @@ node_ed25519_id_matches(const node_t *node, const ed25519_public_key_t *id)
   }
 }
 
+/** Dummy object that should be unreturnable.  Used to ensure that
+ * node_get_protover_summary_flags() always returns non-NULL. */
+static const protover_summary_flags_t zero_protover_flags = {
+  0,0,0,0,0,0,0
+};
+
+/** Return the protover_summary_flags for a given node. */
+static const protover_summary_flags_t *
+node_get_protover_summary_flags(const node_t *node)
+{
+  if (node->rs) {
+    return &node->rs->pv;
+  } else if (node->ri) {
+    return &node->ri->pv;
+  } else {
+    /* This should be impossible: every node should have a routerstatus or a
+     * router descriptor or both. But just in case we've messed up somehow,
+     * return a nice empty set of flags to indicate "this node supports
+     * nothing." */
+    tor_assert_nonfatal_unreached_once();
+    return &zero_protover_flags;
+  }
+}
+
 /** Return true iff <b>node</b> supports authenticating itself
  * by ed25519 ID during the link handshake.  If <b>compatible_with_us</b>,
  * it needs to be using a link authentication method that we understand.
@@ -969,23 +993,13 @@ node_supports_ed25519_link_authentication(const node_t *node,
 {
   if (! node_get_ed25519_id(node))
     return 0;
-  if (node->ri) {
-    const char *protos = node->ri->protocol_list;
-    if (protos == NULL)
-      return 0;
-    if (compatible_with_us)
-      return protocol_list_supports_protocol(protos, PRT_LINKAUTH, 3);
-    else
-      return protocol_list_supports_protocol_or_later(protos, PRT_LINKAUTH, 3);
-  }
-  if (node->rs) {
-    if (compatible_with_us)
-      return node->rs->supports_ed25519_link_handshake_compat;
-    else
-      return node->rs->supports_ed25519_link_handshake_any;
-  }
-  tor_assert_nonfatal_unreached_once();
-  return 0;
+
+  const protover_summary_flags_t *pv = node_get_protover_summary_flags(node);
+
+  if (compatible_with_us)
+    return pv->supports_ed25519_link_handshake_compat;
+  else
+    return pv->supports_ed25519_link_handshake_any;
 }
 
 /** Return true iff <b>node</b> supports the hidden service directory version
@@ -995,27 +1009,7 @@ node_supports_v3_hsdir(const node_t *node)
 {
   tor_assert(node);
 
-  if (node->rs) {
-    return node->rs->supports_v3_hsdir;
-  }
-  if (node->ri) {
-    if (node->ri->protocol_list == NULL) {
-      return 0;
-    }
-    /* Bug #22447 forces us to filter on tor version:
-     * If platform is a Tor version, and older than 0.3.0.8, return False.
-     * Else, obey the protocol list. */
-    if (node->ri->platform) {
-      if (!strcmpstart(node->ri->platform, "Tor ") &&
-          !tor_version_as_new_as(node->ri->platform, "0.3.0.8")) {
-        return 0;
-      }
-    }
-    return protocol_list_supports_protocol(node->ri->protocol_list,
-                                           PRT_HSDIR, PROTOVER_HSDIR_V3);
-  }
-  tor_assert_nonfatal_unreached_once();
-  return 0;
+  return node_get_protover_summary_flags(node)->supports_v3_hsdir;
 }
 
 /** Return true iff <b>node</b> supports ed25519 authentication as an hidden
@@ -1025,18 +1019,7 @@ node_supports_ed25519_hs_intro(const node_t *node)
 {
   tor_assert(node);
 
-  if (node->rs) {
-    return node->rs->supports_ed25519_hs_intro;
-  }
-  if (node->ri) {
-    if (node->ri->protocol_list == NULL) {
-      return 0;
-    }
-    return protocol_list_supports_protocol(node->ri->protocol_list,
-                                           PRT_HSINTRO, PROTOVER_HS_INTRO_V3);
-  }
-  tor_assert_nonfatal_unreached_once();
-  return 0;
+  return node_get_protover_summary_flags(node)->supports_ed25519_hs_intro;
 }
 
 /** Return true iff <b>node</b> supports to be a rendezvous point for hidden
@@ -1046,19 +1029,7 @@ node_supports_v3_rendezvous_point(const node_t *node)
 {
   tor_assert(node);
 
-  if (node->rs) {
-    return node->rs->supports_v3_rendezvous_point;
-  }
-  if (node->ri) {
-    if (node->ri->protocol_list == NULL) {
-      return 0;
-    }
-    return protocol_list_supports_protocol(node->ri->protocol_list,
-                                           PRT_HSREND,
-                                           PROTOVER_HS_RENDEZVOUS_POINT_V3);
-  }
-  tor_assert_nonfatal_unreached_once();
-  return 0;
+  return node_get_protover_summary_flags(node)->supports_v3_rendezvous_point;
 }
 
 /** Return the RSA ID key's SHA1 digest for the provided node. */

src/or/or.h

@@ -2200,6 +2200,43 @@ typedef struct signed_descriptor_t {
 /** A signed integer representing a country code. */
 typedef int16_t country_t;
 
+/** Flags used to summarize the declared protocol versions of a relay,
+ * so we don't need to parse them again and again. */
+typedef struct protover_summary_flags_t {
+  /** True iff we have a proto line for this router, or a versions line
+   * from which we could infer the protocols. */
+  unsigned int protocols_known:1;
+
+  /** True iff this router has a version or protocol list that allows it to
+   * accept EXTEND2 cells. This requires Relay=2. */
+  unsigned int supports_extend2_cells:1;
+
+  /** True iff this router has a protocol list that allows it to negotiate
+   * ed25519 identity keys on a link handshake with us. This
+   * requires LinkAuth=3. */
+  unsigned int supports_ed25519_link_handshake_compat:1;
+
+  /** True iff this router has a protocol list that allows it to negotiate
+   * ed25519 identity keys on a link handshake, at all. This requires some
+   * LinkAuth=X for X >= 3. */
+  unsigned int supports_ed25519_link_handshake_any:1;
+
+  /** True iff this router has a protocol list that allows it to be an
+   * introduction point supporting ed25519 authentication key which is part of
+   * the v3 protocol detailed in proposal 224. This requires HSIntro=4. */
+  unsigned int supports_ed25519_hs_intro : 1;
+
+  /** True iff this router has a protocol list that allows it to be an hidden
+   * service directory supporting version 3 as seen in proposal 224. This
+   * requires HSDir=2. */
+  unsigned int supports_v3_hsdir : 1;
+
+  /** True iff this router has a protocol list that allows it to be an hidden
+   * service rendezvous point supporting version 3 as seen in proposal 224.
+   * This requires HSRend=2. */
+  unsigned int supports_v3_rendezvous_point: 1;
+} protover_summary_flags_t;
+
 /** Information about another onion router in the network. */
 typedef struct {
   signed_descriptor_t cache_info;
@@ -2268,6 +2305,9 @@ typedef struct {
    * this routerinfo. Used only during voting. */
   unsigned int omit_from_vote:1;
 
+  /** Flags to summarize the protocol versions for this routerinfo_t. */
+  protover_summary_flags_t pv;
+
 /** Tor can use this router for general positions in circuits; we got it
  * from a directory server as usual, or we're an authority and a server
  * uploaded it. */
@@ -2346,42 +2386,15 @@ typedef struct routerstatus_t {
   unsigned int is_v2_dir:1; /** True iff this router publishes an open DirPort
                              * or it claims to accept tunnelled dir requests.
                              */
-  /** True iff we have a proto line for this router, or a versions line
-   * from which we could infer the protocols. */
-  unsigned int protocols_known:1;
-
-  /** True iff this router has a version or protocol list that allows it to
-   * accept EXTEND2 cells */
-  unsigned int supports_extend2_cells:1;
-
-  /** True iff this router has a protocol list that allows it to negotiate
-   * ed25519 identity keys on a link handshake with us. */
-  unsigned int supports_ed25519_link_handshake_compat:1;
-
-  /** True iff this router has a protocol list that allows it to negotiate
-   * ed25519 identity keys on a link handshake, at all. */
-  unsigned int supports_ed25519_link_handshake_any:1;
-
-  /** True iff this router has a protocol list that allows it to be an
-   * introduction point supporting ed25519 authentication key which is part of
-   * the v3 protocol detailed in proposal 224. This requires HSIntro=4. */
-  unsigned int supports_ed25519_hs_intro : 1;
-
-  /** True iff this router has a protocol list that allows it to be an hidden
-   * service directory supporting version 3 as seen in proposal 224. This
-   * requires HSDir=2. */
-  unsigned int supports_v3_hsdir : 1;
-
-  /** True iff this router has a protocol list that allows it to be an hidden
-   * service rendezvous point supporting version 3 as seen in proposal 224.
-   * This requires HSRend=2. */
-  unsigned int supports_v3_rendezvous_point: 1;
 
   unsigned int has_bandwidth:1; /**< The vote/consensus had bw info */
   unsigned int has_exitsummary:1; /**< The vote/consensus had exit summaries */
   unsigned int bw_is_unmeasured:1; /**< This is a consensus entry, with
                                     * the Unmeasured flag set. */
 
+  /** Flags to summarize the protocol versions for this routerstatus_t. */
+  protover_summary_flags_t pv;
+
   uint32_t bandwidth_kb; /**< Bandwidth (capacity) of the router as reported in
                        * the vote/consensus, in kilobytes/sec. */
 

src/or/routerlist.c

@@ -5665,11 +5665,11 @@ routerstatus_version_supports_extend2_cells(const routerstatus_t *rs,
     return allow_unknown_versions;
   }
 
-  if (!rs->protocols_known) {
+  if (!rs->pv.protocols_known) {
     return allow_unknown_versions;
   }
 
-  return rs->supports_extend2_cells;
+  return rs->pv.supports_extend2_cells;
 }
 
 /** Assert that the internal representation of <b>rl</b> is

src/or/routerparse.c

@@ -1895,12 +1895,19 @@ router_parse_entry_from_string(const char *s, const char *end,
     }
   }
 
-  if ((tok = find_opt_by_keyword(tokens, K_PLATFORM))) {
-    router->platform = tor_strdup(tok->args[0]);
-  }
+  {
+    const char *version = NULL, *protocols = NULL;
+    if ((tok = find_opt_by_keyword(tokens, K_PLATFORM))) {
+      router->platform = tor_strdup(tok->args[0]);
+      version = tok->args[0];
+    }
+
+    if ((tok = find_opt_by_keyword(tokens, K_PROTO))) {
+      router->protocol_list = tor_strdup(tok->args[0]);
+      protocols = tok->args[0];
+    }
 
-  if ((tok = find_opt_by_keyword(tokens, K_PROTO))) {
-    router->protocol_list = tor_strdup(tok->args[0]);
+    summarize_protover_flags(&router->pv, protocols, version);
   }
 
   if ((tok = find_opt_by_keyword(tokens, K_CONTACT))) {
@@ -2530,6 +2537,50 @@ routerstatus_parse_guardfraction(const char *guardfraction_str,
   return 0;
 }
 
+/** Summarize the protocols listed in <b>protocols</b> into <b>out</b>,
+ * falling back or correcting them based on <b>version</b> as appropriate.
+ */
+STATIC void
+summarize_protover_flags(protover_summary_flags_t *out,
+                         const char *protocols,
+                         const char *version)
+{
+  tor_assert(out);
+  memset(out, 0, sizeof(*out));
+  if (protocols) {
+    out->protocols_known = 1;
+    out->supports_extend2_cells =
+      protocol_list_supports_protocol(protocols, PRT_RELAY, 2);
+    out->supports_ed25519_link_handshake_compat =
+      protocol_list_supports_protocol(protocols, PRT_LINKAUTH, 3);
+    out->supports_ed25519_link_handshake_any =
+      protocol_list_supports_protocol_or_later(protocols, PRT_LINKAUTH, 3);
+    out->supports_ed25519_hs_intro =
+      protocol_list_supports_protocol(protocols, PRT_HSINTRO, 4);
+    out->supports_v3_hsdir =
+      protocol_list_supports_protocol(protocols, PRT_HSDIR,
+                                      PROTOVER_HSDIR_V3);
+    out->supports_v3_rendezvous_point =
+      protocol_list_supports_protocol(protocols, PRT_HSREND,
+                                      PROTOVER_HS_RENDEZVOUS_POINT_V3);
+  }
+  if (version && !strcmpstart(version, "Tor ")) {
+    if (!out->protocols_known) {
+      /* The version is a "Tor" version, and where there is no
+       * list of protocol versions that we should be looking at instead. */
+
+      out->supports_extend2_cells =
+        tor_version_as_new_as(version, "0.2.4.8-alpha");
+      out->protocols_known = 1;
+    } else {
+      /* Bug #22447 forces us to filter on this version. */
+      if (!tor_version_as_new_as(version, "0.3.0.8")) {
+        out->supports_v3_hsdir = 0;
+      }
+    }
+  }
+}
+
 /** Given a string at *<b>s</b>, containing a routerstatus object, and an
  * empty smartlist at <b>tokens</b>, parse and return the first router status
  * object in the string, and advance *<b>s</b> to just after the end of the
@@ -2695,44 +2746,21 @@ routerstatus_parse_entry_from_string(memarea_t *area,
     if (consensus_method >= MIN_METHOD_FOR_EXCLUDING_INVALID_NODES)
       rs->is_valid = 1;
   }
-  int found_protocol_list = 0;
-  if ((tok = find_opt_by_keyword(tokens, K_PROTO))) {
-    found_protocol_list = 1;
-    rs->protocols_known = 1;
-    rs->supports_extend2_cells =
-      protocol_list_supports_protocol(tok->args[0], PRT_RELAY, 2);
-    rs->supports_ed25519_link_handshake_compat =
-      protocol_list_supports_protocol(tok->args[0], PRT_LINKAUTH, 3);
-    rs->supports_ed25519_link_handshake_any =
-      protocol_list_supports_protocol_or_later(tok->args[0], PRT_LINKAUTH, 3);
-    rs->supports_ed25519_hs_intro =
-      protocol_list_supports_protocol(tok->args[0], PRT_HSINTRO, 4);
-    rs->supports_v3_hsdir =
-      protocol_list_supports_protocol(tok->args[0], PRT_HSDIR,
-                                      PROTOVER_HSDIR_V3);
-    rs->supports_v3_rendezvous_point =
-      protocol_list_supports_protocol(tok->args[0], PRT_HSDIR,
-                                      PROTOVER_HS_RENDEZVOUS_POINT_V3);
-  }
-  if ((tok = find_opt_by_keyword(tokens, K_V))) {
-    tor_assert(tok->n_args == 1);
-    if (!strcmpstart(tok->args[0], "Tor ") && !found_protocol_list) {
-      /* We only do version checks like this in the case where
-       * the version is a "Tor" version, and where there is no
-       * list of protocol versions that we should be looking at instead. */
-      rs->supports_extend2_cells =
-        tor_version_as_new_as(tok->args[0], "0.2.4.8-alpha");
-      rs->protocols_known = 1;
-    }
-    if (!strcmpstart(tok->args[0], "Tor ") && found_protocol_list) {
-      /* Bug #22447 forces us to filter on this version. */
-      if (!tor_version_as_new_as(tok->args[0], "0.3.0.8")) {
-        rs->supports_v3_hsdir = 0;
+  {
+    const char *protocols = NULL, *version = NULL;
+    if ((tok = find_opt_by_keyword(tokens, K_PROTO))) {
+      tor_assert(tok->n_args == 1);
+      protocols = tok->args[0];
+    }
+    if ((tok = find_opt_by_keyword(tokens, K_V))) {
+      tor_assert(tok->n_args == 1);
+      version = tok->args[0];
+      if (vote_rs) {
+        vote_rs->version = tor_strdup(tok->args[0]);
       }
     }
-    if (vote_rs) {
-      vote_rs->version = tor_strdup(tok->args[0]);
-    }
+
+    summarize_protover_flags(&rs->pv, protocols, version);
   }
 
   /* handle weighting/bandwidth info */

src/or/routerparse.h

@@ -133,6 +133,10 @@ MOCK_DECL(STATIC int, router_compute_hash_final,(char *digest,
                            digest_algorithm_t alg));
 MOCK_DECL(STATIC int, signed_digest_equals,
           (const uint8_t *d1, const uint8_t *d2, size_t len));
+
+STATIC void summarize_protover_flags(protover_summary_flags_t *out,
+                                     const char *protocols,
+                                     const char *version);
 #endif /* defined(ROUTERPARSE_PRIVATE) */
 
 #define ED_DESC_SIGNATURE_PREFIX "Tor router descriptor signature v1"

src/test/test_hs_common.c

@@ -289,7 +289,7 @@ helper_add_hsdir_to_networkstatus(networkstatus_t *ns,
 
   memcpy(rs->identity_digest, identity, DIGEST_LEN);
   rs->is_hs_dir = is_hsdir;
-  rs->supports_v3_hsdir = 1;
+  rs->pv.supports_v3_hsdir = 1;
   strlcpy(rs->nickname, nickname, sizeof(rs->nickname));
   tor_addr_parse(&ipv4_addr, "1.2.3.4");
   ri->addr = tor_addr_to_ipv4h(&ipv4_addr);

src/test/test_hs_service.c

@@ -20,6 +20,7 @@
 #define STATEFILE_PRIVATE
 #define TOR_CHANNEL_INTERNAL_
 #define HS_CLIENT_PRIVATE
+#define ROUTERPARSE_PRIVATE
 
 #include "test.h"
 #include "test_helpers.h"
@@ -37,6 +38,7 @@
 #include "networkstatus.h"
 #include "nodelist.h"
 #include "relay.h"
+#include "routerparse.h"
 
 #include "hs_common.h"
 #include "hs_config.h"
@@ -1210,6 +1212,7 @@ test_build_update_descriptors(void *arg)
     /* Ugly yes but we never free the "ri" object so this just makes things
      * easier. */
     ri.protocol_list = (char *) "HSDir=1-2 LinkAuth=3";
+    summarize_protover_flags(&ri.pv, ri.protocol_list, NULL);
     ret = curve25519_secret_key_generate(&curve25519_secret_key, 0);
     tt_int_op(ret, OP_EQ, 0);
     ri.onion_curve25519_pkey =