diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 0be54c533c..b56190fee2 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -75,7 +75,7 @@ static int onion_pick_cpath_exit(origin_circuit_t *circ, extend_info_t *exit, int is_hs_v3_rp_circuit); static crypt_path_t *onion_next_hop_in_cpath(crypt_path_t *cpath); static int onion_extend_cpath(origin_circuit_t *circ); -static int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice); +STATIC int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice); static int circuit_send_first_onion_skin(origin_circuit_t *circ); static int circuit_build_no_more_hops(origin_circuit_t *circ); static int circuit_send_intermediate_onion_skin(origin_circuit_t *circ, @@ -2586,7 +2586,7 @@ onion_extend_cpath(origin_circuit_t *circ) /** Create a new hop, annotate it with information about its * corresponding router choice, and append it to the * end of the cpath head_ptr. */ -static int +STATIC int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice) { crypt_path_t *hop = tor_malloc_zero(sizeof(crypt_path_t)); diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 1335e12c6a..d4657921ea 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -585,7 +585,8 @@ circuit_expire_building(void) TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len : -1, circuit_state_to_string(victim->state), - channel_state_to_string(victim->n_chan->state)); + victim->n_chan ? + channel_state_to_string(victim->n_chan->state) : "none"); /* We count the timeout here for CBT, because technically this * was a timeout, and the timeout value needs to reset if we @@ -610,7 +611,8 @@ circuit_expire_building(void) TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len : -1, circuit_state_to_string(victim->state), - channel_state_to_string(victim->n_chan->state), + victim->n_chan ? + channel_state_to_string(victim->n_chan->state) : "none", (long)build_close_ms); } } diff --git a/src/test/Makefile.nmake b/src/test/Makefile.nmake index 605f1a92c3..cfbe281b94 100644 --- a/src/test/Makefile.nmake +++ b/src/test/Makefile.nmake @@ -18,6 +18,7 @@ TEST_OBJECTS = test.obj test_addr.obj test_channel.obj test_channeltls.obj \ test_config.obj test_connection.obj \ test_cell_formats.obj test_relay.obj test_replay.obj \ test_channelpadding.obj \ + test_circuitstats.obj \ test_scheduler.obj test_introduce.obj test_hs.obj tinytest.obj tinytest.obj: ..\ext\tinytest.c diff --git a/src/test/include.am b/src/test/include.am index bbf8a370e2..b3a99503d2 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -98,6 +98,7 @@ src_test_test_SOURCES = \ src/test/test_circuitmux.c \ src/test/test_circuitbuild.c \ src/test/test_circuituse.c \ + src/test/test_circuitstats.c \ src/test/test_compat_libevent.c \ src/test/test_config.c \ src/test/test_connection.c \ diff --git a/src/test/test.c b/src/test/test.c index 00857c2386..bf591e1761 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1178,6 +1178,7 @@ struct testgroup_t testgroups[] = { { "circuitlist/", circuitlist_tests }, { "circuitmux/", circuitmux_tests }, { "circuituse/", circuituse_tests }, + { "circuitstats/", circuitstats_tests }, { "compat/libevent/", compat_libevent_tests }, { "config/", config_tests }, { "connection/", connection_tests }, diff --git a/src/test/test.h b/src/test/test.h index b1a3366a80..792ef6a065 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -188,6 +188,7 @@ extern struct testcase_t circuitbuild_tests[]; extern struct testcase_t circuitlist_tests[]; extern struct testcase_t circuitmux_tests[]; extern struct testcase_t circuituse_tests[]; +extern struct testcase_t circuitstats_tests[]; extern struct testcase_t compat_libevent_tests[]; extern struct testcase_t config_tests[]; extern struct testcase_t connection_tests[]; diff --git a/src/test/test_circuitstats.c b/src/test/test_circuitstats.c new file mode 100644 index 0000000000..1c6a2ba28f --- /dev/null +++ b/src/test/test_circuitstats.c @@ -0,0 +1,199 @@ +/* Copyright (c) 2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#define CIRCUITBUILD_PRIVATE +#define CIRCUITSTATS_PRIVATE +#define CHANNEL_PRIVATE_ + +#include "or.h" +#include "test.h" +#include "test_helpers.h" +#include "log_test_helpers.h" +#include "config.h" +#include "circuitlist.h" +#include "circuitbuild.h" +#include "circuitstats.h" +#include "circuituse.h" +#include "channel.h" + +void test_circuitstats_timeout(void *arg); +void test_circuitstats_hoplen(void *arg); +origin_circuit_t *subtest_fourhop_circuit(struct timeval, int); +origin_circuit_t *add_opened_threehop(void); +origin_circuit_t *build_unopened_fourhop(struct timeval); + +void circuit_free(circuit_t *circ); +int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice); + +static int marked_for_close; +/* Mock function because we are not trying to test the close circuit that does + * an awful lot of checks on the circuit object. */ +static void +mock_circuit_mark_for_close(circuit_t *circ, int reason, int line, + const char *file) +{ + (void) circ; + (void) reason; + (void) line; + (void) file; + marked_for_close = 1; + return; +} + +origin_circuit_t * +add_opened_threehop(void) +{ + origin_circuit_t *or_circ = origin_circuit_new(); + extend_info_t fakehop; + memset(&fakehop, 0, sizeof(fakehop)); + + TO_CIRCUIT(or_circ)->purpose = CIRCUIT_PURPOSE_C_GENERAL; + + or_circ->build_state = tor_malloc_zero(sizeof(cpath_build_state_t)); + or_circ->build_state->desired_path_len = DEFAULT_ROUTE_LEN; + + onion_append_hop(&or_circ->cpath, &fakehop); + onion_append_hop(&or_circ->cpath, &fakehop); + onion_append_hop(&or_circ->cpath, &fakehop); + + or_circ->has_opened = 1; + TO_CIRCUIT(or_circ)->state = CIRCUIT_STATE_OPEN; + TO_CIRCUIT(or_circ)->purpose = CIRCUIT_PURPOSE_C_GENERAL; + + return or_circ; +} + +origin_circuit_t * +build_unopened_fourhop(struct timeval circ_start_time) +{ + origin_circuit_t *or_circ = origin_circuit_new(); + extend_info_t *fakehop = tor_malloc_zero(sizeof(extend_info_t)); + memset(fakehop, 0, sizeof(extend_info_t)); + + TO_CIRCUIT(or_circ)->purpose = CIRCUIT_PURPOSE_C_GENERAL; + TO_CIRCUIT(or_circ)->timestamp_began = circ_start_time; + TO_CIRCUIT(or_circ)->timestamp_created = circ_start_time; + + or_circ->build_state = tor_malloc_zero(sizeof(cpath_build_state_t)); + or_circ->build_state->desired_path_len = 4; + + onion_append_hop(&or_circ->cpath, fakehop); + onion_append_hop(&or_circ->cpath, fakehop); + onion_append_hop(&or_circ->cpath, fakehop); + onion_append_hop(&or_circ->cpath, fakehop); + + return or_circ; +} + +origin_circuit_t * +subtest_fourhop_circuit(struct timeval circ_start_time, int should_timeout) +{ + origin_circuit_t *or_circ = build_unopened_fourhop(circ_start_time); + + // Now make them open one at a time and call + // circuit_build_times_handle_completed_hop(); + or_circ->cpath->state = CPATH_STATE_OPEN; + circuit_build_times_handle_completed_hop(or_circ); + tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, 0); + + or_circ->cpath->next->state = CPATH_STATE_OPEN; + circuit_build_times_handle_completed_hop(or_circ); + tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, 0); + + // Third hop: We should count it now. + or_circ->cpath->next->next->state = CPATH_STATE_OPEN; + circuit_build_times_handle_completed_hop(or_circ); + tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, + !should_timeout); // 1 if counted, 0 otherwise + + // Fourth hop: Don't double count + or_circ->cpath->next->next->next->state = CPATH_STATE_OPEN; + circuit_build_times_handle_completed_hop(or_circ); + tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, + !should_timeout); + + done: + return or_circ; +} + +void +test_circuitstats_hoplen(void *arg) +{ + /* Plan: + * 0. Test no other opened circs (relaxed timeout) + * 1. Check >3 hop circ building w/o timeout + * 2. Check >3 hop circs w/ timeouts.. + */ + struct timeval circ_start_time; + origin_circuit_t *threehop = NULL; + origin_circuit_t *fourhop = NULL; + (void)arg; + MOCK(circuit_mark_for_close_, mock_circuit_mark_for_close); + + circuit_build_times_init(get_circuit_build_times_mutable()); + + // Let's set a close_ms to 2X the initial timeout, so we can + // test relaxed functionality (which uses the close_ms timeout) + get_circuit_build_times_mutable()->close_ms *= 2; + + tor_gettimeofday(&circ_start_time); + circ_start_time.tv_sec -= 119; // make us hit "relaxed" cutoff + + // Test 1: Build a fourhop circuit that should get marked + // as relaxed and eventually counted by circuit_expire_building + // (but not before) + fourhop = subtest_fourhop_circuit(circ_start_time, 0); + tt_int_op(fourhop->relaxed_timeout, OP_EQ, 0); + tt_int_op(marked_for_close, OP_EQ, 0); + circuit_expire_building(); + tt_int_op(marked_for_close, OP_EQ, 0); + tt_int_op(fourhop->relaxed_timeout, OP_EQ, 1); + TO_CIRCUIT(fourhop)->timestamp_began.tv_sec -= 119; + circuit_expire_building(); + tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, 1); + tt_int_op(marked_for_close, OP_EQ, 1); + + circuit_free(TO_CIRCUIT(fourhop)); + circuit_build_times_reset(get_circuit_build_times_mutable()); + + // Test 2: Add a threehop circuit for non-relaxed timeouts + threehop = add_opened_threehop(); + + /* This circuit should not timeout */ + tor_gettimeofday(&circ_start_time); + circ_start_time.tv_sec -= 59; + fourhop = subtest_fourhop_circuit(circ_start_time, 0); + circuit_expire_building(); + tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, 1); + tt_int_op(TO_CIRCUIT(fourhop)->purpose, OP_NE, + CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT); + + circuit_free((circuit_t *)fourhop); + circuit_build_times_reset(get_circuit_build_times_mutable()); + + /* Test 3: This circuit should now time out and get marked as a + * measurement circuit, but still get counted (and counted only once) + */ + circ_start_time.tv_sec -= 2; + fourhop = subtest_fourhop_circuit(circ_start_time, 0); + tt_int_op(TO_CIRCUIT(fourhop)->purpose, OP_EQ, + CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT); + tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, 1); + circuit_expire_building(); + tt_int_op(get_circuit_build_times()->total_build_times, OP_EQ, 1); + + done: + UNMOCK(circuit_mark_for_close_); + circuit_free(TO_CIRCUIT(threehop)); + circuit_free(TO_CIRCUIT(fourhop)); + circuit_build_times_free_timeouts(get_circuit_build_times_mutable()); +} + +#define TEST_CIRCUITSTATS(name, flags) \ + { #name, test_##name, (flags), NULL, NULL } + +struct testcase_t circuitstats_tests[] = { + TEST_CIRCUITSTATS(circuitstats_hoplen, TT_FORK), + END_OF_TESTCASES +}; +