commit 6170d3fcf12f4345ec13561d0f444930f2f12e84 Author: teor <teor@torproject.org> Date: Tue Jul 31 14:30:17 2018 +1000 hs: Onion services put IPv6 addresses in service descriptors Rewrite service_intro_point_new() to take a node_t. Since node_get_link_specifier_smartlist() supports IPv6 link specifiers, this refactor adds IPv6 addresses to onion service descriptors. Part of 23576, implements 26992. --- changes/bug23576 | 7 ++++ src/feature/hs/hs_common.c | 18 +++++++- src/feature/hs/hs_service.c | 96 ++++++++----------------------------------- src/feature/hs/hs_service.h | 5 +-- src/test/test_hs_cell.c | 4 +- src/test/test_hs_intropoint.c | 4 +- src/test/test_hs_service.c | 2 +- 7 files changed, 48 insertions(+), 88 deletions(-) diff --git a/changes/bug23576 b/changes/bug23576 new file mode 100644 index 000000000..edcae02e5 --- /dev/null +++ b/changes/bug23576 @@ -0,0 +1,7 @@ + o Minor features (IPv6, v3 onion services): + - Make v3 onion services put IPv6 addresses in service + descriptors. Before this change, service descriptors only + contained IPv4 addressesd. Implements 26992. + o Code simplification and refactoring: + - Simplify v3 onion service link specifier handling code. + Fixes bug 23576; bugfix on 0.3.2.1-alpha. diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c index ebe49f09a..5d8f54230 100644 --- a/src/feature/hs/hs_common.c +++ b/src/feature/hs/hs_common.c @@ -1697,6 +1697,12 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, tor_assert(lspecs); + if (smartlist_len(lspecs) == 0) { + log_fn(LOG_PROTOCOL_WARN, LD_REND, "Empty link specifier list."); + /* Return NULL. */ + goto done; + } + SMARTLIST_FOREACH_BEGIN(lspecs, const link_specifier_t *, ls) { switch (link_specifier_get_ls_type(ls)) { case LS_IPV4: @@ -1730,6 +1736,12 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, /* 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" : ""); goto done; } @@ -1748,6 +1760,10 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, * 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; } @@ -1755,7 +1771,7 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, validate: /* We'll validate now that the address we've picked isn't a private one. If - * it is, are we allowing to extend to private address? */ + * it is, are we allowed to extend to private addresses? */ if (!extend_info_addr_is_allowed(&addr_v4)) { log_fn(LOG_PROTOCOL_WARN, LD_REND, "Requested address is private and we are not allowed to extend to " diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index b94dd9a48..6e4da8856 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -426,23 +426,16 @@ service_intro_point_free_void(void *obj) } /* Return a newly allocated service intro point and fully initialized from the - * given extend_info_t ei if non NULL. - * If is_legacy is true, we also generate the legacy key. - * If supports_ed25519_link_handshake_any is true, we add the relay's ed25519 - * key to the link specifiers. + * given node_t node, if non NULL. * - * If ei is NULL, returns a hs_service_intro_point_t with an empty link + * If node is NULL, returns a hs_service_intro_point_t with an empty link * specifier list and no onion key. (This is used for testing.) * On any other error, NULL is returned. * - * ei must be an extend_info_t containing an IPv4 address. (We will add supoort - * for IPv6 in a later release.) When calling extend_info_from_node(), pass - * 0 in for_direct_connection to make sure ei always has an IPv4 address. */ + * node must be an node_t with an IPv4 address. */ STATIC hs_service_intro_point_t * -service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy, - unsigned int supports_ed25519_link_handshake_any) +service_intro_point_new(const node_t *node) { - hs_desc_link_specifier_t *ls; hs_service_intro_point_t *ip; ip = tor_malloc_zero(sizeof(*ip)); @@ -472,12 +465,17 @@ service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy, ip->replay_cache = replaycache_new(0, 0); /* Initialize the base object. We don't need the certificate object. */ - ip->base.link_specifiers = smartlist_new(); + ip->base.link_specifiers = node_get_link_specifier_smartlist(node, 0); + + if (node == NULL) { + goto done; + } /* Generate the encryption key for this intro point. */ curve25519_keypair_generate(&ip->enc_key_kp, 0); - /* Figure out if this chosen node supports v3 or is legacy only. */ - if (is_legacy) { + /* Figure out if this chosen node supports v3 or is legacy only. + * NULL nodes are used in the unit tests. */ + if (!node_supports_ed25519_hs_intro(node)) { ip->base.is_only_legacy = 1; /* Legacy mode that is doesn't support v3+ with ed25519 auth key. */ ip->legacy_key = crypto_pk_new(); @@ -490,40 +488,9 @@ service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy, } } - if (ei == NULL) { - goto done; - } - - /* We'll try to add all link specifiers. Legacy is mandatory. - * IPv4 or IPv6 is required, and we always send IPv4. */ - ls = hs_desc_link_specifier_new(ei, LS_IPV4); - /* It is impossible to have an extend info object without a v4. */ - if (BUG(!ls)) { - goto err; - } - smartlist_add(ip->base.link_specifiers, ls); - - ls = hs_desc_link_specifier_new(ei, LS_LEGACY_ID); - /* It is impossible to have an extend info object without an identity - * digest. */ - if (BUG(!ls)) { - goto err; - } - smartlist_add(ip->base.link_specifiers, ls); - - /* ed25519 identity key is optional for intro points. If the node supports - * ed25519 link authentication, we include it. */ - if (supports_ed25519_link_handshake_any) { - ls = hs_desc_link_specifier_new(ei, LS_ED25519_ID); - if (ls) { - smartlist_add(ip->base.link_specifiers, ls); - } - } - - /* IPv6 is not supported in this release. */ - - /* Finally, copy onion key from the extend_info_t object. */ - memcpy(&ip->onion_key, &ei->curve25519_onion_key, sizeof(ip->onion_key)); + /* Finally, copy onion key from the node. */ + memcpy(&ip->onion_key, node_get_curve25519_onion_key(node), + sizeof(ip->onion_key)); done: return ip; @@ -2106,7 +2073,6 @@ static hs_service_intro_point_t * pick_intro_point(unsigned int direct_conn, smartlist_t *exclude_nodes) { const node_t *node; - extend_info_t *info = NULL; hs_service_intro_point_t *ip = NULL; /* Normal 3-hop introduction point flags. */ router_crn_flags_t flags = CRN_NEED_UPTIME | CRN_NEED_DESC; @@ -2127,43 +2093,17 @@ pick_intro_point(unsigned int direct_conn, smartlist_t *exclude_nodes) * we don't want to use that node anymore. */ smartlist_add(exclude_nodes, (void *) node); - /* We do this to ease our life but also this call makes appropriate checks - * of the node object such as validating ntor support for instance. - * - * We must provide an extend_info for clients to connect over a 3-hop path, - * so we don't pass direct_conn here. */ - info = extend_info_from_node(node, 0); - if (BUG(info == NULL)) { - goto err; - } - - /* Let's do a basic sanity check here so that we don't end up advertising the - * ed25519 identity key of relays that don't actually support the link - * protocol */ - if (!node_supports_ed25519_link_authentication(node, 0)) { - tor_assert_nonfatal(ed25519_public_key_is_zero(&info->ed_identity)); - } else { - /* Make sure we *do* have an ed key if we support the link authentication. - * Sending an empty key would result in a failure to extend. */ - tor_assert_nonfatal(!ed25519_public_key_is_zero(&info->ed_identity)); - } + /* Create our objects and populate them with the node information. */ + ip = service_intro_point_new(node); - /* Create our objects and populate them with the node information. - * We don't care if the intro's link auth is compatible with us, because - * we are sending the ed25519 key to a remote client via the descriptor. */ - ip = service_intro_point_new(info, !node_supports_ed25519_hs_intro(node), - node_supports_ed25519_link_authentication(node, - 0)); if (ip == NULL) { goto err; } - log_info(LD_REND, "Picked intro point: %s", extend_info_describe(info)); - extend_info_free(info); + log_info(LD_REND, "Picked intro point: %s", node_describe(node)); return ip; err: service_intro_point_free(ip); - extend_info_free(info); return NULL; } diff --git a/src/feature/hs/hs_service.h b/src/feature/hs/hs_service.h index ec53f2f23..8d7f77321 100644 --- a/src/feature/hs/hs_service.h +++ b/src/feature/hs/hs_service.h @@ -369,10 +369,7 @@ STATIC hs_service_t *find_service(hs_service_ht *map, STATIC void remove_service(hs_service_ht *map, hs_service_t *service); STATIC int register_service(hs_service_ht *map, hs_service_t *service); /* Service introduction point functions. */ -STATIC hs_service_intro_point_t *service_intro_point_new( - const extend_info_t *ei, - unsigned int is_legacy, - unsigned int supports_ed25519_link_handshake_any); +STATIC hs_service_intro_point_t *service_intro_point_new(const node_t *node); STATIC void service_intro_point_free_(hs_service_intro_point_t *ip); #define service_intro_point_free(ip) \ FREE_AND_NULL(hs_service_intro_point_t, \ diff --git a/src/test/test_hs_cell.c b/src/test/test_hs_cell.c index 0c93f593c..6e00e8807 100644 --- a/src/test/test_hs_cell.c +++ b/src/test/test_hs_cell.c @@ -39,7 +39,7 @@ test_gen_establish_intro_cell(void *arg) attempt to parse it. */ { /* We only need the auth key pair here. */ - hs_service_intro_point_t *ip = service_intro_point_new(NULL, 0, 0); + hs_service_intro_point_t *ip = service_intro_point_new(NULL); /* Auth key pair is generated in the constructor so we are all set for * using this IP object. */ ret = hs_cell_build_establish_intro(circ_nonce, ip, buf); @@ -107,7 +107,7 @@ test_gen_establish_intro_cell_bad(void *arg) ed25519_sign_prefixed() function and make it fail. */ cell = trn_cell_establish_intro_new(); tt_assert(cell); - ip = service_intro_point_new(NULL, 0, 0); + ip = service_intro_point_new(NULL); cell_len = hs_cell_build_establish_intro(circ_nonce, ip, NULL); service_intro_point_free(ip); expect_log_msg_containing("Unable to make signature for " diff --git a/src/test/test_hs_intropoint.c b/src/test/test_hs_intropoint.c index 660f21ffd..b7163c5c1 100644 --- a/src/test/test_hs_intropoint.c +++ b/src/test/test_hs_intropoint.c @@ -50,7 +50,7 @@ new_establish_intro_cell(const char *circ_nonce, /* Auth key pair is generated in the constructor so we are all set for * using this IP object. */ - ip = service_intro_point_new(NULL, 0, 0); + ip = service_intro_point_new(NULL); tt_assert(ip); cell_len = hs_cell_build_establish_intro(circ_nonce, ip, buf); tt_i64_op(cell_len, OP_GT, 0); @@ -76,7 +76,7 @@ new_establish_intro_encoded_cell(const char *circ_nonce, uint8_t *cell_out) /* Auth key pair is generated in the constructor so we are all set for * using this IP object. */ - ip = service_intro_point_new(NULL, 0, 0); + ip = service_intro_point_new(NULL); tt_assert(ip); cell_len = hs_cell_build_establish_intro(circ_nonce, ip, cell_out); tt_i64_op(cell_len, OP_GT, 0); diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index 43bf89438..e8bdcd86e 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -329,7 +329,7 @@ static hs_service_intro_point_t * helper_create_service_ip(void) { hs_desc_link_specifier_t *ls; - hs_service_intro_point_t *ip = service_intro_point_new(NULL, 0, 0); + hs_service_intro_point_t *ip = service_intro_point_new(NULL); tor_assert(ip); /* Add a first unused link specifier. */ ls = tor_malloc_zero(sizeof(*ls));