diff --git a/src/or/hs_client.c b/src/or/hs_client.c index 9f4dba04d4..4f467c7ec6 100644 --- a/src/or/hs_client.c +++ b/src/or/hs_client.c @@ -162,6 +162,7 @@ static routerstatus_t * pick_hsdir_v3(const ed25519_public_key_t *onion_identity_pk) { int retval; + unsigned int is_new_tp = 0; char base64_blinded_pubkey[ED25519_BASE64_LEN + 1]; uint64_t current_time_period = hs_get_time_period_num(0); smartlist_t *responsible_hsdirs; @@ -182,8 +183,9 @@ pick_hsdir_v3(const ed25519_public_key_t *onion_identity_pk) } /* Get responsible hsdirs of service for this time period */ - hs_get_responsible_hsdirs(&blinded_pubkey, current_time_period, 0, 1, - responsible_hsdirs); + is_new_tp = hs_time_between_tp_and_srv(NULL, time(NULL)); + hs_get_responsible_hsdirs(&blinded_pubkey, current_time_period, + is_new_tp, 1, responsible_hsdirs); log_debug(LD_REND, "Found %d responsible HSDirs and about to pick one.", smartlist_len(responsible_hsdirs)); diff --git a/src/or/hs_common.c b/src/or/hs_common.c index 3bf423f855..ee08aff7b3 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -589,7 +589,7 @@ compute_disaster_srv(uint64_t time_period_num, uint8_t *srv_out) * would have to do it thousands of times in a row, we always cache the * computer disaster SRV (and its corresponding time period num) in case we * want to reuse it soon after. We need to cache two SRVs, one for each active - * time period (in case of overlap mode). + * time period. */ static uint8_t cached_disaster_srv[2][DIGEST256_LEN]; static uint64_t cached_time_period_nums[2] = {0}; @@ -1034,10 +1034,22 @@ hs_build_blinded_keypair(const ed25519_keypair_t *kp, memwipe(param, 0, sizeof(param)); } -/* Return true if overlap mode is active given the date in consensus. If - * consensus is NULL, then we use the latest live consensus we can find. */ +/* Return true if we are currently in the time segment between a new time + * period and a new SRV (in the real network that happens between 12:00 and + * 00:00 UTC). Here is a diagram showing exactly when this returns true: + * + * +------------------------------------------------------------------+ + * | | + * | 00:00 12:00 00:00 12:00 00:00 12:00 | + * | SRV#1 TP#1 SRV#2 TP#2 SRV#3 TP#3 | + * | | + * | $==========|-----------$===========|-----------$===========| | + * | ^^^^^^^^^^^^ ^^^^^^^^^^^^ | + * | | + * +------------------------------------------------------------------+ + */ MOCK_IMPL(int, -hs_overlap_mode_is_active, (const networkstatus_t *consensus, time_t now)) +hs_time_between_tp_and_srv, (const networkstatus_t *consensus, time_t now)) { time_t valid_after; time_t srv_start_time, tp_start_time; @@ -1049,19 +1061,18 @@ hs_overlap_mode_is_active, (const networkstatus_t *consensus, time_t now)) } } - /* We consider to be in overlap mode when we are in the period of time - * between a fresh SRV and the beginning of the new time period (in the - * normal network this is between 00:00 (inclusive) and 12:00 UTC - * (exclusive)) */ + /* Get start time of next TP and of current SRV protocol run, and check if we + * are between them. */ valid_after = consensus->valid_after; - srv_start_time =sr_state_get_start_time_of_current_protocol_run(valid_after); + srv_start_time = + sr_state_get_start_time_of_current_protocol_run(valid_after); tp_start_time = hs_get_start_time_of_next_time_period(srv_start_time); if (valid_after >= srv_start_time && valid_after < tp_start_time) { - return 1; + return 0; } - return 0; + return 1; } /* Return 1 if any virtual port in ports needs a circuit with good uptime. @@ -1267,9 +1278,9 @@ node_has_hsdir_index(const node_t *node) /* For a given blinded key and time period number, get the responsible HSDir * and put their routerstatus_t object in the responsible_dirs list. If * is_next_period is true, the next hsdir_index of the node_t is used. If - * is_client is true, the spread fetch consensus parameter is used else the - * spread store is used which is only for upload. This function can't fail but - * it is possible that the responsible_dirs list contains fewer nodes than + * 'for_fetching' is true, the spread fetch consensus parameter is used else + * the spread store is used which is only for upload. This function can't fail + * but it is possible that the responsible_dirs list contains fewer nodes than * expected. * * This function goes over the latest consensus routerstatus list and sorts it @@ -1277,8 +1288,8 @@ node_has_hsdir_index(const node_t *node) * node. All of this makes it a bit CPU intensive so use it wisely. */ void hs_get_responsible_hsdirs(const ed25519_public_key_t *blinded_pk, - uint64_t time_period_num, int is_next_period, - int is_client, smartlist_t *responsible_dirs) + uint64_t time_period_num, int is_new_tp, + int for_fetching, smartlist_t *responsible_dirs) { smartlist_t *sorted_nodes; /* The compare function used for the smartlist bsearch. We have two @@ -1322,10 +1333,10 @@ hs_get_responsible_hsdirs(const ed25519_public_key_t *blinded_pk, /* First thing we have to do is sort all node_t by hsdir_index. The * is_next_period tells us if we want the current or the next one. Set the * bsearch compare function also while we are at it. */ - if (is_client) { + if (for_fetching) { smartlist_sort(sorted_nodes, compare_node_fetch_hsdir_index); cmp_fct = compare_digest_to_fetch_hsdir_index; - } else if (is_next_period) { + } else if (is_new_tp) { smartlist_sort(sorted_nodes, compare_node_store_second_hsdir_index); cmp_fct = compare_digest_to_store_second_hsdir_index; } else { @@ -1341,8 +1352,8 @@ hs_get_responsible_hsdirs(const ed25519_public_key_t *blinded_pk, uint8_t hs_index[DIGEST256_LEN] = {0}; /* Number of node to add to the responsible dirs list depends on if we are * trying to fetch or store. A client always fetches. */ - int n_to_add = (is_client) ? hs_get_hsdir_spread_fetch() : - hs_get_hsdir_spread_store(); + int n_to_add = (for_fetching) ? hs_get_hsdir_spread_fetch() : + hs_get_hsdir_spread_store(); /* Get the index that we should use to select the node. */ hs_build_hs_index(replica, blinded_pk, time_period_num, hs_index); diff --git a/src/or/hs_common.h b/src/or/hs_common.h index f09bbe94dd..2229ae48ad 100644 --- a/src/or/hs_common.h +++ b/src/or/hs_common.h @@ -142,10 +142,12 @@ typedef struct rend_service_port_config_t { /* Hidden service directory index used in a node_t which is set once we set * the consensus. */ typedef struct hsdir_index_t { - /* Index to use when fetching a descriptor. */ + /* HSDir index to use when fetching a descriptor. */ uint8_t fetch[DIGEST256_LEN]; - /* Index to store the first and second descriptor. */ + /* HSDir index used by services to store their first and second + * descriptor. The first descriptor is the one that uses older TP and SRV + * values than the second one. */ uint8_t store_first[DIGEST256_LEN]; uint8_t store_second[DIGEST256_LEN]; } hsdir_index_t; @@ -202,7 +204,7 @@ time_t hs_get_start_time_of_next_time_period(time_t now); link_specifier_t *hs_link_specifier_dup(const link_specifier_t *lspec); -MOCK_DECL(int, hs_overlap_mode_is_active, +MOCK_DECL(int, hs_time_between_tp_and_srv, (const networkstatus_t *consensus, time_t now)); uint8_t *hs_get_current_srv(uint64_t time_period_num, @@ -222,8 +224,8 @@ int32_t hs_get_hsdir_spread_fetch(void); int32_t hs_get_hsdir_spread_store(void); void hs_get_responsible_hsdirs(const ed25519_public_key_t *blinded_pk, - uint64_t time_period_num, int is_next_period, - int is_client, smartlist_t *responsible_dirs); + uint64_t time_period_num, int is_next_period, + int for_fetching, smartlist_t *responsible_dirs); routerstatus_t *hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str); diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 8cdf95d193..41154d270e 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -2380,19 +2380,23 @@ set_descriptor_revision_counter(hs_descriptor_t *hs_desc) * if PublishHidServDescriptors is false. */ STATIC void upload_descriptor_to_all(const hs_service_t *service, - hs_service_descriptor_t *desc, int for_next_period) + hs_service_descriptor_t *desc) { + unsigned int is_new_tp = 0; smartlist_t *responsible_dirs = NULL; tor_assert(service); tor_assert(desc); + /* Do we have a new TP that is are we between a new time period and the next + * SRV creation? */ + is_new_tp = hs_time_between_tp_and_srv(NULL, approx_time()); /* Get our list of responsible HSDir. */ responsible_dirs = smartlist_new(); /* The parameter 0 means that we aren't a client so tell the function to use * the spread store consensus paremeter. */ hs_get_responsible_hsdirs(&desc->blinded_kp.pubkey, desc->time_period_num, - for_next_period, 0, responsible_dirs); + is_new_tp, 0, responsible_dirs); /** Clear list of previous hsdirs since we are about to upload to a new * list. Let's keep it up to date. */ @@ -2539,8 +2543,6 @@ run_upload_descriptor_event(time_t now) /* Run v3+ check. */ FOR_EACH_SERVICE_BEGIN(service) { FOR_EACH_DESCRIPTOR_BEGIN(service, desc) { - int for_next_period = 0; - /* If we were asked to re-examine the hash ring, and it changed, then schedule an upload */ if (consider_republishing_hs_descriptors && @@ -2566,12 +2568,7 @@ run_upload_descriptor_event(time_t now) * accurate because all circuits have been established. */ build_desc_intro_points(service, desc, now); - /* The next descriptor needs the next time period for computing the - * corresponding hashring. */ - if (desc == service->desc_next) { - for_next_period = 1; - } - upload_descriptor_to_all(service, desc, for_next_period); + upload_descriptor_to_all(service, desc); } FOR_EACH_DESCRIPTOR_END; } FOR_EACH_SERVICE_END; diff --git a/src/or/hs_service.h b/src/or/hs_service.h index 7a392d7420..248df27e10 100644 --- a/src/or/hs_service.h +++ b/src/or/hs_service.h @@ -337,8 +337,7 @@ STATIC int write_address_to_file(const hs_service_t *service, const char *fname_); STATIC void upload_descriptor_to_all(const hs_service_t *service, - hs_service_descriptor_t *desc, - int for_next_period); + hs_service_descriptor_t *desc); STATIC void service_desc_schedule_upload(hs_service_descriptor_t *desc, time_t now, diff --git a/src/or/nodelist.c b/src/or/nodelist.c index b8baee54f1..2dadfe54a8 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -208,8 +208,7 @@ node_set_hsdir_index(node_t *node, const networkstatus_t *ns) /* We always use the current time period for fetching descs */ fetch_tp = current_time_period_num; - /* Now extract the needed SRVs and time periods for building hsdir indices */ - if (!hs_overlap_mode_is_active(ns, now)) { + if (hs_time_between_tp_and_srv(ns, now)) { fetch_srv = hs_get_current_srv(fetch_tp, ns); store_first_tp = hs_get_previous_time_period_num(0); diff --git a/src/test/test_hs_common.c b/src/test/test_hs_common.c index ab8b943346..4d28f53af1 100644 --- a/src/test/test_hs_common.c +++ b/src/test/test_hs_common.c @@ -465,7 +465,7 @@ test_desc_reupload_logic(void *arg) } /* Now let's upload our desc to all hsdirs */ - upload_descriptor_to_all(service, desc, 0); + upload_descriptor_to_all(service, desc); /* Check that previous hsdirs were populated */ tt_int_op(smartlist_len(desc->previous_hsdirs), OP_EQ, 6); @@ -503,7 +503,7 @@ test_desc_reupload_logic(void *arg) tt_int_op(smartlist_len(desc->previous_hsdirs), OP_EQ, 6); /* Now order another upload and see that we keep having 6 prev hsdirs */ - upload_descriptor_to_all(service, desc, 0); + upload_descriptor_to_all(service, desc); /* Check that previous hsdirs were populated */ tt_int_op(smartlist_len(desc->previous_hsdirs), OP_EQ, 6); @@ -536,7 +536,7 @@ test_desc_reupload_logic(void *arg) tt_int_op(smartlist_len(desc->previous_hsdirs), OP_EQ, 0); /* Now reupload again: see that the prev hsdir set got populated again. */ - upload_descriptor_to_all(service, desc, 0); + upload_descriptor_to_all(service, desc); tt_int_op(smartlist_len(desc->previous_hsdirs), OP_EQ, 6); done: @@ -740,6 +740,55 @@ test_parse_extended_hostname(void *arg) done: ; } +static void +test_time_between_tp_and_srv(void *arg) +{ + int ret; + networkstatus_t ns; + (void) arg; + + /* This function should be returning true where "^" are: + * + * +------------------------------------------------------------------+ + * | | + * | 00:00 12:00 00:00 12:00 00:00 12:00 | + * | SRV#1 TP#1 SRV#2 TP#2 SRV#3 TP#3 | + * | | + * | $==========|-----------$===========|-----------$===========| | + * | ^^^^^^^^^^^^ ^^^^^^^^^^^^ | + * | | + * +------------------------------------------------------------------+ + */ + + ret = parse_rfc1123_time("Sat, 26 Oct 1985 00:00:00 UTC", &ns.valid_after); + tt_int_op(ret, OP_EQ, 0); + ret = hs_time_between_tp_and_srv(&ns, 0); + tt_int_op(ret, OP_EQ, 0); + + ret = parse_rfc1123_time("Sat, 26 Oct 1985 11:00:00 UTC", &ns.valid_after); + tt_int_op(ret, OP_EQ, 0); + ret = hs_time_between_tp_and_srv(&ns, 0); + tt_int_op(ret, OP_EQ, 0); + + ret = parse_rfc1123_time("Sat, 26 Oct 1985 12:00:00 UTC", &ns.valid_after); + tt_int_op(ret, OP_EQ, 0); + ret = hs_time_between_tp_and_srv(&ns, 0); + tt_int_op(ret, OP_EQ, 1); + + ret = parse_rfc1123_time("Sat, 26 Oct 1985 23:00:00 UTC", &ns.valid_after); + tt_int_op(ret, OP_EQ, 0); + ret = hs_time_between_tp_and_srv(&ns, 0); + tt_int_op(ret, OP_EQ, 1); + + ret = parse_rfc1123_time("Sat, 26 Oct 1985 00:00:00 UTC", &ns.valid_after); + tt_int_op(ret, OP_EQ, 0); + ret = hs_time_between_tp_and_srv(&ns, 0); + tt_int_op(ret, OP_EQ, 0); + + done: + ; +} + struct testcase_t hs_common_tests[] = { { "build_address", test_build_address, TT_FORK, NULL, NULL }, @@ -759,6 +808,8 @@ struct testcase_t hs_common_tests[] = { NULL, NULL }, { "parse_extended_hostname", test_parse_extended_hostname, TT_FORK, NULL, NULL }, + { "time_between_tp_and_srv", test_time_between_tp_and_srv, TT_FORK, + NULL, NULL }, END_OF_TESTCASES };