[tor-commits] [tor/master] Return NULL from extend_info_from_node if the node has no allowed address
nickm at torproject.org
nickm at torproject.org
Thu Feb 11 17:37:18 UTC 2016
commit 1401117ff2bc5fc90df51d19c3c0d7abc439c34e
Author: teor (Tim Wilson-Brown) <teor2345 at gmail.com>
Date: Fri Jan 22 17:43:24 2016 +1100
Return NULL from extend_info_from_node if the node has no allowed address
Modify callers to correctly handle these new NULL returns:
* fix assert in onion_extend_cpath
* warn and discard circuit in circuit_get_open_circ_or_launch
* warn, discard circuit, and tell controller in handle_control_extendcircuit
---
src/or/circuitbuild.c | 35 +++++++++++++++++++++++------------
src/or/circuituse.c | 7 ++++++-
src/or/control.c | 18 ++++++++++++++++--
3 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index dcb9de3..daf0b2a 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -2241,9 +2241,11 @@ onion_extend_cpath(origin_circuit_t *circ)
if (r) {
/* If we're a client, use the preferred address rather than the
primary address, for potentially connecting to an IPv6 OR
- port. */
- info = extend_info_from_node(r, server_mode(get_options()) == 0);
- tor_assert(info);
+ port. Servers always want the primary (IPv4) address. */
+ int client = (server_mode(get_options()) == 0);
+ info = extend_info_from_node(r, client);
+ /* Clients can fail to find an allowed address */
+ tor_assert(info || client);
}
} else {
const node_t *r =
@@ -2318,34 +2320,43 @@ extend_info_new(const char *nickname, const char *digest,
* <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.
+ * 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.
**/
extend_info_t *
extend_info_from_node(const node_t *node, int for_direct_connect)
{
tor_addr_port_t ap;
+ int valid_addr = 0;
if (node->ri == NULL && (node->rs == NULL || node->md == NULL))
return NULL;
- /* Choose a preferred address first, but fall back to an allowed address*/
+ /* Choose a preferred address first, but fall back to an allowed address.
+ * choose_address returns 1 on success, but get_prim_orport returns 0. */
if (for_direct_connect)
- fascist_firewall_choose_address_node(node, FIREWALL_OR_CONNECTION, 0, &ap);
+ valid_addr = fascist_firewall_choose_address_node(node,
+ FIREWALL_OR_CONNECTION,
+ 0, &ap);
else
- node_get_prim_orport(node, &ap);
+ valid_addr = !node_get_prim_orport(node, &ap);
- log_debug(LD_CIRC, "using %s for %s",
- fmt_addrport(&ap.addr, ap.port),
- node->ri ? node->ri->nickname : node->rs->nickname);
+ if (valid_addr)
+ log_debug(LD_CIRC, "using %s for %s",
+ fmt_addrport(&ap.addr, ap.port),
+ node->ri ? node->ri->nickname : node->rs->nickname);
+ else
+ log_warn(LD_CIRC, "Could not choose valid address for %s",
+ node->ri ? node->ri->nickname : node->rs->nickname);
- if (node->ri)
+ if (valid_addr && node->ri)
return extend_info_new(node->ri->nickname,
node->identity,
node->ri->onion_pkey,
node->ri->onion_curve25519_pkey,
&ap.addr,
ap.port);
- else if (node->rs && node->md)
+ else if (valid_addr && node->rs && node->md)
return extend_info_new(node->rs->nickname,
node->identity,
node->md->onion_pkey,
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index e742a56..4831f2b 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -2006,8 +2006,13 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn,
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
- the primary. */
+ the primary. */
extend_info = extend_info_from_node(r, conn->want_onehop ? 1 : 0);
+ if (!extend_info) {
+ log_warn(LD_CIRC,"Could not make a one-hop connection to %s. "
+ "Discarding this circuit.", conn->chosen_exit_name);
+ return -1;
+ }
} else {
log_debug(LD_DIR, "considering %d, %s",
want_onehop, conn->chosen_exit_name);
diff --git a/src/or/control.c b/src/or/control.c
index 66182fe..2c0209e 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -2864,12 +2864,26 @@ 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;
SMARTLIST_FOREACH(nodes, const node_t *, node,
{
- extend_info_t *info = extend_info_from_node(node, 0);
- tor_assert(info); /* True, since node_has_descriptor(node) == true */
+ extend_info_t *info = extend_info_from_node(node, first_node);
+ if (first_node && !info) {
+ log_warn(LD_CONTROL,
+ "controller tried to connect to a node that 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);
+ connection_write_str_to_buf("551 Couldn't start circuit\r\n", conn);
+ goto done;
+ } else {
+ /* True, since node_has_descriptor(node) == true and we are extending
+ * to the node's primary address */
+ tor_assert(info);
+ }
circuit_append_new_exit(circ, info);
extend_info_free(info);
+ first_node = 0;
});
/* now that we've populated the cpath, start extending */
More information about the tor-commits
mailing list