tor-commits April 2017

tor-commits@lists.torproject.org

19 participants
966 discussions

[tor/master] Unit tests for consdiffmgr module
by nickm＠torproject.org 24 Apr '17

24 Apr '17

commit 7fc37d41b47f106939cae559a6874fa46bca6150 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 14 12:35:02 2017 -0400 Unit tests for consdiffmgr module Initial tests. These just try adding a few consensuses, looking them up, and making sure that consensus diffs are generated in a more or less reasonable-looking way. It's enough for 87% coverage, but it leaves out a lot of functionality. --- src/or/consdiffmgr.c | 4 +- src/or/consdiffmgr.h | 5 + src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_consdiffmgr.c | 305 ++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 316 insertions(+), 1 deletion(-) diff --git a/src/or/consdiffmgr.c b/src/or/consdiffmgr.c index 7d26b0c..0920d08 100644 --- a/src/or/consdiffmgr.c +++ b/src/or/consdiffmgr.c @@ -11,6 +11,8 @@ * and serve the diffs from those documents to the latest consensus. */ +#define CONSDIFFMGR_PRIVATE + #include "or.h" #include "conscache.h" #include "consdiff.h" @@ -122,7 +124,7 @@ cdm_labels_prepend_sha3(config_line_t **labels, * <b>valid_after</b> time in the cache. Return that consensus if it's * present, or NULL if it's missing. */ -static consensus_cache_entry_t * +STATIC consensus_cache_entry_t * cdm_cache_lookup_consensus(consensus_flavor_t flavor, time_t valid_after) { char formatted_time[ISO_TIME_LEN+1]; diff --git a/src/or/consdiffmgr.h b/src/or/consdiffmgr.h index 860e055..474876e 100644 --- a/src/or/consdiffmgr.h +++ b/src/or/consdiffmgr.h @@ -34,5 +34,10 @@ int consdiffmgr_cleanup(void); void consdiffmgr_configure(const consdiff_cfg_t *cfg); void consdiffmgr_free_all(void); +#ifdef CONSDIFFMGR_PRIVATE +STATIC consensus_cache_entry_t *cdm_cache_lookup_consensus( + consensus_flavor_t flavor, time_t valid_after); +#endif + #endif diff --git a/src/test/include.am b/src/test/include.am index c92eab1..1b16d0f 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -89,6 +89,7 @@ src_test_test_SOURCES = \ src/test/test_connection.c \ src/test/test_conscache.c \ src/test/test_consdiff.c \ + src/test/test_consdiffmgr.c \ src/test/test_containers.c \ src/test/test_controller.c \ src/test/test_controller_events.c \ diff --git a/src/test/test.c b/src/test/test.c index 77ea449..90e3d02 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1197,6 +1197,7 @@ struct testgroup_t testgroups[] = { { "connection/", connection_tests }, { "conscache/", conscache_tests }, { "consdiff/", consdiff_tests }, + { "consdiffmgr/", consdiffmgr_tests }, { "container/", container_tests }, { "control/", controller_tests }, { "control/event/", controller_event_tests }, diff --git a/src/test/test.h b/src/test/test.h index 252a239..3d7d05e 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -192,6 +192,7 @@ extern struct testcase_t config_tests[]; extern struct testcase_t connection_tests[]; extern struct testcase_t conscache_tests[]; extern struct testcase_t consdiff_tests[]; +extern struct testcase_t consdiffmgr_tests[]; extern struct testcase_t container_tests[]; extern struct testcase_t controller_tests[]; extern struct testcase_t controller_event_tests[]; diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c new file mode 100644 index 0000000..e80a39e --- /dev/null +++ b/src/test/test_consdiffmgr.c @@ -0,0 +1,305 @@ +/* Copyright (c) 2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#define CONSDIFFMGR_PRIVATE + +#include "or.h" +#include "config.h" +#include "conscache.h" +#include "consdiff.h" +#include "consdiffmgr.h" +#include "cpuworker.h" +#include "networkstatus.h" +#include "workqueue.h" + +#include "test.h" +#include "log_test_helpers.h" + +// ============================== Setup/teardown the consdiffmgr +// These functions get run before/after each test in this module + +static void * +consdiffmgr_test_setup(const struct testcase_t *arg) +{ + (void)arg; + char *ddir_fname = tor_strdup(get_fname_rnd("datadir_cdm")); + tor_free(get_options_mutable()->DataDirectory); + get_options_mutable()->DataDirectory = ddir_fname; // now owns the pointer. + check_private_dir(ddir_fname, CPD_CREATE, NULL); + + consdiff_cfg_t consdiff_cfg = { 7200, 300 }; + consdiffmgr_configure(&consdiff_cfg); + return (void *)1; // must return something non-null. +} +static int +consdiffmgr_test_teardown(const struct testcase_t *arg, void *ignore) +{ + (void)arg; + (void)ignore; + consdiffmgr_free_all(); + return 1; +} +static struct testcase_setup_t setup_diffmgr = { + consdiffmgr_test_setup, + consdiffmgr_test_teardown +}; + +// ============================== NS faking functions +// These functions are for making quick fake consensus objects and +// strings that are just good enough for consdiff and consdiffmgr. + +static networkstatus_t * +fake_ns_new(consensus_flavor_t flav, time_t valid_after) +{ + networkstatus_t *ns = tor_malloc_zero(sizeof(networkstatus_t)); + ns->type = NS_TYPE_CONSENSUS; + ns->flavor = flav; + ns->valid_after = valid_after; + return ns; +} + +static char * +fake_ns_body_new(consensus_flavor_t flav, time_t valid_after) +{ + const char *flavor_string = flav == FLAV_NS ? "" : " microdesc"; + char valid_after_string[ISO_TIME_LEN+1]; + + format_iso_time(valid_after_string, valid_after); + char *random_stuff = crypto_random_hostname(3, 25, "junk ", ""); + + char *consensus; + tor_asprintf(&consensus, + "network-status-version 3%s\n" + "vote-status consensus\n" + "valid-after %s\n" + "r name ccccccccccccccccc etc\nsample\n" + "r name eeeeeeeeeeeeeeeee etc\nbar\n" + "%s\n", + flavor_string, + valid_after_string, + random_stuff); + tor_free(random_stuff); + return consensus; +} + +// ============================== Cpuworker mocking code +// These mocking functions and types capture the cpuworker calls +// so we can inspect them and run them in the main thread. +static smartlist_t *fake_cpuworker_queue = NULL; +typedef struct fake_work_queue_ent_t { + enum workqueue_reply_t (*fn)(void *, void *); + void (*reply_fn)(void *); + void *arg; +} fake_work_queue_ent_t; +static struct workqueue_entry_s * +mock_cpuworker_queue_work(enum workqueue_reply_t (*fn)(void *, void *), + void (*reply_fn)(void *), + void *arg) +{ + if (! fake_cpuworker_queue) + fake_cpuworker_queue = smartlist_new(); + + fake_work_queue_ent_t *ent = tor_malloc_zero(sizeof(*ent)); + ent->fn = fn; + ent->reply_fn = reply_fn; + ent->arg = arg; + smartlist_add(fake_cpuworker_queue, ent); + return (struct workqueue_entry_s *)ent; +} +static int +mock_cpuworker_run_work(void) +{ + if (! fake_cpuworker_queue) + return 0; + SMARTLIST_FOREACH(fake_cpuworker_queue, fake_work_queue_ent_t *, ent, { + enum workqueue_reply_t r = ent->fn(NULL, ent->arg); + if (r != WQ_RPL_REPLY) + return -1; + }); + return 0; +} +static void +mock_cpuworker_handle_replies(void) +{ + if (! fake_cpuworker_queue) + return; + SMARTLIST_FOREACH(fake_cpuworker_queue, fake_work_queue_ent_t *, ent, { + ent->reply_fn(ent->arg); + }); + smartlist_free(fake_cpuworker_queue); + fake_cpuworker_queue = NULL; +} + +// ============================== Beginning of tests + +static void +test_consdiffmgr_add(void *arg) +{ + (void) arg; + time_t now = approx_time(); + + consensus_cache_entry_t *ent = NULL; + networkstatus_t *ns_tmp = fake_ns_new(FLAV_NS, now); + const char *dummy = "foo"; + int r = consdiffmgr_add_consensus(dummy, ns_tmp); + tt_int_op(r, OP_EQ, 0); + + /* If we add it again, it won't work */ + setup_capture_of_logs(LOG_INFO); + dummy = "bar"; + r = consdiffmgr_add_consensus(dummy, ns_tmp); + tt_int_op(r, OP_EQ, -1); + expect_single_log_msg_containing("We already have a copy of that " + "consensus"); + mock_clean_saved_logs(); + + /* But it will work fine if the flavor is different */ + dummy = "baz"; + ns_tmp->flavor = FLAV_MICRODESC; + r = consdiffmgr_add_consensus(dummy, ns_tmp); + tt_int_op(r, OP_EQ, 0); + + /* And it will work fine if the time is different */ + dummy = "quux"; + ns_tmp->flavor = FLAV_NS; + ns_tmp->valid_after = now - 60; + r = consdiffmgr_add_consensus(dummy, ns_tmp); + tt_int_op(r, OP_EQ, 0); + + /* If we add one a long long time ago, it will fail. */ + dummy = "xyzzy"; + ns_tmp->valid_after = 86400 * 100; /* A few months into 1970 */ + r = consdiffmgr_add_consensus(dummy, ns_tmp); + tt_int_op(r, OP_EQ, -1); + expect_single_log_msg_containing("it's too old."); + + /* Try looking up a consensuses. */ + ent = cdm_cache_lookup_consensus(FLAV_NS, now-60); + tt_assert(ent); + consensus_cache_entry_incref(ent); + size_t s; + const uint8_t *body; + r = consensus_cache_entry_get_body(ent, &body, &s); + tt_int_op(r, OP_EQ, 0); + tt_int_op(s, OP_EQ, 4); + tt_mem_op(body, OP_EQ, "quux", 4); + + /* Try looking up another entry, but fail */ + tt_assert(NULL == cdm_cache_lookup_consensus(FLAV_MICRODESC, now-60)); + tt_assert(NULL == cdm_cache_lookup_consensus(FLAV_NS, now-61)); + + done: + networkstatus_vote_free(ns_tmp); + teardown_capture_of_logs(); + consensus_cache_entry_decref(ent); +} + +static void +test_consdiffmgr_make_diffs(void *arg) +{ + (void)arg; + networkstatus_t *ns = NULL; + char *ns_body = NULL, *md_ns_body = NULL, *md_ns_body_2 = NULL; + char *applied = NULL, *diff_text = NULL; + time_t now = approx_time(); + int r; + consensus_cache_entry_t *diff = NULL; + uint8_t md_ns_sha3[DIGEST256_LEN]; + consdiff_status_t diff_status; + + MOCK(cpuworker_queue_work, mock_cpuworker_queue_work); + + // Try rescan with no consensuses: shouldn't crash or queue work. + consdiffmgr_rescan(); + tt_ptr_op(NULL, OP_EQ, fake_cpuworker_queue); + + // Make two consensuses, 1 hour sec ago. + ns = fake_ns_new(FLAV_NS, now-3600); + ns_body = fake_ns_body_new(FLAV_NS, now-3600); + r = consdiffmgr_add_consensus(ns_body, ns); + networkstatus_vote_free(ns); + tor_free(ns_body); + tt_int_op(r, OP_EQ, 0); + + ns = fake_ns_new(FLAV_MICRODESC, now-3600); + md_ns_body = fake_ns_body_new(FLAV_MICRODESC, now-3600); + r = consdiffmgr_add_consensus(md_ns_body, ns); + crypto_digest256((char*)md_ns_sha3, md_ns_body, strlen(md_ns_body), + DIGEST_SHA3_256); + networkstatus_vote_free(ns); + tt_int_op(r, OP_EQ, 0); + + // No diffs will be generated. + consdiffmgr_rescan(); + tt_ptr_op(NULL, OP_EQ, fake_cpuworker_queue); + + // Add a MD consensus from 45 minutes ago. This should cause one diff + // worth of work to get queued. + ns = fake_ns_new(FLAV_MICRODESC, now-45*60); + md_ns_body_2 = fake_ns_body_new(FLAV_MICRODESC, now-45*60); + r = consdiffmgr_add_consensus(md_ns_body_2, ns); + networkstatus_vote_free(ns); + tt_int_op(r, OP_EQ, 0); + + consdiffmgr_rescan(); + tt_ptr_op(NULL, OP_NE, fake_cpuworker_queue); + tt_int_op(1, OP_EQ, smartlist_len(fake_cpuworker_queue)); + // XXXX not working yet + /* + diff_status = consdiffmgr_find_diff_from(&diff, FLAV_MICRODESC, + DIGEST_SHA3_256, + md_ns_sha3, DIGEST256_LEN); + tt_int_op(CONSDIFF_IN_PROGRESS, OP_EQ, diff_status); + */ + + // Now run that process and get the diff. + r = mock_cpuworker_run_work(); + tt_int_op(r, OP_EQ, 0); + mock_cpuworker_handle_replies(); + + // At this point we should be able to get that diff. + diff_status = consdiffmgr_find_diff_from(&diff, FLAV_MICRODESC, + DIGEST_SHA3_256, + md_ns_sha3, DIGEST256_LEN); + tt_int_op(CONSDIFF_AVAILABLE, OP_EQ, diff_status); + tt_assert(diff); + + /* Make sure applying the diff actually works */ + const uint8_t *diff_body; + size_t diff_size; + r = consensus_cache_entry_get_body(diff, &diff_body, &diff_size); + tt_int_op(r, OP_EQ, 0); + diff_text = tor_memdup_nulterm(diff_body, diff_size); + applied = consensus_diff_apply(md_ns_body, diff_text); + tt_assert(applied); + tt_str_op(applied, OP_EQ, md_ns_body_2); + + /* Rescan again: no more work to do. */ + consdiffmgr_rescan(); + tt_ptr_op(NULL, OP_EQ, fake_cpuworker_queue); + + done: + tor_free(md_ns_body); + tor_free(md_ns_body_2); + tor_free(diff_text); + tor_free(applied); +} + +#define TEST(name) \ + { #name, test_consdiffmgr_ ## name , TT_FORK, &setup_diffmgr, NULL } + +struct testcase_t consdiffmgr_tests[] = { + TEST(add), + TEST(make_diffs), + // XXXX Test: deleting consensuses for being too old + // XXXX Test: deleting diffs for not being to most recent consensus + // XXXX Test: Objects of unrecognized doctype are not cleaned. + // XXXX Test: Objects with bad iso time are not cleaned. + // XXXX Test: only generate diffs to most recent consensus + // XXXX Test: making diffs from some old consensuses, but having diffs + // for others. + // XXXX Test: Failure to open cache??? + // XXXX Test: failure to create consensus diff. + END_OF_TESTCASES +}; +

1 0

[tor/master] Merge branch 'consdiff_numeric_squashed'
by nickm＠torproject.org 24 Apr '17

24 Apr '17

commit 9ba10d714a8ae84864c4fe159f4d6780a93d168f Merge: 8ff2a8c a16de7a Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Apr 24 09:33:55 2017 -0400 Merge branch 'consdiff_numeric_squashed' src/or/consdiff.c | 3 +++ src/test/test_consdiff.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 1 deletion(-)

1 0

[tor/master] consdiff: Reject ranges with non-numeric chars
by nickm＠torproject.org 24 Apr '17

24 Apr '17

commit a16de7a7cff14e133e01f5605f4600c67de5d91f Author: Sebastian Hahn <sebastian(a)torproject.org> Date: Tue Apr 18 13:43:55 2017 +0200 consdiff: Reject ranges with non-numeric chars Fixes bug #21964 --- src/or/consdiff.c | 3 +++ src/test/test_consdiff.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/or/consdiff.c b/src/or/consdiff.c index d2a2af1..7e3d674 100644 --- a/src/or/consdiff.c +++ b/src/or/consdiff.c @@ -757,6 +757,9 @@ get_linenum(const char **s, int *num_out) { int ok; char *next; + if (!TOR_ISDIGIT(**s)) { + return -1; + } *num_out = (int) tor_parse_long(*s, 10, 0, INT32_MAX, &ok, &next); if (ok && next) { *s = next; diff --git a/src/test/test_consdiff.c b/src/test/test_consdiff.c index 1b4e2ad..829e8f8 100644 --- a/src/test/test_consdiff.c +++ b/src/test/test_consdiff.c @@ -746,6 +746,55 @@ test_consdiff_apply_ed_diff(void *arg) smartlist_clear(diff); + /* Ranges must be numeric only and cannot contain spaces. */ + smartlist_add_linecpy(diff, area, "0, 4d"); + mock_clean_saved_logs(); + cons2 = apply_ed_diff(cons1, diff, 0); + tt_ptr_op(NULL, OP_EQ, cons2); + expect_single_log_msg_containing("an ed command was missing a range " + "end line number."); + + smartlist_clear(diff); + + /* '+' is not a number. */ + smartlist_add_linecpy(diff, area, "+0,4d"); + mock_clean_saved_logs(); + cons2 = apply_ed_diff(cons1, diff, 0); + tt_ptr_op(NULL, OP_EQ, cons2); + expect_single_log_msg_containing("an ed command was missing a line number"); + + smartlist_clear(diff); + + /* range duplication */ + smartlist_add_linecpy(diff, area, "0,4d,5d"); + mock_clean_saved_logs(); + cons2 = apply_ed_diff(cons1, diff, 0); + tt_ptr_op(NULL, OP_EQ, cons2); + expect_single_log_msg_containing("an ed command longer than one char was " + "found"); + + smartlist_clear(diff); + + /* space before command */ + smartlist_add_linecpy(diff, area, "0,4 d"); + mock_clean_saved_logs(); + cons2 = apply_ed_diff(cons1, diff, 0); + tt_ptr_op(NULL, OP_EQ, cons2); + expect_single_log_msg_containing("an ed command longer than one char was " + "found"); + + smartlist_clear(diff); + + /* space inside number */ + smartlist_add_linecpy(diff, area, "0,4 5d"); + mock_clean_saved_logs(); + cons2 = apply_ed_diff(cons1, diff, 0); + tt_ptr_op(NULL, OP_EQ, cons2); + expect_single_log_msg_containing("an ed command longer than one char was " + "found"); + + smartlist_clear(diff); + /* Test appending text, 'a'. */ consensus_split_lines(diff, "3a\nU\nO\n.\n0a\nV\n.\n", area); cons2 = apply_ed_diff(cons1, diff, 0); @@ -775,7 +824,7 @@ test_consdiff_apply_ed_diff(void *arg) smartlist_free(cons2); /* Test changing text, 'c'. */ - consensus_split_lines(diff, "4c\nT\nX\n.\n1, 2c\nM\n.\n", area); + consensus_split_lines(diff, "4c\nT\nX\n.\n1,2c\nM\n.\n", area); cons2 = apply_ed_diff(cons1, diff, 0); tt_ptr_op(NULL, OP_NE, cons2); tt_int_op(5, OP_EQ, smartlist_len(cons2));

1 0

[tor/release-0.2.9] control: Wrong check on base16_decode return value
by nickm＠torproject.org 24 Apr '17

24 Apr '17

commit 4a9f6894300781b998b0753dd0f5a388bb61cc91 Author: David Goulet <dgoulet(a)torproject.org> Date: Fri Apr 21 11:49:30 2017 -0400 control: Wrong check on base16_decode return value The GETINFO extra-info/digest/<digest> broke in commit 568dc27a19 that refactored the base16_decode() API to return the decoded length. Unfortunately, that if() condition should have checked for the correct length instead of an error which broke the command in tor-0.2.9.1-alpha. Fixes #22034 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/bug22034 | 4 ++++ src/or/control.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/changes/bug22034 b/changes/bug22034 new file mode 100644 index 0000000..6d9e188 --- /dev/null +++ b/changes/bug22034 @@ -0,0 +1,4 @@ + o Minor bugfixes (control port, regression): + - The GETINFO extra-info/digest/<digest> command was broken because of a + wrong base16 decode return value check. In was introduced in a refactor + of that API. Fixex bug #22034; bugfix on tor-0.2.9.1-alpha. diff --git a/src/or/control.c b/src/or/control.c index c8c5062..1bf1e33 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -2055,7 +2055,7 @@ getinfo_helper_dir(control_connection_t *control_conn, char d[DIGEST_LEN]; signed_descriptor_t *sd = NULL; if (base16_decode(d, sizeof(d), question, strlen(question)) - != sizeof(d)) { + == sizeof(d)) { /* XXXX this test should move into extrainfo_get_by_descriptor_digest, * but I don't want to risk affecting other parts of the code, * especially since the rules for using our own extrainfo (including

1 0

[tor/release-0.2.9] Merge branch 'maint-0.2.9' into release-0.2.9
by nickm＠torproject.org 24 Apr '17

24 Apr '17

commit af9bbaa8c2480679171be2e8cfb4b7b0ef38099c Merge: e0d8ce3 4a9f689 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Apr 24 09:31:28 2017 -0400 Merge branch 'maint-0.2.9' into release-0.2.9 changes/bug22034 | 4 ++++ src/or/control.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-)

1 0

[tor/maint-0.2.9] control: Wrong check on base16_decode return value
by nickm＠torproject.org 24 Apr '17

24 Apr '17

1 0

[tor/release-0.3.0] control: Wrong check on base16_decode return value
by nickm＠torproject.org 24 Apr '17

24 Apr '17

1 0

[tor/maint-0.3.0] Merge branch 'maint-0.2.9' into maint-0.3.0
by nickm＠torproject.org 24 Apr '17

24 Apr '17

commit ae374e0a56014b72e54adf2afb5676ae6788124c Merge: f1613b5 4a9f689 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Apr 24 09:20:26 2017 -0400 Merge branch 'maint-0.2.9' into maint-0.3.0 changes/bug22034 | 4 ++++ src/or/control.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-)

1 0

[tor/release-0.3.0] Merge branch 'maint-0.2.9' into maint-0.3.0
by nickm＠torproject.org 24 Apr '17

24 Apr '17

1 0

[tor/maint-0.3.0] control: Wrong check on base16_decode return value
by nickm＠torproject.org 24 Apr '17

24 Apr '17

1 0

← Newer
1
...
28
29
30
31
32
33
34
...
97
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

tor-commits April 2017