From fecd583c0e7e580f07d81b24b1a3522c52938c61 Mon Sep 17 00:00:00 2001 From: cypherpunks Date: Sat, 18 Aug 2018 19:18:48 +0000 Subject: [PATCH 1/8] rust: abort on panic in all profiles Until https://github.com/rust-lang/rust/issues/52652 is fixed, unwinding on panic is potentially unsound in a mixed C/Rust codebase. The codebase is supposed to be panic-free already, but just to be safe. This started mattering at commit d1820c1516a31a149fc51a9e5126bf899e4c4e08. Fixes #27199; bugfix on tor-0.3.3.1-alpha. --- changes/bug27199 | 3 +++ src/rust/Cargo.toml | 10 ++++++++++ 2 files changed, 13 insertions(+) create mode 100644 changes/bug27199 diff --git a/changes/bug27199 b/changes/bug27199 new file mode 100644 index 0000000000..f9d2a422f9 --- /dev/null +++ b/changes/bug27199 @@ -0,0 +1,3 @@ + o Minor bugfixes (rust): + - Abort on panic in all build profiles, instead of potentially unwinding + into C code. Fixes bug 27199; bugfix on 0.3.3.1-alpha. diff --git a/src/rust/Cargo.toml b/src/rust/Cargo.toml index 953c9b96b7..c22eea984d 100644 --- a/src/rust/Cargo.toml +++ b/src/rust/Cargo.toml @@ -1,7 +1,17 @@ [workspace] members = ["tor_util", "protover", "smartlist", "external", "tor_allocate", "tor_rust"] +# Can remove panic="abort" when this issue is fixed: +# https://github.com/rust-lang/rust/issues/52652 +[profile.dev] +panic = "abort" + [profile.release] debug = true panic = "abort" +[profile.test] +panic = "abort" + +[profile.bench] +panic = "abort" From 85c598cbc2d4ca1a7c729864002b5b222980ce19 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 8 Jan 2019 18:21:10 +1000 Subject: [PATCH 2/8] stats: Make PaddingStatistics depend on ExtraInfoStatistics When ExtraInfoStatistics is 0, stop including PaddingStatistics in relay and bridge extra-info documents. Fixes bug 29017; bugfix on 0.3.1.1-alpha. --- changes/bug29017 | 4 ++++ doc/tor.1.txt | 2 +- src/or/router.c | 11 +++++------ 3 files changed, 10 insertions(+), 7 deletions(-) create mode 100644 changes/bug29017 diff --git a/changes/bug29017 b/changes/bug29017 new file mode 100644 index 0000000000..5c4a53c43f --- /dev/null +++ b/changes/bug29017 @@ -0,0 +1,4 @@ + o Minor bugfixes (stats): + - When ExtraInfoStatistics is 0, stop including PaddingStatistics in + relay and bridge extra-info documents. Fixes bug 29017; + bugfix on 0.3.1.1-alpha. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index c089bffbb0..790ac6f6ae 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -2266,7 +2266,7 @@ is non-zero): extra-info document. (Default: 0) [[PaddingStatistics]] **PaddingStatistics** **0**|**1**:: - Relays only. + Relays and bridges only. When this option is enabled, Tor collects statistics for padding cells sent and received by this relay, in addition to total cell counts. These statistics are rounded, and omitted if traffic is low. This diff --git a/src/or/router.c b/src/or/router.c index edaa040dd7..c308bcfae1 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -3304,12 +3304,11 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, "conn-bi-direct", now, &contents) > 0) { smartlist_add(chunks, contents); } - } - - if (options->PaddingStatistics) { - contents = rep_hist_get_padding_count_lines(); - if (contents) - smartlist_add(chunks, contents); + if (options->PaddingStatistics) { + contents = rep_hist_get_padding_count_lines(); + if (contents) + smartlist_add(chunks, contents); + } } /* Add information about the pluggable transports we support. */ From 78220aae1ed3994453413559ff8b068578b31818 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Guisado?= Date: Wed, 23 Jan 2019 00:04:57 +0100 Subject: [PATCH 3/8] Add circuit time check before logging about relaxing circuit time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José M. Guisado --- changes/bug28698 | 3 +++ src/core/or/circuituse.c | 30 ++++++++++++++++++------------ 2 files changed, 21 insertions(+), 12 deletions(-) create mode 100644 changes/bug28698 diff --git a/changes/bug28698 b/changes/bug28698 new file mode 100644 index 0000000000..716aa0c552 --- /dev/null +++ b/changes/bug28698 @@ -0,0 +1,3 @@ + o Minor bugfix (logging): + - Avoid logging about relaxing circuits when their time is fixed. + Fixes bug 28698; bugfix on 0.2.4.7-alpha diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c index 8918dd077e..e306307c4c 100644 --- a/src/core/or/circuituse.c +++ b/src/core/or/circuituse.c @@ -545,6 +545,8 @@ circuit_expire_building(void) SMARTLIST_FOREACH_BEGIN(circuit_get_global_list(), circuit_t *,victim) { struct timeval cutoff; + bool fixed_time = circuit_build_times_disabled(get_options()); + if (!CIRCUIT_IS_ORIGIN(victim) || /* didn't originate here */ victim->marked_for_close) /* don't mess with marked circs */ continue; @@ -599,17 +601,19 @@ circuit_expire_building(void) if (!TO_ORIGIN_CIRCUIT(victim)->relaxed_timeout) { int first_hop_succeeded = TO_ORIGIN_CIRCUIT(victim)->cpath->state == CPATH_STATE_OPEN; - log_info(LD_CIRC, - "No circuits are opened. Relaxing timeout for circuit %d " - "(a %s %d-hop circuit in state %s with channel state %s).", - TO_ORIGIN_CIRCUIT(victim)->global_identifier, - circuit_purpose_to_string(victim->purpose), - TO_ORIGIN_CIRCUIT(victim)->build_state ? - TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len : - -1, - circuit_state_to_string(victim->state), - victim->n_chan ? - channel_state_to_string(victim->n_chan->state) : "none"); + if (!fixed_time) { + log_info(LD_CIRC, + "No circuits are opened. Relaxing timeout for circuit %d " + "(a %s %d-hop circuit in state %s with channel state %s).", + TO_ORIGIN_CIRCUIT(victim)->global_identifier, + circuit_purpose_to_string(victim->purpose), + TO_ORIGIN_CIRCUIT(victim)->build_state ? + TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len : + -1, + circuit_state_to_string(victim->state), + victim->n_chan ? + channel_state_to_string(victim->n_chan->state) : "none"); + } /* We count the timeout here for CBT, because technically this * was a timeout, and the timeout value needs to reset if we @@ -623,7 +627,8 @@ circuit_expire_building(void) } else { static ratelim_t relax_timeout_limit = RATELIM_INIT(3600); const double build_close_ms = get_circuit_build_close_time_ms(); - log_fn_ratelim(&relax_timeout_limit, LOG_NOTICE, LD_CIRC, + if (!fixed_time) { + log_fn_ratelim(&relax_timeout_limit, LOG_NOTICE, LD_CIRC, "No circuits are opened. Relaxed timeout for circuit %d " "(a %s %d-hop circuit in state %s with channel state %s) to " "%ldms. However, it appears the circuit has timed out " @@ -637,6 +642,7 @@ circuit_expire_building(void) victim->n_chan ? channel_state_to_string(victim->n_chan->state) : "none", (long)build_close_ms); + } } } From 1b9e77349ffbe813c7b16af67c069d967327f63b Mon Sep 17 00:00:00 2001 From: Kris Katterjohn Date: Fri, 25 Jan 2019 15:52:40 -0600 Subject: [PATCH 4/8] Fix some error-checking logic and a misleading error message When IPv4Only (IPv6Only) was used but the address could not be interpreted as a IPv4 (IPv6) address, the error message referred to the wrong IP version. This also fixes up the error-checking logic so it's more precise about what's being checked. Fixes bug 13221; bugfix on 0.2.3.9-alpha Signed-off-by: Kris Katterjohn --- changes/bug13221 | 5 +++++ src/or/config.c | 8 ++++---- 2 files changed, 9 insertions(+), 4 deletions(-) create mode 100644 changes/bug13221 diff --git a/changes/bug13221 b/changes/bug13221 new file mode 100644 index 0000000000..13935a1921 --- /dev/null +++ b/changes/bug13221 @@ -0,0 +1,5 @@ + o Minor bugfixes (logging): + - Correct a misleading error message when IPv4Only or IPv6Only + is used but the resolved address can not be interpreted as an + address of the specified IP version. Fixes bug 13221; bugfix + on 0.2.3.9-alpha. Patch from Kris Katterjohn. diff --git a/src/or/config.c b/src/or/config.c index 810f1e9a7a..e0d1794bdf 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -6650,13 +6650,13 @@ parse_port_config(smartlist_t *out, portname, escaped(ports->value)); goto err; } - if (bind_ipv4_only && tor_addr_family(&addr) == AF_INET6) { - log_warn(LD_CONFIG, "Could not interpret %sPort address as IPv6", + if (bind_ipv4_only && tor_addr_family(&addr) != AF_INET) { + log_warn(LD_CONFIG, "Could not interpret %sPort address as IPv4", portname); goto err; } - if (bind_ipv6_only && tor_addr_family(&addr) == AF_INET) { - log_warn(LD_CONFIG, "Could not interpret %sPort address as IPv4", + if (bind_ipv6_only && tor_addr_family(&addr) != AF_INET6) { + log_warn(LD_CONFIG, "Could not interpret %sPort address as IPv6", portname); goto err; } From 389ee834b6795ccabbe3d4c7edfdaf8d89696438 Mon Sep 17 00:00:00 2001 From: Kris Katterjohn Date: Mon, 21 Jan 2019 13:12:53 -0600 Subject: [PATCH 5/8] Log the correct "auto" port number for listening sockets When "auto" was used for the port number for a listening socket, the message logged after opening the socket would incorrectly say port 0 instead of the actual port used. Fixes bug 29144; bugfix on 0.3.5.1-alpha Signed-off-by: Kris Katterjohn --- changes/bug29144 | 5 +++++ src/core/mainloop/connection.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changes/bug29144 diff --git a/changes/bug29144 b/changes/bug29144 new file mode 100644 index 0000000000..5801224f14 --- /dev/null +++ b/changes/bug29144 @@ -0,0 +1,5 @@ + o Minor bugfixes (logging): + - Log the correct port number for listening sockets when "auto" is + used to let Tor pick the port number. Previously, port 0 was + logged instead of the actual port number. Fixes bug 29144; + bugfix on 0.3.5.1-alpha. Patch from Kris Katterjohn. diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index f2a646c5f9..7b8dc7f364 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -1527,7 +1527,7 @@ connection_listener_new(const struct sockaddr *listensockaddr, conn_type_to_string(type), conn->address); } else { log_notice(LD_NET, "Opened %s on %s", - conn_type_to_string(type), fmt_addrport(&addr, usePort)); + conn_type_to_string(type), fmt_addrport(&addr, gotPort)); } return conn; From b4e44a371f077f5a9fca19f94e6ff5f604f3e9d3 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 8 Mar 2019 09:54:54 -0500 Subject: [PATCH 6/8] hs-v2: Copy needed information between service on prunning Turns out that when reloading a tor configured with hidden service(s), we weren't copying all the needed information between the old service object to the new one. For instance, the desc_is_dirty timestamp wasn't which could lead to the service uploading its descriptor much later than it would need to. The replaycache wasn't also moved over and some intro point information as well. Fixes #23790 Signed-off-by: David Goulet --- changes/bug23790 | 6 ++++++ src/or/rendservice.c | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 changes/bug23790 diff --git a/changes/bug23790 b/changes/bug23790 new file mode 100644 index 0000000000..4aaf616e4d --- /dev/null +++ b/changes/bug23790 @@ -0,0 +1,6 @@ + o Minor bugfixes (hidden service v2): + - When reloading tor (HUP) configured with hidden service(s), some + information weren't copy to the new service object. One problem with this + was that tor would wait at least the RendPostPeriod time before uploading + the descriptor if the reload happened before the descriptor needed to be + published. Fixes bug 23790; bugfix on 0.2.1.9-alpha. diff --git a/src/or/rendservice.c b/src/or/rendservice.c index da200d1381..32b856452d 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -532,6 +532,30 @@ rend_service_check_dir_and_add(smartlist_t *service_list, } } +/* Copy relevant data from service src to dst while pruning the service lists. + * This should only be called during the pruning process which takes existing + * services and copy their data to the newly configured services. The src + * service replaycache will be set to NULL after this call. */ +static void +copy_service_on_prunning(rend_service_t *dst, rend_service_t *src) +{ + tor_assert(dst); + tor_assert(src); + + /* Keep the timestamps for when the content changed and the next upload + * time so we can properly upload the descriptor if needed for the new + * service object. */ + dst->desc_is_dirty = src->desc_is_dirty; + dst->next_upload_time = src->next_upload_time; + /* Move the replaycache to the new object. */ + dst->accepted_intro_dh_parts = src->accepted_intro_dh_parts; + src->accepted_intro_dh_parts = NULL; + /* Copy intro point information to destination service. */ + dst->intro_period_started = src->intro_period_started; + dst->n_intro_circuits_launched = src->n_intro_circuits_launched; + dst->n_intro_points_wanted = src->n_intro_points_wanted; +} + /** Set up rend_service_list, based on the values of HiddenServiceDir and * HiddenServicePort in options. Return 0 on success and -1 on * failure. (If validate_only is set, parse, warn and return as @@ -812,6 +836,9 @@ rend_config_services(const or_options_t *options, int validate_only) smartlist_add_all(new->expiring_nodes, old->expiring_nodes); smartlist_clear(old->expiring_nodes); smartlist_add(surviving_services, old); + + /* Copy service flags to the new service object. */ + copy_service_on_prunning(new, old); break; } } SMARTLIST_FOREACH_END(old); From add0f89c14b4aab6726e11acdcd864ee0c91543b Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Sat, 9 Mar 2019 02:55:28 -0500 Subject: [PATCH 7/8] relays shouldn't close idle rend circuits Allow connections to single onion services to remain idle without being disconnected. Relays acting as rendezvous points for single onion services were mistakenly closing idle established rendezvous circuits after 60 seconds, thinking that they are unused directory-fetching circuits that had served their purpose. Fixes bug 29665; bugfix on 0.2.1.26. --- changes/bug29665 | 7 +++++++ src/or/circuituse.c | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 changes/bug29665 diff --git a/changes/bug29665 b/changes/bug29665 new file mode 100644 index 0000000000..d89046faf5 --- /dev/null +++ b/changes/bug29665 @@ -0,0 +1,7 @@ + o Minor bugfixes (single onion services): + - Allow connections to single onion services to remain idle without + being disconnected. Relays acting as rendezvous points for + single onion services were mistakenly closing idle established + rendezvous circuits after 60 seconds, thinking that they are unused + directory-fetching circuits that had served their purpose. Fixes + bug 29665; bugfix on 0.2.1.26. diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 96cd3cd7e8..7c0b60293d 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1381,10 +1381,13 @@ circuit_expire_old_circuits_serverside(time_t now) or_circ = TO_OR_CIRCUIT(circ); /* If the circuit has been idle for too long, and there are no streams * on it, and it ends here, and it used a create_fast, mark it for close. + * + * Also if there is a rend_splice on it, it's a single onion service + * circuit and we should not close it. */ if (or_circ->is_first_hop && !circ->n_chan && !or_circ->n_streams && !or_circ->resolving_streams && - or_circ->p_chan && + or_circ->p_chan && !or_circ->rend_splice && channel_when_last_xmit(or_circ->p_chan) <= cutoff) { log_info(LD_CIRC, "Closing circ_id %u (empty %d secs ago)", (unsigned)or_circ->p_circ_id, From a3bc950e4201fe49e7a0996753a4af83d822828b Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Sat, 9 Mar 2019 03:18:25 -0500 Subject: [PATCH 8/8] relays shouldn't close idle rend circuits Allow connections to single onion services to remain idle without being disconnected. Relays acting as rendezvous points for single onion services were mistakenly closing idle established rendezvous circuits after 60 seconds, thinking that they are unused directory-fetching circuits that had served their purpose. Fixes bug 29665; bugfix on 0.2.1.26. --- changes/bug29665 | 7 +++++++ src/or/circuituse.c | 4 ++++ 2 files changed, 11 insertions(+) create mode 100644 changes/bug29665 diff --git a/changes/bug29665 b/changes/bug29665 new file mode 100644 index 0000000000..d89046faf5 --- /dev/null +++ b/changes/bug29665 @@ -0,0 +1,7 @@ + o Minor bugfixes (single onion services): + - Allow connections to single onion services to remain idle without + being disconnected. Relays acting as rendezvous points for + single onion services were mistakenly closing idle established + rendezvous circuits after 60 seconds, thinking that they are unused + directory-fetching circuits that had served their purpose. Fixes + bug 29665; bugfix on 0.2.1.26. diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 8e007ce920..03f2ae52cf 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1560,10 +1560,14 @@ circuit_expire_old_circuits_serverside(time_t now) or_circ = TO_OR_CIRCUIT(circ); /* If the circuit has been idle for too long, and there are no streams * on it, and it ends here, and it used a create_fast, mark it for close. + * + * Also if there is a rend_splice on it, it's a single onion service + * circuit and we should not close it. */ if (or_circ->p_chan && channel_is_client(or_circ->p_chan) && !circ->n_chan && !or_circ->n_streams && !or_circ->resolving_streams && + !or_circ->rend_splice && channel_when_last_xmit(or_circ->p_chan) <= cutoff) { log_info(LD_CIRC, "Closing circ_id %u (empty %d secs ago)", (unsigned)or_circ->p_circ_id,