July 2020 - tor-commits - lists.torproject.org

[tor/master] New functions to describe connections and peers.
by nickm＠torproject.org 16 Jul '20

16 Jul '20

commit ba7816db5f55d76e925c2edcbbbe689490d85650 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Jul 13 14:35:51 2020 -0400 New functions to describe connections and peers. These will let us refactor away a lot of code that currently uses "addr" and "address" for logging purposes. --- src/core/mainloop/connection.c | 150 +++++++++++++++++++++++++++++++++++++++++ src/core/mainloop/connection.h | 3 + 2 files changed, 153 insertions(+) diff --git a/src/core/… [View More]mainloop/connection.c b/src/core/mainloop/connection.c index 1d53e077d1..a91bc6408f 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -352,13 +352,163 @@ conn_state_to_string(int type, int state) break; } + if (state == 0) { + return "uninitialized"; + } + log_warn(LD_BUG, "unknown connection state %d (type %d)", state, type); tor_snprintf(buf, sizeof(buf), "unknown state [%d] on unknown [%s] connection", state, conn_type_to_string(type)); + tor_assert_nonfatal_unreached_once(); return buf; } +/** + * Helper: describe the peer or address of connection @a conn in a + * human-readable manner. + * + * Returns a pointer to a static buffer; future calls to + * connection_describe_peer_internal() will invalidate this buffer. + * + * If <b>include_preposition</b> is true, include a preposition before the + * peer address. + * + * Nobody should parse the output of this function; it can and will change in + * future versions of tor. + **/ +static const char * +connection_describe_peer_internal(const connection_t *conn, + bool include_preposition) +{ + IF_BUG_ONCE(!conn) { + return "null peer"; + } + + static char peer_buf[256]; + const tor_addr_t *addr = &conn->addr; + const char *address = NULL; + const char *prep; + char extra_buf[128]; + extra_buf[0] = 0; + + /* First, figure out the preposition to use */ + switch (conn->type) { + CASE_ANY_LISTENER_TYPE: + prep = "on"; + break; + case CONN_TYPE_EXIT: + prep = "to"; + break; + case CONN_TYPE_CONTROL: + case CONN_TYPE_AP: + case CONN_TYPE_EXT_OR: + prep = "from"; + break; + default: + prep = "with"; + break; + } + + /* Now figure out the address. */ + if (conn->socket_family == AF_UNIX) { + /* For unix sockets, we always use the `address` string. */ + address = conn->address ? conn->address : "unix socket"; + } else if (conn->type == CONN_TYPE_OR) { + /* For OR connections, we have a lot to do. */ + const or_connection_t *or_conn = TO_OR_CONN((connection_t *)conn); + char id_buf[HEX_DIGEST_LEN+1]; + /* we report 'real_addr' as the address we're talking with */ + addr = &or_conn->real_addr; + /* We report the IDs we're talking to... */ + if (fast_digest_is_zero(or_conn->identity_digest)) { + strlcpy(id_buf, "unknown", sizeof(id_buf)); + } else { + base16_encode(id_buf, sizeof(id_buf), + or_conn->identity_digest, DIGEST_LEN); + } + tor_snprintf(extra_buf, sizeof(extra_buf), + " ID=%s", id_buf); + if (! tor_addr_eq(addr, &conn->addr)) { + /* We report canonical address, if it's different */ + char canonical_addr_buf[TOR_ADDR_BUF_LEN]; + if (tor_addr_to_str(canonical_addr_buf, &conn->addr, + sizeof(canonical_addr_buf), 1)) { + strlcat(extra_buf, " canonical_addr=", sizeof(extra_buf)); + strlcat(extra_buf, canonical_addr_buf, sizeof(extra_buf)); + } + } + } else if (conn->type == CONN_TYPE_EXIT) { + if (tor_addr_is_null(addr)) { + address = conn->address; + strlcpy(extra_buf, " (DNS lookup pending)", sizeof(extra_buf)); + } + } + + char addr_buf[TOR_ADDR_BUF_LEN]; + if (address == NULL) { + if (tor_addr_family(addr) == 0) { + address = "<unset>"; + } else { + address = tor_addr_to_str(addr_buf, addr, sizeof(addr_buf), 1); + if (!address) { + address = "<can't format!>"; + tor_assert_nonfatal_unreached_once(); + } + } + } + + const char *sp = include_preposition ? " " : ""; + if (! include_preposition) + prep = ""; + + tor_snprintf(peer_buf, sizeof(peer_buf), + "%s%s%s%s", prep, sp, address, extra_buf); + return peer_buf; +} + +/** + * Describe the peer or address of connection @a conn in a + * human-readable manner. + * + * Returns a pointer to a static buffer; future calls to + * connection_describe_peer() or connection_describe() will invalidate this + * buffer. + * + * Nobody should parse the output of this function; it can and will change in + * future versions of tor. + **/ +const char * +connection_describe_peer(const connection_t *conn) +{ + return connection_describe_peer_internal(conn, false); +} + +/** + * Describe a connection for logging purposes. + * + * Returns a pointer to a static buffer; future calls to connection_describe() + * will invalidate this buffer. + * + * Nobody should parse the output of this function; it can and will change in + * future versions of tor. + **/ +const char * +connection_describe(const connection_t *conn) +{ + IF_BUG_ONCE(!conn) { + return "null connection"; + } + static char desc_buf[256]; + const char *peer = connection_describe_peer_internal(conn, true); + tor_snprintf(desc_buf, sizeof(desc_buf), + "%s connection (%s) %s", + conn_type_to_string(conn->type), + conn_state_to_string(conn->type, conn->state), + peer); + return desc_buf; +} + /** Allocate and return a new dir_connection_t, initialized as by * connection_init(). */ dir_connection_t * diff --git a/src/core/mainloop/connection.h b/src/core/mainloop/connection.h index bcd3d590a5..af45007676 100644 --- a/src/core/mainloop/connection.h +++ b/src/core/mainloop/connection.h @@ -116,6 +116,9 @@ const char *conn_type_to_string(int type); const char *conn_state_to_string(int type, int state); int conn_listener_type_supports_af_unix(int type); +const char *connection_describe(const connection_t *conn); +const char *connection_describe_peer(const connection_t *conn); + struct dir_connection_t *dir_connection_new(int socket_family); struct or_connection_t *or_connection_new(int type, int socket_family); struct edge_connection_t *edge_connection_new(int type, int socket_family); [View Less]

1 0

[tor/master] connection_describe_peer(): add nonzero ports to nonscrubbed addrs.
by nickm＠torproject.org 16 Jul '20

16 Jul '20

commit aa33b88778c2eda110eb79cfe887621853010287 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Jul 13 14:47:58 2020 -0400 connection_describe_peer(): add nonzero ports to nonscrubbed addrs. --- src/core/mainloop/connection.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index bde002fd2e..a3cebfecdd 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/… [View More]

1 0

[tor/master] Update test_rebind.py to expect new format.
by nickm＠torproject.org 16 Jul '20

16 Jul '20

commit 382387882c3f9cfaac6ee0c83908dc24135ba5c4 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 14 11:21:49 2020 -0400 Update test_rebind.py to expect new format. --- src/test/test_rebind.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/test_rebind.py b/src/test/test_rebind.py index 3fc3deb68e..6b72ece911 100644 --- a/src/test/test_rebind.py +++ b/src/test/test_rebind.py @@ -116,7 +116,7 @@ tor_process = subprocess.Popen([tor_path, if … [View More]

1 0

[tor/master] Use connection_describe() for log messages.
by nickm＠torproject.org 16 Jul '20

16 Jul '20

commit 9d2867c396161e6a5a7604a5de062d8e13719d17 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Jul 13 15:14:41 2020 -0400 Use connection_describe() for log messages. --- src/app/config/config.c | 4 +- src/app/main/main.c | 10 +- src/core/mainloop/connection.c | 57 +++++------ src/core/or/channeltls.c | 80 +++++++-------- src/core/or/connection_edge.c | 17 ++-- src/core/or/connection_or.c | 64 ++++++------ src/… [View More]feature/dircache/dircache.c | 19 ++-- src/feature/dirclient/dirclient.c | 203 ++++++++++++++++++++------------------ src/feature/dircommon/directory.c | 8 +- src/feature/hs/hs_client.c | 6 +- 10 files changed, 229 insertions(+), 239 deletions(-) diff --git a/src/app/config/config.c b/src/app/config/config.c index f4172bd84e..facd35b7e4 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -1736,8 +1736,8 @@ options_rollback_listener_transaction(listener_transaction_t *xn) SMARTLIST_FOREACH(xn->new_listeners, connection_t *, conn, { - log_notice(LD_NET, "Closing partially-constructed %s on %s:%d", - conn_type_to_string(conn->type), conn->address, conn->port); + log_notice(LD_NET, "Closing partially-constructed %s", + connection_describe(conn)); connection_close_immediate(conn); connection_mark_for_close(conn); }); diff --git a/src/app/main/main.c b/src/app/main/main.c index 4b1308ddf0..02c4c5bd58 100644 --- a/src/app/main/main.c +++ b/src/app/main/main.c @@ -338,15 +338,11 @@ dumpstats(int severity) SMARTLIST_FOREACH_BEGIN(get_connection_array(), connection_t *, conn) { int i = conn_sl_idx; tor_log(severity, LD_GENERAL, - "Conn %d (socket %d) type %d (%s), state %d (%s), created %d secs ago", - i, (int)conn->s, conn->type, conn_type_to_string(conn->type), - conn->state, conn_state_to_string(conn->type, conn->state), + "Conn %d (socket %d) is a %s, created %d secs ago", + i, (int)conn->s, + connection_describe(conn), (int)(now - conn->timestamp_created)); if (!connection_is_listener(conn)) { - tor_log(severity,LD_GENERAL, - "Conn %d is to %s:%d.", i, - safe_str_client(conn->address), - conn->port); tor_log(severity,LD_GENERAL, "Conn %d: %d bytes waiting on inbuf (len %d, last read %d secs ago)", i, diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index a3cebfecdd..be952fdedf 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -1711,13 +1711,8 @@ connection_listener_new(const struct sockaddr *listensockaddr, */ connection_check_oos(get_n_open_sockets(), 0); - if (conn->socket_family == AF_UNIX) { - log_notice(LD_NET, "Opened %s on %s", - conn_type_to_string(type), conn->address); - } else { - log_notice(LD_NET, "Opened %s on %s", - conn_type_to_string(type), fmt_addrport(&addr, gotPort)); - } + log_notice(LD_NET, "Opened %s", connection_describe(conn)); + return conn; err: @@ -2770,8 +2765,8 @@ connection_read_https_proxy_response(connection_t *conn) if (parse_http_response(headers, &status_code, &date_header, NULL, &reason) < 0) { log_warn(LD_NET, - "Unparseable headers from proxy (connecting to '%s'). Closing.", - conn->address); + "Unparseable headers from proxy (%s). Closing.", + connection_describe(conn)); tor_free(headers); return -1; } @@ -2780,8 +2775,8 @@ connection_read_https_proxy_response(connection_t *conn) if (status_code == 200) { log_info(LD_NET, - "HTTPS connect to '%s' successful! (200 %s) Starting TLS.", - conn->address, escaped(reason)); + "HTTPS connect for %s successful! (200 %s) Starting TLS.", + connection_describe(conn), escaped(reason)); tor_free(reason); return 1; } @@ -2997,16 +2992,16 @@ connection_read_proxy_handshake(connection_t *conn) if (ret < 0) { if (reason) { - log_warn(LD_NET, "Proxy Client: unable to connect to %s:%d (%s)", - conn->address, conn->port, escaped(reason)); + log_warn(LD_NET, "Proxy Client: unable to connect %s (%s)", + connection_describe(conn), escaped(reason)); tor_free(reason); } else { - log_warn(LD_NET, "Proxy Client: unable to connect to %s:%d", - conn->address, conn->port); + log_warn(LD_NET, "Proxy Client: unable to connect %s", + connection_describe(conn)); } } else if (ret == 1) { - log_info(LD_NET, "Proxy Client: connection to %s:%d successful", - conn->address, conn->port); + log_info(LD_NET, "Proxy Client: %s successful", + connection_describe(conn)); } return ret; @@ -3197,8 +3192,8 @@ retry_all_listeners(smartlist_t *new_conns, int close_all_noncontrol) connection_t *old_conn = r->old_conn; if (skip) { - log_debug(LD_NET, "Skipping creating new listener for %s:%d", - old_conn->address, old_conn->port); + log_debug(LD_NET, "Skipping creating new listener for %s", + connection_describe(old_conn)); continue; } @@ -3214,10 +3209,11 @@ retry_all_listeners(smartlist_t *new_conns, int close_all_noncontrol) smartlist_add(new_conns, new_conn); - log_notice(LD_NET, "Closed no-longer-configured %s on %s:%d " - "(replaced by %s:%d)", - conn_type_to_string(old_conn->type), old_conn->address, - old_conn->port, new_conn->address, new_conn->port); + char *old_desc = tor_strdup(connection_describe(old_conn)); + log_notice(LD_NET, "Closed no-longer-configured %s " + "(replaced by %s)", + old_desc, connection_describe(new_conn)); + tor_free(old_desc); } SMARTLIST_FOREACH_END(r); #endif /* defined(ENABLE_LISTENER_REBIND) */ @@ -4025,17 +4021,14 @@ connection_buf_read_from_socket(connection_t *conn, ssize_t *max_to_read, switch (result) { case TOR_TLS_CLOSE: case TOR_TLS_ERROR_IO: - log_debug(LD_NET,"TLS connection closed %son read. Closing. " - "(Nickname %s, address %s)", - result == TOR_TLS_CLOSE ? "cleanly " : "", - or_conn->nickname ? or_conn->nickname : "not set", - conn->address); + log_debug(LD_NET,"TLS %s closed %son read. Closing.", + connection_describe(conn), + result == TOR_TLS_CLOSE ? "cleanly " : ""); return result; CASE_TOR_TLS_ERROR_ANY_NONIO: - log_debug(LD_NET,"tls error [%s]. breaking (nickname %s, address %s).", + log_debug(LD_NET,"tls error [%s] from %s. Breaking.", tor_tls_err_to_string(result), - or_conn->nickname ? or_conn->nickname : "not set", - conn->address); + connection_describe(conn)); return result; case TOR_TLS_WANTWRITE: connection_start_writing(conn); @@ -4888,7 +4881,7 @@ any_other_active_or_conns(const or_connection_t *this_conn) connection_t *conn = connection_get_another_active_or_conn(this_conn); if (conn != NULL) { log_debug(LD_DIR, "%s: Found an OR connection: %s", - __func__, conn->address); + __func__, connection_describe(conn)); return 1; } diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c index 6503c2ebb0..e4dbdda7db 100644 --- a/src/core/or/channeltls.c +++ b/src/core/or/channeltls.c @@ -1519,7 +1519,7 @@ channel_tls_process_versions_cell(var_cell_t *cell, channel_tls_t *chan) log_fn(LOG_WARN, LD_OR, "Negotiated link with non-2 protocol after doing a v2 TLS " "handshake with %s. Closing connection.", - fmt_addr(&chan->conn->base_.addr)); + connection_describe_peer(TO_CONN(chan->conn))); connection_or_close_for_error(chan->conn, 0); return; } @@ -1531,10 +1531,9 @@ channel_tls_process_versions_cell(var_cell_t *cell, channel_tls_t *chan) if (chan->conn->link_proto == 2) { log_info(LD_OR, - "Negotiated version %d with %s:%d; sending NETINFO.", + "Negotiated version %d on %s; sending NETINFO.", highest_supported_version, - safe_str_client(chan->conn->base_.address), - chan->conn->base_.port); + connection_describe(TO_CONN(chan->conn))); if (connection_or_send_netinfo(chan->conn) < 0) { connection_or_close_for_error(chan->conn, 0); @@ -1554,10 +1553,9 @@ channel_tls_process_versions_cell(var_cell_t *cell, channel_tls_t *chan) tor_assert(chan->conn->link_proto >= 3); log_info(LD_OR, - "Negotiated version %d with %s:%d; %s%s%s%s%s", + "Negotiated version %d with on %s; %s%s%s%s%s", highest_supported_version, - safe_str_client(chan->conn->base_.address), - chan->conn->base_.port, + connection_describe(TO_CONN(chan->conn)), send_any ? "Sending cells:" : "Waiting for CERTS cell", send_versions ? " VERSIONS" : "", send_certs ? " CERTS" : "", @@ -1950,18 +1948,16 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan) if (connection_or_set_state_open(chan->conn) < 0) { log_fn(LOG_PROTOCOL_WARN, LD_OR, - "Got good NETINFO cell from %s:%d; but " + "Got good NETINFO cell on %s; but " "was unable to make the OR connection become open.", - safe_str_client(chan->conn->base_.address), - chan->conn->base_.port); + connection_describe(TO_CONN(chan->conn))); connection_or_close_for_error(chan->conn, 0); } else { log_info(LD_OR, - "Got good NETINFO cell from %s:%d; OR connection is now " + "Got good NETINFO cell on %s; OR connection is now " "open, using protocol version %d. Its ID digest is %s. " "Our address is apparently %s.", - safe_str_client(chan->conn->base_.address), - chan->conn->base_.port, + connection_describe(TO_CONN(chan->conn)), (int)(chan->conn->link_proto), hex_str(identity_digest, DIGEST_LEN), tor_addr_is_null(&my_apparent_addr) ? @@ -2046,9 +2042,9 @@ channel_tls_process_certs_cell(var_cell_t *cell, channel_tls_t *chan) #define ERR(s) \ do { \ log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, \ - "Received a bad CERTS cell from %s:%d: %s", \ - safe_str(chan->conn->base_.address), \ - chan->conn->base_.port, (s)); \ + "Received a bad CERTS cell on %s: %s", \ + connection_describe(TO_CONN(chan->conn)), \ + (s)); \ connection_or_close_for_error(chan->conn, 0); \ goto err; \ } while (0) @@ -2096,9 +2092,8 @@ channel_tls_process_certs_cell(var_cell_t *cell, channel_tls_t *chan) tor_x509_cert_t *x509_cert = tor_x509_cert_decode(cert_body, cert_len); if (!x509_cert) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Received undecodable certificate in CERTS cell from %s:%d", - safe_str(chan->conn->base_.address), - chan->conn->base_.port); + "Received undecodable certificate in CERTS cell on %s", + connection_describe(TO_CONN(chan->conn))); } else { if (x509_certs[cert_type]) { tor_x509_cert_free(x509_cert); @@ -2114,9 +2109,8 @@ channel_tls_process_certs_cell(var_cell_t *cell, channel_tls_t *chan) if (!ed_cert) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Received undecodable Ed certificate " - "in CERTS cell from %s:%d", - safe_str(chan->conn->base_.address), - chan->conn->base_.port); + "in CERTS cell on %s", + connection_describe(TO_CONN(chan->conn))); } else { if (ed_certs[cert_type]) { tor_cert_free(ed_cert); @@ -2226,9 +2220,9 @@ channel_tls_process_certs_cell(var_cell_t *cell, channel_tls_t *chan) ERR("Problem setting or checking peer id"); log_info(LD_HANDSHAKE, - "Got some good certificates from %s:%d: Authenticated it with " + "Got some good certificates on %s: Authenticated it with " "RSA%s", - safe_str(chan->conn->base_.address), chan->conn->base_.port, + connection_describe(TO_CONN(chan->conn)), checked_ed_id ? " and Ed25519" : ""); if (!public_server_mode(get_options())) { @@ -2240,11 +2234,10 @@ channel_tls_process_certs_cell(var_cell_t *cell, channel_tls_t *chan) } else { /* We can't call it authenticated till we see an AUTHENTICATE cell. */ log_info(LD_OR, - "Got some good RSA%s certificates from %s:%d. " + "Got some good RSA%s certificates on %s. " "Waiting for AUTHENTICATE.", checked_ed_id ? " and Ed25519" : "", - safe_str(chan->conn->base_.address), - chan->conn->base_.port); + connection_describe(TO_CONN(chan->conn))); /* XXXX check more stuff? */ } @@ -2293,9 +2286,9 @@ channel_tls_process_auth_challenge_cell(var_cell_t *cell, channel_tls_t *chan) #define ERR(s) \ do { \ log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, \ - "Received a bad AUTH_CHALLENGE cell from %s:%d: %s", \ - safe_str(chan->conn->base_.address), \ - chan->conn->base_.port, (s)); \ + "Received a bad AUTH_CHALLENGE cell on %s: %s", \ + connection_describe(TO_CONN(chan->conn)), \ + (s)); \ connection_or_close_for_error(chan->conn, 0); \ goto done; \ } while (0) @@ -2340,10 +2333,9 @@ channel_tls_process_auth_challenge_cell(var_cell_t *cell, channel_tls_t *chan) if (use_type >= 0) { log_info(LD_OR, - "Got an AUTH_CHALLENGE cell from %s:%d: Sending " + "Got an AUTH_CHALLENGE cell on %s: Sending " "authentication type %d", - safe_str(chan->conn->base_.address), - chan->conn->base_.port, + connection_describe(TO_CONN(chan->conn)), use_type); if (connection_or_send_authenticate_cell(chan->conn, use_type) < 0) { @@ -2354,10 +2346,9 @@ channel_tls_process_auth_challenge_cell(var_cell_t *cell, channel_tls_t *chan) } } else { log_info(LD_OR, - "Got an AUTH_CHALLENGE cell from %s:%d, but we don't " + "Got an AUTH_CHALLENGE cell on %s, but we don't " "know any of its authentication types. Not authenticating.", - safe_str(chan->conn->base_.address), - chan->conn->base_.port); + connection_describe(TO_CONN(chan->conn))); } if (connection_or_send_netinfo(chan->conn) < 0) { @@ -2397,9 +2388,9 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan) #define ERR(s) \ do { \ log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, \ - "Received a bad AUTHENTICATE cell from %s:%d: %s", \ - safe_str(chan->conn->base_.address), \ - chan->conn->base_.port, (s)); \ + "Received a bad AUTHENTICATE cell on %s: %s", \ + connection_describe(TO_CONN(chan->conn)), \ + (s)); \ connection_or_close_for_error(chan->conn, 0); \ var_cell_free(expected_cell); \ return; \ @@ -2560,9 +2551,9 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan) crypto_pk_free(identity_rcvd); log_debug(LD_HANDSHAKE, - "Calling connection_or_init_conn_from_address for %s " + "Calling connection_or_init_conn_from_address on %s " " from %s, with%s ed25519 id.", - safe_str(chan->conn->base_.address), + connection_describe(TO_CONN(chan->conn)), __func__, ed_identity_received ? "" : "out"); @@ -2575,10 +2566,9 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan) 0); log_debug(LD_HANDSHAKE, - "Got an AUTHENTICATE cell from %s:%d, type %d: Looks good.", - safe_str(chan->conn->base_.address), - chan->conn->base_.port, - authtype); + "Got an AUTHENTICATE cell on %s, type %d: Looks good.", + connection_describe(TO_CONN(chan->conn)), + authtype); } var_cell_free(expected_cell); diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c index 45d2a9664a..cd4ec8be7b 100644 --- a/src/core/or/connection_edge.c +++ b/src/core/or/connection_edge.c @@ -424,9 +424,7 @@ warn_if_hs_unreachable(const edge_connection_t *conn, uint8_t reason) char *m; if ((m = rate_limit_log(&warn_limit, approx_time()))) { log_warn(LD_EDGE, "Onion service connection to %s failed (%s)", - (conn->base_.socket_family == AF_UNIX) ? - safe_str(conn->base_.address) : - safe_str(fmt_addrport(&conn->base_.addr, conn->base_.port)), + connection_describe_peer(TO_CONN(conn)), stream_end_reason_to_string(reason)); tor_free(m); } @@ -922,9 +920,8 @@ connection_edge_finished_connecting(edge_connection_t *edge_conn) conn = TO_CONN(edge_conn); tor_assert(conn->state == EXIT_CONN_STATE_CONNECTING); - log_info(LD_EXIT,"Exit connection to %s:%u (%s) established.", - escaped_safe_str(conn->address), conn->port, - safe_str(fmt_and_decorate_addr(&conn->addr))); + log_info(LD_EXIT,"%s established.", + connection_describe(conn)); rep_hist_note_exit_stream_opened(conn->port); @@ -3840,8 +3837,8 @@ handle_hs_exit_conn(circuit_t *circ, edge_connection_t *conn) return -1; } if (ret < 0) { - log_info(LD_REND, "Didn't find rendezvous service (addr%s, port %d)", - fmt_addr(&TO_CONN(conn)->addr), TO_CONN(conn)->port); + log_info(LD_REND, "Didn't find rendezvous service at %s", + connection_describe_peer(TO_CONN(conn))); /* Send back reason DONE because we want to make hidden service port * scanning harder thus instead of returning that the exit policy * didn't match, which makes it obvious that the port is closed, @@ -4192,8 +4189,8 @@ connection_exit_connect(edge_connection_t *edge_conn) &why_failed_exit_policy)) { if (BUG(!why_failed_exit_policy)) why_failed_exit_policy = ""; - log_info(LD_EXIT,"%s:%d failed exit policy%s. Closing.", - escaped_safe_str_client(conn->address), conn->port, + log_info(LD_EXIT,"%s failed exit policy%s. Closing.", + connection_describe(conn), why_failed_exit_policy); connection_edge_end(edge_conn, END_STREAM_REASON_EXITPOLICY); circuit_detach_stream(circuit_get_by_edge_conn(edge_conn), edge_conn); diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c index 079fa3dfcb..e62c28821a 100644 --- a/src/core/or/connection_or.c +++ b/src/core/or/connection_or.c @@ -151,9 +151,9 @@ connection_or_set_identity_digest(or_connection_t *conn, if (conn->chan) chan = TLS_CHAN_TO_BASE(conn->chan); - log_info(LD_HANDSHAKE, "Set identity digest for %p (%s): %s %s.", + log_info(LD_HANDSHAKE, "Set identity digest for %s at %p: %s %s.", + connection_describe(TO_CONN(conn)), conn, - escaped_safe_str(conn->base_.address), hex_str(rsa_digest, DIGEST_LEN), ed25519_fmt(ed_id)); log_info(LD_HANDSHAKE, " (Previously: %s %s)", @@ -575,11 +575,9 @@ connection_or_process_inbuf(or_connection_t *conn) * 100% true. */ if (buf_datalen(conn->base_.inbuf) > MAX_OR_INBUF_WHEN_NONOPEN) { log_fn(LOG_PROTOCOL_WARN, LD_NET, "Accumulated too much data (%d bytes) " - "on nonopen OR connection %s %s:%u in state %s; closing.", + "on non-open %s; closing.", (int)buf_datalen(conn->base_.inbuf), - connection_or_nonopen_was_started_here(conn) ? "to" : "from", - conn->base_.address, conn->base_.port, - conn_state_to_string(conn->base_.type, conn->base_.state)); + connection_describe(TO_CONN(conn))); connection_or_close_for_error(conn, 0); ret = -1; } @@ -691,8 +689,8 @@ connection_or_finished_connecting(or_connection_t *or_conn) conn = TO_CONN(or_conn); tor_assert(conn->state == OR_CONN_STATE_CONNECTING); - log_debug(LD_HANDSHAKE,"OR connect() to router at %s:%u finished.", - conn->address,conn->port); + log_debug(LD_HANDSHAKE,"connect finished for %s", + connection_describe(conn)); if (proxy_type != PROXY_NONE) { /* start proxy handshake */ @@ -1010,9 +1008,10 @@ connection_or_single_set_badness_(time_t now, or_conn->base_.timestamp_created + TIME_BEFORE_OR_CONN_IS_TOO_OLD < now) { log_info(LD_OR, - "Marking OR conn to %s:%d as too old for new circuits " + "Marking %s as too old for new circuits " "(fd "TOR_SOCKET_T_FORMAT", %d secs old).", - or_conn->base_.address, or_conn->base_.port, or_conn->base_.s, + connection_describe(TO_CONN(or_conn)), + or_conn->base_.s, (int)(now - or_conn->base_.timestamp_created)); connection_or_mark_bad_for_new_circs(or_conn); } @@ -1077,10 +1076,11 @@ connection_or_group_set_badness_(smartlist_t *group, int force) /* We have at least one open canonical connection to this router, * and this one is open but not canonical. Mark it bad. */ log_info(LD_OR, - "Marking OR conn to %s:%d as unsuitable for new circuits: " + "Marking %s unsuitable for new circuits: " "(fd "TOR_SOCKET_T_FORMAT", %d secs old). It is not " "canonical, and we have another connection to that OR that is.", - or_conn->base_.address, or_conn->base_.port, or_conn->base_.s, + connection_describe(TO_CONN(or_conn)), + or_conn->base_.s, (int)(now - or_conn->base_.timestamp_created)); connection_or_mark_bad_for_new_circs(or_conn); continue; @@ -1121,22 +1121,24 @@ connection_or_group_set_badness_(smartlist_t *group, int force) /* This isn't the best conn, _and_ the best conn is better than it */ if (best->is_canonical) { log_info(LD_OR, - "Marking OR conn to %s:%d as unsuitable for new circuits: " + "Marking %s as unsuitable for new circuits: " "(fd "TOR_SOCKET_T_FORMAT", %d secs old). " "We have a better canonical one " "(fd "TOR_SOCKET_T_FORMAT"; %d secs old).", - or_conn->base_.address, or_conn->base_.port, or_conn->base_.s, + connection_describe(TO_CONN(or_conn)), + or_conn->base_.s, (int)(now - or_conn->base_.timestamp_created), best->base_.s, (int)(now - best->base_.timestamp_created)); connection_or_mark_bad_for_new_circs(or_conn); } else if (!tor_addr_compare(&or_conn->real_addr, &best->real_addr, CMP_EXACT)) { log_info(LD_OR, - "Marking OR conn to %s:%d as unsuitable for new circuits: " + "Marking %s unsuitable for new circuits: " "(fd "TOR_SOCKET_T_FORMAT", %d secs old). We have a better " "one with the " "same address (fd "TOR_SOCKET_T_FORMAT"; %d secs old).", - or_conn->base_.address, or_conn->base_.port, or_conn->base_.s, + connection_describe(TO_CONN(or_conn)), + or_conn->base_.s, (int)(now - or_conn->base_.timestamp_created), best->base_.s, (int)(now - best->base_.timestamp_created)); connection_or_mark_bad_for_new_circs(or_conn); @@ -1464,10 +1466,9 @@ connection_or_connect, (const tor_addr_t *_addr, uint16_t port, * that is we haven't had a failure earlier. This is to avoid to try to * constantly connect to relays that we think are not reachable. */ if (!should_connect_to_relay(conn)) { - log_info(LD_GENERAL, "Can't connect to identity %s at %s:%u because we " + log_info(LD_GENERAL, "Can't connect to %s because we " "failed earlier. Refusing.", - hex_str(id_digest, DIGEST_LEN), fmt_addr(&TO_CONN(conn)->addr), - TO_CONN(conn)->port); + connection_describe_peer(TO_CONN(conn))); connection_free_(TO_CONN(conn)); return NULL; } @@ -1507,7 +1508,7 @@ connection_or_connect, (const tor_addr_t *_addr, uint16_t port, "transport proxy supporting '%s'. This can happen if you " "haven't provided a ClientTransportPlugin line, or if " "your pluggable transport proxy stopped running.", - fmt_addrport(&TO_CONN(conn)->addr, TO_CONN(conn)->port), + connection_describe_peer(TO_CONN(conn)), transport_name, transport_name); control_event_bootstrap_prob_or( @@ -1516,9 +1517,9 @@ connection_or_connect, (const tor_addr_t *_addr, uint16_t port, conn); } else { - log_warn(LD_GENERAL, "Tried to connect to '%s' through a proxy, but " + log_warn(LD_GENERAL, "Tried to connect to %s through a proxy, but " "the proxy address could not be found.", - fmt_addrport(&TO_CONN(conn)->addr, TO_CONN(conn)->port)); + connection_describe_peer(TO_CONN(conn))); } connection_free_(TO_CONN(conn)); @@ -1891,9 +1892,9 @@ connection_or_client_learned_peer_id(or_connection_t *conn, const int expected_ed_key = ! ed25519_public_key_is_zero(&chan->ed25519_identity); - log_info(LD_HANDSHAKE, "learned peer id for %p (%s): %s, %s", + log_info(LD_HANDSHAKE, "learned peer id for %s at %p: %s, %s", + connection_describe(TO_CONN(conn)), conn, - safe_str_client(conn->base_.address), hex_str((const char*)rsa_peer_id, DIGEST_LEN), ed25519_fmt(ed_peer_id)); @@ -1907,9 +1908,9 @@ connection_or_client_learned_peer_id(or_connection_t *conn, conn->nickname[0] = '$'; base16_encode(conn->nickname+1, HEX_DIGEST_LEN+1, conn->identity_digest, DIGEST_LEN); - log_info(LD_HANDSHAKE, "Connected to router %s at %s:%d without knowing " - "its key. Hoping for the best.", - conn->nickname, conn->base_.address, conn->base_.port); + log_info(LD_HANDSHAKE, "Connected to router at %s without knowing " + "its key. Hoping for the best.", + connection_describe_peer(TO_CONN(conn))); /* if it's a bridge and we didn't know its identity fingerprint, now * we do -- remember it for future attempts. */ learned_router_identity(&conn->base_.addr, conn->base_.port, @@ -1983,9 +1984,9 @@ connection_or_client_learned_peer_id(or_connection_t *conn, } log_fn(severity, LD_HANDSHAKE, - "Tried connecting to router at %s:%d, but RSA + ed25519 identity " + "Tried connecting to router at %s, but RSA + ed25519 identity " "keys were not as expected: wanted %s + %s but got %s + %s.%s", - conn->base_.address, conn->base_.port, + connection_describe_peer(TO_CONN(conn)), expected_rsa, expected_ed, seen_rsa, seen_ed, extra_log); /* Tell the new guard API about the channel failure */ @@ -2057,11 +2058,10 @@ connection_tls_finish_handshake(or_connection_t *conn) tor_assert(!started_here); - log_debug(LD_HANDSHAKE,"%s tls handshake on %p with %s done, using " + log_debug(LD_HANDSHAKE,"%s tls handshake on %s done, using " "ciphersuite %s. verifying.", started_here?"outgoing":"incoming", - conn, - safe_str_client(conn->base_.address), + connection_describe_peer(TO_CONN(conn)), tor_tls_get_ciphersuite_name(conn->tls)); if (connection_or_check_valid_tls_handshake(conn, started_here, diff --git a/src/feature/dircache/dircache.c b/src/feature/dircache/dircache.c index 691c100974..efcae41084 100644 --- a/src/feature/dircache/dircache.c +++ b/src/feature/dircache/dircache.c @@ -1614,7 +1614,8 @@ directory_handle_command_post,(dir_connection_t *conn, const char *headers, if (!public_server_mode(options)) { log_info(LD_DIR, "Rejected dir post request from %s " - "since we're not a public relay.", conn->base_.address); + "since we're not a public relay.", + connection_describe_peer(TO_CONN(conn))); write_short_http_response(conn, 503, "Not acting as a public relay"); goto done; } @@ -1630,7 +1631,8 @@ directory_handle_command_post,(dir_connection_t *conn, const char *headers, !strcmpstart(url,"/tor/rendezvous2/publish")) { if (rend_cache_store_v2_desc_as_dir(body) < 0) { log_warn(LD_REND, "Rejected v2 rend descriptor (body size %d) from %s.", - (int)body_len, conn->base_.address); + (int)body_len, + connection_describe_peer(TO_CONN(conn))); write_short_http_response(conn, 400, "Invalid v2 service descriptor rejected"); } else { @@ -1686,7 +1688,8 @@ directory_handle_command_post,(dir_connection_t *conn, const char *headers, log_info(LD_DIRSERV, "Rejected router descriptor or extra-info from %s " "(\"%s\").", - conn->base_.address, msg); + connection_describe_peer(TO_CONN(conn)), + msg); write_short_http_response(conn, 400, msg); } goto done; @@ -1701,7 +1704,8 @@ directory_handle_command_post,(dir_connection_t *conn, const char *headers, } else { tor_assert(msg); log_warn(LD_DIRSERV, "Rejected vote from %s (\"%s\").", - conn->base_.address, msg); + connection_describe_peer(TO_CONN(conn)), + msg); write_short_http_response(conn, status, msg); } goto done; @@ -1714,7 +1718,8 @@ directory_handle_command_post,(dir_connection_t *conn, const char *headers, write_short_http_response(conn, 200, msg?msg:"Signatures stored"); } else { log_warn(LD_DIR, "Unable to store signatures posted by %s: %s", - conn->base_.address, msg?msg:"???"); + connection_describe_peer(TO_CONN(conn)), + msg?msg:"???"); write_short_http_response(conn, 400, msg?msg:"Unable to store signatures"); } @@ -1775,8 +1780,8 @@ directory_handle_command(dir_connection_t *conn) &body, &body_len, MAX_DIR_UL_SIZE, 0)) { case -1: /* overflow */ log_warn(LD_DIRSERV, - "Request too large from address '%s' to DirPort. Closing.", - safe_str(conn->base_.address)); + "Request too large from %s to DirPort. Closing.", + connection_describe_peer(TO_CONN(conn))); return -1; case 0: log_debug(LD_DIRSERV,"command not all here yet."); diff --git a/src/feature/dirclient/dirclient.c b/src/feature/dirclient/dirclient.c index 249618f081..bc4162cf52 100644 --- a/src/feature/dirclient/dirclient.c +++ b/src/feature/dirclient/dirclient.c @@ -740,8 +740,8 @@ connection_dir_client_request_failed(dir_connection_t *conn) if (conn->base_.purpose == DIR_PURPOSE_FETCH_SERVERDESC || conn->base_.purpose == DIR_PURPOSE_FETCH_EXTRAINFO) { log_info(LD_DIR, "Giving up on serverdesc/extrainfo fetch from " - "directory server at '%s'; retrying", - conn->base_.address); + "directory server at %s; retrying", + connection_describe_peer(TO_CONN(conn))); if (conn->router_purpose == ROUTER_PURPOSE_BRIDGE) connection_dir_bridge_routerdesc_failed(conn); connection_dir_download_routerdesc_failed(conn); @@ -750,18 +750,19 @@ connection_dir_client_request_failed(dir_connection_t *conn) networkstatus_consensus_download_failed(0, conn->requested_resource); } else if (conn->base_.purpose == DIR_PURPOSE_FETCH_CERTIFICATE) { log_info(LD_DIR, "Giving up on certificate fetch from directory server " - "at '%s'; retrying", - conn->base_.address); + "at %s; retrying", + connection_describe_peer(TO_CONN(conn))); connection_dir_download_cert_failed(conn, 0); } else if (conn->base_.purpose == DIR_PURPOSE_FETCH_DETACHED_SIGNATURES) { - log_info(LD_DIR, "Giving up downloading detached signatures from '%s'", - conn->base_.address); + log_info(LD_DIR, "Giving up downloading detached signatures from %s", + connection_describe_peer(TO_CONN(conn))); } else if (conn->base_.purpose == DIR_PURPOSE_FETCH_STATUS_VOTE) { - log_info(LD_DIR, "Giving up downloading votes from '%s'", - conn->base_.address); + log_info(LD_DIR, "Giving up downloading votes from %s", + connection_describe_peer(TO_CONN(conn))); } else if (conn->base_.purpose == DIR_PURPOSE_FETCH_MICRODESC) { log_info(LD_DIR, "Giving up on downloading microdescriptors from " - "directory server at '%s'; will retry", conn->base_.address); + "directory server at %s; will retry", + connection_describe_peer(TO_CONN(conn))); connection_dir_download_routerdesc_failed(conn); } } @@ -1754,10 +1755,10 @@ directory_send_command(dir_connection_t *conn, smartlist_free(headers); log_debug(LD_DIR, - "Sent request to directory server '%s:%d': " + "Sent request to directory server %s " "(purpose: %d, request size: %"TOR_PRIuSZ", " "payload size: %"TOR_PRIuSZ")", - conn->base_.address, conn->base_.port, + connection_describe_peer(TO_CONN(conn)), conn->base_.purpose, (total_request_len), (payload ? payload_len : 0)); @@ -1893,9 +1894,10 @@ dir_client_decompress_response_body(char **bodyp, size_t *bodylenp, } tor_log(severity, LD_HTTP, - "HTTP body from server '%s:%d' was labeled as %s, " + "HTTP body from %s was labeled as %s, " "%s it seems to be %s.%s", - conn->base_.address, conn->base_.port, description1, + connection_describe(TO_CONN(conn)), + description1, guessed != compression?"but":"and", description2, (compression>0 && guessed>0 && want_to_try_both)? @@ -1941,11 +1943,11 @@ dir_client_decompress_response_body(char **bodyp, size_t *bodylenp, * we didn't manage to uncompress it, then warn and bail. */ if (!plausible && !new_body) { log_fn(LOG_PROTOCOL_WARN, LD_HTTP, - "Unable to decompress HTTP body (tried %s%s%s, server '%s:%d').", + "Unable to decompress HTTP body (tried %s%s%s, on %s).", description1, tried_both?" and ":"", tried_both?description2:"", - conn->base_.address, conn->base_.port); + connection_describe(TO_CONN(conn))); rv = -1; goto done; } @@ -2052,8 +2054,8 @@ connection_dir_client_reached_eof(dir_connection_t *conn) allow_partial)) { case -1: /* overflow */ log_warn(LD_PROTOCOL, - "'fetch' response too large (server '%s:%d'). Closing.", - conn->base_.address, conn->base_.port); + "'fetch' response too large (%s). Closing.", + connection_describe(TO_CONN(conn))); return -1; case 0: log_info(LD_HTTP, @@ -2064,22 +2066,22 @@ connection_dir_client_reached_eof(dir_connection_t *conn) if (parse_http_response(headers, &status_code, &date_header, &compression, &reason) < 0) { - log_warn(LD_HTTP,"Unparseable headers (server '%s:%d'). Closing.", - conn->base_.address, conn->base_.port); - + log_warn(LD_HTTP,"Unparseable headers (%s). Closing.", + connection_describe(TO_CONN(conn))); rv = -1; goto done; } if (!reason) reason = tor_strdup("[no reason given]"); tor_log(LOG_DEBUG, LD_DIR, - "Received response from directory server '%s:%d': %d %s " + "Received response on %s: %d %s " "(purpose: %d, response size: %"TOR_PRIuSZ #ifdef MEASUREMENTS_21206 ", data cells received: %d, data cells sent: %d" #endif ", compression: %d)", - conn->base_.address, conn->base_.port, status_code, + connection_describe(TO_CONN(conn)), + status_code, escaped(reason), conn->base_.purpose, (received_bytes), #ifdef MEASUREMENTS_21206 @@ -2133,9 +2135,9 @@ connection_dir_client_reached_eof(dir_connection_t *conn) dir_server_t *ds; const char *id_digest = conn->identity_digest; log_info(LD_DIR,"Received http status code %d (%s) from server " - "'%s:%d'. I'll try again soon.", - status_code, escaped(reason), conn->base_.address, - conn->base_.port); + "%s. I'll try again soon.", + status_code, escaped(reason), + connection_describe_peer(TO_CONN(conn))); time_t now = approx_time(); if ((rs = router_get_mutable_consensus_status_by_id(id_digest))) rs->last_dir_503_at = now; @@ -2240,9 +2242,9 @@ handle_response_fetch_consensus(dir_connection_t *conn, int severity = (status_code == 304) ? LOG_INFO : LOG_WARN; tor_log(severity, LD_DIR, "Received http status code %d (%s) from server " - "'%s:%d' while fetching consensus directory.", - status_code, escaped(reason), conn->base_.address, - conn->base_.port); + "%s while fetching consensus directory.", + status_code, escaped(reason), + connection_describe_peer(TO_CONN(conn))); networkstatus_consensus_download_failed(status_code, flavname); return -1; } @@ -2277,21 +2279,21 @@ handle_response_fetch_consensus(dir_connection_t *conn, tor_munmap_file(mapped_consensus); if (new_consensus == NULL) { log_warn(LD_DIR, "Could not apply consensus diff received from server " - "'%s:%d'", conn->base_.address, conn->base_.port); + "%s", connection_describe_peer(TO_CONN(conn))); // XXXX If this happens too many times, we should maybe not use // XXXX this directory for diffs any more? networkstatus_consensus_download_failed(0, flavname); return -1; } log_info(LD_DIR, "Applied consensus diff (size %d) from server " - "'%s:%d', resulting in a new consensus document (size %d).", - (int)body_len, conn->base_.address, conn->base_.port, + "%s, resulting in a new consensus document (size %d).", + (int)body_len, connection_describe_peer(TO_CONN(conn)), (int)strlen(new_consensus)); consensus = new_consensus; sourcename = "generated based on a diff"; } else { log_info(LD_DIR,"Received consensus directory (body size %d) from server " - "'%s:%d'", (int)body_len, conn->base_.address, conn->base_.port); + "%s", (int)body_len, connection_describe_peer(TO_CONN(conn))); consensus = body; sourcename = "downloaded"; } @@ -2302,8 +2304,9 @@ handle_response_fetch_consensus(dir_connection_t *conn, conn->identity_digest))<0) { log_fn(r<-1?LOG_WARN:LOG_INFO, LD_DIR, "Unable to load %s consensus directory %s from " - "server '%s:%d'. I'll try again soon.", - flavname, sourcename, conn->base_.address, conn->base_.port); + "server %s. I'll try again soon.", + flavname, sourcename, + connection_describe_peer(TO_CONN(conn))); networkstatus_consensus_download_failed(0, flavname); tor_free(new_consensus); return -1; @@ -2344,15 +2347,16 @@ handle_response_fetch_certificate(dir_connection_t *conn, if (status_code != 200) { log_warn(LD_DIR, "Received http status code %d (%s) from server " - "'%s:%d' while fetching \"/tor/keys/%s\".", - status_code, escaped(reason), conn->base_.address, - conn->base_.port, conn->requested_resource); + "%s while fetching \"/tor/keys/%s\".", + status_code, escaped(reason), + connection_describe_peer(TO_CONN(conn)), + conn->requested_resource); connection_dir_download_cert_failed(conn, status_code); return -1; } log_info(LD_DIR,"Received authority certificates (body size %d) from " - "server '%s:%d'", - (int)body_len, conn->base_.address, conn->base_.port); + "server %s", + (int)body_len, connection_describe_peer(TO_CONN(conn))); /* * Tell trusted_dirs_load_certs_from_string() whether it was by fp @@ -2403,14 +2407,15 @@ handle_response_fetch_status_vote(dir_connection_t *conn, const char *msg; int st; - log_info(LD_DIR,"Got votes (body size %d) from server %s:%d", - (int)body_len, conn->base_.address, conn->base_.port); + log_info(LD_DIR,"Got votes (body size %d) from server %s", + (int)body_len, connection_describe_peer(TO_CONN(conn))); if (status_code != 200) { log_warn(LD_DIR, "Received http status code %d (%s) from server " - "'%s:%d' while fetching \"/tor/status-vote/next/%s.z\".", - status_code, escaped(reason), conn->base_.address, - conn->base_.port, conn->requested_resource); + "%s while fetching \"/tor/status-vote/next/%s.z\".", + status_code, escaped(reason), + connection_describe_peer(TO_CONN(conn)), + conn->requested_resource); return -1; } dirvote_add_vote(body, 0, &msg, &st); @@ -2438,19 +2443,21 @@ handle_response_fetch_detached_signatures(dir_connection_t *conn, const size_t body_len = args->body_len; const char *msg = NULL; - log_info(LD_DIR,"Got detached signatures (body size %d) from server %s:%d", - (int)body_len, conn->base_.address, conn->base_.port); + log_info(LD_DIR,"Got detached signatures (body size %d) from server %s", + (int)body_len, + connection_describe_peer(TO_CONN(conn))); if (status_code != 200) { log_warn(LD_DIR, - "Received http status code %d (%s) from server '%s:%d' while fetching " + "Received http status code %d (%s) from server %s while fetching " "\"/tor/status-vote/next/consensus-signatures.z\".", - status_code, escaped(reason), conn->base_.address, - conn->base_.port); + status_code, escaped(reason), + connection_describe_peer(TO_CONN(conn))); return -1; } if (dirvote_add_signatures(body, conn->base_.address, &msg)<0) { - log_warn(LD_DIR, "Problem adding detached signatures from %s:%d: %s", - conn->base_.address, conn->base_.port, msg?msg:"???"); + log_warn(LD_DIR, "Problem adding detached signatures from %s: %s", + connection_describe_peer(TO_CONN(conn)), + msg?msg:"???"); } return 0; @@ -2476,9 +2483,9 @@ handle_response_fetch_desc(dir_connection_t *conn, int n_asked_for = 0; int descriptor_digests = conn->requested_resource && !strcmpstart(conn->requested_resource,"d/"); - log_info(LD_DIR,"Received %s (body size %d) from server '%s:%d'", + log_info(LD_DIR,"Received %s (body size %d) from server %s", was_ei ? "extra server info" : "server info", - (int)body_len, conn->base_.address, conn->base_.port); + (int)body_len, connection_describe_peer(TO_CONN(conn))); if (conn->requested_resource && (!strcmpstart(conn->requested_resource,"d/") || !strcmpstart(conn->requested_resource,"fp/"))) { @@ -2494,10 +2501,11 @@ handle_response_fetch_desc(dir_connection_t *conn, /* 404 means that it didn't have them; no big deal. * Older (pre-0.1.1.8) servers said 400 Servers unavailable instead. */ log_fn(dir_okay ? LOG_INFO : LOG_WARN, LD_DIR, - "Received http status code %d (%s) from server '%s:%d' " + "Received http status code %d (%s) from server %s " "while fetching \"/tor/server/%s\". I'll try again soon.", - status_code, escaped(reason), conn->base_.address, - conn->base_.port, conn->requested_resource); + status_code, escaped(reason), + connection_describe_peer(TO_CONN(conn)), + conn->requested_resource); if (!which) { connection_dir_download_routerdesc_failed(conn); } else { @@ -2537,10 +2545,10 @@ handle_response_fetch_desc(dir_connection_t *conn, } } if (which) { /* mark remaining ones as failed */ - log_info(LD_DIR, "Received %d/%d %s requested from %s:%d", + log_info(LD_DIR, "Received %d/%d %s requested from %s", n_asked_for-smartlist_len(which), n_asked_for, was_ei ? "extra-info documents" : "router descriptors", - conn->base_.address, (int)conn->base_.port); + connection_describe_peer(TO_CONN(conn))); if (smartlist_len(which)) { dir_routerdesc_download_failed(which, status_code, conn->router_purpose, @@ -2571,9 +2579,9 @@ handle_response_fetch_microdesc(dir_connection_t *conn, smartlist_t *which = NULL; log_info(LD_DIR,"Received answer to microdescriptor request (status %d, " - "body size %d) from server '%s:%d'", - status_code, (int)body_len, conn->base_.address, - conn->base_.port); + "body size %d) from server %s", + status_code, (int)body_len, + connection_describe_peer(TO_CONN(conn))); tor_assert(conn->requested_resource && !strcmpstart(conn->requested_resource, "d/")); tor_assert_nonfatal(!fast_mem_is_zero(conn->identity_digest, DIGEST_LEN)); @@ -2583,10 +2591,11 @@ handle_response_fetch_microdesc(dir_connection_t *conn, DSR_DIGEST256|DSR_BASE64); if (status_code != 200) { log_info(LD_DIR, "Received status code %d (%s) from server " - "'%s:%d' while fetching \"/tor/micro/%s\". I'll try again " + "%s while fetching \"/tor/micro/%s\". I'll try again " "soon.", - status_code, escaped(reason), conn->base_.address, - (int)conn->base_.port, conn->requested_resource); + status_code, escaped(reason), + connection_describe_peer(TO_CONN(conn)), + conn->requested_resource); dir_microdesc_download_failed(which, status_code, conn->identity_digest); SMARTLIST_FOREACH(which, char *, cp, tor_free(cp)); smartlist_free(which); @@ -2661,8 +2670,8 @@ handle_response_upload_dir(dir_connection_t *conn, break; case 400: log_warn(LD_GENERAL,"http status 400 (%s) response from " - "dirserver '%s:%d'. Please correct.", - escaped(reason), conn->base_.address, conn->base_.port); + "dirserver %s. Please correct.", + escaped(reason), connection_describe_peer(TO_CONN(conn))); control_event_server_status(LOG_WARN, "BAD_SERVER_DESCRIPTOR DIRAUTH=%s:%d REASON=\"%s\"", conn->base_.address, conn->base_.port, escaped(reason)); @@ -2670,10 +2679,10 @@ handle_response_upload_dir(dir_connection_t *conn, default: log_warn(LD_GENERAL, "HTTP status %d (%s) was unexpected while uploading " - "descriptor to server '%s:%d'. Possibly the server is " + "descriptor to server %s'. Possibly the server is " "misconfigured?", - status_code, escaped(reason), conn->base_.address, - conn->base_.port); + status_code, escaped(reason), + connection_describe_peer(TO_CONN(conn))); break; } /* return 0 in all cases, since we don't want to mark any @@ -2696,21 +2705,21 @@ handle_response_upload_vote(dir_connection_t *conn, switch (status_code) { case 200: { - log_notice(LD_DIR,"Uploaded a vote to dirserver %s:%d", - conn->base_.address, conn->base_.port); + log_notice(LD_DIR,"Uploaded a vote to dirserver %s", + connection_describe_peer(TO_CONN(conn))); } break; case 400: log_warn(LD_DIR,"http status 400 (%s) response after uploading " - "vote to dirserver '%s:%d'. Please correct.", - escaped(reason), conn->base_.address, conn->base_.port); + "vote to dirserver %s. Please correct.", + escaped(reason), connection_describe_peer(TO_CONN(conn))); break; default: log_warn(LD_GENERAL, "HTTP status %d (%s) was unexpected while uploading " - "vote to server '%s:%d'.", - status_code, escaped(reason), conn->base_.address, - conn->base_.port); + "vote to server %s.", + status_code, escaped(reason), + connection_describe_peer(TO_CONN(conn))); break; } /* return 0 in all cases, since we don't want to mark any @@ -2732,21 +2741,21 @@ handle_response_upload_signatures(dir_connection_t *conn, switch (status_code) { case 200: { - log_notice(LD_DIR,"Uploaded signature(s) to dirserver %s:%d", - conn->base_.address, conn->base_.port); + log_notice(LD_DIR,"Uploaded signature(s) to dirserver %s", + connection_describe_peer(TO_CONN(conn))); } break; case 400: log_warn(LD_DIR,"http status 400 (%s) response after uploading " - "signatures to dirserver '%s:%d'. Please correct.", - escaped(reason), conn->base_.address, conn->base_.port); + "signatures to dirserver %s. Please correct.", + escaped(reason), connection_describe_peer(TO_CONN(conn))); break; default: log_warn(LD_GENERAL, "HTTP status %d (%s) was unexpected while uploading " - "signatures to server '%s:%d'.", - status_code, escaped(reason), conn->base_.address, - conn->base_.port); + "signatures to server %s.", + status_code, escaped(reason), + connection_describe_peer(TO_CONN(conn))); break; } /* return 0 in all cases, since we don't want to mark any @@ -2861,10 +2870,10 @@ handle_response_fetch_renddesc_v2(dir_connection_t *conn, default: log_warn(LD_REND, "Fetching v2 rendezvous descriptor failed: " "http status %d (%s) response unexpected while " - "fetching v2 hidden service descriptor (server '%s:%d'). " + "fetching v2 hidden service descriptor (server %s). " "Retrying at another directory.", - status_code, escaped(reason), conn->base_.address, - conn->base_.port); + status_code, escaped(reason), + connection_describe_peer(TO_CONN(conn))); SEND_HS_DESC_FAILED_EVENT("UNEXPECTED"); SEND_HS_DESC_FAILED_CONTENT(); break; @@ -2908,15 +2917,15 @@ handle_response_upload_renddesc_v2(dir_connection_t *conn, break; case 400: log_warn(LD_REND,"http status 400 (%s) response from dirserver " - "'%s:%d'. Malformed rendezvous descriptor?", - escaped(reason), conn->base_.address, conn->base_.port); + "%s. Malformed rendezvous descriptor?", + escaped(reason), connection_describe_peer(TO_CONN(conn))); SEND_HS_DESC_UPLOAD_FAILED_EVENT("UPLOAD_REJECTED"); break; default: log_warn(LD_REND,"http status %d (%s) response unexpected (server " - "'%s:%d').", - status_code, escaped(reason), conn->base_.address, - conn->base_.port); + "%s).", + status_code, escaped(reason), + connection_describe_peer(TO_CONN(conn))); SEND_HS_DESC_UPLOAD_FAILED_EVENT("UNEXPECTED"); break; } @@ -2954,17 +2963,17 @@ handle_response_upload_hsdesc(dir_connection_t *conn, log_fn(LOG_PROTOCOL_WARN, LD_REND, "Uploading hidden service descriptor: http " "status 400 (%s) response from dirserver " - "'%s:%d'. Malformed hidden service descriptor?", - escaped(reason), conn->base_.address, conn->base_.port); + "%s. Malformed hidden service descriptor?", + escaped(reason), connection_describe_peer(TO_CONN(conn))); hs_control_desc_event_failed(conn->hs_ident, conn->identity_digest, "UPLOAD_REJECTED"); break; default: log_warn(LD_REND, "Uploading hidden service descriptor: http " "status %d (%s) response unexpected (server " - "'%s:%d').", - status_code, escaped(reason), conn->base_.address, - conn->base_.port); + "%s').", + status_code, escaped(reason), + connection_describe_peer(TO_CONN(conn))); hs_control_desc_event_failed(conn->hs_ident, conn->identity_digest, "UNEXPECTED"); break; @@ -3116,7 +3125,7 @@ connection_dir_close_consensus_fetches(dir_connection_t *except_this_one, if (d == except_this_one) continue; log_info(LD_DIR, "Closing consensus fetch (to %s) since one " - "has just arrived.", TO_CONN(d)->address); + "has just arrived.", connection_describe_peer(TO_CONN(d))); connection_mark_for_close(TO_CONN(d)); } SMARTLIST_FOREACH_END(d); smartlist_free(conns_to_close); diff --git a/src/feature/dircommon/directory.c b/src/feature/dircommon/directory.c index b177fe5201..7f9f157a49 100644 --- a/src/feature/dircommon/directory.c +++ b/src/feature/dircommon/directory.c @@ -455,9 +455,9 @@ connection_dir_process_inbuf(dir_connection_t *conn) if (connection_get_inbuf_len(TO_CONN(conn)) > max_size) { log_warn(LD_HTTP, - "Too much data received from directory connection (%s): " + "Too much data received from %s: " "denial of service attempt, or you need to upgrade?", - conn->base_.address); + connection_describe(TO_CONN(conn))); connection_mark_for_close(TO_CONN(conn)); return -1; } @@ -540,8 +540,8 @@ connection_dir_finished_connecting(dir_connection_t *conn) tor_assert(conn->base_.type == CONN_TYPE_DIR); tor_assert(conn->base_.state == DIR_CONN_STATE_CONNECTING); - log_debug(LD_HTTP,"Dir connection to router %s:%u established.", - conn->base_.address,conn->base_.port); + log_debug(LD_HTTP,"Dir connection to %s established.", + connection_describe_peer(TO_CONN(conn))); /* start flushing conn */ conn->base_.state = DIR_CONN_STATE_CLIENT_SENDING; diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 09f0657fa8..23768213f3 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -1554,9 +1554,9 @@ client_dir_fetch_unexpected(dir_connection_t *dir_conn, const char *reason, log_warn(LD_REND, "Fetching v3 hidden service descriptor failed: " "http status %d (%s) response unexpected from HSDir " - "server '%s:%d'. Retrying at another directory.", - status_code, escaped(reason), TO_CONN(dir_conn)->address, - TO_CONN(dir_conn)->port); + "server %s'. Retrying at another directory.", + status_code, escaped(reason), + connection_describe_peer(TO_CONN(dir_conn))); /* Fire control port FAILED event. */ hs_control_desc_event_failed(dir_conn->hs_ident, dir_conn->identity_digest, "UNEXPECTED"); [View Less]

1 0

[tor/master] Minor refactoring on channel_get_remote_addr_if_possible()
by nickm＠torproject.org 16 Jul '20

16 Jul '20

commit 0ccdf05dc54bf956348b0647414732b6a497d17a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 14 13:53:27 2020 -0400 Minor refactoring on channel_get_remote_addr_if_possible() * We no longer call this an optional method * We document that it returns the real address, not a canonical one. * We have it try harder if the real address hasn't been set yet. --- src/core/or/channel.c | 14 ++++++++------ src/core/or/channel.h | 8 +++----- src/core/… [View More]or/channeltls.c | 32 +++++++++++++++++++++----------- src/test/test_channel.c | 11 ++++++++++- 4 files changed, 42 insertions(+), 23 deletions(-) diff --git a/src/core/or/channel.c b/src/core/or/channel.c index 20ccf41306..deb27a4d78 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -2835,22 +2835,24 @@ channel_get_canonical_remote_descr,(channel_t *chan)) } /** - * Get remote address if possible. + * Get the remote address for this channel, if possible. * * Write the remote address out to a tor_addr_t if the underlying transport * supports this operation, and return 1. Return 0 if the underlying transport * doesn't let us do this. + * + * Always returns the "real" address of the peer -- the one we're connected to + * on the internet. */ MOCK_IMPL(int, -channel_get_addr_if_possible,(const channel_t *chan, tor_addr_t *addr_out)) +channel_get_addr_if_possible,(const channel_t *chan, + tor_addr_t *addr_out)) { tor_assert(chan); tor_assert(addr_out); + tor_assert(chan->get_remote_addr); - if (chan->get_remote_addr) - return chan->get_remote_addr(chan, addr_out); - /* Else no support, method not implemented */ - else return 0; + return chan->get_remote_addr(chan, addr_out); } /** diff --git a/src/core/or/channel.h b/src/core/or/channel.h index 5fe1fb9cc4..9aa59da2cb 100644 --- a/src/core/or/channel.h +++ b/src/core/or/channel.h @@ -329,11 +329,9 @@ struct channel_t { */ double (*get_overhead_estimate)(channel_t *); /* - * Ask the underlying transport what the remote endpoint address is, in - * a tor_addr_t. This is optional and subclasses may leave this NULL. - * If they implement it, they should write the address out to the - * provided tor_addr_t *, and return 1 if successful or 0 if no address - * available. + * Ask the underlying transport what the remote endpoint address is, in a + * tor_addr_t. Write the address out to the provided tor_addr_t *, and + * return 1 if successful or 0 if no address available. */ int (*get_remote_addr)(const channel_t *, tor_addr_t *); int (*get_transport_name)(channel_t *chan, char **transport_out); diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c index e4dbdda7db..8273235efb 100644 --- a/src/core/or/channeltls.c +++ b/src/core/or/channeltls.c @@ -103,9 +103,8 @@ static void channel_tls_close_method(channel_t *chan); static const char * channel_tls_describe_transport_method(channel_t *chan); static void channel_tls_free_method(channel_t *chan); static double channel_tls_get_overhead_estimate_method(channel_t *chan); -static int -channel_tls_get_remote_addr_method(const channel_t *chan, - tor_addr_t *addr_out); +static int channel_tls_get_remote_addr_method(const channel_t *chan, + tor_addr_t *addr_out); static int channel_tls_get_transport_name_method(channel_t *chan, char **transport_out); static const char * @@ -512,24 +511,35 @@ channel_tls_get_overhead_estimate_method(channel_t *chan) * Get the remote address of a channel_tls_t. * * This implements the get_remote_addr method for channel_tls_t; copy the - * remote endpoint of the channel to addr_out and return 1 (always - * succeeds for this transport). + * remote endpoint of the channel to addr_out and return 1. (Always + * succeeds if this channel is attached to an OR connection.) + * + * Always returns the real address of the peer, not the canonical address. */ static int -channel_tls_get_remote_addr_method(const channel_t *chan, tor_addr_t *addr_out) +channel_tls_get_remote_addr_method(const channel_t *chan, + tor_addr_t *addr_out) { - int rv = 0; const channel_tls_t *tlschan = BASE_CHAN_TO_TLS((channel_t*) chan); tor_assert(tlschan); tor_assert(addr_out); - if (tlschan->conn) { + if (tlschan->conn == NULL) { + tor_addr_make_unspec(addr_out); + return 0; + } + + if (! tor_addr_is_null(&tlschan->conn->real_addr)) { + /* They want the real address, and real_addr is set. */ tor_addr_copy(addr_out, &(tlschan->conn->real_addr)); - rv = 1; - } else tor_addr_make_unspec(addr_out); + } else { + /* We'll have to give them the nominal address, which hopefully has + * not been overwritten yet. */ + tor_addr_copy(addr_out, &TO_CONN(tlschan->conn)->addr); + } - return rv; + return 1; } /** diff --git a/src/test/test_channel.c b/src/test/test_channel.c index 2b723b4a8d..efe195eac4 100644 --- a/src/test/test_channel.c +++ b/src/test/test_channel.c @@ -166,12 +166,20 @@ static const char * chan_test_get_remote_descr(channel_t *ch, int flags) { tt_assert(ch); - tt_int_op(flags & ~(GRD_FLAG_ORIGINAL | GRD_FLAG_ADDR_ONLY), OP_EQ, 0); + tt_int_op(flags & ~(GRD_FLAG_ORIGINAL), OP_EQ, 0); done: return "Fake channel for unit tests; no real endpoint"; } +static int +chan_test_get_remote_addr(const channel_t *ch, tor_addr_t *out) +{ + (void)ch; + tor_addr_from_ipv4h(out, 0x7f000001); + return 1; +} + static int chan_test_num_cells_writeable(channel_t *ch) { @@ -269,6 +277,7 @@ new_fake_channel(void) chan->close = chan_test_close; chan->num_cells_writeable = chan_test_num_cells_writeable; chan->get_remote_descr = chan_test_get_remote_descr; + chan->get_remote_addr = chan_test_get_remote_addr; chan->write_packed_cell = chan_test_write_packed_cell; chan->write_var_cell = chan_test_write_var_cell; chan->state = CHANNEL_STATE_OPEN; [View Less]

1 0

[tor/master] channelpadding: don't call method pointer directly.
by nickm＠torproject.org 16 Jul '20

16 Jul '20

commit f9aafcd64e69d602eb4284e530fe9b8973cf7406 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 14 14:12:02 2020 -0400 channelpadding: don't call method pointer directly. --- src/core/or/channelpadding.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/or/channelpadding.c b/src/core/or/channelpadding.c index c754a58c42..0e88b8b3bd 100644 --- a/src/core/or/channelpadding.c +++ b/src/core/or/channelpadding.c @@ -265,7 +265,7 @@ … [View More]

1 0

[tor/master] Refactor channel description internals.
by nickm＠torproject.org 16 Jul '20

16 Jul '20

commit 4f4785a8c113806d00c6a31154956edb86c1639f Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 14 14:26:24 2020 -0400 Refactor channel description internals. Now that we've clarified that these functions only need to describe the peer in a human-readable way, we can have them delegate to connection_describe_peer(). --- src/core/or/channel.c | 19 ++++++------------ src/core/or/channel.h | 8 +++----- src/core/or/channeltls.c | 51 +++++++++++--… [View More]----------------------------------- src/test/test_channel.c | 5 ++--- 4 files changed, 22 insertions(+), 61 deletions(-) diff --git a/src/core/or/channel.c b/src/core/or/channel.c index 5ea38256ac..2de164b39c 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -2794,31 +2794,24 @@ channel_listener_dump_transport_statistics(channel_listener_t *chan_l, const char * channel_get_actual_remote_descr(channel_t *chan) { - tor_assert(chan); - tor_assert(chan->get_remote_descr); - - /* Param 1 indicates the actual description */ - return chan->get_remote_descr(chan, GRD_FLAG_ORIGINAL); + return channel_get_canonical_remote_descr(chan); } /** * Return text description of the remote endpoint canonical address. * - * This function return a test provided by the lower layer of the remote - * endpoint for this channel; it should use the known canonical address for - * this OR's identity digest if possible. + * This function returns a human-readable string for logging; nothing + * should parse it or rely on a particular format. * - * Subsequent calls to channel_get_{actual,canonical}_remote_{address,descr} - * may invalidate the return value from this function. + * Subsequent calls to this function may invalidate its return value. */ MOCK_IMPL(const char *, channel_get_canonical_remote_descr,(channel_t *chan)) { tor_assert(chan); - tor_assert(chan->get_remote_descr); + tor_assert(chan->describe_peer); - /* Param 0 indicates the canonicalized description */ - return chan->get_remote_descr(chan, 0); + return chan->describe_peer(chan); } /** diff --git a/src/core/or/channel.h b/src/core/or/channel.h index f047d24f66..917941a0b6 100644 --- a/src/core/or/channel.h +++ b/src/core/or/channel.h @@ -336,13 +336,11 @@ struct channel_t { int (*get_remote_addr)(const channel_t *, tor_addr_t *); int (*get_transport_name)(channel_t *chan, char **transport_out); -#define GRD_FLAG_ORIGINAL 1 /** - * Get a text description of the remote endpoint; canonicalized if the flag - * GRD_FLAG_ORIGINAL is not set, or the one we originally connected - * to/received from if it is. + * Get a human-readable text description of the remote endpoint, for + * logging. */ - const char * (*get_remote_descr)(channel_t *, int); + const char * (*describe_peer)(const channel_t *); /** Check if the lower layer has queued writes */ int (*has_queued_writes)(channel_t *); /** diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c index 3a10b2c27d..ade26684b8 100644 --- a/src/core/or/channeltls.c +++ b/src/core/or/channeltls.c @@ -107,8 +107,7 @@ static int channel_tls_get_remote_addr_method(const channel_t *chan, tor_addr_t *addr_out); static int channel_tls_get_transport_name_method(channel_t *chan, char **transport_out); -static const char * -channel_tls_get_remote_descr_method(channel_t *chan, int flags); +static const char *channel_tls_describe_peer_method(const channel_t *chan); static int channel_tls_has_queued_writes_method(channel_t *chan); static int channel_tls_is_canonical_method(channel_t *chan, int req); static int @@ -164,7 +163,7 @@ channel_tls_common_init(channel_tls_t *tlschan) chan->free_fn = channel_tls_free_method; chan->get_overhead_estimate = channel_tls_get_overhead_estimate_method; chan->get_remote_addr = channel_tls_get_remote_addr_method; - chan->get_remote_descr = channel_tls_get_remote_descr_method; + chan->describe_peer = channel_tls_describe_peer_method; chan->get_transport_name = channel_tls_get_transport_name_method; chan->has_queued_writes = channel_tls_has_queued_writes_method; chan->is_canonical = channel_tls_is_canonical_method; @@ -567,50 +566,22 @@ channel_tls_get_transport_name_method(channel_t *chan, char **transport_out) } /** - * Get endpoint description of a channel_tls_t. + * Get a human-readable endpoint description of a channel_tls_t. * - * This implements the get_remote_descr method for channel_tls_t; it returns - * a text description of the remote endpoint of the channel suitable for use - * in log messages. The req parameter is 0 for the canonical address or 1 for - * the actual address seen. + * This format is intended for logging, and may change in the future; + * nothing should parse or rely on its particular details. */ static const char * -channel_tls_get_remote_descr_method(channel_t *chan, int flags) +channel_tls_describe_peer_method(const channel_t *chan) { - static char buf[TOR_ADDRPORT_BUF_LEN]; - channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan); - connection_t *conn; - const char *answer = NULL; - char *addr_str; - + const channel_tls_t *tlschan = BASE_CHAN_TO_TLS((channel_t*)chan); tor_assert(tlschan); if (tlschan->conn) { - conn = TO_CONN(tlschan->conn); - switch (flags) { - case 0: - /* Canonical address with port*/ - tor_snprintf(buf, TOR_ADDRPORT_BUF_LEN, - "%s:%u", conn->address, conn->port); - answer = buf; - break; - case GRD_FLAG_ORIGINAL: - /* Actual address with port */ - addr_str = tor_addr_to_str_dup(&(tlschan->conn->real_addr)); - tor_snprintf(buf, TOR_ADDRPORT_BUF_LEN, "%s:%u", addr_str, conn->port); - tor_free(addr_str); - answer = buf; - break; - default: - /* Something's broken in channel.c */ - tor_assert_nonfatal_unreached_once(); - } + return connection_describe_peer(TO_CONN(tlschan->conn)); } else { - strlcpy(buf, "(No connection)", sizeof(buf)); - answer = buf; + return "(No connection)"; } - - return answer; } /** @@ -1903,8 +1874,8 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan) if (me && !TLS_CHAN_TO_BASE(chan)->is_canonical_to_peer && channel_is_canonical(TLS_CHAN_TO_BASE(chan))) { - const char *descr = - TLS_CHAN_TO_BASE(chan)->get_remote_descr(TLS_CHAN_TO_BASE(chan), 0); + const char *descr = channel_get_actual_remote_descr( + TLS_CHAN_TO_BASE(chan)); log_info(LD_OR, "We made a connection to a relay at %s (fp=%s) but we think " "they will not consider this connection canonical. They " diff --git a/src/test/test_channel.c b/src/test/test_channel.c index efe195eac4..14d8a4eae7 100644 --- a/src/test/test_channel.c +++ b/src/test/test_channel.c @@ -163,10 +163,9 @@ chan_test_finish_close(channel_t *ch) } static const char * -chan_test_get_remote_descr(channel_t *ch, int flags) +chan_test_describe_peer(const channel_t *ch) { tt_assert(ch); - tt_int_op(flags & ~(GRD_FLAG_ORIGINAL), OP_EQ, 0); done: return "Fake channel for unit tests; no real endpoint"; @@ -276,7 +275,7 @@ new_fake_channel(void) chan->close = chan_test_close; chan->num_cells_writeable = chan_test_num_cells_writeable; - chan->get_remote_descr = chan_test_get_remote_descr; + chan->describe_peer = chan_test_describe_peer; chan->get_remote_addr = chan_test_get_remote_addr; chan->write_packed_cell = chan_test_write_packed_cell; chan->write_var_cell = chan_test_write_var_cell; [View Less]

1 0

[tor/master] Remove "ADDR_ONLY" mode from channel_get_*_remote_descr.
by nickm＠torproject.org 16 Jul '20

16 Jul '20

commit db21e91f4bda5860dea2d817c1f707f57119abee Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 14 14:08:55 2020 -0400 Remove "ADDR_ONLY" mode from channel_get_*_remote_descr. This mode was only used in one place, and it caused a dangerous mingling of functionality. The method is supposed to _describe_ the peer's address, not give its actual address. We already had a function to get the actual address. --- src/core/or/channel.c | 13 ----… [View More]--------- src/core/or/channel.h | 5 +---- src/core/or/channeltls.c | 12 ------------ src/core/or/connection_edge.c | 9 ++++++--- 4 files changed, 7 insertions(+), 32 deletions(-) diff --git a/src/core/or/channel.c b/src/core/or/channel.c index deb27a4d78..5ea38256ac 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -2801,19 +2801,6 @@ channel_get_actual_remote_descr(channel_t *chan) return chan->get_remote_descr(chan, GRD_FLAG_ORIGINAL); } -/** - * Return the text address of the remote endpoint. - * - * Subsequent calls to channel_get_{actual,canonical}_remote_{address,descr} - * may invalidate the return value from this function. - */ -const char * -channel_get_actual_remote_address(channel_t *chan) -{ - /* Param 1 indicates the actual description */ - return chan->get_remote_descr(chan, GRD_FLAG_ORIGINAL|GRD_FLAG_ADDR_ONLY); -} - /** * Return text description of the remote endpoint canonical address. * diff --git a/src/core/or/channel.h b/src/core/or/channel.h index 9aa59da2cb..f047d24f66 100644 --- a/src/core/or/channel.h +++ b/src/core/or/channel.h @@ -337,12 +337,10 @@ struct channel_t { int (*get_transport_name)(channel_t *chan, char **transport_out); #define GRD_FLAG_ORIGINAL 1 -#define GRD_FLAG_ADDR_ONLY 2 /** * Get a text description of the remote endpoint; canonicalized if the flag * GRD_FLAG_ORIGINAL is not set, or the one we originally connected - * to/received from if it is. If GRD_FLAG_ADDR_ONLY is set, we return only - * the original address. + * to/received from if it is. */ const char * (*get_remote_descr)(channel_t *, int); /** Check if the lower layer has queued writes */ @@ -723,7 +721,6 @@ const char * channel_describe_transport(channel_t *chan); MOCK_DECL(void, channel_dump_statistics, (channel_t *chan, int severity)); void channel_dump_transport_statistics(channel_t *chan, int severity); const char * channel_get_actual_remote_descr(channel_t *chan); -const char * channel_get_actual_remote_address(channel_t *chan); MOCK_DECL(int, channel_get_addr_if_possible, (const channel_t *chan, tor_addr_t *addr_out)); MOCK_DECL(const char *, channel_get_canonical_remote_descr,(channel_t *chan)); diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c index 8273235efb..3a10b2c27d 100644 --- a/src/core/or/channeltls.c +++ b/src/core/or/channeltls.c @@ -601,18 +601,6 @@ channel_tls_get_remote_descr_method(channel_t *chan, int flags) tor_free(addr_str); answer = buf; break; - case GRD_FLAG_ADDR_ONLY: - /* Canonical address, no port */ - strlcpy(buf, conn->address, sizeof(buf)); - answer = buf; - break; - case GRD_FLAG_ORIGINAL|GRD_FLAG_ADDR_ONLY: - /* Actual address, no port */ - addr_str = tor_addr_to_str_dup(&(tlschan->conn->real_addr)); - strlcpy(buf, addr_str, sizeof(buf)); - tor_free(addr_str); - answer = buf; - break; default: /* Something's broken in channel.c */ tor_assert_nonfatal_unreached_once(); diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c index cd4ec8be7b..2798d1bf37 100644 --- a/src/core/or/connection_edge.c +++ b/src/core/or/connection_edge.c @@ -3996,10 +3996,13 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ) * caller might want to know whether the remote IP address has changed, * and we might already have corrected base_.addr[ess] for the relay's * canonical IP address. */ - if (or_circ && or_circ->p_chan) - address = tor_strdup(channel_get_actual_remote_address(or_circ->p_chan)); - else + tor_addr_t chan_addr; + if (or_circ && or_circ->p_chan && + channel_get_addr_if_possible(or_circ->p_chan, &chan_addr)) { + address = tor_addr_to_str_dup(&chan_addr); + } else { address = tor_strdup("127.0.0.1"); + } port = 1; /* XXXX This value is never actually used anywhere, and there * isn't "really" a connection here. But we * need to set it to something nonzero. */ [View Less]

1 0

[tor/master] Collapse channel_get_*_remote_addr() into a single function.
by nickm＠torproject.org 16 Jul '20

16 Jul '20

commit ab428ef60b701075158f1c06676bcc394f0969b2 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 14 14:28:33 2020 -0400 Collapse channel_get_*_remote_addr() into a single function. Since we can list the real address and the canonical one in a human-readable format we don't need to pick. --- src/core/or/channel.c | 26 +++++--------------------- src/core/or/channel.h | 3 +-- src/core/or/channelpadding.c | 4 ++-… [View More]- src/core/or/channeltls.c | 2 +- src/core/or/circuitbuild.c | 2 +- src/core/or/circuitlist.c | 2 +- src/core/or/circuituse.c | 4 ++-- src/core/or/command.c | 12 ++++++------ src/core/or/connection_edge.c | 2 +- src/feature/relay/circuitbuild_relay.c | 2 +- src/test/test_circuitbuild.c | 4 ++-- 11 files changed, 23 insertions(+), 40 deletions(-) diff --git a/src/core/or/channel.c b/src/core/or/channel.c index 2de164b39c..8390fc1ecc 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -2553,7 +2553,7 @@ channel_dump_statistics, (channel_t *chan, int severity)) /* Handle remote address and descriptions */ have_remote_addr = channel_get_addr_if_possible(chan, &remote_addr); if (have_remote_addr) { - char *actual = tor_strdup(channel_get_actual_remote_descr(chan)); + char *actual = tor_strdup(channel_describe_peer(chan)); remote_addr_str = tor_addr_to_str_dup(&remote_addr); tor_log(severity, LD_GENERAL, " * Channel %"PRIu64 " says its remote address" @@ -2561,18 +2561,18 @@ channel_dump_statistics, (channel_t *chan, int severity)) "actual description of \"%s\"", (chan->global_identifier), safe_str(remote_addr_str), - safe_str(channel_get_canonical_remote_descr(chan)), + safe_str(channel_describe_peer(chan)), safe_str(actual)); tor_free(remote_addr_str); tor_free(actual); } else { - char *actual = tor_strdup(channel_get_actual_remote_descr(chan)); + char *actual = tor_strdup(channel_describe_peer(chan)); tor_log(severity, LD_GENERAL, " * Channel %"PRIu64 " does not know its remote " "address, but gives a canonical description of \"%s\" and an " "actual description of \"%s\"", (chan->global_identifier), - channel_get_canonical_remote_descr(chan), + channel_describe_peer(chan), actual); tor_free(actual); } @@ -2781,22 +2781,6 @@ channel_listener_dump_transport_statistics(channel_listener_t *chan_l, if (chan_l->dumpstats) chan_l->dumpstats(chan_l, severity); } -/** - * Return text description of the remote endpoint. - * - * This function return a test provided by the lower layer of the remote - * endpoint for this channel; it should specify the actual address connected - * to/from. - * - * Subsequent calls to channel_get_{actual,canonical}_remote_{address,descr} - * may invalidate the return value from this function. - */ -const char * -channel_get_actual_remote_descr(channel_t *chan) -{ - return channel_get_canonical_remote_descr(chan); -} - /** * Return text description of the remote endpoint canonical address. * @@ -2806,7 +2790,7 @@ channel_get_actual_remote_descr(channel_t *chan) * Subsequent calls to this function may invalidate its return value. */ MOCK_IMPL(const char *, -channel_get_canonical_remote_descr,(channel_t *chan)) +channel_describe_peer,(channel_t *chan)) { tor_assert(chan); tor_assert(chan->describe_peer); diff --git a/src/core/or/channel.h b/src/core/or/channel.h index 917941a0b6..d52ebdf619 100644 --- a/src/core/or/channel.h +++ b/src/core/or/channel.h @@ -718,10 +718,9 @@ channel_is_in_state(channel_t *chan, channel_state_t state) const char * channel_describe_transport(channel_t *chan); MOCK_DECL(void, channel_dump_statistics, (channel_t *chan, int severity)); void channel_dump_transport_statistics(channel_t *chan, int severity); -const char * channel_get_actual_remote_descr(channel_t *chan); MOCK_DECL(int, channel_get_addr_if_possible, (const channel_t *chan, tor_addr_t *addr_out)); -MOCK_DECL(const char *, channel_get_canonical_remote_descr,(channel_t *chan)); +MOCK_DECL(const char *, channel_describe_peer,(channel_t *chan)); int channel_has_queued_writes(channel_t *chan); int channel_is_bad_for_new_circs(channel_t *chan); void channel_mark_bad_for_new_circs(channel_t *chan); diff --git a/src/core/or/channelpadding.c b/src/core/or/channelpadding.c index 0e88b8b3bd..d0c43e8bdc 100644 --- a/src/core/or/channelpadding.c +++ b/src/core/or/channelpadding.c @@ -265,7 +265,7 @@ channelpadding_update_padding_for_channel(channel_t *chan, log_fn_ratelim(&relay_limit,LOG_PROTOCOL_WARN,LD_PROTOCOL, "Got a PADDING_NEGOTIATE from relay at %s (%s). " "This should not happen.", - channel_get_canonical_remote_descr(chan), + channel_describe_peer(chan), hex_str(chan->identity_digest, DIGEST_LEN)); return -1; } @@ -399,7 +399,7 @@ channelpadding_send_padding_cell_for_callback(channel_t *chan) "Sending netflow keepalive on %"PRIu64" to %s (%s) after " "%"PRId64" ms. Delta %"PRId64"ms", (chan->global_identifier), - safe_str_client(channel_get_canonical_remote_descr(chan)), + safe_str_client(channel_describe_peer(chan)), safe_str_client(hex_str(chan->identity_digest, DIGEST_LEN)), (monotime_coarse_diff_msec(&chan->timestamp_xfer,&now)), ( diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c index ade26684b8..082a035c3c 100644 --- a/src/core/or/channeltls.c +++ b/src/core/or/channeltls.c @@ -1874,7 +1874,7 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan) if (me && !TLS_CHAN_TO_BASE(chan)->is_canonical_to_peer && channel_is_canonical(TLS_CHAN_TO_BASE(chan))) { - const char *descr = channel_get_actual_remote_descr( + const char *descr = channel_describe_peer( TLS_CHAN_TO_BASE(chan)); log_info(LD_OR, "We made a connection to a relay at %s (fp=%s) but we think " diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index fd56ca1c65..6ad19eebad 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -632,7 +632,7 @@ circuit_n_chan_done(channel_t *chan, int status, int close_origin_circuits) tor_assert(chan); log_debug(LD_CIRC,"chan to %s, status=%d", - channel_get_canonical_remote_descr(chan), status); + channel_describe_peer(chan), status); pending_circs = smartlist_new(); circuit_get_all_pending_on_channel(pending_circs, chan); diff --git a/src/core/or/circuitlist.c b/src/core/or/circuitlist.c index 2455aa8779..af98af362a 100644 --- a/src/core/or/circuitlist.c +++ b/src/core/or/circuitlist.c @@ -619,7 +619,7 @@ circuit_count_pending_on_channel(channel_t *chan) cnt = smartlist_len(sl); smartlist_free(sl); log_debug(LD_CIRC,"or_conn to %s, %d pending circs", - channel_get_canonical_remote_descr(chan), + channel_describe_peer(chan), cnt); return cnt; } diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c index d2bdf77d8d..3c8a7c4a89 100644 --- a/src/core/or/circuituse.c +++ b/src/core/or/circuituse.c @@ -819,7 +819,7 @@ circuit_expire_building(void) log_info(LD_CIRC, "Abandoning circ %u %s:%u (state %d,%d:%s, purpose %d, " "len %d)", TO_ORIGIN_CIRCUIT(victim)->global_identifier, - channel_get_canonical_remote_descr(victim->n_chan), + channel_describe_peer(victim->n_chan), (unsigned)victim->n_circ_id, TO_ORIGIN_CIRCUIT(victim)->has_opened, victim->state, circuit_state_to_string(victim->state), @@ -1857,7 +1857,7 @@ circuit_build_failed(origin_circuit_t *circ) "from the first hop (%s). I'm going to try to rotate to a " "better connection.", TO_CIRCUIT(circ)->n_circ_id, circ->global_identifier, - channel_get_canonical_remote_descr(n_chan)); + channel_describe_peer(n_chan)); n_chan->is_bad_for_new_circs = 1; } else { log_info(LD_OR, diff --git a/src/core/or/command.c b/src/core/or/command.c index 8a1d2066cc..9226309ff7 100644 --- a/src/core/or/command.c +++ b/src/core/or/command.c @@ -252,7 +252,7 @@ command_process_create_cell(cell_t *cell, channel_t *chan) log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Received a create cell (type %d) from %s with zero circID; " " ignoring.", (int)cell->command, - channel_get_actual_remote_descr(chan)); + channel_describe_peer(chan)); return; } @@ -295,7 +295,7 @@ command_process_create_cell(cell_t *cell, channel_t *chan) "Received create cell (type %d) from %s, but we're connected " "to it as a client. " "Sending back a destroy.", - (int)cell->command, channel_get_canonical_remote_descr(chan)); + (int)cell->command, channel_describe_peer(chan)); channel_send_destroy(cell->circ_id, chan, END_CIRC_REASON_TORPROTOCOL); return; @@ -475,7 +475,7 @@ command_process_relay_cell(cell_t *cell, channel_t *chan) log_debug(LD_OR, "unknown circuit %u on connection from %s. Dropping.", (unsigned)cell->circ_id, - channel_get_canonical_remote_descr(chan)); + channel_describe_peer(chan)); return; } @@ -536,7 +536,7 @@ command_process_relay_cell(cell_t *cell, channel_t *chan) control_event_circ_bandwidth_used_for_circ(TO_ORIGIN_CIRCUIT(circ)); } else if (circ->n_chan) { log_warn(LD_OR, " upstream=%s", - channel_get_actual_remote_descr(circ->n_chan)); + channel_describe_peer(circ->n_chan)); } circuit_mark_for_close(circ, END_CIRC_REASON_TORPROTOCOL); return; @@ -547,7 +547,7 @@ command_process_relay_cell(cell_t *cell, channel_t *chan) "Received too many RELAY_EARLY cells on circ %u from %s." " Closing circuit.", (unsigned)cell->circ_id, - safe_str(channel_get_canonical_remote_descr(chan))); + safe_str(channel_describe_peer(chan))); circuit_mark_for_close(circ, END_CIRC_REASON_TORPROTOCOL); return; } @@ -618,7 +618,7 @@ command_process_destroy_cell(cell_t *cell, channel_t *chan) if (!circ) { log_info(LD_OR,"unknown circuit %u on connection from %s. Dropping.", (unsigned)cell->circ_id, - channel_get_canonical_remote_descr(chan)); + channel_describe_peer(chan)); return; } log_debug(LD_OR,"Received for circID %u.",(unsigned)cell->circ_id); diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c index 2798d1bf37..8eb9f8ba0f 100644 --- a/src/core/or/connection_edge.c +++ b/src/core/or/connection_edge.c @@ -3973,7 +3973,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ) * proxies. */ log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Attempt by %s to open a stream %s. Closing.", - safe_str(channel_get_canonical_remote_descr(or_circ->p_chan)), + safe_str(channel_describe_peer(or_circ->p_chan)), client_chan ? "on first hop of circuit" : "from unknown relay"); relay_send_end_cell_from_edge(rh.stream_id, circ, diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c index 881cbd51be..ad20e143be 100644 --- a/src/feature/relay/circuitbuild_relay.c +++ b/src/feature/relay/circuitbuild_relay.c @@ -501,7 +501,7 @@ circuit_extend(struct cell_t *cell, struct circuit_t *circ) circ->n_chan = n_chan; log_debug(LD_CIRC, "n_chan is %s.", - channel_get_canonical_remote_descr(n_chan)); + channel_describe_peer(n_chan)); if (circuit_deliver_create_cell(circ, &ec.create_cell, 1) < 0) return -1; diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index 66751bc5aa..74824a1bc1 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -1286,7 +1286,7 @@ test_circuit_extend(void *arg) MOCK(server_mode, mock_server_mode); /* Mock a debug function, but otherwise ignore it */ - MOCK(channel_get_canonical_remote_descr, + MOCK(channel_describe_peer, mock_channel_get_canonical_remote_descr); setup_full_capture_of_logs(LOG_INFO); @@ -1479,7 +1479,7 @@ test_circuit_extend(void *arg) UNMOCK(server_mode); server = 0; - UNMOCK(channel_get_canonical_remote_descr); + UNMOCK(channel_describe_peer); UNMOCK(extend_cell_parse); memset(&mock_extend_cell_parse_cell_out, 0, [View Less]

1 0

[tor/master] Improvements to output format for OR connection descriptions.
by nickm＠torproject.org 16 Jul '20

16 Jul '20

commit 9b5567df4eee09ab23dce3dec18801c57e39388b Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jul 15 16:56:37 2020 -0400 Improvements to output format for OR connection descriptions. Only log the 'real_addr' when it is set to something. Only log the ID when it is set. When scrubbing the address, don't include a canonical address. (There should never be a canonical address for a connection with no ID set.) --- src/core/mainloop/… [View More]

1 0