commit ceb6585a4bc2beadde2fb194395711e72ee8559d Author: Nick Mathewson nickm@torproject.org Date: Wed Jul 22 15:20:41 2020 -0400

Treat all extorport connections with un-set addresses as remote

Without this fix, if an PT forgets to send a USERADDR command, that results in a connection getting treated as local for the purposes of rate-limiting.

If the PT _does_ use USERADDR, we still believe it.

Closes ticket 33747. --- changes/ticket33747 | 7 +++++++ src/core/mainloop/connection.c | 7 ++++++- src/core/or/connection_st.h | 3 +++ src/feature/relay/ext_orport.c | 5 ++++- 4 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/changes/ticket33747 b/changes/ticket33747 new file mode 100644 index 0000000000..57c72e9d0a --- /dev/null +++ b/changes/ticket33747 @@ -0,0 +1,7 @@ + o Minor bugfixes (rate limiting, bridges, pluggable transports): + - On a bridge, treat all connections from an ExtORPort as remote + by default for the purposes of rate-limiting. Previously, + bridges would treat the connection as local unless they explicitly + received a "USERADDR" command. ExtORPort connections still + count as local if there is a USERADDR command with an explicit local + address. Fixes bug 33747; bugfix on 0.2.5.1-alpha. diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index 3595bba85c..e895f8a73d 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -379,8 +379,12 @@ or_connection_new(int type, int socket_family)

connection_or_set_canonical(or_conn, 0);

- if (type == CONN_TYPE_EXT_OR) + if (type == CONN_TYPE_EXT_OR) { + /* If we aren't told an address for this connection, we should + * presume it isn't local, and should be rate-limited. */ + TO_CONN(or_conn)->always_rate_limit_as_remote = 1; connection_or_set_ext_or_identifier(or_conn); + }

return or_conn; } @@ -3025,6 +3029,7 @@ connection_is_rate_limited(connection_t *conn) if (conn->linked) return 0; /* Internal connection */ else if (! options->CountPrivateBandwidth && + ! conn->always_rate_limit_as_remote && (tor_addr_family(&conn->addr) == AF_UNSPEC || /* no address */ tor_addr_family(&conn->addr) == AF_UNIX || /* no address */ tor_addr_is_internal(&conn->addr, 0))) diff --git a/src/core/or/connection_st.h b/src/core/or/connection_st.h index d1430eda14..c197a81340 100644 --- a/src/core/or/connection_st.h +++ b/src/core/or/connection_st.h @@ -64,6 +64,9 @@ struct connection_t { /** True if connection_handle_write is currently running on this connection. */ unsigned int in_connection_handle_write:1; + /** If true, then we treat this connection as remote for the purpose of + * rate-limiting, no matter what its address is. */ + unsigned int always_rate_limit_as_remote:1;

/* For linked connections: */ diff --git a/src/feature/relay/ext_orport.c b/src/feature/relay/ext_orport.c index 56c5bb96f5..136aee3084 100644 --- a/src/feature/relay/ext_orport.c +++ b/src/feature/relay/ext_orport.c @@ -494,6 +494,10 @@ connection_ext_or_handle_cmd_useraddr(connection_t *conn, } conn->address = tor_addr_to_str_dup(&addr);

+ /* Now that we know the address, we don't have to manually override rate + * limiting. */ + conn->always_rate_limit_as_remote = 0; + return 0; }

@@ -659,4 +663,3 @@ ext_orport_free_all(void) if (ext_or_auth_cookie) /* Free the auth cookie */ tor_free(ext_or_auth_cookie); } -