[tor-commits] [tor/master] Check for "the right descriptor", not just "any descriptor".
nickm at torproject.org
nickm at torproject.org
Sun Apr 22 23:44:35 UTC 2018
commit 948dd2c79ea9ca1fd06c13f275515a1745c46986
Author: Nick Mathewson <nickm at 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 <b>routers</b> 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
* <b>for_direct_connect</b> is true, in which case the preferred
* address is used instead. May return NULL if there is not enough
- * info about <b>node</b> 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 <b>node</b> 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 <b>guard</b> */
+/** Return true iff we know a preferred descriptor for <b>guard</b> */
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 <b>node</b> has the kind of descriptor we would prefer to
+ * use for it, given our configuration and how we intend to use the node.
+ *
+ * If <b>for_direct_connect</b> 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 <b>node</b>. */
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.",
More information about the tor-commits
mailing list