commit 6413b2102f4ac968092a6b1bc58b78d4bb7f90de Author: AmreshVenugopal amresh.venugopal@gmail.com Date: Tue Sep 17 00:11:18 2019 +0530
control: Add GETINFO support for dumping microdesc consensus
- Allows control port to read microdesc consensus using: GETINFO dir/status-vote/microdesc/consensus
add: Helper function `getinfo_helper_current_consensus` test: check if GETINFO commands return expected consensus data.
Resolves 31684. --- changes/ticket31684 | 6 ++ src/feature/control/control_getinfo.c | 65 ++++++++++++---- src/feature/control/control_getinfo.h | 4 + src/feature/dircache/dirserv.c | 4 +- src/feature/dircache/dirserv.h | 2 +- src/feature/nodelist/networkstatus.c | 8 +- src/feature/nodelist/networkstatus.h | 3 + src/lib/fs/mmap.c | 16 ++-- src/lib/fs/mmap.h | 5 +- src/test/test_controller.c | 142 ++++++++++++++++++++++++++++++++++ 10 files changed, 222 insertions(+), 33 deletions(-)
diff --git a/changes/ticket31684 b/changes/ticket31684 new file mode 100644 index 000000000..4a9b7cb4a --- /dev/null +++ b/changes/ticket31684 @@ -0,0 +1,6 @@ + o Minor features (onion services): + - Implement a new GETINFO command to fetch microdescriptor consensus. + Closes ticket 31684. + o Code simplification and refactoring (onion services): + - Create a helper function that can fetch network status or microdesc + consensuses. diff --git a/src/feature/control/control_getinfo.c b/src/feature/control/control_getinfo.c index 3e31bb9e8..e83b9f6f9 100644 --- a/src/feature/control/control_getinfo.c +++ b/src/feature/control/control_getinfo.c @@ -325,6 +325,42 @@ getinfo_helper_current_time(control_connection_t *control_conn, return 0; }
+/** GETINFO helper for dumping different consensus flavors + * returns: 0 on success -1 on error. */ +STATIC int +getinfo_helper_current_consensus(consensus_flavor_t flavor, + char** answer, + const char** errmsg) +{ + const char *flavor_name = networkstatus_get_flavor_name(flavor); + if (!strcmp(flavor_name, "??")) { + *errmsg = "Could not open cached consensus. " + "Make sure FetchUselessDescriptors is set to 1."; + return -1; + } + if (we_want_to_fetch_flavor(get_options(), flavor)) { + /** Check from the cache */ + const cached_dir_t *consensus = dirserv_get_consensus(flavor_name); + if (consensus) { + *answer = tor_strdup(consensus->dir); + } + } + if (!*answer) { /* try loading it from disk */ + + tor_mmap_t *mapped = networkstatus_map_cached_consensus(flavor_name); + if (mapped) { + *answer = tor_memdup_nulterm(mapped->data, mapped->size); + tor_munmap_file(mapped); + } + if (!*answer) { /* generate an error */ + *errmsg = "Could not open cached consensus. " + "Make sure FetchUselessDescriptors is set to 1."; + return -1; + } + } + return 0; +} + /** Implementation helper for GETINFO: knows the answers for questions about * directory information. */ STATIC int @@ -576,23 +612,18 @@ getinfo_helper_dir(control_connection_t *control_conn, smartlist_free(descs); } else if (!strcmpstart(question, "dir/status/")) { *answer = tor_strdup(""); - } else if (!strcmp(question, "dir/status-vote/current/consensus")) { /* v3 */ - if (we_want_to_fetch_flavor(get_options(), FLAV_NS)) { - const cached_dir_t *consensus = dirserv_get_consensus("ns"); - if (consensus) - *answer = tor_strdup(consensus->dir); + } else if (!strcmp(question, "dir/status-vote/current/consensus")) { + int consensus_result = getinfo_helper_current_consensus(FLAV_NS, + answer, errmsg); + if (consensus_result == -1) { + return -1; } - if (!*answer) { /* try loading it from disk */ - tor_mmap_t *mapped = networkstatus_map_cached_consensus("ns"); - if (mapped) { - *answer = tor_memdup_nulterm(mapped->data, mapped->size); - tor_munmap_file(mapped); - } - if (!*answer) { /* generate an error */ - *errmsg = "Could not open cached consensus. " - "Make sure FetchUselessDescriptors is set to 1."; - return -1; - } + } else if (!strcmp(question, + "dir/status-vote/current/consensus-microdesc")) { + int consensus_result = getinfo_helper_current_consensus(FLAV_MICRODESC, + answer, errmsg); + if (consensus_result == -1) { + return -1; } } else if (!strcmp(question, "network-status")) { /* v1 */ static int network_status_warned = 0; @@ -1513,6 +1544,8 @@ static const getinfo_item_t getinfo_items[] = { "v2 networkstatus docs as retrieved from a DirPort."), ITEM("dir/status-vote/current/consensus", dir, "v3 Networkstatus consensus as retrieved from a DirPort."), + ITEM("dir/status-vote/current/consensus-microdesc", dir, + "v3 Microdescriptors consensus as retrieved from a DirPort."), ITEM("exit-policy/default", policies, "The default value appended to the configured exit policy."), ITEM("exit-policy/reject-private/default", policies, diff --git a/src/feature/control/control_getinfo.h b/src/feature/control/control_getinfo.h index 52978686d..86aaf7c4f 100644 --- a/src/feature/control/control_getinfo.h +++ b/src/feature/control/control_getinfo.h @@ -48,6 +48,10 @@ STATIC int getinfo_helper_downloads( control_connection_t *control_conn, const char *question, char **answer, const char **errmsg); +STATIC int getinfo_helper_current_consensus( + consensus_flavor_t flavor, + char **answer, + const char **errmsg); STATIC int getinfo_helper_dir( control_connection_t *control_conn, const char *question, char **answer, diff --git a/src/feature/dircache/dirserv.c b/src/feature/dircache/dirserv.c index 79400bf15..85331bfea 100644 --- a/src/feature/dircache/dirserv.c +++ b/src/feature/dircache/dirserv.c @@ -259,8 +259,8 @@ dirserv_set_cached_consensus_networkstatus(const char *networkstatus,
/** Return the latest downloaded consensus networkstatus in encoded, signed, * optionally compressed format, suitable for sending to clients. */ -cached_dir_t * -dirserv_get_consensus(const char *flavor_name) +MOCK_IMPL(cached_dir_t *, +dirserv_get_consensus,(const char *flavor_name)) { if (!cached_consensuses) return NULL; diff --git a/src/feature/dircache/dirserv.h b/src/feature/dircache/dirserv.h index 7f944459d..0c15c9ad1 100644 --- a/src/feature/dircache/dirserv.h +++ b/src/feature/dircache/dirserv.h @@ -82,7 +82,7 @@ int directory_permits_begindir_requests(const or_options_t *options); int directory_too_idle_to_fetch_descriptors(const or_options_t *options, time_t now);
-cached_dir_t *dirserv_get_consensus(const char *flavor_name); +MOCK_DECL(cached_dir_t *, dirserv_get_consensus, (const char *flavor_name)); void dirserv_set_cached_consensus_networkstatus(const char *consensus, size_t consensus_len, const char *flavor_name, diff --git a/src/feature/nodelist/networkstatus.c b/src/feature/nodelist/networkstatus.c index 496bafb86..b4b98301b 100644 --- a/src/feature/nodelist/networkstatus.c +++ b/src/feature/nodelist/networkstatus.c @@ -216,10 +216,10 @@ networkstatus_reset_download_failures(void) }
/** Return the filename used to cache the consensus of a given flavor */ -static char * -networkstatus_get_cache_fname(int flav, - const char *flavorname, - int unverified_consensus) +MOCK_IMPL(char *, +networkstatus_get_cache_fname,(int flav, + const char *flavorname, + int unverified_consensus)) { char buf[128]; const char *prefix; diff --git a/src/feature/nodelist/networkstatus.h b/src/feature/nodelist/networkstatus.h index 600fd7fbd..e2c6ba611 100644 --- a/src/feature/nodelist/networkstatus.h +++ b/src/feature/nodelist/networkstatus.h @@ -16,6 +16,9 @@
void networkstatus_reset_warnings(void); void networkstatus_reset_download_failures(void); +MOCK_DECL(char *,networkstatus_get_cache_fname,(int flav, + const char *flavorname, + int unverified_consensus)); tor_mmap_t *networkstatus_map_cached_consensus(const char *flavorname); int router_reload_consensus_networkstatus(void); void routerstatus_free_(routerstatus_t *rs); diff --git a/src/lib/fs/mmap.c b/src/lib/fs/mmap.c index f71c0cff7..9d50a476b 100644 --- a/src/lib/fs/mmap.c +++ b/src/lib/fs/mmap.c @@ -42,8 +42,8 @@ * failure, return NULL. Sets errno properly, using ERANGE to mean * "empty file". Must only be called on trusted Tor-owned files, as changing * the underlying file's size causes unspecified behavior. */ -tor_mmap_t * -tor_mmap_file(const char *filename) +MOCK_IMPL(tor_mmap_t *, +tor_mmap_file,(const char *filename)) { int fd; /* router file */ char *string; @@ -111,8 +111,8 @@ tor_mmap_file(const char *filename) } /** Release storage held for a memory mapping; returns 0 on success, * or -1 on failure (and logs a warning). */ -int -tor_munmap_file(tor_mmap_t *handle) +MOCK_IMPL(int, +tor_munmap_file,(tor_mmap_t *handle)) { int res;
@@ -132,8 +132,8 @@ tor_munmap_file(tor_mmap_t *handle) return res; } #elif defined(_WIN32) -tor_mmap_t * -tor_mmap_file(const char *filename) +MOCK_IMPL(tor_mmap_t *, +tor_mmap_file,(const char *filename)) { TCHAR tfilename[MAX_PATH]= {0}; tor_mmap_t *res = tor_malloc_zero(sizeof(tor_mmap_t)); @@ -213,8 +213,8 @@ tor_mmap_file(const char *filename) }
/* Unmap the file, and return 0 for success or -1 for failure */ -int -tor_munmap_file(tor_mmap_t *handle) +MOCK_IMPL(int, +tor_munmap_file,(tor_mmap_t *handle)) { if (handle == NULL) return 0; diff --git a/src/lib/fs/mmap.h b/src/lib/fs/mmap.h index 61aad544b..beb053510 100644 --- a/src/lib/fs/mmap.h +++ b/src/lib/fs/mmap.h @@ -13,6 +13,7 @@ #define TOR_MMAP_H
#include "lib/cc/compat_compiler.h" +#include "lib/testsupport/testsupport.h" #include <stddef.h>
#ifdef _WIN32 @@ -35,7 +36,7 @@ typedef struct tor_mmap_t {
} tor_mmap_t;
-tor_mmap_t *tor_mmap_file(const char *filename); -int tor_munmap_file(tor_mmap_t *handle); +MOCK_DECL(tor_mmap_t *, tor_mmap_file, (const char *filename)); +MOCK_DECL(int, tor_munmap_file, (tor_mmap_t *handle));
#endif /* !defined(TOR_MMAP_H) */ diff --git a/src/test/test_controller.c b/src/test/test_controller.c index b9cbe0a14..d11b26e8d 100644 --- a/src/test/test_controller.c +++ b/src/test/test_controller.c @@ -27,6 +27,7 @@ #include "feature/dirclient/download_status_st.h" #include "feature/nodelist/microdesc_st.h" #include "feature/nodelist/node_st.h" +#include "feature/dircache/dirserv.c"
typedef struct { const char *input; @@ -1689,6 +1690,143 @@ test_download_status_bridge(void *arg) return; }
+/** Mock cached consensus */ +static cached_dir_t *mock_ns_consensus_cache; +static cached_dir_t *mock_microdesc_consensus_cache; + +/** Mock the function that retrieves consensus from cache. These use a + * global variable so that they can be cleared from within the test. + * The actual code retains the pointer to the consensus data, but + * we are doing this here, to prevent memory leaks + * from within the tests */ +static cached_dir_t * +mock_dirserv_get_consensus(const char *flavor_name) +{ + if (!strcmp(flavor_name, "ns")) { + mock_ns_consensus_cache = tor_malloc_zero(sizeof(cached_dir_t)); + mock_ns_consensus_cache->dir = tor_strdup("mock_ns_consensus"); + return mock_ns_consensus_cache; + } else { + mock_microdesc_consensus_cache = tor_malloc_zero(sizeof(cached_dir_t)); + mock_microdesc_consensus_cache->dir = tor_strdup( + "mock_microdesc_consensus"); + return mock_microdesc_consensus_cache; + } +} + +/** Mock the function that retrieves consensuses + * from a files in the directory. */ +static tor_mmap_t * +mock_tor_mmap_file(const char* filename) +{ + tor_mmap_t *res; + res = tor_malloc_zero(sizeof(tor_mmap_t)); + if (strstr(filename, "cached-consensus") != NULL) { + res->data = "mock_ns_consensus"; + } else if (strstr(filename, "cached-microdesc-consensus") != NULL) { + res->data = "mock_microdesc_consensus"; + } else { + res->data = "."; + } + res->size = strlen(res->data); + return res; +} + +/** Mock the function that clears file data + * loaded into the memory */ +static int +mock_tor_munmap_file(tor_mmap_t *handle) +{ + tor_free(handle); + return 0; +} + +static void +test_getinfo_helper_current_consensus_from_file(void *arg) +{ + /* We just need one of these to pass, it doesn't matter what's in it */ + control_connection_t dummy; + /* Get results out */ + char *answer = NULL; + const char *errmsg = NULL; + + (void)arg; + + setup_bridge_mocks(); + MOCK(tor_mmap_file, mock_tor_mmap_file); + MOCK(tor_munmap_file, mock_tor_munmap_file); + + getinfo_helper_dir(&dummy, + "dir/status-vote/current/consensus", + &answer, + &errmsg); + tt_str_op(answer, OP_EQ, "mock_ns_consensus"); + tt_ptr_op(errmsg, OP_EQ, NULL); + tor_free(answer); + errmsg = NULL; + + getinfo_helper_dir(&dummy, + "dir/status-vote/current/consensus-microdesc", + &answer, + &errmsg); + tt_str_op(answer, OP_EQ, "mock_microdesc_consensus"); + tt_ptr_op(errmsg, OP_EQ, NULL); + errmsg = NULL; + + done: + clear_bridge_mocks(); + tor_free(answer); + UNMOCK(tor_mmap_file); + UNMOCK(tor_munmap_file); + return; +} + +static void +test_getinfo_helper_current_consensus_from_cache(void *arg) +{ + /* We just need one of these to pass, it doesn't matter what's in it */ + control_connection_t dummy; + /* Get results out */ + char *answer = NULL; + const char *errmsg = NULL; + + (void)arg; + setup_bridge_mocks(); + or_options_t *options = get_options_mutable(); + int previous_fetch_value = options->FetchUselessDescriptors; + options->FetchUselessDescriptors = 1; + MOCK(dirserv_get_consensus, mock_dirserv_get_consensus); + + getinfo_helper_dir(&dummy, + "dir/status-vote/current/consensus", + &answer, + &errmsg); + tt_str_op(answer, OP_EQ, "mock_ns_consensus"); + tt_ptr_op(errmsg, OP_EQ, NULL); + tor_free(answer); + tor_free(mock_ns_consensus_cache->dir); + tor_free(mock_ns_consensus_cache); + errmsg = NULL; + + getinfo_helper_dir(&dummy, + "dir/status-vote/current/consensus-microdesc", + &answer, + &errmsg); + tt_str_op(answer, OP_EQ, "mock_microdesc_consensus"); + tt_ptr_op(errmsg, OP_EQ, NULL); + tor_free(mock_microdesc_consensus_cache->dir); + tor_free(answer); + errmsg = NULL; + options->FetchUselessDescriptors = previous_fetch_value; + + done: + clear_bridge_mocks(); + tor_free(answer); + tor_free(mock_microdesc_consensus_cache); + UNMOCK(dirserv_get_consensus); + return; +} + /** Set timeval to a mock date and time. This is necessary * to make tor_gettimeofday() mockable. */ static void @@ -1838,6 +1976,10 @@ struct testcase_t controller_tests[] = { NULL }, { "download_status_consensus", test_download_status_consensus, 0, NULL, NULL }, + {"getinfo_helper_current_consensus_from_cache", + test_getinfo_helper_current_consensus_from_cache, 0, NULL, NULL }, + {"getinfo_helper_current_consensus_from_file", + test_getinfo_helper_current_consensus_from_file, 0, NULL, NULL }, { "download_status_cert", test_download_status_cert, 0, NULL, NULL }, { "download_status_desc", test_download_status_desc, 0, NULL, NULL },