commit b65f8c419a4eb2608beecbf31af0b5bdc6cc38ec Author: Neel Chauhan <neel@neelc.org> Date: Sun Nov 4 20:08:57 2018 -0500 Add firewall_choose_address_ls() and hs_get_extend_info_from_lspecs() tests --- src/core/or/policies.c | 19 ++++ src/feature/hs/hs_common.c | 20 +++- src/test/test_policy.c | 235 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 271 insertions(+), 3 deletions(-) diff --git a/src/core/or/policies.c b/src/core/or/policies.c index b58a5c9cb..83d9a53fc 100644 --- a/src/core/or/policies.c +++ b/src/core/or/policies.c @@ -1031,6 +1031,15 @@ fascist_firewall_choose_address_ls(const smartlist_t *lspecs, tor_assert(ap); + if (lspecs == NULL) { + log_warn(LD_BUG, "Unknown or missing link specifiers"); + return; + } + if (smartlist_len(lspecs) == 0) { + log_warn(LD_PROTOCOL, "Link specifiers are empty"); + return; + } + tor_addr_make_null(&ap->addr, AF_UNSPEC); ap->port = 0; @@ -1062,6 +1071,16 @@ fascist_firewall_choose_address_ls(const smartlist_t *lspecs, } } SMARTLIST_FOREACH_END(ls); + /* If we don't have IPv4 or IPv6 in link specifiers, log a bug and return. */ + if (!have_v4 && !have_v6) { + if (!have_v6) { + log_warn(LD_PROTOCOL, "None of our link specifiers have IPv4 or IPv6"); + } else { + log_warn(LD_PROTOCOL, "None of our link specifiers have IPv4"); + } + return; + } + /* Here, don't check for DirPorts as link specifiers are only used for * ORPorts. */ const or_options_t *options = get_options(); diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c index ca7e991b7..9b24129b6 100644 --- a/src/feature/hs/hs_common.c +++ b/src/feature/hs/hs_common.c @@ -1697,7 +1697,15 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, tor_addr_make_null(&ap.addr, AF_UNSPEC); ap.port = 0; - tor_assert(lspecs); + if (lspecs == NULL) { + log_warn(LD_BUG, "Specified link specifiers is null"); + goto done; + } + + if (onion_key == NULL) { + log_warn(LD_BUG, "Specified onion key is null"); + goto done; + } if (smartlist_len(lspecs) == 0) { log_fn(LOG_PROTOCOL_WARN, LD_REND, "Empty link specifier list."); @@ -1744,8 +1752,14 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, fascist_firewall_choose_address_ls(lspecs, 0, &ap); /* Legacy ID is mandatory, and we require an IP address. */ - if (!tor_addr_port_is_valid_ap(&ap, 0) || !have_legacy_id) { - /* If we're missing the legacy ID or the IP address, return NULL. */ + if (!tor_addr_port_is_valid_ap(&ap, 0)) { + /* If we're missing the IP address, log a warning and return NULL. */ + log_info(LD_NET, "Unreachable or invalid IP address in link state"); + goto done; + } + if (!have_legacy_id) { + /* If we're missing the legacy ID, log a warning and return NULL. */ + log_warn(LD_PROTOCOL, "Missing Legacy ID in link state"); goto done; } diff --git a/src/test/test_policy.c b/src/test/test_policy.c index 46d4a1b94..e58bb3d17 100644 --- a/src/test/test_policy.c +++ b/src/test/test_policy.c @@ -6,13 +6,18 @@ #include "core/or/or.h" #include "app/config/config.h" +#include "core/or/circuitbuild.h" #include "core/or/policies.h" #include "feature/dirparse/policy_parse.h" +#include "feature/hs/hs_common.h" +#include "feature/hs/hs_descriptor.h" #include "feature/relay/router.h" #include "lib/encoding/confline.h" #include "test/test.h" +#include "test/log_test_helpers.h" #include "core/or/addr_policy_st.h" +#include "core/or/extend_info_st.h" #include "core/or/port_cfg_st.h" #include "feature/nodelist/node_st.h" #include "feature/nodelist/routerinfo_st.h" @@ -2024,6 +2029,101 @@ test_policies_fascist_firewall_allows_address(void *arg) expect_ap); \ STMT_END +/* Check that fascist_firewall_choose_address_ls() returns the expected + * results. */ +#define CHECK_CHOSEN_ADDR_NULL_LS() \ + STMT_BEGIN \ + tor_addr_port_t chosen_ls_ap; \ + tor_addr_make_null(&chosen_ls_ap.addr, AF_UNSPEC); \ + chosen_ls_ap.port = 0; \ + setup_full_capture_of_logs(LOG_WARN); \ + fascist_firewall_choose_address_ls(NULL, 1, &chosen_ls_ap); \ + expect_single_log_msg("Unknown or missing link specifiers"); \ + teardown_capture_of_logs(); \ + STMT_END + +#define CHECK_CHOSEN_ADDR_LS(fake_ls, pref_only, expect_rv, expect_ap) \ + STMT_BEGIN \ + tor_addr_port_t chosen_ls_ap; \ + tor_addr_make_null(&chosen_ls_ap.addr, AF_UNSPEC); \ + chosen_ls_ap.port = 0; \ + setup_full_capture_of_logs(LOG_WARN); \ + fascist_firewall_choose_address_ls(fake_ls, pref_only, &chosen_ls_ap); \ + if (smartlist_len(fake_ls) == 0) { \ + expect_single_log_msg("Link specifiers are empty"); \ + } else { \ + expect_no_log_entry(); \ + tt_assert(tor_addr_eq(&(expect_ap).addr, &chosen_ls_ap.addr)); \ + tt_int_op((expect_ap).port, OP_EQ, chosen_ls_ap.port); \ + } \ + teardown_capture_of_logs(); \ + STMT_END + +#define CHECK_LS_LEGACY_ONLY(fake_ls) \ + STMT_BEGIN \ + tor_addr_port_t chosen_ls_ap; \ + tor_addr_make_null(&chosen_ls_ap.addr, AF_UNSPEC); \ + chosen_ls_ap.port = 0; \ + setup_full_capture_of_logs(LOG_WARN); \ + fascist_firewall_choose_address_ls(fake_ls, 0, &chosen_ls_ap); \ + expect_single_log_msg("None of our link specifiers have IPv4 or IPv6"); \ + teardown_capture_of_logs(); \ + STMT_END + +#define CHECK_HS_EXTEND_INFO_ADDR_LS(fake_ls, direct_conn, expect_ap) \ + STMT_BEGIN \ + curve25519_secret_key_t seckey; \ + curve25519_secret_key_generate(&seckey, 0); \ + curve25519_public_key_t pubkey; \ + curve25519_public_key_generate(&pubkey, &seckey); \ + setup_full_capture_of_logs(LOG_WARN); \ + extend_info_t *ei = hs_get_extend_info_from_lspecs(fake_ls, &pubkey, \ + direct_conn); \ + if (fake_ls == NULL) { \ + tt_ptr_op(ei, OP_EQ, NULL); \ + expect_single_log_msg("Specified link specifiers is null"); \ + } else { \ + expect_no_log_entry(); \ + tt_assert(tor_addr_eq(&(expect_ap).addr, &ei->addr)); \ + tt_int_op((expect_ap).port, OP_EQ, ei->port); \ + extend_info_free(ei); \ + } \ + teardown_capture_of_logs(); \ + STMT_END + +#define CHECK_HS_EXTEND_INFO_ADDR_LS_NULL_KEY(fake_ls) \ + STMT_BEGIN \ + setup_full_capture_of_logs(LOG_WARN); \ + extend_info_t *ei = hs_get_extend_info_from_lspecs(fake_ls, NULL, 0); \ + tt_ptr_op(ei, OP_EQ, NULL); \ + expect_single_log_msg("Specified onion key is null"); \ + teardown_capture_of_logs(); \ + STMT_END + +#define CHECK_HS_EXTEND_INFO_ADDR_LS_EXPECT_NULL(fake_ls, direct_conn) \ + STMT_BEGIN \ + curve25519_secret_key_t seckey; \ + curve25519_secret_key_generate(&seckey, 0); \ + curve25519_public_key_t pubkey; \ + curve25519_public_key_generate(&pubkey, &seckey); \ + extend_info_t *ei = hs_get_extend_info_from_lspecs(fake_ls, &pubkey, \ + direct_conn); \ + tt_ptr_op(ei, OP_EQ, NULL); \ + STMT_END + +#define CHECK_HS_EXTEND_INFO_ADDR_LS_EXPECT_MSG(fake_ls, msg_level, msg) \ + STMT_BEGIN \ + curve25519_secret_key_t seckey; \ + curve25519_secret_key_generate(&seckey, 0); \ + curve25519_public_key_t pubkey; \ + curve25519_public_key_generate(&pubkey, &seckey); \ + setup_full_capture_of_logs(msg_level); \ + extend_info_t *ei = hs_get_extend_info_from_lspecs(fake_ls, &pubkey, 0); \ + tt_ptr_op(ei, OP_EQ, NULL); \ + expect_single_log_msg(msg); \ + teardown_capture_of_logs(); \ + STMT_END + /** Mock the preferred address function to return zero (prefer IPv4). */ static int mock_fascist_firewall_rand_prefer_ipv6_addr_use_ipv4(void) @@ -2472,6 +2572,141 @@ test_policies_fascist_firewall_choose_address(void *arg) UNMOCK(fascist_firewall_rand_prefer_ipv6_addr); + /* Test firewall_choose_address_ls(). To do this, we make a fake link + * specifier. */ + smartlist_t *lspecs = smartlist_new(), + *lspecs_blank = smartlist_new(), + *lspecs_v4 = smartlist_new(), + *lspecs_v6 = smartlist_new(), + *lspecs_no_legacy = smartlist_new(), + *lspecs_legacy_only = smartlist_new(); + link_specifier_t *fake_ls; + + /* IPv4 link specifier */ + fake_ls = link_specifier_new(); + link_specifier_set_ls_type(fake_ls, LS_IPV4); + link_specifier_set_un_ipv4_addr(fake_ls, + tor_addr_to_ipv4h(&ipv4_or_ap.addr)); + link_specifier_set_un_ipv4_port(fake_ls, ipv4_or_ap.port); + link_specifier_set_ls_len(fake_ls, sizeof(ipv4_or_ap.addr.addr.in_addr) + + sizeof(ipv4_or_ap.port)); + smartlist_add(lspecs, fake_ls); + smartlist_add(lspecs_v4, fake_ls); + smartlist_add(lspecs_no_legacy, fake_ls); + + /* IPv6 link specifier */ + fake_ls = link_specifier_new(); + link_specifier_set_ls_type(fake_ls, LS_IPV6); + size_t addr_len = link_specifier_getlen_un_ipv6_addr(fake_ls); + const uint8_t *in6_addr = tor_addr_to_in6_addr8(&ipv6_or_ap.addr); + uint8_t *ipv6_array = link_specifier_getarray_un_ipv6_addr(fake_ls); + memcpy(ipv6_array, in6_addr, addr_len); + link_specifier_set_un_ipv6_port(fake_ls, ipv6_or_ap.port); + link_specifier_set_ls_len(fake_ls, addr_len + sizeof(ipv6_or_ap.port)); + smartlist_add(lspecs, fake_ls); + smartlist_add(lspecs_v6, fake_ls); + + /* Legacy ID link specifier */ + fake_ls = link_specifier_new(); + link_specifier_set_ls_type(fake_ls, LS_LEGACY_ID); + uint8_t *legacy_id = link_specifier_getarray_un_legacy_id(fake_ls); + memset(legacy_id, 'A', sizeof(*legacy_id)); + link_specifier_set_ls_len(fake_ls, + link_specifier_getlen_un_legacy_id(fake_ls)); + smartlist_add(lspecs, fake_ls); + smartlist_add(lspecs_legacy_only, fake_ls); + smartlist_add(lspecs_v4, fake_ls); + smartlist_add(lspecs_v6, fake_ls); + + /* Check with bogus requests. */ + tor_addr_port_t null_ap; \ + tor_addr_make_null(&null_ap.addr, AF_UNSPEC); \ + null_ap.port = 0; \ + + /* Check for a null link state. */ + CHECK_CHOSEN_ADDR_NULL_LS(); + CHECK_HS_EXTEND_INFO_ADDR_LS(NULL, 1, null_ap); + + /* Check for a blank link state. */ + CHECK_CHOSEN_ADDR_LS(lspecs_blank, 0, 0, null_ap); + CHECK_HS_EXTEND_INFO_ADDR_LS_EXPECT_NULL(lspecs_blank, 0); + + /* Check for a link state with only a Legacy ID. */ + CHECK_LS_LEGACY_ONLY(lspecs_legacy_only); + CHECK_HS_EXTEND_INFO_ADDR_LS_EXPECT_NULL(lspecs_legacy_only, 0); + smartlist_free(lspecs_legacy_only); + + /* Check with a null onion_key. */ + CHECK_HS_EXTEND_INFO_ADDR_LS_NULL_KEY(lspecs_blank); + smartlist_free(lspecs_blank); + + /* Check with a null onion_key. */ + CHECK_HS_EXTEND_INFO_ADDR_LS_EXPECT_MSG(lspecs_no_legacy, LOG_WARN, + "Missing Legacy ID in link state"); + smartlist_free(lspecs_no_legacy); + + /* Enable both IPv4 and IPv6. */ + memset(&mock_options, 0, sizeof(or_options_t)); + mock_options.ClientUseIPv4 = 1; + mock_options.ClientUseIPv6 = 1; + + /* Prefer IPv4, enable both IPv4 and IPv6. */ + mock_options.ClientPreferIPv6ORPort = 0; + + CHECK_CHOSEN_ADDR_LS(lspecs, 0, 1, ipv4_or_ap); + CHECK_CHOSEN_ADDR_LS(lspecs, 1, 1, ipv4_or_ap); + + CHECK_HS_EXTEND_INFO_ADDR_LS(lspecs, 1, ipv4_or_ap); + CHECK_HS_EXTEND_INFO_ADDR_LS(lspecs, 0, ipv4_or_ap); + + /* Prefer IPv6, enable both IPv4 and IPv6. */ + mock_options.ClientPreferIPv6ORPort = 1; + + CHECK_CHOSEN_ADDR_LS(lspecs, 0, 1, ipv6_or_ap); + CHECK_CHOSEN_ADDR_LS(lspecs, 1, 1, ipv6_or_ap); + + CHECK_HS_EXTEND_INFO_ADDR_LS(lspecs, 1, ipv6_or_ap); + CHECK_HS_EXTEND_INFO_ADDR_LS(lspecs, 0, ipv4_or_ap); + + /* IPv4-only. */ + memset(&mock_options, 0, sizeof(or_options_t)); + mock_options.ClientUseIPv4 = 1; + mock_options.ClientUseIPv6 = 0; + + CHECK_CHOSEN_ADDR_LS(lspecs, 0, 1, ipv4_or_ap); + CHECK_CHOSEN_ADDR_LS(lspecs, 1, 1, ipv4_or_ap); + + CHECK_CHOSEN_ADDR_LS(lspecs_v6, 0, 0, null_ap); + + CHECK_HS_EXTEND_INFO_ADDR_LS(lspecs, 1, ipv4_or_ap); + CHECK_HS_EXTEND_INFO_ADDR_LS(lspecs, 0, ipv4_or_ap); + + CHECK_HS_EXTEND_INFO_ADDR_LS_EXPECT_NULL(lspecs_v6, 0); + CHECK_HS_EXTEND_INFO_ADDR_LS_EXPECT_NULL(lspecs_v6, 1); + + /* IPv6-only. */ + memset(&mock_options, 0, sizeof(or_options_t)); + mock_options.ClientUseIPv4 = 0; + mock_options.ClientUseIPv6 = 1; + + CHECK_CHOSEN_ADDR_LS(lspecs, 0, 1, ipv6_or_ap); + CHECK_CHOSEN_ADDR_LS(lspecs, 1, 1, ipv6_or_ap); + + CHECK_CHOSEN_ADDR_LS(lspecs_v4, 0, 0, null_ap); + + CHECK_HS_EXTEND_INFO_ADDR_LS(lspecs, 1, ipv6_or_ap); + CHECK_HS_EXTEND_INFO_ADDR_LS(lspecs, 0, ipv4_or_ap); + + CHECK_HS_EXTEND_INFO_ADDR_LS_EXPECT_NULL(lspecs_v4, 1); + CHECK_HS_EXTEND_INFO_ADDR_LS_EXPECT_NULL(lspecs_v6, 0); + + smartlist_free(lspecs_v4); + smartlist_free(lspecs_v6); + + SMARTLIST_FOREACH(lspecs, link_specifier_t *, lspec, \ + link_specifier_free(lspec)); \ + smartlist_free(lspecs); + done: UNMOCK(get_options); }