From da268e3b508fb5f9ab3b930deded331c387f7243 Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Tue, 31 Jul 2018 08:17:59 -0400 Subject: [PATCH 1/8] Add function fascist_firewall_choose_address_ls() --- src/core/or/policies.c | 55 ++++++++++++++++++++++++++++++++++++++++++ src/core/or/policies.h | 3 +++ 2 files changed, 58 insertions(+) diff --git a/src/core/or/policies.c b/src/core/or/policies.c index f59894ea8f..26a053821a 100644 --- a/src/core/or/policies.c +++ b/src/core/or/policies.c @@ -31,6 +31,7 @@ #include "ht.h" #include "lib/crypt_ops/crypto_rand.h" #include "lib/encoding/confline.h" +#include "trunnel/ed25519_cert.h" #include "core/or/addr_policy_st.h" #include "feature/dirclient/dir_server_st.h" @@ -1015,6 +1016,60 @@ fascist_firewall_choose_address_rs(const routerstatus_t *rs, } } +/** Like fascist_firewall_choose_address_base(), but takes in a smartlist + * lspecs consisting of one or more link specifiers. + */ +void +fascist_firewall_choose_address_ls(const smartlist_t *lspecs, + int pref_only, tor_addr_port_t* ap, + int direct_conn) +{ + int have_v4 = 0, have_v6 = 0; + uint16_t port_v4 = 0, port_v6 = 0; + tor_addr_t addr_v4, addr_v6; + + SMARTLIST_FOREACH_BEGIN(lspecs, const link_specifier_t *, ls) { + switch (link_specifier_get_ls_type(ls)) { + case LS_IPV4: + /* Skip if we already seen a v4. */ + if (have_v4) continue; + tor_addr_from_ipv4h(&addr_v4, + link_specifier_get_un_ipv4_addr(ls)); + port_v4 = link_specifier_get_un_ipv4_port(ls); + have_v4 = 1; + break; + case LS_IPV6: + /* Skip if we already seen a v6, or deliberately skip it if we're not a + * direct connection. */ + if (have_v6 || !direct_conn) continue; + tor_addr_from_ipv6_bytes(&addr_v6, + (const char *) link_specifier_getconstarray_un_ipv6_addr(ls)); + port_v6 = link_specifier_get_un_ipv6_port(ls); + have_v6 = 1; + break; + default: + /* Ignore unknown. */ + break; + } + } SMARTLIST_FOREACH_END(ls); + + tor_assert(ap); + + tor_addr_make_null(&ap->addr, AF_UNSPEC); + ap->port = 0; + + /* Here, don't check for DirPorts as link specifiers are only used for + * ORPorts. */ + const or_options_t *options = get_options(); + int pref_ipv6 = fascist_firewall_prefer_ipv6_orport(options); + /* Assume that the DirPorts are zero as link specifiers only use ORPorts. */ + fascist_firewall_choose_address_base(&addr_v4, port_v4, 0, + &addr_v6, port_v6, 0, + FIREWALL_OR_CONNECTION, + pref_only, pref_ipv6, + ap); +} + /** Like fascist_firewall_choose_address_base(), but takes node, and * looks up the node's IPv6 preference rather than taking an argument * for pref_ipv6. */ diff --git a/src/core/or/policies.h b/src/core/or/policies.h index 324c1c2dd1..f4c68fd952 100644 --- a/src/core/or/policies.h +++ b/src/core/or/policies.h @@ -92,6 +92,9 @@ int fascist_firewall_allows_dir_server(const dir_server_t *ds, void fascist_firewall_choose_address_rs(const routerstatus_t *rs, firewall_connection_t fw_connection, int pref_only, tor_addr_port_t* ap); +void fascist_firewall_choose_address_ls(const smartlist_t *lspecs, + int pref_only, tor_addr_port_t* ap, + int direct_conn); void fascist_firewall_choose_address_node(const node_t *node, firewall_connection_t fw_connection, int pref_only, tor_addr_port_t* ap); From 26183476575abf0f8daccc2f4ca8be4ba0e2a5de Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Tue, 31 Jul 2018 08:41:21 -0400 Subject: [PATCH 2/8] Use fascist_firewall_choose_address_ls() in hs_get_extend_info_from_lspecs() --- changes/bug23588 | 7 ++++ src/core/or/policies.c | 22 ++++++----- src/core/or/policies.h | 3 +- src/feature/hs/hs_common.c | 78 +++++++++++++++----------------------- 4 files changed, 52 insertions(+), 58 deletions(-) create mode 100644 changes/bug23588 diff --git a/changes/bug23588 b/changes/bug23588 new file mode 100644 index 0000000000..754d083614 --- /dev/null +++ b/changes/bug23588 @@ -0,0 +1,7 @@ + o Minor bugfixes (address selection): + - Introduce fascist_firewall_choose_address_ls() which chooses an + IPv6 or IPv4 address based on a smartlist of link specifiers of + what is available and what we prefer. We use this function in + hs_get_extend_info_from_lspecs(). Fixes bug 23588; bugfix on + 0.3.5.1-alpha. Patch by Neel Chauhan. + diff --git a/src/core/or/policies.c b/src/core/or/policies.c index 26a053821a..b58a5c9cb6 100644 --- a/src/core/or/policies.c +++ b/src/core/or/policies.c @@ -1017,17 +1017,26 @@ fascist_firewall_choose_address_rs(const routerstatus_t *rs, } /** Like fascist_firewall_choose_address_base(), but takes in a smartlist - * lspecs consisting of one or more link specifiers. + * lspecs consisting of one or more link specifiers. We assume + * fw_connection is FIREWALL_OR_CONNECTION as link specifiers cannot + * contain DirPorts. */ void fascist_firewall_choose_address_ls(const smartlist_t *lspecs, - int pref_only, tor_addr_port_t* ap, - int direct_conn) + int pref_only, tor_addr_port_t* ap) { int have_v4 = 0, have_v6 = 0; uint16_t port_v4 = 0, port_v6 = 0; tor_addr_t addr_v4, addr_v6; + tor_assert(ap); + + tor_addr_make_null(&ap->addr, AF_UNSPEC); + ap->port = 0; + + tor_addr_make_null(&addr_v4, AF_INET); + tor_addr_make_null(&addr_v6, AF_INET6); + SMARTLIST_FOREACH_BEGIN(lspecs, const link_specifier_t *, ls) { switch (link_specifier_get_ls_type(ls)) { case LS_IPV4: @@ -1041,7 +1050,7 @@ fascist_firewall_choose_address_ls(const smartlist_t *lspecs, case LS_IPV6: /* Skip if we already seen a v6, or deliberately skip it if we're not a * direct connection. */ - if (have_v6 || !direct_conn) continue; + if (have_v6) continue; tor_addr_from_ipv6_bytes(&addr_v6, (const char *) link_specifier_getconstarray_un_ipv6_addr(ls)); port_v6 = link_specifier_get_un_ipv6_port(ls); @@ -1053,11 +1062,6 @@ fascist_firewall_choose_address_ls(const smartlist_t *lspecs, } } SMARTLIST_FOREACH_END(ls); - tor_assert(ap); - - tor_addr_make_null(&ap->addr, AF_UNSPEC); - ap->port = 0; - /* 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/core/or/policies.h b/src/core/or/policies.h index f4c68fd952..3c46363c04 100644 --- a/src/core/or/policies.h +++ b/src/core/or/policies.h @@ -93,8 +93,7 @@ void fascist_firewall_choose_address_rs(const routerstatus_t *rs, firewall_connection_t fw_connection, int pref_only, tor_addr_port_t* ap); void fascist_firewall_choose_address_ls(const smartlist_t *lspecs, - int pref_only, tor_addr_port_t* ap, - int direct_conn); + int pref_only, tor_addr_port_t* ap); void fascist_firewall_choose_address_node(const node_t *node, firewall_connection_t fw_connection, int pref_only, tor_addr_port_t* ap); diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c index d4736c2862..ca7e991b75 100644 --- a/src/feature/hs/hs_common.c +++ b/src/feature/hs/hs_common.c @@ -1672,14 +1672,16 @@ hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str, return hs_dir; } -/* From a list of link specifier, an onion key and if we are requesting a - * direct connection (ex: single onion service), return a newly allocated - * extend_info_t object. This function always returns an extend info with - * an IPv4 address, or NULL. +/* Given a list of link specifiers lspecs, a curve 25519 onion_key, and + * a direct connection boolean direct_conn (true for single onion services), + * return a newly allocated extend_info_t object. + * + * This function always returns an extend info with a valid IP address and + * ORPort, or NULL. If direct_conn is false, the IP address is always IPv4. * * It performs the following checks: - * if either IPv4 or legacy ID is missing, return NULL. - * if direct_conn, and we can't reach the IPv4 address, return NULL. + * if there is no usable IP address, or legacy ID is missing, return NULL. + * if direct_conn, and we can't reach any IP address, return NULL. */ extend_info_t * hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, @@ -1688,10 +1690,12 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, { int have_v4 = 0, have_legacy_id = 0, have_ed25519_id = 0; char legacy_id[DIGEST_LEN] = {0}; - uint16_t port_v4 = 0; - tor_addr_t addr_v4; ed25519_public_key_t ed25519_pk; extend_info_t *info = NULL; + tor_addr_port_t ap; + + tor_addr_make_null(&ap.addr, AF_UNSPEC); + ap.port = 0; tor_assert(lspecs); @@ -1704,11 +1708,14 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, SMARTLIST_FOREACH_BEGIN(lspecs, const link_specifier_t *, ls) { switch (link_specifier_get_ls_type(ls)) { case LS_IPV4: - /* Skip if we already seen a v4. */ - if (have_v4) continue; - tor_addr_from_ipv4h(&addr_v4, + /* Skip if we already seen a v4. If direct_conn is true, we skip this + * block because fascist_firewall_choose_address_ls() will set ap. If + * direct_conn is false, set ap to the first IPv4 address and port in + * the link specifiers.*/ + if (have_v4 || direct_conn) continue; + tor_addr_from_ipv4h(&ap.addr, link_specifier_get_un_ipv4_addr(ls)); - port_v4 = link_specifier_get_un_ipv4_port(ls); + ap.port = link_specifier_get_un_ipv4_port(ls); have_v4 = 1; break; case LS_LEGACY_ID: @@ -1732,55 +1739,32 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, } } SMARTLIST_FOREACH_END(ls); - /* Legacy ID is mandatory, and we require IPv4. */ - if (!have_v4 || !have_legacy_id) { - bool both = !have_v4 && !have_legacy_id; - log_fn(LOG_PROTOCOL_WARN, LD_REND, "Missing %s%s%s link specifier%s.", - !have_v4 ? "IPv4" : "", - both ? " and " : "", - !have_legacy_id ? "legacy ID" : "", - both ? "s" : ""); + /* Choose a preferred address first, but fall back to an allowed address. */ + if (direct_conn) + 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. */ goto done; } - /* We know we have IPv4, because we just checked. */ - if (!direct_conn) { - /* All clients can extend to any IPv4 via a 3-hop path. */ - goto validate; - } else if (direct_conn && - fascist_firewall_allows_address_addr(&addr_v4, port_v4, - FIREWALL_OR_CONNECTION, - 0, 0)) { - /* Direct connection and we can reach it in IPv4 so go for it. */ - goto validate; + /* We will add support for falling back to a 3-hop path in a later + * release. */ - /* We will add support for falling back to a 3-hop path in a later - * release. */ - } else { - /* If we can't reach IPv4, return NULL. */ - log_fn(LOG_PROTOCOL_WARN, LD_REND, - "Received an IPv4 link specifier, " - "but the address is not reachable: %s:%u", - fmt_addr(&addr_v4), port_v4); - goto done; - } - - /* We will add support for IPv6 in a later release. */ - - validate: /* We'll validate now that the address we've picked isn't a private one. If * it is, are we allowed to extend to private addresses? */ - if (!extend_info_addr_is_allowed(&addr_v4)) { + if (!extend_info_addr_is_allowed(&ap.addr)) { log_fn(LOG_PROTOCOL_WARN, LD_REND, "Requested address is private and we are not allowed to extend to " - "it: %s:%u", fmt_addr(&addr_v4), port_v4); + "it: %s:%u", fmt_addr(&ap.addr), ap.port); goto done; } /* We do have everything for which we think we can connect successfully. */ info = extend_info_new(NULL, legacy_id, (have_ed25519_id) ? &ed25519_pk : NULL, NULL, - onion_key, &addr_v4, port_v4); + onion_key, &ap.addr, ap.port); done: return info; } From b65f8c419a4eb2608beecbf31af0b5bdc6cc38ec Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Sun, 4 Nov 2018 20:08:57 -0500 Subject: [PATCH 3/8] 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 b58a5c9cb6..83d9a53fc0 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 ca7e991b75..9b24129b63 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 46d4a1b94a..e58bb3d174 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); } From b19dd1bb11a59ab38dc1b45f2108a4a6f5ce2813 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 12 Dec 2018 12:43:48 +1000 Subject: [PATCH 4/8] Update 23588 changes file to say what the patch actually does Stop ignoring IPv6 link specifiers sent to v3 onion services. v3 onion service IPv6 support is still incomplete, see 23493 for details. Fixes bug 23588; bugfix on 0.3.2.1-alpha. Patch by Neel Chauhan. --- changes/bug23588 | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/changes/bug23588 b/changes/bug23588 index 754d083614..86064ab313 100644 --- a/changes/bug23588 +++ b/changes/bug23588 @@ -1,7 +1,5 @@ - o Minor bugfixes (address selection): - - Introduce fascist_firewall_choose_address_ls() which chooses an - IPv6 or IPv4 address based on a smartlist of link specifiers of - what is available and what we prefer. We use this function in - hs_get_extend_info_from_lspecs(). Fixes bug 23588; bugfix on - 0.3.5.1-alpha. Patch by Neel Chauhan. - + o Minor bugfixes (v3 onion services): + - Stop ignoring IPv6 link specifiers sent to v3 onion services. + v3 onion service IPv6 support is still incomplete, see 23493 for + details. Fixes bug 23588; bugfix on 0.3.2.1-alpha. + Patch by Neel Chauhan. From abe086dd976174ea52707f0ab27aae31228dfc2b Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 24 Apr 2019 16:57:49 +1000 Subject: [PATCH 5/8] test/hs: Re-enable the IPv6 intro point in the HS tests Tests 23588, partially reverts 23576. Implements 29237. --- src/test/hs_test_helpers.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/test/hs_test_helpers.c b/src/test/hs_test_helpers.c index c57bdc730b..0a21fe576b 100644 --- a/src/test/hs_test_helpers.c +++ b/src/test/hs_test_helpers.c @@ -162,11 +162,8 @@ hs_helper_build_hs_desc_impl(unsigned int no_ip, /* Add four intro points. */ smartlist_add(desc->encrypted_data.intro_points, hs_helper_build_intro_point(signing_kp, now, "1.2.3.4", 0)); -/* IPv6-only introduction points are not supported yet, see #23588 */ -#if 0 smartlist_add(desc->encrypted_data.intro_points, hs_helper_build_intro_point(signing_kp, now, "[2600::1]", 0)); -#endif smartlist_add(desc->encrypted_data.intro_points, hs_helper_build_intro_point(signing_kp, now, "3.2.1.4", 1)); smartlist_add(desc->encrypted_data.intro_points, From f05e3f3c9af2cd05404a0646d4f7acc670c421da Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 24 Apr 2019 17:23:00 +1000 Subject: [PATCH 6/8] test/hs: Correctly convert an IPv6 intro point to an extend_info Part of #23588. --- src/test/test_hs_client.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/test/test_hs_client.c b/src/test/test_hs_client.c index 8362b6cbda..aba0c937cc 100644 --- a/src/test/test_hs_client.c +++ b/src/test/test_hs_client.c @@ -479,6 +479,17 @@ test_client_pick_intro(void *arg) SMARTLIST_FOREACH_BEGIN(desc->encrypted_data.intro_points, hs_desc_intro_point_t *, ip) { extend_info_t *intro_ei = desc_intro_point_to_extend_info(ip); + /* desc_intro_point_to_extend_info() doesn't return IPv6 intro points + * yet, because we can't extend to them. See #24404, #24451, and #24181. + */ + if (intro_ei == NULL) { + /* Pretend we're making a direct connection, and that we can use IPv6 + */ + get_options_mutable()->ClientUseIPv6 = 1; + intro_ei = hs_get_extend_info_from_lspecs(ip->link_specifiers, + &ip->onion_key, 1); + tt_assert(tor_addr_family(&intro_ei->addr) == AF_INET6); + } tt_assert(intro_ei); if (intro_ei) { const char *ptr; From 3d89f0374a20571ca04068f10291ffefc9d76b40 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 24 Apr 2019 17:23:53 +1000 Subject: [PATCH 7/8] hs_config: Allow Tor to be configured as an IPv6-only v3 single onion service Part of #23588. --- src/feature/hs/hs_config.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/feature/hs/hs_config.c b/src/feature/hs/hs_config.c index ee4499ef5b..87f6257591 100644 --- a/src/feature/hs/hs_config.c +++ b/src/feature/hs/hs_config.c @@ -496,15 +496,6 @@ config_generic_service(const config_line_t *line_, * becomes a single onion service. */ if (rend_service_non_anonymous_mode_enabled(options)) { config->is_single_onion = 1; - /* We will add support for IPv6-only v3 single onion services in a future - * Tor version. This won't catch "ReachableAddresses reject *4", but that - * option doesn't work anyway. */ - if (options->ClientUseIPv4 == 0 && config->version == HS_VERSION_THREE) { - log_warn(LD_CONFIG, "IPv6-only v3 single onion services are not " - "supported. Set HiddenServiceSingleHopMode 0 and " - "HiddenServiceNonAnonymousMode 0, or set ClientUseIPv4 1."); - goto err; - } } /* Success */ From de91b838496a3646721608bdb7740696601f57c4 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 23 Apr 2019 12:36:06 +1000 Subject: [PATCH 8/8] practracker: Accept ~80 extra lines in src/core/or/policies.c Part of 23588. --- scripts/maint/practracker/exceptions.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index 1992c563ac..0a5f3b1699 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -117,7 +117,7 @@ problem include-count /src/core/or/connection_or.c 51 problem function-size /src/core/or/connection_or.c:connection_or_group_set_badness_() 105 problem function-size /src/core/or/connection_or.c:connection_or_client_learned_peer_id() 144 problem function-size /src/core/or/connection_or.c:connection_or_compute_authenticate_cell_body() 235 -problem file-size /src/core/or/policies.c 3171 +problem file-size /src/core/or/policies.c 3249 problem function-size /src/core/or/policies.c:policy_summarize() 107 problem function-size /src/core/or/protover.c:protover_all_supported() 116 problem file-size /src/core/or/relay.c 3173