[tor-commits] [tor/release-0.2.1] Issues with router_get_by_nickname()

[arma at torproject.org]

commit e1d86d381714cb4e85f543fab0ca71f13e58767b
Author: Robert Hogan <robert at roberthogan.net>
Date:   Thu Sep 30 21:41:20 2010 +0100

    Issues with router_get_by_nickname()
    
    https://trac.torproject.org/projects/tor/ticket/1859
    
    There are two problems in this bug:
    
    1. When an OP makes a .exit request specifying itself as the exit, and the exit
       is not yet listed, Tor gets all the routerinfos needed for the circuit but
       discovers in circuit_is_acceptable() that its own routerinfo is not in the
       routerdigest list and cannot be used. Tor then gets locked in a cycle of
       repeating these two steps. When gathering the routerinfos for a circuit,
       specifically when the exit has been chosen by .exit notation, Tor needs to
       apply the same rules it uses later on when deciding if it can build a
       circuit with those routerinfos.
    
    2. A different bug arises in the above situation when the Tor instance's
       routerinfo *is* listed in the routerlist, it shares its nickname with a
       number of other Tor nodes, and it does not have 'Named' rights to its
       nickname.
       So for example, if (i) there are five nodes named Bob in the network, (ii) I
       am running one of them but am flagged as 'Unnamed' because someone else
       claimed the 'Bob' nickname first, and (iii) I run my Tor as both client
       and exit the following can happen to me:
         - I go to www.evil.com
         - I click on a link www.evil.com.bob.exit
         - My request will exit through my own Tor node rather than the 'Named'
           node Bob or any of the others.
         - www.evil.com now knows I am actually browsing from the same computer
           that is running my 'Bob' node
    
    So to solve both issues we need to ensure:
    
    - When fulfilling a .exit request we only choose a routerinfo if it exists in
      the routerlist, even when that routerinfo is ours.
    - When getting a router by nickname we only return our own router information
      if it is not going to be used for building a circuit.
    
    We ensure this by removing the special treatment afforded our own router in
    router_get_by_nickname(). This means the function will only return the
    routerinfo of our own router if it is in the routerlist built from authority
    info and has a unique nickname or is bound to a non-unique nickname.
    
    There are some uses of router_get_by_nickname() where we are looking for the
    router by name because of a configuration directive, specifically local
    declaration of NodeFamilies and EntryNodes and other routers' declaration of
    MyFamily. In these cases it is not at first clear if we need to continue
    returning our own routerinfo even if our router is not listed and/or has a
    non-unique nickname with the Unnamed flag.
    
    The patch treats each of these cases as follows:
    
    Other Routers' Declaration of MyFamily
     This happens in routerlist_add_family(). If another router declares our router
     in its family and our router has the Unnamed flag or is not in the routerlist
     yet, should we take advantage of the fact that we know our own routerinfo to
     add us in anyway? This patch says 'no, treat our own router just like any
     other'. This is a safe choice because it ensures our client has the same view
     of the network as other clients. We also have no good way of knowing if our
     router is Named or not independently of the authorities, so we have to rely on
     them in this.
    
    Local declaration of NodeFamilies
     Again, we have no way of knowing if the declaration 'NodeFamilies
     Bob,Alice,Ringo' refers to our router Bob or the Named router Bob, so we have
    to defer to the authorities and treat our own router like any other.
    
    Local declaration of NodeFamilies
     Again, same as above. There's also no good reason we would want our client to
     choose it's own router as an entry guard if it does not meet the requirements
     expected of any other router on the network.
    
    In order to reduce the possibility of error, the patch also replaces two
    instances where we were using router_get_by_nickname() with calls to
    router_get_by_hexdigest() where the identity digest of the router
    is available.
---
 src/or/connection_edge.c |    2 +-
 src/or/rendclient.c      |    2 +-
 src/or/routerlist.c      |    3 ---
 3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index ded02e9..eba83ba 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -571,7 +571,7 @@ circuit_discard_optional_exit_enclaves(extend_info_t *info)
         !edge_conn->chosen_exit_retries)
       continue;
     r1 = router_get_by_nickname(edge_conn->chosen_exit_name, 0);
-    r2 = router_get_by_nickname(info->nickname, 0);
+    r2 = router_get_by_hexdigest(info->identity_digest);
     if (!r1 || !r2 || r1 != r2)
       continue;
     tor_assert(edge_conn->socks_request);
diff --git a/src/or/rendclient.c b/src/or/rendclient.c
index 7023b7f..97fb6f0 100644
--- a/src/or/rendclient.c
+++ b/src/or/rendclient.c
@@ -810,7 +810,7 @@ rend_client_get_random_intro(const rend_data_t *rend_query)
   intro = smartlist_get(entry->parsed->intro_nodes, i);
   /* Do we need to look up the router or is the extend info complete? */
   if (!intro->extend_info->onion_key) {
-    router = router_get_by_nickname(intro->extend_info->nickname, 0);
+    router = router_get_by_hexdigest(intro->extend_info->identity_digest);
     if (!router) {
       log_info(LD_REND, "Unknown router with nickname '%s'; trying another.",
                intro->extend_info->nickname);
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 2fc8073..9622406 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1989,9 +1989,6 @@ router_get_by_nickname(const char *nickname, int warn_if_unnamed)
     return router_get_by_hexdigest(nickname);
   if (!strcasecmp(nickname, UNNAMED_ROUTER_NICKNAME))
     return NULL;
-  if (server_mode(get_options()) &&
-      !strcasecmp(nickname, get_options()->Nickname))
-    return router_get_my_routerinfo();
 
   maybedigest = (strlen(nickname) >= HEX_DIGEST_LEN) &&
     (base16_decode(digest,DIGEST_LEN,nickname,HEX_DIGEST_LEN) == 0);