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

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

24 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/release-0.3.5] Stop libevent from reading data from closed connections.
by teor＠torproject.org 24 Nov '19

24 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/release-0.4.0] Stop libevent from reading data from closed connections.
by teor＠torproject.org 24 Nov '19

24 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/release-0.4.0] hs-v3: Make service pick the exact amount of intro points
by teor＠torproject.org 24 Nov '19

24 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/release-0.4.0] mainloop: Disable periodic events before a destroy
by teor＠torproject.org 24 Nov '19

24 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/release-0.4.0] tls: Log TLS read buffer length bugs once
by teor＠torproject.org 24 Nov '19

24 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/release-0.3.5] Merge branch 'maint-0.3.5' into release-0.3.5
by teor＠torproject.org 24 Nov '19

24 Nov '19

commit 90b8c21671b72a694e0c5a266d1b150abe2860a1 Merge: d1bd19868 501b5174d Author: teor <teor(a)torproject.org> Date: Mon Nov 25 12:36:53 2019 +1000 Merge branch 'maint-0.3.5' into release-0.3.5 changes/bug30344 | 4 ++++ changes/bug31939 | 3 +++ changes/ticket28970 | 6 ++++++ changes/ticket31548 | 7 +++++++ changes/ticket32058 | 5 +++++ src/core/mainloop/connection.c | 18 ++++++++++++------ src/core/mainloop/mainloop.c | 10 ++++++++++ src/core/mainloop/periodic.c | 5 +++++ src/feature/hs/hs_client.c | 8 ++++++-- src/feature/hs/hs_service.c | 9 +++++++++ src/lib/tls/buffers_tls.c | 4 ++-- 11 files changed, 69 insertions(+), 10 deletions(-)

1 0

[tor/release-0.3.5] Merge remote-tracking branch 'tor-github/pr/1395' into maint-0.3.5
by teor＠torproject.org 24 Nov '19

24 Nov '19

commit 83424cb62f19ee09cba769a6c9a8cc94fc3c4cb7 Merge: f9812ee5f 984a28f3e Author: teor <teor(a)torproject.org> Date: Mon Nov 25 12:33:14 2019 +1000 Merge remote-tracking branch 'tor-github/pr/1395' into maint-0.3.5 changes/ticket31548 | 7 +++++++ src/feature/hs/hs_service.c | 9 +++++++++ 2 files changed, 16 insertions(+)

1 0

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

24 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

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

24 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