Commit 80d38873 authored by Nick Mathewson's avatar Nick Mathewson 🤹
Browse files

Refactor node lookup APIs to take flags 

Right now there's a single warn_if_unnamed flag for
router_get_consensus_status_by_nickname() and
node_get_by_nickname(), that is nearly always 1.  I've turned it
into an 'unsigned' bitfield, and inverted its sense.  I've added the
flags argument to node_get_by_hex_id() too, though it does nothing
there right now.

I've removed the router_get_consensus_status_by_nickname() function,
since it was only used in once place.

This patch changes the warning behavior of GETINFO ns/name/<name>,
since all other name lookups from the controller currently warn.

Later I'm going to add more flags, for ed25519 support.
parent d7a3e336

src/or/addressmap.c

+1 −1
Original line number Diff line number Diff line
@@ -214,7 +214,7 @@ addressmap_clear_excluded_trackexithosts(const or_options_t *options) 
      dot--;

    if (*dot == '.') dot++;

    nodename = tor_strndup(dot, len-5-(dot-target));;

    node = node_get_by_nickname(nodename, 0);

    node = node_get_by_nickname(nodename, NNF_NO_WARN_UNNAMED);

    tor_free(nodename);

    if (!node ||

        (allow_nodes && !routerset_contains_node(allow_nodes, node)) ||

src/or/circuituse.c

+3 −3
Original line number Diff line number Diff line
@@ -2119,7 +2119,7 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn, 
    } else {

      /* XXXX Duplicates checks in connection_ap_handshake_attach_circuit:

       * refactor into a single function. */

      const node_t *node = node_get_by_nickname(conn->chosen_exit_name, 1);

      const node_t *node = node_get_by_nickname(conn->chosen_exit_name, 0);

      int opt = conn->chosen_exit_optional;

      if (node && !connection_ap_can_use_exit(conn, node)) {

        log_fn(opt ? LOG_INFO : LOG_WARN, LD_APP,
@@ -2199,7 +2199,7 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn, 
      if (conn->chosen_exit_name) {

        const node_t *r;

        int opt = conn->chosen_exit_optional;

        r = node_get_by_nickname(conn->chosen_exit_name, 1);

        r = node_get_by_nickname(conn->chosen_exit_name, 0);

        if (r && node_has_descriptor(r)) {

          /* We might want to connect to an IPv6 bridge for loading

             descriptors so we use the preferred address rather than
@@ -2598,7 +2598,7 @@ connection_ap_handshake_attach_circuit(entry_connection_t *conn) 
     * open to that exit. See what exit we meant, and whether we can use it.

     */

    if (conn->chosen_exit_name) {

      const node_t *node = node_get_by_nickname(conn->chosen_exit_name, 1);

      const node_t *node = node_get_by_nickname(conn->chosen_exit_name, 0);

      int opt = conn->chosen_exit_optional;

      if (!node && !want_onehop) {

        /* We ran into this warning when trying to extend a circuit to a

src/or/connection_edge.c

+5 −4
Original line number Diff line number Diff line
@@ -1074,7 +1074,8 @@ circuit_discard_optional_exit_enclaves(extend_info_t *info) 
    if (!entry_conn->chosen_exit_optional &&

        !entry_conn->chosen_exit_retries)

      continue;

    r1 = node_get_by_nickname(entry_conn->chosen_exit_name, 0);

    r1 = node_get_by_nickname(entry_conn->chosen_exit_name,

                              NNF_NO_WARN_UNNAMED);

    r2 = node_get_by_id(info->identity_digest);

    if (!r1 || !r2 || r1 != r2)

      continue;
@@ -1508,7 +1509,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, 
      if (s[1] != '\0') {

        /* Looks like a real .exit one. */

        conn->chosen_exit_name = tor_strdup(s+1);

        node = node_get_by_nickname(conn->chosen_exit_name, 1);

        node = node_get_by_nickname(conn->chosen_exit_name, 0);



        if (exit_source == ADDRMAPSRC_TRACKEXIT) {

          /* We 5 tries before it expires the addressmap */
@@ -1529,7 +1530,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, 
       * form that means (foo's address).foo.exit. */



      conn->chosen_exit_name = tor_strdup(socks->address);

      node = node_get_by_nickname(conn->chosen_exit_name, 1);

      node = node_get_by_nickname(conn->chosen_exit_name, 0);

      if (node) {

        *socks->address = 0;

        node_get_address_string(node, socks->address, sizeof(socks->address));
@@ -3630,7 +3631,7 @@ connection_ap_can_use_exit(const entry_connection_t *conn, 
   */

  if (conn->chosen_exit_name) {

    const node_t *chosen_exit =

      node_get_by_nickname(conn->chosen_exit_name, 1);

      node_get_by_nickname(conn->chosen_exit_name, 0);

    if (!chosen_exit || tor_memneq(chosen_exit->identity,

                               exit_node->identity, DIGEST_LEN)) {

      /* doesn't match */

src/or/control.c

+8 −8
Original line number Diff line number Diff line
@@ -1886,7 +1886,7 @@ getinfo_helper_dir(control_connection_t *control_conn, 
  (void) control_conn;

  if (!strcmpstart(question, "desc/id/")) {

    const routerinfo_t *ri = NULL;

    const node_t *node = node_get_by_hex_id(question+strlen("desc/id/"));

    const node_t *node = node_get_by_hex_id(question+strlen("desc/id/"), 0);

    if (node)

      ri = node->ri;

    if (ri) {
@@ -1905,7 +1905,7 @@ getinfo_helper_dir(control_connection_t *control_conn, 
    /* XXX Setting 'warn_if_unnamed' here is a bit silly -- the

     * warning goes to the user, not to the controller. */

    const node_t *node =

      node_get_by_nickname(question+strlen("desc/name/"), 1);

      node_get_by_nickname(question+strlen("desc/name/"), 0);

    if (node)

      ri = node->ri;

    if (ri) {
@@ -1991,7 +1991,7 @@ getinfo_helper_dir(control_connection_t *control_conn, 
      return -1;

    }

  } else if (!strcmpstart(question, "md/id/")) {

    const node_t *node = node_get_by_hex_id(question+strlen("md/id/"));

    const node_t *node = node_get_by_hex_id(question+strlen("md/id/"), 0);

    const microdesc_t *md = NULL;

    if (node) md = node->md;

    if (md && md->body) {
@@ -2000,7 +2000,7 @@ getinfo_helper_dir(control_connection_t *control_conn, 
  } else if (!strcmpstart(question, "md/name/")) {

    /* XXX Setting 'warn_if_unnamed' here is a bit silly -- the

     * warning goes to the user, not to the controller. */

    const node_t *node = node_get_by_nickname(question+strlen("md/name/"), 1);

    const node_t *node = node_get_by_nickname(question+strlen("md/name/"), 0);

    /* XXXX duplicated code */

    const microdesc_t *md = NULL;

    if (node) md = node->md;
@@ -2013,7 +2013,7 @@ getinfo_helper_dir(control_connection_t *control_conn, 
  } else if (!strcmpstart(question, "desc-annotations/id/")) {

    const routerinfo_t *ri = NULL;

    const node_t *node =

      node_get_by_hex_id(question+strlen("desc-annotations/id/"));

      node_get_by_hex_id(question+strlen("desc-annotations/id/"), 0);

    if (node)

      ri = node->ri;

    if (ri) {
@@ -3394,7 +3394,7 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len, 


  nodes = smartlist_new();

  SMARTLIST_FOREACH_BEGIN(router_nicknames, const char *, n) {

    const node_t *node = node_get_by_nickname(n, 1);

    const node_t *node = node_get_by_nickname(n, 0);

    if (!node) {

      connection_printf_to_buf(conn, "552 No such router \"%s\"\r\n", n);

      goto done;
@@ -4158,7 +4158,7 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len, 
      const char *server;



      server = arg + strlen(opt_server);

      node = node_get_by_hex_id(server);

      node = node_get_by_hex_id(server, 0);

      if (!node) {

        connection_printf_to_buf(conn, "552 Server \"%s\" not found\r\n",

                                 server);
@@ -4239,7 +4239,7 @@ handle_control_hspost(control_connection_t *conn, 
    SMARTLIST_FOREACH_BEGIN(args, const char *, arg) {

      if (!strcasecmpstart(arg, opt_server)) {

        const char *server = arg + strlen(opt_server);

        const node_t *node = node_get_by_hex_id(server);

        const node_t *node = node_get_by_hex_id(server, 0);



        if (!node || !node->rs) {

          connection_printf_to_buf(conn, "552 Server \"%s\" not found\r\n",

src/or/networkstatus.c

+2 −16
Original line number Diff line number Diff line
@@ -794,21 +794,6 @@ router_get_consensus_status_by_id(const char *digest) 
  return router_get_mutable_consensus_status_by_id(digest);

}



/** Given a nickname (possibly verbose, possibly a hexadecimal digest), return

 * the corresponding routerstatus_t, or NULL if none exists.  Warn the

 * user if <b>warn_if_unnamed</b> is set, and they have specified a router by

 * nickname, but the Named flag isn't set for that router. */

const routerstatus_t *

router_get_consensus_status_by_nickname(const char *nickname,

                                        int warn_if_unnamed)

{

  const node_t *node = node_get_by_nickname(nickname, warn_if_unnamed);

  if (node)

    return node->rs;

  else

    return NULL;

}



/** Return the identity digest that's mapped to officially by

 * <b>nickname</b>. */

const char *
@@ -2555,7 +2540,8 @@ getinfo_helper_networkstatus(control_connection_t *conn, 
    }

    status = router_get_consensus_status_by_id(d);

  } else if (!strcmpstart(question, "ns/name/")) {

    status = router_get_consensus_status_by_nickname(question+8, 0);

    const node_t *n = node_get_by_nickname(question+8, 0);

    status = n ? n->rs : NULL;

  } else if (!strcmpstart(question, "ns/purpose/")) {

    *answer = networkstatus_getinfo_by_purpose(question+11, time(NULL));

    return *answer ? 0 : -1;