From 91a6fbcf777ae94661a2a27fc4180dcb28d1f974 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 16 Apr 2017 18:45:48 -0400 Subject: [PATCH 01/18] Begin work to turn "directory request" into an object This patch adds the object type, and makes all of the functions that used to feed into directory_initiate_command_rend() feed into that type instead. --- src/or/directory.c | 155 ++++++++++++++++++++++++++++++++++++++++++++- src/or/directory.h | 27 ++++++++ 2 files changed, 180 insertions(+), 2 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 1b999ee7c3..d8aab268f6 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -1168,6 +1168,117 @@ directory_initiate_command(const tor_addr_t *or_addr, uint16_t or_port, if_modified_since, NULL, NULL); } +struct directory_request_t { + tor_addr_port_t or_addr_port; + tor_addr_port_t dir_addr_port; + char digest[DIGEST_LEN]; + uint8_t dir_purpose; + uint8_t router_purpose; + dir_indirection_t indirection; + const char *resource; + const char *payload; + size_t payload_len; + time_t if_modified_since; + const rend_data_t *rend_query; + circuit_guard_state_t *guard_state; // XXXX Does this belong? +}; + +directory_request_t * +directory_request_new(uint8_t dir_purpose) +{ + tor_assert(dir_purpose >= DIR_PURPOSE_MIN_); + tor_assert(dir_purpose <= DIR_PURPOSE_MAX_); + tor_assert(dir_purpose != DIR_PURPOSE_SERVER); + tor_assert(dir_purpose != DIR_PURPOSE_HAS_FETCHED_RENDDESC_V2); + + directory_request_t *result = tor_malloc_zero(sizeof(*result)); + result->dir_purpose = dir_purpose; + result->router_purpose = ROUTER_PURPOSE_GENERAL; + result->indirection = DIRIND_ONEHOP; + return result; +} +void +directory_request_free(directory_request_t *req) +{ + if (req == NULL) + return; + tor_free(req); +} + +void +directory_request_set_or_addr_port(directory_request_t *req, + const tor_addr_port_t *p) +{ + memcpy(&req->or_addr_port, p, sizeof(*p)); +} +void +directory_request_set_dir_addr_port(directory_request_t *req, + const tor_addr_port_t *p) +{ + memcpy(&req->dir_addr_port, p, sizeof(*p)); +} +void +directory_request_set_directory_id_digest(directory_request_t *req, + const char *digest) +{ + memcpy(req->digest, digest, DIGEST_LEN); +} +void +directory_request_set_router_purpose(directory_request_t *req, + uint8_t router_purpose) +{ + tor_assert(router_purpose == ROUTER_PURPOSE_GENERAL || + router_purpose == ROUTER_PURPOSE_BRIDGE); + // assert that it actually makes sense to set this purpose, given + // the dir_purpose. + req->router_purpose = router_purpose; +} +void +directory_request_set_indirection(directory_request_t *req, + dir_indirection_t indirection) +{ + req->indirection = indirection; +} +// DOCDOC lifetime +void +directory_request_set_resource(directory_request_t *req, + const char *resource) +{ + req->resource = resource; +} +// DOCDOC Lifetime +void +directory_request_set_payload(directory_request_t *req, + const char *payload, + size_t payload_len) +{ + tor_assert(DIR_PURPOSE_IS_UPLOAD(req->dir_purpose) || + // XXXX why not included? + req->dir_purpose == DIR_PURPOSE_UPLOAD_RENDDESC_V2); + + req->payload = payload; + req->payload_len = payload_len; +} +void +directory_request_set_if_modified_since(directory_request_t *req, + time_t if_modified_since) +{ + req->if_modified_since = if_modified_since; +} +// DOCDOC lifetime +void +directory_request_set_rend_query(directory_request_t *req, + const rend_data_t *query) +{ + req->rend_query = query; +} +void +directory_request_set_guard_state(directory_request_t *req, + circuit_guard_state_t *state) +{ + req->guard_state = state; +} + /** Same as directory_initiate_command(), but accepts rendezvous data to * fetch a hidden service descriptor, and takes its address & port arguments * as tor_addr_port_t. */ @@ -1175,14 +1286,54 @@ static void directory_initiate_command_rend(const tor_addr_port_t *or_addr_port, const tor_addr_port_t *dir_addr_port, const char *digest, - uint8_t dir_purpose, uint8_t router_purpose, + uint8_t dir_purpose, + uint8_t router_purpose, dir_indirection_t indirection, const char *resource, - const char *payload, size_t payload_len, + const char *payload, + size_t payload_len, time_t if_modified_since, const rend_data_t *rend_query, circuit_guard_state_t *guard_state) { + directory_request_t *r = directory_request_new(dir_purpose); + directory_request_set_or_addr_port(r, or_addr_port); + directory_request_set_dir_addr_port(r, dir_addr_port); + directory_request_set_directory_id_digest(r, digest); + directory_request_set_router_purpose(r, router_purpose); + directory_request_set_indirection(r, indirection); + if (resource) + directory_request_set_resource(r, resource); + if (payload) + directory_request_set_payload(r, payload, payload_len); + if (if_modified_since) + directory_request_set_if_modified_since(r, if_modified_since); + if (rend_query) + directory_request_set_rend_query(r, rend_query); + if (guard_state) + directory_request_set_guard_state(r, guard_state); + + directory_initiate_request(r); + + directory_request_free(r); +} + +void +directory_initiate_request(directory_request_t *request) +{ + const tor_addr_port_t *or_addr_port = &request->or_addr_port; + const tor_addr_port_t *dir_addr_port = &request->dir_addr_port; + const char *digest = request->digest; + const uint8_t dir_purpose = request->dir_purpose; + const uint8_t router_purpose = request->router_purpose; + const dir_indirection_t indirection = request->indirection; + const char *resource = request->resource; + const char *payload = request->payload; + const size_t payload_len = request->payload_len; + const time_t if_modified_since = request->if_modified_since; + const rend_data_t *rend_query = request->rend_query; + circuit_guard_state_t *guard_state = request->guard_state; + tor_assert(or_addr_port); tor_assert(dir_addr_port); tor_assert(or_addr_port->port || dir_addr_port->port); diff --git a/src/or/directory.h b/src/or/directory.h index 0c5db3e070..18fe5505c7 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -41,6 +41,31 @@ typedef enum { int directory_must_use_begindir(const or_options_t *options); +typedef struct directory_request_t directory_request_t; +directory_request_t *directory_request_new(uint8_t dir_purpose); +void directory_request_free(directory_request_t *req); +void directory_request_set_or_addr_port(directory_request_t *req, + const tor_addr_port_t *p); +void directory_request_set_dir_addr_port(directory_request_t *req, + const tor_addr_port_t *p); +void directory_request_set_directory_id_digest(directory_request_t *req, + const char *digest); +void directory_request_set_router_purpose(directory_request_t *req, + uint8_t router_purpose); +void directory_request_set_indirection(directory_request_t *req, + dir_indirection_t indirection); +void directory_request_set_resource(directory_request_t *req, + const char *resource); +void directory_request_set_payload(directory_request_t *req, + const char *payload, + size_t payload_len); +void directory_request_set_if_modified_since(directory_request_t *req, + time_t if_modified_since); +void directory_request_set_rend_query(directory_request_t *req, + const rend_data_t *query); +void directory_request_set_guard_state(directory_request_t *req, + struct circuit_guard_state_t *state); + MOCK_DECL(void, directory_initiate_command_routerstatus, (const routerstatus_t *status, uint8_t dir_purpose, @@ -63,6 +88,8 @@ void directory_initiate_command_routerstatus_rend(const routerstatus_t *status, const rend_data_t *rend_query, struct circuit_guard_state_t *guard_state); +void directory_initiate_request(directory_request_t *request); + int parse_http_response(const char *headers, int *code, time_t *date, compress_method_t *compression, char **response); From a55bd00b0f41afca767a57e2da4a3dc6a26d2110 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 21 Apr 2017 14:17:14 -0400 Subject: [PATCH 02/18] continue refactoring directory API: remove initiate_command_rend This commit mainly moves the responsibility for directory request construction one level higher. It also allows a directory request to contain a pointer to a routerstatus, which will get turned into the correct contact information at the last minute. --- src/or/directory.c | 240 +++++++++++++++++++++++---------------------- src/or/directory.h | 3 + 2 files changed, 127 insertions(+), 116 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index d8aab268f6..ea3683d34b 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -63,7 +63,7 @@ * multi-hop circuits for anonymity. * * Directory requests are launched by calling - * directory_initiate_command_rend() or one of its numerous variants. This + * directory_initiate_request(). This * launch the connection, will construct an HTTP request with * directory_send_command(), send the and wait for a response. The client * later handles the response with connection_dir_client_reached_eof(), @@ -118,20 +118,6 @@ static void dir_microdesc_download_failed(smartlist_t *failed, int status_code); static int client_likes_consensus(networkstatus_t *v, const char *want_url); -static void directory_initiate_command_rend( - const tor_addr_port_t *or_addr_port, - const tor_addr_port_t *dir_addr_port, - const char *digest, - uint8_t dir_purpose, - uint8_t router_purpose, - dir_indirection_t indirection, - const char *resource, - const char *payload, - size_t payload_len, - time_t if_modified_since, - const rend_data_t *rend_query, - circuit_guard_state_t *guard_state); - static void connection_dir_close_consensus_fetches( dir_connection_t *except_this_one, const char *resource); @@ -566,20 +552,19 @@ MOCK_IMPL(void, directory_get_from_dirserver, ( routerinfo_t *ri = node->ri; /* clients always make OR connections to bridges */ tor_addr_port_t or_ap; - tor_addr_port_t nil_dir_ap; + directory_request_t *req = directory_request_new(dir_purpose); /* we are willing to use a non-preferred address if we need to */ fascist_firewall_choose_address_node(node, FIREWALL_OR_CONNECTION, 0, &or_ap); - tor_addr_make_null(&nil_dir_ap.addr, AF_INET); - nil_dir_ap.port = 0; - directory_initiate_command_rend(&or_ap, - &nil_dir_ap, - ri->cache_info.identity_digest, - dir_purpose, - router_purpose, - DIRIND_ONEHOP, - resource, NULL, 0, if_modified_since, - NULL, guard_state); + directory_request_set_or_addr_port(req, &or_ap); + directory_request_set_directory_id_digest(req, + ri->cache_info.identity_digest); + directory_request_set_router_purpose(req, router_purpose); + directory_request_set_resource(req, resource); + directory_request_set_if_modified_since(req, if_modified_since); + directory_request_set_guard_state(req, guard_state); + directory_initiate_request(req); + directory_request_free(req); } else { if (guard_state) { entry_guard_cancel(&guard_state); @@ -792,61 +777,29 @@ directory_initiate_command_routerstatus_rend(const routerstatus_t *status, const rend_data_t *rend_query, circuit_guard_state_t *guard_state) { - const or_options_t *options = get_options(); - const node_t *node; - tor_addr_port_t use_or_ap, use_dir_ap; - const int anonymized_connection = dirind_is_anon(indirection); + directory_request_t *req = directory_request_new(dir_purpose); + directory_request_set_routerstatus(req, status); + directory_request_set_router_purpose(req, router_purpose); + directory_request_set_indirection(req, indirection); - tor_assert(status != NULL); - - node = node_get_by_id(status->identity_digest); - - /* XXX The below check is wrong: !node means it's not in the consensus, - * but we haven't checked if we have a descriptor for it -- and also, - * we only care about the descriptor if it's a begindir-style anonymized - * connection. */ - if (!node && anonymized_connection) { - log_info(LD_DIR, "Not sending anonymized request to directory '%s'; we " - "don't have its router descriptor.", - routerstatus_describe(status)); - return; - } - - if (options->ExcludeNodes && options->StrictNodes && - routerset_contains_routerstatus(options->ExcludeNodes, status, -1)) { - log_warn(LD_DIR, "Wanted to contact directory mirror %s for %s, but " - "it's in our ExcludedNodes list and StrictNodes is set. " - "Skipping. This choice might make your Tor not work.", - routerstatus_describe(status), - dir_conn_purpose_to_string(dir_purpose)); - return; - } - - /* At this point, if we are a client making a direct connection to a - * directory server, we have selected a server that has at least one address - * allowed by ClientUseIPv4/6 and Reachable{"",OR,Dir}Addresses. This - * selection uses the preference in ClientPreferIPv6{OR,Dir}Port, if - * possible. (If UseBridges is set, clients always use IPv6, and prefer it - * by default.) - * - * Now choose an address that we can use to connect to the directory server. - */ - if (directory_choose_address_routerstatus(status, indirection, &use_or_ap, - &use_dir_ap) < 0) { - return; - } + if (resource) + directory_request_set_resource(req, resource); + if (payload) + directory_request_set_payload(req, payload, payload_len); + if (if_modified_since) + directory_request_set_if_modified_since(req, if_modified_since); + if (rend_query) + directory_request_set_rend_query(req, rend_query); + if (guard_state) + directory_request_set_guard_state(req, guard_state); /* We don't retry the alternate OR/Dir address for the same directory if * the address we choose fails (#6772). * Instead, we'll retry another directory on failure. */ - directory_initiate_command_rend(&use_or_ap, &use_dir_ap, - status->identity_digest, - dir_purpose, router_purpose, - indirection, resource, - payload, payload_len, if_modified_since, - rend_query, - guard_state); + directory_initiate_request(req); + + directory_request_free(req); } /** Launch a new connection to the directory server status to @@ -1161,17 +1114,31 @@ directory_initiate_command(const tor_addr_t *or_addr, uint16_t or_port, dir_ap.port = dir_port = 0; } - directory_initiate_command_rend(&or_ap, &dir_ap, - digest, dir_purpose, - router_purpose, indirection, - resource, payload, payload_len, - if_modified_since, NULL, NULL); + directory_request_t *req = directory_request_new(dir_purpose); + directory_request_set_or_addr_port(req, &or_ap); + directory_request_set_dir_addr_port(req, &dir_ap); + directory_request_set_directory_id_digest(req, digest); + + directory_request_set_router_purpose(req, router_purpose); + directory_request_set_indirection(req, indirection); + if (resource) + directory_request_set_resource(req, resource); + if (payload) + directory_request_set_payload(req, payload, payload_len); + if (if_modified_since) + directory_request_set_if_modified_since(req, if_modified_since); + + directory_initiate_request(req); + directory_request_free(req); } struct directory_request_t { tor_addr_port_t or_addr_port; tor_addr_port_t dir_addr_port; char digest[DIGEST_LEN]; + + const routerstatus_t *routerstatus; + uint8_t dir_purpose; uint8_t router_purpose; dir_indirection_t indirection; @@ -1192,6 +1159,10 @@ directory_request_new(uint8_t dir_purpose) tor_assert(dir_purpose != DIR_PURPOSE_HAS_FETCHED_RENDDESC_V2); directory_request_t *result = tor_malloc_zero(sizeof(*result)); + tor_addr_make_null(&result->or_addr_port.addr, AF_INET); + result->or_addr_port.port = 0; + tor_addr_make_null(&result->dir_addr_port.addr, AF_INET); + result->dir_addr_port.port = 0; result->dir_purpose = dir_purpose; result->router_purpose = ROUTER_PURPOSE_GENERAL; result->indirection = DIRIND_ONEHOP; @@ -1279,48 +1250,87 @@ directory_request_set_guard_state(directory_request_t *req, req->guard_state = state; } -/** Same as directory_initiate_command(), but accepts rendezvous data to - * fetch a hidden service descriptor, and takes its address & port arguments - * as tor_addr_port_t. */ -static void -directory_initiate_command_rend(const tor_addr_port_t *or_addr_port, - const tor_addr_port_t *dir_addr_port, - const char *digest, - uint8_t dir_purpose, - uint8_t router_purpose, - dir_indirection_t indirection, - const char *resource, - const char *payload, - size_t payload_len, - time_t if_modified_since, - const rend_data_t *rend_query, - circuit_guard_state_t *guard_state) +static int +directory_request_is_dir_specified(const directory_request_t *req) { - directory_request_t *r = directory_request_new(dir_purpose); - directory_request_set_or_addr_port(r, or_addr_port); - directory_request_set_dir_addr_port(r, dir_addr_port); - directory_request_set_directory_id_digest(r, digest); - directory_request_set_router_purpose(r, router_purpose); - directory_request_set_indirection(r, indirection); - if (resource) - directory_request_set_resource(r, resource); - if (payload) - directory_request_set_payload(r, payload, payload_len); - if (if_modified_since) - directory_request_set_if_modified_since(r, if_modified_since); - if (rend_query) - directory_request_set_rend_query(r, rend_query); - if (guard_state) - directory_request_set_guard_state(r, guard_state); + return (req->or_addr_port.port || req->dir_addr_port.port) && + ! tor_digest_is_zero(req->digest); +} +void +directory_request_set_routerstatus(directory_request_t *req, + const routerstatus_t *status) +{ + req->routerstatus = status; +} +static int +directory_request_set_dir_from_routerstatus(directory_request_t *req) - directory_initiate_request(r); +{ + const routerstatus_t *status = req->routerstatus; + if (status == NULL) + return -1; + const or_options_t *options = get_options(); + const node_t *node; + tor_addr_port_t use_or_ap, use_dir_ap; + const int anonymized_connection = dirind_is_anon(req->indirection); - directory_request_free(r); + tor_assert(status != NULL); + + node = node_get_by_id(status->identity_digest); + + /* XXX The below check is wrong: !node means it's not in the consensus, + * but we haven't checked if we have a descriptor for it -- and also, + * we only care about the descriptor if it's a begindir-style anonymized + * connection. */ + if (!node && anonymized_connection) { + log_info(LD_DIR, "Not sending anonymized request to directory '%s'; we " + "don't have its router descriptor.", + routerstatus_describe(status)); + return -1; + } + + if (options->ExcludeNodes && options->StrictNodes && + routerset_contains_routerstatus(options->ExcludeNodes, status, -1)) { + log_warn(LD_DIR, "Wanted to contact directory mirror %s for %s, but " + "it's in our ExcludedNodes list and StrictNodes is set. " + "Skipping. This choice might make your Tor not work.", + routerstatus_describe(status), + dir_conn_purpose_to_string(req->dir_purpose)); + return -1; + } + + /* At this point, if we are a client making a direct connection to a + * directory server, we have selected a server that has at least one address + * allowed by ClientUseIPv4/6 and Reachable{"",OR,Dir}Addresses. This + * selection uses the preference in ClientPreferIPv6{OR,Dir}Port, if + * possible. (If UseBridges is set, clients always use IPv6, and prefer it + * by default.) + * + * Now choose an address that we can use to connect to the directory server. + */ + if (directory_choose_address_routerstatus(status, + req->indirection, &use_or_ap, + &use_dir_ap) < 0) { + return -1; + } + + directory_request_set_or_addr_port(req, &use_or_ap); + directory_request_set_dir_addr_port(req, &use_dir_ap); + directory_request_set_directory_id_digest(req, status->identity_digest); + return 0; } void directory_initiate_request(directory_request_t *request) { + tor_assert(request); + if (request->routerstatus) { + tor_assert_nonfatal(! directory_request_is_dir_specified(request)); + if (directory_request_set_dir_from_routerstatus(request) < 0) { + return; + } + } + const tor_addr_port_t *or_addr_port = &request->or_addr_port; const tor_addr_port_t *dir_addr_port = &request->dir_addr_port; const char *digest = request->digest; @@ -1334,8 +1344,6 @@ directory_initiate_request(directory_request_t *request) const rend_data_t *rend_query = request->rend_query; circuit_guard_state_t *guard_state = request->guard_state; - tor_assert(or_addr_port); - tor_assert(dir_addr_port); tor_assert(or_addr_port->port || dir_addr_port->port); tor_assert(digest); diff --git a/src/or/directory.h b/src/or/directory.h index 18fe5505c7..5d27767051 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -66,6 +66,9 @@ void directory_request_set_rend_query(directory_request_t *req, void directory_request_set_guard_state(directory_request_t *req, struct circuit_guard_state_t *state); +void directory_request_set_routerstatus(directory_request_t *req, + const routerstatus_t *rs); + MOCK_DECL(void, directory_initiate_command_routerstatus, (const routerstatus_t *status, uint8_t dir_purpose, From c300fd7e65b699c3d2704737cd0c73276ca0a346 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 21 Apr 2017 14:31:35 -0400 Subject: [PATCH 03/18] Have directory_get_from_all_authorities use requests. --- src/or/directory.c | 16 +++++++++------- src/or/directory.h | 2 +- src/test/test_dir.c | 15 ++++++++++++--- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index ea3683d34b..1996e565b9 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -655,15 +655,17 @@ directory_get_from_all_authorities(uint8_t dir_purpose, SMARTLIST_FOREACH_BEGIN(router_get_trusted_dir_servers(), dir_server_t *, ds) { - routerstatus_t *rs; if (router_digest_is_me(ds->digest)) continue; if (!(ds->type & V3_DIRINFO)) continue; - rs = &ds->fake_status; - directory_initiate_command_routerstatus(rs, dir_purpose, router_purpose, - DIRIND_ONEHOP, resource, NULL, - 0, 0, NULL); + const routerstatus_t *rs = &ds->fake_status; + directory_request_t *req = directory_request_new(dir_purpose); + directory_request_set_routerstatus(req, rs); + directory_request_set_router_purpose(req, router_purpose); + directory_request_set_resource(req, resource); + directory_initiate_request(req); + directory_request_free(req); } SMARTLIST_FOREACH_END(ds); } @@ -1320,8 +1322,8 @@ directory_request_set_dir_from_routerstatus(directory_request_t *req) return 0; } -void -directory_initiate_request(directory_request_t *request) +MOCK_IMPL(void, +directory_initiate_request,(directory_request_t *request)) { tor_assert(request); if (request->routerstatus) { diff --git a/src/or/directory.h b/src/or/directory.h index 5d27767051..56bbfe4b4b 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -91,7 +91,7 @@ void directory_initiate_command_routerstatus_rend(const routerstatus_t *status, const rend_data_t *rend_query, struct circuit_guard_state_t *guard_state); -void directory_initiate_request(directory_request_t *request); +MOCK_DECL(void, directory_initiate_request, (directory_request_t *request)); int parse_http_response(const char *headers, int *code, time_t *date, compress_method_t *compression, char **response); diff --git a/src/test/test_dir.c b/src/test/test_dir.c index e0c92a74c5..048b714b63 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -4541,6 +4541,8 @@ directory_initiate_command_routerstatus, (const routerstatus_t *status, size_t payload_len, time_t if_modified_since, circuit_guard_state_t *guardstate)); +NS_DECL(void, +directory_initiate_request, (directory_request_t *req)); static void test_dir_should_not_init_request_to_ourselves(void *data) @@ -4616,7 +4618,7 @@ test_dir_should_init_request_to_dir_auths(void *data) dir_server_t *ds = NULL; (void) data; - NS_MOCK(directory_initiate_command_routerstatus); + NS_MOCK(directory_initiate_request); clear_dir_servers(); routerlist_free_all(); @@ -4627,11 +4629,11 @@ test_dir_should_init_request_to_dir_auths(void *data) dir_server_add(ds); directory_get_from_all_authorities(DIR_PURPOSE_FETCH_STATUS_VOTE, 0, NULL); - tt_int_op(CALLED(directory_initiate_command_routerstatus), OP_EQ, 1); + tt_int_op(CALLED(directory_initiate_request), OP_EQ, 1); directory_get_from_all_authorities(DIR_PURPOSE_FETCH_DETACHED_SIGNATURES, 0, NULL); - tt_int_op(CALLED(directory_initiate_command_routerstatus), OP_EQ, 2); + tt_int_op(CALLED(directory_initiate_request), OP_EQ, 2); done: NS_UNMOCK(directory_initiate_command_routerstatus); @@ -4662,6 +4664,13 @@ NS(directory_initiate_command_routerstatus)(const routerstatus_t *status, CALLED(directory_initiate_command_routerstatus)++; } +void +NS(directory_initiate_request)(directory_request_t *req) +{ + (void)req; + CALLED(directory_initiate_request)++; +} + static void test_dir_choose_compression_level(void* data) { From eee733be33cc2ed698b35b4ea226390f534daa6a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 21 Apr 2017 14:35:42 -0400 Subject: [PATCH 04/18] Refactor directory_post_to_dirservers to use requests --- src/or/directory.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 1996e565b9..386ba2f39d 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -407,11 +407,14 @@ directory_post_to_dirservers(uint8_t dir_purpose, uint8_t router_purpose, } else { indirection = DIRIND_DIRECT_CONN; } - directory_initiate_command_routerstatus(rs, dir_purpose, - router_purpose, - indirection, - NULL, payload, upload_len, 0, - NULL); + + directory_request_t *req = directory_request_new(dir_purpose); + directory_request_set_routerstatus(req, rs); + directory_request_set_router_purpose(req, router_purpose); + directory_request_set_indirection(req, indirection); + directory_request_set_payload(req, payload, upload_len); + directory_initiate_request(req); + directory_request_free(req); } SMARTLIST_FOREACH_END(ds); if (!found) { char *s = authdir_type_to_string(type); From 4ae65bb5d37b85df19984dc19eedca31f2ae5557 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 21 Apr 2017 14:40:48 -0400 Subject: [PATCH 05/18] Refactor directory_get_from_dirserver to use request API. --- src/or/directory.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 386ba2f39d..12c45cfff5 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -627,12 +627,16 @@ MOCK_IMPL(void, directory_get_from_dirserver, ( if (rs) { const dir_indirection_t indirection = get_via_tor ? DIRIND_ANONYMOUS : DIRIND_ONEHOP; - directory_initiate_command_routerstatus(rs, dir_purpose, - router_purpose, - indirection, - resource, NULL, 0, - if_modified_since, - guard_state); + directory_request_t *req = directory_request_new(dir_purpose); + directory_request_set_routerstatus(req, rs); + directory_request_set_router_purpose(req, router_purpose); + directory_request_set_indirection(req, indirection); + directory_request_set_resource(req, resource); + directory_request_set_if_modified_since(req, if_modified_since); + if (guard_state) + directory_request_set_guard_state(req, guard_state); + directory_initiate_request(req); + directory_request_free(req); } else { log_notice(LD_DIR, "While fetching directory info, " From 4e393f53188055e478c0292af504fb97d02a4ff2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 21 Apr 2017 14:47:14 -0400 Subject: [PATCH 06/18] Make launch_direct_bridge_descriptor_fetch use dirreqs. --- src/or/bridges.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/or/bridges.c b/src/or/bridges.c index 255e0a4f7c..ef0638c0a7 100644 --- a/src/or/bridges.c +++ b/src/or/bridges.c @@ -570,12 +570,18 @@ launch_direct_bridge_descriptor_fetch(bridge_info_t *bridge) return; } - directory_initiate_command(&bridge->addr, bridge->port, - NULL, 0, /*no dirport*/ - bridge->identity, - DIR_PURPOSE_FETCH_SERVERDESC, - ROUTER_PURPOSE_BRIDGE, - DIRIND_ONEHOP, "authority.z", NULL, 0, 0); + tor_addr_port_t bridge_addrport; + memcpy(&bridge_addrport.addr, &bridge->addr, sizeof(tor_addr_t)); + bridge_addrport.port = bridge->port; + + directory_request_t *req = + directory_request_new(DIR_PURPOSE_FETCH_SERVERDESC); + directory_request_set_or_addr_port(req, &bridge_addrport); + directory_request_set_directory_id_digest(req, bridge->identity); + directory_request_set_router_purpose(req, ROUTER_PURPOSE_BRIDGE); + directory_request_set_resource(req, "authority.z"); + directory_initiate_request(req); + directory_request_free(req); } /** Fetching the bridge descriptor from the bridge authority returned a From 8aadd60eac6d2b4feb176e34805b4bd5a8b77aed Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 21 Apr 2017 15:08:28 -0400 Subject: [PATCH 07/18] Replace remaining directory_initiate_command_* instances --- src/or/connection_edge.c | 2 +- src/or/or.h | 3 ++- src/or/rendclient.c | 16 +++++++++------- src/or/rendservice.c | 17 ++++++++++------- src/or/router.c | 22 ++++++++++++++++------ src/or/routerlist.c | 37 +++++++++++++++++++++---------------- 6 files changed, 59 insertions(+), 38 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 2c60d8dddb..d1728eca6d 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -2527,7 +2527,7 @@ connection_ap_handshake_send_begin(entry_connection_t *ap_conn) /* Sensitive directory connections must have an anonymous path length. * Otherwise, directory connections are typically one-hop. * This matches the earlier check for directory connection path anonymity - * in directory_initiate_command_rend(). */ + * in directory_initiate_request(). */ if (purpose_needs_anonymity(linked_dir_conn_base->purpose, TO_DIR_CONN(linked_dir_conn_base)->router_purpose, TO_DIR_CONN(linked_dir_conn_base)->requested_resource)) { diff --git a/src/or/or.h b/src/or/or.h index a7b3a66561..21b8ca5018 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -5303,7 +5303,8 @@ typedef struct dir_server_t { * address information from published? */ routerstatus_t fake_status; /**< Used when we need to pass this trusted - * dir_server_t to directory_initiate_command_* + * dir_server_t to + * directory_request_set_routerstatus. * as a routerstatus_t. Not updated by the * router-status management code! **/ diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 8d2ae03c9e..1ae03ed408 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -756,13 +756,15 @@ directory_get_from_hs_dir(const char *desc_id, /* Send fetch request. (Pass query and possibly descriptor cookie so that * they can be written to the directory connection and be referred to when * the response arrives. */ - directory_initiate_command_routerstatus_rend(hs_dir, - DIR_PURPOSE_FETCH_RENDDESC_V2, - ROUTER_PURPOSE_GENERAL, - how_to_fetch, - desc_id_base32, - NULL, 0, 0, - rend_query, NULL); + directory_request_t *req = + directory_request_new(DIR_PURPOSE_FETCH_RENDDESC_V2); + directory_request_set_routerstatus(req, hs_dir); + directory_request_set_indirection(req, how_to_fetch); + directory_request_set_resource(req, desc_id_base32); + directory_request_set_rend_query(req, rend_query); + directory_initiate_request(req); + directory_request_free(req); + log_info(LD_REND, "Sending fetch request for v2 descriptor for " "service '%s' with descriptor ID '%s', auth type %d, " "and descriptor cookie '%s' to hidden service " diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 3c4b6775c0..82d8e4c48e 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -3712,13 +3712,16 @@ directory_post_to_hs_dir(rend_service_descriptor_t *renddesc, * request. Lookup is made in rend_service_desc_has_uploaded(). */ rend_data = rend_data_client_create(service_id, desc->desc_id, NULL, REND_NO_AUTH); - directory_initiate_command_routerstatus_rend(hs_dir, - DIR_PURPOSE_UPLOAD_RENDDESC_V2, - ROUTER_PURPOSE_GENERAL, - DIRIND_ANONYMOUS, NULL, - desc->desc_str, - strlen(desc->desc_str), - 0, rend_data, NULL); + directory_request_t *req = + directory_request_new(DIR_PURPOSE_UPLOAD_RENDDESC_V2); + directory_request_set_routerstatus(req, hs_dir); + directory_request_set_indirection(req, DIRIND_ANONYMOUS); + directory_request_set_payload(req, + desc->desc_str, strlen(desc->desc_str)); + directory_request_set_rend_query(req, rend_data); + directory_initiate_request(req); + directory_request_free(req); + rend_data_free(rend_data); base32_encode(desc_id_base32, sizeof(desc_id_base32), desc->desc_id, DIGEST_LEN); diff --git a/src/or/router.c b/src/or/router.c index 7fb49e8eba..2336a5d3df 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1470,13 +1470,23 @@ consider_testing_reachability(int test_or, int test_dir) !connection_get_by_type_addr_port_purpose( CONN_TYPE_DIR, &addr, me->dir_port, DIR_PURPOSE_FETCH_SERVERDESC)) { + tor_addr_port_t my_orport, my_dirport; + memcpy(&my_orport.addr, &addr, sizeof(addr)); + memcpy(&my_dirport.addr, &addr, sizeof(addr)); + my_orport.port = me->or_port; + my_dirport.port = me->dir_port; /* ask myself, via tor, for my server descriptor. */ - directory_initiate_command(&addr, me->or_port, - &addr, me->dir_port, - me->cache_info.identity_digest, - DIR_PURPOSE_FETCH_SERVERDESC, - ROUTER_PURPOSE_GENERAL, - DIRIND_ANON_DIRPORT, "authority.z", NULL, 0, 0); + directory_request_t *req = + directory_request_new(DIR_PURPOSE_FETCH_SERVERDESC); + directory_request_set_or_addr_port(req, &my_orport); + directory_request_set_dir_addr_port(req, &my_dirport); + directory_request_set_directory_id_digest(req, + me->cache_info.identity_digest); + // ask via an anon circuit, connecting to our dirport. + directory_request_set_indirection(req, DIRIND_ANON_DIRPORT); + directory_request_set_resource(req, "authority.z"); + directory_initiate_request(req); + directory_request_free(req); } } diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 0b0bb4b1d2..bff2cca048 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -947,6 +947,7 @@ authority_certs_fetch_resource_impl(const char *resource, const dir_indirection_t indirection = get_via_tor ? DIRIND_ANONYMOUS : DIRIND_ONEHOP; + directory_request_t *req = NULL; /* If we've just downloaded a consensus from a bridge, re-use that * bridge */ if (options->UseBridges && node && node->ri && !get_via_tor) { @@ -955,23 +956,26 @@ authority_certs_fetch_resource_impl(const char *resource, /* we are willing to use a non-preferred address if we need to */ fascist_firewall_choose_address_node(node, FIREWALL_OR_CONNECTION, 0, &or_ap); - directory_initiate_command(&or_ap.addr, or_ap.port, - NULL, 0, /*no dirport*/ - dir_hint, - DIR_PURPOSE_FETCH_CERTIFICATE, - 0, - indirection, - resource, NULL, 0, 0); - return; + + req = directory_request_new(DIR_PURPOSE_FETCH_CERTIFICATE); + directory_request_set_or_addr_port(req, &or_ap); + if (dir_hint) + directory_request_set_directory_id_digest(req, dir_hint); } if (rs) { /* If we've just downloaded a consensus from a directory, re-use that * directory */ - directory_initiate_command_routerstatus(rs, - DIR_PURPOSE_FETCH_CERTIFICATE, - 0, indirection, resource, NULL, - 0, 0, NULL); + req = directory_request_new(DIR_PURPOSE_FETCH_CERTIFICATE); + directory_request_set_routerstatus(req, rs); + } + + if (req) { + /* Fill in the other request fields, and send the request. */ + directory_request_set_indirection(req, indirection); + directory_request_set_resource(req, resource); + directory_initiate_request(req); + directory_request_free(req); return; } @@ -4932,10 +4936,11 @@ MOCK_IMPL(STATIC void, initiate_descriptor_downloads, if (source) { /* We know which authority or directory mirror we want. */ - directory_initiate_command_routerstatus(source, purpose, - ROUTER_PURPOSE_GENERAL, - DIRIND_ONEHOP, - resource, NULL, 0, 0, NULL); + directory_request_t *req = directory_request_new(purpose); + directory_request_set_routerstatus(req, source); + directory_request_set_resource(req, resource); + directory_initiate_request(req); + directory_request_free(req); } else { directory_get_from_dirserver(purpose, ROUTER_PURPOSE_GENERAL, resource, pds_flags, DL_WANT_ANY_DIRSERVER); From 3327e675fdb16b2109fb384cbc030ed3d4c86442 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 21 Apr 2017 15:14:52 -0400 Subject: [PATCH 08/18] Remove old directory_initiate_command_*() functions. --- src/or/directory.c | 132 +------------------------------------------- src/or/directory.h | 30 ---------- src/test/test_dir.c | 51 +++-------------- 3 files changed, 12 insertions(+), 201 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 12c45cfff5..41d96c6a46 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -772,78 +772,6 @@ directory_choose_address_routerstatus(const routerstatus_t *status, return 0; } -/** Same as directory_initiate_command_routerstatus(), but accepts - * rendezvous data to fetch a hidden service descriptor. */ -void -directory_initiate_command_routerstatus_rend(const routerstatus_t *status, - uint8_t dir_purpose, - uint8_t router_purpose, - dir_indirection_t indirection, - const char *resource, - const char *payload, - size_t payload_len, - time_t if_modified_since, - const rend_data_t *rend_query, - circuit_guard_state_t *guard_state) -{ - directory_request_t *req = directory_request_new(dir_purpose); - directory_request_set_routerstatus(req, status); - directory_request_set_router_purpose(req, router_purpose); - directory_request_set_indirection(req, indirection); - - if (resource) - directory_request_set_resource(req, resource); - if (payload) - directory_request_set_payload(req, payload, payload_len); - if (if_modified_since) - directory_request_set_if_modified_since(req, if_modified_since); - if (rend_query) - directory_request_set_rend_query(req, rend_query); - if (guard_state) - directory_request_set_guard_state(req, guard_state); - - /* We don't retry the alternate OR/Dir address for the same directory if - * the address we choose fails (#6772). - * Instead, we'll retry another directory on failure. */ - - directory_initiate_request(req); - - directory_request_free(req); -} - -/** Launch a new connection to the directory server status to - * upload or download a server or rendezvous - * descriptor. dir_purpose determines what - * kind of directory connection we're launching, and must be one of - * DIR_PURPOSE_{FETCH|UPLOAD}_{DIR|RENDDESC_V2}. router_purpose - * specifies the descriptor purposes we have in mind (currently only - * used for FETCH_DIR). - * - * When uploading, payload and payload_len determine the content - * of the HTTP post. Otherwise, payload should be NULL. - * - * When fetching a rendezvous descriptor, resource is the service ID we - * want to fetch. - */ -MOCK_IMPL(void, directory_initiate_command_routerstatus, - (const routerstatus_t *status, - uint8_t dir_purpose, - uint8_t router_purpose, - dir_indirection_t indirection, - const char *resource, - const char *payload, - size_t payload_len, - time_t if_modified_since, - circuit_guard_state_t *guard_state)) -{ - directory_initiate_command_routerstatus_rend(status, dir_purpose, - router_purpose, - indirection, resource, - payload, payload_len, - if_modified_since, NULL, - guard_state); -} - /** Return true iff conn is the client side of a directory connection * we launched to ourself in order to determine the reachability of our * dir_port. */ @@ -1085,62 +1013,6 @@ directory_command_should_use_begindir(const or_options_t *options, return 1; } -/** Helper for directory_initiate_command_rend: send the - * command to a server whose OR address/port is or_addr/or_port, - * whose directory address/port is dir_addr/dir_port, whose - * identity key digest is digest, with purposes dir_purpose and - * router_purpose, making an (in)direct connection as specified in - * indirection, with command resource, payload of - * payload_len, and asking for a result only if_modified_since. - */ -void -directory_initiate_command(const tor_addr_t *or_addr, uint16_t or_port, - const tor_addr_t *dir_addr, uint16_t dir_port, - const char *digest, - uint8_t dir_purpose, uint8_t router_purpose, - dir_indirection_t indirection, const char *resource, - const char *payload, size_t payload_len, - time_t if_modified_since) -{ - tor_addr_port_t or_ap, dir_ap; - - /* Use the null tor_addr and 0 port if the address or port isn't valid. */ - if (tor_addr_port_is_valid(or_addr, or_port, 0)) { - tor_addr_copy(&or_ap.addr, or_addr); - or_ap.port = or_port; - } else { - /* the family doesn't matter here, so make it IPv4 */ - tor_addr_make_null(&or_ap.addr, AF_INET); - or_ap.port = or_port = 0; - } - - if (tor_addr_port_is_valid(dir_addr, dir_port, 0)) { - tor_addr_copy(&dir_ap.addr, dir_addr); - dir_ap.port = dir_port; - } else { - /* the family doesn't matter here, so make it IPv4 */ - tor_addr_make_null(&dir_ap.addr, AF_INET); - dir_ap.port = dir_port = 0; - } - - directory_request_t *req = directory_request_new(dir_purpose); - directory_request_set_or_addr_port(req, &or_ap); - directory_request_set_dir_addr_port(req, &dir_ap); - directory_request_set_directory_id_digest(req, digest); - - directory_request_set_router_purpose(req, router_purpose); - directory_request_set_indirection(req, indirection); - if (resource) - directory_request_set_resource(req, resource); - if (payload) - directory_request_set_payload(req, payload, payload_len); - if (if_modified_since) - directory_request_set_if_modified_since(req, if_modified_since); - - directory_initiate_request(req); - directory_request_free(req); -} - struct directory_request_t { tor_addr_port_t or_addr_port; tor_addr_port_t dir_addr_port; @@ -1630,7 +1502,9 @@ copy_ipv6_address(char* destination, const char* source, size_t len, } /** Queue an appropriate HTTP command on conn-\>outbuf. The other args - * are as in directory_initiate_command(). + * are as in directory_request_set_...() + * + * */ static void directory_send_command(dir_connection_t *conn, diff --git a/src/or/directory.h b/src/or/directory.h index 56bbfe4b4b..7399b706ad 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -69,28 +69,6 @@ void directory_request_set_guard_state(directory_request_t *req, void directory_request_set_routerstatus(directory_request_t *req, const routerstatus_t *rs); -MOCK_DECL(void, directory_initiate_command_routerstatus, - (const routerstatus_t *status, - uint8_t dir_purpose, - uint8_t router_purpose, - dir_indirection_t indirection, - const char *resource, - const char *payload, - size_t payload_len, - time_t if_modified_since, - struct circuit_guard_state_t *guard_state)); - -void directory_initiate_command_routerstatus_rend(const routerstatus_t *status, - uint8_t dir_purpose, - uint8_t router_purpose, - dir_indirection_t indirection, - const char *resource, - const char *payload, - size_t payload_len, - time_t if_modified_since, - const rend_data_t *rend_query, - struct circuit_guard_state_t *guard_state); - MOCK_DECL(void, directory_initiate_request, (directory_request_t *request)); int parse_http_response(const char *headers, int *code, time_t *date, @@ -102,14 +80,6 @@ int connection_dir_process_inbuf(dir_connection_t *conn); int connection_dir_finished_flushing(dir_connection_t *conn); int connection_dir_finished_connecting(dir_connection_t *conn); void connection_dir_about_to_close(dir_connection_t *dir_conn); -void directory_initiate_command(const tor_addr_t *or_addr, uint16_t or_port, - const tor_addr_t *dir_addr, uint16_t dir_port, - const char *digest, - uint8_t dir_purpose, uint8_t router_purpose, - dir_indirection_t indirection, - const char *resource, - const char *payload, size_t payload_len, - time_t if_modified_since); #define DSR_HEX (1<<0) #define DSR_BASE64 (1<<1) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 048b714b63..048d773475 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -4531,16 +4531,6 @@ test_dir_should_use_directory_guards(void *data) tor_free(errmsg); } -NS_DECL(void, -directory_initiate_command_routerstatus, (const routerstatus_t *status, - uint8_t dir_purpose, - uint8_t router_purpose, - dir_indirection_t indirection, - const char *resource, - const char *payload, - size_t payload_len, - time_t if_modified_since, - circuit_guard_state_t *guardstate)); NS_DECL(void, directory_initiate_request, (directory_request_t *req)); @@ -4552,7 +4542,7 @@ test_dir_should_not_init_request_to_ourselves(void *data) crypto_pk_t *key = pk_generate(2); (void) data; - NS_MOCK(directory_initiate_command_routerstatus); + NS_MOCK(directory_initiate_request); clear_dir_servers(); routerlist_free_all(); @@ -4567,15 +4557,15 @@ test_dir_should_not_init_request_to_ourselves(void *data) dir_server_add(ourself); directory_get_from_all_authorities(DIR_PURPOSE_FETCH_STATUS_VOTE, 0, NULL); - tt_int_op(CALLED(directory_initiate_command_routerstatus), OP_EQ, 0); + tt_int_op(CALLED(directory_initiate_request), OP_EQ, 0); directory_get_from_all_authorities(DIR_PURPOSE_FETCH_DETACHED_SIGNATURES, 0, NULL); - tt_int_op(CALLED(directory_initiate_command_routerstatus), OP_EQ, 0); + tt_int_op(CALLED(directory_initiate_request), OP_EQ, 0); done: - NS_UNMOCK(directory_initiate_command_routerstatus); + NS_UNMOCK(directory_initiate_request); clear_dir_servers(); routerlist_free_all(); crypto_pk_free(key); @@ -4589,7 +4579,7 @@ test_dir_should_not_init_request_to_dir_auths_without_v3_info(void *data) | MICRODESC_DIRINFO; (void) data; - NS_MOCK(directory_initiate_command_routerstatus); + NS_MOCK(directory_initiate_request); clear_dir_servers(); routerlist_free_all(); @@ -4600,14 +4590,14 @@ test_dir_should_not_init_request_to_dir_auths_without_v3_info(void *data) dir_server_add(ds); directory_get_from_all_authorities(DIR_PURPOSE_FETCH_STATUS_VOTE, 0, NULL); - tt_int_op(CALLED(directory_initiate_command_routerstatus), OP_EQ, 0); + tt_int_op(CALLED(directory_initiate_request), OP_EQ, 0); directory_get_from_all_authorities(DIR_PURPOSE_FETCH_DETACHED_SIGNATURES, 0, NULL); - tt_int_op(CALLED(directory_initiate_command_routerstatus), OP_EQ, 0); + tt_int_op(CALLED(directory_initiate_request), OP_EQ, 0); done: - NS_UNMOCK(directory_initiate_command_routerstatus); + NS_UNMOCK(directory_initiate_request); clear_dir_servers(); routerlist_free_all(); } @@ -4636,34 +4626,11 @@ test_dir_should_init_request_to_dir_auths(void *data) tt_int_op(CALLED(directory_initiate_request), OP_EQ, 2); done: - NS_UNMOCK(directory_initiate_command_routerstatus); + NS_UNMOCK(directory_initiate_request); clear_dir_servers(); routerlist_free_all(); } -void -NS(directory_initiate_command_routerstatus)(const routerstatus_t *status, - uint8_t dir_purpose, - uint8_t router_purpose, - dir_indirection_t indirection, - const char *resource, - const char *payload, - size_t payload_len, - time_t if_modified_since, - circuit_guard_state_t *guardstate) -{ - (void)status; - (void)dir_purpose; - (void)router_purpose; - (void)indirection; - (void)resource; - (void)payload; - (void)payload_len; - (void)if_modified_since; - (void)guardstate; - CALLED(directory_initiate_command_routerstatus)++; -} - void NS(directory_initiate_request)(directory_request_t *req) { From 81045e19eb49731e26c0bb70cc922de7aca1b1e8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 21 Apr 2017 15:33:29 -0400 Subject: [PATCH 09/18] Document the new directory_request_t API and add some assertions --- src/or/directory.c | 100 ++++++++++++++++++++++++++++++++++++++++++--- src/or/directory.h | 6 +++ 2 files changed, 100 insertions(+), 6 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 41d96c6a46..b504808b60 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -1014,23 +1014,44 @@ directory_command_should_use_begindir(const or_options_t *options, } struct directory_request_t { + /** + * These fields specify which directory we're contacting. Routerstatus, + * if present, overrides the other fields. + * + * @{ */ tor_addr_port_t or_addr_port; tor_addr_port_t dir_addr_port; char digest[DIGEST_LEN]; const routerstatus_t *routerstatus; - + /** @} */ + /** One of DIR_PURPOSE_* other than DIR_PURPOSE_SERVER. Describes what + * kind of operation we'll be doing (upload/download), and of what kind + * of document. */ uint8_t dir_purpose; + /** One of ROUTER_PURPOSE_*; used for uploads and downloads of routerinfo + * and extrainfo docs. */ uint8_t router_purpose; + /** Enum: determines whether to anonymize, and whether to use dirport or + * orport. */ dir_indirection_t indirection; + /** Alias to the variable part of the URL for this request */ const char *resource; + /** Alias to the payload to upload (if any) */ const char *payload; + /** Number of bytes to upload from payload */ size_t payload_len; + /** Value to send in an if-modified-since header, or 0 for none. */ time_t if_modified_since; + /** Hidden-service-specific information */ const rend_data_t *rend_query; circuit_guard_state_t *guard_state; // XXXX Does this belong? }; +/** + * Create and return a new directory_request_t with purpose + * dir_purpose. + */ directory_request_t * directory_request_new(uint8_t dir_purpose) { @@ -1049,6 +1070,9 @@ directory_request_new(uint8_t dir_purpose) result->indirection = DIRIND_ONEHOP; return result; } +/** + * Release all resources held by req. + */ void directory_request_free(directory_request_t *req) { @@ -1056,25 +1080,44 @@ directory_request_free(directory_request_t *req) return; tor_free(req); } - +/** + * Set the address and OR port to use for this directory request. If there is + * no OR port, we'll have to connect over the dirport. (If there are both, + * the indirection setting determins which to use.) + */ void directory_request_set_or_addr_port(directory_request_t *req, const tor_addr_port_t *p) { memcpy(&req->or_addr_port, p, sizeof(*p)); } +/** + * Set the address and dirport to use for this directory request. If there + * is no dirport, we'll have to connect over the OR port. (If there are both, + * the indirection setting determins which to use.) + */ void directory_request_set_dir_addr_port(directory_request_t *req, const tor_addr_port_t *p) { memcpy(&req->dir_addr_port, p, sizeof(*p)); } +/** + * Set the RSA identity digest of the directory to use for this directory + * request. + */ void directory_request_set_directory_id_digest(directory_request_t *req, const char *digest) { memcpy(req->digest, digest, DIGEST_LEN); } +/** + * Set the router purpose associated with uploaded and downloaded router + * descriptors and extrainfo documents in this directory request. The purpose + * must be one of ROUTER_PURPOSE_GENERAL (the default) or + * ROUTER_PURPOSE_BRIDGE. + */ void directory_request_set_router_purpose(directory_request_t *req, uint8_t router_purpose) @@ -1085,20 +1128,36 @@ directory_request_set_router_purpose(directory_request_t *req, // the dir_purpose. req->router_purpose = router_purpose; } +/** + * Set the indirection to be used for the directory request. The indirection + * parameter configures whether to connect to a DirPort or ORPort, and whether + * to anonymize the connection. DIRIND_ONEHOP (use ORPort, don't anonymize) + * is the default. See dir_indirection_t for more information. + */ void directory_request_set_indirection(directory_request_t *req, dir_indirection_t indirection) { req->indirection = indirection; } -// DOCDOC lifetime + +/** + * Set a pointer to the resource to request from a directory. Different + * request types use resources to indicate different components of their URL. + * Note that only an alias to resource is stored, so the + * resource must outlive the request. + */ void directory_request_set_resource(directory_request_t *req, const char *resource) { req->resource = resource; } -// DOCDOC Lifetime +/** + * Set a pointer to the payload to include with this directory request, along + * with its length. Note that only an alias to payload is stored, so + * the payload must outlive the request. + */ void directory_request_set_payload(directory_request_t *req, const char *payload, @@ -1111,44 +1170,69 @@ directory_request_set_payload(directory_request_t *req, req->payload = payload; req->payload_len = payload_len; } +/** + * Set an if-modified-since date to send along with the request. The + * default is 0 (meaning, send no if-modified-since header). + */ void directory_request_set_if_modified_since(directory_request_t *req, time_t if_modified_since) { req->if_modified_since = if_modified_since; } -// DOCDOC lifetime +/** + * Set an object containing HS data to be associated with this request. Note + * that only an alias to query is stored, so the query object + * must outlive the request. + */ void directory_request_set_rend_query(directory_request_t *req, const rend_data_t *query) { + if (query) { + tor_assert(req->dir_purpose == DIR_PURPOSE_FETCH_RENDDESC_V2 || + req->dir_purpose == DIR_PURPOSE_UPLOAD_RENDDESC_V2); + } req->rend_query = query; } void directory_request_set_guard_state(directory_request_t *req, circuit_guard_state_t *state) { + // XXXX make static. req->guard_state = state; } static int directory_request_is_dir_specified(const directory_request_t *req) { + // XXXX inline and revise return (req->or_addr_port.port || req->dir_addr_port.port) && ! tor_digest_is_zero(req->digest); } + +/** + * Set the routerstatus to use for the directory associated with this + * request. If this option is set, then no other function to set the + * directory's address or identity should be called. + */ void directory_request_set_routerstatus(directory_request_t *req, const routerstatus_t *status) { req->routerstatus = status; } +/** + * Helper: update the addresses, ports, and identities in req + * from the routerstatus object in req. Return 0 on success. + * On failure, warn and return -1. + */ static int directory_request_set_dir_from_routerstatus(directory_request_t *req) { const routerstatus_t *status = req->routerstatus; - if (status == NULL) + if (BUG(status == NULL)) return -1; const or_options_t *options = get_options(); const node_t *node; @@ -1201,6 +1285,10 @@ directory_request_set_dir_from_routerstatus(directory_request_t *req) return 0; } +/** + * Launch the provided directory request, configured in request. + * After this function is called, you can free request. + */ MOCK_IMPL(void, directory_initiate_request,(directory_request_t *request)) { diff --git a/src/or/directory.h b/src/or/directory.h index 7399b706ad..6b2f50b45d 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -41,6 +41,12 @@ typedef enum { int directory_must_use_begindir(const or_options_t *options); +/** + * A directory_request_t describes the information about a directory request + * at the client side. It describes what we're going to ask for, which + * directory we're going to ask for it, how we're going to contact that + * directory, and (in some cases) what to do with it when we're done. + */ typedef struct directory_request_t directory_request_t; directory_request_t *directory_request_new(uint8_t dir_purpose); void directory_request_free(directory_request_t *req); From d7fab3603760b40d4db2214b63e958714db5d818 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 21 Apr 2017 15:36:08 -0400 Subject: [PATCH 10/18] Make directory_request_set_guard_state static. --- src/or/directory.c | 13 ++++++++++--- src/or/directory.h | 2 -- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index b504808b60..4a1b68c497 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -120,6 +120,8 @@ static int client_likes_consensus(networkstatus_t *v, const char *want_url); static void connection_dir_close_consensus_fetches( dir_connection_t *except_this_one, const char *resource); +static void directory_request_set_guard_state(directory_request_t *req, + struct circuit_guard_state_t *state); /********* START VARIABLES **********/ @@ -1045,7 +1047,10 @@ struct directory_request_t { time_t if_modified_since; /** Hidden-service-specific information */ const rend_data_t *rend_query; - circuit_guard_state_t *guard_state; // XXXX Does this belong? + /** Used internally to directory.c: gets informed when the attempt to + * connect to the directory succeeds or fails, if that attempt bears on the + * directory's usability as a directory guard. */ + circuit_guard_state_t *guard_state; }; /** @@ -1195,11 +1200,13 @@ directory_request_set_rend_query(directory_request_t *req, } req->rend_query = query; } -void +/** Set a static circuit_guard_state_t object to affliate with the request in + * req. This object will receive notification when the attempt to + * connect to the guard either succeeds or fails. */ +static void directory_request_set_guard_state(directory_request_t *req, circuit_guard_state_t *state) { - // XXXX make static. req->guard_state = state; } diff --git a/src/or/directory.h b/src/or/directory.h index 6b2f50b45d..291ced4191 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -69,8 +69,6 @@ void directory_request_set_if_modified_since(directory_request_t *req, time_t if_modified_since); void directory_request_set_rend_query(directory_request_t *req, const rend_data_t *query); -void directory_request_set_guard_state(directory_request_t *req, - struct circuit_guard_state_t *state); void directory_request_set_routerstatus(directory_request_t *req, const routerstatus_t *rs); From 900d36e3b101d9ac6ff854d4ec0ba8ac63369cc9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 21 Apr 2017 15:37:38 -0400 Subject: [PATCH 11/18] Rename, revise, and document directory_request_is_dir_specified. --- src/or/directory.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 4a1b68c497..881069024d 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -1210,12 +1210,17 @@ directory_request_set_guard_state(directory_request_t *req, req->guard_state = state; } +/** + * Internal: Return true if any information for contacting the directory in + * req has been set, other than by the routerstatus. */ static int -directory_request_is_dir_specified(const directory_request_t *req) +directory_request_dir_contact_info_specified(const directory_request_t *req) { - // XXXX inline and revise - return (req->or_addr_port.port || req->dir_addr_port.port) && - ! tor_digest_is_zero(req->digest); + /* We only check for ports here, since we don't use an addr unless the port + * is set */ + return (req->or_addr_port.port || + req->dir_addr_port.port || + ! tor_digest_is_zero(req->digest)); } /** @@ -1301,7 +1306,8 @@ directory_initiate_request,(directory_request_t *request)) { tor_assert(request); if (request->routerstatus) { - tor_assert_nonfatal(! directory_request_is_dir_specified(request)); + tor_assert_nonfatal( + ! directory_request_dir_contact_info_specified(request)); if (directory_request_set_dir_from_routerstatus(request) < 0) { return; } From cc677e88d4795a6fc38339aa85ac1eae9cf14347 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 21 Apr 2017 15:49:10 -0400 Subject: [PATCH 12/18] Refactor directory_send_command to take a directory_request_t --- src/or/directory.c | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 881069024d..fbfc66a734 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -98,9 +98,8 @@ * connection_finished_connecting() in connection.c */ static void directory_send_command(dir_connection_t *conn, - int purpose, int direct, const char *resource, - const char *payload, size_t payload_len, - time_t if_modified_since); + int direct, + const directory_request_t *request); static int body_is_plausible(const char *body, size_t body_len, int purpose); static char *http_get_header(const char *headers, const char *which); static void http_set_address_origin(const char *headers, connection_t *conn); @@ -1320,9 +1319,6 @@ directory_initiate_request,(directory_request_t *request)) const uint8_t router_purpose = request->router_purpose; const dir_indirection_t indirection = request->indirection; const char *resource = request->resource; - const char *payload = request->payload; - const size_t payload_len = request->payload_len; - const time_t if_modified_since = request->if_modified_since; const rend_data_t *rend_query = request->rend_query; circuit_guard_state_t *guard_state = request->guard_state; @@ -1441,9 +1437,7 @@ directory_initiate_request,(directory_request_t *request)) /* fall through */ case 0: /* queue the command on the outbuf */ - directory_send_command(conn, dir_purpose, 1, resource, - payload, payload_len, - if_modified_since); + directory_send_command(conn, 1, request); connection_watch_events(TO_CONN(conn), READ_EVENT | WRITE_EVENT); /* writable indicates finish, readable indicates broken link, error indicates broken link in windowsland. */ @@ -1497,9 +1491,7 @@ directory_initiate_request,(directory_request_t *request)) } conn->base_.state = DIR_CONN_STATE_CLIENT_SENDING; /* queue the command on the outbuf */ - directory_send_command(conn, dir_purpose, 0, resource, - payload, payload_len, - if_modified_since); + directory_send_command(conn, 0, request); connection_watch_events(TO_CONN(conn), READ_EVENT|WRITE_EVENT); connection_start_reading(ENTRY_TO_CONN(linked_conn)); @@ -1602,17 +1594,22 @@ copy_ipv6_address(char* destination, const char* source, size_t len, } } -/** Queue an appropriate HTTP command on conn-\>outbuf. The other args - * are as in directory_request_set_...() - * - * +/** Queue an appropriate HTTP command for request on + * conn-\>outbuf. If direct is true, we're making a + * non-anonymized connection to the dirport. */ static void directory_send_command(dir_connection_t *conn, - int purpose, int direct, const char *resource, - const char *payload, size_t payload_len, - time_t if_modified_since) + const int direct, + const directory_request_t *req) { + tor_assert(req); + const int purpose = req->dir_purpose; + const char *resource = req->resource; + const char *payload = req->payload; + const size_t payload_len = req->payload_len; + const time_t if_modified_since = req->if_modified_since; + char proxystring[256]; char hoststring[128]; /* NEEDS to be the same size hoststring. From 2a99bf008df2e03a709fdd4a9be5d3ddc3a34238 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 21 Apr 2017 15:54:50 -0400 Subject: [PATCH 13/18] move the definition of directory_command_should_use_begindir --- src/or/directory.c | 76 +++++++++++++++++++++++----------------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index fbfc66a734..8eb2fdafb6 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -956,6 +956,44 @@ directory_must_use_begindir(const or_options_t *options) return !public_server_mode(options); } +struct directory_request_t { + /** + * These fields specify which directory we're contacting. Routerstatus, + * if present, overrides the other fields. + * + * @{ */ + tor_addr_port_t or_addr_port; + tor_addr_port_t dir_addr_port; + char digest[DIGEST_LEN]; + + const routerstatus_t *routerstatus; + /** @} */ + /** One of DIR_PURPOSE_* other than DIR_PURPOSE_SERVER. Describes what + * kind of operation we'll be doing (upload/download), and of what kind + * of document. */ + uint8_t dir_purpose; + /** One of ROUTER_PURPOSE_*; used for uploads and downloads of routerinfo + * and extrainfo docs. */ + uint8_t router_purpose; + /** Enum: determines whether to anonymize, and whether to use dirport or + * orport. */ + dir_indirection_t indirection; + /** Alias to the variable part of the URL for this request */ + const char *resource; + /** Alias to the payload to upload (if any) */ + const char *payload; + /** Number of bytes to upload from payload */ + size_t payload_len; + /** Value to send in an if-modified-since header, or 0 for none. */ + time_t if_modified_since; + /** Hidden-service-specific information */ + const rend_data_t *rend_query; + /** Used internally to directory.c: gets informed when the attempt to + * connect to the directory succeeds or fails, if that attempt bears on the + * directory's usability as a directory guard. */ + circuit_guard_state_t *guard_state; +}; + /** Evaluate the situation and decide if we should use an encrypted * "begindir-style" connection for this directory request. * 0) If there is no DirPort, yes. @@ -1014,44 +1052,6 @@ directory_command_should_use_begindir(const or_options_t *options, return 1; } -struct directory_request_t { - /** - * These fields specify which directory we're contacting. Routerstatus, - * if present, overrides the other fields. - * - * @{ */ - tor_addr_port_t or_addr_port; - tor_addr_port_t dir_addr_port; - char digest[DIGEST_LEN]; - - const routerstatus_t *routerstatus; - /** @} */ - /** One of DIR_PURPOSE_* other than DIR_PURPOSE_SERVER. Describes what - * kind of operation we'll be doing (upload/download), and of what kind - * of document. */ - uint8_t dir_purpose; - /** One of ROUTER_PURPOSE_*; used for uploads and downloads of routerinfo - * and extrainfo docs. */ - uint8_t router_purpose; - /** Enum: determines whether to anonymize, and whether to use dirport or - * orport. */ - dir_indirection_t indirection; - /** Alias to the variable part of the URL for this request */ - const char *resource; - /** Alias to the payload to upload (if any) */ - const char *payload; - /** Number of bytes to upload from payload */ - size_t payload_len; - /** Value to send in an if-modified-since header, or 0 for none. */ - time_t if_modified_since; - /** Hidden-service-specific information */ - const rend_data_t *rend_query; - /** Used internally to directory.c: gets informed when the attempt to - * connect to the directory succeeds or fails, if that attempt bears on the - * directory's usability as a directory guard. */ - circuit_guard_state_t *guard_state; -}; - /** * Create and return a new directory_request_t with purpose * dir_purpose. From d8ff01b2b4ceb5a7f43422a059b31f2f2956d264 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 21 Apr 2017 15:55:23 -0400 Subject: [PATCH 14/18] Refactor directory_command_should_use_begindir to use directory_request_t --- src/or/directory.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 8eb2fdafb6..7767bb99d1 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -1007,12 +1007,16 @@ struct directory_request_t { */ static int directory_command_should_use_begindir(const or_options_t *options, - const tor_addr_t *or_addr, int or_port, - const tor_addr_t *dir_addr, int dir_port, - dir_indirection_t indirection, + const directory_request_t *req, const char **reason) { - (void)dir_addr; + const tor_addr_t *or_addr = &req->or_addr_port.addr; + //const tor_addr_t *dir_addr = &req->dir_addr_port.addr; + const int or_port = req->or_addr_port.port; + const int dir_port = req->dir_addr_port.port; + + const dir_indirection_t indirection = req->indirection; + tor_assert(reason); *reason = NULL; @@ -1331,11 +1335,8 @@ directory_initiate_request,(directory_request_t *request)) const char *begindir_reason = NULL; /* Should the connection be to a relay's OR port (and inside that we will * send our directory request)? */ - const int use_begindir = directory_command_should_use_begindir(options, - &or_addr_port->addr, or_addr_port->port, - &dir_addr_port->addr, dir_addr_port->port, - indirection, - &begindir_reason); + const int use_begindir = + directory_command_should_use_begindir(options, request, &begindir_reason); /* Will the connection go via a three-hop Tor circuit? Note that this * is separate from whether it will use_begindir. */ From 91553417c7820d3620d98dc044c5d445535135f0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 21 Apr 2017 16:39:05 -0400 Subject: [PATCH 15/18] Fix a comment that held the remaining reference to the old api. --- src/test/test_connection.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/test_connection.c b/src/test/test_connection.c index 05dd6f4ec9..7e5193b203 100644 --- a/src/test/test_connection.c +++ b/src/test/test_connection.c @@ -265,7 +265,7 @@ test_conn_get_rend_setup(const struct testcase_t *tc) rend_cache_init(); - /* TODO: use directory_initiate_command_rend() to do this - maybe? */ + /* TODO: use directory_initiate_request() to do this - maybe? */ tor_assert(strlen(TEST_CONN_REND_ADDR) == REND_SERVICE_ID_LEN_BASE32); conn->rend_data = rend_data_client_create(TEST_CONN_REND_ADDR, NULL, NULL, REND_NO_AUTH); From 0f6420e53aa33a9e754f1d6c3c5bec12ac65f153 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 24 Apr 2017 12:15:25 -0400 Subject: [PATCH 16/18] Changes file for dirreq refactoring branch --- changes/ticket21646 | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changes/ticket21646 diff --git a/changes/ticket21646 b/changes/ticket21646 new file mode 100644 index 0000000000..a0e4fb6352 --- /dev/null +++ b/changes/ticket21646 @@ -0,0 +1,6 @@ + o Code simplification and refactoring: + - Our API to launch directory requests has been greatly simplified + to become more extensible and less error-prone. We'll be using + this to improve support for adding extra headers to directory + requests. Closes ticket 21646. + From 52316f9969a049ec5e2d72c2f0002f0ed32b60cb Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 27 Apr 2017 09:27:00 -0400 Subject: [PATCH 17/18] Include UPLOAD_RENDDESC_V2 in PURPOSE_IS_UPLOAD This was only used in one place before, and it's safe to update it. --- src/or/directory.c | 4 +--- src/or/or.h | 7 ++++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 7767bb99d1..872cce2c9e 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -1171,9 +1171,7 @@ directory_request_set_payload(directory_request_t *req, const char *payload, size_t payload_len) { - tor_assert(DIR_PURPOSE_IS_UPLOAD(req->dir_purpose) || - // XXXX why not included? - req->dir_purpose == DIR_PURPOSE_UPLOAD_RENDDESC_V2); + tor_assert(DIR_PURPOSE_IS_UPLOAD(req->dir_purpose)); req->payload = payload; req->payload_len = payload_len; diff --git a/src/or/or.h b/src/or/or.h index 21b8ca5018..5a42aada50 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -423,12 +423,13 @@ typedef enum { #define DIR_PURPOSE_FETCH_MICRODESC 19 #define DIR_PURPOSE_MAX_ 19 -/** True iff p is a purpose corresponding to uploading data to a - * directory server. */ +/** True iff p is a purpose corresponding to uploading + * data to a directory server. */ #define DIR_PURPOSE_IS_UPLOAD(p) \ ((p)==DIR_PURPOSE_UPLOAD_DIR || \ (p)==DIR_PURPOSE_UPLOAD_VOTE || \ - (p)==DIR_PURPOSE_UPLOAD_SIGNATURES) + (p)==DIR_PURPOSE_UPLOAD_SIGNATURES || \ + (p)==DIR_PURPOSE_UPLOAD_RENDDESC_V2) #define EXIT_PURPOSE_MIN_ 1 /** This exit stream wants to do an ordinary connect. */ From b4fe0a6a0321f241cc0a37ca6fc528aae33afd74 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 27 Apr 2017 09:30:08 -0400 Subject: [PATCH 18/18] Improve control flow in authority_certs_fetch_resource_impl --- src/or/routerlist.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/or/routerlist.c b/src/or/routerlist.c index bff2cca048..060862f25c 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -961,17 +961,16 @@ authority_certs_fetch_resource_impl(const char *resource, directory_request_set_or_addr_port(req, &or_ap); if (dir_hint) directory_request_set_directory_id_digest(req, dir_hint); - } - - if (rs) { - /* If we've just downloaded a consensus from a directory, re-use that + } else if (rs) { + /* And if we've just downloaded a consensus from a directory, re-use that * directory */ req = directory_request_new(DIR_PURPOSE_FETCH_CERTIFICATE); directory_request_set_routerstatus(req, rs); } if (req) { - /* Fill in the other request fields, and send the request. */ + /* We've set up a request object -- fill in the other request fields, and + * send the request. */ directory_request_set_indirection(req, indirection); directory_request_set_resource(req, resource); directory_initiate_request(req);