April 2020 - tor-commits - lists.torproject.org

[tor/master] test/circuitbuild: Add tests for open_connection_for_extend
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit 7bc34133225c38b8ababbf9ff4c220285e65b1ef Author: teor <teor(a)torproject.org> Date: Fri Apr 3 19:02:26 2020 +1000 test/circuitbuild: Add tests for open_connection_for_extend Part of 33633. --- src/test/test_circuitbuild.c | 157 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 157 insertions(+) diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index 310806bd0..2d40e976d 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -685,6 +685,162 @@ test_circuit_extend_lspec_valid(void *arg) tor_free(p_chan); } +static int mock_circuit_close_calls = 0; +static void +mock_circuit_mark_for_close_(circuit_t *circ, int reason, + int line, const char *cfile) +{ + (void)circ; + (void)reason; + (void)line; + (void)cfile; + mock_circuit_close_calls++; +} + +static int mock_channel_connect_calls = 0; +static channel_t *mock_channel_connect_nchan = NULL; +static channel_t * +mock_channel_connect_for_circuit(const tor_addr_t *addr, + uint16_t port, + const char *id_digest, + const struct ed25519_public_key_t *ed_id) +{ + (void)addr; + (void)port; + (void)id_digest; + (void)ed_id; + mock_channel_connect_calls++; + return mock_channel_connect_nchan; +} + +/* Test the different cases in circuit_open_connection_for_extend(). */ +static void +test_circuit_open_connection_for_extend(void *arg) +{ + (void)arg; + extend_cell_t *ec = tor_malloc_zero(sizeof(extend_cell_t)); + circuit_t *circ = tor_malloc_zero(sizeof(circuit_t)); + channel_t *fake_n_chan = tor_malloc_zero(sizeof(channel_t)); + + MOCK(circuit_mark_for_close_, mock_circuit_mark_for_close_); + mock_circuit_close_calls = 0; + MOCK(channel_connect_for_circuit, mock_channel_connect_for_circuit); + mock_channel_connect_calls = 0; + mock_channel_connect_nchan = NULL; + + setup_full_capture_of_logs(LOG_INFO); + + /* Circuit must be non-NULL */ + mock_circuit_close_calls = 0; + mock_channel_connect_calls = 0; + tor_capture_bugs_(1); + circuit_open_connection_for_extend(ec, NULL, 0); + /* We can't close a NULL circuit */ + tt_int_op(mock_circuit_close_calls, OP_EQ, 0); + tt_int_op(mock_channel_connect_calls, OP_EQ, 0); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); + tt_str_op(smartlist_get(tor_get_captured_bug_log_(), 0), OP_EQ, + "!(ASSERT_PREDICT_UNLIKELY_(!circ))"); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + + /* Extend cell must be non-NULL */ + mock_circuit_close_calls = 0; + mock_channel_connect_calls = 0; + tor_capture_bugs_(1); + circuit_open_connection_for_extend(NULL, circ, 0); + tt_int_op(mock_circuit_close_calls, OP_EQ, 1); + tt_int_op(mock_channel_connect_calls, OP_EQ, 0); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); + tt_str_op(smartlist_get(tor_get_captured_bug_log_(), 0), OP_EQ, + "!(ASSERT_PREDICT_UNLIKELY_(!ec))"); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + + /* Extend cell and circuit must be non-NULL */ + mock_circuit_close_calls = 0; + mock_channel_connect_calls = 0; + tor_capture_bugs_(1); + circuit_open_connection_for_extend(NULL, NULL, 0); + /* We can't close a NULL circuit */ + tt_int_op(mock_circuit_close_calls, OP_EQ, 0); + tt_int_op(mock_channel_connect_calls, OP_EQ, 0); + /* Since we're using IF_BUG_ONCE(), we might not log any bugs */ + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_GE, 0); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_LE, 2); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + + /* Succeed, but don't try to open a connection */ + mock_circuit_close_calls = 0; + mock_channel_connect_calls = 0; + circuit_open_connection_for_extend(ec, circ, 0); + /* If we haven't closed the circuit, that's success */ + tt_int_op(mock_circuit_close_calls, OP_EQ, 0); + tt_int_op(mock_channel_connect_calls, OP_EQ, 0); + /* Check state */ + tt_ptr_op(circ->n_hop, OP_NE, NULL); + tt_ptr_op(circ->n_chan_create_cell, OP_NE, NULL); + tt_int_op(circ->state, OP_EQ, CIRCUIT_STATE_CHAN_WAIT); + /* Cleanup */ + mock_clean_saved_logs(); + tor_free(circ->n_hop); + tor_free(circ->n_chan_create_cell); + circ->state = 0; + + /* Try to open a connection, but fail with a NULL n_chan */ + mock_circuit_close_calls = 0; + mock_channel_connect_calls = 0; + circuit_open_connection_for_extend(ec, circ, 1); + /* Try to connect, but fail, and close the circuit */ + tt_int_op(mock_circuit_close_calls, OP_EQ, 1); + tt_int_op(mock_channel_connect_calls, OP_EQ, 1); + expect_log_msg("Launching n_chan failed. Closing circuit.\n"); + /* Check state */ + tt_ptr_op(circ->n_hop, OP_NE, NULL); + tt_ptr_op(circ->n_chan_create_cell, OP_NE, NULL); + tt_int_op(circ->state, OP_EQ, CIRCUIT_STATE_CHAN_WAIT); + /* Cleanup */ + mock_clean_saved_logs(); + tor_free(circ->n_hop); + tor_free(circ->n_chan_create_cell); + circ->state = 0; + + /* Try to open a connection, and succeed, because n_chan is not NULL */ + mock_channel_connect_nchan = fake_n_chan; + mock_circuit_close_calls = 0; + mock_channel_connect_calls = 0; + circuit_open_connection_for_extend(ec, circ, 1); + /* Try to connect, and succeed, leaving the circuit open */ + tt_int_op(mock_circuit_close_calls, OP_EQ, 0); + tt_int_op(mock_channel_connect_calls, OP_EQ, 1); + /* Check state */ + tt_ptr_op(circ->n_hop, OP_NE, NULL); + tt_ptr_op(circ->n_chan_create_cell, OP_NE, NULL); + tt_int_op(circ->state, OP_EQ, CIRCUIT_STATE_CHAN_WAIT); + /* Cleanup */ + mock_clean_saved_logs(); + tor_free(circ->n_hop); + tor_free(circ->n_chan_create_cell); + circ->state = 0; + mock_channel_connect_nchan = NULL; + + done: + tor_end_capture_bugs_(); + teardown_capture_of_logs(); + + UNMOCK(circuit_mark_for_close_); + mock_circuit_close_calls = 0; + UNMOCK(channel_connect_for_circuit); + mock_channel_connect_calls = 0; + + tor_free(ec); + tor_free(circ->n_hop); + tor_free(circ->n_chan_create_cell); + tor_free(circ); + tor_free(fake_n_chan); +} + #define TEST(name, flags, setup, cleanup) \ { #name, test_ ## name, flags, setup, cleanup } @@ -705,5 +861,6 @@ struct testcase_t circuitbuild_tests[] = { TEST_CIRCUIT(extend_state_valid, TT_FORK), TEST_CIRCUIT(extend_add_ed25519, TT_FORK), TEST_CIRCUIT(extend_lspec_valid, TT_FORK), + TEST_CIRCUIT(open_connection_for_extend, TT_FORK), END_OF_TESTCASES };

1 0

[tor/master] circuitbuild: Make some functions mockable
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit 42fdbbb50bbb9d0f2e15012256a088f3f1c22f8d Author: teor <teor(a)torproject.org> Date: Fri Apr 3 19:01:19 2020 +1000 circuitbuild: Make some functions mockable Part of 33633. --- src/core/or/circuitbuild.c | 8 ++++---- src/core/or/circuitbuild.h | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index 0527548f2..71a51e61a 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -95,10 +95,10 @@ static const node_t *choose_good_middle_server(uint8_t purpose, * and then calls command_setup_channel() to give it the right * callbacks. */ -channel_t * -channel_connect_for_circuit(const tor_addr_t *addr, uint16_t port, - const char *id_digest, - const struct ed25519_public_key_t *ed_id) +MOCK_IMPL(channel_t *, +channel_connect_for_circuit,(const tor_addr_t *addr, uint16_t port, + const char *id_digest, + const struct ed25519_public_key_t *ed_id)) { channel_t *chan; diff --git a/src/core/or/circuitbuild.h b/src/core/or/circuitbuild.h index c66c0dc75..cf9999092 100644 --- a/src/core/or/circuitbuild.h +++ b/src/core/or/circuitbuild.h @@ -73,13 +73,13 @@ void circuit_upgrade_circuits_from_guard_wait(void); struct ed25519_public_key_t; -channel_t * -channel_connect_for_circuit(const tor_addr_t *addr, - uint16_t port, - const char *id_digest, - const struct ed25519_public_key_t *ed_id); -struct create_cell_t; +MOCK_DECL(channel_t *, +channel_connect_for_circuit,(const tor_addr_t *addr, + uint16_t port, + const char *id_digest, + const struct ed25519_public_key_t *ed_id)); +struct create_cell_t; int circuit_deliver_create_cell(circuit_t *circ, const struct create_cell_t *create_cell,

1 0

[tor/master] test/circuitbuild: Add tests for extend_lspec_valid
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit 46980d767d643abe4415b3ea7e6728aecb848119 Author: teor <teor(a)torproject.org> Date: Fri Apr 3 14:36:29 2020 +1000 test/circuitbuild: Add tests for extend_lspec_valid Part of 33633. --- src/test/test_circuitbuild.c | 224 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 224 insertions(+) diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index 9a0599ba6..310806bd0 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -13,6 +13,7 @@ #include "test/test_helpers.h" #include "test/log_test_helpers.h" +#define CONFIG_PRIVATE #include "app/config/config.h" #include "core/or/channel.h" @@ -462,6 +463,228 @@ test_circuit_extend_add_ed25519(void *arg) tor_free(fake_node); } +static or_options_t *mocked_options = NULL; +static const or_options_t * +mock_get_options(void) +{ + return mocked_options; +} + +#define PUBLIC_IPV4 "1.2.3.4" +#define INTERNAL_IPV4 "0.0.0.1" + +#define VALID_PORT 0x1234 + +/* Test the different cases in circuit_extend_lspec_valid_helper(). */ +static void +test_circuit_extend_lspec_valid(void *arg) +{ + (void)arg; + extend_cell_t *ec = tor_malloc_zero(sizeof(extend_cell_t)); + channel_t *p_chan = tor_malloc_zero(sizeof(channel_t)); + or_circuit_t *or_circ = tor_malloc_zero(sizeof(or_circuit_t)); + circuit_t *circ = TO_CIRCUIT(or_circ); + + or_options_t *fake_options = options_new(); + MOCK(get_options, mock_get_options); + mocked_options = fake_options; + + setup_full_capture_of_logs(LOG_INFO); + + /* Extend cell must be non-NULL */ + tor_capture_bugs_(1); + tt_int_op(circuit_extend_lspec_valid_helper(NULL, circ), OP_EQ, -1); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); + tt_str_op(smartlist_get(tor_get_captured_bug_log_(), 0), OP_EQ, + "!(ASSERT_PREDICT_UNLIKELY_(!ec))"); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + + /* Circuit must be non-NULL */ + tor_capture_bugs_(1); + tt_int_op(circuit_extend_lspec_valid_helper(ec, NULL), OP_EQ, -1); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); + tt_str_op(smartlist_get(tor_get_captured_bug_log_(), 0), OP_EQ, + "!(ASSERT_PREDICT_UNLIKELY_(!circ))"); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + + /* Extend cell and circuit must be non-NULL */ + tor_capture_bugs_(1); + tt_int_op(circuit_extend_lspec_valid_helper(NULL, NULL), OP_EQ, -1); + /* Since we're using IF_BUG_ONCE(), we might not log any bugs */ + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_GE, 0); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_LE, 2); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + + /* IPv4 addr or port are 0, these should fail */ + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + expect_log_msg("Client asked me to extend to " + "zero destination port or addr.\n"); + mock_clean_saved_logs(); + + tor_addr_parse(&ec->orport_ipv4.addr, PUBLIC_IPV4); + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + expect_log_msg("Client asked me to extend to " + "zero destination port or addr.\n"); + mock_clean_saved_logs(); + tor_addr_make_null(&ec->orport_ipv4.addr, AF_INET); + + ec->orport_ipv4.port = VALID_PORT; + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + expect_log_msg("Client asked me to extend to " + "zero destination port or addr.\n"); + mock_clean_saved_logs(); + ec->orport_ipv4.port = 0; + + /* IPv4 addr is internal, and port is valid. + * Result depends on ExtendAllowPrivateAddresses. */ + tor_addr_parse(&ec->orport_ipv4.addr, INTERNAL_IPV4); + ec->orport_ipv4.port = VALID_PORT; + + fake_options->ExtendAllowPrivateAddresses = 0; + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + expect_log_msg("Client asked me to extend to a private address.\n"); + mock_clean_saved_logs(); + fake_options->ExtendAllowPrivateAddresses = 0; + + /* If we pass the private address check, but don't have the right + * OR circuit magic number, we trigger another bug */ + fake_options->ExtendAllowPrivateAddresses = 1; + tor_capture_bugs_(1); + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); + tt_str_op(smartlist_get(tor_get_captured_bug_log_(), 0), OP_EQ, + "!(ASSERT_PREDICT_UNLIKELY_(circ->magic != 0x98ABC04Fu))"); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + fake_options->ExtendAllowPrivateAddresses = 0; + + /* Now set the right magic */ + or_circ->base_.magic = OR_CIRCUIT_MAGIC; + + /* If we pass the OR circuit magic check, but don't have p_chan, + * we trigger another bug */ + fake_options->ExtendAllowPrivateAddresses = 1; + tor_capture_bugs_(1); + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); + tt_str_op(smartlist_get(tor_get_captured_bug_log_(), 0), OP_EQ, + "!(ASSERT_PREDICT_UNLIKELY_(!p_chan))"); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + fake_options->ExtendAllowPrivateAddresses = 0; + + /* We can also pass the OR circuit magic check with a public address */ + tor_addr_parse(&ec->orport_ipv4.addr, PUBLIC_IPV4); + fake_options->ExtendAllowPrivateAddresses = 0; + tor_capture_bugs_(1); + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + /* Since we're using IF_BUG_ONCE(), expect 0-1 bug logs */ + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_GE, 0); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_LE, 1); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + fake_options->ExtendAllowPrivateAddresses = 0; + + tor_addr_make_null(&ec->orport_ipv4.addr, AF_INET); + ec->orport_ipv4.port = 0x0000; + + /* Now let's fake a p_chan and the addresses */ + tor_addr_parse(&ec->orport_ipv4.addr, PUBLIC_IPV4); + ec->orport_ipv4.port = VALID_PORT; + or_circ->p_chan = p_chan; + + /* This is a trivial failure: node_id and p_chan->identity_digest are both + * zeroed */ + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + expect_log_msg("Client asked me to extend back to the previous hop.\n"); + mock_clean_saved_logs(); + + /* Let's check with non-zero identities as well */ + memset(ec->node_id, 0xAA, sizeof(ec->node_id)); + memset(p_chan->identity_digest, 0xAA, sizeof(p_chan->identity_digest)); + + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + expect_log_msg("Client asked me to extend back to the previous hop.\n"); + mock_clean_saved_logs(); + + memset(ec->node_id, 0, sizeof(ec->node_id)); + memset(p_chan->identity_digest, 0, sizeof(p_chan->identity_digest)); + + /* Let's pass the node_id test */ + memset(ec->node_id, 0xAA, sizeof(ec->node_id)); + memset(p_chan->identity_digest, 0xBB, sizeof(p_chan->identity_digest)); + + /* ed_pubkey is zero, and that's allowed, so we should succeed */ + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, 0); + mock_clean_saved_logs(); + + /* Fail on matching non-zero identities */ + memset(&ec->ed_pubkey, 0xEE, sizeof(ec->ed_pubkey)); + memset(&p_chan->ed25519_identity, 0xEE, sizeof(p_chan->ed25519_identity)); + + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + expect_log_msg("Client asked me to extend back to the previous hop " + "(by Ed25519 ID).\n"); + mock_clean_saved_logs(); + + memset(&ec->ed_pubkey, 0, sizeof(ec->ed_pubkey)); + memset(&p_chan->ed25519_identity, 0, sizeof(p_chan->ed25519_identity)); + + /* Succeed on different, non-zero identities */ + memset(&ec->ed_pubkey, 0xDD, sizeof(ec->ed_pubkey)); + memset(&p_chan->ed25519_identity, 0xEE, sizeof(p_chan->ed25519_identity)); + + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, 0); + mock_clean_saved_logs(); + + memset(&ec->ed_pubkey, 0, sizeof(ec->ed_pubkey)); + memset(&p_chan->ed25519_identity, 0, sizeof(p_chan->ed25519_identity)); + + /* Succeed if the client knows the identity, but we don't */ + memset(&ec->ed_pubkey, 0xDD, sizeof(ec->ed_pubkey)); + memset(&p_chan->ed25519_identity, 0x00, sizeof(p_chan->ed25519_identity)); + + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, 0); + mock_clean_saved_logs(); + + memset(&ec->ed_pubkey, 0, sizeof(ec->ed_pubkey)); + memset(&p_chan->ed25519_identity, 0, sizeof(p_chan->ed25519_identity)); + + /* Succeed if we know the identity, but the client doesn't */ + memset(&ec->ed_pubkey, 0x00, sizeof(ec->ed_pubkey)); + memset(&p_chan->ed25519_identity, 0xEE, sizeof(p_chan->ed25519_identity)); + + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, 0); + mock_clean_saved_logs(); + + memset(&ec->ed_pubkey, 0, sizeof(ec->ed_pubkey)); + memset(&p_chan->ed25519_identity, 0, sizeof(p_chan->ed25519_identity)); + + /* Cleanup the node ids */ + memset(ec->node_id, 0, sizeof(ec->node_id)); + memset(p_chan->identity_digest, 0, sizeof(p_chan->identity_digest)); + + /* Cleanup the p_chan and the addresses */ + tor_addr_make_null(&ec->orport_ipv4.addr, AF_UNSPEC); + ec->orport_ipv4.port = 0; + or_circ->p_chan = NULL; + + done: + tor_end_capture_bugs_(); + teardown_capture_of_logs(); + + UNMOCK(get_options); + or_options_free(fake_options); + mocked_options = NULL; + + tor_free(ec); + tor_free(or_circ); + tor_free(p_chan); +} + #define TEST(name, flags, setup, cleanup) \ { #name, test_ ## name, flags, setup, cleanup } @@ -481,5 +704,6 @@ struct testcase_t circuitbuild_tests[] = { TEST_CIRCUIT(extend_state_valid, TT_FORK), TEST_CIRCUIT(extend_add_ed25519, TT_FORK), + TEST_CIRCUIT(extend_lspec_valid, TT_FORK), END_OF_TESTCASES };

1 0

[tor/master] relay: Run "make autostyle"
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit cb262930f5b028bfa5297b4a0199aead66d35f0a Author: teor <teor(a)torproject.org> Date: Tue Mar 17 17:43:02 2020 +1000 relay: Run "make autostyle" But only take the changes from the relay module. Part of 33633. --- src/feature/relay/circuitbuild_relay.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/feature/relay/circuitbuild_relay.h b/src/feature/relay/circuitbuild_relay.h index 19dbc4b94..d14f304f1 100644 --- a/src/feature/relay/circuitbuild_relay.h +++ b/src/feature/relay/circuitbuild_relay.h @@ -41,7 +41,7 @@ int onionskin_answer(struct or_circuit_t *circ, const char *keys, size_t keys_len, const uint8_t *rend_circ_nonce); -#else +#else /* !defined(HAVE_MODULE_RELAY) */ static inline int circuit_extend(struct cell_t *cell, struct circuit_t *circ) @@ -67,7 +67,7 @@ onionskin_answer(struct or_circuit_t *circ, return -1; } -#endif +#endif /* defined(HAVE_MODULE_RELAY) */ #ifdef TOR_UNIT_TESTS @@ -79,6 +79,6 @@ STATIC void circuit_open_connection_for_extend(const struct extend_cell_t *ec, struct circuit_t *circ, int should_launch); -#endif +#endif /* defined(TOR_UNIT_TESTS) */ #endif /* !defined(TOR_FEATURE_RELAY_CIRCUITBUILD_RELAY_H) */

1 0

[tor/master] core/or: Make some functions mockable
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit 2b66429fcfe3dbbaa559b0b2de910e0266f27698 Author: teor <teor(a)torproject.org> Date: Mon Apr 6 19:14:27 2020 +1000 core/or: Make some functions mockable Preparation for testing circuit_extend(). Part of 33633. --- src/core/or/channel.c | 16 ++++++++-------- src/core/or/channel.h | 7 ++++--- src/core/or/circuitbuild.c | 8 ++++---- src/core/or/circuitbuild.h | 8 ++++---- src/core/or/onion.c | 8 +++++--- src/core/or/onion.h | 6 ++++-- 6 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/core/or/channel.c b/src/core/or/channel.c index 542b4d88b..9c1e8368e 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -2370,12 +2370,12 @@ channel_is_better(channel_t *a, channel_t *b) * and set *launch_out to a boolean indicated whether the caller should try to * launch a new channel with channel_connect(). */ -channel_t * -channel_get_for_extend(const char *rsa_id_digest, - const ed25519_public_key_t *ed_id, - const tor_addr_t *target_addr, - const char **msg_out, - int *launch_out) +MOCK_IMPL(channel_t *, +channel_get_for_extend,(const char *rsa_id_digest, + const ed25519_public_key_t *ed_id, + const tor_addr_t *target_addr, + const char **msg_out, + int *launch_out)) { channel_t *chan, *best = NULL; int n_inprogress_goodaddr = 0, n_old = 0; @@ -2821,8 +2821,8 @@ channel_get_actual_remote_address(channel_t *chan) * Subsequent calls to channel_get_{actual,canonical}_remote_{address,descr} * may invalidate the return value from this function. */ -const char * -channel_get_canonical_remote_descr(channel_t *chan) +MOCK_IMPL(const char *, +channel_get_canonical_remote_descr,(channel_t *chan)) { tor_assert(chan); tor_assert(chan->get_remote_descr); diff --git a/src/core/or/channel.h b/src/core/or/channel.h index 2e2936a69..49331d5d5 100644 --- a/src/core/or/channel.h +++ b/src/core/or/channel.h @@ -658,11 +658,12 @@ channel_t * channel_connect(const tor_addr_t *addr, uint16_t port, const char *rsa_id_digest, const struct ed25519_public_key_t *ed_id); -channel_t * channel_get_for_extend(const char *rsa_id_digest, +MOCK_DECL(channel_t *, channel_get_for_extend,( + const char *rsa_id_digest, const struct ed25519_public_key_t *ed_id, const tor_addr_t *target_addr, const char **msg_out, - int *launch_out); + int *launch_out)); /* Ask which of two channels is better for circuit-extension purposes */ int channel_is_better(channel_t *a, channel_t *b); @@ -723,7 +724,7 @@ const char * channel_get_actual_remote_descr(channel_t *chan); const char * channel_get_actual_remote_address(channel_t *chan); MOCK_DECL(int, channel_get_addr_if_possible, (channel_t *chan, tor_addr_t *addr_out)); -const char * channel_get_canonical_remote_descr(channel_t *chan); +MOCK_DECL(const char *, channel_get_canonical_remote_descr,(channel_t *chan)); int channel_has_queued_writes(channel_t *chan); int channel_is_bad_for_new_circs(channel_t *chan); void channel_mark_bad_for_new_circs(channel_t *chan); diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index 71a51e61a..ce0f9618f 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -698,10 +698,10 @@ circuit_n_chan_done(channel_t *chan, int status, int close_origin_circuits) * gave us via an EXTEND cell, so we shouldn't worry if we don't understand * it. Return -1 if we failed to find a suitable circid, else return 0. */ -int -circuit_deliver_create_cell(circuit_t *circ, - const struct create_cell_t *create_cell, - int relayed) +MOCK_IMPL(int, +circuit_deliver_create_cell,(circuit_t *circ, + const struct create_cell_t *create_cell, + int relayed)) { cell_t cell; circid_t id; diff --git a/src/core/or/circuitbuild.h b/src/core/or/circuitbuild.h index cf9999092..e62bb41de 100644 --- a/src/core/or/circuitbuild.h +++ b/src/core/or/circuitbuild.h @@ -80,10 +80,10 @@ channel_connect_for_circuit,(const tor_addr_t *addr, const struct ed25519_public_key_t *ed_id)); struct create_cell_t; -int -circuit_deliver_create_cell(circuit_t *circ, - const struct create_cell_t *create_cell, - int relayed); +MOCK_DECL(int, +circuit_deliver_create_cell,(circuit_t *circ, + const struct create_cell_t *create_cell, + int relayed)); #ifdef CIRCUITBUILD_PRIVATE STATIC circid_t get_unique_circ_id_by_chan(channel_t *chan); diff --git a/src/core/or/onion.c b/src/core/or/onion.c index 4a5b296b9..45144b5e6 100644 --- a/src/core/or/onion.c +++ b/src/core/or/onion.c @@ -374,9 +374,11 @@ extend_cell_from_extend2_cell_body(extend_cell_t *cell_out, /** Parse an EXTEND or EXTEND2 cell (according to command) from the * payload_length bytes of payload into cell_out. Return * 0 on success, -1 on failure. */ -int -extend_cell_parse(extend_cell_t *cell_out, const uint8_t command, - const uint8_t *payload, size_t payload_length) +MOCK_IMPL(int, +extend_cell_parse,(extend_cell_t *cell_out, + const uint8_t command, + const uint8_t *payload, + size_t payload_length)) { tor_assert(cell_out); diff --git a/src/core/or/onion.h b/src/core/or/onion.h index ff3083f37..72470d1cb 100644 --- a/src/core/or/onion.h +++ b/src/core/or/onion.h @@ -74,8 +74,10 @@ void create_cell_init(create_cell_t *cell_out, uint8_t cell_type, const uint8_t *onionskin); int create_cell_parse(create_cell_t *cell_out, const cell_t *cell_in); int created_cell_parse(created_cell_t *cell_out, const cell_t *cell_in); -int extend_cell_parse(extend_cell_t *cell_out, const uint8_t command, - const uint8_t *payload_in, size_t payload_len); +MOCK_DECL(int,extend_cell_parse,(extend_cell_t *cell_out, + const uint8_t command, + const uint8_t *payload_in, + size_t payload_len)); int extended_cell_parse(extended_cell_t *cell_out, const uint8_t command, const uint8_t *payload_in, size_t payload_len);

1 0

[tor/master] changes: file for 33633
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit aa560a5ade65ea7a2dc1eab3bf7d82d805c8d785 Author: teor <teor(a)torproject.org> Date: Wed Mar 18 19:15:14 2020 +1000 changes: file for 33633 --- changes/ticket33633 | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/changes/ticket33633 b/changes/ticket33633 new file mode 100644 index 000000000..de030a600 --- /dev/null +++ b/changes/ticket33633 @@ -0,0 +1,6 @@ + o Code simplification and refactoring: + - Move the circuit extend code to the relay module. + Split the circuit extend function into smaller functions. + Closes ticket 33633. + - Move LOG_PROTOCOL_WARN to app/config.c. Resolves a dependency inversion. + Closes ticket 33633.

1 0

[tor/master] channel: Rewrite the channel_get_for_extend() comments
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit 8f3cbe755bc6817a164fe59b5ed1916a1c71fb5b Author: teor <teor(a)torproject.org> Date: Mon Apr 6 19:11:17 2020 +1000 channel: Rewrite the channel_get_for_extend() comments Explain what the function does now. Fix some typos. Part of 33633. --- src/core/or/channel.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/core/or/channel.c b/src/core/or/channel.c index 160ab587f..542b4d88b 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -2360,12 +2360,15 @@ channel_is_better(channel_t *a, channel_t *b) /** * Get a channel to extend a circuit. * - * Pick a suitable channel to extend a circuit to given the desired digest - * the address we believe is correct for that digest; this tries to see - * if we already have one for the requested endpoint, but if there is no good - * channel, set *msg_out to a message describing the channel's state - * and our next action, and set *launch_out to a boolean indicated whether - * the caller should try to launch a new channel with channel_connect(). + * Given the desired relay identity, pick a suitable channel to extend a + * circuit to the target address requsted by the client. Search for an + * existing channel for the requested endpoint. Make sure the channel is + * usable for new circuits, and matches the target address. + * + * Try to return the best channel. But if there is no good channel, set + * *msg_out to a message describing the channel's state and our next action, + * and set *launch_out to a boolean indicated whether the caller should try to + * launch a new channel with channel_connect(). */ channel_t * channel_get_for_extend(const char *rsa_id_digest, @@ -2383,9 +2386,7 @@ channel_get_for_extend(const char *rsa_id_digest, chan = channel_find_by_remote_identity(rsa_id_digest, ed_id); - /* Walk the list, unrefing the old one and refing the new at each - * iteration. - */ + /* Walk the list of channels */ for (; chan; chan = channel_next_with_rsa_identity(chan)) { tor_assert(tor_memeq(chan->identity_digest, rsa_id_digest, DIGEST_LEN));

1 0

[tor/master] test/circuitbuild: Add a test for onionskin_answer()
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit 00a45a900ed4784b2a1110fef3a2e5eefb4b049a Author: teor <teor(a)torproject.org> Date: Tue Apr 7 18:26:57 2020 +1000 test/circuitbuild: Add a test for onionskin_answer() Part of 33633. --- src/test/test_circuitbuild.c | 80 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index 7a95c3719..6934cf3d7 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -1154,6 +1154,83 @@ test_circuit_extend(void *arg) tor_free(fake_n_chan); } +/* Test the different cases in onionskin_answer(). */ +static void +test_onionskin_answer(void *arg) +{ + (void)arg; + created_cell_t *created_cell = tor_malloc_zero(sizeof(created_cell_t)); + or_circuit_t *or_circ = tor_malloc_zero(sizeof(or_circuit_t)); + char keys[CPATH_KEY_MATERIAL_LEN] = {0}; + uint8_t rend_circ_nonce[DIGEST_LEN] = {0}; + + setup_full_capture_of_logs(LOG_INFO); + + /* Circuit must be non-NULL */ + tor_capture_bugs_(1); + tt_int_op(onionskin_answer(NULL, created_cell, + keys, CPATH_KEY_MATERIAL_LEN, + rend_circ_nonce), OP_EQ, -1); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); + tt_str_op(smartlist_get(tor_get_captured_bug_log_(), 0), OP_EQ, + "!(ASSERT_PREDICT_UNLIKELY_(!circ))"); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + + /* Created cell must be non-NULL */ + tor_capture_bugs_(1); + tt_int_op(onionskin_answer(or_circ, NULL, + keys, CPATH_KEY_MATERIAL_LEN, + rend_circ_nonce), OP_EQ, -1); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); + tt_str_op(smartlist_get(tor_get_captured_bug_log_(), 0), OP_EQ, + "!(ASSERT_PREDICT_UNLIKELY_(!created_cell))"); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + + /* Keys must be non-NULL */ + tor_capture_bugs_(1); + tt_int_op(onionskin_answer(or_circ, created_cell, + NULL, CPATH_KEY_MATERIAL_LEN, + rend_circ_nonce), OP_EQ, -1); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); + tt_str_op(smartlist_get(tor_get_captured_bug_log_(), 0), OP_EQ, + "!(ASSERT_PREDICT_UNLIKELY_(!keys))"); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + + /* The rend circuit nonce must be non-NULL */ + tor_capture_bugs_(1); + tt_int_op(onionskin_answer(or_circ, created_cell, + keys, CPATH_KEY_MATERIAL_LEN, + NULL), OP_EQ, -1); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); + tt_str_op(smartlist_get(tor_get_captured_bug_log_(), 0), OP_EQ, + "!(ASSERT_PREDICT_UNLIKELY_(!rend_circ_nonce))"); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + + /* Also, the keys length must be CPATH_KEY_MATERIAL_LEN, but we can't catch + * asserts in unit tests. */ + + /* Fail when formatting the created cell */ + tt_int_op(onionskin_answer(or_circ, created_cell, + keys, CPATH_KEY_MATERIAL_LEN, + rend_circ_nonce), OP_EQ, -1); + expect_log_msg("couldn't format created cell (type=0, len=0).\n"); + mock_clean_saved_logs(); + + /* TODO: test the rest of onionskin_answer(), see #33860 */ + /* TODO: mock created_cell_format for the next test */ + + done: + tor_end_capture_bugs_(); + teardown_capture_of_logs(); + + tor_free(created_cell); + tor_free(or_circ); +} + #define TEST(name, flags, setup, cleanup) \ { #name, test_ ## name, flags, setup, cleanup } @@ -1176,5 +1253,8 @@ struct testcase_t circuitbuild_tests[] = { TEST_CIRCUIT(extend_lspec_valid, TT_FORK), TEST_CIRCUIT(open_connection_for_extend, TT_FORK), TEST_CIRCUIT(extend, TT_FORK), + + TEST(onionskin_answer, TT_FORK, NULL, NULL), + END_OF_TESTCASES };

1 0

[tor/master] relay/circuitbuild: Refactor circuit_extend()
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit d79e5d52bc6bfa5550614e58c1700885912fcff3 Author: teor <teor(a)torproject.org> Date: Mon Apr 6 19:21:13 2020 +1000 relay/circuitbuild: Refactor circuit_extend() Make the "else" case explicit at the end of the function. Part of 33633. --- src/feature/relay/circuitbuild_relay.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c index ae8581075..2fa92eeac 100644 --- a/src/feature/relay/circuitbuild_relay.c +++ b/src/feature/relay/circuitbuild_relay.c @@ -295,18 +295,20 @@ circuit_extend(struct cell_t *cell, struct circuit_t *circ) * OR_CONN_STATE_OPEN. */ return 0; - } - - tor_assert(!circ->n_hop); /* Connection is already established. */ - circ->n_chan = n_chan; - log_debug(LD_CIRC, - "n_chan is %s.", - channel_get_canonical_remote_descr(n_chan)); - - if (circuit_deliver_create_cell(circ, &ec.create_cell, 1) < 0) - return -1; + } else { + /* Connection is already established. + * So we need to extend the circuit to the next hop. */ + tor_assert(!circ->n_hop); + circ->n_chan = n_chan; + log_debug(LD_CIRC, + "n_chan is %s.", + channel_get_canonical_remote_descr(n_chan)); + + if (circuit_deliver_create_cell(circ, &ec.create_cell, 1) < 0) + return -1; - return 0; + return 0; + } } /** On a relay, accept a create cell, initialise a circuit, and send a

1 0

[tor/master] relay: Run practracker --regen
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit 75aba30abf50b918ef2188354176bbc2ae59c99c Author: teor <teor(a)torproject.org> Date: Tue Mar 17 17:45:49 2020 +1000 relay: Run practracker --regen Lock in some file and function size improvements. Accept some small file size and dependency increases. Part of 33633. --- scripts/maint/practracker/exceptions.txt | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index 389ecbe0c..128e5f74f 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -83,9 +83,9 @@ problem dependency-violation /src/core/mainloop/netstatus.c 4 problem dependency-violation /src/core/mainloop/periodic.c 2 problem dependency-violation /src/core/or/address_set.c 1 problem dependency-violation /src/core/or/cell_queue_st.h 1 -problem file-size /src/core/or/channel.c 3464 +problem file-size /src/core/or/channel.c 3500 problem dependency-violation /src/core/or/channel.c 9 -problem file-size /src/core/or/channel.h 775 +problem file-size /src/core/or/channel.h 800 problem dependency-violation /src/core/or/channel.h 1 problem dependency-violation /src/core/or/channelpadding.c 6 problem function-size /src/core/or/channeltls.c:channel_tls_handle_var_cell() 160 @@ -94,9 +94,8 @@ problem function-size /src/core/or/channeltls.c:channel_tls_process_netinfo_cell problem function-size /src/core/or/channeltls.c:channel_tls_process_certs_cell() 246 problem function-size /src/core/or/channeltls.c:channel_tls_process_authenticate_cell() 202 problem dependency-violation /src/core/or/channeltls.c 11 -problem include-count /src/core/or/circuitbuild.c 54 +problem include-count /src/core/or/circuitbuild.c 53 problem function-size /src/core/or/circuitbuild.c:get_unique_circ_id_by_chan() 128 -problem function-size /src/core/or/circuitbuild.c:circuit_extend() 147 problem function-size /src/core/or/circuitbuild.c:choose_good_exit_server_general() 206 problem dependency-violation /src/core/or/circuitbuild.c 25 problem include-count /src/core/or/circuitlist.c 55 @@ -129,7 +128,7 @@ problem function-size /src/core/or/circuituse.c:connection_ap_handshake_attach_c problem dependency-violation /src/core/or/circuituse.c 24 problem function-size /src/core/or/command.c:command_process_create_cell() 156 problem function-size /src/core/or/command.c:command_process_relay_cell() 132 -problem dependency-violation /src/core/or/command.c 8 +problem dependency-violation /src/core/or/command.c 9 problem file-size /src/core/or/connection_edge.c 4655 problem include-count /src/core/or/connection_edge.c 65 problem function-size /src/core/or/connection_edge.c:connection_ap_expire_beginning() 117 @@ -147,7 +146,7 @@ problem function-size /src/core/or/connection_or.c:connection_or_client_learned_ problem dependency-violation /src/core/or/connection_or.c 21 problem dependency-violation /src/core/or/dos.c 6 problem dependency-violation /src/core/or/onion.c 2 -problem file-size /src/core/or/or.h 1107 +problem file-size /src/core/or/or.h 1105 problem include-count /src/core/or/or.h 48 problem dependency-violation /src/core/or/or.h 1 problem dependency-violation /src/core/or/or_periodic.c 1 @@ -156,7 +155,7 @@ problem function-size /src/core/or/policies.c:policy_summarize() 107 problem dependency-violation /src/core/or/policies.c 14 problem function-size /src/core/or/protover.c:protover_all_supported() 117 problem dependency-violation /src/core/or/reasons.c 2 -problem file-size /src/core/or/relay.c 3264 +problem file-size /src/core/or/relay.c 3300 problem function-size /src/core/or/relay.c:circuit_receive_relay_cell() 127 problem function-size /src/core/or/relay.c:relay_send_command_from_edge_() 109 problem function-size /src/core/or/relay.c:connection_ap_process_end_not_open() 192 @@ -164,7 +163,7 @@ problem function-size /src/core/or/relay.c:connection_edge_process_relay_cell_no problem function-size /src/core/or/relay.c:handle_relay_cell_command() 369 problem function-size /src/core/or/relay.c:connection_edge_package_raw_inbuf() 128 problem function-size /src/core/or/relay.c:circuit_resume_edge_reading_helper() 146 -problem dependency-violation /src/core/or/relay.c 16 +problem dependency-violation /src/core/or/relay.c 17 problem dependency-violation /src/core/or/scheduler.c 1 problem function-size /src/core/or/scheduler_kist.c:kist_scheduler_run() 171 problem dependency-violation /src/core/or/scheduler_kist.c 2

1 0