diff options
author | teor <teor@torproject.org> | 2019-11-25 12:36:59 +1000 |
---|---|---|
committer | teor <teor@torproject.org> | 2019-11-25 12:36:59 +1000 |
commit | 055f5d4d1b2c8ea5a85691f7110e82a9c468b6dd (patch) | |
tree | beefb267f60a2800e2499cc2a7e354408ef81f9c | |
parent | c17ab20ac0d3efb1b9c46fa12685db5b09015481 (diff) | |
parent | 501b5174d84a121fe5932433e54b2929a5c2876d (diff) | |
download | tor-055f5d4d1b2c8ea5a85691f7110e82a9c468b6dd.tar.gz tor-055f5d4d1b2c8ea5a85691f7110e82a9c468b6dd.zip |
Merge branch 'maint-0.3.5' into maint-0.4.0
-rw-r--r-- | changes/bug30344 | 4 | ||||
-rw-r--r-- | changes/bug31939 | 3 | ||||
-rw-r--r-- | changes/ticket28970 | 6 | ||||
-rw-r--r-- | changes/ticket31548 | 7 | ||||
-rw-r--r-- | changes/ticket32058 | 5 | ||||
-rw-r--r-- | src/core/mainloop/connection.c | 18 | ||||
-rw-r--r-- | src/core/mainloop/mainloop.c | 10 | ||||
-rw-r--r-- | src/core/mainloop/periodic.c | 5 | ||||
-rw-r--r-- | src/feature/hs/hs_client.c | 8 | ||||
-rw-r--r-- | src/feature/hs/hs_service.c | 9 | ||||
-rw-r--r-- | src/lib/tls/buffers_tls.c | 4 |
11 files changed, 69 insertions, 10 deletions
diff --git a/changes/bug30344 b/changes/bug30344 new file mode 100644 index 0000000000..37561bf944 --- /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/changes/bug31939 b/changes/bug31939 new file mode 100644 index 0000000000..a36ea495d6 --- /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/changes/ticket28970 b/changes/ticket28970 new file mode 100644 index 0000000000..138c575fcc --- /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/changes/ticket31548 b/changes/ticket31548 new file mode 100644 index 0000000000..fef0b5d01f --- /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/changes/ticket32058 b/changes/ticket32058 new file mode 100644 index 0000000000..b40bcda416 --- /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/connection.c b/src/core/mainloop/connection.c index 73b8ef4da5..21d7bd6d0d 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 193df61d01..e8de578b67 100644 --- a/src/core/mainloop/mainloop.c +++ b/src/core/mainloop/mainloop.c @@ -877,6 +877,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 diff --git a/src/core/mainloop/periodic.c b/src/core/mainloop/periodic.c index c0363b15ea..5c01213d17 100644 --- a/src/core/mainloop/periodic.c +++ b/src/core/mainloop/periodic.c @@ -133,6 +133,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; } diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 2a5765aec2..fd2d266453 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; } diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index e32f021742..160fb87397 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); diff --git a/src/lib/tls/buffers_tls.c b/src/lib/tls/buffers_tls.c index 3c18cc7e43..bf03b61459 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) { |