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

[tor/master] test: Move some test code to test helpers.
by nickm＠torproject.org 07 Jul '17

07 Jul '17

commit dc3a2037f54bcef14b7021b847edd91d36bf4514 Author: George Kadianakis <desnacked(a)riseup.net> Date: Tue May 2 16:35:22 2017 +0300 test: Move some test code to test helpers. Move code to create connection streams and rend_data structures to test_helpers so that we can use them from the e2e rendezvous circuit unittests. --- src/test/include.am | 1 + src/test/rend_test_helpers.c | 16 ++++++++ src/test/rend_test_helpers.h | 1 + src/test/test_connection.c | 93 ++---------------------------------------- src/test/test_connection.h | 13 ++++++ src/test/test_helpers.c | 96 ++++++++++++++++++++++++++++++++++++++++++++ src/test/test_helpers.h | 3 ++ src/test/test_rendcache.c | 16 -------- 8 files changed, 133 insertions(+), 106 deletions(-) diff --git a/src/test/include.am b/src/test/include.am index 29ba1ce..91feae4 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -270,6 +270,7 @@ noinst_HEADERS+= \ src/test/test.h \ src/test/test_helpers.h \ src/test/test_dir_common.h \ + src/test/test_connection.h \ src/test/test_descriptors.inc \ src/test/example_extrainfo.inc \ src/test/failing_routerdescs.inc \ diff --git a/src/test/rend_test_helpers.c b/src/test/rend_test_helpers.c index f788004..095bfec 100644 --- a/src/test/rend_test_helpers.c +++ b/src/test/rend_test_helpers.c @@ -71,3 +71,19 @@ create_descriptor(rend_service_descriptor_t **generated, char **service_id, crypto_pk_free(pk2); } +rend_data_t * +mock_rend_data(const char *onion_address) +{ + rend_data_v2_t *v2_data = tor_malloc_zero(sizeof(*v2_data)); + rend_data_t *rend_query = &v2_data->base_; + rend_query->version = 2; + + strlcpy(v2_data->onion_address, onion_address, + sizeof(v2_data->onion_address)); + v2_data->auth_type = REND_NO_AUTH; + rend_query->hsdirs_fp = smartlist_new(); + smartlist_add(rend_query->hsdirs_fp, tor_memdup("aaaaaaaaaaaaaaaaaaaaaaaa", + DIGEST_LEN)); + return rend_query; +} + diff --git a/src/test/rend_test_helpers.h b/src/test/rend_test_helpers.h index 486adba..6f0ef11 100644 --- a/src/test/rend_test_helpers.h +++ b/src/test/rend_test_helpers.h @@ -10,6 +10,7 @@ void generate_desc(int time_diff, rend_encoded_v2_service_descriptor_t **desc, char **service_id, int intro_points); void create_descriptor(rend_service_descriptor_t **generated, char **service_id, int intro_points); +rend_data_t *mock_rend_data(const char *onion_address); #endif diff --git a/src/test/test_connection.c b/src/test/test_connection.c index 7e5193b..f252902 100644 --- a/src/test/test_connection.c +++ b/src/test/test_connection.c @@ -17,9 +17,8 @@ #include "rendcache.h" #include "directory.h" -static void test_conn_lookup_addr_helper(const char *address, - int family, - tor_addr_t *addr); +#include "test_connection.h" +#include "test_helpers.h" static void * test_conn_get_basic_setup(const struct testcase_t *tc); static int test_conn_get_basic_teardown(const struct testcase_t *tc, @@ -62,48 +61,7 @@ static int test_conn_get_rsrc_teardown(const struct testcase_t *tc, #define TEST_CONN_UNATTACHED_STATE (AP_CONN_STATE_CIRCUIT_WAIT) #define TEST_CONN_ATTACHED_STATE (AP_CONN_STATE_CONNECT_WAIT) -#define TEST_CONN_FD_INIT 50 -static int mock_connection_connect_sockaddr_called = 0; -static int fake_socket_number = TEST_CONN_FD_INIT; - -static int -mock_connection_connect_sockaddr(connection_t *conn, - const struct sockaddr *sa, - socklen_t sa_len, - const struct sockaddr *bindaddr, - socklen_t bindaddr_len, - int *socket_error) -{ - (void)sa_len; - (void)bindaddr; - (void)bindaddr_len; - - tor_assert(conn); - tor_assert(sa); - tor_assert(socket_error); - - mock_connection_connect_sockaddr_called++; - - conn->s = fake_socket_number++; - tt_assert(SOCKET_OK(conn->s)); - /* We really should call tor_libevent_initialize() here. Because we don't, - * we are relying on other parts of the code not checking if the_event_base - * (and therefore event->ev_base) is NULL. */ - tt_assert(connection_add_connecting(conn) == 0); - - done: - /* Fake "connected" status */ - return 1; -} - -static int -fake_close_socket(evutil_socket_t sock) -{ - (void)sock; - return 0; -} - -static void +void test_conn_lookup_addr_helper(const char *address, int family, tor_addr_t *addr) { int rv = 0; @@ -122,51 +80,6 @@ test_conn_lookup_addr_helper(const char *address, int family, tor_addr_t *addr) tor_addr_make_null(addr, TEST_CONN_FAMILY); } -static connection_t * -test_conn_get_connection(uint8_t state, uint8_t type, uint8_t purpose) -{ - connection_t *conn = NULL; - tor_addr_t addr; - int socket_err = 0; - int in_progress = 0; - - MOCK(connection_connect_sockaddr, - mock_connection_connect_sockaddr); - MOCK(tor_close_socket, fake_close_socket); - - init_connection_lists(); - - conn = connection_new(type, TEST_CONN_FAMILY); - tt_assert(conn); - - test_conn_lookup_addr_helper(TEST_CONN_ADDRESS, TEST_CONN_FAMILY, &addr); - tt_assert(!tor_addr_is_null(&addr)); - - tor_addr_copy_tight(&conn->addr, &addr); - conn->port = TEST_CONN_PORT; - mock_connection_connect_sockaddr_called = 0; - in_progress = connection_connect(conn, TEST_CONN_ADDRESS_PORT, &addr, - TEST_CONN_PORT, &socket_err); - tt_assert(mock_connection_connect_sockaddr_called == 1); - tt_assert(!socket_err); - tt_assert(in_progress == 0 || in_progress == 1); - - /* fake some of the attributes so the connection looks OK */ - conn->state = state; - conn->purpose = purpose; - assert_connection_ok(conn, time(NULL)); - - UNMOCK(connection_connect_sockaddr); - UNMOCK(tor_close_socket); - return conn; - - /* On failure */ - done: - UNMOCK(connection_connect_sockaddr); - UNMOCK(tor_close_socket); - return NULL; -} - static void * test_conn_get_basic_setup(const struct testcase_t *tc) { diff --git a/src/test/test_connection.h b/src/test/test_connection.h new file mode 100644 index 0000000..392783b --- /dev/null +++ b/src/test/test_connection.h @@ -0,0 +1,13 @@ +/* Copyright (c) 2014-2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** Some constants used by test_connection and helpers */ +#define TEST_CONN_FAMILY (AF_INET) +#define TEST_CONN_ADDRESS "127.0.0.1" +#define TEST_CONN_PORT (12345) +#define TEST_CONN_ADDRESS_PORT "127.0.0.1:12345" +#define TEST_CONN_FD_INIT 50 + +void test_conn_lookup_addr_helper(const char *address, + int family, tor_addr_t *addr); + diff --git a/src/test/test_helpers.c b/src/test/test_helpers.c index 9fada5a..22d9de3 100644 --- a/src/test/test_helpers.c +++ b/src/test/test_helpers.c @@ -7,9 +7,14 @@ */ #define ROUTERLIST_PRIVATE +#define CONNECTION_PRIVATE +#define MAIN_PRIVATE + #include "orconfig.h" #include "or.h" +#include "connection.h" +#include "main.h" #include "relay.h" #include "routerlist.h" #include "nodelist.h" @@ -17,6 +22,7 @@ #include "test.h" #include "test_helpers.h" +#include "test_connection.h" #ifdef HAVE_CFLAG_WOVERLENGTH_STRINGS DISABLE_GCC_WARNING(overlength-strings) @@ -143,3 +149,93 @@ mock_tor_addr_lookup__fail_on_bad_addrs(const char *name, return tor_addr_lookup__real(name, family, out); } +/*********** Helper funcs for making new connections/streams *****************/ + +/* Helper for test_conn_get_connection() */ +static int +fake_close_socket(evutil_socket_t sock) +{ + (void)sock; + return 0; +} + +static int mock_connection_connect_sockaddr_called = 0; +static int fake_socket_number = TEST_CONN_FD_INIT; + +/* Helper for test_conn_get_connection() */ +static int +mock_connection_connect_sockaddr(connection_t *conn, + const struct sockaddr *sa, + socklen_t sa_len, + const struct sockaddr *bindaddr, + socklen_t bindaddr_len, + int *socket_error) +{ + (void)sa_len; + (void)bindaddr; + (void)bindaddr_len; + + tor_assert(conn); + tor_assert(sa); + tor_assert(socket_error); + + mock_connection_connect_sockaddr_called++; + + conn->s = fake_socket_number++; + tt_assert(SOCKET_OK(conn->s)); + /* We really should call tor_libevent_initialize() here. Because we don't, + * we are relying on other parts of the code not checking if the_event_base + * (and therefore event->ev_base) is NULL. */ + tt_assert(connection_add_connecting(conn) == 0); + + done: + /* Fake "connected" status */ + return 1; +} + +/** Create and return a new connection/stream */ +connection_t * +test_conn_get_connection(uint8_t state, uint8_t type, uint8_t purpose) +{ + connection_t *conn = NULL; + tor_addr_t addr; + int socket_err = 0; + int in_progress = 0; + + MOCK(connection_connect_sockaddr, + mock_connection_connect_sockaddr); + MOCK(tor_close_socket, fake_close_socket); + + init_connection_lists(); + + conn = connection_new(type, TEST_CONN_FAMILY); + tt_assert(conn); + + test_conn_lookup_addr_helper(TEST_CONN_ADDRESS, TEST_CONN_FAMILY, &addr); + tt_assert(!tor_addr_is_null(&addr)); + + tor_addr_copy_tight(&conn->addr, &addr); + conn->port = TEST_CONN_PORT; + mock_connection_connect_sockaddr_called = 0; + in_progress = connection_connect(conn, TEST_CONN_ADDRESS_PORT, &addr, + TEST_CONN_PORT, &socket_err); + tt_assert(mock_connection_connect_sockaddr_called == 1); + tt_assert(!socket_err); + tt_assert(in_progress == 0 || in_progress == 1); + + /* fake some of the attributes so the connection looks OK */ + conn->state = state; + conn->purpose = purpose; + assert_connection_ok(conn, time(NULL)); + + UNMOCK(connection_connect_sockaddr); + UNMOCK(tor_close_socket); + return conn; + + /* On failure */ + done: + UNMOCK(connection_connect_sockaddr); + UNMOCK(tor_close_socket); + return NULL; +} + diff --git a/src/test/test_helpers.h b/src/test/test_helpers.h index 4621631..aa7aff1 100644 --- a/src/test/test_helpers.h +++ b/src/test/test_helpers.h @@ -20,6 +20,9 @@ void connection_write_to_buf_mock(const char *string, size_t len, int mock_tor_addr_lookup__fail_on_bad_addrs(const char *name, uint16_t family, tor_addr_t *out); +connection_t *test_conn_get_connection(uint8_t state, + uint8_t type, uint8_t purpose); + extern const char TEST_DESCRIPTORS[]; #endif diff --git a/src/test/test_rendcache.c b/src/test/test_rendcache.c index feba8f6..c1c1789 100644 --- a/src/test/test_rendcache.c +++ b/src/test/test_rendcache.c @@ -21,22 +21,6 @@ static const int TIME_IN_THE_PAST = -(REND_CACHE_MAX_AGE + \ REND_CACHE_MAX_SKEW + 60); static const int TIME_IN_THE_FUTURE = REND_CACHE_MAX_SKEW + 60; -static rend_data_t * -mock_rend_data(const char *onion_address) -{ - rend_data_v2_t *v2_data = tor_malloc_zero(sizeof(*v2_data)); - rend_data_t *rend_query = &v2_data->base_; - rend_query->version = 2; - - strlcpy(v2_data->onion_address, onion_address, - sizeof(v2_data->onion_address)); - v2_data->auth_type = REND_NO_AUTH; - rend_query->hsdirs_fp = smartlist_new(); - smartlist_add(rend_query->hsdirs_fp, tor_memdup("aaaaaaaaaaaaaaaaaaaaaaaa", - DIGEST_LEN)); - return rend_query; -} - static void test_rend_cache_lookup_entry(void *data) {

1 0

[tor/master] Explicit length checks in circuit_init_cpath_crypto().
by nickm＠torproject.org 07 Jul '17

07 Jul '17

commit c4d17faf81d8cfe4cf943ba11be03413c58f4d44 Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Jul 6 14:15:23 2017 +0300 Explicit length checks in circuit_init_cpath_crypto(). --- src/or/circuitbuild.c | 23 ++++++++++++++++++----- src/or/circuitbuild.h | 5 +++-- src/or/command.c | 3 ++- src/or/cpuworker.c | 2 +- src/or/hs_circuit.c | 6 ++++-- src/or/rendservice.c | 4 +++- 6 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index fdcf3e7..0dd33fc 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -46,6 +46,7 @@ #include "crypto.h" #include "directory.h" #include "entrynodes.h" +#include "hs_ntor.h" #include "main.h" #include "microdesc.h" #include "networkstatus.h" @@ -1331,7 +1332,7 @@ circuit_extend(cell_t *cell, circuit_t *circ) * service circuits and key_data must be at least * HS_NTOR_KEY_EXPANSION_KDF_OUT_LEN bytes in length. * - * If is_hs_v3 is not set, key_data must contain CPATH_KEY_MATERIAL + * If is_hs_v3 is not set, key_data must contain CPATH_KEY_MATERIAL_LEN * bytes, which are used as follows: * - 20 to initialize f_digest * - 20 to initialize b_digest @@ -1339,9 +1340,12 @@ circuit_extend(cell_t *cell, circuit_t *circ) * - 16 to key b_crypto * * (If 'reverse' is true, then f_XX and b_XX are swapped.) + * + * Return 0 if init was successful, else -1 if it failed. */ int -circuit_init_cpath_crypto(crypt_path_t *cpath, const char *key_data, +circuit_init_cpath_crypto(crypt_path_t *cpath, + const char *key_data, size_t key_data_len, int reverse, int is_hs_v3) { crypto_digest_t *tmp_digest; @@ -1354,6 +1358,13 @@ circuit_init_cpath_crypto(crypt_path_t *cpath, const char *key_data, tor_assert(!(cpath->f_crypto || cpath->b_crypto || cpath->f_digest || cpath->b_digest)); + /* Basic key size validation */ + if (is_hs_v3 && BUG(key_data_len != HS_NTOR_KEY_EXPANSION_KDF_OUT_LEN)) { + return -1; + } else if (!is_hs_v3 && BUG(key_data_len != CPATH_KEY_MATERIAL_LEN)) { + return -1; + } + /* If we are using this cpath for next gen onion services use SHA3-256, otherwise use good ol' SHA1 */ if (is_hs_v3) { @@ -1450,7 +1461,7 @@ circuit_finish_handshake(origin_circuit_t *circ, onion_handshake_state_release(&hop->handshake_state); - if (circuit_init_cpath_crypto(hop, keys, 0, 0)<0) { + if (circuit_init_cpath_crypto(hop, keys, sizeof(keys), 0, 0)<0) { return -END_CIRC_REASON_TORPROTOCOL; } @@ -1517,12 +1528,14 @@ circuit_truncated(origin_circuit_t *circ, crypt_path_t *layer, int reason) int onionskin_answer(or_circuit_t *circ, const created_cell_t *created_cell, - const char *keys, + const char *keys, size_t keys_len, const uint8_t *rend_circ_nonce) { cell_t cell; crypt_path_t *tmp_cpath; + tor_assert(keys_len == CPATH_KEY_MATERIAL_LEN); + if (created_cell_format(&cell, created_cell) < 0) { log_warn(LD_BUG,"couldn't format created cell (type=%d, len=%d)", (int)created_cell->cell_type, (int)created_cell->handshake_len); @@ -1538,7 +1551,7 @@ onionskin_answer(or_circuit_t *circ, log_debug(LD_CIRC,"init digest forward 0x%.8x, backward 0x%.8x.", (unsigned int)get_uint32(keys), (unsigned int)get_uint32(keys+20)); - if (circuit_init_cpath_crypto(tmp_cpath, keys, 0, 0)<0) { + if (circuit_init_cpath_crypto(tmp_cpath, keys, keys_len, 0, 0)<0) { log_warn(LD_BUG,"Circuit initialization failed"); tor_free(tmp_cpath); return -1; diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h index 6910b3a..62a6367 100644 --- a/src/or/circuitbuild.h +++ b/src/or/circuitbuild.h @@ -31,7 +31,8 @@ int circuit_timeout_want_to_count_circ(origin_circuit_t *circ); int circuit_send_next_onion_skin(origin_circuit_t *circ); void circuit_note_clock_jumped(int seconds_elapsed); int circuit_extend(cell_t *cell, circuit_t *circ); -int circuit_init_cpath_crypto(crypt_path_t *cpath, const char *key_data, +int circuit_init_cpath_crypto(crypt_path_t *cpath, + const char *key_data, size_t key_data_len, int reverse, int is_hs_v3); struct created_cell_t; int circuit_finish_handshake(origin_circuit_t *circ, @@ -40,7 +41,7 @@ int circuit_truncated(origin_circuit_t *circ, crypt_path_t *layer, int reason); int onionskin_answer(or_circuit_t *circ, const struct created_cell_t *created_cell, - const char *keys, + const char *keys, size_t keys_len, const uint8_t *rend_circ_nonce); MOCK_DECL(int, circuit_all_predicted_ports_handled, (time_t now, int *need_uptime, diff --git a/src/or/command.c b/src/or/command.c index c667cbb..2c82984 100644 --- a/src/or/command.c +++ b/src/or/command.c @@ -381,7 +381,8 @@ command_process_create_cell(cell_t *cell, channel_t *chan) created_cell.handshake_len = len; if (onionskin_answer(circ, &created_cell, - (const char *)keys, rend_circ_nonce)<0) { + (const char *)keys, sizeof(keys), + rend_circ_nonce)<0) { log_warn(LD_OR,"Failed to reply to CREATE_FAST cell. Closing."); circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL); return; diff --git a/src/or/cpuworker.c b/src/or/cpuworker.c index 06d45f9..f786d79 100644 --- a/src/or/cpuworker.c +++ b/src/or/cpuworker.c @@ -372,7 +372,7 @@ cpuworker_onion_handshake_replyfn(void *work_) if (onionskin_answer(circ, &rpl.created_cell, - (const char*)rpl.keys, + (const char*)rpl.keys, sizeof(rpl.keys), rpl.rend_auth_material) < 0) { log_warn(LD_OR,"onionskin_answer failed. Closing."); circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL); diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index 1729080..42c5dcb 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -60,7 +60,7 @@ create_rend_cpath(const uint8_t *ntor_key_seed, int is_service_side) cpath = tor_malloc_zero(sizeof(crypt_path_t)); cpath->magic = CRYPT_PATH_MAGIC; - if (circuit_init_cpath_crypto(cpath, (char*)keys, + if (circuit_init_cpath_crypto(cpath, (char*)keys, sizeof(keys), is_service_side, 1) < 0) { tor_free(cpath); goto err; @@ -96,7 +96,9 @@ create_rend_cpath_legacy(origin_circuit_t *circ, const uint8_t *rend_cell_body) goto err; } /* ... and set up cpath. */ - if (circuit_init_cpath_crypto(hop, keys+DIGEST_LEN, 0, 0)<0) + if (circuit_init_cpath_crypto(hop, + keys+DIGEST_LEN, sizeof(keys)-DIGEST_LEN, + 0, 0) < 0) goto err; /* Check whether the digest is right... */ diff --git a/src/or/rendservice.c b/src/or/rendservice.c index d389a87..b8e704e 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -2195,7 +2195,9 @@ rend_service_receive_introduction(origin_circuit_t *circuit, cpath->rend_dh_handshake_state = dh; dh = NULL; - if (circuit_init_cpath_crypto(cpath,keys+DIGEST_LEN,1, 0)<0) + if (circuit_init_cpath_crypto(cpath, + keys+DIGEST_LEN, sizeof(keys)-DIGEST_LEN, + 1, 0)<0) goto err; memcpy(cpath->rend_circ_nonce, keys, DIGEST_LEN);

1 0

[tor/master] Improve docs based on Nick's review.
by nickm＠torproject.org 07 Jul '17

07 Jul '17

commit 91da032e9cc264d88802d0bfc072b0b4117c5f33 Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Jul 6 13:37:55 2017 +0300 Improve docs based on Nick's review. --- src/or/hs_circuit.c | 3 +-- src/or/or.h | 10 ++++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index f8c95f7..02c9afa 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -135,8 +135,7 @@ finalize_rend_circuit(origin_circuit_t *circ, crypt_path_t *hop, /* All is well. Extend the circuit. */ hop->state = CPATH_STATE_OPEN; - /* Set the windows to default. These are the windows that the client thinks - * the service has. */ + /* Set the windows to default. */ hop->package_window = circuit_initial_package_window(); hop->deliver_window = CIRCWINDOW_START; diff --git a/src/or/or.h b/src/or/or.h index db77d08..dcda7e2 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1638,8 +1638,9 @@ typedef struct edge_connection_t { * an exit)? */ rend_data_t *rend_data; - /* Hidden service connection identifier that is which service is being - * queried? */ + /* Hidden service connection identifier for edge connections. Used by the HS + * client-side code to identify client SOCKS connections and by the + * service-side code to match HS circuits with their streams. */ hs_ident_edge_conn_t *hs_ident; uint32_t address_ttl; /**< TTL for address-to-addr mapping on exit @@ -1792,8 +1793,9 @@ typedef struct dir_connection_t { /** What rendezvous service are we querying for? */ rend_data_t *rend_data; - /* Hidden service connection identifier that is which service is being - * queried? */ + /* Hidden service connection identifier for dir connections: Used by HS + client-side code to fetch HS descriptors, and by the service-side code to + upload descriptors. */ hs_ident_dir_conn_t *hs_ident; /** If this is a one-hop connection, tracks the state of the directory guard

1 0

[tor/master] Rename rend_circuit_validate_purpose() based on Nick's review.
by nickm＠torproject.org 07 Jul '17

07 Jul '17

commit b490ae68c7d3a14766db898b48aeb5cee8924de1 Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Jul 6 13:51:06 2017 +0300 Rename rend_circuit_validate_purpose() based on Nick's review. --- src/or/hs_circuit.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index 2d186e9..213f5cb 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -16,16 +16,16 @@ #include "hs_ntor.h" /* A circuit is about to become an e2e rendezvous circuit. Check - * circ_purpose and ensure that it's properly set. Return 0 if circuit - * purpose is properly set, otherwise return -1. */ + * circ_purpose and ensure that it's properly set. Return true iff + * circuit purpose is properly set, otherwise return false. */ static int -rend_circuit_validate_purpose(unsigned int circ_purpose, int is_service_side) +circuit_purpose_is_correct_for_rend(unsigned int circ_purpose, int is_service_side) { if (is_service_side) { if (circ_purpose != CIRCUIT_PURPOSE_S_CONNECT_REND) { log_fn(LOG_PROTOCOL_WARN, LD_GENERAL, "HS e2e circuit setup with wrong purpose(%d)", circ_purpose); - return -1; + return 0; } } @@ -34,11 +34,11 @@ rend_circuit_validate_purpose(unsigned int circ_purpose, int is_service_side) circ_purpose != CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED) { log_fn(LOG_PROTOCOL_WARN, LD_GENERAL, "Client e2e circuit setup with wrong purpose(%d)", circ_purpose); - return -1; + return 0; } } - return 0; + return 1; } /* Create and return a crypt path for the final hop of a v3 prop224 rendezvous @@ -172,8 +172,8 @@ hs_circuit_setup_e2e_rend_circ(origin_circuit_t *circ, const uint8_t *ntor_key_seed, int is_service_side) { - if (BUG(rend_circuit_validate_purpose(TO_CIRCUIT(circ)->purpose, - is_service_side)) < 0) { + if (BUG(!circuit_purpose_is_correct_for_rend(TO_CIRCUIT(circ)->purpose, + is_service_side))) { return -1; } @@ -197,7 +197,9 @@ int hs_circuit_setup_e2e_rend_circ_legacy_client(origin_circuit_t *circ, const uint8_t *rend_cell_body) { - if (BUG(rend_circuit_validate_purpose(TO_CIRCUIT(circ)->purpose, 0)) < 0) { + + if (BUG(!circuit_purpose_is_correct_for_rend( + TO_CIRCUIT(circ)->purpose, 0))) { return -1; }

1 0

[tor/master] test: Add client-side unittests for e2e rend circuits.
by nickm＠torproject.org 07 Jul '17

07 Jul '17

commit 173dd486e3648ff24a7814679ac6e60e1da7a16b Author: George Kadianakis <desnacked(a)riseup.net> Date: Tue May 2 16:36:59 2017 +0300 test: Add client-side unittests for e2e rend circuits. --- src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_helpers.h | 6 +- src/test/test_hs_client.c | 280 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 287 insertions(+), 2 deletions(-) diff --git a/src/test/include.am b/src/test/include.am index 91feae4..e7a2e02 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -115,6 +115,7 @@ src_test_test_SOURCES = \ src/test/test_extorport.c \ src/test/test_hs.c \ src/test/test_hs_service.c \ + src/test/test_hs_client.c \ src/test/test_hs_intropoint.c \ src/test/test_handles.c \ src/test/test_hs_cache.c \ diff --git a/src/test/test.c b/src/test/test.c index 68f5f90..31b3db3 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1215,6 +1215,7 @@ struct testgroup_t testgroups[] = { { "hs_cache/", hs_cache }, { "hs_descriptor/", hs_descriptor }, { "hs_service/", hs_service_tests }, + { "hs_client/", hs_client_tests }, { "hs_intropoint/", hs_intropoint_tests }, { "introduce/", introduce_tests }, { "keypin/", keypin_tests }, diff --git a/src/test/test.h b/src/test/test.h index 6abaf39..4de0da9 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -209,6 +209,7 @@ extern struct testcase_t hs_tests[]; extern struct testcase_t hs_cache[]; extern struct testcase_t hs_descriptor[]; extern struct testcase_t hs_service_tests[]; +extern struct testcase_t hs_client_tests[]; extern struct testcase_t hs_intropoint_tests[]; extern struct testcase_t introduce_tests[]; extern struct testcase_t keypin_tests[]; diff --git a/src/test/test_helpers.h b/src/test/test_helpers.h index aa7aff1..96a4b59 100644 --- a/src/test/test_helpers.h +++ b/src/test/test_helpers.h @@ -1,9 +1,11 @@ -/* Copyright (c) 2014-2017, The Tor Project, Inc. */ +/* Copyright (c) 2017, The Tor Project, Inc. */ /* See LICENSE for licensing information */ #ifndef TOR_TEST_HELPERS_H #define TOR_TEST_HELPERS_H +#include "or.h" + const char *get_yesterday_date_str(void); circuit_t * dummy_origin_circuit_new(int num_cells); @@ -25,5 +27,5 @@ connection_t *test_conn_get_connection(uint8_t state, extern const char TEST_DESCRIPTORS[]; -#endif +#endif /* TOR_TEST_HELPERS_H */ diff --git a/src/test/test_hs_client.c b/src/test/test_hs_client.c new file mode 100644 index 0000000..9e5fe04 --- /dev/null +++ b/src/test/test_hs_client.c @@ -0,0 +1,280 @@ +/* Copyright (c) 2016-2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file test_hs_client.c + * \brief Test prop224 HS client functionality. + */ + +#define CRYPTO_PRIVATE +#define MAIN_PRIVATE +#define TOR_CHANNEL_INTERNAL_ +#define CIRCUITBUILD_PRIVATE +#define CIRCUITLIST_PRIVATE +#define CONNECTION_PRIVATE + +#include "test.h" +#include "test_helpers.h" +#include "log_test_helpers.h" +#include "rend_test_helpers.h" + +#include "config.h" +#include "crypto.h" +#include "channeltls.h" + +#include "hs_circuit.h" +#include "hs_ident.h" +#include "circuitlist.h" +#include "circuitbuild.h" +#include "connection.h" +#include "connection_edge.h" + +static int +mock_connection_ap_handshake_send_begin(entry_connection_t *ap_conn) +{ + (void) ap_conn; + return 0; +} + +/* Test helper function: Setup a circuit and a stream with the same hidden + * service destination, and put them in circ_out and + * conn_out. Make the stream wait for circuits to be established to the + * hidden service. */ +static int +helper_get_circ_and_stream_for_test(origin_circuit_t **circ_out, + connection_t **conn_out, + int is_legacy) +{ + int retval; + channel_tls_t *n_chan=NULL; + rend_data_t *conn_rend_data = NULL; + origin_circuit_t *or_circ = NULL; + connection_t *conn = NULL; + ed25519_public_key_t service_pk; + + /* Make a dummy connection stream and make it wait for our circuit */ + conn = test_conn_get_connection(AP_CONN_STATE_CIRCUIT_WAIT, + CONN_TYPE_AP /* ??? */, + 0); + if (is_legacy) { + /* Legacy: Setup rend_data of stream */ + char service_id[REND_SERVICE_ID_LEN_BASE32+1] = {0}; + TO_EDGE_CONN(conn)->rend_data = mock_rend_data(service_id); + conn_rend_data = TO_EDGE_CONN(conn)->rend_data; + } else { + /* prop224: Setup hs conn identifier on the stream */ + ed25519_secret_key_t sk; + tt_int_op(0, OP_EQ, ed25519_secret_key_generate(&sk, 0)); + tt_int_op(0, OP_EQ, ed25519_public_key_generate(&service_pk, &sk)); + + /* Setup hs_conn_identifier of stream */ + TO_EDGE_CONN(conn)->hs_ident = hs_ident_edge_conn_new(&service_pk); + } + + /* Make it wait for circuit */ + connection_ap_mark_as_pending_circuit(TO_ENTRY_CONN(conn)); + + /* This is needed to silence a BUG warning from + connection_edge_update_circuit_isolation() */ + TO_ENTRY_CONN(conn)->original_dest_address = + tor_strdup(TO_ENTRY_CONN(conn)->socks_request->address); + + /****************************************************/ + + /* Now make dummy circuit */ + or_circ = origin_circuit_new(); + + or_circ->base_.purpose = CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED; + + or_circ->build_state = tor_malloc_zero(sizeof(cpath_build_state_t)); + or_circ->build_state->is_internal = 1; + + if (is_legacy) { + /* Legacy: Setup rend data and final cpath */ + or_circ->build_state->pending_final_cpath = + tor_malloc_zero(sizeof(crypt_path_t)); + or_circ->build_state->pending_final_cpath->magic = CRYPT_PATH_MAGIC; + or_circ->build_state->pending_final_cpath->rend_dh_handshake_state = + crypto_dh_new(DH_TYPE_REND); + tt_assert( + or_circ->build_state->pending_final_cpath->rend_dh_handshake_state); + retval = crypto_dh_generate_public( + or_circ->build_state->pending_final_cpath->rend_dh_handshake_state); + tt_int_op(retval, ==, 0); + or_circ->rend_data = rend_data_dup(conn_rend_data); + } else { + /* prop224: Setup hs ident on the circuit */ + or_circ->hs_ident = hs_ident_circuit_new(&service_pk, + HS_IDENT_CIRCUIT_RENDEZVOUS); + } + + TO_CIRCUIT(or_circ)->state = CIRCUIT_STATE_OPEN; + + /* fake n_chan */ + n_chan = tor_malloc_zero(sizeof(channel_tls_t)); + n_chan->base_.global_identifier = 1; + or_circ->base_.n_chan = &(n_chan->base_); + + *circ_out = or_circ; + *conn_out = conn; + + return 0; + + done: + /* something failed */ + return -1; +} + +/* Test: Ensure that setting up legacy e2e rendezvous circuits works + * correctly. */ +static void +test_e2e_rend_circuit_setup_legacy(void *arg) +{ + int retval; + origin_circuit_t *or_circ = NULL; + connection_t *conn = NULL; + + (void) arg; + + /** In this test we create a v2 legacy HS stream and a circuit with the same + * hidden service destination. We make the stream wait for circuits to be + * established to the hidden service, and then we complete the circuit using + * the hs_circuit_setup_e2e_rend_circ_legacy_client() function. We then + * check that the end-to-end cpath was setup correctly and that the stream + * was attached to the circuit as expected. */ + + MOCK(connection_ap_handshake_send_begin, + mock_connection_ap_handshake_send_begin); + + /* Setup */ + retval = helper_get_circ_and_stream_for_test( &or_circ, &conn, 1); + tt_int_op(retval, OP_EQ, 0); + tt_assert(or_circ); + tt_assert(conn); + + /* Check number of hops */ + retval = cpath_get_n_hops(&or_circ->cpath); + tt_int_op(retval, ==, 0); + + /* Check that our stream is not attached on any circuits */ + tt_assert(!TO_EDGE_CONN(conn)->on_circuit); + + /********************************************** */ + + /* Make a good RENDEZVOUS1 cell body because it needs to pass key exchange + * digest verification... */ + uint8_t rend_cell_body[DH_KEY_LEN+DIGEST_LEN] = {2}; + { + char keys[DIGEST_LEN+CPATH_KEY_MATERIAL_LEN]; + crypto_dh_t *dh_state = + or_circ->build_state->pending_final_cpath->rend_dh_handshake_state; + /* compute and overwrite digest of cell body with the right value */ + retval = crypto_dh_compute_secret(LOG_PROTOCOL_WARN, dh_state, + (char*)rend_cell_body, DH_KEY_LEN, + keys, DIGEST_LEN+CPATH_KEY_MATERIAL_LEN); + tt_int_op(retval, OP_GT, 0); + memcpy(rend_cell_body+DH_KEY_LEN, keys, DIGEST_LEN); + } + + /* Setup the circuit */ + retval = hs_circuit_setup_e2e_rend_circ_legacy_client(or_circ, + rend_cell_body); + tt_int_op(retval, OP_EQ, 0); + + /**********************************************/ + + /* See that a hop was added to the circuit's cpath */ + retval = cpath_get_n_hops(&or_circ->cpath); + tt_int_op(retval, OP_EQ, 1); + + /* Check the digest algo */ + tt_int_op(or_circ->cpath->f_digest->algorithm, OP_EQ, DIGEST_SHA1); + tt_int_op(or_circ->cpath->b_digest->algorithm, OP_EQ, DIGEST_SHA1); + tt_assert(or_circ->cpath->f_crypto); + tt_assert(or_circ->cpath->b_crypto); + + /* Ensure that circ purpose was changed */ + tt_int_op(or_circ->base_.purpose, OP_EQ, CIRCUIT_PURPOSE_C_REND_JOINED); + + /* Test that stream got attached */ + tt_ptr_op(TO_EDGE_CONN(conn)->on_circuit, OP_EQ, TO_CIRCUIT(or_circ)); + + done: + connection_free_(conn); + tor_free(TO_CIRCUIT(or_circ)->n_chan); + circuit_free(TO_CIRCUIT(or_circ)); +} + +/* Test: Ensure that setting up v3 rendezvous circuits works correctly. */ +static void +test_e2e_rend_circuit_setup(void *arg) +{ + uint8_t ntor_key_seed[DIGEST256_LEN] = {0}; + origin_circuit_t *or_circ; + int retval; + connection_t *conn = NULL; + + (void) arg; + + /** In this test we create a prop224 v3 HS stream and a circuit with the same + * hidden service destination. We make the stream wait for circuits to be + * established to the hidden service, and then we complete the circuit using + * the hs_circuit_setup_e2e_rend_circ() function. We then check that the + * end-to-end cpath was setup correctly and that the stream was attached to + * the circuit as expected. */ + + MOCK(connection_ap_handshake_send_begin, + mock_connection_ap_handshake_send_begin); + + /* Setup */ + retval = helper_get_circ_and_stream_for_test( &or_circ, &conn, 0); + tt_int_op(retval, OP_EQ, 0); + tt_assert(or_circ); + tt_assert(conn); + + /* Check number of hops: There should be no hops yet to this circ */ + retval = cpath_get_n_hops(&or_circ->cpath); + tt_int_op(retval, ==, 0); + tt_assert(!or_circ->cpath); + + /* Check that our stream is not attached on any circuits */ + tt_assert(!TO_EDGE_CONN(conn)->on_circuit); + + /**********************************************/ + + /* Setup the circuit */ + retval = hs_circuit_setup_e2e_rend_circ(or_circ, ntor_key_seed, 0); + tt_int_op(retval, OP_EQ, 0); + + /**********************************************/ + + /* See that a hop was added to the circuit's cpath */ + retval = cpath_get_n_hops(&or_circ->cpath); + tt_int_op(retval, OP_EQ, 1); + + /* Check that the crypt path has prop224 algorithm parameters */ + tt_int_op(or_circ->cpath->f_digest->algorithm, OP_EQ, DIGEST_SHA3_256); + tt_int_op(or_circ->cpath->b_digest->algorithm, OP_EQ, DIGEST_SHA3_256); + tt_assert(or_circ->cpath->f_crypto); + tt_assert(or_circ->cpath->b_crypto); + + /* Ensure that circ purpose was changed */ + tt_int_op(or_circ->base_.purpose, OP_EQ, CIRCUIT_PURPOSE_C_REND_JOINED); + + /* Test that stream got attached */ + tt_ptr_op(TO_EDGE_CONN(conn)->on_circuit, OP_EQ, TO_CIRCUIT(or_circ)); + + done: + connection_free_(conn); + tor_free(TO_CIRCUIT(or_circ)->n_chan); + circuit_free(TO_CIRCUIT(or_circ)); +} + +struct testcase_t hs_client_tests[] = { + { "e2e_rend_circuit_setup_legacy", test_e2e_rend_circuit_setup_legacy, + TT_FORK, NULL, NULL }, + { "e2e_rend_circuit_setup", test_e2e_rend_circuit_setup, + TT_FORK, NULL, NULL }, + END_OF_TESTCASES +}; +

1 0

[tor/master] test: Add service-side unittests for e2e rendezvous circuits.
by nickm＠torproject.org 07 Jul '17

07 Jul '17

commit 9bccc04f8d8e5a8b7d9ae4f18c9fe2c3264bef0c Author: George Kadianakis <desnacked(a)riseup.net> Date: Tue May 2 16:37:17 2017 +0300 test: Add service-side unittests for e2e rendezvous circuits. --- src/test/test_hs_service.c | 90 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 84 insertions(+), 6 deletions(-) diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index fcfb3b9..b5aaa0c 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -6,20 +6,34 @@ * \brief Test hidden service functionality. */ +#define CIRCUITBUILD_PRIVATE +#define CIRCUITLIST_PRIVATE +#define CONNECTION_PRIVATE +#define CRYPTO_PRIVATE #define HS_COMMON_PRIVATE -#define HS_SERVICE_PRIVATE #define HS_INTROPOINT_PRIVATE +#define MAIN_PRIVATE +#define TOR_CHANNEL_INTERNAL_ #include "test.h" #include "log_test_helpers.h" -#include "crypto.h" - -#include "hs/cell_establish_intro.h" +#include "rend_test_helpers.h" + +#include "or.h" +#include "channeltls.h" +#include "circuitbuild.h" +#include "circuitlist.h" +#include "circuituse.h" +#include "config.h" +#include "connection.h" +#include "hs_circuit.h" #include "hs_common.h" -#include "hs_service.h" +#include "hs_ident.h" #include "hs_intropoint.h" - #include "hs_ntor.h" +#include "hs_service.h" +#include "main.h" +#include "rendservice.h" /** We simulate the creation of an outgoing ESTABLISH_INTRO cell, and then we * parse it from the receiver side. */ @@ -235,6 +249,68 @@ test_time_period(void *arg) ; } +/* Test: Ensure that setting up rendezvous circuits works correctly. */ +static void +test_e2e_rend_circuit_setup(void *arg) +{ + ed25519_public_key_t service_pk; + origin_circuit_t *or_circ; + int retval; + + /** In this test we create a v3 prop224 service-side rendezvous circuit. + * We simulate an HS ntor key exchange with a client, and check that + * the circuit was setup correctly and is ready to accept rendezvous data */ + + (void) arg; + + /* Now make dummy circuit */ + { + or_circ = origin_circuit_new(); + + or_circ->base_.purpose = CIRCUIT_PURPOSE_S_CONNECT_REND; + + or_circ->build_state = tor_malloc_zero(sizeof(cpath_build_state_t)); + or_circ->build_state->is_internal = 1; + + /* prop224: Setup hs conn identifier on the stream */ + ed25519_secret_key_t sk; + tt_int_op(0, OP_EQ, ed25519_secret_key_generate(&sk, 0)); + tt_int_op(0, OP_EQ, ed25519_public_key_generate(&service_pk, &sk)); + + or_circ->hs_ident = hs_ident_circuit_new(&service_pk, + HS_IDENT_CIRCUIT_RENDEZVOUS); + + TO_CIRCUIT(or_circ)->state = CIRCUIT_STATE_OPEN; + } + + /* Check number of hops */ + retval = cpath_get_n_hops(&or_circ->cpath); + tt_int_op(retval, OP_EQ, 0); + + /* Setup the circuit: do the ntor key exchange */ + { + uint8_t ntor_key_seed[DIGEST256_LEN] = {2}; + retval = hs_circuit_setup_e2e_rend_circ(or_circ, ntor_key_seed, 1); + tt_int_op(retval, OP_EQ, 0); + } + + /* See that a hop was added to the circuit's cpath */ + retval = cpath_get_n_hops(&or_circ->cpath); + tt_int_op(retval, OP_EQ, 1); + + /* Check the digest algo */ + tt_int_op(or_circ->cpath->f_digest->algorithm, OP_EQ, DIGEST_SHA3_256); + tt_int_op(or_circ->cpath->b_digest->algorithm, OP_EQ, DIGEST_SHA3_256); + tt_assert(or_circ->cpath->f_crypto); + tt_assert(or_circ->cpath->b_crypto); + + /* Ensure that circ purpose was changed */ + tt_int_op(or_circ->base_.purpose, OP_EQ, CIRCUIT_PURPOSE_S_REND_JOINED); + + done: + circuit_free(TO_CIRCUIT(or_circ)); +} + struct testcase_t hs_service_tests[] = { { "gen_establish_intro_cell", test_gen_establish_intro_cell, TT_FORK, NULL, NULL }, @@ -244,6 +320,8 @@ struct testcase_t hs_service_tests[] = { NULL, NULL }, { "time_period", test_time_period, TT_FORK, NULL, NULL }, + { "e2e_rend_circuit_setup", test_e2e_rend_circuit_setup, TT_FORK, + NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/master] Rename get_rend_cpath() to create_rend_cpath().
by nickm＠torproject.org 07 Jul '17

07 Jul '17

commit 2432499705b0df5427604b5bf501bb22ccd73131 Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Jul 6 13:53:08 2017 +0300 Rename get_rend_cpath() to create_rend_cpath(). based on Nick's review. --- src/or/hs_circuit.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index 213f5cb..1729080 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -48,7 +48,7 @@ circuit_purpose_is_correct_for_rend(unsigned int circ_purpose, int is_service_si * If is_service_side is set, we are the hidden service and the final * hop of the rendezvous circuit is the client on the other side. */ static crypt_path_t * -get_rend_cpath(const uint8_t *ntor_key_seed, int is_service_side) +create_rend_cpath(const uint8_t *ntor_key_seed, int is_service_side) { uint8_t keys[HS_NTOR_KEY_EXPANSION_KDF_OUT_LEN]; crypt_path_t *cpath = NULL; @@ -77,7 +77,7 @@ get_rend_cpath(const uint8_t *ntor_key_seed, int is_service_side) * rend_cell_body (which must be at least DH_KEY_LEN+DIGEST_LEN bytes). */ static crypt_path_t * -get_rend_cpath_legacy(origin_circuit_t *circ, const uint8_t *rend_cell_body) +create_rend_cpath_legacy(origin_circuit_t *circ, const uint8_t *rend_cell_body) { crypt_path_t *hop = NULL; char keys[DIGEST_LEN+CPATH_KEY_MATERIAL_LEN]; @@ -177,7 +177,7 @@ hs_circuit_setup_e2e_rend_circ(origin_circuit_t *circ, return -1; } - crypt_path_t *hop = get_rend_cpath(ntor_key_seed, is_service_side); + crypt_path_t *hop = create_rend_cpath(ntor_key_seed, is_service_side); if (!hop) { log_warn(LD_REND, "Couldn't get v3 %s cpath!", is_service_side ? "service-side" : "client-side"); @@ -203,7 +203,7 @@ hs_circuit_setup_e2e_rend_circ_legacy_client(origin_circuit_t *circ, return -1; } - crypt_path_t *hop = get_rend_cpath_legacy(circ, rend_cell_body); + crypt_path_t *hop = create_rend_cpath_legacy(circ, rend_cell_body); if (!hop) { log_warn(LD_GENERAL, "Couldn't get v2 cpath."); return -1;

1 0

[tor/master] Turn some warnings into bugs and non-fatal asserts.
by nickm＠torproject.org 07 Jul '17

07 Jul '17

commit fee95dabcfe21a35e3f5a1ad4b455fe4be1f749a Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Jul 6 13:50:16 2017 +0300 Turn some warnings into bugs and non-fatal asserts. --- src/or/connection_edge.c | 6 ++---- src/or/hs_circuit.c | 8 ++++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 0e18550..5e6041f 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -3566,10 +3566,8 @@ int connection_edge_is_rendezvous_stream(const edge_connection_t *conn) { tor_assert(conn); - - if (BUG(conn->rend_data && conn->hs_ident)) { - log_warn(LD_BUG, "Connection has both rend_data and hs_ident..."); - } + /* It should not be possible to set both of these structs */ + tor_assert_nonfatal(!(conn->rend_data && conn->hs_ident)); if (conn->rend_data || conn->hs_ident) { return 1; diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index 02c9afa..2d186e9 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -23,8 +23,8 @@ rend_circuit_validate_purpose(unsigned int circ_purpose, int is_service_side) { if (is_service_side) { if (circ_purpose != CIRCUIT_PURPOSE_S_CONNECT_REND) { - log_warn(LD_GENERAL, "HS e2e circuit setup with wrong purpose(%d)", - circ_purpose); + log_fn(LOG_PROTOCOL_WARN, LD_GENERAL, + "HS e2e circuit setup with wrong purpose(%d)", circ_purpose); return -1; } } @@ -32,8 +32,8 @@ rend_circuit_validate_purpose(unsigned int circ_purpose, int is_service_side) if (!is_service_side) { if (circ_purpose != CIRCUIT_PURPOSE_C_REND_READY && circ_purpose != CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED) { - log_warn(LD_GENERAL, "Client e2e circuit setup with wrong purpose(%d)", - circ_purpose); + log_fn(LOG_PROTOCOL_WARN, LD_GENERAL, + "Client e2e circuit setup with wrong purpose(%d)", circ_purpose); return -1; } }

1 0

[tor/master] Explicit length checks in create_rend_cpath().
by nickm＠torproject.org 07 Jul '17

07 Jul '17

commit 70d08f764d9912e66a2c6c0f3e4241f563d53ebd Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Jul 6 16:23:30 2017 +0300 Explicit length checks in create_rend_cpath(). Had to also edit hs_ntor_circuit_key_expansion() to make it happen. --- src/or/hs_circuit.c | 13 +++++++++---- src/or/hs_circuit.h | 1 + src/or/hs_ntor.c | 19 ++++++++++++++++--- src/or/hs_ntor.h | 4 ++-- src/test/test_hs_client.c | 4 +++- src/test/test_hs_service.c | 4 +++- 6 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index 42c5dcb..f2ea8f5 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -48,13 +48,17 @@ circuit_purpose_is_correct_for_rend(unsigned int circ_purpose, int is_service_si * If is_service_side is set, we are the hidden service and the final * hop of the rendezvous circuit is the client on the other side. */ static crypt_path_t * -create_rend_cpath(const uint8_t *ntor_key_seed, int is_service_side) +create_rend_cpath(const uint8_t *ntor_key_seed, size_t seed_len, + int is_service_side) { uint8_t keys[HS_NTOR_KEY_EXPANSION_KDF_OUT_LEN]; crypt_path_t *cpath = NULL; /* Do the key expansion */ - hs_ntor_circuit_key_expansion(ntor_key_seed, keys); + if (hs_ntor_circuit_key_expansion(ntor_key_seed, seed_len, + keys, sizeof(keys)) < 0) { + goto err; + } /* Setup the cpath */ cpath = tor_malloc_zero(sizeof(crypt_path_t)); @@ -171,7 +175,7 @@ finalize_rend_circuit(origin_circuit_t *circ, crypt_path_t *hop, * Return 0 if the operation went well; in case of error return -1. */ int hs_circuit_setup_e2e_rend_circ(origin_circuit_t *circ, - const uint8_t *ntor_key_seed, + const uint8_t *ntor_key_seed, size_t seed_len, int is_service_side) { if (BUG(!circuit_purpose_is_correct_for_rend(TO_CIRCUIT(circ)->purpose, @@ -179,7 +183,8 @@ hs_circuit_setup_e2e_rend_circ(origin_circuit_t *circ, return -1; } - crypt_path_t *hop = create_rend_cpath(ntor_key_seed, is_service_side); + crypt_path_t *hop = create_rend_cpath(ntor_key_seed, seed_len, + is_service_side); if (!hop) { log_warn(LD_REND, "Couldn't get v3 %s cpath!", is_service_side ? "service-side" : "client-side"); diff --git a/src/or/hs_circuit.h b/src/or/hs_circuit.h index 1c2924c..71ce5c3 100644 --- a/src/or/hs_circuit.h +++ b/src/or/hs_circuit.h @@ -15,6 +15,7 @@ int hs_circuit_setup_e2e_rend_circ(origin_circuit_t *circ, const uint8_t *ntor_key_seed, + size_t seed_len, int is_service_side); int hs_circuit_setup_e2e_rend_circ_legacy_client(origin_circuit_t *circ, const uint8_t *rend_cell_body); diff --git a/src/or/hs_ntor.c b/src/or/hs_ntor.c index 668ef22..a416bc4 100644 --- a/src/or/hs_ntor.c +++ b/src/or/hs_ntor.c @@ -582,14 +582,25 @@ hs_ntor_client_rendezvous2_mac_is_good( /** Given the rendezvous key seed in ntor_key_seed (of size * DIGEST256_LEN), do the circuit key expansion as specified by section * '4.2.1. Key expansion' and place the keys in keys_out (which must be - * of size HS_NTOR_KEY_EXPANSION_KDF_OUT_LEN). */ -void -hs_ntor_circuit_key_expansion(const uint8_t *ntor_key_seed, uint8_t *keys_out) + * of size HS_NTOR_KEY_EXPANSION_KDF_OUT_LEN). + * + * Return 0 if things went well, else return -1. */ +int +hs_ntor_circuit_key_expansion(const uint8_t *ntor_key_seed, size_t seed_len, + uint8_t *keys_out, size_t keys_out_len) { uint8_t *ptr; uint8_t kdf_input[NTOR_KEY_EXPANSION_KDF_INPUT_LEN]; crypto_xof_t *xof; + /* Sanity checks on lengths to make sure we are good */ + if (BUG(seed_len != DIGEST256_LEN)) { + return -1; + } + if (BUG(keys_out_len != HS_NTOR_KEY_EXPANSION_KDF_OUT_LEN)) { + return -1; + } + /* Let's build the input to the KDF */ ptr = kdf_input; APPEND(ptr, ntor_key_seed, DIGEST256_LEN); @@ -601,5 +612,7 @@ hs_ntor_circuit_key_expansion(const uint8_t *ntor_key_seed, uint8_t *keys_out) crypto_xof_add_bytes(xof, kdf_input, sizeof(kdf_input)); crypto_xof_squeeze_bytes(xof, keys_out, HS_NTOR_KEY_EXPANSION_KDF_OUT_LEN); crypto_xof_free(xof); + + return 0; } diff --git a/src/or/hs_ntor.h b/src/or/hs_ntor.h index 3a97e17..37c3261 100644 --- a/src/or/hs_ntor.h +++ b/src/or/hs_ntor.h @@ -55,8 +55,8 @@ int hs_ntor_service_get_rendezvous1_keys( const curve25519_public_key_t *client_ephemeral_enc_pubkey, hs_ntor_rend_cell_keys_t *hs_ntor_rend_cell_keys_out); -void hs_ntor_circuit_key_expansion(const uint8_t *ntor_key_seed, - uint8_t *keys_out); +int hs_ntor_circuit_key_expansion(const uint8_t *ntor_key_seed, size_t seed_len, + uint8_t *keys_out, size_t keys_out_len); int hs_ntor_client_rendezvous2_mac_is_good( const hs_ntor_rend_cell_keys_t *hs_ntor_rend_cell_keys, diff --git a/src/test/test_hs_client.c b/src/test/test_hs_client.c index 9e5fe04..938d3d2 100644 --- a/src/test/test_hs_client.c +++ b/src/test/test_hs_client.c @@ -243,7 +243,9 @@ test_e2e_rend_circuit_setup(void *arg) /**********************************************/ /* Setup the circuit */ - retval = hs_circuit_setup_e2e_rend_circ(or_circ, ntor_key_seed, 0); + retval = hs_circuit_setup_e2e_rend_circ(or_circ, + ntor_key_seed, sizeof(ntor_key_seed), + 0); tt_int_op(retval, OP_EQ, 0); /**********************************************/ diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index b5aaa0c..5793747 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -290,7 +290,9 @@ test_e2e_rend_circuit_setup(void *arg) /* Setup the circuit: do the ntor key exchange */ { uint8_t ntor_key_seed[DIGEST256_LEN] = {2}; - retval = hs_circuit_setup_e2e_rend_circ(or_circ, ntor_key_seed, 1); + retval = hs_circuit_setup_e2e_rend_circ(or_circ, + ntor_key_seed, sizeof(ntor_key_seed), + 1); tt_int_op(retval, OP_EQ, 0); }

1 0

[tor/master] Hide crypto_digest_t again and use an accessor for tests.
by nickm＠torproject.org 07 Jul '17

07 Jul '17

commit f35f52e8697407dde391ebc4f1be6ba76e1a1bb2 Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Jul 6 16:39:48 2017 +0300 Hide crypto_digest_t again and use an accessor for tests. --- src/common/crypto.c | 27 +++++++++++++++++++++++++++ src/common/crypto.h | 16 +--------------- src/test/test_hs_client.c | 12 ++++++++---- src/test/test_hs_service.c | 6 ++++-- 4 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/common/crypto.c b/src/common/crypto.c index 8b214a6..875b4ee 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -1839,6 +1839,33 @@ crypto_digest_algorithm_get_length(digest_algorithm_t alg) } } +/** Intermediate information about the digest of a stream of data. */ +struct crypto_digest_t { + digest_algorithm_t algorithm; /**< Which algorithm is in use? */ + /** State for the digest we're using. Only one member of the + * union is usable, depending on the value of algorithm. Note also + * that space for other members might not even be allocated! + */ + union { + SHA_CTX sha1; /**< state for SHA1 */ + SHA256_CTX sha2; /**< state for SHA256 */ + SHA512_CTX sha512; /**< state for SHA512 */ + keccak_state sha3; /**< state for SHA3-[256,512] */ + } d; +}; + +#ifdef TOR_UNIT_TESTS + +digest_algorithm_t +crypto_digest_get_algorithm(crypto_digest_t *digest) +{ + tor_assert(digest); + + return digest->algorithm; +} + +#endif + /** * Return the number of bytes we need to malloc in order to get a * crypto_digest_t for alg, or the number of bytes we need to wipe diff --git a/src/common/crypto.h b/src/common/crypto.h index 3766830..5951321 100644 --- a/src/common/crypto.h +++ b/src/common/crypto.h @@ -339,21 +339,6 @@ void crypto_add_spaces_to_fp(char *out, size_t outlen, const char *in); #ifdef CRYPTO_PRIVATE -/** Intermediate information about the digest of a stream of data. */ -struct crypto_digest_t { - digest_algorithm_t algorithm; /**< Which algorithm is in use? */ - /** State for the digest we're using. Only one member of the - * union is usable, depending on the value of algorithm. Note also - * that space for other members might not even be allocated! - */ - union { - SHA_CTX sha1; /**< state for SHA1 */ - SHA256_CTX sha2; /**< state for SHA256 */ - SHA512_CTX sha512; /**< state for SHA512 */ - keccak_state sha3; /**< state for SHA3-[256,512] */ - } d; -}; - STATIC int crypto_force_rand_ssleay(void); STATIC int crypto_strongest_rand_raw(uint8_t *out, size_t out_len); @@ -365,6 +350,7 @@ extern int break_strongest_rng_fallback; #ifdef TOR_UNIT_TESTS void crypto_pk_assign_(crypto_pk_t *dest, const crypto_pk_t *src); +digest_algorithm_t crypto_digest_get_algorithm(crypto_digest_t *digest); #endif #endif diff --git a/src/test/test_hs_client.c b/src/test/test_hs_client.c index 938d3d2..77fee88 100644 --- a/src/test/test_hs_client.c +++ b/src/test/test_hs_client.c @@ -188,8 +188,10 @@ test_e2e_rend_circuit_setup_legacy(void *arg) tt_int_op(retval, OP_EQ, 1); /* Check the digest algo */ - tt_int_op(or_circ->cpath->f_digest->algorithm, OP_EQ, DIGEST_SHA1); - tt_int_op(or_circ->cpath->b_digest->algorithm, OP_EQ, DIGEST_SHA1); + tt_int_op(crypto_digest_get_algorithm(or_circ->cpath->f_digest), + OP_EQ, DIGEST_SHA1); + tt_int_op(crypto_digest_get_algorithm(or_circ->cpath->b_digest), + OP_EQ, DIGEST_SHA1); tt_assert(or_circ->cpath->f_crypto); tt_assert(or_circ->cpath->b_crypto); @@ -255,8 +257,10 @@ test_e2e_rend_circuit_setup(void *arg) tt_int_op(retval, OP_EQ, 1); /* Check that the crypt path has prop224 algorithm parameters */ - tt_int_op(or_circ->cpath->f_digest->algorithm, OP_EQ, DIGEST_SHA3_256); - tt_int_op(or_circ->cpath->b_digest->algorithm, OP_EQ, DIGEST_SHA3_256); + tt_int_op(crypto_digest_get_algorithm(or_circ->cpath->f_digest), + OP_EQ, DIGEST_SHA3_256); + tt_int_op(crypto_digest_get_algorithm(or_circ->cpath->b_digest), + OP_EQ, DIGEST_SHA3_256); tt_assert(or_circ->cpath->f_crypto); tt_assert(or_circ->cpath->b_crypto); diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index 5793747..aa9a12e 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -301,8 +301,10 @@ test_e2e_rend_circuit_setup(void *arg) tt_int_op(retval, OP_EQ, 1); /* Check the digest algo */ - tt_int_op(or_circ->cpath->f_digest->algorithm, OP_EQ, DIGEST_SHA3_256); - tt_int_op(or_circ->cpath->b_digest->algorithm, OP_EQ, DIGEST_SHA3_256); + tt_int_op(crypto_digest_get_algorithm(or_circ->cpath->f_digest), + OP_EQ, DIGEST_SHA3_256); + tt_int_op(crypto_digest_get_algorithm(or_circ->cpath->b_digest), + OP_EQ, DIGEST_SHA3_256); tt_assert(or_circ->cpath->f_crypto); tt_assert(or_circ->cpath->b_crypto);

1 0