November 2019 - tor-commits - lists.torproject.org

[tor/maint-0.4.1] respect accountingmax during soft hibernation
by teor＠torproject.org 25 Nov '19

25 Nov '19

commit a7ccd9a997bc85c0e03926b7d24f7e05a4a51d39 Author: Roger Dingledine <arma(a)torproject.org> Date: Mon Oct 21 04:14:42 2019 -0400 respect accountingmax during soft hibernation Relays now respect their AccountingMax bandwidth again. When relays entered "soft" hibernation (which typically starts when we've hit 90% of our AccountingMax), we had stopped checking whether we should enter hard hibernation. Soft hibernation refuses new connections and new circuits, but the existing circuits can continue, meaning that relays could have exceeded their configured AccountingMax. This commit rolls back some of the cpu-saving fixes, where we tried to avoid calling so many of our events while we're off the network. That's because PERIODIC_EVENT_FLAG_NEED_NET checks net_is_disabled(), which returns true even if we're only in soft hibernation. Fixes bug 32108; bugfix on 0.4.0.1-alpha. --- changes/bug32108 | 8 ++++++++ src/core/mainloop/mainloop.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/changes/bug32108 b/changes/bug32108 new file mode 100644 index 000000000..2806fa3e5 --- /dev/null +++ b/changes/bug32108 @@ -0,0 +1,8 @@ + o Major bugfixes (relay): + - Relays now respect their AccountingMax bandwidth again. When relays + entered "soft" hibernation (which typically starts when we've hit + 90% of our AccountingMax), we had stopped checking whether we should + enter hard hibernation. Soft hibernation refuses new connections and + new circuits, but the existing circuits can continue, meaning that + relays could have exceeded their configured AccountingMax. Fixes + bug 32108; bugfix on 0.4.0.1-alpha. diff --git a/src/core/mainloop/mainloop.c b/src/core/mainloop/mainloop.c index 18e87fa87..193df61d0 100644 --- a/src/core/mainloop/mainloop.c +++ b/src/core/mainloop/mainloop.c @@ -1397,7 +1397,7 @@ STATIC periodic_event_item_t periodic_events[] = { /* This is a legacy catch-all callback that runs once per second if * we are online and active. */ CALLBACK(second_elapsed, NET_PARTICIPANT, - FL(NEED_NET)|FL(RUN_ON_DISABLE)), + FL(RUN_ON_DISABLE)), /* XXXX Do we have a reason to do this on a callback? Does it do any good at * all? For now, if we're dormant, we can let our listeners decay. */

1 0

[tor/maint-0.4.0] tls: Log TLS read buffer length bugs once
by teor＠torproject.org 25 Nov '19

25 Nov '19

commit ff304f3be7d6b5753d2066a0af9772b451f7f442 Author: teor <teor(a)torproject.org> Date: Fri Oct 4 12:33:34 2019 +1000 tls: Log TLS read buffer length bugs once Rather than filling the logs with similar warnings. Fixes bug 31939; bugfix on 0.3.0.4-rc. --- changes/bug31939 | 3 +++ src/lib/tls/buffers_tls.c | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/changes/bug31939 b/changes/bug31939 new file mode 100644 index 000000000..a36ea495d --- /dev/null +++ b/changes/bug31939 @@ -0,0 +1,3 @@ + o Minor bugfixes (tls, logging): + - Log TLS read buffer length bugs once, rather than filling the logs + with similar warnings. Fixes bug 31939; bugfix on 0.3.0.4-rc. diff --git a/src/lib/tls/buffers_tls.c b/src/lib/tls/buffers_tls.c index c176162c3..e92cb9163 100644 --- a/src/lib/tls/buffers_tls.c +++ b/src/lib/tls/buffers_tls.c @@ -68,9 +68,9 @@ buf_read_from_tls(buf_t *buf, tor_tls_t *tls, size_t at_most) check_no_tls_errors(); - if (BUG(buf->datalen >= INT_MAX)) + IF_BUG_ONCE(buf->datalen >= INT_MAX) return -1; - if (BUG(buf->datalen >= INT_MAX - at_most)) + IF_BUG_ONCE(buf->datalen >= INT_MAX - at_most) return -1; while (at_most > total_read) {

1 0

[tor/maint-0.4.0] Stop libevent from reading data from closed connections.
by teor＠torproject.org 25 Nov '19

25 Nov '19

commit 841cff6e4fe1cdd370cd51019e69c6c564e8059c Author: George Kadianakis <desnacked(a)riseup.net> Date: Mon Sep 30 18:29:00 2019 +0300 Stop libevent from reading data from closed connections. Code adapted from Rob's proposed patch in #30344. Also add a comment in connection_mark_for_close_internal_() on why we should not be adding extra code there without a very good reason. --- changes/bug30344 | 4 ++++ src/core/mainloop/connection.c | 18 ++++++++++++------ src/core/mainloop/mainloop.c | 10 ++++++++++ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/changes/bug30344 b/changes/bug30344 new file mode 100644 index 000000000..37561bf94 --- /dev/null +++ b/changes/bug30344 @@ -0,0 +1,4 @@ + o Minor bugfixes (connection): + - Avoid reading data from closed connections, which can cause needless + loops in libevent and infinite loops in Shadow. Fixes bug 30344; bugfix + on 0.1.1.1-alpha. diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index 2f03d919a..3595bba85 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -897,13 +897,19 @@ connection_mark_for_close_(connection_t *conn, int line, const char *file) } /** Mark <b>conn</b> to be closed next time we loop through - * conn_close_if_marked() in main.c; the _internal version bypasses the - * CONN_TYPE_OR checks; this should be called when you either are sure that - * if this is an or_connection_t the controlling channel has been notified - * (e.g. with connection_or_notify_error()), or you actually are the + * conn_close_if_marked() in main.c. + * + * This _internal version bypasses the CONN_TYPE_OR checks; this should be + * called when you either are sure that if this is an or_connection_t the + * controlling channel has been notified (e.g. with + * connection_or_notify_error()), or you actually are the * connection_or_close_for_error() or connection_or_close_normally() function. - * For all other cases, use connection_mark_and_flush() instead, which - * checks for or_connection_t properly, instead. See below. + * For all other cases, use connection_mark_and_flush() which checks for + * or_connection_t properly, instead. See below. + * + * We want to keep this function simple and quick, since it can be called from + * quite deep in the call chain, and hence it should avoid having side-effects + * that interfere with its callers view of the connection. */ MOCK_IMPL(void, connection_mark_for_close_internal_, (connection_t *conn, diff --git a/src/core/mainloop/mainloop.c b/src/core/mainloop/mainloop.c index 6e2b300fb..4b3c3bf6a 100644 --- a/src/core/mainloop/mainloop.c +++ b/src/core/mainloop/mainloop.c @@ -879,6 +879,16 @@ conn_read_callback(evutil_socket_t fd, short event, void *_conn) /* assert_connection_ok(conn, time(NULL)); */ + /* Handle marked for close connections early */ + if (conn->marked_for_close && connection_is_reading(conn)) { + /* Libevent says we can read, but we are marked for close so we will never + * try to read again. We will try to close the connection below inside of + * close_closeable_connections(), but let's make sure not to cause Libevent + * to spin on conn_read_callback() while we wait for the socket to let us + * flush to it.*/ + connection_stop_reading(conn); + } + if (connection_handle_read(conn) < 0) { if (!conn->marked_for_close) { #ifndef _WIN32

1 0

[tor/maint-0.4.0] mainloop: Disable periodic events before a destroy
by teor＠torproject.org 25 Nov '19

25 Nov '19

commit 4a8d4913227ea1d6b9302cda4703516595a3c1b5 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Oct 15 13:33:48 2019 -0400 mainloop: Disable periodic events before a destroy When tearing down all periodic events during shutdown, disable them first so their enable flag is updated. This allows the tor_api.h to relaunch tor properly after a clean shutdown. Fixes #32058 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket32058 | 5 +++++ src/core/mainloop/periodic.c | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/changes/ticket32058 b/changes/ticket32058 new file mode 100644 index 000000000..b40bcda41 --- /dev/null +++ b/changes/ticket32058 @@ -0,0 +1,5 @@ + o Minor bugfixes (mainloop, periodic events): + - Periodic events enabled flag was not unset properly when shutting down tor + cleanly. This had the side effect to not re-enable periodic events when + tor_api.h is used to relaunch tor after a shutdown. Fixes bug 32058; + bugfix on 0.3.3.1-alpha. diff --git a/src/core/mainloop/periodic.c b/src/core/mainloop/periodic.c index 34690c54d..2651bbbc8 100644 --- a/src/core/mainloop/periodic.c +++ b/src/core/mainloop/periodic.c @@ -137,6 +137,11 @@ periodic_event_destroy(periodic_event_item_t *event) { if (!event) return; + + /* First disable the event so we first cancel the event and set its enabled + * flag properly. */ + periodic_event_disable(event); + mainloop_event_free(event->ev); event->last_action_time = 0; }

1 0

[tor/maint-0.4.0] hs-v3: Make service pick the exact amount of intro points
by teor＠torproject.org 25 Nov '19

25 Nov '19

commit 984a28f3e502c6df9e28057c3e934b98df83d8e9 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Aug 29 10:46:24 2019 -0400 hs-v3: Make service pick the exact amount of intro points When encoding introduction points, we were not checking if that intro points had an established circuit. When botting up, the service will pick, by default, 3 + 2 intro points and the first 3 that establish, we use them and upload the descriptor. However, the intro point is removed from the service descriptor list only when the circuit has opened and we see that we have already enough intro points, it is then removed. But it is possible that the service establishes 3 intro points successfully before the other(s) have even opened yet. This lead to the service encoding extra intro points in the descriptor even though the circuit is not opened or might never establish (#31561). Fixes #31548 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket31548 | 7 +++++++ src/feature/hs/hs_service.c | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/changes/ticket31548 b/changes/ticket31548 new file mode 100644 index 000000000..fef0b5d01 --- /dev/null +++ b/changes/ticket31548 @@ -0,0 +1,7 @@ + o Major bugfixes (hidden service v3): + - Make onion service always use the exact amount of configured intro points + (or less due to node exlusion). Before, a service could sometimes pick + more intro points than configured with the + HiddenServiceNumIntroductionPoints option. Fixes bug 31548; bugfix on + 0.3.2.1-alpha. + diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index 402929036..4d0d92645 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -1686,6 +1686,15 @@ build_desc_intro_points(const hs_service_t *service, DIGEST256MAP_FOREACH(desc->intro_points.map, key, const hs_service_intro_point_t *, ip) { + if (!ip->circuit_established) { + /* Ignore un-established intro points. They can linger in that list + * because their circuit has not opened and they haven't been removed + * yet even though we have enough intro circuits. + * + * Due to #31561, it can stay in that list until rotation so this check + * prevents to publish an intro point without a circuit. */ + continue; + } hs_desc_intro_point_t *desc_ip = hs_desc_intro_point_new(); if (setup_desc_intro_point(&desc->signing_kp, ip, now, desc_ip) < 0) { hs_desc_intro_point_free(desc_ip);

1 0

[tor/maint-0.4.0] Remove overly strict assertions in transports.c.
by teor＠torproject.org 25 Nov '19

25 Nov '19

commit 389b37246eb6310401bfc3bcb80ee2fb8dcac56b Author: Alexander Færøy <ahf(a)torproject.org> Date: Tue Sep 24 16:42:00 2019 +0200 Remove overly strict assertions in transports.c. This patch removes an overly strict tor_assert() and an ignorable BUG() expression. Both of these would trigger if a PT was unable to configure itself during startup. The easy way to trigger this is to configure an obfs4 bridge where you make the obfs4 process try to bind on a port number under 1024. See: https://bugs.torproject.org/31091 --- changes/ticket31091 | 3 +++ src/feature/client/transports.c | 6 ++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/changes/ticket31091 b/changes/ticket31091 new file mode 100644 index 000000000..3cb9a2c37 --- /dev/null +++ b/changes/ticket31091 @@ -0,0 +1,3 @@ + o Minor bugfixes (pluggable transports): + - Remove overly strict assertions that triggers when a pluggable transport + is spawned in an unsuccessful manner. Fixes bug 31091; bugfix on 0.4.0.1-alpha. diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index e7ff3bf34..f34d4af48 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -1826,15 +1826,13 @@ managed_proxy_stdout_callback(process_t *process, managed_proxy_t *mp = process_get_data(process); - if (BUG(mp == NULL)) + if (mp == NULL) return; handle_proxy_line(line, mp); - if (proxy_configuration_finished(mp)) { + if (proxy_configuration_finished(mp)) handle_finished_proxy(mp); - tor_assert(mp->conf_state == PT_PROTO_COMPLETED); - } } /** Callback function that is called when our PT process have data on its

1 0

[tor/maint-0.4.0] respect accountingmax during soft hibernation
by teor＠torproject.org 25 Nov '19

25 Nov '19

commit a7ccd9a997bc85c0e03926b7d24f7e05a4a51d39 Author: Roger Dingledine <arma(a)torproject.org> Date: Mon Oct 21 04:14:42 2019 -0400 respect accountingmax during soft hibernation Relays now respect their AccountingMax bandwidth again. When relays entered "soft" hibernation (which typically starts when we've hit 90% of our AccountingMax), we had stopped checking whether we should enter hard hibernation. Soft hibernation refuses new connections and new circuits, but the existing circuits can continue, meaning that relays could have exceeded their configured AccountingMax. This commit rolls back some of the cpu-saving fixes, where we tried to avoid calling so many of our events while we're off the network. That's because PERIODIC_EVENT_FLAG_NEED_NET checks net_is_disabled(), which returns true even if we're only in soft hibernation. Fixes bug 32108; bugfix on 0.4.0.1-alpha. --- changes/bug32108 | 8 ++++++++ src/core/mainloop/mainloop.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/changes/bug32108 b/changes/bug32108 new file mode 100644 index 000000000..2806fa3e5 --- /dev/null +++ b/changes/bug32108 @@ -0,0 +1,8 @@ + o Major bugfixes (relay): + - Relays now respect their AccountingMax bandwidth again. When relays + entered "soft" hibernation (which typically starts when we've hit + 90% of our AccountingMax), we had stopped checking whether we should + enter hard hibernation. Soft hibernation refuses new connections and + new circuits, but the existing circuits can continue, meaning that + relays could have exceeded their configured AccountingMax. Fixes + bug 32108; bugfix on 0.4.0.1-alpha. diff --git a/src/core/mainloop/mainloop.c b/src/core/mainloop/mainloop.c index 18e87fa87..193df61d0 100644 --- a/src/core/mainloop/mainloop.c +++ b/src/core/mainloop/mainloop.c @@ -1397,7 +1397,7 @@ STATIC periodic_event_item_t periodic_events[] = { /* This is a legacy catch-all callback that runs once per second if * we are online and active. */ CALLBACK(second_elapsed, NET_PARTICIPANT, - FL(NEED_NET)|FL(RUN_ON_DISABLE)), + FL(RUN_ON_DISABLE)), /* XXXX Do we have a reason to do this on a callback? Does it do any good at * all? For now, if we're dormant, we can let our listeners decay. */

1 0

[tor/maint-0.4.0] hs-v3: Remove a BUG() caused by an acceptable race
by teor＠torproject.org 25 Nov '19

25 Nov '19

commit ed57a04a65a59ee744910a9db22a81359dac3491 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Oct 23 10:20:09 2019 -0400 hs-v3: Remove a BUG() caused by an acceptable race hs_client_purge_state() and hs_cache_clean_as_client() can remove a descriptor from the client cache with a NEWNYM or simply when the descriptor expires. Which means that for an INTRO circuit being established during that time, once it opens, we lookup the descriptor to get the IP object but hey surprised, no more descriptor. The approach here is minimalist that is accept the race and close the circuit since we can not continue. Before that, the circuit would stay opened and the client wait the SockTimeout. Fixers #28970. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket28970 | 6 ++++++ src/feature/hs/hs_client.c | 8 ++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/changes/ticket28970 b/changes/ticket28970 new file mode 100644 index 000000000..138c575fc --- /dev/null +++ b/changes/ticket28970 @@ -0,0 +1,6 @@ + o Minor bugfixes (clietn, hidden service v3): + - Fix a BUG() assertion that occurs within a very small race window between + a client intro circuit opens and its descriptor that gets cleaned up from + the cache. The circuit is now closed which will trigger a re-fetch of the + descriptor and continue the HS connection. Fixes bug 28970; bugfix on + 0.3.2.1-alpha. diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 2a5765aec..fd2d26645 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -672,8 +672,12 @@ setup_intro_circ_auth_key(origin_circuit_t *circ) tor_assert(circ); desc = hs_cache_lookup_as_client(&circ->hs_ident->identity_pk); - if (BUG(desc == NULL)) { - /* Opening intro circuit without the descriptor is no good... */ + if (desc == NULL) { + /* There is a very small race window between the opening of this circuit + * and the client descriptor cache that gets purged (NEWNYM) or the + * cleaned up because it expired. Mark the circuit for close so a new + * descriptor fetch can occur. */ + circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL); goto end; }

1 0

[tor/maint-0.4.0] Merge remote-tracking branch 'tor-github/pr/1422' into maint-0.3.5
by teor＠torproject.org 25 Nov '19

25 Nov '19

commit 400cee261ecccd024f6cb13dd1a940d4b9d0057b Merge: 54c01119e 4a8d49132 Author: teor <teor(a)torproject.org> Date: Mon Nov 25 12:34:29 2019 +1000 Merge remote-tracking branch 'tor-github/pr/1422' into maint-0.3.5 changes/ticket32058 | 5 +++++ src/core/mainloop/periodic.c | 5 +++++ 2 files changed, 10 insertions(+)

1 0

[tor/maint-0.4.0] Merge remote-tracking branch 'tor-github/pr/1405' into maint-0.3.5
by teor＠torproject.org 25 Nov '19

25 Nov '19

commit 54c01119ed59668bfa8ed6ef978201731bef0554 Merge: 83424cb62 841cff6e4 Author: teor <teor(a)torproject.org> Date: Mon Nov 25 12:33:55 2019 +1000 Merge remote-tracking branch 'tor-github/pr/1405' into maint-0.3.5 changes/bug30344 | 4 ++++ src/core/mainloop/connection.c | 18 ++++++++++++------ src/core/mainloop/mainloop.c | 10 ++++++++++ 3 files changed, 26 insertions(+), 6 deletions(-)

1 0