diff --git a/changes/bug24895 b/changes/bug24895 new file mode 100644 index 0000000000..7edde94a0b --- /dev/null +++ b/changes/bug24895 @@ -0,0 +1,8 @@ + o Major bugfixes (onion services): + - Fix an "off by 2" error in counting rendezvous failures on the onion + service side. While we thought we would stop the rendezvous attempt + after one failed circuit, we were actually making three circuit attempts + before giving up. Now switch to a default of 2, and allow the consensus + parameter "hs_service_max_rdv_failures" to override. Fixes bug 24895; + bugfix on 0.0.6. + diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index c896eeb0cc..753c0fcd6a 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -485,9 +485,14 @@ can_relaunch_service_rendezvous_point(const origin_circuit_t *circ) 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. */ + int max_rend_failures = hs_get_service_max_rend_failures() - 1; + /* A failure count that has reached maximum allowed or circuit that expired, * we skip relaunching. */ - if (circ->build_state->failure_count > MAX_REND_FAILURES || + if (circ->build_state->failure_count > max_rend_failures || circ->build_state->expiry_time <= time(NULL)) { log_info(LD_REND, "Attempt to build a rendezvous circuit to %s has " "failed with %d attempts and expiry time %ld. " diff --git a/src/or/hs_common.c b/src/or/hs_common.c index 4c56b2d11f..0e7d789705 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -210,6 +210,23 @@ hs_check_service_private_dir(const char *username, const char *path, return 0; } +/* Default, minimum and maximum values for the maximum rendezvous failures + * consensus parameter. */ +#define MAX_REND_FAILURES_DEFAULT 2 +#define MAX_REND_FAILURES_MIN 1 +#define MAX_REND_FAILURES_MAX 10 + +/** How many times will a hidden service operator attempt to connect to + * a requested rendezvous point before giving up? */ +int +hs_get_service_max_rend_failures(void) +{ + return networkstatus_get_param(NULL, "hs_service_max_rdv_failures", + MAX_REND_FAILURES_DEFAULT, + MAX_REND_FAILURES_MIN, + MAX_REND_FAILURES_MAX); +} + /** Get the default HS time period length in minutes from the consensus. */ STATIC uint64_t get_time_period_length(void) diff --git a/src/or/hs_common.h b/src/or/hs_common.h index f59c2b6050..83ba1b8599 100644 --- a/src/or/hs_common.h +++ b/src/or/hs_common.h @@ -177,6 +177,8 @@ void hs_cleanup_circ(circuit_t *circ); int hs_check_service_private_dir(const char *username, const char *path, unsigned int dir_group_readable, unsigned int create); +int hs_get_service_max_rend_failures(void); + char *hs_path_from_filename(const char *directory, const char *filename); void hs_build_address(const ed25519_public_key_t *key, uint8_t version, char *addr_out); diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 06166d7cf3..52bd6683fd 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -2042,7 +2042,8 @@ rend_service_receive_introduction(origin_circuit_t *circuit, /* Launch a circuit to the client's chosen rendezvous point. */ - for (i=0;ibase_.purpose == CIRCUIT_PURPOSE_S_CONNECT_REND); - oldstate = oldcirc->build_state; tor_assert(oldstate);