February 2011 - tor-commits - lists.torproject.org

[tor/maint-0.2.2] Issues with router_get_by_nickname()
by nickm＠torproject.org 22 Feb '11

22 Feb '11

commit e1d86d381714cb4e85f543fab0ca71f13e58767b Author: Robert Hogan <robert(a)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);

1 0

[tor/maint-0.2.2] Issues with router_get_by_nickname()
by nickm＠torproject.org 22 Feb '11

22 Feb '11

commit 7488fe5a224bf1c40d22769ff3986361d947d6f7 Author: Robert Hogan <robert(a)roberthogan.net> Date: Sun Oct 17 12:27:57 2010 +0100 Issues with router_get_by_nickname() https://trac.torproject.org/projects/tor/ticket/1859 Use router_get_by_digest() instead of router_get_by_hexdigest() in circuit_discard_optional_exit_enclaves() and rend_client_get_random_intro(), per Nick's comments. Using router_get_by_digest() in rend_client_get_random_intro() will break hidden services published by Tor versions pre 0.1.2.18 and 0.2.07-alpha as they only publish by nickname. This is acceptable however as these versions only publish to authority tor26 and don't work for versions in the 0.2.2.x series anyway. --- src/or/connection_edge.c | 2 +- src/or/rendclient.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index eba83ba..f72aa94 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_hexdigest(info->identity_digest); + r2 = router_get_by_digest(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 97fb6f0..ca3f213 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_hexdigest(intro->extend_info->identity_digest); + router = router_get_by_digest(intro->extend_info->identity_digest); if (!router) { log_info(LD_REND, "Unknown router with nickname '%s'; trying another.", intro->extend_info->nickname);

1 0

[tor/maint-0.2.2] Issues with router_get_by_nickname() (3)
by nickm＠torproject.org 22 Feb '11

22 Feb '11

commit 5cd7953a2112d6ae8702c741d73b8b6a2d0c5251 Author: Robert Hogan <robert(a)roberthogan.net> Date: Sun Oct 17 15:12:25 2010 +0100 Issues with router_get_by_nickname() (3) Add changes file --- changes/bug1859 | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/changes/bug1859 b/changes/bug1859 new file mode 100644 index 0000000..5b139f3 --- /dev/null +++ b/changes/bug1859 @@ -0,0 +1,9 @@ + o Minor bugfixes: + - Bring the logic that gathers routerinfos and assesses the + acceptability of circuits into line. This prevents a Tor OP from getting + locked in a cycle of choosing its local OR as an exit for a path (due to + a .exit request) and then rejecting the circuit because its OR is not + listed yet. Also prevent Tor clients from using an OR running in the same + instance as an exit (due to a .exit request) if the OR does not meet the + same requirements expected of an OR running elsewhere. + Fixes bug 1859; bugfix on 0.2.0-alpha.

1 0

[tor/maint-0.2.1] Merge remote branch 'public/bug1859_021' into maint-0.2.1
by nickm＠torproject.org 22 Feb '11

22 Feb '11

commit 491abbc65e535b5308d6c57ab9ab3900ac2dd2d2 Merge: 7605985 2392290 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 22 17:19:41 2011 -0500 Merge remote branch 'public/bug1859_021' into maint-0.2.1 changes/bug1859 | 9 +++++++++ src/or/connection_edge.c | 2 +- src/or/rendclient.c | 5 ++++- src/or/routerlist.c | 3 --- 4 files changed, 14 insertions(+), 5 deletions(-) diff --combined src/or/connection_edge.c index 052aa37,f72aa94..5609c1d --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@@ -1,7 -1,7 +1,7 @@@ /* Copyright (c) 2001 Matej Pfajfar. * Copyright (c) 2001-2004, Roger Dingledine. * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. - * Copyright (c) 2007-2010, The Tor Project, Inc. */ + * Copyright (c) 2007-2011, The Tor Project, Inc. */ /* See LICENSE for licensing information */ /** @@@ -571,7 -571,7 +571,7 @@@ circuit_discard_optional_exit_enclaves( !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_digest(info->identity_digest); if (!r1 || !r2 || r1 != r2) continue; tor_assert(edge_conn->socks_request); @@@ -1139,23 -1139,9 +1139,23 @@@ address_is_in_virtual_range(const char return 0; } +/** Increment the value of next_virtual_addr; reset it to the start of the + * virtual address range if it wraps around. + */ +static INLINE void +increment_virtual_addr(void) +{ + ++next_virtual_addr; + if (addr_mask_cmp_bits(next_virtual_addr, virtual_addr_network, + virtual_addr_netmask_bits)) + next_virtual_addr = virtual_addr_network; +} + /** Return a newly allocated string holding an address of type * (one of RESOLVED_TYPE_{IPV4|HOSTNAME}) that has not yet been mapped, * and that is very unlikely to be the address of any real host. + * + * May return NULL if we have run out of virtual addresses. */ static char * addressmap_get_virtual_address(int type) @@@ -1180,26 -1166,25 +1180,26 @@@ /* Don't hand out any .0 or .255 address. */ while ((next_virtual_addr & 0xff) == 0 || (next_virtual_addr & 0xff) == 0xff) { - ++next_virtual_addr; + increment_virtual_addr(); + if (! --available) { + log_warn(LD_CONFIG, "Ran out of virtual addresses!"); + return NULL; + } } in.s_addr = htonl(next_virtual_addr); tor_inet_ntoa(&in, buf, sizeof(buf)); if (!strmap_get(addressmap, buf)) { - ++next_virtual_addr; + increment_virtual_addr(); break; } - ++next_virtual_addr; + increment_virtual_addr(); --available; - log_info(LD_CONFIG, "%d addrs available", (int)available); - if (! --available) { + // log_info(LD_CONFIG, "%d addrs available", (int)available); + if (! available) { log_warn(LD_CONFIG, "Ran out of virtual addresses!"); return NULL; } - if (addr_mask_cmp_bits(next_virtual_addr, virtual_addr_network, - virtual_addr_netmask_bits)) - next_virtual_addr = virtual_addr_network; } return tor_strdup(buf); } else { @@@ -1214,15 -1199,14 +1214,15 @@@ * allocated string. If another address of the same type is already * mapped to new_address, try to return a copy of that address. * - * The string in new_address may be freed, or inserted into a map - * as appropriate. + * The string in new_address may be freed or inserted into a map + * as appropriate. May return NULL if are out of virtual addresses. **/ const char * addressmap_register_virtual_address(int type, char *new_address) { char **addrp; virtaddress_entry_t *vent; + int vent_needs_to_be_added = 0; tor_assert(new_address); tor_assert(addressmap); @@@ -1231,7 -1215,7 +1231,7 @@@ vent = strmap_get(virtaddress_reversemap, new_address); if (!vent) { vent = tor_malloc_zero(sizeof(virtaddress_entry_t)); - strmap_set(virtaddress_reversemap, new_address, vent); + vent_needs_to_be_added = 1; } addrp = (type == RESOLVED_TYPE_IPV4) ? @@@ -1241,7 -1225,6 +1241,7 @@@ if (ent && ent->new_address && !strcasecmp(new_address, ent->new_address)) { tor_free(new_address); + tor_assert(!vent_needs_to_be_added); return tor_strdup(*addrp); } else log_warn(LD_BUG, @@@ -1253,14 -1236,7 +1253,14 @@@ tor_free(*addrp); *addrp = addressmap_get_virtual_address(type); + if (!*addrp) { + tor_free(vent); + tor_free(new_address); + return NULL; + } log_info(LD_APP, "Registering map from %s to %s", *addrp, new_address); + if (vent_needs_to_be_added) + strmap_set(virtaddress_reversemap, new_address, vent); addressmap_register(*addrp, new_address, 2, ADDRMAPSRC_CONTROLLER); #if 0 @@@ -1439,12 -1415,7 +1439,12 @@@ connection_ap_handshake_rewrite_and_att const char *new_addr; new_addr = addressmap_register_virtual_address( RESOLVED_TYPE_IPV4, tor_strdup(socks->address)); - tor_assert(new_addr); + if (! new_addr) { + log_warn(LD_APP, "Unable to automap address %s", + escaped_safe_str(socks->address)); + connection_mark_unattached_ap(conn, END_STREAM_REASON_INTERNAL); + return -1; + } log_info(LD_APP, "Automapping %s to %s", escaped_safe_str(socks->address), safe_str(new_addr)); strlcpy(socks->address, new_addr, sizeof(socks->address)); @@@ -1459,8 -1430,7 +1459,8 @@@ tor_snprintf(socks->address, sizeof(socks->address), "REVERSE[%s]", orig_address); connection_ap_handshake_socks_resolved(conn, RESOLVED_TYPE_HOSTNAME, - strlen(result), result, -1, + strlen(result), (uint8_t*)result, + -1, map_expires); connection_mark_unattached_ap(conn, END_STREAM_REASON_DONE | @@@ -1592,8 -1562,7 +1592,8 @@@ /* remember _what_ is supposed to have been resolved. */ strlcpy(socks->address, orig_address, sizeof(socks->address)); connection_ap_handshake_socks_resolved(conn,RESOLVED_TYPE_IPV4,4, - (char*)&answer,-1,map_expires); + (uint8_t*)&answer, + -1,map_expires); connection_mark_unattached_ap(conn, END_STREAM_REASON_DONE | END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED); @@@ -2342,7 -2311,7 +2342,7 @@@ voi connection_ap_handshake_socks_resolved(edge_connection_t *conn, int answer_type, size_t answer_len, - const char *answer, + const uint8_t *answer, int ttl, time_t expires) { @@@ -2356,7 -2325,7 +2356,7 @@@ client_dns_set_addressmap(conn->socks_request->address, a, conn->chosen_exit_name, ttl); } else if (answer_type == RESOLVED_TYPE_HOSTNAME && answer_len < 256) { - char *cp = tor_strndup(answer, answer_len); + char *cp = tor_strndup((char*)answer, answer_len); client_dns_set_reverse_addressmap(conn->socks_request->address, cp, conn->chosen_exit_name, ttl); @@@ -2367,14 -2336,14 +2367,14 @@@ if (conn->is_dns_request) { if (conn->dns_server_request) { /* We had a request on our DNS port: answer it. */ - dnsserv_resolved(conn, answer_type, answer_len, answer, ttl); + dnsserv_resolved(conn, answer_type, answer_len, (char*)answer, ttl); conn->socks_request->has_finished = 1; return; } else { /* This must be a request from the controller. We already sent * a mapaddress if there's a ttl. */ tell_controller_about_resolved_result(conn, answer_type, answer_len, - answer, ttl, expires); + (char*)answer, ttl, expires); conn->socks_request->has_finished = 1; return; } @@@ -2518,8 -2487,6 +2518,8 @@@ connection_exit_begin_conn(cell_t *cell or_circ = TO_OR_CIRCUIT(circ); relay_header_unpack(&rh, cell->payload); + if (rh.length > RELAY_PAYLOAD_SIZE) + return -1; /* Note: we have to use relay_send_command_from_edge here, not * connection_edge_end or connection_edge_send_command, since those require @@@ -2543,8 -2510,7 +2543,8 @@@ END_STREAM_REASON_TORPROTOCOL, NULL); return 0; } - if (parse_addr_port(LOG_PROTOCOL_WARN, cell->payload+RELAY_HEADER_SIZE, + if (parse_addr_port(LOG_PROTOCOL_WARN, + (char*)(cell->payload+RELAY_HEADER_SIZE), &address,NULL,&port)<0) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Unable to parse addr:port in relay begin cell. Closing."); @@@ -2695,8 -2661,6 +2695,8 @@@ connection_exit_begin_resolve(cell_t *c assert_circuit_ok(TO_CIRCUIT(circ)); relay_header_unpack(&rh, cell->payload); + if (rh.length > RELAY_PAYLOAD_SIZE) + return -1; /* This 'dummy_conn' only exists to remember the stream ID * associated with the resolve request; and to make the @@@ -2707,9 -2671,8 +2707,9 @@@ */ dummy_conn = edge_connection_new(CONN_TYPE_EXIT, AF_INET); dummy_conn->stream_id = rh.stream_id; - dummy_conn->_base.address = tor_strndup(cell->payload+RELAY_HEADER_SIZE, - rh.length); + dummy_conn->_base.address = tor_strndup( + (char*)cell->payload+RELAY_HEADER_SIZE, + rh.length); dummy_conn->_base.port = 0; dummy_conn->_base.state = EXIT_CONN_STATE_RESOLVEFAILED; dummy_conn->_base.purpose = EXIT_PURPOSE_RESOLVE; diff --combined src/or/rendclient.c index 783a661,1f253c1..af91099 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@@ -1,5 -1,5 +1,5 @@@ /* Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. - * Copyright (c) 2007-2010, The Tor Project, Inc. */ + * Copyright (c) 2007-2011, The Tor Project, Inc. */ /* See LICENSE for licensing information */ /** @@@ -130,7 -130,7 +130,7 @@@ rend_client_send_introduction(origin_ci cpath = rendcirc->build_state->pending_final_cpath = tor_malloc_zero(sizeof(crypt_path_t)); cpath->magic = CRYPT_PATH_MAGIC; - if (!(cpath->dh_handshake_state = crypto_dh_new())) { + if (!(cpath->dh_handshake_state = crypto_dh_new(DH_TYPE_REND))) { log_warn(LD_BUG, "Internal error: couldn't allocate DH."); goto err; } @@@ -193,7 -193,6 +193,7 @@@ /*XXX maybe give crypto_pk_public_hybrid_encrypt a max_len arg, * to avoid buffer overflows? */ r = crypto_pk_public_hybrid_encrypt(intro_key, payload+DIGEST_LEN, + sizeof(payload)-DIGEST_LEN, tmp, (int)(dh_offset+DH_KEY_LEN), PK_PKCS1_OAEP_PADDING, 0); @@@ -244,7 -243,7 +244,7 @@@ rend_client_rendcirc_has_opened(origin_ */ int rend_client_introduction_acked(origin_circuit_t *circ, - const char *request, size_t request_len) + const uint8_t *request, size_t request_len) { origin_circuit_t *rendcirc; (void) request; // XXXX Use this. @@@ -629,7 -628,7 +629,7 @@@ rend_client_remove_intro_point(extend_i * the circuit to C_REND_READY. */ int -rend_client_rendezvous_acked(origin_circuit_t *circ, const char *request, +rend_client_rendezvous_acked(origin_circuit_t *circ, const uint8_t *request, size_t request_len) { (void) request; @@@ -655,7 -654,7 +655,7 @@@ /** Bob sent us a rendezvous cell; join the circuits. */ int -rend_client_receive_rendezvous(origin_circuit_t *circ, const char *request, +rend_client_receive_rendezvous(origin_circuit_t *circ, const uint8_t *request, size_t request_len) { crypt_path_t *hop; @@@ -683,8 -682,7 +683,8 @@@ tor_assert(circ->build_state->pending_final_cpath); hop = circ->build_state->pending_final_cpath; tor_assert(hop->dh_handshake_state); - if (crypto_dh_compute_secret(hop->dh_handshake_state, request, DH_KEY_LEN, + if (crypto_dh_compute_secret(hop->dh_handshake_state, (char*)request, + DH_KEY_LEN, keys, DIGEST_LEN+CPATH_KEY_MATERIAL_LEN)<0) { log_warn(LD_GENERAL, "Couldn't complete DH handshake."); goto err; @@@ -717,10 -715,8 +717,10 @@@ * attach only the connections that are waiting on this circuit, rather * than trying to attach them all. See comments bug 743. */ connection_ap_attach_pending(); + memset(keys, 0, sizeof(keys)); return 0; err: + memset(keys, 0, sizeof(keys)); circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_TORPROTOCOL); return -1; } @@@ -814,7 -810,10 +814,10 @@@ rend_client_get_random_intro(const rend 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); + if (tor_digest_is_zero(intro->extend_info->identity_digest)) + router = router_get_by_hexdigest(intro->extend_info->nickname); + else + router = router_get_by_digest(intro->extend_info->identity_digest); if (!router) { log_info(LD_REND, "Unknown router with nickname '%s'; trying another.", intro->extend_info->nickname); diff --combined src/or/routerlist.c index 7c8e36e,9622406..fb8fb88 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@@ -1,7 -1,7 +1,7 @@@ /* Copyright (c) 2001 Matej Pfajfar. * Copyright (c) 2001-2004, Roger Dingledine. * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. - * Copyright (c) 2007-2010, The Tor Project, Inc. */ + * Copyright (c) 2007-2011, The Tor Project, Inc. */ /* See LICENSE for licensing information */ /** @@@ -1989,9 -1989,6 +1989,6 @@@ router_get_by_nickname(const char *nick 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); @@@ -4676,8 -4673,7 +4673,8 @@@ routerinfo_incompatible_with_extrainfo( if (ei->pending_sig) { char signed_digest[128]; - if (crypto_pk_public_checksig(ri->identity_pkey, signed_digest, + if (crypto_pk_public_checksig(ri->identity_pkey, + signed_digest, sizeof(signed_digest), ei->pending_sig, ei->pending_sig_len) != DIGEST_LEN || memcmp(signed_digest, ei->cache_info.signed_descriptor_digest, DIGEST_LEN)) {

1 0

[tor/maint-0.2.1] Fix a remaining bug in Robert's bug1859 fix.
by nickm＠torproject.org 22 Feb '11

22 Feb '11

commit 2392290c1887b732fd2309e57ce04ab71b9b4e69 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Oct 21 11:08:15 2010 -0400 Fix a remaining bug in Robert's bug1859 fix. When intro->extend_info is created for an introduction point, it only starts out with a nickname, not necessarily an identity digest. Thus, doing router_get_by_digest isn't necessarily safe. --- src/or/rendclient.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/src/or/rendclient.c b/src/or/rendclient.c index ca3f213..1f253c1 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -810,7 +810,10 @@ 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_digest(intro->extend_info->identity_digest); + if (tor_digest_is_zero(intro->extend_info->identity_digest)) + router = router_get_by_hexdigest(intro->extend_info->nickname); + else + router = router_get_by_digest(intro->extend_info->identity_digest); if (!router) { log_info(LD_REND, "Unknown router with nickname '%s'; trying another.", intro->extend_info->nickname);

1 0

[tor/maint-0.2.1] Issues with router_get_by_nickname() (3)
by nickm＠torproject.org 22 Feb '11

22 Feb '11

commit 5cd7953a2112d6ae8702c741d73b8b6a2d0c5251 Author: Robert Hogan <robert(a)roberthogan.net> Date: Sun Oct 17 15:12:25 2010 +0100 Issues with router_get_by_nickname() (3) Add changes file --- changes/bug1859 | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/changes/bug1859 b/changes/bug1859 new file mode 100644 index 0000000..5b139f3 --- /dev/null +++ b/changes/bug1859 @@ -0,0 +1,9 @@ + o Minor bugfixes: + - Bring the logic that gathers routerinfos and assesses the + acceptability of circuits into line. This prevents a Tor OP from getting + locked in a cycle of choosing its local OR as an exit for a path (due to + a .exit request) and then rejecting the circuit because its OR is not + listed yet. Also prevent Tor clients from using an OR running in the same + instance as an exit (due to a .exit request) if the OR does not meet the + same requirements expected of an OR running elsewhere. + Fixes bug 1859; bugfix on 0.2.0-alpha.

1 0

[tor/maint-0.2.1] Issues with router_get_by_nickname()
by nickm＠torproject.org 22 Feb '11

22 Feb '11

commit 7488fe5a224bf1c40d22769ff3986361d947d6f7 Author: Robert Hogan <robert(a)roberthogan.net> Date: Sun Oct 17 12:27:57 2010 +0100 Issues with router_get_by_nickname() https://trac.torproject.org/projects/tor/ticket/1859 Use router_get_by_digest() instead of router_get_by_hexdigest() in circuit_discard_optional_exit_enclaves() and rend_client_get_random_intro(), per Nick's comments. Using router_get_by_digest() in rend_client_get_random_intro() will break hidden services published by Tor versions pre 0.1.2.18 and 0.2.07-alpha as they only publish by nickname. This is acceptable however as these versions only publish to authority tor26 and don't work for versions in the 0.2.2.x series anyway. --- src/or/connection_edge.c | 2 +- src/or/rendclient.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index eba83ba..f72aa94 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_hexdigest(info->identity_digest); + r2 = router_get_by_digest(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 97fb6f0..ca3f213 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_hexdigest(intro->extend_info->identity_digest); + router = router_get_by_digest(intro->extend_info->identity_digest); if (!router) { log_info(LD_REND, "Unknown router with nickname '%s'; trying another.", intro->extend_info->nickname);

1 0

[tor/maint-0.2.1] Issues with router_get_by_nickname()
by nickm＠torproject.org 22 Feb '11

22 Feb '11

commit e1d86d381714cb4e85f543fab0ca71f13e58767b Author: Robert Hogan <robert(a)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);

1 0

[torspec/master] Add proposal 178-param-voting.txt from Sebastian
by nickm＠torproject.org 22 Feb '11

22 Feb '11

commit f9ce33d250dc807f2126f325ed63e6c5893db80d Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 22 17:00:45 2011 -0500 Add proposal 178-param-voting.txt from Sebastian --- proposals/000-index.txt | 2 + proposals/178-param-voting.txt | 85 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 0 deletions(-) diff --git a/proposals/000-index.txt b/proposals/000-index.txt index 580ce36..ebeeb90 100644 --- a/proposals/000-index.txt +++ b/proposals/000-index.txt @@ -98,6 +98,7 @@ Proposals by number: 175 Automatically promoting Tor clients to nodes [DRAFT] 176 Proposed version-3 link handshake for Tor [DRAFT] 177 Abstaining from votes on individual flags [DRAFT] +178 Require majority of authorities to vote for consensus parameters [DRAFT] Proposals by status: @@ -113,6 +114,7 @@ Proposals by status: 175 Automatically promoting Tor clients to nodes 176 Proposed version-3 link handshake for Tor [for 0.2.3] 177 Abstaining from votes on individual flags + 178 Require majority of authorities to vote for consensus parameters NEEDS-REVISION: 131 Help users to verify they are using Tor OPEN: diff --git a/proposals/178-param-voting.txt b/proposals/178-param-voting.txt new file mode 100644 index 0000000..ff3d055 --- /dev/null +++ b/proposals/178-param-voting.txt @@ -0,0 +1,85 @@ +Filename: 178-param-voting.txt +Title: Require majority of authorities to vote for consensus parameters +Author: Sebastian Hahn +Created: 16-Feb-2011 +Status: Draft + +Overview: + +The consensus that the directory authorities create may contain one or +more parameters (32-bit signed integers) that influence the behavior +of Tor nodes (see proposal 167, "Vote on network parameters in +consensus" for more details). + +Currently (as of consensus method 11), a consensus will end up +containing a parameter if at least one directory authority votes for +that paramater. The value of the parameter will be the low-median of +all the votes for this parameter. + +This proposal aims at changing this voting process to be more secure +against tampering by a non-majority of directory authorities. + +Motivation: + +To prevent a minority of the directory authorities from influencing +the value of a parameter unduly, the majority of directory authorities +has to vote for that parameter. This is not currently happening, and +it was in fact not uncommon for a single authority to govern the value +of a consensus parameter. + +Design: + +When the consensus is generated, the directory authorities ensure that +a param is only included in the list of params if at least half of the +total number of authorities votes for that param. The value chosen is +the low-median of all the votes. We don't mandate that the authorities +have to vote on exactly the same value for it to be included because +some consensus parameters could be the result of active measurements +that individual authorities make. + +Security implications: + +This change is aimed at improving the security of Tor nodes against +attacks carried out by a minority of directory authorities. It is +possible that a consensus parameter that would be helpful to the +network is not included because not enough directory authorities +voted for it, but since clients are required to have sane defaults +in case the parameter is absent this does not carry a security risk. + +Specification: + +dir-spec section 3.4 currently says: + + Entries are given on the "params" line for every keyword on which any + authority voted. The values given are the low-median of all votes on + that keyword. + +It is proposed that the above is changed to: + + Entries are given on the "params" line for every keyword on which a + majority of authorities (total authorities, not just those + participating this vote) voted on. The values given are the + low-median of all votes on that keyword. XXX note previous behaviour. + +The following should be added to the bottom of section 3.4.: + + * If consensus method 12 or later is used, only consensus + parameters that more than half of the total number of + authorities voted for are included in the consensus. + +The following line should be added to the bottom of section 3.4.1.: + + "12" -- Params are only included if a majority voted for them + +Compatibility: + +A sufficient number of directory authorities must upgrade to the new +consensus method used to calculate the params in the way this proposal +calls for, otherwise the old mechanism is used. Nodes that do not act +as directory authorities do not need to be upgraded and should +experience no change in behaviour. + +Implementation: + +An example implementation of this feature can be found in +https://gitweb.torproject.org/sebastian/tor.git, branch safer_params.

1 0

[githax/master] Add Runa to authors.txt
by nickm＠torproject.org 22 Feb '11

22 Feb '11

commit 7caf8e2533909df4ed38084a55ebe2ea7157b381 Author: Sebastian Hahn <sebastian(a)torproject.org> Date: Tue Feb 22 22:13:12 2011 +0100 Add Runa to authors.txt --- authors.txt | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/authors.txt b/authors.txt index 7286321..49d0775 100644 --- a/authors.txt +++ b/authors.txt @@ -24,6 +24,7 @@ pde = Peter Eckersley <pde(a)eff.org> phobos = Andrew Lewman <andrew(a)torproject.org> pootle = translation bot <tor-translation(a)torproject.org> sah = Steven Hazel <sah(a)freehaven.net> +runa = Runa A. Sandvik <runa.sandvik(a)gmail.com> sebastian = Sebastian Hahn <sebastian(a)torproject.org> sjm217 = Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> syverson = Paul Syverson <syverson(a)itd.nrl.navy.mil>

1 0