commit 195ccce94e250a150e208f7a8fb9ba8375b6fe89 Author: Chelsea H. Komlo <chelsea.komlo@gmail.com> Date: Sat Oct 8 19:28:38 2016 -0500 Refactor to use purpose_needs_anonymity and remove is_sensitive_dir_purpose --- changes/ticket20077 | 3 +++ src/or/connection_edge.c | 2 +- src/or/directory.c | 25 +++++++++---------------- src/or/directory.h | 3 --- src/test/test_dir.c | 47 ++++++++++++++++++++++++++++++++++++++++++----- 5 files changed, 55 insertions(+), 25 deletions(-) diff --git a/changes/ticket20077 b/changes/ticket20077 new file mode 100644 index 0000000..bc0e38e --- /dev/null +++ b/changes/ticket20077 @@ -0,0 +1,3 @@ + o Code simplification and refactoring: + - Remove redundant behavior of is_sensitive_dir_purpose, refactor to use + only purpose_needs_anonymity diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 24842e4..44dfcef 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -2434,7 +2434,7 @@ connection_ap_handshake_send_begin(entry_connection_t *ap_conn) * Otherwise, directory connections are typically one-hop. * This matches the earlier check for directory connection path anonymity * in directory_initiate_command_rend(). */ - if (is_sensitive_dir_purpose(linked_dir_conn_base->purpose)) { + if (purpose_needs_anonymity(linked_dir_conn_base->purpose, 0)) { assert_circ_anonymity_ok(circ, options); } } else { diff --git a/src/or/directory.c b/src/or/directory.c index 1f894d9..8dc3095 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -120,17 +120,22 @@ static void connection_dir_close_consensus_fetches( /********* END VARIABLES ************/ -/** Return true iff the directory purpose <b>dir_purpose</b> (and if it's - * fetching descriptors, it's fetching them for <b>router_purpose</b>) - * must use an anonymous connection to a directory. */ +/** Return false if the directory purpose <b>dir_purpose</b> + * does not require an anonymous (three-hop) connection. + * + * Return true 1) by default, 2) if all directory actions have + * specifically been configured to be over an anonymous connection, + * or 3) if the router is a bridge */ int purpose_needs_anonymity(uint8_t dir_purpose, uint8_t router_purpose) { if (get_options()->AllDirActionsPrivate) return 1; + if (router_purpose == ROUTER_PURPOSE_BRIDGE) return 1; /* if no circuits yet, this might break bootstrapping, but it's * needed to be safe. */ + if (dir_purpose == DIR_PURPOSE_UPLOAD_DIR || dir_purpose == DIR_PURPOSE_UPLOAD_VOTE || dir_purpose == DIR_PURPOSE_UPLOAD_SIGNATURES || @@ -1078,18 +1083,6 @@ directory_initiate_command(const tor_addr_t *or_addr, uint16_t or_port, if_modified_since, NULL); } -/** Return non-zero iff a directory connection with purpose - * <b>dir_purpose</b> reveals sensitive information about a Tor - * instance's client activities. (Such connections must be performed - * through normal three-hop Tor circuits.) */ -int -is_sensitive_dir_purpose(uint8_t dir_purpose) -{ - return ((dir_purpose == DIR_PURPOSE_HAS_FETCHED_RENDDESC_V2) || - (dir_purpose == DIR_PURPOSE_UPLOAD_RENDDESC_V2) || - (dir_purpose == DIR_PURPOSE_FETCH_RENDDESC_V2)); -} - /** 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. */ @@ -1137,7 +1130,7 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port, log_debug(LD_DIR, "Initiating %s", dir_conn_purpose_to_string(dir_purpose)); - if (is_sensitive_dir_purpose(dir_purpose)) { + if (purpose_needs_anonymity(dir_purpose, router_purpose)) { tor_assert(anonymized_connection || rend_non_anonymous_mode_enabled(options)); } diff --git a/src/or/directory.h b/src/or/directory.h index 9477948..f04e7ab 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -132,10 +132,7 @@ int download_status_get_n_failures(const download_status_t *dls); int download_status_get_n_attempts(const download_status_t *dls); time_t download_status_get_next_attempt_at(const download_status_t *dls); -/* Yes, these two functions are confusingly similar. - * Let's sort that out in #20077. */ int purpose_needs_anonymity(uint8_t dir_purpose, uint8_t router_purpose); -int is_sensitive_dir_purpose(uint8_t dir_purpose); #ifdef TOR_UNIT_TESTS /* Used only by directory.c and test_dir.c */ diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 77c5dc1..73a8a35 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -3253,17 +3253,52 @@ test_dir_http_handling(void *args) } static void -test_dir_purpose_needs_anonymity(void *arg) +test_dir_purpose_needs_anonymity_returns_true_for_bridges(void *arg) { (void)arg; + tt_int_op(1, ==, purpose_needs_anonymity(0, ROUTER_PURPOSE_BRIDGE)); - tt_int_op(1, ==, purpose_needs_anonymity(0, ROUTER_PURPOSE_GENERAL)); - tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_MICRODESC, - ROUTER_PURPOSE_GENERAL)); + tt_int_op(1, ==, purpose_needs_anonymity(DIR_PURPOSE_HAS_FETCHED_RENDDESC_V2, + ROUTER_PURPOSE_BRIDGE)); + done: ; +} + +static void +test_dir_purpose_needs_anonymity_returns_true_for_sensitive_purpose(void *arg) +{ + (void)arg; + + tt_int_op(1, ==, purpose_needs_anonymity( + DIR_PURPOSE_HAS_FETCHED_RENDDESC_V2, + ROUTER_PURPOSE_GENERAL)); + tt_int_op(1, ==, purpose_needs_anonymity( + DIR_PURPOSE_UPLOAD_RENDDESC_V2, 0)); + tt_int_op(1, ==, purpose_needs_anonymity( + DIR_PURPOSE_FETCH_RENDDESC_V2, 0)); done: ; } static void +test_dir_purpose_needs_anonymity_ret_false_for_non_sensitive_conn(void *arg) +{ + (void)arg; + + tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_UPLOAD_DIR, + ROUTER_PURPOSE_GENERAL)); + tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_UPLOAD_VOTE, 0)); + tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_UPLOAD_SIGNATURES, 0)); + tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_STATUS_VOTE, 0)); + tt_int_op(0, ==, purpose_needs_anonymity( + DIR_PURPOSE_FETCH_DETACHED_SIGNATURES, 0)); + tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_CONSENSUS, 0)); + tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_CERTIFICATE, 0)); + tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_SERVERDESC, 0)); + tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_EXTRAINFO, 0)); + tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_MICRODESC, 0)); + done: ; +} + +static void test_dir_fetch_type(void *arg) { (void)arg; @@ -5464,7 +5499,9 @@ struct testcase_t dir_tests[] = { DIR(fmt_control_ns, 0), DIR(dirserv_set_routerstatus_testing, 0), DIR(http_handling, 0), - DIR(purpose_needs_anonymity, 0), + DIR(purpose_needs_anonymity_returns_true_for_bridges, 0), + DIR(purpose_needs_anonymity_returns_true_for_sensitive_purpose, 0), + DIR(purpose_needs_anonymity_ret_false_for_non_sensitive_conn, 0), DIR(fetch_type, 0), DIR(packages, 0), DIR(download_status_schedule, 0),