[tor-commits] [tor/master] relay/circuitbuild: Re-use IPv6 connections for circuits
nickm at torproject.org
nickm at torproject.org
Wed Apr 29 23:23:42 UTC 2020
commit 16f3f6a1afe5dcd75536039029f51392d05ce153
Author: teor <teor at torproject.org>
Date: Wed Apr 15 13:04:33 2020 +1000
relay/circuitbuild: Re-use IPv6 connections for circuits
Search for existing connections using the remote IPv4 and IPv6
addresses.
Part of 33817.
---
src/core/or/channel.c | 57 ++++++++++++++++++++++------------
src/core/or/channel.h | 3 +-
src/core/or/circuitbuild.c | 16 +++++++---
src/feature/relay/circuitbuild_relay.c | 13 +++++++-
src/lib/net/address.h | 4 +++
src/test/test_channel.c | 37 +++++++++++++++-------
src/test/test_circuitbuild.c | 6 ++--
7 files changed, 96 insertions(+), 40 deletions(-)
diff --git a/src/core/or/channel.c b/src/core/or/channel.c
index 89804826a..93245ce81 100644
--- a/src/core/or/channel.c
+++ b/src/core/or/channel.c
@@ -85,8 +85,10 @@
/* Static function prototypes */
-static int channel_matches_target_addr_for_extend(channel_t *chan,
- const tor_addr_t *target);
+static int channel_matches_target_addr_for_extend(
+ channel_t *chan,
+ const tor_addr_t *target_ipv4_addr,
+ const tor_addr_t *target_ipv6_addr);
/* Global lists of channels */
@@ -2365,9 +2367,9 @@ channel_is_better(channel_t *a, channel_t *b)
* Get a channel to extend a circuit.
*
* Given the desired relay identity, pick a suitable channel to extend a
- * circuit to the target address requsted by the client. Search for an
- * existing channel for the requested endpoint. Make sure the channel is
- * usable for new circuits, and matches the target address.
+ * circuit to the target IPv4 or IPv6 address requsted by the client. Search
+ * for an existing channel for the requested endpoint. Make sure the channel
+ * is usable for new circuits, and matches one of the target addresses.
*
* Try to return the best channel. But if there is no good channel, set
* *msg_out to a message describing the channel's state and our next action,
@@ -2377,7 +2379,8 @@ channel_is_better(channel_t *a, channel_t *b)
MOCK_IMPL(channel_t *,
channel_get_for_extend,(const char *rsa_id_digest,
const ed25519_public_key_t *ed_id,
- const tor_addr_t *target_addr,
+ const tor_addr_t *target_ipv4_addr,
+ const tor_addr_t *target_ipv6_addr,
const char **msg_out,
int *launch_out))
{
@@ -2409,11 +2412,15 @@ channel_get_for_extend,(const char *rsa_id_digest,
continue;
}
+ const int matches_target =
+ channel_matches_target_addr_for_extend(chan,
+ target_ipv4_addr,
+ target_ipv6_addr);
/* Never return a non-open connection. */
if (!CHANNEL_IS_OPEN(chan)) {
/* If the address matches, don't launch a new connection for this
* circuit. */
- if (channel_matches_target_addr_for_extend(chan, target_addr))
+ if (matches_target)
++n_inprogress_goodaddr;
continue;
}
@@ -2424,22 +2431,21 @@ channel_get_for_extend,(const char *rsa_id_digest,
continue;
}
- /* Never return a non-canonical connection using a recent link protocol
- * if the address is not what we wanted.
+ /* If the connection is using a recent link protocol, only return canonical
+ * connections, when the address is one of the addresses we wanted.
*
* The channel_is_canonical_is_reliable() function asks the lower layer
- * if we should trust channel_is_canonical(). The below is from the
- * comments of the old circuit_or_get_for_extend() and applies when
+ * if we should trust channel_is_canonical(). It only applies when
* the lower-layer transport is channel_tls_t.
*
- * (For old link protocols, we can't rely on is_canonical getting
+ * For old link protocols, we can't rely on is_canonical getting
* set properly if we're talking to the right address, since we might
* have an out-of-date descriptor, and we will get no NETINFO cell to
- * tell us about the right address.)
+ * tell us about the right address.
*/
if (!channel_is_canonical(chan) &&
channel_is_canonical_is_reliable(chan) &&
- !channel_matches_target_addr_for_extend(chan, target_addr)) {
+ !matches_target) {
++n_noncanonical;
continue;
}
@@ -3302,20 +3308,33 @@ channel_matches_extend_info(channel_t *chan, extend_info_t *extend_info)
}
/**
- * Check if a channel matches a given target address; return true iff we do.
+ * Check if a channel matches the given target IPv4 or IPv6 addresses.
+ * If either address matches, return true. If neither address matches,
+ * return false.
+ *
+ * Both addresses can't be NULL.
*
* This function calls into the lower layer and asks if this channel thinks
- * it matches a given target address for circuit extension purposes.
+ * it matches the target addresses for circuit extension purposes.
*/
int
channel_matches_target_addr_for_extend(channel_t *chan,
- const tor_addr_t *target)
+ const tor_addr_t *target_ipv4_addr,
+ const tor_addr_t *target_ipv6_addr)
{
tor_assert(chan);
tor_assert(chan->matches_target);
- tor_assert(target);
- return chan->matches_target(chan, target);
+ IF_BUG_ONCE(!target_ipv4_addr && !target_ipv6_addr)
+ return 0;
+
+ if (target_ipv4_addr && chan->matches_target(chan, target_ipv4_addr))
+ return 1;
+
+ if (target_ipv6_addr && chan->matches_target(chan, target_ipv6_addr))
+ return 1;
+
+ return 0;
}
/**
diff --git a/src/core/or/channel.h b/src/core/or/channel.h
index f86e77992..4968c8714 100644
--- a/src/core/or/channel.h
+++ b/src/core/or/channel.h
@@ -661,7 +661,8 @@ channel_t * channel_connect(const tor_addr_t *addr, uint16_t port,
MOCK_DECL(channel_t *, channel_get_for_extend,(
const char *rsa_id_digest,
const struct ed25519_public_key_t *ed_id,
- const tor_addr_t *target_addr,
+ const tor_addr_t *target_ipv4_addr,
+ const tor_addr_t *target_ipv6_addr,
const char **msg_out,
int *launch_out));
diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c
index ce0f9618f..0381a4dc3 100644
--- a/src/core/or/circuitbuild.c
+++ b/src/core/or/circuitbuild.c
@@ -559,11 +559,17 @@ circuit_handle_first_hop(origin_circuit_t *circ)
fmt_addrport(&firsthop->extend_info->addr,
firsthop->extend_info->port));
- n_chan = channel_get_for_extend(firsthop->extend_info->identity_digest,
- &firsthop->extend_info->ed_identity,
- &firsthop->extend_info->addr,
- &msg,
- &should_launch);
+ /* We'll cleanup this code in #33220, when we add an IPv6 address to
+ * extend_info_t. */
+ const bool addr_is_ipv4 =
+ (tor_addr_family(&firsthop->extend_info->addr) == AF_INET);
+ n_chan = channel_get_for_extend(
+ firsthop->extend_info->identity_digest,
+ &firsthop->extend_info->ed_identity,
+ addr_is_ipv4 ? &firsthop->extend_info->addr : NULL,
+ addr_is_ipv4 ? NULL : &firsthop->extend_info->addr,
+ &msg,
+ &should_launch);
if (!n_chan) {
/* not currently connected in a useful way. */
diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c
index 7d3d58977..a926a1d81 100644
--- a/src/feature/relay/circuitbuild_relay.c
+++ b/src/feature/relay/circuitbuild_relay.c
@@ -330,9 +330,20 @@ circuit_extend(struct cell_t *cell, struct circuit_t *circ)
if (circuit_extend_lspec_valid_helper(&ec, circ) < 0)
return -1;
+ /* Check the addresses, without logging */
+ const int ipv4_valid =
+ (circuit_extend_addr_port_helper(&ec.orport_ipv4, false, false, 0) == 0);
+ const int ipv6_valid =
+ (circuit_extend_addr_port_helper(&ec.orport_ipv6, false, false, 0) == 0);
+ IF_BUG_ONCE(!ipv4_valid && !ipv6_valid) {
+ /* circuit_extend_lspec_valid_helper() should have caught this */
+ return -1;
+ }
+
n_chan = channel_get_for_extend((const char*)ec.node_id,
&ec.ed_pubkey,
- &ec.orport_ipv4.addr,
+ ipv4_valid ? &ec.orport_ipv4.addr : NULL,
+ ipv6_valid ? &ec.orport_ipv6.addr : NULL,
&msg,
&should_launch);
diff --git a/src/lib/net/address.h b/src/lib/net/address.h
index 611d1ca1e..651d6bc71 100644
--- a/src/lib/net/address.h
+++ b/src/lib/net/address.h
@@ -104,6 +104,10 @@ int tor_addr_from_sockaddr(tor_addr_t *a, const struct sockaddr *sa,
uint16_t *port_out);
void tor_addr_make_unspec(tor_addr_t *a);
void tor_addr_make_null(tor_addr_t *a, sa_family_t family);
+#define tor_addr_port_make_null(addr, port, family) \
+ (void)(tor_addr_make_null(addr, family), (port) = 0)
+#define tor_addr_port_make_null_ap(ap, family) \
+ tor_addr_port_make_null(&(ap)->addr, (ap)->port, family)
char *tor_sockaddr_to_str(const struct sockaddr *sa);
/** Return an in6_addr* equivalent to <b>a</b>, or NULL if <b>a</b> is not
diff --git a/src/test/test_channel.c b/src/test/test_channel.c
index f7efbd7ab..849cc497f 100644
--- a/src/test/test_channel.c
+++ b/src/test/test_channel.c
@@ -1326,7 +1326,7 @@ test_channel_for_extend(void *arg)
channel_t *ret_chan = NULL;
char digest[DIGEST_LEN];
ed25519_public_key_t ed_id;
- tor_addr_t addr;
+ tor_addr_t ipv4_addr, ipv6_addr;
const char *msg;
int launch;
time_t now = time(NULL);
@@ -1336,6 +1336,9 @@ test_channel_for_extend(void *arg)
memset(digest, 'A', sizeof(digest));
memset(&ed_id, 'B', sizeof(ed_id));
+ tor_addr_make_null(&ipv4_addr, AF_INET);
+ tor_addr_make_null(&ipv6_addr, AF_INET6);
+
chan1 = new_fake_channel();
tt_assert(chan1);
/* Need to be registered to get added to the id map. */
@@ -1366,7 +1369,8 @@ test_channel_for_extend(void *arg)
tt_ptr_op(channel_find_by_remote_identity(digest, &ed_id), OP_EQ, chan1);
/* The expected result is chan2 because it is older than chan1. */
- ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch);
+ ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
+ &msg, &launch);
tt_assert(ret_chan);
tt_ptr_op(ret_chan, OP_EQ, chan2);
tt_int_op(launch, OP_EQ, 0);
@@ -1374,7 +1378,8 @@ test_channel_for_extend(void *arg)
/* Switch that around from previous test. */
chan2->timestamp_created = chan1->timestamp_created + 1;
- ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch);
+ ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
+ &msg, &launch);
tt_assert(ret_chan);
tt_ptr_op(ret_chan, OP_EQ, chan1);
tt_int_op(launch, OP_EQ, 0);
@@ -1383,7 +1388,8 @@ test_channel_for_extend(void *arg)
/* Same creation time, num circuits will be used and they both have 0 so the
* channel 2 should be picked due to how channel_is_better() works. */
chan2->timestamp_created = chan1->timestamp_created;
- ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch);
+ ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
+ &msg, &launch);
tt_assert(ret_chan);
tt_ptr_op(ret_chan, OP_EQ, chan1);
tt_int_op(launch, OP_EQ, 0);
@@ -1394,7 +1400,8 @@ test_channel_for_extend(void *arg)
/* Condemned the older channel. */
chan1->state = CHANNEL_STATE_CLOSING;
- ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch);
+ ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
+ &msg, &launch);
tt_assert(ret_chan);
tt_ptr_op(ret_chan, OP_EQ, chan2);
tt_int_op(launch, OP_EQ, 0);
@@ -1403,7 +1410,8 @@ test_channel_for_extend(void *arg)
/* Make the older channel a client one. */
channel_mark_client(chan1);
- ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch);
+ ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
+ &msg, &launch);
tt_assert(ret_chan);
tt_ptr_op(ret_chan, OP_EQ, chan2);
tt_int_op(launch, OP_EQ, 0);
@@ -1413,8 +1421,9 @@ test_channel_for_extend(void *arg)
/* Non matching ed identity with valid digest. */
ed25519_public_key_t dumb_ed_id;
memset(&dumb_ed_id, 0, sizeof(dumb_ed_id));
- ret_chan = channel_get_for_extend(digest, &dumb_ed_id, &addr, &msg,
- &launch);
+ ret_chan = channel_get_for_extend(digest, &dumb_ed_id,
+ &ipv4_addr, &ipv6_addr,
+ &msg, &launch);
tt_assert(!ret_chan);
tt_str_op(msg, OP_EQ, "Not connected. Connecting.");
tt_int_op(launch, OP_EQ, 1);
@@ -1423,7 +1432,8 @@ test_channel_for_extend(void *arg)
test_chan_should_match_target = 1;
chan1->state = CHANNEL_STATE_OPENING;
chan2->state = CHANNEL_STATE_OPENING;
- ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch);
+ ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
+ &msg, &launch);
tt_assert(!ret_chan);
tt_str_op(msg, OP_EQ, "Connection in progress; waiting.");
tt_int_op(launch, OP_EQ, 0);
@@ -1432,7 +1442,8 @@ test_channel_for_extend(void *arg)
/* Mark channel 1 as bad for circuits. */
channel_mark_bad_for_new_circs(chan1);
- ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch);
+ ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
+ &msg, &launch);
tt_assert(ret_chan);
tt_ptr_op(ret_chan, OP_EQ, chan2);
tt_int_op(launch, OP_EQ, 0);
@@ -1442,7 +1453,8 @@ test_channel_for_extend(void *arg)
/* Mark both channels as unusable. */
channel_mark_bad_for_new_circs(chan1);
channel_mark_bad_for_new_circs(chan2);
- ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch);
+ ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
+ &msg, &launch);
tt_assert(!ret_chan);
tt_str_op(msg, OP_EQ, "Connections all too old, or too non-canonical. "
" Launching a new one.");
@@ -1453,7 +1465,8 @@ test_channel_for_extend(void *arg)
/* Non canonical channels. */
test_chan_should_match_target = 0;
test_chan_canonical_should_be_reliable = 1;
- ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch);
+ ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
+ &msg, &launch);
tt_assert(!ret_chan);
tt_str_op(msg, OP_EQ, "Connections all too old, or too non-canonical. "
" Launching a new one.");
diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c
index a8d6323e4..668d9869d 100644
--- a/src/test/test_circuitbuild.c
+++ b/src/test/test_circuitbuild.c
@@ -993,13 +993,15 @@ static channel_t *mock_channel_get_for_extend_nchan = NULL;
static channel_t *
mock_channel_get_for_extend(const char *rsa_id_digest,
const ed25519_public_key_t *ed_id,
- const tor_addr_t *target_addr,
+ const tor_addr_t *target_ipv4_addr,
+ const tor_addr_t *target_ipv6_addr,
const char **msg_out,
int *launch_out)
{
(void)rsa_id_digest;
(void)ed_id;
- (void)target_addr;
+ (void)target_ipv4_addr;
+ (void)target_ipv6_addr;
/* channel_get_for_extend() requires non-NULL arguments */
tt_ptr_op(msg_out, OP_NE, NULL);
More information about the tor-commits
mailing list