commit b89d2fa1db2379bffd2e2b4c851c3facc57b6ed8 Author: George Kadianakis <desnacked@riseup.net> Date: Fri Aug 4 12:21:14 2017 +0300 Don't set HSDir index if we don't have a live consensus. We also had to alter the SRV functions to take a consensus as optional input, since we might be setting our HSDir index using a consensus that is currently being processed and won't be returned by the networkstatus_get_live_consensus() function. This change has two results: a) It makes sure we are using a fresh consensus with the right SRV value when we are calculating the HSDir hash ring. b) It ensures that we will not use the sr_get_current/previous() functions when we don't have a consensus which would have falsely triggered the disaster SRV logic. --- src/or/hs_common.c | 11 ++++++----- src/or/hs_common.h | 6 ++++-- src/or/networkstatus.c | 15 +++++++++++---- src/or/networkstatus.h | 1 + src/or/nodelist.c | 13 +++++++++---- src/or/shared_random.c | 38 ++++++++++++++++++++++++++++++-------- src/or/shared_random.h | 4 ++-- 7 files changed, 63 insertions(+), 25 deletions(-) diff --git a/src/or/hs_common.c b/src/or/hs_common.c index 570c1132c..2894d0a28 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -1058,12 +1058,13 @@ hs_build_hsdir_index(const ed25519_public_key_t *identity_pk, /* Return a newly allocated buffer containing the current shared random value * or if not present, a disaster value is computed using the given time period - * number. This function can't fail. */ + * number. If a consensus is provided in <b>ns</b>, use it to get the SRV + * value. This function can't fail. */ uint8_t * -hs_get_current_srv(uint64_t time_period_num) +hs_get_current_srv(uint64_t time_period_num, const networkstatus_t *ns) { uint8_t *sr_value = tor_malloc_zero(DIGEST256_LEN); - const sr_srv_t *current_srv = sr_get_current(); + const sr_srv_t *current_srv = sr_get_current(ns); if (current_srv) { memcpy(sr_value, current_srv->value, sizeof(current_srv->value)); @@ -1078,10 +1079,10 @@ hs_get_current_srv(uint64_t time_period_num) * value or if not present, a disaster value is computed using the given time * period number. This function can't fail. */ uint8_t * -hs_get_previous_srv(uint64_t time_period_num) +hs_get_previous_srv(uint64_t time_period_num, const networkstatus_t *ns) { uint8_t *sr_value = tor_malloc_zero(DIGEST256_LEN); - const sr_srv_t *previous_srv = sr_get_previous(); + const sr_srv_t *previous_srv = sr_get_previous(ns); if (previous_srv) { memcpy(sr_value, previous_srv->value, sizeof(previous_srv->value)); diff --git a/src/or/hs_common.h b/src/or/hs_common.h index 268a69bb5..7e37f81e5 100644 --- a/src/or/hs_common.h +++ b/src/or/hs_common.h @@ -200,8 +200,10 @@ link_specifier_t *hs_link_specifier_dup(const link_specifier_t *lspec); MOCK_DECL(int, hs_overlap_mode_is_active, (const networkstatus_t *consensus, time_t now)); -uint8_t *hs_get_current_srv(uint64_t time_period_num); -uint8_t *hs_get_previous_srv(uint64_t time_period_num); +uint8_t *hs_get_current_srv(uint64_t time_period_num, + const networkstatus_t *ns); +uint8_t *hs_get_previous_srv(uint64_t time_period_num, + const networkstatus_t *ns); void hs_build_hsdir_index(const ed25519_public_key_t *identity_pk, const uint8_t *srv, uint64_t period_num, diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index aff36b4c0..82ceb8a9e 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1393,14 +1393,21 @@ networkstatus_get_latest_consensus_by_flavor,(consensus_flavor_t f)) MOCK_IMPL(networkstatus_t *, networkstatus_get_live_consensus,(time_t now)) { - if (networkstatus_get_latest_consensus() && - networkstatus_get_latest_consensus()->valid_after <= now && - now <= networkstatus_get_latest_consensus()->valid_until) - return networkstatus_get_latest_consensus(); + networkstatus_t *ns = networkstatus_get_latest_consensus(); + if (ns && networkstatus_is_live(ns, now)) + return ns; else return NULL; } +/** Given a consensus in <b>ns</b>, validate that it's currently live and + * unexpired. */ +int +networkstatus_is_live(const networkstatus_t *ns, time_t now) +{ + return (ns->valid_after <= now && now <= ns->valid_until); +} + /** Determine if <b>consensus</b> is valid or expired recently enough that * we can still use it. * diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h index e774c4d26..f9320747d 100644 --- a/src/or/networkstatus.h +++ b/src/or/networkstatus.h @@ -81,6 +81,7 @@ MOCK_DECL(networkstatus_t *,networkstatus_get_latest_consensus,(void)); MOCK_DECL(networkstatus_t *,networkstatus_get_latest_consensus_by_flavor, (consensus_flavor_t f)); MOCK_DECL(networkstatus_t *, networkstatus_get_live_consensus,(time_t now)); +int networkstatus_is_live(const networkstatus_t *ns, time_t now); int networkstatus_consensus_reasonably_live(const networkstatus_t *consensus, time_t now); int networkstatus_valid_until_is_reasonably_live(time_t valid_until, diff --git a/src/or/nodelist.c b/src/or/nodelist.c index e900b5145..0fcaea626 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -179,7 +179,7 @@ node_get_or_create(const char *identity_digest) static void node_set_hsdir_index(node_t *node, const networkstatus_t *ns) { - time_t now = time(NULL); + time_t now = approx_time(); const ed25519_public_key_t *node_identity_pk; uint8_t *next_hsdir_index_srv = NULL, *current_hsdir_index_srv = NULL; uint64_t next_time_period_num, current_time_period_num; @@ -187,6 +187,11 @@ node_set_hsdir_index(node_t *node, const networkstatus_t *ns) tor_assert(node); tor_assert(ns); + if (!networkstatus_is_live(ns, now)) { + log_info(LD_GENERAL, "Not setting hsdir index with a non-live consensus."); + goto done; + } + node_identity_pk = node_get_ed25519_id(node); if (node_identity_pk == NULL) { log_debug(LD_GENERAL, "ed25519 identity public key not found when " @@ -205,15 +210,15 @@ node_set_hsdir_index(node_t *node, const networkstatus_t *ns) * period, we have to use the current SRV and use the previous SRV for the * current time period. If the current or previous SRV can't be found, the * disaster one is returned. */ - next_hsdir_index_srv = hs_get_current_srv(next_time_period_num); + next_hsdir_index_srv = hs_get_current_srv(next_time_period_num, ns); /* The following can be confusing so again, in overlap mode, we use our * previous SRV for our _current_ hsdir index. */ - current_hsdir_index_srv = hs_get_previous_srv(current_time_period_num); + current_hsdir_index_srv = hs_get_previous_srv(current_time_period_num, ns); } else { /* If NOT in overlap mode, we only need to compute the current hsdir index * for the ongoing time period and thus the current SRV. If it can't be * found, the disaster one is returned. */ - current_hsdir_index_srv = hs_get_current_srv(current_time_period_num); + current_hsdir_index_srv = hs_get_current_srv(current_time_period_num, ns); } /* Build the current hsdir index. */ diff --git a/src/or/shared_random.c b/src/or/shared_random.c index ec2533dad..e4ee64139 100644 --- a/src/or/shared_random.c +++ b/src/or/shared_random.c @@ -1393,11 +1393,22 @@ sr_get_previous_for_control(void) /* Return current shared random value from the latest consensus. Caller can * NOT keep a reference to the returned pointer. Return NULL if none. */ const sr_srv_t * -sr_get_current(void) +sr_get_current(const networkstatus_t *ns) { - const networkstatus_t *c = networkstatus_get_latest_consensus(); - if (c) { - return c->sr_info.current_srv; + const networkstatus_t *consensus; + + /* Use provided ns else get a live one */ + if (ns) { + consensus = ns; + } else { + consensus = networkstatus_get_live_consensus(approx_time()); + } + /* Ideally we would never be asked for an SRV without a live consensus. Make + * sure this assumption is correct. */ + tor_assert_nonfatal(consensus); + + if (consensus) { + return consensus->sr_info.current_srv; } return NULL; } @@ -1405,11 +1416,22 @@ sr_get_current(void) /* Return previous shared random value from the latest consensus. Caller can * NOT keep a reference to the returned pointer. Return NULL if none. */ const sr_srv_t * -sr_get_previous(void) +sr_get_previous(const networkstatus_t *ns) { - const networkstatus_t *c = networkstatus_get_latest_consensus(); - if (c) { - return c->sr_info.previous_srv; + const networkstatus_t *consensus; + + /* Use provided ns else get a live one */ + if (ns) { + consensus = ns; + } else { + consensus = networkstatus_get_live_consensus(approx_time()); + } + /* Ideally we would never be asked for an SRV without a live consensus. Make + * sure this assumption is correct. */ + tor_assert_nonfatal(consensus); + + if (consensus) { + return consensus->sr_info.previous_srv; } return NULL; } diff --git a/src/or/shared_random.h b/src/or/shared_random.h index 58ea360df..76d5b9542 100644 --- a/src/or/shared_random.h +++ b/src/or/shared_random.h @@ -130,8 +130,8 @@ sr_commit_t *sr_generate_our_commit(time_t timestamp, char *sr_get_current_for_control(void); char *sr_get_previous_for_control(void); -const sr_srv_t *sr_get_current(void); -const sr_srv_t *sr_get_previous(void); +const sr_srv_t *sr_get_current(const networkstatus_t *ns); +const sr_srv_t *sr_get_previous(const networkstatus_t *ns); #ifdef SHARED_RANDOM_PRIVATE