commit d67bf8b2f2376096ada2b1ea1c6cd19ddbed9ead Author: David Goulet dgoulet@ev0ke.net Date: Mon Jun 1 13:17:37 2015 -0400

Upload descriptor when all intro points are ready

To upload a HS descriptor, this commits makes it that we wait for all introduction point to be fully established.

Else, the HS ends up uploading a descriptor that may contain intro points that are not yet "valid" meaning not yet established or proven to work. It could also trigger three uploads for the *same* descriptor if every intro points takes more than 30 seconds to establish because of desc_is_dirty being set at each intro established.

To achieve that, n_intro_points_established varialbe is added to the rend_service_t object that is incremented when we established introduction point and decremented when we remove a valid intro point from our list.

The condition to upload a descriptor also changes to test if all intro points are ready by making sure we have equal or more wanted intro points that are ready.

The desc_id_dirty flag is kept to be able to still use the RendInitialPostPeriod option.

This partially fixes #13483.

Signed-off-by: David Goulet dgoulet@ev0ke.net --- src/or/or.h | 4 ++++ src/or/rendservice.c | 58 ++++++++++++++++++++++++++++++++------------------ 2 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/src/or/or.h b/src/or/or.h index 0deb4a7..55ad1dd 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4942,6 +4942,10 @@ typedef struct rend_intro_point_t { * still in the consensus. After MAX_INTRO_POINT_CIRCUIT_RETRIES, we give * up on it. */ unsigned int circuit_retries; + + /** (Service side only) Set if this intro point has an established circuit + * and unset if it doesn't. */ + unsigned int circuit_established:1; } rend_intro_point_t;

#define REND_PROTOCOL_VERSION_BITMASK_WIDTH 16 diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 8c383a8..a86245b 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -2689,24 +2689,16 @@ rend_service_launch_establish_intro(rend_service_t *service, }

/** Return the number of introduction points that are or have been - * established for the given service address in <b>query</b>. */ -static int -count_established_intro_points(const char *query) + * established for the given service. */ +static unsigned int +count_established_intro_points(const rend_service_t *service) { - int num_ipos = 0; - SMARTLIST_FOREACH_BEGIN(circuit_get_global_list(), circuit_t *, circ) { - if (!circ->marked_for_close && - circ->state == CIRCUIT_STATE_OPEN && - (circ->purpose == CIRCUIT_PURPOSE_S_ESTABLISH_INTRO || - circ->purpose == CIRCUIT_PURPOSE_S_INTRO)) { - origin_circuit_t *oc = TO_ORIGIN_CIRCUIT(circ); - if (oc->rend_data && - !rend_cmp_service_ids(query, oc->rend_data->onion_address)) - num_ipos++; - } - } - SMARTLIST_FOREACH_END(circ); - return num_ipos; + unsigned int num = 0; + + SMARTLIST_FOREACH(service->intro_nodes, rend_intro_point_t *, intro, + num += intro->circuit_established + ); + return num; }

/** Called when we're done building a circuit to an introduction point: @@ -2747,9 +2739,8 @@ rend_service_intro_has_opened(origin_circuit_t *circuit) * redefine this one as a general circuit or close it, depending. * Substract the amount of expiring nodes here since the circuits are * still opened. */ - if ((count_established_intro_points(serviceid) - - smartlist_len(service->expiring_nodes)) > - (int)service->n_intro_points_wanted) { /* XXX023 remove cast */ + if (count_established_intro_points(service) > + service->n_intro_points_wanted) { const or_options_t *options = get_options(); /* Remove the intro point associated with this circuit, it's being * repurposed or closed thus cleanup memory. */ @@ -2857,6 +2848,7 @@ rend_service_intro_established(origin_circuit_t *circuit, size_t request_len) { rend_service_t *service; + rend_intro_point_t *intro; char serviceid[REND_SERVICE_ID_LEN_BASE32+1]; (void) request; (void) request_len; @@ -2874,6 +2866,19 @@ rend_service_intro_established(origin_circuit_t *circuit, (unsigned)circuit->base_.n_circ_id); goto err; } + /* We've just successfully established a intro circuit to one of our + * introduction point, account for it. */ + intro = find_intro_point(circuit); + if (intro == NULL) { + log_warn(LD_REND, + "Introduction circuit established without a rend_intro_point_t " + "object for service %s on circuit %u", + safe_str_client(serviceid), (unsigned)circuit->base_.n_circ_id); + goto err; + } + intro->circuit_established = 1; + /* We might not have every introduction point ready but at this point we + * know that the descriptor needs to be uploaded. */ service->desc_is_dirty = time(NULL); circuit_change_purpose(TO_CIRCUIT(circuit), CIRCUIT_PURPOSE_S_INTRO);

@@ -3390,6 +3395,10 @@ remove_invalid_intro_points(rend_service_t *service, " (circuit disappeared).", safe_str_client(extend_info_describe(intro->extend_info)), safe_str_client(service->service_id)); + /* We've lost the circuit for this intro point, flag it so it can be + * accounted for when considiring uploading a descriptor. */ + intro->circuit_established = 0; + /* Node is gone or we've reached our maximum circuit creationg retry * count, clean up everything, we'll find a new one. */ if (node == NULL || @@ -3421,6 +3430,9 @@ remove_invalid_intro_points(rend_service_t *service, safe_str_client(service->service_id)); smartlist_add(service->expiring_nodes, intro); SMARTLIST_DEL_CURRENT(service->intro_nodes, intro); + /* Intro point is expired, we need a new one thus don't consider it + * anymore has a valid established intro point. */ + intro->circuit_established = 0; } } SMARTLIST_FOREACH_END(intro); } @@ -3639,9 +3651,13 @@ rend_consider_services_upload(time_t now) service->next_upload_time = now + rendinitialpostdelay + crypto_rand_int(2*rendpostperiod); } + /* Does every introduction points have been established? */ + unsigned int intro_points_ready = + count_established_intro_points(service) >= service->n_intro_points_wanted; if (service->next_upload_time < now || (service->desc_is_dirty && - service->desc_is_dirty < now-rendinitialpostdelay)) { + service->desc_is_dirty < now-rendinitialpostdelay && + intro_points_ready)) { /* if it's time, or if the directory servers have a wrong service * descriptor and ours has been stable for rendinitialpostdelay seconds, * upload a new one of each format. */