January 2017 - tor-commits - lists.torproject.org

[tor/master] circuit: Add a function to get the next service intro circuit
by nickm＠torproject.org 13 Jan '17

13 Jan '17

commit 8a05e1a5d2227b0333940d004b97b1c96fa9d991 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Dec 21 13:34:45 2016 -0500 circuit: Add a function to get the next service intro circuit Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/circuitlist.c | 35 +++++++++++++++++++++++++++++++++++ src/or/circuitlist.h | 1 + 2 files changed, 36 insertions(+) diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 5943e51..4ff5eff 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -1440,6 +1440,41 @@ circuit_get_ready_rend_circ_by_rend_data(const rend_data_t *rend_data) return NULL; } +/** Return the first service introduction circuit originating from the global + * circuit list after start or at the start of the list if start + * is NULL. Return NULL if no circuit is found. + * + * A service introduction point circuit has a purpose of either + * CIRCUIT_PURPOSE_S_ESTABLISH_INTRO or CIRCUIT_PURPOSE_S_INTRO. This does not + * return a circuit marked for close and its state must be open. */ +origin_circuit_t * +circuit_get_next_service_intro_circ(origin_circuit_t *start) +{ + int idx = 0; + smartlist_t *lst = circuit_get_global_list(); + + if (start) { + idx = TO_CIRCUIT(start)->global_circuitlist_idx + 1; + } + + for ( ; idx < smartlist_len(lst); ++idx) { + circuit_t *circ = smartlist_get(lst, idx); + + /* Ignore a marked for close circuit or purpose not matching a service + * intro point or if the state is not open. */ + if (circ->marked_for_close || circ->state != CIRCUIT_STATE_OPEN || + (circ->purpose != CIRCUIT_PURPOSE_S_ESTABLISH_INTRO && + circ->purpose != CIRCUIT_PURPOSE_S_INTRO)) { + continue; + } + /* The purposes we are looking for are only for origin circuits so the + * following is valid. */ + return TO_ORIGIN_CIRCUIT(circ); + } + /* Not found. */ + return NULL; +} + /** Return the first circuit originating here in global_circuitlist after * start whose purpose is purpose, and where digest (if * set) matches the private key digest of the rend data associated with the diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h index d83801a..6abee37 100644 --- a/src/or/circuitlist.h +++ b/src/or/circuitlist.h @@ -47,6 +47,7 @@ origin_circuit_t *circuit_get_ready_rend_circ_by_rend_data( const rend_data_t *rend_data); origin_circuit_t *circuit_get_next_by_pk_and_purpose(origin_circuit_t *start, const uint8_t *digest, uint8_t purpose); +origin_circuit_t *circuit_get_next_service_intro_circ(origin_circuit_t *start); origin_circuit_t *circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info, int flags); void circuit_mark_all_unused_circs(void);

1 0

[tor/master] test: Add unit test for prune_services_on_reload()
by nickm＠torproject.org 13 Jan '17

13 Jan '17

commit 2d1fa58fb442122a64cdef9be1d18e8f53067038 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Dec 21 15:00:02 2016 -0500 test: Add unit test for prune_services_on_reload() Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/rendservice.c | 2 +- src/or/rendservice.h | 3 ++ src/test/test_hs.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 1 deletion(-) diff --git a/src/or/rendservice.c b/src/or/rendservice.c index a4e6452..2d94bb8 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -545,7 +545,7 @@ rend_service_check_dir_and_add(smartlist_t *service_list, /* If this is a reload and there were hidden services configured before, * keep the introduction points that are still needed and close the * other ones. */ -static void +STATIC void prune_services_on_reload(smartlist_t *old_service_list, smartlist_t *new_service_list) { diff --git a/src/or/rendservice.h b/src/or/rendservice.h index 4e6b9a2..3bfac0b 100644 --- a/src/or/rendservice.h +++ b/src/or/rendservice.h @@ -132,6 +132,9 @@ STATIC int rend_service_poison_new_single_onion_dir( STATIC ssize_t encode_establish_intro_cell_legacy(char *cell_body_out, crypto_pk_t *intro_key, char *rend_circ_nonce); +STATIC void prune_services_on_reload(smartlist_t *old_service_list, + smartlist_t *new_service_list); + #endif int num_rend_services(void); diff --git a/src/test/test_hs.c b/src/test/test_hs.c index ac9788c..fbaabe9 100644 --- a/src/test/test_hs.c +++ b/src/test/test_hs.c @@ -787,6 +787,126 @@ test_single_onion_poisoning(void *arg) tor_free(mock_options->DataDirectory); } +static rend_service_t * +helper_create_rend_service(const char *path) +{ + rend_service_t *s = tor_malloc_zero(sizeof(rend_service_t)); + s->ports = smartlist_new(); + s->intro_nodes = smartlist_new(); + s->expiring_nodes = smartlist_new(); + if (path) { + s->directory = tor_strdup(path); + } + return s; +} + +static void +test_prune_services_on_reload(void *arg) +{ + smartlist_t *new = smartlist_new(), *old = smartlist_new(); + /* Non ephemeral service. */ + rend_service_t *s1 = helper_create_rend_service("SomePath"); + /* Create a non ephemeral service with the _same_ path as so we can test the + * transfer of introduction point between the same services on reload. */ + rend_service_t *s2 = helper_create_rend_service(s1->directory); + /* Ephemeral service (directory is NULL). */ + rend_service_t *e1 = helper_create_rend_service(NULL); + rend_service_t *e2 = helper_create_rend_service(NULL); + + (void) arg; + + { + /* Add both services to the old list. */ + smartlist_add(old, s1); + smartlist_add(old, e1); + /* Only put the non ephemeral in the new list. */ + smartlist_add(new, s1); + prune_services_on_reload(old, new); + /* We expect that the ephemeral one is in the new list but removed from + * the old one. */ + tt_int_op(smartlist_len(old), OP_EQ, 1); + tt_assert(smartlist_get(old, 0) == s1); + tt_int_op(smartlist_len(new), OP_EQ, 2); + tt_assert(smartlist_get(new, 0) == s1); + tt_assert(smartlist_get(new, 1) == e1); + /* Cleanup for next test. */ + smartlist_clear(new); + smartlist_clear(old); + } + + { + /* This test will make sure that only the ephemeral service is kept if the + * new list is empty. The old list should contain only the non ephemeral + * one. */ + smartlist_add(old, s1); + smartlist_add(old, e1); + prune_services_on_reload(old, new); + tt_int_op(smartlist_len(old), OP_EQ, 1); + tt_assert(smartlist_get(old, 0) == s1); + tt_int_op(smartlist_len(new), OP_EQ, 1); + tt_assert(smartlist_get(new, 0) == e1); + /* Cleanup for next test. */ + smartlist_clear(new); + smartlist_clear(old); + } + + { + /* This test makes sure that the new list stays the same even from the old + * list being completely different. */ + smartlist_add(new, s1); + smartlist_add(new, e1); + prune_services_on_reload(old, new); + tt_int_op(smartlist_len(old), OP_EQ, 0); + tt_int_op(smartlist_len(new), OP_EQ, 2); + tt_assert(smartlist_get(new, 0) == s1); + tt_assert(smartlist_get(new, 1) == e1); + /* Cleanup for next test. */ + smartlist_clear(new); + } + + { + rend_intro_point_t ip1; + /* This IP should be found in the s2 service after pruning. */ + smartlist_add(s1->intro_nodes, &ip1); + /* Setup our list. */ + smartlist_add(old, s1); + smartlist_add(new, s2); + prune_services_on_reload(old, new); + tt_int_op(smartlist_len(old), OP_EQ, 1); + /* Intro nodes have been moved to the s2 in theory so it must be empty. */ + tt_int_op(smartlist_len(s1->intro_nodes), OP_EQ, 0); + tt_int_op(smartlist_len(new), OP_EQ, 1); + rend_service_t *elem = smartlist_get(new, 0); + tt_assert(elem); + tt_assert(elem == s2); + tt_int_op(smartlist_len(elem->intro_nodes), OP_EQ, 1); + tt_assert(smartlist_get(elem->intro_nodes, 0) == &ip1); + smartlist_clear(s1->intro_nodes); + smartlist_clear(s2->intro_nodes); + /* Cleanup for next test. */ + smartlist_clear(new); + smartlist_clear(old); + } + + { + /* Test two ephemeral services. */ + smartlist_add(old, e1); + smartlist_add(old, e2); + prune_services_on_reload(old, new); + /* Check if they've all been transfered. */ + tt_int_op(smartlist_len(old), OP_EQ, 0); + tt_int_op(smartlist_len(new), OP_EQ, 2); + } + + done: + rend_service_free(s1); + rend_service_free(s2); + rend_service_free(e1); + rend_service_free(e2); + smartlist_free(new); + smartlist_free(old); +} + struct testcase_t hs_tests[] = { { "hs_rend_data", test_hs_rend_data, TT_FORK, NULL, NULL }, @@ -807,6 +927,9 @@ struct testcase_t hs_tests[] = { TT_FORK, &passthrough_setup, (void*)(CREATE_HS_DIR2) }, { "single_onion_poisoning_create_dir_both", test_single_onion_poisoning, TT_FORK, &passthrough_setup, (void*)(CREATE_HS_DIR1 | CREATE_HS_DIR2) }, + { "prune_services_on_reload", test_prune_services_on_reload, TT_FORK, + NULL, NULL }, + END_OF_TESTCASES };

1 0

[tor/master] Merge remote-tracking branch 'dgoulet/bug21054_030_01'
by nickm＠torproject.org 13 Jan '17

13 Jan '17

commit 3e45b12f38beca55f58ea612a9f77580e76e37d8 Merge: ac75c33 2d1fa58 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jan 13 16:45:55 2017 -0500 Merge remote-tracking branch 'dgoulet/bug21054_030_01' changes/bug21054 | 5 ++ src/or/circuitlist.c | 35 +++++++++++ src/or/circuitlist.h | 1 + src/or/rendservice.c | 172 ++++++++++++++++++++++++++++----------------------- src/or/rendservice.h | 3 + src/test/test_hs.c | 123 ++++++++++++++++++++++++++++++++++++ 6 files changed, 261 insertions(+), 78 deletions(-)

1 0

[tor/master] hs: Move and improve the service pruning code
by nickm＠torproject.org 13 Jan '17

13 Jan '17

commit 36b5ca2c8b07f4cbdb1b76e9fb0c10fff3d540ac Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Dec 21 13:35:22 2016 -0500 hs: Move and improve the service pruning code First, this commit moves the code used to prune the service list when reloading Tor (HUP signal for instance) to a function from rend_config_services(). Second, fix bug #21054, improve the code by using the newly added circuit_get_next_service_intro_circ() function instead of poking at the global list directly and add _many_ more comments. Fixes #21054. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/bug21054 | 5 ++ src/or/rendservice.c | 172 ++++++++++++++++++++++++++++----------------------- 2 files changed, 99 insertions(+), 78 deletions(-) diff --git a/changes/bug21054 b/changes/bug21054 new file mode 100644 index 0000000..bc49949 --- /dev/null +++ b/changes/bug21054 @@ -0,0 +1,5 @@ + o Minor bugfixes (hidden service): + - Fix the config reload pruning of old vs new services so it actually + works when both ephemeral and non ephemeral services were configured + which lead to a BUG() stacktrace. Close #21054; Bugfix on + tor-0.3.0.1-alpha. diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 1713a84..a4e6452 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -542,6 +542,95 @@ rend_service_check_dir_and_add(smartlist_t *service_list, return rend_add_service(s_list, service); } +/* If this is a reload and there were hidden services configured before, + * keep the introduction points that are still needed and close the + * other ones. */ +static void +prune_services_on_reload(smartlist_t *old_service_list, + smartlist_t *new_service_list) +{ + origin_circuit_t *ocirc = NULL; + smartlist_t *surviving_services = NULL; + + tor_assert(old_service_list); + tor_assert(new_service_list); + + /* This contains all _existing_ services that survives the relaod that is + * that haven't been removed from the configuration. The difference between + * this list and the new service list is that the new list can possibly + * contain newly configured service that have no introduction points opened + * yet nor key material loaded or generated. */ + surviving_services = smartlist_new(); + + /* Preserve the existing ephemeral services. + * + * This is the ephemeral service equivalent of the "Copy introduction + * points to new services" block, except there's no copy required since + * the service structure isn't regenerated. + * + * After this is done, all ephemeral services will be: + * * Removed from old_service_list, so the equivalent non-ephemeral code + * will not attempt to preserve them. + * * Added to the new_service_list (that previously only had the + * services listed in the configuration). + * * Added to surviving_services, which is the list of services that + * will NOT have their intro point closed. + */ + SMARTLIST_FOREACH_BEGIN(old_service_list, rend_service_t *, old) { + if (rend_service_is_ephemeral(old)) { + SMARTLIST_DEL_CURRENT(old_service_list, old); + smartlist_add(surviving_services, old); + smartlist_add(new_service_list, old); + } + } SMARTLIST_FOREACH_END(old); + + /* Copy introduction points to new services. This is O(n^2), but it's only + * called on reconfigure, so it's ok performance wise. */ + SMARTLIST_FOREACH_BEGIN(new_service_list, rend_service_t *, new) { + SMARTLIST_FOREACH_BEGIN(old_service_list, rend_service_t *, old) { + /* Skip ephemeral services as we only want to copy introduction points + * from current services to newly configured one that already exists. + * The same directory means it's the same service. */ + if (rend_service_is_ephemeral(new) || rend_service_is_ephemeral(old) || + strcmp(old->directory, new->directory)) { + continue; + } + smartlist_add_all(new->intro_nodes, old->intro_nodes); + smartlist_clear(old->intro_nodes); + smartlist_add_all(new->expiring_nodes, old->expiring_nodes); + smartlist_clear(old->expiring_nodes); + /* This regular service will survive the closing IPs step after. */ + smartlist_add(surviving_services, old); + break; + } SMARTLIST_FOREACH_END(old); + } SMARTLIST_FOREACH_END(new); + + /* For every service introduction circuit we can find, see if we have a + * matching surviving configured service. If not, close the circuit. */ + while ((ocirc = circuit_get_next_service_intro_circ(ocirc))) { + int keep_it = 0; + tor_assert(ocirc->rend_data); + SMARTLIST_FOREACH_BEGIN(surviving_services, const rend_service_t *, s) { + if (rend_circuit_pk_digest_eq(ocirc, (uint8_t *) s->pk_digest)) { + /* Keep this circuit as we have a matching configured service. */ + keep_it = 1; + break; + } + } SMARTLIST_FOREACH_END(s); + if (keep_it) { + continue; + } + log_info(LD_REND, "Closing intro point %s for service %s.", + safe_str_client(extend_info_describe( + ocirc->build_state->chosen_exit)), + safe_str_client(rend_data_get_address(ocirc->rend_data))); + /* Reason is FINISHED because service has been removed and thus the + * circuit is considered old/uneeded. */ + circuit_mark_for_close(TO_CIRCUIT(ocirc), END_CIRC_REASON_FINISHED); + } + smartlist_free(surviving_services); +} + /** Set up rend_service_list, based on the values of HiddenServiceDir and * HiddenServicePort in options. Return 0 on success and -1 on * failure. (If validate_only is set, parse, warn and return as @@ -807,84 +896,11 @@ rend_config_services(const or_options_t *options, int validate_only) * keep the introduction points that are still needed and close the * other ones. */ if (old_service_list && !validate_only) { - smartlist_t *surviving_services = smartlist_new(); - - /* Preserve the existing ephemeral services. - * - * This is the ephemeral service equivalent of the "Copy introduction - * points to new services" block, except there's no copy required since - * the service structure isn't regenerated. - * - * After this is done, all ephemeral services will be: - * * Removed from old_service_list, so the equivalent non-ephemeral code - * will not attempt to preserve them. - * * Added to the new rend_service_list (that previously only had the - * services listed in the configuration). - * * Added to surviving_services, which is the list of services that - * will NOT have their intro point closed. - */ - SMARTLIST_FOREACH(old_service_list, rend_service_t *, old, { - if (rend_service_is_ephemeral(old)) { - SMARTLIST_DEL_CURRENT(old_service_list, old); - smartlist_add(surviving_services, old); - smartlist_add(rend_service_list, old); - } - }); - - /* Copy introduction points to new services. */ - /* XXXX This is O(n^2), but it's only called on reconfigure, so it's - * probably ok? */ - SMARTLIST_FOREACH_BEGIN(rend_service_list, rend_service_t *, new) { - SMARTLIST_FOREACH_BEGIN(old_service_list, rend_service_t *, old) { - if (BUG(rend_service_is_ephemeral(new)) || - BUG(rend_service_is_ephemeral(old))) { - continue; - } - if (BUG(!new->directory) || BUG(!old->directory) || - strcmp(old->directory, new->directory)) { - continue; - } - smartlist_add_all(new->intro_nodes, old->intro_nodes); - smartlist_clear(old->intro_nodes); - smartlist_add_all(new->expiring_nodes, old->expiring_nodes); - smartlist_clear(old->expiring_nodes); - smartlist_add(surviving_services, old); - break; - } SMARTLIST_FOREACH_END(old); - } SMARTLIST_FOREACH_END(new); - - /* Close introduction circuits of services we don't serve anymore. */ - /* XXXX it would be nicer if we had a nicer abstraction to use here, - * so we could just iterate over the list of services to close, but - * once again, this isn't critical-path code. */ - SMARTLIST_FOREACH_BEGIN(circuit_get_global_list(), circuit_t *, circ) { - if (!circ->marked_for_close && - circ->state == CIRCUIT_STATE_OPEN && - (circ->purpose == CIRCUIT_PURPOSE_S_ESTABLISH_INTRO || - circ->purpose == CIRCUIT_PURPOSE_S_INTRO)) { - origin_circuit_t *oc = TO_ORIGIN_CIRCUIT(circ); - int keep_it = 0; - tor_assert(oc->rend_data); - SMARTLIST_FOREACH(surviving_services, rend_service_t *, ptr, { - if (rend_circuit_pk_digest_eq(oc, (uint8_t *) ptr->pk_digest)) { - keep_it = 1; - break; - } - }); - if (keep_it) - continue; - log_info(LD_REND, "Closing intro point %s for service %s.", - safe_str_client(extend_info_describe( - oc->build_state->chosen_exit)), - rend_data_get_address(oc->rend_data)); - circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED); - /* XXXX Is there another reason we should use here? */ - } - } - SMARTLIST_FOREACH_END(circ); - smartlist_free(surviving_services); - SMARTLIST_FOREACH(old_service_list, rend_service_t *, ptr, - rend_service_free(ptr)); + prune_services_on_reload(old_service_list, rend_service_list); + /* Every remaining service in the old list have been removed from the + * configuration so clean them up safely. */ + SMARTLIST_FOREACH(old_service_list, rend_service_t *, s, + rend_service_free(s)); smartlist_free(old_service_list); }

1 0

[tor/master] Merge remote-tracking branch 'dgoulet/bug20992_030_01'
by nickm＠torproject.org 13 Jan '17

13 Jan '17

commit dafdbc2e9f65d068354f43222296223a4e063155 Merge: c93428f c0a0c19 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jan 13 16:32:58 2017 -0500 Merge remote-tracking branch 'dgoulet/bug20992_030_01' src/test/test_hs_intropoint.c | 168 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 168 insertions(+)

1 0

[tor/master] test: Add ESTABLISH_INTRO unit tests
by nickm＠torproject.org 13 Jan '17

13 Jan '17

commit c0a0c197257c6909ea46dbb18e3b84f7caa8f7e9 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Jan 10 09:24:21 2017 -0500 test: Add ESTABLISH_INTRO unit tests This commit adds 3 unit tests which validates a wrong signature length, a wrong authentication key length and a wrong MAC in the cell. Closes #20992 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/test/test_hs_intropoint.c | 168 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 168 insertions(+) diff --git a/src/test/test_hs_intropoint.c b/src/test/test_hs_intropoint.c index 608988b..b4a81ea 100644 --- a/src/test/test_hs_intropoint.c +++ b/src/test/test_hs_intropoint.c @@ -12,12 +12,14 @@ #define CIRCUITLIST_PRIVATE #include "test.h" +#include "log_test_helpers.h" #include "crypto.h" #include "or.h" #include "ht.h" #include "hs/cell_establish_intro.h" +#include "hs_common.h" #include "hs_service.h" #include "hs_circuitmap.h" #include "hs_intropoint.h" @@ -143,6 +145,163 @@ test_establish_intro_wrong_keytype2(void *arg) circuit_free(TO_CIRCUIT(intro_circ)); } +/* Send a legit ESTABLISH_INTRO cell but with a wrong MAC. Should fail. */ +static void +test_establish_intro_wrong_mac(void *arg) +{ + int retval; + hs_cell_establish_intro_t *establish_intro_cell = NULL; + or_circuit_t *intro_circ = or_circuit_new(0,NULL);; + uint8_t cell_body[RELAY_PAYLOAD_SIZE]; + ssize_t cell_len = 0; + uint8_t circuit_key_material[DIGEST_LEN] = {0}; + + (void)arg; + + /* Get the auth key of the intro point */ + crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material)); + helper_prepare_circ_for_intro(intro_circ, circuit_key_material); + + /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we + attempt to parse it. */ + establish_intro_cell = generate_establish_intro_cell(circuit_key_material, + sizeof(circuit_key_material)); + tt_assert(establish_intro_cell); + /* Mangle one byte of the MAC. */ + uint8_t *handshake_ptr = + hs_cell_establish_intro_getarray_handshake_mac(establish_intro_cell); + handshake_ptr[TRUNNEL_SHA3_256_LEN - 1]++; + /* We need to resign the payload with that change. */ + { + ed25519_signature_t sig; + ed25519_keypair_t key_struct; + /* New keypair for the signature since we don't have access to the private + * key material generated earlier when creating the cell. */ + retval = ed25519_keypair_generate(&key_struct, 0); + tt_int_op(retval, OP_EQ, 0); + uint8_t *auth_key_ptr = + hs_cell_establish_intro_getarray_auth_key(establish_intro_cell); + memcpy(auth_key_ptr, key_struct.pubkey.pubkey, ED25519_PUBKEY_LEN); + /* Encode payload so we can sign it. */ + cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body), + establish_intro_cell); + tt_int_op(cell_len, >, 0); + + retval = ed25519_sign_prefixed(&sig, cell_body, + cell_len - + (ED25519_SIG_LEN + + sizeof(establish_intro_cell->sig_len)), + ESTABLISH_INTRO_SIG_PREFIX, &key_struct); + tt_int_op(retval, OP_EQ, 0); + /* And write the signature to the cell */ + uint8_t *sig_ptr = + hs_cell_establish_intro_getarray_sig(establish_intro_cell); + memcpy(sig_ptr, sig.sig, establish_intro_cell->sig_len); + /* Re-encode with the new signature. */ + cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body), + establish_intro_cell); + } + + /* Receive the cell. Should fail because our MAC is wrong. */ + setup_full_capture_of_logs(LOG_INFO); + retval = hs_intro_received_establish_intro(intro_circ, cell_body, cell_len); + expect_log_msg_containing("ESTABLISH_INTRO handshake_auth not as expected"); + teardown_capture_of_logs(); + tt_int_op(retval, ==, -1); + + done: + hs_cell_establish_intro_free(establish_intro_cell); + circuit_free(TO_CIRCUIT(intro_circ)); +} + +/* Send a legit ESTABLISH_INTRO cell but with a wrong auth key length. Should + * fail. */ +static void +test_establish_intro_wrong_auth_key_len(void *arg) +{ + int retval; + hs_cell_establish_intro_t *establish_intro_cell = NULL; + or_circuit_t *intro_circ = or_circuit_new(0,NULL);; + uint8_t cell_body[RELAY_PAYLOAD_SIZE]; + ssize_t cell_len = 0; + size_t bad_auth_key_len = ED25519_PUBKEY_LEN - 1; + uint8_t circuit_key_material[DIGEST_LEN] = {0}; + + (void)arg; + + /* Get the auth key of the intro point */ + crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material)); + helper_prepare_circ_for_intro(intro_circ, circuit_key_material); + + /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we + attempt to parse it. */ + establish_intro_cell = generate_establish_intro_cell(circuit_key_material, + sizeof(circuit_key_material)); + tt_assert(establish_intro_cell); + /* Mangle the auth key length. */ + hs_cell_establish_intro_set_auth_key_len(establish_intro_cell, + bad_auth_key_len); + hs_cell_establish_intro_setlen_auth_key(establish_intro_cell, + bad_auth_key_len); + cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body), + establish_intro_cell); + tt_int_op(cell_len, >, 0); + + /* Receive the cell. Should fail. */ + setup_full_capture_of_logs(LOG_INFO); + retval = hs_intro_received_establish_intro(intro_circ, cell_body, cell_len); + expect_log_msg_containing("ESTABLISH_INTRO auth key length is invalid"); + teardown_capture_of_logs(); + tt_int_op(retval, ==, -1); + + done: + hs_cell_establish_intro_free(establish_intro_cell); + circuit_free(TO_CIRCUIT(intro_circ)); +} + +/* Send a legit ESTABLISH_INTRO cell but with a wrong sig length. Should + * fail. */ +static void +test_establish_intro_wrong_sig_len(void *arg) +{ + int retval; + hs_cell_establish_intro_t *establish_intro_cell = NULL; + or_circuit_t *intro_circ = or_circuit_new(0,NULL);; + uint8_t cell_body[RELAY_PAYLOAD_SIZE]; + ssize_t cell_len = 0; + size_t bad_sig_len = ED25519_SIG_LEN - 1; + uint8_t circuit_key_material[DIGEST_LEN] = {0}; + + (void)arg; + + /* Get the auth key of the intro point */ + crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material)); + helper_prepare_circ_for_intro(intro_circ, circuit_key_material); + + /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we + attempt to parse it. */ + establish_intro_cell = generate_establish_intro_cell(circuit_key_material, + sizeof(circuit_key_material)); + tt_assert(establish_intro_cell); + /* Mangle the signature length. */ + hs_cell_establish_intro_set_sig_len(establish_intro_cell, bad_sig_len); + hs_cell_establish_intro_setlen_sig(establish_intro_cell, bad_sig_len); + cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body), + establish_intro_cell); + tt_int_op(cell_len, >, 0); + + /* Receive the cell. Should fail. */ + setup_full_capture_of_logs(LOG_INFO); + retval = hs_intro_received_establish_intro(intro_circ, cell_body, cell_len); + expect_log_msg_containing("ESTABLISH_INTRO sig len is invalid"); + teardown_capture_of_logs(); + tt_int_op(retval, ==, -1); + + done: + hs_cell_establish_intro_free(establish_intro_cell); + circuit_free(TO_CIRCUIT(intro_circ)); +} + /* Send a legit ESTABLISH_INTRO cell but slightly change the signature. Should * fail. */ static void @@ -356,6 +515,15 @@ struct testcase_t hs_intropoint_tests[] = { { "receive_establish_intro_wrong_sig", test_establish_intro_wrong_sig, TT_FORK, NULL, NULL }, + { "receive_establish_intro_wrong_sig_len", + test_establish_intro_wrong_sig_len, TT_FORK, NULL, NULL }, + + { "receive_establish_intro_wrong_auth_key_len", + test_establish_intro_wrong_auth_key_len, TT_FORK, NULL, NULL }, + + { "receive_establish_intro_wrong_mac", + test_establish_intro_wrong_mac, TT_FORK, NULL, NULL }, + END_OF_TESTCASES };

1 0

[tor/master] fix wide lines
by nickm＠torproject.org 13 Jan '17

13 Jan '17

commit ac75c33991ac3ba4180d7a20971f35482da2d425 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jan 13 16:35:35 2017 -0500 fix wide lines --- src/test/test_hs_intropoint.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/test_hs_intropoint.c b/src/test/test_hs_intropoint.c index b4a81ea..dbd884c 100644 --- a/src/test/test_hs_intropoint.c +++ b/src/test/test_hs_intropoint.c @@ -165,7 +165,7 @@ test_establish_intro_wrong_mac(void *arg) /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we attempt to parse it. */ establish_intro_cell = generate_establish_intro_cell(circuit_key_material, - sizeof(circuit_key_material)); + sizeof(circuit_key_material)); tt_assert(establish_intro_cell); /* Mangle one byte of the MAC. */ uint8_t *handshake_ptr = @@ -236,7 +236,7 @@ test_establish_intro_wrong_auth_key_len(void *arg) /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we attempt to parse it. */ establish_intro_cell = generate_establish_intro_cell(circuit_key_material, - sizeof(circuit_key_material)); + sizeof(circuit_key_material)); tt_assert(establish_intro_cell); /* Mangle the auth key length. */ hs_cell_establish_intro_set_auth_key_len(establish_intro_cell, @@ -281,7 +281,7 @@ test_establish_intro_wrong_sig_len(void *arg) /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we attempt to parse it. */ establish_intro_cell = generate_establish_intro_cell(circuit_key_material, - sizeof(circuit_key_material)); + sizeof(circuit_key_material)); tt_assert(establish_intro_cell); /* Mangle the signature length. */ hs_cell_establish_intro_set_sig_len(establish_intro_cell, bad_sig_len);

1 0

[tor/master] Remove trailing whitespace
by nickm＠torproject.org 13 Jan '17

13 Jan '17

commit c93428f457273189d8f5ea3ff3e4f1d7fe6ba6d2 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jan 13 12:31:06 2017 -0500 Remove trailing whitespace --- doc/HACKING/HelpfulTools.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/HACKING/HelpfulTools.md b/doc/HACKING/HelpfulTools.md index 8e9051d..67481ac 100644 --- a/doc/HACKING/HelpfulTools.md +++ b/doc/HACKING/HelpfulTools.md @@ -173,25 +173,25 @@ Here are some basic instructions * `opcontrol --dump;` * `opreport -l that_dir/*` - Profit - + Profiling Tor with perf ----------------------- This works with a running Tor, and requires root. - + 1. Decide how long you want to profile for. Start with (say) 30 seconds. If that works, try again with longer times. 2. Find the PID of your running tor process. 3. Run `perf record --call-graph dwarf -p <PID> sleep <SECONDS>` - + (You may need to do this as root.) - + You might need to add `-e cpu-clock` as an option to the perf record line above, if you are on an older CPU without access to hardware profiling events, or in a VM, or something. - + 4. Now you have a perf.data file. Have a look at it with `perf report --no-children --sort symbol,dso` or `perf report --no-children --sort symbol,dso --stdio --header`. How does it look? @@ -220,7 +220,7 @@ performance! See the gperftools manual for more info, but basically: 2. Run `env CPUPROFILE=/tmp/profile src/or/tor -f <path/torrc>`. The profile file is not written to until Tor finishes execuction. - + 3. Run `pprof src/or/tor /tm/profile` to start the REPL. Generating and analyzing a callgraph

1 0

[tor/master] improved doc/HelpfulTools.md on profiling Tor
by nickm＠torproject.org 13 Jan '17

13 Jan '17

commit f634499044059e863173858793c717aa0634c1ae Author: Pierre-Antoine Rault <par(a)rigelk.eu> Date: Fri Jan 13 17:01:49 2017 +0100 improved doc/HelpfulTools.md on profiling Tor (Based on join work at https://pad.riseup.net/p/profiling-tor) --- doc/HACKING/HelpfulTools.md | 55 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/doc/HACKING/HelpfulTools.md b/doc/HACKING/HelpfulTools.md index a7f36e6..8e9051d 100644 --- a/doc/HACKING/HelpfulTools.md +++ b/doc/HACKING/HelpfulTools.md @@ -142,6 +142,12 @@ run `make test-network`. We also have scripts to run integration tests using Stem. To try them, set `STEM_SOURCE_DIR` to your Stem source directory, and run `test-stem`. +Profiling Tor +------------- + +Ongoing notes about Tor profiling can be found at +https://pad.riseup.net/p/profiling-tor + Profiling Tor with oprofile --------------------------- @@ -167,6 +173,55 @@ Here are some basic instructions * `opcontrol --dump;` * `opreport -l that_dir/*` - Profit + +Profiling Tor with perf +----------------------- + +This works with a running Tor, and requires root. + +1. Decide how long you want to profile for. Start with (say) 30 seconds. If that + works, try again with longer times. + +2. Find the PID of your running tor process. + +3. Run `perf record --call-graph dwarf -p <PID> sleep <SECONDS>` + + (You may need to do this as root.) + + You might need to add `-e cpu-clock` as an option to the perf record line + above, if you are on an older CPU without access to hardware profiling + events, or in a VM, or something. + +4. Now you have a perf.data file. Have a look at it with `perf report + --no-children --sort symbol,dso` or `perf report --no-children --sort + symbol,dso --stdio --header`. How does it look? + +5a. Once you have a nice big perf.data file, you can compress it, encrypt it, + and send it to your favorite Tor developers. + +5b. Or maybe you'd rather not send a nice big perf.data file. Who knows what's + in that!? It's kinda scary. To generate a less scary file, you can use `perf + report -g > <FILENAME>.out`. Then you can compress that and put it somewhere + public. + +Profiling Tor with gperftools aka Google-performance-tools +---------------------------------------------------------- + +This should work on nearly any unixy system. It doesn't seem to be compatible +with RunAsDaemon though. + +Beforehand, install google-perftools. + +1. You need to rebuild Tor, hack the linking steps to add `-lprofiler` to the + libs. You can do this by adding `LIBS=-lprofiler` when you call `./configure`. + +Now you can run Tor with profiling enabled, and use the pprof utility to look at +performance! See the gperftools manual for more info, but basically: + +2. Run `env CPUPROFILE=/tmp/profile src/or/tor -f <path/torrc>`. The profile file + is not written to until Tor finishes execuction. + +3. Run `pprof src/or/tor /tm/profile` to start the REPL. Generating and analyzing a callgraph ------------------------------------

1 0

[tor/master] hs: Log if service can't connect to application
by nickm＠torproject.org 13 Jan '17

13 Jan '17

commit fb8dad5cebe08e7cef1c026f8c878a9d2ab2dcae Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Jan 10 08:35:15 2017 -0500 hs: Log if service can't connect to application In order to help an HS operator knowing if the application configured behind it is not working properly, add a log at warning level for the connection refused or timeout case. This log will only be printed if a client connection fails and is rate limited. Closes #21019 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/connection_edge.c | 32 +++++++++++++++++++++++++++++++- src/or/connection_edge.h | 2 +- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 415f9e4..82d5d3c 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -329,6 +329,33 @@ relay_send_end_cell_from_edge(streamid_t stream_id, circuit_t *circ, payload, 1, cpath_layer); } +/* If the connection conn is attempting to connect to an external + * destination that is an hidden service and the reason is a connection + * refused or timeout, log it so the operator can take appropriate actions. + * The log statement is a rate limited warning. */ +static void +warn_if_hs_unreachable(const edge_connection_t *conn, uint8_t reason) +{ + tor_assert(conn); + + if (conn->base_.type == CONN_TYPE_EXIT && + connection_edge_is_rendezvous_stream(conn) && + (reason == END_STREAM_REASON_CONNECTREFUSED || + reason == END_STREAM_REASON_TIMEOUT)) { +#define WARN_FAILED_HS_CONNECTION 300 + static ratelim_t warn_limit = RATELIM_INIT(WARN_FAILED_HS_CONNECTION); + char *m; + if ((m = rate_limit_log(&warn_limit, approx_time()))) { + log_warn(LD_EDGE, "Onion service connection to %s failed (%s)", + (conn->base_.socket_family == AF_UNIX) ? + safe_str(conn->base_.address) : + safe_str(fmt_addrport(&conn->base_.addr, conn->base_.port)), + stream_end_reason_to_string(reason)); + tor_free(m); + } + } +} + /** Send a relay end cell from stream conn down conn's circuit, and * remember that we've done so. If this is not a client connection, set the * relay end cell's reason for closing as reason. @@ -386,6 +413,9 @@ connection_edge_end(edge_connection_t *conn, uint8_t reason) conn->base_.s); connection_edge_send_command(conn, RELAY_COMMAND_END, payload, payload_len); + /* We'll log warn if the connection was an hidden service and couldn't be + * made because the service wasn't available. */ + warn_if_hs_unreachable(conn, control_reason); } else { log_debug(LD_EDGE,"No circ to send end on conn " "(fd "TOR_SOCKET_T_FORMAT").", @@ -3521,7 +3551,7 @@ connection_exit_connect_dir(edge_connection_t *exitconn) * it is a general stream. */ int -connection_edge_is_rendezvous_stream(edge_connection_t *conn) +connection_edge_is_rendezvous_stream(const edge_connection_t *conn) { tor_assert(conn); if (conn->rend_data) diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h index 5dfc8af..61b5752 100644 --- a/src/or/connection_edge.h +++ b/src/or/connection_edge.h @@ -60,7 +60,7 @@ void connection_ap_handshake_socks_resolved_addr(entry_connection_t *conn, int connection_exit_begin_conn(cell_t *cell, circuit_t *circ); int connection_exit_begin_resolve(cell_t *cell, or_circuit_t *circ); void connection_exit_connect(edge_connection_t *conn); -int connection_edge_is_rendezvous_stream(edge_connection_t *conn); +int connection_edge_is_rendezvous_stream(const edge_connection_t *conn); int connection_ap_can_use_exit(const entry_connection_t *conn, const node_t *exit); void connection_ap_expire_beginning(void);

1 0