tor-commits May 2018

tor-commits@lists.torproject.org

17 participants
1514 discussions

[tor/release-0.3.3] Add a changes file for 25691/25692
by nickm＠torproject.org 27 May '18

27 May '18

commit 0f3d73a2d0ef22d51183768308887d0271ef20f0 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Apr 16 11:47:59 2018 -0400 Add a changes file for 25691/25692 --- changes/bug25691_again | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/changes/bug25691_again b/changes/bug25691_again new file mode 100644 index 000000000..3d0d91bfd --- /dev/null +++ b/changes/bug25691_again @@ -0,0 +1,6 @@ + o Minor bugfixes (path selection): + - Only select relays when they have the descriptors we prefer to + use for them. This change fixes a bug where we could select + a relay because it had _some_ descriptor, but reject it later with + a nonfatal assertion error because it didn't have the exact one we + wanted. Fixes bugs 25691 and 25692; bugfix on 0.3.3.4-alpha.

1 0

[tor/release-0.3.3] Rename node_has_descriptor() to node_has_any_descriptor()
by nickm＠torproject.org 27 May '18

27 May '18

commit 47163780c3682d881ad6e8044320a57fce439ea1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Apr 16 11:51:02 2018 -0400 Rename node_has_descriptor() to node_has_any_descriptor() Changing the name of this function should help keep us from misusing it when node_has_preferred_descriptor() would be more appropriate. --- src/or/circuitbuild.c | 2 +- src/or/control.c | 2 +- src/or/nodelist.c | 9 ++++++--- src/or/nodelist.h | 2 +- src/or/routerlist.c | 4 ++-- src/test/test_hs_common.c | 3 ++- 6 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 96cc8369d..06aff0620 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2468,7 +2468,7 @@ count_acceptable_nodes, (smartlist_t *nodes)) if (! node->is_valid) // log_debug(LD_CIRC,"Nope, the directory says %d is not valid.",i); continue; - if (! node_has_descriptor(node)) + if (! node_has_any_descriptor(node)) continue; /* The node has a descriptor, so we can just check the ntor key directly */ if (!node_has_curve25519_onion_key(node)) diff --git a/src/or/control.c b/src/or/control.c index fa62e9dbd..f87a0ca48 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -3498,7 +3498,7 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len, connection_printf_to_buf(conn, "552 No such router \"%s\"\r\n", n); goto done; } - if (!node_has_descriptor(node)) { + if (!node_has_any_descriptor(node)) { connection_printf_to_buf(conn, "552 No descriptor for \"%s\"\r\n", n); goto done; } diff --git a/src/or/nodelist.c b/src/or/nodelist.c index 2962d3cb4..212606d2f 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -1131,10 +1131,13 @@ node_is_dir(const node_t *node) } } -/** Return true iff node has either kind of usable descriptor -- that - * is, a routerdescriptor or a microdescriptor. */ +/** Return true iff node has either kind of descriptor -- that + * is, a routerdescriptor or a microdescriptor. + * + * You should probably use node_has_preferred_descriptor() instead. + **/ int -node_has_descriptor(const node_t *node) +node_has_any_descriptor(const node_t *node) { return (node->ri || (node->rs && node->md)); diff --git a/src/or/nodelist.h b/src/or/nodelist.h index 5e7254f60..00f12ca1e 100644 --- a/src/or/nodelist.h +++ b/src/or/nodelist.h @@ -46,7 +46,7 @@ void node_get_verbose_nickname(const node_t *node, void node_get_verbose_nickname_by_id(const char *id_digest, char *verbose_name_out); int node_is_dir(const node_t *node); -int node_has_descriptor(const node_t *node); +int node_has_any_descriptor(const node_t *node); int node_has_preferred_descriptor(const node_t *node, int for_direct_connect); int node_get_purpose(const node_t *node); diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 2ab5017b7..1bfbd9f67 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -2758,7 +2758,7 @@ frac_nodes_with_descriptors(const smartlist_t *sl, total <= 0.0) { int n_with_descs = 0; SMARTLIST_FOREACH(sl, const node_t *, node, { - if (node_has_descriptor(node)) + if (node_has_any_descriptor(node)) n_with_descs++; }); return ((double)n_with_descs) / (double)smartlist_len(sl); @@ -2766,7 +2766,7 @@ frac_nodes_with_descriptors(const smartlist_t *sl, present = 0.0; SMARTLIST_FOREACH_BEGIN(sl, const node_t *, node) { - if (node_has_descriptor(node)) + if (node_has_any_descriptor(node)) present += bandwidths[node_sl_idx]; } SMARTLIST_FOREACH_END(node); diff --git a/src/test/test_hs_common.c b/src/test/test_hs_common.c index 8c273c963..17ba11ca7 100644 --- a/src/test/test_hs_common.c +++ b/src/test/test_hs_common.c @@ -305,7 +305,8 @@ helper_add_hsdir_to_networkstatus(networkstatus_t *ns, node_t *node = node_get_mutable_by_id(ri->cache_info.identity_digest); tt_assert(node); node->rs = rs; - /* We need this to exist for node_has_descriptor() to return true. */ + /* We need this to exist for node_has_preferred_descriptor() to return + * true. */ node->md = tor_malloc_zero(sizeof(microdesc_t)); /* Do this now the nodelist_set_routerinfo() function needs a "rs" to set * the indexes which it doesn't have when it is called. */

1 0

[tor/release-0.3.3] Repair the legacy_hs/pick_tor2web_rendezvous_node unit test
by nickm＠torproject.org 27 May '18

27 May '18

commit f1c1328f8518ad46f09d36ef3545a7ac266658eb Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Apr 16 11:52:57 2018 -0400 Repair the legacy_hs/pick_tor2web_rendezvous_node unit test It tried to pick nodes for which only routerinfo_t items are set, but without setting UseMicroDescriptors to 0. This won't work any more, now that we're strict about using the right descriptor types due to 25691/25692/25213. --- src/test/test_hs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/test_hs.c b/src/test/test_hs.c index 9189bb65b..64448de51 100644 --- a/src/test/test_hs.c +++ b/src/test/test_hs.c @@ -361,6 +361,7 @@ test_pick_tor2web_rendezvous_node(void *arg) /* Parse Tor2webRendezvousPoints as a routerset. */ options->Tor2webRendezvousPoints = routerset_new(); + options->UseMicrodescriptors = 0; retval = routerset_parse(options->Tor2webRendezvousPoints, tor2web_rendezvous_str, "test_tor2web_rp");

1 0

[tor/release-0.3.3] control EXTENDCIRCUIT: check node_has_preferred_descriptor().
by nickm＠torproject.org 27 May '18

27 May '18

commit 0941c8bfe6f34024153ae3ed67e4b1a05f9a6bfe Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Apr 22 12:27:21 2018 -0400 control EXTENDCIRCUIT: check node_has_preferred_descriptor(). Suggested by teor during code review for 25691. --- src/or/control.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/or/control.c b/src/or/control.c index f87a0ca48..028339f49 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -3492,17 +3492,19 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len, smartlist_free(args); nodes = smartlist_new(); + int first_node = zero_circ; SMARTLIST_FOREACH_BEGIN(router_nicknames, const char *, n) { 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; } - if (!node_has_any_descriptor(node)) { + if (!node_has_preferred_descriptor(node, first_node)) { connection_printf_to_buf(conn, "552 No descriptor for \"%s\"\r\n", n); goto done; } smartlist_add(nodes, (void*)node); + first_node = 0; } SMARTLIST_FOREACH_END(n); if (!smartlist_len(nodes)) { connection_write_str_to_buf("512 No router names provided\r\n", conn); @@ -3515,14 +3517,15 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len, } /* now circ refers to something that is ready to be extended */ - int first_node = zero_circ; + first_node = zero_circ; SMARTLIST_FOREACH(nodes, const node_t *, node, { extend_info_t *info = extend_info_from_node(node, first_node); if (!info) { tor_assert_nonfatal(first_node); log_warn(LD_CONTROL, - "controller tried to connect to a node that doesn't have any " + "controller tried to connect to a node that lacks a suitable " + "descriptor, or which doesn't have any " "addresses that are allowed by the firewall configuration; " "circuit marked for closing."); circuit_mark_for_close(TO_CIRCUIT(circ), -END_CIRC_REASON_CONNECTFAILED);

1 0

[tor/release-0.3.3] Merge branch 'maint-0.3.3' into release-0.3.3
by nickm＠torproject.org 27 May '18

27 May '18

commit db9ca07c46bbdf5ad63d03c6b1401a6e5c83770e Merge: c8dad0492 fa1890e97 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun May 27 10:04:42 2018 -0400 Merge branch 'maint-0.3.3' into release-0.3.3 changes/bug25691_again | 6 +++++ src/or/circuitbuild.c | 67 ++++++++++++++++++++++++----------------------- src/or/circuituse.c | 4 +-- src/or/control.c | 9 ++++--- src/or/entrynodes.c | 7 ++--- src/or/hs_common.c | 15 ++++++++--- src/or/nodelist.c | 39 ++++++++++++++++++++++++--- src/or/nodelist.h | 4 ++- src/or/rendservice.c | 2 +- src/or/routerlist.c | 6 ++--- src/test/test_hs.c | 1 + src/test/test_hs_common.c | 3 ++- 12 files changed, 108 insertions(+), 55 deletions(-)

1 0

[tor/release-0.3.3] Merge remote-tracking branch 'public/bug25691_033_again_squashed' into maint-0.3.3
by nickm＠torproject.org 27 May '18

27 May '18

commit fa1890e97f19a444f8103c1207d7d18c02887b1f Merge: 0ef432d45 0941c8bfe Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun May 27 10:03:11 2018 -0400 Merge remote-tracking branch 'public/bug25691_033_again_squashed' into maint-0.3.3 changes/bug25691_again | 6 +++++ src/or/circuitbuild.c | 67 ++++++++++++++++++++++++----------------------- src/or/circuituse.c | 4 +-- src/or/control.c | 9 ++++--- src/or/entrynodes.c | 7 ++--- src/or/hs_common.c | 15 ++++++++--- src/or/nodelist.c | 39 ++++++++++++++++++++++++--- src/or/nodelist.h | 4 ++- src/or/rendservice.c | 2 +- src/or/routerlist.c | 6 ++--- src/test/test_hs.c | 1 + src/test/test_hs_common.c | 3 ++- 12 files changed, 108 insertions(+), 55 deletions(-)

1 0

[tor/maint-0.3.3] Use node_has_preferred_descriptor() in another case
by nickm＠torproject.org 27 May '18

27 May '18

commit 388d217c40fea4dfd9c2782ae6036126258fc292 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Apr 16 11:40:14 2018 -0400 Use node_has_preferred_descriptor() in another case In router_add_running_nodes_to_smartlist(), we had an inline implementation of the logic from node_has_descriptor(), which should be changed to node_has_preferred_descriptor(). --- src/or/routerlist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/routerlist.c b/src/or/routerlist.c index bc3abb236..2ab5017b7 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -2335,7 +2335,7 @@ router_add_running_nodes_to_smartlist(smartlist_t *sl, int need_uptime, SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), const node_t *, node) { if (!node->is_running || !node->is_valid) continue; - if (need_desc && !(node->ri || (node->rs && node->md))) + if (need_desc && !node_has_preferred_descriptor(node, direct_conn)) continue; if (node->ri && node->ri->purpose != ROUTER_PURPOSE_GENERAL) continue;

1 0

[tor/maint-0.3.3] control EXTENDCIRCUIT: check node_has_preferred_descriptor().
by nickm＠torproject.org 27 May '18

27 May '18

1 0

[tor/maint-0.3.3] Add a changes file for 25691/25692
by nickm＠torproject.org 27 May '18

27 May '18

1 0

[tor/maint-0.3.3] Check for "the right descriptor", not just "any descriptor".
by nickm＠torproject.org 27 May '18

27 May '18

commit 948dd2c79ea9ca1fd06c13f275515a1745c46986 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Apr 16 10:38:55 2018 -0400 Check for "the right descriptor", not just "any descriptor". This patch adds a new node_has_preferred_descriptor() function, and replaces most users of node_has_descriptor() with it. That's an important change, since as of d1874b433953f64 (our fix for #25213), we are willing to say that a node has _some_ descriptor, but not the _right_ descriptor for a particular use case. Part of a fix for 25691 and 25692. --- src/or/circuitbuild.c | 28 ++++++++++++---------------- src/or/circuituse.c | 4 ++-- src/or/entrynodes.c | 7 ++++--- src/or/hs_common.c | 15 +++++++++++---- src/or/nodelist.c | 30 +++++++++++++++++++++++++++++- src/or/nodelist.h | 2 ++ src/or/rendservice.c | 2 +- 7 files changed, 61 insertions(+), 27 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 5f1f8122f..019b9c51f 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -417,7 +417,7 @@ onion_populate_cpath(origin_circuit_t *circ) circ->cpath->extend_info->identity_digest); /* If we don't know the node and its descriptor, we must be bootstrapping. */ - if (!node || !node_has_descriptor(node)) { + if (!node || !node_has_preferred_descriptor(node, 1)) { return 0; } } @@ -1869,7 +1869,7 @@ choose_good_exit_server_general(int need_uptime, int need_capacity) */ continue; } - if (!node_has_descriptor(node)) { + if (!node_has_preferred_descriptor(node, 0)) { n_supported[i] = -1; continue; } @@ -2443,6 +2443,10 @@ circuit_extend_to_new_exit(origin_circuit_t *circ, extend_info_t *exit_ei) /** Return the number of routers in routers that are currently up * and available for building circuits through. + * + * (Note that this function may overcount or undercount, if we have + * descriptors that are not the type we would prefer to use for some + * particular router. See bug #25885.) */ MOCK_IMPL(STATIC int, count_acceptable_nodes, (smartlist_t *nodes)) @@ -2847,9 +2851,10 @@ extend_info_new(const char *nickname, * of the node (i.e. its IPv4 address) unless * for_direct_connect is true, in which case the preferred * address is used instead. May return NULL if there is not enough - * info about node to extend to it--for example, if there is no - * routerinfo_t or microdesc_t, or if for_direct_connect is true and none of - * the node's addresses are allowed by tor's firewall and IP version config. + * info about node to extend to it--for example, if the preferred + * routerinfo_t or microdesc_t is missing, or if for_direct_connect is + * true and none of the node's addresses is allowed by tor's firewall + * and IP version config. **/ extend_info_t * extend_info_from_node(const node_t *node, int for_direct_connect) @@ -2857,17 +2862,8 @@ extend_info_from_node(const node_t *node, int for_direct_connect) tor_addr_port_t ap; int valid_addr = 0; - const int is_bridge = node_is_a_configured_bridge(node); - const int we_use_mds = we_use_microdescriptors_for_circuits(get_options()); - - if ((is_bridge && for_direct_connect) || !we_use_mds) { - /* We need an ri in this case. */ - if (!node->ri) - return NULL; - } else { - /* Otherwise we need an md. */ - if (node->rs == NULL || node->md == NULL) - return NULL; + if (!node_has_preferred_descriptor(node, for_direct_connect)) { + return NULL; } /* Choose a preferred address first, but fall back to an allowed address. diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 3125fff65..5d8af4c6c 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -2384,7 +2384,7 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn, const node_t *r; int opt = conn->chosen_exit_optional; r = node_get_by_nickname(conn->chosen_exit_name, 0); - if (r && node_has_descriptor(r)) { + if (r && node_has_preferred_descriptor(r, conn->want_onehop ? 1 : 0)) { /* We might want to connect to an IPv6 bridge for loading descriptors so we use the preferred address rather than the primary. */ @@ -2394,7 +2394,7 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn, "Discarding this circuit.", conn->chosen_exit_name); return -1; } - } else { /* ! (r && node_has_descriptor(r)) */ + } else { /* ! (r && node_has_preferred_descriptor(...)) */ log_debug(LD_DIR, "considering %d, %s", want_onehop, conn->chosen_exit_name); if (want_onehop && conn->chosen_exit_name[0] == '$') { diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 2b6ff38c9..54638810f 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -185,14 +185,14 @@ should_apply_guardfraction(const networkstatus_t *ns) return options->UseGuardFraction; } -/** Return true iff we know a descriptor for guard */ +/** Return true iff we know a preferred descriptor for guard */ static int guard_has_descriptor(const entry_guard_t *guard) { const node_t *node = node_get_by_id(guard->identity); if (!node) return 0; - return node_has_descriptor(node); + return node_has_preferred_descriptor(node, 1); } /** @@ -2269,7 +2269,8 @@ entry_guard_pick_for_circuit(guard_selection_t *gs, // XXXX #20827 check Ed ID. if (! node) goto fail; - if (BUG(usage != GUARD_USAGE_DIRGUARD && !node_has_descriptor(node))) + if (BUG(usage != GUARD_USAGE_DIRGUARD && + !node_has_preferred_descriptor(node, 1))) goto fail; *chosen_node_out = node; diff --git a/src/or/hs_common.c b/src/or/hs_common.c index 6d97c8775..10b56c0ba 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -1280,8 +1280,10 @@ node_has_hsdir_index(const node_t *node) tor_assert(node_supports_v3_hsdir(node)); /* A node can't have an HSDir index without a descriptor since we need desc - * to get its ed25519 key */ - if (!node_has_descriptor(node)) { + * to get its ed25519 key. for_direct_connect should be zero, since we + * always use the consensus-indexed node's keys to build the hash ring, even + * if some of the consensus-indexed nodes are also bridges. */ + if (!node_has_preferred_descriptor(node, 0)) { return 0; } @@ -1612,12 +1614,17 @@ hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str) hs_clean_last_hid_serv_requests(now); /* Only select those hidden service directories to which we did not send a - * request recently and for which we have a router descriptor here. */ + * request recently and for which we have a router descriptor here. + * + * Use for_direct_connect==0 even if we will be connecting to the node + * directly, since we always use the key information in the + * consensus-indexed node descriptors for building the index. + **/ SMARTLIST_FOREACH_BEGIN(responsible_dirs, routerstatus_t *, dir) { time_t last = hs_lookup_last_hid_serv_request(dir, req_key_str, 0, 0); const node_t *node = node_get_by_id(dir->identity_digest); if (last + hs_hsdir_requery_period(options) >= now || - !node || !node_has_descriptor(node)) { + !node || !node_has_preferred_descriptor(node, 0)) { SMARTLIST_DEL_CURRENT(responsible_dirs, dir); continue; } diff --git a/src/or/nodelist.c b/src/or/nodelist.c index 3a26aee61..2962d3cb4 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -43,6 +43,7 @@ #include "or.h" #include "address.h" #include "address_set.h" +#include "bridges.h" #include "config.h" #include "control.h" #include "dirserv.h" @@ -1139,6 +1140,32 @@ node_has_descriptor(const node_t *node) (node->rs && node->md)); } +/** Return true iff node has the kind of descriptor we would prefer to + * use for it, given our configuration and how we intend to use the node. + * + * If for_direct_connect is true, we intend to connect to the node + * directly, as the first hop of a circuit; otherwise, we intend to connect to + * it indirectly, or use it as if we were connecting to it indirectly. */ +int +node_has_preferred_descriptor(const node_t *node, + int for_direct_connect) +{ + const int is_bridge = node_is_a_configured_bridge(node); + const int we_use_mds = we_use_microdescriptors_for_circuits(get_options()); + + if ((is_bridge && for_direct_connect) || !we_use_mds) { + /* We need an ri in this case. */ + if (!node->ri) + return 0; + } else { + /* Otherwise we need an rs and an md. */ + if (node->rs == NULL || node->md == NULL) + return 0; + } + + return 1; +} + /** Return the router_purpose of node. */ int node_get_purpose(const node_t *node) @@ -2221,7 +2248,8 @@ compute_frac_paths_available(const networkstatus_t *consensus, nu); SMARTLIST_FOREACH_BEGIN(myexits_unflagged, const node_t *, node) { - if (node_has_descriptor(node) && node_exit_policy_rejects_all(node)) { + if (node_has_preferred_descriptor(node, 0) && + node_exit_policy_rejects_all(node)) { SMARTLIST_DEL_CURRENT(myexits_unflagged, node); /* this node is not actually an exit */ np--; diff --git a/src/or/nodelist.h b/src/or/nodelist.h index 043d7b341..5e7254f60 100644 --- a/src/or/nodelist.h +++ b/src/or/nodelist.h @@ -47,6 +47,8 @@ void node_get_verbose_nickname_by_id(const char *id_digest, char *verbose_name_out); int node_is_dir(const node_t *node); int node_has_descriptor(const node_t *node); +int node_has_preferred_descriptor(const node_t *node, + int for_direct_connect); int node_get_purpose(const node_t *node); #define node_is_bridge(node) \ (node_get_purpose((node)) == ROUTER_PURPOSE_BRIDGE) diff --git a/src/or/rendservice.c b/src/or/rendservice.c index cc2242977..1a93c3643 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -3596,7 +3596,7 @@ directory_post_to_hs_dir(rend_service_descriptor_t *renddesc, /* Don't upload descriptor if we succeeded in doing so last time. */ continue; node = node_get_by_id(hs_dir->identity_digest); - if (!node || !node_has_descriptor(node)) { + if (!node || !node_has_preferred_descriptor(node,0)) { log_info(LD_REND, "Not launching upload for for v2 descriptor to " "hidden service directory %s; we don't have its " "router descriptor. Queuing for later upload.",

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

tor-commits May 2018