commit 72c7f81459e087e2a0485361eb34db1023d12155 Author: David Goulet <dgoulet@torproject.org> Date: Fri Sep 1 16:18:50 2017 -0400 prop224: When computing hsdir index and time period, use valid_after time Use the valid_after time from the consensus to get the time period number else we might get out of sync with the overlap period that uses valid_after. Make it an optional feature since some functions require passing a specific time (like hs_get_start_time_of_next_time_period()). Signed-off-by: David Goulet <dgoulet@torproject.org> --- src/or/hs_client.c | 7 ++++--- src/or/hs_common.c | 15 +++++++++++++-- src/or/hs_service.c | 4 ++-- src/or/nodelist.c | 8 +++++--- src/test/test_hs_service.c | 22 ++++++++++++++++++++-- 5 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/or/hs_client.c b/src/or/hs_client.c index 0e8fc6c24..9f4dba04d 100644 --- a/src/or/hs_client.c +++ b/src/or/hs_client.c @@ -29,6 +29,7 @@ #include "connection.h" #include "hs_ntor.h" #include "circuitbuild.h" +#include "networkstatus.h" /* Get all connections that are waiting on a circuit and flag them back to * waiting for a hidden service descriptor for the given service key @@ -110,7 +111,7 @@ static int directory_launch_v3_desc_fetch(const ed25519_public_key_t *onion_identity_pk, const routerstatus_t *hsdir) { - uint64_t current_time_period = hs_get_time_period_num(approx_time()); + uint64_t current_time_period = hs_get_time_period_num(0); ed25519_public_key_t blinded_pubkey; char base64_blinded_pubkey[ED25519_BASE64_LEN + 1]; hs_ident_dir_conn_t hs_conn_dir_ident; @@ -162,7 +163,7 @@ pick_hsdir_v3(const ed25519_public_key_t *onion_identity_pk) { int retval; char base64_blinded_pubkey[ED25519_BASE64_LEN + 1]; - uint64_t current_time_period = hs_get_time_period_num(approx_time()); + uint64_t current_time_period = hs_get_time_period_num(0); smartlist_t *responsible_hsdirs; ed25519_public_key_t blinded_pubkey; routerstatus_t *hsdir_rs = NULL; @@ -906,7 +907,7 @@ hs_client_decode_descriptor(const char *desc_str, /* Create subcredential for this HS so that we can decrypt */ { - uint64_t current_time_period = hs_get_time_period_num(approx_time()); + uint64_t current_time_period = hs_get_time_period_num(0); hs_build_blinded_pubkey(service_identity_pk, NULL, 0, current_time_period, &blinded_pubkey); hs_get_subcredential(service_identity_pk, &blinded_pubkey, subcredential); diff --git a/src/or/hs_common.c b/src/or/hs_common.c index 5ea44b97e..36f94507e 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -211,15 +211,26 @@ get_time_period_length(void) return (uint64_t) time_period_length; } -/** Get the HS time period number at time <b>now</b> */ +/** Get the HS time period number at time <b>now</b>. If <b>now</b> is not set, + * we try to get the time ourselves. */ uint64_t hs_get_time_period_num(time_t now) { uint64_t time_period_num; + time_t current_time; + + /* If no time is specified, set current time based on consensus time, and + * only fall back to system time if that fails. */ + if (now != 0) { + current_time = now; + } else { + networkstatus_t *ns = networkstatus_get_live_consensus(approx_time()); + current_time = ns ? ns->valid_after : approx_time(); + } /* Start by calculating minutes since the epoch */ uint64_t time_period_length = get_time_period_length(); - uint64_t minutes_since_epoch = now / 60; + uint64_t minutes_since_epoch = current_time / 60; /* Apply the rotation offset as specified by prop224 (section * [TIME-PERIODS]), so that new time periods synchronize nicely with SRV diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 706890e34..49dfa2c4f 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -1376,7 +1376,7 @@ build_all_descriptors(time_t now) /* This means we just booted up because else this descriptor will never * be NULL as it should always point to the descriptor that was in * desc_next after rotation. */ - build_service_descriptor(service, now, hs_get_time_period_num(now), + build_service_descriptor(service, now, hs_get_time_period_num(0), &service->desc_current); log_info(LD_REND, "Hidden service %s current descriptor successfully " @@ -1387,7 +1387,7 @@ build_all_descriptors(time_t now) * we are in the overlap period for the _next_ time period since it means * we either just booted or we just rotated our descriptors. */ if (hs_overlap_mode_is_active(NULL, now) && service->desc_next == NULL) { - build_service_descriptor(service, now, hs_get_next_time_period_num(now), + build_service_descriptor(service, now, hs_get_next_time_period_num(0), &service->desc_next); log_info(LD_REND, "Hidden service %s next descriptor successfully " "built. Now scheduled for upload.", diff --git a/src/or/nodelist.c b/src/or/nodelist.c index 03b315e68..80f3b2b0a 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -200,9 +200,11 @@ node_set_hsdir_index(node_t *node, const networkstatus_t *ns) goto done; } - /* Get the current and next time period number, we might use them both. */ - current_time_period_num = hs_get_time_period_num(now); - next_time_period_num = hs_get_next_time_period_num(now); + /* Get the current and next time period number, we might use them both. We + * use the valid_after time of the consensus because we use that time to + * detect if we are in the overlap period or not. */ + current_time_period_num = hs_get_time_period_num(0); + next_time_period_num = hs_get_next_time_period_num(0); if (hs_overlap_mode_is_active(ns, now)) { /* We are in overlap mode, this means that our consensus has just cycled diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index 4c7880c0e..357fade04 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -1043,8 +1043,6 @@ test_build_update_descriptors(void *arg) { int ret; time_t now = time(NULL); - uint64_t period_num = hs_get_time_period_num(now); - uint64_t next_period_num = hs_get_next_time_period_num(now); node_t *node; hs_service_t *service; hs_service_intro_point_t *ip_cur, *ip_next; @@ -1055,9 +1053,20 @@ test_build_update_descriptors(void *arg) MOCK(hs_overlap_mode_is_active, mock_hs_overlap_mode_is_active_true); MOCK(get_or_state, get_or_state_replacement); + MOCK(networkstatus_get_live_consensus, + mock_networkstatus_get_live_consensus); dummy_state = tor_malloc_zero(sizeof(or_state_t)); + ret = parse_rfc1123_time("Sat, 26 Oct 1985 13:00:00 UTC", + &mock_ns.valid_after); + ret = parse_rfc1123_time("Sat, 26 Oct 1985 14:00:00 UTC", + &mock_ns.fresh_until); + tt_int_op(ret, OP_EQ, 0); + + uint64_t period_num = hs_get_time_period_num(0); + uint64_t next_period_num = hs_get_next_time_period_num(0); + /* Create a service without a current descriptor to trigger a build. */ service = hs_service_new(get_options()); tt_assert(service); @@ -1203,8 +1212,17 @@ test_upload_descriptors(void *arg) MOCK(hs_overlap_mode_is_active, mock_hs_overlap_mode_is_active_true); MOCK(get_or_state, get_or_state_replacement); + MOCK(networkstatus_get_live_consensus, + mock_networkstatus_get_live_consensus); + dummy_state = tor_malloc_zero(sizeof(or_state_t)); + ret = parse_rfc1123_time("Sat, 26 Oct 1985 13:00:00 UTC", + &mock_ns.valid_after); + ret = parse_rfc1123_time("Sat, 26 Oct 1985 14:00:00 UTC", + &mock_ns.fresh_until); + tt_int_op(ret, OP_EQ, 0); + /* Create a service with no descriptor. It's added to the global map. */ service = hs_service_new(get_options()); tt_assert(service);