commit 02cff32d7947cd230b1dab299b29dec5315ea4b9 Author: Nick Mathewson nickm@torproject.org Date: Mon Jul 13 12:13:41 2020 -0400

Improve docs for addr, address, and real_addr fields

These fields have a complicated history, some slightly complicated behavior, and some definitely inadequate documentation. Before we go fixing them up, let's document how they work now. --- src/core/or/connection_st.h | 52 +++++++++++++++++++++++++++++++++++++----- src/core/or/or_connection_st.h | 16 +++++++++---- 2 files changed, 58 insertions(+), 10 deletions(-)

diff --git a/src/core/or/connection_st.h b/src/core/or/connection_st.h index 55d94d9451..508328f75c 100644 --- a/src/core/or/connection_st.h +++ b/src/core/or/connection_st.h @@ -109,6 +109,39 @@ struct connection_t {

int socket_family; /**< Address family of this connection's socket. Usually * AF_INET, but it can also be AF_UNIX, or AF_INET6 */ + /** + * IP address on the internet of this connection's peer, usually. + * + * This address may come from several sources. If this is an outbound + * connection, it is the address we are trying to connect to--either + * directly through `s`, or via a proxy. (If we used a proxy, then + * `getpeername(s)` will not give this address.) + * + * For incoming connections, this field is the address we got from + * getpeername() or accept(), as updated by any proxy that we + * are using (for example, an ExtORPort proxy). + * + * For listeners, this is the address we are trying to bind to. + * + * If this connection is using a unix socket, then this address is a null + * address, and the real address is in the `address` field. + * + * If this connection represents a request made somewhere other than via + * TCP (for example, a UDP dns request, or a controller resolve request), + * then this address is the address that originated the request. + * + * TECHNICAL DEBT: + * + * There are a few places in the code that modify this address, + * or use it in other ways that we don't currently like. Please don't add + * any more! + * + * The misuses of this field include: + * * Setting it to the canonical address of a relay on an OR connection. + * * Setting it on linked connections, possibly. + * * Updating it based on the Forwarded-For header-- Forwarded-For is + * set by a proxy, but not a local trusted troxy. + **/ tor_addr_t addr; /**< IP that socket "s" is directly connected to; * may be the IP address for a proxy or pluggable transport, * see "address" for the address of the final destination. @@ -122,12 +155,19 @@ struct connection_t { * marked.) */ const char *marked_for_close_file; /**< For debugging: in which file were * we marked for close? */ - char *address; /**< FQDN (or IP) and port of the final destination for this - * connection; this is always the remote address, it is - * passed to a proxy or pluggable transport if one in use. - * See "addr" and "port" for the address that socket "s" is - * directly connected to. - * strdup into this, because free_connection() frees it. */ + /** + * String address of the peer of this connection. + * + * TECHNICAL DEBT: + * + * This field serves many purposes, and they're not all pretty. In addition + * to describing the peer we're connected to, it can also hold: + * + * * An address we're trying to resolve (as an exit). + * * A unix address we're trying to bind to (as a listener). + * * A canonical address for an OR connection. + **/ + char *address; /** Another connection that's connected to this one in lieu of a socket. */ struct connection_t *linked_conn;

diff --git a/src/core/or/or_connection_st.h b/src/core/or/or_connection_st.h index 92956c2847..2507f90803 100644 --- a/src/core/or/or_connection_st.h +++ b/src/core/or/or_connection_st.h @@ -49,10 +49,18 @@ struct or_connection_t { /* Channel using this connection */ channel_tls_t *chan;

- tor_addr_t real_addr; /**< The actual address that this connection came from - * or went to. The <b>addr</b> field is prone to - * getting overridden by the address from the router - * descriptor matching <b>identity_digest</b>. */ + /** + * The actual address (as modified by any proxies) that this connection + * came from or went to. (See connection_t.addr for caveats.) + * + * TECHNICAL DEBT: + * + * This field shouldn't really exist. We need it because our code + * overwrites conenction_t.addr with the "canonical address" of the OR we + * are talking to, taken from the descriptor of the authenticated OR. + * That's a bad choice. + **/ + tor_addr_t real_addr;

/** Should this connection be used for extending circuits to the server * matching the <b>identity_digest</b> field? Set to true if we're pretty