This is an automated email from the git hooks/post-receive script.
dgoulet pushed a change to branch main in repository tor.
from 95f61cc84f Merge branch 'maint-0.4.7' new de0fda7a79 circ: Set proper timeout cutoff for HS circuits new 921268d4ce circ: Get rid of hs_circ_has_timed_out new 04cccd7074 hs: Retry service rendezvous on circuit close new 4ed67fe174 changes: Ticket 40694 new dd272b6ef4 Merge remote-tracking branch 'tor-gitlab/mr/638' new 88b5daf152 circ: Set proper timeout cutoff for HS circuits new 5b44a32c59 circ: Get rid of hs_circ_has_timed_out new 78c184d2fe hs: Retry service rendezvous on circuit close new 609a82a595 changes: Ticket 40694 new 8bf9c4be8d Merge branch 'maint-0.4.7'
The 10 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference.
Summary of changes: changes/ticket40694 | 5 ++ src/core/or/circuituse.c | 108 ++++++++++++++-------------------------- src/core/or/origin_circuit_st.h | 24 --------- src/feature/hs/hs_circuit.c | 21 +------- src/feature/hs/hs_circuit.h | 2 +- src/feature/hs/hs_service.c | 3 ++ 6 files changed, 47 insertions(+), 116 deletions(-) create mode 100644 changes/ticket40694
This is an automated email from the git hooks/post-receive script.
dgoulet pushed a commit to branch main in repository tor.
commit de0fda7a79947d8243e2be45ab51bc9179eac94a Author: David Goulet dgoulet@torproject.org AuthorDate: Wed Oct 19 14:50:00 2022 -0400
circ: Set proper timeout cutoff for HS circuits
Explicitly set the S_CONNECT_REND purpose to a 4-hop cutoff.
As for the established rendezvous circuit waiting on the RENDEZVOUS2, set one that is very long considering the possible waiting time for the service to get the request and join our rendezvous.
Part of #40694
Signed-off-by: David Goulet dgoulet@torproject.org --- src/core/or/circuituse.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c index a259957d37..ad78e05d76 100644 --- a/src/core/or/circuituse.c +++ b/src/core/or/circuituse.c @@ -451,7 +451,8 @@ circuit_expire_building(void) * custom timeouts yet */ struct timeval general_cutoff, begindir_cutoff, fourhop_cutoff, close_cutoff, extremely_old_cutoff, hs_extremely_old_cutoff, - cannibalized_cutoff, c_intro_cutoff, s_intro_cutoff, stream_cutoff; + cannibalized_cutoff, c_intro_cutoff, s_intro_cutoff, stream_cutoff, + c_rend_ready_cutoff; const or_options_t *options = get_options(); struct timeval now; cpath_build_state_t *build_state; @@ -531,6 +532,16 @@ circuit_expire_building(void) /* Server intro circs have an extra round trip */ SET_CUTOFF(s_intro_cutoff, get_circuit_build_timeout_ms() * (9/6.0) + 1000);
+ /* For circuit purpose set to: CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED. + * + * The cutoff of such circuit is very high because we end up in this state if + * once the INTRODUCE_ACK is received which could be before the service + * receives the INTRODUCE2 cell. The worst case is a full 3-hop latency + * (intro -> service), 4-hop circuit creation latency (service -> RP), and + * finally a 7-hop latency for the RENDEZVOUS2 cell to arrive (service -> + * client). */ + SET_CUTOFF(c_rend_ready_cutoff, get_circuit_build_timeout_ms() * 3 + 1000); + SET_CUTOFF(close_cutoff, get_circuit_build_close_time_ms()); SET_CUTOFF(extremely_old_cutoff, get_circuit_build_close_time_ms()*2 + 1000);
@@ -568,8 +579,12 @@ circuit_expire_building(void) cutoff = c_intro_cutoff; else if (victim->purpose == CIRCUIT_PURPOSE_S_ESTABLISH_INTRO) cutoff = s_intro_cutoff; - else if (victim->purpose == CIRCUIT_PURPOSE_C_ESTABLISH_REND) - cutoff = stream_cutoff; + else if (victim->purpose == CIRCUIT_PURPOSE_S_CONNECT_REND) { + /* Service connecting to a rendezvous point is a four hop circuit. We set + * it explicitly here because this function is a clusterf***. */ + cutoff = fourhop_cutoff; + } else if (victim->purpose == CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED) + cutoff = c_rend_ready_cutoff; else if (victim->purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) cutoff = close_cutoff; else if (TO_ORIGIN_CIRCUIT(victim)->has_opened &&
This is an automated email from the git hooks/post-receive script.
dgoulet pushed a commit to branch main in repository tor.
commit 921268d4ce3e55c87e1df91a17b96d8d1407fbb7 Author: David Goulet dgoulet@torproject.org AuthorDate: Wed Oct 19 15:11:11 2022 -0400
circ: Get rid of hs_circ_has_timed_out
Logic is too convoluted and we can't efficiently apply a specific timeout depending on the purpose.
Remove it and instead rely on the right circuit cutoff instead of keeping this flagged circuit open forever.
Part of #40694
Signed-off-by: David Goulet dgoulet@torproject.org --- src/core/or/circuituse.c | 86 +++++++++-------------------------------- src/core/or/origin_circuit_st.h | 16 -------- src/feature/hs/hs_circuit.c | 5 --- 3 files changed, 19 insertions(+), 88 deletions(-)
diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c index ad78e05d76..acb9a7fba1 100644 --- a/src/core/or/circuituse.c +++ b/src/core/or/circuituse.c @@ -133,11 +133,6 @@ circuit_is_acceptable(const origin_circuit_t *origin_circ, return 0; }
- /* If this is a timed-out hidden service circuit, skip it. */ - if (origin_circ->hs_circ_has_timed_out) { - return 0; - } - if (purpose == CIRCUIT_PURPOSE_C_GENERAL || purpose == CIRCUIT_PURPOSE_C_HSDIR_GET || purpose == CIRCUIT_PURPOSE_S_HSDIR_POST || @@ -334,7 +329,6 @@ circuit_get_best(const entry_connection_t *conn, { origin_circuit_t *best=NULL; struct timeval now; - int intro_going_on_but_too_old = 0;
tor_assert(conn);
@@ -353,15 +347,6 @@ circuit_get_best(const entry_connection_t *conn, continue; origin_circ = TO_ORIGIN_CIRCUIT(circ);
- /* Log an info message if we're going to launch a new intro circ in - * parallel */ - if (purpose == CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT && - !must_be_open && origin_circ->hs_circ_has_timed_out && - !circ->marked_for_close) { - intro_going_on_but_too_old = 1; - continue; - } - if (!circuit_is_acceptable(origin_circ,conn,must_be_open,purpose, need_uptime,need_internal, (time_t)now.tv_sec)) continue; @@ -374,11 +359,6 @@ circuit_get_best(const entry_connection_t *conn, } SMARTLIST_FOREACH_END(circ);
- if (!best && intro_going_on_but_too_old) - log_info(LD_REND|LD_CIRC, "There is an intro circuit being created " - "right now, but it has already taken quite a while. Starting " - "one in parallel."); - return best; }
@@ -450,7 +430,7 @@ circuit_expire_building(void) * circuit_build_times_get_initial_timeout() if we haven't computed * custom timeouts yet */ struct timeval general_cutoff, begindir_cutoff, fourhop_cutoff, - close_cutoff, extremely_old_cutoff, hs_extremely_old_cutoff, + close_cutoff, extremely_old_cutoff, cannibalized_cutoff, c_intro_cutoff, s_intro_cutoff, stream_cutoff, c_rend_ready_cutoff; const or_options_t *options = get_options(); @@ -545,10 +525,6 @@ circuit_expire_building(void) SET_CUTOFF(close_cutoff, get_circuit_build_close_time_ms()); SET_CUTOFF(extremely_old_cutoff, get_circuit_build_close_time_ms()*2 + 1000);
- SET_CUTOFF(hs_extremely_old_cutoff, - MAX(get_circuit_build_close_time_ms()*2 + 1000, - options->SocksTimeout * 1000)); - bool fixed_time = circuit_build_times_disabled(get_options());
SMARTLIST_FOREACH_BEGIN(circuit_get_global_list(), circuit_t *,victim) { @@ -595,9 +571,6 @@ circuit_expire_building(void) else cutoff = general_cutoff;
- if (TO_ORIGIN_CIRCUIT(victim)->hs_circ_has_timed_out) - cutoff = hs_extremely_old_cutoff; - if (timercmp(&victim->timestamp_began, &cutoff, OP_GT)) continue; /* it's still young, leave it alone */
@@ -769,51 +742,30 @@ circuit_expire_building(void) } }
- /* If this is a hidden service client circuit which is far enough along in - * connecting to its destination, and we haven't already flagged it as - * 'timed out', flag it so we'll launch another intro or rend circ, but - * don't mark it for close yet. - * - * (Circs flagged as 'timed out' are given a much longer timeout - * period above, so we won't close them in the next call to - * circuit_expire_building.) */ - if (!(TO_ORIGIN_CIRCUIT(victim)->hs_circ_has_timed_out)) { - switch (victim->purpose) { - case CIRCUIT_PURPOSE_C_REND_READY: - /* We only want to spare a rend circ iff it has been specified in an - * INTRODUCE1 cell sent to a hidden service. */ - if (!hs_circ_is_rend_sent_in_intro1(CONST_TO_ORIGIN_CIRCUIT(victim))) { - break; - } - FALLTHROUGH; - case CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT: - case CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED: - /* If we have reached this line, we want to spare the circ for now. */ - log_info(LD_CIRC,"Marking circ %u (state %d:%s, purpose %d) " - "as timed-out HS circ", - (unsigned)victim->n_circ_id, - victim->state, circuit_state_to_string(victim->state), - victim->purpose); - TO_ORIGIN_CIRCUIT(victim)->hs_circ_has_timed_out = 1; + /* Special checks for onion service circuits. */ + switch (victim->purpose) { + case CIRCUIT_PURPOSE_C_REND_READY: + /* We only want to spare a rend circ iff it has been specified in an + * INTRODUCE1 cell sent to a hidden service. */ + if (hs_circ_is_rend_sent_in_intro1(CONST_TO_ORIGIN_CIRCUIT(victim))) { continue; - default: - break; } - } - - /* If this is a service-side rendezvous circuit which is far - * enough along in connecting to its destination, consider sparing - * it. */ - if (!(TO_ORIGIN_CIRCUIT(victim)->hs_circ_has_timed_out) && - victim->purpose == CIRCUIT_PURPOSE_S_CONNECT_REND) { - log_info(LD_CIRC,"Marking circ %u (state %d:%s, purpose %d) " - "as timed-out HS circ; relaunching rendezvous attempt.", + break; + case CIRCUIT_PURPOSE_S_CONNECT_REND: + /* The connection to the rendezvous point has timed out, close it and + * retry if possible. */ + log_info(LD_CIRC,"Rendezvous circ %u (state %d:%s, purpose %d) " + "as timed-out, closing it. Relaunching rendezvous attempt.", (unsigned)victim->n_circ_id, victim->state, circuit_state_to_string(victim->state), victim->purpose); - TO_ORIGIN_CIRCUIT(victim)->hs_circ_has_timed_out = 1; hs_circ_retry_service_rendezvous_point(TO_ORIGIN_CIRCUIT(victim)); - continue; + /* We'll close as a timeout the victim circuit. The rendezvous point + * won't keep both circuits, it only keeps the newest (for the same + * cookie). */ + break; + default: + break; }
if (victim->n_chan) diff --git a/src/core/or/origin_circuit_st.h b/src/core/or/origin_circuit_st.h index 6c86a56000..2cd8a33abc 100644 --- a/src/core/or/origin_circuit_st.h +++ b/src/core/or/origin_circuit_st.h @@ -205,22 +205,6 @@ struct origin_circuit_t { * (in host byte order) for response comparison. */ uint32_t pathbias_probe_nonce;
- /** Set iff this is a hidden-service circuit which has timed out - * according to our current circuit-build timeout, but which has - * been kept around because it might still succeed in connecting to - * its destination, and which is not a fully-connected rendezvous - * circuit. - * - * (We clear this flag for client-side rendezvous circuits when they - * are 'joined' to the other side's rendezvous circuit, so that - * connection_ap_handshake_attach_circuit can put client streams on - * the circuit. We also clear this flag for service-side rendezvous - * circuits when they are 'joined' to a client's rend circ, but only - * for symmetry with the client case. Client-side introduction - * circuits are closed when we get a joined rend circ, and - * service-side introduction circuits never have this flag set.) */ - unsigned int hs_circ_has_timed_out : 1; - /** Set iff this circuit has been given a relaxed timeout because * no circuits have opened. Used to prevent spamming logs. */ unsigned int relaxed_timeout : 1; diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index d253802f79..307557dc86 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -126,11 +126,6 @@ finalize_rend_circuit(origin_circuit_t *circ, crypt_path_t *hop, hop->package_window = circuit_initial_package_window(); hop->deliver_window = CIRCWINDOW_START;
- /* Now that this circuit has finished connecting to its destination, - * make sure circuit_get_open_circ_or_launch is willing to return it - * so we can actually use it. */ - circ->hs_circ_has_timed_out = 0; - /* If congestion control, transfer ccontrol onto the cpath. */ if (TO_CIRCUIT(circ)->ccontrol) { hop->ccontrol = TO_CIRCUIT(circ)->ccontrol;
This is an automated email from the git hooks/post-receive script.
dgoulet pushed a commit to branch main in repository tor.
commit 04cccd70744a09492b70baf974b2953d6e5c6d0a Author: David Goulet dgoulet@torproject.org AuthorDate: Wed Oct 19 15:27:22 2022 -0400
hs: Retry service rendezvous on circuit close
Move the retry from circuit_expire_building() to when the offending circuit is being closed.
Fixes #40695
Signed-off-by: David Goulet dgoulet@torproject.org --- src/core/or/circuituse.c | 1 - src/core/or/origin_circuit_st.h | 8 -------- src/feature/hs/hs_circuit.c | 16 +--------------- src/feature/hs/hs_circuit.h | 2 +- src/feature/hs/hs_service.c | 3 +++ 5 files changed, 5 insertions(+), 25 deletions(-)
diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c index acb9a7fba1..dbeea10821 100644 --- a/src/core/or/circuituse.c +++ b/src/core/or/circuituse.c @@ -759,7 +759,6 @@ circuit_expire_building(void) (unsigned)victim->n_circ_id, victim->state, circuit_state_to_string(victim->state), victim->purpose); - hs_circ_retry_service_rendezvous_point(TO_ORIGIN_CIRCUIT(victim)); /* We'll close as a timeout the victim circuit. The rendezvous point * won't keep both circuits, it only keeps the newest (for the same * cookie). */ diff --git a/src/core/or/origin_circuit_st.h b/src/core/or/origin_circuit_st.h index 2cd8a33abc..73b971f72d 100644 --- a/src/core/or/origin_circuit_st.h +++ b/src/core/or/origin_circuit_st.h @@ -209,14 +209,6 @@ struct origin_circuit_t { * no circuits have opened. Used to prevent spamming logs. */ unsigned int relaxed_timeout : 1;
- /** Set iff this is a service-side rendezvous circuit for which a - * new connection attempt has been launched. We consider launching - * a new service-side rend circ to a client when the previous one - * fails; now that we don't necessarily close a service-side rend - * circ when we launch a new one to the same client, this flag keeps - * us from launching two retries for the same failed rend circ. */ - unsigned int hs_service_side_rend_circ_has_been_relaunched : 1; - /** What commands were sent over this circuit that decremented the * RELAY_EARLY counter? This is for debugging task 878. */ uint8_t relay_early_commands[MAX_RELAY_EARLY_CELLS_PER_CIRCUIT]; diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index 307557dc86..fa8f9e05cf 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -433,16 +433,6 @@ can_relaunch_service_rendezvous_point(const origin_circuit_t *circ)
/* XXX: Retrying under certain condition. This is related to #22455. */
- /* Avoid to relaunch twice a circuit to the same rendezvous point at the - * same time. */ - if (circ->hs_service_side_rend_circ_has_been_relaunched) { - log_info(LD_REND, "Rendezvous circuit to %s has already been retried. " - "Skipping retry.", - safe_str_client( - extend_info_describe(circ->build_state->chosen_exit))); - goto disallow; - } - /* We check failure_count >= hs_get_service_max_rend_failures()-1 below, and * the -1 is because we increment the failure count for our current failure * *after* this clause. */ @@ -684,7 +674,7 @@ hs_circ_service_get_established_intro_circ(const hs_service_intro_point_t *ip) * - We've already retried this specific rendezvous circuit. */ void -hs_circ_retry_service_rendezvous_point(origin_circuit_t *circ) +hs_circ_retry_service_rendezvous_point(const origin_circuit_t *circ) { tor_assert(circ); tor_assert(TO_CIRCUIT(circ)->purpose == CIRCUIT_PURPOSE_S_CONNECT_REND); @@ -694,10 +684,6 @@ hs_circ_retry_service_rendezvous_point(origin_circuit_t *circ) goto done; }
- /* Flag the circuit that we are relaunching, to avoid to relaunch twice a - * circuit to the same rendezvous point at the same time. */ - circ->hs_service_side_rend_circ_has_been_relaunched = 1; - /* Legacy services don't have a hidden service ident. */ if (circ->hs_ident) { retry_service_rendezvous_point(circ); diff --git a/src/feature/hs/hs_circuit.h b/src/feature/hs/hs_circuit.h index 808e648951..afbff7b894 100644 --- a/src/feature/hs/hs_circuit.h +++ b/src/feature/hs/hs_circuit.h @@ -33,7 +33,7 @@ int hs_circ_launch_intro_point(hs_service_t *service, int hs_circ_launch_rendezvous_point(const hs_service_t *service, const curve25519_public_key_t *onion_key, const uint8_t *rendezvous_cookie); -void hs_circ_retry_service_rendezvous_point(origin_circuit_t *circ); +void hs_circ_retry_service_rendezvous_point(const origin_circuit_t *circ);
origin_circuit_t *hs_circ_service_get_intro_circ( const hs_service_intro_point_t *ip); diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index 716386408a..4e971233af 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -3675,6 +3675,9 @@ hs_service_circuit_cleanup_on_close(const circuit_t *circ) hs_metrics_close_established_rdv( &CONST_TO_ORIGIN_CIRCUIT(circ)->hs_ident->identity_pk); break; + case CIRCUIT_PURPOSE_S_CONNECT_REND: + hs_circ_retry_service_rendezvous_point(CONST_TO_ORIGIN_CIRCUIT(circ)); + break; default: break; }
This is an automated email from the git hooks/post-receive script.
dgoulet pushed a commit to branch main in repository tor.
commit 4ed67fe174410386e500329e9e6ad73eef4be4e7 Author: David Goulet dgoulet@torproject.org AuthorDate: Mon Oct 24 11:14:50 2022 -0400
changes: Ticket 40694
Signed-off-by: David Goulet dgoulet@torproject.org --- changes/ticket40694 | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/changes/ticket40694 b/changes/ticket40694 new file mode 100644 index 0000000000..f17639cc27 --- /dev/null +++ b/changes/ticket40694 @@ -0,0 +1,5 @@ + o Major bugfixes (onion service): + - Set a much higher circuit build timeout for opened client rendezvous + circuit. Before this, tor would time them out very quickly leading to many + unnecessary retries and thus more load on the network. Fixes bug 40694; + bugfix on 0.3.5.1-alpha.
This is an automated email from the git hooks/post-receive script.
dgoulet pushed a commit to branch main in repository tor.
commit 88b5daf152edd7203989cfc0d077962a1c97623d Author: David Goulet dgoulet@torproject.org AuthorDate: Wed Oct 19 14:50:00 2022 -0400
circ: Set proper timeout cutoff for HS circuits
Explicitly set the S_CONNECT_REND purpose to a 4-hop cutoff.
As for the established rendezvous circuit waiting on the RENDEZVOUS2, set one that is very long considering the possible waiting time for the service to get the request and join our rendezvous.
Part of #40694
Signed-off-by: David Goulet dgoulet@torproject.org --- src/core/or/circuituse.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c index a259957d37..ad78e05d76 100644 --- a/src/core/or/circuituse.c +++ b/src/core/or/circuituse.c @@ -451,7 +451,8 @@ circuit_expire_building(void) * custom timeouts yet */ struct timeval general_cutoff, begindir_cutoff, fourhop_cutoff, close_cutoff, extremely_old_cutoff, hs_extremely_old_cutoff, - cannibalized_cutoff, c_intro_cutoff, s_intro_cutoff, stream_cutoff; + cannibalized_cutoff, c_intro_cutoff, s_intro_cutoff, stream_cutoff, + c_rend_ready_cutoff; const or_options_t *options = get_options(); struct timeval now; cpath_build_state_t *build_state; @@ -531,6 +532,16 @@ circuit_expire_building(void) /* Server intro circs have an extra round trip */ SET_CUTOFF(s_intro_cutoff, get_circuit_build_timeout_ms() * (9/6.0) + 1000);
+ /* For circuit purpose set to: CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED. + * + * The cutoff of such circuit is very high because we end up in this state if + * once the INTRODUCE_ACK is received which could be before the service + * receives the INTRODUCE2 cell. The worst case is a full 3-hop latency + * (intro -> service), 4-hop circuit creation latency (service -> RP), and + * finally a 7-hop latency for the RENDEZVOUS2 cell to arrive (service -> + * client). */ + SET_CUTOFF(c_rend_ready_cutoff, get_circuit_build_timeout_ms() * 3 + 1000); + SET_CUTOFF(close_cutoff, get_circuit_build_close_time_ms()); SET_CUTOFF(extremely_old_cutoff, get_circuit_build_close_time_ms()*2 + 1000);
@@ -568,8 +579,12 @@ circuit_expire_building(void) cutoff = c_intro_cutoff; else if (victim->purpose == CIRCUIT_PURPOSE_S_ESTABLISH_INTRO) cutoff = s_intro_cutoff; - else if (victim->purpose == CIRCUIT_PURPOSE_C_ESTABLISH_REND) - cutoff = stream_cutoff; + else if (victim->purpose == CIRCUIT_PURPOSE_S_CONNECT_REND) { + /* Service connecting to a rendezvous point is a four hop circuit. We set + * it explicitly here because this function is a clusterf***. */ + cutoff = fourhop_cutoff; + } else if (victim->purpose == CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED) + cutoff = c_rend_ready_cutoff; else if (victim->purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) cutoff = close_cutoff; else if (TO_ORIGIN_CIRCUIT(victim)->has_opened &&
This is an automated email from the git hooks/post-receive script.
dgoulet pushed a commit to branch main in repository tor.
commit 5b44a32c5964d78dcaf87ab9485efba4ddd8769a Author: David Goulet dgoulet@torproject.org AuthorDate: Wed Oct 19 15:11:11 2022 -0400
circ: Get rid of hs_circ_has_timed_out
Logic is too convoluted and we can't efficiently apply a specific timeout depending on the purpose.
Remove it and instead rely on the right circuit cutoff instead of keeping this flagged circuit open forever.
Part of #40694
Signed-off-by: David Goulet dgoulet@torproject.org --- src/core/or/circuituse.c | 86 +++++++++-------------------------------- src/core/or/origin_circuit_st.h | 16 -------- src/feature/hs/hs_circuit.c | 5 --- 3 files changed, 19 insertions(+), 88 deletions(-)
diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c index ad78e05d76..acb9a7fba1 100644 --- a/src/core/or/circuituse.c +++ b/src/core/or/circuituse.c @@ -133,11 +133,6 @@ circuit_is_acceptable(const origin_circuit_t *origin_circ, return 0; }
- /* If this is a timed-out hidden service circuit, skip it. */ - if (origin_circ->hs_circ_has_timed_out) { - return 0; - } - if (purpose == CIRCUIT_PURPOSE_C_GENERAL || purpose == CIRCUIT_PURPOSE_C_HSDIR_GET || purpose == CIRCUIT_PURPOSE_S_HSDIR_POST || @@ -334,7 +329,6 @@ circuit_get_best(const entry_connection_t *conn, { origin_circuit_t *best=NULL; struct timeval now; - int intro_going_on_but_too_old = 0;
tor_assert(conn);
@@ -353,15 +347,6 @@ circuit_get_best(const entry_connection_t *conn, continue; origin_circ = TO_ORIGIN_CIRCUIT(circ);
- /* Log an info message if we're going to launch a new intro circ in - * parallel */ - if (purpose == CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT && - !must_be_open && origin_circ->hs_circ_has_timed_out && - !circ->marked_for_close) { - intro_going_on_but_too_old = 1; - continue; - } - if (!circuit_is_acceptable(origin_circ,conn,must_be_open,purpose, need_uptime,need_internal, (time_t)now.tv_sec)) continue; @@ -374,11 +359,6 @@ circuit_get_best(const entry_connection_t *conn, } SMARTLIST_FOREACH_END(circ);
- if (!best && intro_going_on_but_too_old) - log_info(LD_REND|LD_CIRC, "There is an intro circuit being created " - "right now, but it has already taken quite a while. Starting " - "one in parallel."); - return best; }
@@ -450,7 +430,7 @@ circuit_expire_building(void) * circuit_build_times_get_initial_timeout() if we haven't computed * custom timeouts yet */ struct timeval general_cutoff, begindir_cutoff, fourhop_cutoff, - close_cutoff, extremely_old_cutoff, hs_extremely_old_cutoff, + close_cutoff, extremely_old_cutoff, cannibalized_cutoff, c_intro_cutoff, s_intro_cutoff, stream_cutoff, c_rend_ready_cutoff; const or_options_t *options = get_options(); @@ -545,10 +525,6 @@ circuit_expire_building(void) SET_CUTOFF(close_cutoff, get_circuit_build_close_time_ms()); SET_CUTOFF(extremely_old_cutoff, get_circuit_build_close_time_ms()*2 + 1000);
- SET_CUTOFF(hs_extremely_old_cutoff, - MAX(get_circuit_build_close_time_ms()*2 + 1000, - options->SocksTimeout * 1000)); - bool fixed_time = circuit_build_times_disabled(get_options());
SMARTLIST_FOREACH_BEGIN(circuit_get_global_list(), circuit_t *,victim) { @@ -595,9 +571,6 @@ circuit_expire_building(void) else cutoff = general_cutoff;
- if (TO_ORIGIN_CIRCUIT(victim)->hs_circ_has_timed_out) - cutoff = hs_extremely_old_cutoff; - if (timercmp(&victim->timestamp_began, &cutoff, OP_GT)) continue; /* it's still young, leave it alone */
@@ -769,51 +742,30 @@ circuit_expire_building(void) } }
- /* If this is a hidden service client circuit which is far enough along in - * connecting to its destination, and we haven't already flagged it as - * 'timed out', flag it so we'll launch another intro or rend circ, but - * don't mark it for close yet. - * - * (Circs flagged as 'timed out' are given a much longer timeout - * period above, so we won't close them in the next call to - * circuit_expire_building.) */ - if (!(TO_ORIGIN_CIRCUIT(victim)->hs_circ_has_timed_out)) { - switch (victim->purpose) { - case CIRCUIT_PURPOSE_C_REND_READY: - /* We only want to spare a rend circ iff it has been specified in an - * INTRODUCE1 cell sent to a hidden service. */ - if (!hs_circ_is_rend_sent_in_intro1(CONST_TO_ORIGIN_CIRCUIT(victim))) { - break; - } - FALLTHROUGH; - case CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT: - case CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED: - /* If we have reached this line, we want to spare the circ for now. */ - log_info(LD_CIRC,"Marking circ %u (state %d:%s, purpose %d) " - "as timed-out HS circ", - (unsigned)victim->n_circ_id, - victim->state, circuit_state_to_string(victim->state), - victim->purpose); - TO_ORIGIN_CIRCUIT(victim)->hs_circ_has_timed_out = 1; + /* Special checks for onion service circuits. */ + switch (victim->purpose) { + case CIRCUIT_PURPOSE_C_REND_READY: + /* We only want to spare a rend circ iff it has been specified in an + * INTRODUCE1 cell sent to a hidden service. */ + if (hs_circ_is_rend_sent_in_intro1(CONST_TO_ORIGIN_CIRCUIT(victim))) { continue; - default: - break; } - } - - /* If this is a service-side rendezvous circuit which is far - * enough along in connecting to its destination, consider sparing - * it. */ - if (!(TO_ORIGIN_CIRCUIT(victim)->hs_circ_has_timed_out) && - victim->purpose == CIRCUIT_PURPOSE_S_CONNECT_REND) { - log_info(LD_CIRC,"Marking circ %u (state %d:%s, purpose %d) " - "as timed-out HS circ; relaunching rendezvous attempt.", + break; + case CIRCUIT_PURPOSE_S_CONNECT_REND: + /* The connection to the rendezvous point has timed out, close it and + * retry if possible. */ + log_info(LD_CIRC,"Rendezvous circ %u (state %d:%s, purpose %d) " + "as timed-out, closing it. Relaunching rendezvous attempt.", (unsigned)victim->n_circ_id, victim->state, circuit_state_to_string(victim->state), victim->purpose); - TO_ORIGIN_CIRCUIT(victim)->hs_circ_has_timed_out = 1; hs_circ_retry_service_rendezvous_point(TO_ORIGIN_CIRCUIT(victim)); - continue; + /* We'll close as a timeout the victim circuit. The rendezvous point + * won't keep both circuits, it only keeps the newest (for the same + * cookie). */ + break; + default: + break; }
if (victim->n_chan) diff --git a/src/core/or/origin_circuit_st.h b/src/core/or/origin_circuit_st.h index 6c86a56000..2cd8a33abc 100644 --- a/src/core/or/origin_circuit_st.h +++ b/src/core/or/origin_circuit_st.h @@ -205,22 +205,6 @@ struct origin_circuit_t { * (in host byte order) for response comparison. */ uint32_t pathbias_probe_nonce;
- /** Set iff this is a hidden-service circuit which has timed out - * according to our current circuit-build timeout, but which has - * been kept around because it might still succeed in connecting to - * its destination, and which is not a fully-connected rendezvous - * circuit. - * - * (We clear this flag for client-side rendezvous circuits when they - * are 'joined' to the other side's rendezvous circuit, so that - * connection_ap_handshake_attach_circuit can put client streams on - * the circuit. We also clear this flag for service-side rendezvous - * circuits when they are 'joined' to a client's rend circ, but only - * for symmetry with the client case. Client-side introduction - * circuits are closed when we get a joined rend circ, and - * service-side introduction circuits never have this flag set.) */ - unsigned int hs_circ_has_timed_out : 1; - /** Set iff this circuit has been given a relaxed timeout because * no circuits have opened. Used to prevent spamming logs. */ unsigned int relaxed_timeout : 1; diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index edb5e692e7..42d3bedc3e 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -126,11 +126,6 @@ finalize_rend_circuit(origin_circuit_t *circ, crypt_path_t *hop, hop->package_window = circuit_initial_package_window(); hop->deliver_window = CIRCWINDOW_START;
- /* Now that this circuit has finished connecting to its destination, - * make sure circuit_get_open_circ_or_launch is willing to return it - * so we can actually use it. */ - circ->hs_circ_has_timed_out = 0; - /* If congestion control, transfer ccontrol onto the cpath. */ if (TO_CIRCUIT(circ)->ccontrol) { hop->ccontrol = TO_CIRCUIT(circ)->ccontrol;
This is an automated email from the git hooks/post-receive script.
dgoulet pushed a commit to branch main in repository tor.
commit 78c184d2fe2c5f14da8c65166521c7291225b5bc Author: David Goulet dgoulet@torproject.org AuthorDate: Wed Oct 19 15:27:22 2022 -0400
hs: Retry service rendezvous on circuit close
Move the retry from circuit_expire_building() to when the offending circuit is being closed.
Fixes #40695
Signed-off-by: David Goulet dgoulet@torproject.org --- src/core/or/circuituse.c | 1 - src/core/or/origin_circuit_st.h | 8 -------- src/feature/hs/hs_circuit.c | 16 +--------------- src/feature/hs/hs_circuit.h | 2 +- src/feature/hs/hs_service.c | 3 +++ 5 files changed, 5 insertions(+), 25 deletions(-)
diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c index acb9a7fba1..dbeea10821 100644 --- a/src/core/or/circuituse.c +++ b/src/core/or/circuituse.c @@ -759,7 +759,6 @@ circuit_expire_building(void) (unsigned)victim->n_circ_id, victim->state, circuit_state_to_string(victim->state), victim->purpose); - hs_circ_retry_service_rendezvous_point(TO_ORIGIN_CIRCUIT(victim)); /* We'll close as a timeout the victim circuit. The rendezvous point * won't keep both circuits, it only keeps the newest (for the same * cookie). */ diff --git a/src/core/or/origin_circuit_st.h b/src/core/or/origin_circuit_st.h index 2cd8a33abc..73b971f72d 100644 --- a/src/core/or/origin_circuit_st.h +++ b/src/core/or/origin_circuit_st.h @@ -209,14 +209,6 @@ struct origin_circuit_t { * no circuits have opened. Used to prevent spamming logs. */ unsigned int relaxed_timeout : 1;
- /** Set iff this is a service-side rendezvous circuit for which a - * new connection attempt has been launched. We consider launching - * a new service-side rend circ to a client when the previous one - * fails; now that we don't necessarily close a service-side rend - * circ when we launch a new one to the same client, this flag keeps - * us from launching two retries for the same failed rend circ. */ - unsigned int hs_service_side_rend_circ_has_been_relaunched : 1; - /** What commands were sent over this circuit that decremented the * RELAY_EARLY counter? This is for debugging task 878. */ uint8_t relay_early_commands[MAX_RELAY_EARLY_CELLS_PER_CIRCUIT]; diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index 42d3bedc3e..53855d40a9 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -433,16 +433,6 @@ can_relaunch_service_rendezvous_point(const origin_circuit_t *circ)
/* XXX: Retrying under certain condition. This is related to #22455. */
- /* Avoid to relaunch twice a circuit to the same rendezvous point at the - * same time. */ - if (circ->hs_service_side_rend_circ_has_been_relaunched) { - log_info(LD_REND, "Rendezvous circuit to %s has already been retried. " - "Skipping retry.", - safe_str_client( - extend_info_describe(circ->build_state->chosen_exit))); - goto disallow; - } - /* We check failure_count >= hs_get_service_max_rend_failures()-1 below, and * the -1 is because we increment the failure count for our current failure * *after* this clause. */ @@ -684,7 +674,7 @@ hs_circ_service_get_established_intro_circ(const hs_service_intro_point_t *ip) * - We've already retried this specific rendezvous circuit. */ void -hs_circ_retry_service_rendezvous_point(origin_circuit_t *circ) +hs_circ_retry_service_rendezvous_point(const origin_circuit_t *circ) { tor_assert(circ); tor_assert(TO_CIRCUIT(circ)->purpose == CIRCUIT_PURPOSE_S_CONNECT_REND); @@ -694,10 +684,6 @@ hs_circ_retry_service_rendezvous_point(origin_circuit_t *circ) goto done; }
- /* Flag the circuit that we are relaunching, to avoid to relaunch twice a - * circuit to the same rendezvous point at the same time. */ - circ->hs_service_side_rend_circ_has_been_relaunched = 1; - /* Legacy services don't have a hidden service ident. */ if (circ->hs_ident) { retry_service_rendezvous_point(circ); diff --git a/src/feature/hs/hs_circuit.h b/src/feature/hs/hs_circuit.h index 808e648951..afbff7b894 100644 --- a/src/feature/hs/hs_circuit.h +++ b/src/feature/hs/hs_circuit.h @@ -33,7 +33,7 @@ int hs_circ_launch_intro_point(hs_service_t *service, int hs_circ_launch_rendezvous_point(const hs_service_t *service, const curve25519_public_key_t *onion_key, const uint8_t *rendezvous_cookie); -void hs_circ_retry_service_rendezvous_point(origin_circuit_t *circ); +void hs_circ_retry_service_rendezvous_point(const origin_circuit_t *circ);
origin_circuit_t *hs_circ_service_get_intro_circ( const hs_service_intro_point_t *ip); diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index ff34e5dc44..1caa5ab64a 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -3675,6 +3675,9 @@ hs_service_circuit_cleanup_on_close(const circuit_t *circ) hs_metrics_close_established_rdv( &CONST_TO_ORIGIN_CIRCUIT(circ)->hs_ident->identity_pk); break; + case CIRCUIT_PURPOSE_S_CONNECT_REND: + hs_circ_retry_service_rendezvous_point(CONST_TO_ORIGIN_CIRCUIT(circ)); + break; default: break; }
This is an automated email from the git hooks/post-receive script.
dgoulet pushed a commit to branch main in repository tor.
commit 609a82a595db45603edc1d007a6657fe4c1b4f5f Author: David Goulet dgoulet@torproject.org AuthorDate: Mon Oct 24 11:14:50 2022 -0400
changes: Ticket 40694
Signed-off-by: David Goulet dgoulet@torproject.org --- changes/ticket40694 | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/changes/ticket40694 b/changes/ticket40694 new file mode 100644 index 0000000000..f17639cc27 --- /dev/null +++ b/changes/ticket40694 @@ -0,0 +1,5 @@ + o Major bugfixes (onion service): + - Set a much higher circuit build timeout for opened client rendezvous + circuit. Before this, tor would time them out very quickly leading to many + unnecessary retries and thus more load on the network. Fixes bug 40694; + bugfix on 0.3.5.1-alpha.
This is an automated email from the git hooks/post-receive script.
dgoulet pushed a commit to branch main in repository tor.
commit dd272b6ef4c28306a3c3966a7a69efd0c6ff42f4 Merge: 95f61cc84f 4ed67fe174 Author: David Goulet dgoulet@torproject.org AuthorDate: Wed Oct 26 15:12:54 2022 -0400
Merge remote-tracking branch 'tor-gitlab/mr/638'
changes/ticket40694 | 5 ++ src/core/or/circuituse.c | 108 ++++++++++++++-------------------------- src/core/or/origin_circuit_st.h | 24 --------- src/feature/hs/hs_circuit.c | 21 +------- src/feature/hs/hs_circuit.h | 2 +- src/feature/hs/hs_service.c | 3 ++ 6 files changed, 47 insertions(+), 116 deletions(-)
This is an automated email from the git hooks/post-receive script.
dgoulet pushed a commit to branch main in repository tor.
commit 8bf9c4be8d51e77c605c065c0b3f32f4dd56581b Merge: dd272b6ef4 609a82a595 Author: David Goulet dgoulet@torproject.org AuthorDate: Wed Oct 26 15:13:00 2022 -0400
Merge branch 'maint-0.4.7'
tor-commits@lists.torproject.org
-
gitolite role