[tor-commits] [tor/master] Validate address more carefully when checking self-reachability
asn at torproject.org
asn at torproject.org
Tue Aug 25 17:37:59 UTC 2020
commit 75772ea096e030ecc79f67b1444cac42aaed7449
Author: Nick Mathewson <nickm at torproject.org>
Date: Thu Aug 6 11:21:00 2020 -0400
Validate address more carefully when checking self-reachability
Previously, we would treat *any* incoming circuit on a non-local
channel as meaning that our ORPort was reachable. With this patch,
we make sure that the address that the peer _says_ we have is the
same as the one we're trying to advertise right now.
Closes 20165. Bugfix on 4f5192b2803c706 in 0.1.0.1-rc, when
reachability self-tests were first introduced.
---
changes/bug20165 | 6 ++++++
src/core/or/channel.c | 2 ++
src/core/or/channel.h | 3 +++
src/core/or/channeltls.c | 7 +++++++
src/feature/relay/circuitbuild_relay.c | 24 +++++++++++++++++-------
src/feature/relay/router.c | 25 +++++++++++++++++++++++++
src/feature/relay/router.h | 1 +
7 files changed, 61 insertions(+), 7 deletions(-)
diff --git a/changes/bug20165 b/changes/bug20165
new file mode 100644
index 0000000000..bbe9f00032
--- /dev/null
+++ b/changes/bug20165
@@ -0,0 +1,6 @@
+ o Minor bugfixes (self-testing):
+ - When receiving an incoming circuit, only accept it as evidence that we
+ are reachable if the declared address of its channel is the same
+ address we think that we have. Otherwise, it could be evidence that
+ we're reachable on some other address. Fixes bug 20165; bugfix on
+ 0.1.0.1-rc.
diff --git a/src/core/or/channel.c b/src/core/or/channel.c
index 5a42d452f2..0f74950b4f 100644
--- a/src/core/or/channel.c
+++ b/src/core/or/channel.c
@@ -872,6 +872,8 @@ channel_init(channel_t *chan)
/* Channel is not in the scheduler heap. */
chan->sched_heap_idx = -1;
+
+ tor_addr_make_unspec(&chan->addr_according_to_peer);
}
/**
diff --git a/src/core/or/channel.h b/src/core/or/channel.h
index d52ebdf619..fc0dd3aaac 100644
--- a/src/core/or/channel.h
+++ b/src/core/or/channel.h
@@ -236,6 +236,9 @@ struct channel_t {
/** The handle to this channel (to free on canceled timers) */
struct channel_handle_t *timer_handle;
+ /** If not UNSPEC, the address that the peer says we have. */
+ tor_addr_t addr_according_to_peer;
+
/**
* These two fields specify the minimum and maximum negotiated timeout
* values for inactivity (send or receive) before we decide to pad a
diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c
index ae60038c34..a9012fd317 100644
--- a/src/core/or/channeltls.c
+++ b/src/core/or/channeltls.c
@@ -1868,6 +1868,13 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan)
}
}
+ if (me) {
+ /* We have a descriptor, so we are a relay: record the address that the
+ * other side said we had. */
+ tor_addr_copy(&TLS_CHAN_TO_BASE(chan)->addr_according_to_peer,
+ &my_apparent_addr);
+ }
+
n_other_addrs = netinfo_cell_get_n_my_addrs(netinfo_cell);
for (uint8_t i = 0; i < n_other_addrs; i++) {
/* Consider all the other addresses; if any matches, this connection is
diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c
index ad20e143be..64f3c341ae 100644
--- a/src/feature/relay/circuitbuild_relay.c
+++ b/src/feature/relay/circuitbuild_relay.c
@@ -588,13 +588,23 @@ onionskin_answer(struct or_circuit_t *circ,
if ((!channel_is_local(circ->p_chan)
|| get_options()->ExtendAllowPrivateAddresses)
&& !channel_is_outgoing(circ->p_chan)) {
- /* record that we could process create cells from a non-local conn
- * that we didn't initiate; presumably this means that create cells
- * can reach us too. */
- tor_addr_t remote_addr;
- if (channel_get_addr_if_possible(circ->p_chan, &remote_addr)) {
- int family = tor_addr_family(&remote_addr);
- router_orport_found_reachable(family);
+ /* Okay, it's a create cell from a non-local connection
+ * that we didn't initiate. Presumably this means that create cells
+ * can reach us too. But what address can they reach us on? */
+ const tor_addr_t *my_supposed_addr = &circ->p_chan->addr_according_to_peer;
+ if (router_addr_is_my_published_addr(my_supposed_addr)) {
+ /* Great, this create cell came on connection where the peer says
+ * that the our address is an address we're actually advertising!
+ * That should mean that we're reachable. But before we finally
+ * declare ourselves reachable, make sure that the address listed
+ * by the peer is the same family as the peer is actually using.
+ */
+ tor_addr_t remote_addr;
+ int family = tor_addr_family(my_supposed_addr);
+ if (channel_get_addr_if_possible(circ->p_chan, &remote_addr) &&
+ tor_addr_family(&remote_addr) == family) {
+ router_orport_found_reachable(family);
+ }
}
}
diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c
index 675b977ade..3fcf0d616b 100644
--- a/src/feature/relay/router.c
+++ b/src/feature/relay/router.c
@@ -1729,6 +1729,31 @@ router_is_me(const routerinfo_t *router)
return router_digest_is_me(router->cache_info.identity_digest);
}
+/**
+ * Return true if we are a server, and if @a addr is an address we are
+ * currently publishing (or trying to publish) in our descriptor.
+ * Return false otherwise.
+ **/
+bool
+router_addr_is_my_published_addr(const tor_addr_t *addr)
+{
+ IF_BUG_ONCE(!addr)
+ return false;
+
+ const routerinfo_t *me = router_get_my_routerinfo();
+ if (!me)
+ return false;
+
+ switch (tor_addr_family(addr)) {
+ case AF_INET:
+ return tor_addr_eq(addr, &me->ipv4_addr);
+ case AF_INET6:
+ return tor_addr_eq(addr, &me->ipv6_addr);
+ default:
+ return false;
+ }
+}
+
/** Return a routerinfo for this OR, rebuilding a fresh one if
* necessary. Return NULL on error, or if called on an OP. */
MOCK_IMPL(const routerinfo_t *,
diff --git a/src/feature/relay/router.h b/src/feature/relay/router.h
index 89b4a479a4..f71ada8eb7 100644
--- a/src/feature/relay/router.h
+++ b/src/feature/relay/router.h
@@ -100,6 +100,7 @@ int router_digest_is_me(const char *digest);
const uint8_t *router_get_my_id_digest(void);
int router_extrainfo_digest_is_me(const char *digest);
int router_is_me(const routerinfo_t *router);
+bool router_addr_is_my_published_addr(const tor_addr_t *addr);
int router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e);
int router_rebuild_descriptor(int force);
char *router_dump_router_to_string(routerinfo_t *router,
More information about the tor-commits
mailing list