commit bfe5a739b79510d7ab32dad6e9882dc7b8bf024f Author: Neel Chauhan neel@neelc.org Date: Sat Apr 28 19:56:12 2018 -0400

Make hsdir_index in node_t a hsdir_index_t rather than a pointer. --- changes/bug23094 | 4 ++++ src/or/hs_common.c | 27 ++++++++++++--------------- src/or/hs_common.h | 13 ------------- src/or/hs_control.c | 3 +-- src/or/hs_service.c | 4 ++-- src/or/nodelist.c | 16 +++++++--------- src/or/or.h | 17 ++++++++++++++--- src/test/test_hs_control.c | 5 ++--- 8 files changed, 42 insertions(+), 47 deletions(-)

diff --git a/changes/bug23094 b/changes/bug23094 new file mode 100644 index 000000000..5040ab4e7 --- /dev/null +++ b/changes/bug23094 @@ -0,0 +1,4 @@ + o Code simplification and refactoring: + - Make hsdir_index in node_t a hsdir_index_t rather than a pointer + as hsdir_index is always present. Also, we move hsdir_index_t into + or.h. Closes ticket 23094. Patch by Neel Chauhan. diff --git a/src/or/hs_common.c b/src/or/hs_common.c index 24eb7a104..c01428099 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -103,7 +103,7 @@ compare_digest_to_fetch_hsdir_index(const void *_key, const void **_member) { const char *key = _key; const node_t *node = *_member; - return tor_memcmp(key, node->hsdir_index->fetch, DIGEST256_LEN); + return tor_memcmp(key, node->hsdir_index.fetch, DIGEST256_LEN); }

/* Helper function: The key is a digest that we compare to a node_t object @@ -114,7 +114,7 @@ compare_digest_to_store_first_hsdir_index(const void *_key, { const char *key = _key; const node_t *node = *_member; - return tor_memcmp(key, node->hsdir_index->store_first, DIGEST256_LEN); + return tor_memcmp(key, node->hsdir_index.store_first, DIGEST256_LEN); }

/* Helper function: The key is a digest that we compare to a node_t object @@ -125,7 +125,7 @@ compare_digest_to_store_second_hsdir_index(const void *_key, { const char *key = _key; const node_t *node = *_member; - return tor_memcmp(key, node->hsdir_index->store_second, DIGEST256_LEN); + return tor_memcmp(key, node->hsdir_index.store_second, DIGEST256_LEN); }

/* Helper function: Compare two node_t objects current hsdir_index. */ @@ -134,8 +134,8 @@ compare_node_fetch_hsdir_index(const void **a, const void **b) { const node_t *node1= *a; const node_t *node2 = *b; - return tor_memcmp(node1->hsdir_index->fetch, - node2->hsdir_index->fetch, + return tor_memcmp(node1->hsdir_index.fetch, + node2->hsdir_index.fetch, DIGEST256_LEN); }

@@ -145,8 +145,8 @@ compare_node_store_first_hsdir_index(const void **a, const void **b) { const node_t *node1= *a; const node_t *node2 = *b; - return tor_memcmp(node1->hsdir_index->store_first, - node2->hsdir_index->store_first, + return tor_memcmp(node1->hsdir_index.store_first, + node2->hsdir_index.store_first, DIGEST256_LEN); }

@@ -156,8 +156,8 @@ compare_node_store_second_hsdir_index(const void **a, const void **b) { const node_t *node1= *a; const node_t *node2 = *b; - return tor_memcmp(node1->hsdir_index->store_second, - node2->hsdir_index->store_second, + return tor_memcmp(node1->hsdir_index.store_second, + node2->hsdir_index.store_second, DIGEST256_LEN); }

@@ -1288,18 +1288,15 @@ node_has_hsdir_index(const node_t *node)

/* At this point, since the node has a desc, this node must also have an * hsdir index. If not, something went wrong, so BUG out. */ - if (BUG(node->hsdir_index == NULL)) { - return 0; - } - if (BUG(tor_mem_is_zero((const char*)node->hsdir_index->fetch, + if (BUG(tor_mem_is_zero((const char*)node->hsdir_index.fetch, DIGEST256_LEN))) { return 0; } - if (BUG(tor_mem_is_zero((const char*)node->hsdir_index->store_first, + if (BUG(tor_mem_is_zero((const char*)node->hsdir_index.store_first, DIGEST256_LEN))) { return 0; } - if (BUG(tor_mem_is_zero((const char*)node->hsdir_index->store_second, + if (BUG(tor_mem_is_zero((const char*)node->hsdir_index.store_second, DIGEST256_LEN))) { return 0; } diff --git a/src/or/hs_common.h b/src/or/hs_common.h index 83ba1b859..ef7d5dca2 100644 --- a/src/or/hs_common.h +++ b/src/or/hs_common.h @@ -156,19 +156,6 @@ typedef struct rend_service_port_config_t { char unix_addr[FLEXIBLE_ARRAY_MEMBER]; } 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 { - /* HSDir index to use when fetching a descriptor. */ - uint8_t fetch[DIGEST256_LEN]; - - /* HSDir index used by services to store their first and second - * descriptor. The first descriptor is chronologically older than the second - * one and uses older TP and SRV values. */ - uint8_t store_first[DIGEST256_LEN]; - uint8_t store_second[DIGEST256_LEN]; -} hsdir_index_t; - void hs_init(void); void hs_free_all(void);

diff --git a/src/or/hs_control.c b/src/or/hs_control.c index 87b4e3fca..eca9ed1dd 100644 --- a/src/or/hs_control.c +++ b/src/or/hs_control.c @@ -39,9 +39,8 @@ hs_control_desc_event_requested(const ed25519_public_key_t *onion_pk, * can't pick a node without an hsdir_index. */ hsdir_node = node_get_by_id(hsdir_rs->identity_digest); tor_assert(hsdir_node); - tor_assert(hsdir_node->hsdir_index); /* This is a fetch event. */ - hsdir_index = hsdir_node->hsdir_index->fetch; + hsdir_index = hsdir_node->hsdir_index.fetch;

/* Trigger the event. */ control_event_hs_descriptor_requested(onion_address, REND_NO_AUTH, diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 7af14373d..4686eda0b 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -2304,8 +2304,8 @@ upload_descriptor_to_hsdir(const hs_service_t *service, /* Logging so we know where it was sent. */ { int is_next_desc = (service->desc_next == desc); - const uint8_t *idx = (is_next_desc) ? hsdir->hsdir_index->store_second: - hsdir->hsdir_index->store_first; + const uint8_t *idx = (is_next_desc) ? hsdir->hsdir_index.store_second: + hsdir->hsdir_index.store_first; log_info(LD_REND, "Service %s %s descriptor of revision %" PRIu64 " initiated upload request to %s with index %s", safe_str_client(service->onion_address), diff --git a/src/or/nodelist.c b/src/or/nodelist.c index e7342f979..675cbb005 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -225,7 +225,6 @@ node_get_or_create(const char *identity_digest)

smartlist_add(the_nodelist->nodes, node); node->nodelist_idx = smartlist_len(the_nodelist->nodes) - 1; - node->hsdir_index = tor_malloc_zero(sizeof(hsdir_index_t));

node->country = -1;

@@ -350,26 +349,26 @@ node_set_hsdir_index(node_t *node, const networkstatus_t *ns)

/* Build the fetch index. */ hs_build_hsdir_index(node_identity_pk, fetch_srv, fetch_tp, - node->hsdir_index->fetch); + node->hsdir_index.fetch);

/* If we are in the time segment between SRV#N and TP#N, the fetch index is the same as the first store index */ if (!hs_in_period_between_tp_and_srv(ns, now)) { - memcpy(node->hsdir_index->store_first, node->hsdir_index->fetch, - sizeof(node->hsdir_index->store_first)); + memcpy(node->hsdir_index.store_first, node->hsdir_index.fetch, + sizeof(node->hsdir_index.store_first)); } else { hs_build_hsdir_index(node_identity_pk, store_first_srv, store_first_tp, - node->hsdir_index->store_first); + node->hsdir_index.store_first); }

/* If we are in the time segment between TP#N and SRV#N+1, the fetch index is the same as the second store index */ if (hs_in_period_between_tp_and_srv(ns, now)) { - memcpy(node->hsdir_index->store_second, node->hsdir_index->fetch, - sizeof(node->hsdir_index->store_second)); + memcpy(node->hsdir_index.store_second, node->hsdir_index.fetch, + sizeof(node->hsdir_index.store_second)); } else { hs_build_hsdir_index(node_identity_pk, store_second_srv, store_second_tp, - node->hsdir_index->store_second); + node->hsdir_index.store_second); }

done: @@ -720,7 +719,6 @@ node_free_(node_t *node) if (node->md) node->md->held_by_nodes--; tor_assert(node->nodelist_idx == -1); - tor_free(node->hsdir_index); tor_free(node); }

diff --git a/src/or/or.h b/src/or/or.h index 475274340..e8edb0067 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -894,8 +894,19 @@ rend_data_v2_t *TO_REND_DATA_V2(const rend_data_t *d) struct hs_ident_edge_conn_t; struct hs_ident_dir_conn_t; struct hs_ident_circuit_t; -/* Stub because we can't include hs_common.h. */ -struct hsdir_index_t; + +/* Hidden service directory index used in a node_t which is set once we set + * the consensus. */ +typedef struct hsdir_index_t { + /* HSDir index to use when fetching a descriptor. */ + uint8_t fetch[DIGEST256_LEN]; + + /* HSDir index used by services to store their first and second + * descriptor. The first descriptor is chronologically older than the second + * one and uses older TP and SRV values. */ + uint8_t store_first[DIGEST256_LEN]; + uint8_t store_second[DIGEST256_LEN]; +} hsdir_index_t;

/** Time interval for tracking replays of DH public keys received in * INTRODUCE2 cells. Used only to avoid launching multiple @@ -2561,7 +2572,7 @@ typedef struct node_t { /* Hidden service directory index data. This is used by a service or client * in order to know what's the hs directory index for this node at the time * the consensus is set. */ - struct hsdir_index_t *hsdir_index; + struct hsdir_index_t hsdir_index; } node_t;

/** Linked list of microdesc hash lines for a single router in a directory diff --git a/src/test/test_hs_control.c b/src/test/test_hs_control.c index 207a55de6..308843e9b 100644 --- a/src/test/test_hs_control.c +++ b/src/test/test_hs_control.c @@ -76,9 +76,8 @@ mock_node_get_by_id(const char *digest) { static node_t node; memcpy(node.identity, digest, DIGEST_LEN); - node.hsdir_index = tor_malloc_zero(sizeof(hsdir_index_t)); - memset(node.hsdir_index->fetch, 'C', DIGEST256_LEN); - memset(node.hsdir_index->store_first, 'D', DIGEST256_LEN); + memset(node.hsdir_index.fetch, 'C', DIGEST256_LEN); + memset(node.hsdir_index.store_first, 'D', DIGEST256_LEN); return &node; }