diff --git a/changes/ticket27471 b/changes/ticket27471 new file mode 100644 index 0000000000..ffe77d268e --- /dev/null +++ b/changes/ticket27471 @@ -0,0 +1,5 @@ + o Minor bugfixes (hidden service v3, client): + - When replacing a descriptor in the client cache with a newer descriptor, + make sure to close all client introduction circuits of the old + descriptor so we don't end up with unusable leftover circuits. Fixes bug + 27471; bugfix on 0.3.2.1-alpha. diff --git a/src/core/or/circuitlist.c b/src/core/or/circuitlist.c index 5ff142c15c..35efc6541f 100644 --- a/src/core/or/circuitlist.c +++ b/src/core/or/circuitlist.c @@ -1644,15 +1644,24 @@ circuit_get_ready_rend_circ_by_rend_data(const rend_data_t *rend_data) return NULL; } -/** Return the first service introduction circuit originating from the global - * circuit list after start or at the start of the list if start - * is NULL. Return NULL if no circuit is found. +/** Return the first introduction circuit originating from the global circuit + * list after start or at the start of the list if start is + * NULL. Return NULL if no circuit is found. * - * A service introduction point circuit has a purpose of either - * CIRCUIT_PURPOSE_S_ESTABLISH_INTRO or CIRCUIT_PURPOSE_S_INTRO. This does not - * return a circuit marked for close and its state must be open. */ + * If want_client_circ is true, then we are looking for client-side + * introduction circuits: A client introduction point circuit has a purpose of + * either CIRCUIT_PURPOSE_C_INTRODUCING, CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT + * or CIRCUIT_PURPOSE_C_INTRODUCE_ACKED. This does not return a circuit marked + * for close, but it returns circuits regardless of their circuit state. + * + * If want_client_circ is false, then we are looking for service-side + * introduction circuits: A service introduction point circuit has a purpose of + * either CIRCUIT_PURPOSE_S_ESTABLISH_INTRO or CIRCUIT_PURPOSE_S_INTRO. This + * does not return circuits marked for close, or in any state other than open. + */ origin_circuit_t * -circuit_get_next_service_intro_circ(origin_circuit_t *start) +circuit_get_next_intro_circ(const origin_circuit_t *start, + bool want_client_circ) { int idx = 0; smartlist_t *lst = circuit_get_global_list(); @@ -1664,13 +1673,29 @@ circuit_get_next_service_intro_circ(origin_circuit_t *start) for ( ; idx < smartlist_len(lst); ++idx) { circuit_t *circ = smartlist_get(lst, idx); - /* Ignore a marked for close circuit or purpose not matching a service - * intro point or if the state is not open. */ - if (circ->marked_for_close || circ->state != CIRCUIT_STATE_OPEN || - (circ->purpose != CIRCUIT_PURPOSE_S_ESTABLISH_INTRO && - circ->purpose != CIRCUIT_PURPOSE_S_INTRO)) { + /* Ignore a marked for close circuit or if the state is not open. */ + if (circ->marked_for_close) { continue; } + + /* Depending on whether we are looking for client or service circs, skip + * circuits with other purposes. */ + if (want_client_circ) { + if (circ->purpose != CIRCUIT_PURPOSE_C_INTRODUCING && + circ->purpose != CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT && + circ->purpose != CIRCUIT_PURPOSE_C_INTRODUCE_ACKED) { + continue; + } + } else { /* we are looking for service-side circs */ + if (circ->state != CIRCUIT_STATE_OPEN) { + continue; + } + if (circ->purpose != CIRCUIT_PURPOSE_S_ESTABLISH_INTRO && + circ->purpose != CIRCUIT_PURPOSE_S_INTRO) { + continue; + } + } + /* The purposes we are looking for are only for origin circuits so the * following is valid. */ return TO_ORIGIN_CIRCUIT(circ); diff --git a/src/core/or/circuitlist.h b/src/core/or/circuitlist.h index dac11431c9..cb89d1820d 100644 --- a/src/core/or/circuitlist.h +++ b/src/core/or/circuitlist.h @@ -202,7 +202,8 @@ origin_circuit_t *circuit_get_ready_rend_circ_by_rend_data( const rend_data_t *rend_data); origin_circuit_t *circuit_get_next_by_pk_and_purpose(origin_circuit_t *start, const uint8_t *digest, uint8_t purpose); -origin_circuit_t *circuit_get_next_service_intro_circ(origin_circuit_t *start); +origin_circuit_t *circuit_get_next_intro_circ(const origin_circuit_t *start, + bool want_client_circ); origin_circuit_t *circuit_get_next_service_rp_circ(origin_circuit_t *start); origin_circuit_t *circuit_get_next_service_hsdir_circ(origin_circuit_t *start); origin_circuit_t *circuit_find_to_cannibalize(uint8_t purpose, diff --git a/src/feature/hs/hs_cache.c b/src/feature/hs/hs_cache.c index b9bcb446a1..afd69e1bec 100644 --- a/src/feature/hs/hs_cache.c +++ b/src/feature/hs/hs_cache.c @@ -647,6 +647,13 @@ cache_store_as_client(hs_cache_client_descriptor_t *client_desc) } /* Remove old entry. Make space for the new one! */ remove_v3_desc_as_client(cache_entry); + + /* We just removed an old descriptor and will replace it. We'll close all + * intro circuits related to this old one so we don't have leftovers. We + * leave the rendezvous circuits opened because they could be in use. */ + hs_client_close_intro_circuits_from_desc(cache_entry->desc); + + /* Free it. */ cache_client_desc_free(cache_entry); } diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 11e24a3660..dfad216abb 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -1844,6 +1844,38 @@ hs_client_reextend_intro_circuit(origin_circuit_t *circ) return ret; } +/* Close all client introduction circuits related to the given descriptor. + * This is called with a descriptor that is about to get replaced in the + * client cache. + * + * Even though the introduction point might be exactly the same, we'll rebuild + * them if needed but the odds are very low that an existing matching + * introduction circuit exists at that stage. */ +void +hs_client_close_intro_circuits_from_desc(const hs_descriptor_t *desc) +{ + origin_circuit_t *ocirc = NULL; + + tor_assert(desc); + + /* We iterate over all client intro circuits because they aren't kept in the + * HS circuitmap. That is probably something we want to do one day. */ + while ((ocirc = circuit_get_next_intro_circ(ocirc, true))) { + if (ocirc->hs_ident == NULL) { + /* Not a v3 circuit, ignore it. */ + continue; + } + + /* Does it match any IP in the given descriptor? If not, ignore. */ + if (find_desc_intro_point_by_ident(ocirc->hs_ident, desc) == NULL) { + continue; + } + + /* We have a match. Close the circuit as consider it expired. */ + circuit_mark_for_close(TO_CIRCUIT(ocirc), END_CIRC_REASON_FINISHED); + } +} + /* Release all the storage held by the client subsystem. */ void hs_client_free_all(void) diff --git a/src/feature/hs/hs_client.h b/src/feature/hs/hs_client.h index fb4f9e9e9f..f6fb167ea2 100644 --- a/src/feature/hs/hs_client.h +++ b/src/feature/hs/hs_client.h @@ -77,6 +77,7 @@ int hs_config_client_authorization(const or_options_t *options, int validate_only); int hs_client_reextend_intro_circuit(origin_circuit_t *circ); +void hs_client_close_intro_circuits_from_desc(const hs_descriptor_t *desc); void hs_client_purge_state(void); diff --git a/src/feature/rend/rendservice.c b/src/feature/rend/rendservice.c index bae9da3fe5..d135581061 100644 --- a/src/feature/rend/rendservice.c +++ b/src/feature/rend/rendservice.c @@ -631,7 +631,7 @@ rend_service_prune_list_impl_(void) /* For every service introduction circuit we can find, see if we have a * matching surviving configured service. If not, close the circuit. */ - while ((ocirc = circuit_get_next_service_intro_circ(ocirc))) { + while ((ocirc = circuit_get_next_intro_circ(ocirc, false))) { int keep_it = 0; if (ocirc->rend_data == NULL) { /* This is a v3 circuit, ignore it. */