diff options
| -rw-r--r-- | changes/bug23603 | 7 | ||||
| -rw-r--r-- | src/or/circuitlist.c | 26 | ||||
| -rw-r--r-- | src/or/hs_circuit.c | 28 | ||||
| -rw-r--r-- | src/or/hs_circuit.h | 3 | ||||
| -rw-r--r-- | src/or/hs_common.c | 1 | ||||
| -rw-r--r-- | src/or/hs_common.h | 2 |
6 files changed, 54 insertions, 13 deletions
diff --git a/changes/bug23603 b/changes/bug23603 new file mode 100644 index 0000000000..dfb2052c9a --- /dev/null +++ b/changes/bug23603 @@ -0,0 +1,7 @@ + o Minor bugfixes (hidden service v3): + - Fix a race between the circuit close and free where the service would + launch a new intro circuit after the close, and then fail to register it + before the free of the previously closed circuit. This was making the + service unable to find the established intro circuit and thus not upload + its descriptor. It can make a service unavailable for up to 24 hours. + Fixes bug 23603; bugfix on 0.3.2.1-alpha. diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index d37d986f17..45d4521c22 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -67,7 +67,6 @@ #include "main.h" #include "hs_circuit.h" #include "hs_circuitmap.h" -#include "hs_common.h" #include "hs_ident.h" #include "networkstatus.h" #include "nodelist.h" @@ -938,6 +937,12 @@ circuit_free(circuit_t *circ) circuit_clear_testing_cell_stats(circ); + /* Cleanup circuit from anything HS v3 related. We also do this when the + * circuit is closed. This is to avoid any code path that free registered + * circuits without closing them before. This needs to be done before the + * hs identifier is freed. */ + hs_circ_cleanup(circ); + if (CIRCUIT_IS_ORIGIN(circ)) { origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); mem = ocirc; @@ -963,7 +968,11 @@ circuit_free(circuit_t *circ) crypto_pk_free(ocirc->intro_key); rend_data_free(ocirc->rend_data); + + /* Finally, free the identifier of the circuit and nullify it so multiple + * cleanup will work. */ hs_ident_circuit_free(ocirc->hs_ident); + ocirc->hs_ident = NULL; tor_free(ocirc->dest_address); if (ocirc->socks_username) { @@ -1022,11 +1031,6 @@ circuit_free(circuit_t *circ) /* Remove from map. */ circuit_set_n_circid_chan(circ, 0, NULL); - /* Clear HS circuitmap token from this circ (if any) */ - if (circ->hs_token) { - hs_circuitmap_remove_circuit(circ); - } - /* Clear cell queue _after_ removing it from the map. Otherwise our * "active" checks will be violated. */ cell_queue_clear(&circ->n_chan_cells); @@ -1917,6 +1921,9 @@ circuit_mark_for_close_, (circuit_t *circ, int reason, int line, } } + /* Notify the HS subsystem that this circuit is closing. */ + hs_circ_cleanup(circ); + if (circuits_pending_close == NULL) circuits_pending_close = smartlist_new(); @@ -1997,13 +2004,6 @@ circuit_about_to_free(circuit_t *circ) orig_reason); } - /* Notify the HS subsystem for any intro point circuit closing so it can be - * dealt with cleanly. */ - if (circ->purpose == CIRCUIT_PURPOSE_S_ESTABLISH_INTRO || - circ->purpose == CIRCUIT_PURPOSE_S_INTRO) { - hs_service_intro_circ_has_closed(TO_ORIGIN_CIRCUIT(circ)); - } - if (circ->purpose == CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT) { origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); int timed_out = (reason == END_CIRC_REASON_TIMEOUT); diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index ee952f4d68..a58166ccde 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -1178,3 +1178,31 @@ hs_circ_send_establish_rendezvous(origin_circuit_t *circ) return -1; } +/* We are about to close or free this <b>circ</b>. Clean it up from any + * related HS data structures. This function can be called multiple times + * safely for the same circuit. */ +void +hs_circ_cleanup(circuit_t *circ) +{ + tor_assert(circ); + + /* If it's a service-side intro circ, notify the HS subsystem for the intro + * point circuit closing so it can be dealt with cleanly. */ + if (circ->purpose == CIRCUIT_PURPOSE_S_ESTABLISH_INTRO || + circ->purpose == CIRCUIT_PURPOSE_S_INTRO) { + hs_service_intro_circ_has_closed(TO_ORIGIN_CIRCUIT(circ)); + } + + /* Clear HS circuitmap token for this circ (if any). Very important to be + * done after the HS subsystem has been notified of the close else the + * circuit will not be found. + * + * We do this at the close if possible because from that point on, the + * circuit is good as dead. We can't rely on removing it in the circuit + * free() function because we open a race window between the close and free + * where we can't register a new circuit for the same intro point. */ + if (circ->hs_token) { + hs_circuitmap_remove_circuit(circ); + } +} + diff --git a/src/or/hs_circuit.h b/src/or/hs_circuit.h index 0a1186dbaa..63ff5e463c 100644 --- a/src/or/hs_circuit.h +++ b/src/or/hs_circuit.h @@ -15,6 +15,9 @@ #include "hs_service.h" +/* Cleanup function when the circuit is closed or/and freed. */ +void hs_circ_cleanup(circuit_t *circ); + /* Circuit API. */ int hs_circ_service_intro_has_opened(hs_service_t *service, hs_service_intro_point_t *ip, diff --git a/src/or/hs_common.c b/src/or/hs_common.c index a0f2af29cd..ad783a36fb 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -22,6 +22,7 @@ #include "hs_client.h" #include "hs_ident.h" #include "hs_service.h" +#include "hs_circuitmap.h" #include "policies.h" #include "rendcommon.h" #include "rendservice.h" diff --git a/src/or/hs_common.h b/src/or/hs_common.h index c95e59a6f8..18c4e70141 100644 --- a/src/or/hs_common.h +++ b/src/or/hs_common.h @@ -161,6 +161,8 @@ typedef struct hsdir_index_t { void hs_init(void); void hs_free_all(void); +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); |