Merge branch 'bug23576-041-rebased-squashed'

changes/bug22781

@@ -0,0 +1,4 @@
+  o Code simplification and refactoring:
+    - Replace hs_desc_link_specifier_t with link_specifier_t,
+      and remove all hs_desc_link_specifier_t-specific code.
+      Fixes bug 22781; bugfix on 0.3.2.1-alpha.

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.

changes/bug29243

@@ -0,0 +1,3 @@
+  o Minor bugfixes (testing, v3 onion services):
+    - Fix some incorrect code in the v3 onion service unit tests.
+      Fixes bug 29243; bugfix on 0.3.2.1-alpha.

src/feature/hs/hs_cell.c

@@ -756,7 +756,14 @@ hs_cell_parse_introduce2(hs_cell_introduce2_data_t *data,
        idx < trn_cell_introduce_encrypted_get_nspec(enc_cell); idx++) {
     link_specifier_t *lspec =
       trn_cell_introduce_encrypted_get_nspecs(enc_cell, idx);
-    smartlist_add(data->link_specifiers, hs_link_specifier_dup(lspec));
+    if (BUG(!lspec)) {
+      goto done;
+    }
+    link_specifier_t *lspec_dup = link_specifier_dup(lspec);
+    if (BUG(!lspec_dup)) {
+      goto done;
+    }
+    smartlist_add(data->link_specifiers, lspec_dup);
   }
 
   /* Success. */

src/feature/hs/hs_circuit.c

@@ -565,81 +565,6 @@ retry_service_rendezvous_point(const origin_circuit_t *circ)
   return;
 }
 
-/* Add all possible link specifiers in node to lspecs:
- *  - legacy ID is mandatory thus MUST be present in node;
- *  - include ed25519 link specifier if present in the node, and the node
- *    supports ed25519 link authentication, even if its link versions are not
- *    compatible with us;
- *  - include IPv4 link specifier, if the primary address is not IPv4, log a
- *    BUG() warning, and return an empty smartlist;
- *  - include IPv6 link specifier if present in the node. */
-static void
-get_lspecs_from_node(const node_t *node, smartlist_t *lspecs)
-{
-  link_specifier_t *ls;
-  tor_addr_port_t ap;
-
-  tor_assert(node);
-  tor_assert(lspecs);
-
-  /* Get the relay's IPv4 address. */
-  node_get_prim_orport(node, &ap);
-
-  /* We expect the node's primary address to be a valid IPv4 address.
-   * This conforms to the protocol, which requires either an IPv4 or IPv6
-   * address (or both). */
-  if (BUG(!tor_addr_is_v4(&ap.addr)) ||
-      BUG(!tor_addr_port_is_valid_ap(&ap, 0))) {
-    return;
-  }
-
-  ls = link_specifier_new();
-  link_specifier_set_ls_type(ls, LS_IPV4);
-  link_specifier_set_un_ipv4_addr(ls, tor_addr_to_ipv4h(&ap.addr));
-  link_specifier_set_un_ipv4_port(ls, ap.port);
-  /* Four bytes IPv4 and two bytes port. */
-  link_specifier_set_ls_len(ls, sizeof(ap.addr.addr.in_addr) +
-                            sizeof(ap.port));
-  smartlist_add(lspecs, ls);
-
-  /* Legacy ID is mandatory and will always be present in node. */
-  ls = link_specifier_new();
-  link_specifier_set_ls_type(ls, LS_LEGACY_ID);
-  memcpy(link_specifier_getarray_un_legacy_id(ls), node->identity,
-         link_specifier_getlen_un_legacy_id(ls));
-  link_specifier_set_ls_len(ls, link_specifier_getlen_un_legacy_id(ls));
-  smartlist_add(lspecs, ls);
-
-  /* ed25519 ID is only included if the node has it, and the node declares a
-     protocol version that supports ed25519 link authentication, even if that
-     link version is not compatible with us. (We are sending the ed25519 key
-     to another tor, which may support different link versions.) */
-  if (!ed25519_public_key_is_zero(&node->ed25519_id) &&
-      node_supports_ed25519_link_authentication(node, 0)) {
-    ls = link_specifier_new();
-    link_specifier_set_ls_type(ls, LS_ED25519_ID);
-    memcpy(link_specifier_getarray_un_ed25519_id(ls), &node->ed25519_id,
-           link_specifier_getlen_un_ed25519_id(ls));
-    link_specifier_set_ls_len(ls, link_specifier_getlen_un_ed25519_id(ls));
-    smartlist_add(lspecs, ls);
-  }
-
-  /* Check for IPv6. If so, include it as well. */
-  if (node_has_ipv6_orport(node)) {
-    ls = link_specifier_new();
-    node_get_pref_ipv6_orport(node, &ap);
-    link_specifier_set_ls_type(ls, LS_IPV6);
-    size_t addr_len = link_specifier_getlen_un_ipv6_addr(ls);
-    const uint8_t *in6_addr = tor_addr_to_in6_addr8(&ap.addr);
-    uint8_t *ipv6_array = link_specifier_getarray_un_ipv6_addr(ls);
-    memcpy(ipv6_array, in6_addr, addr_len);
-    link_specifier_set_un_ipv6_port(ls, ap.port);
-    /* Sixteen bytes IPv6 and two bytes port. */
-    link_specifier_set_ls_len(ls, addr_len + sizeof(ap.port));
-    smartlist_add(lspecs, ls);
-  }
-}
-
 /* Using the given descriptor intro point ip, the node of the
  * rendezvous point rp_node and the service's subcredential, populate the
  * already allocated intro1_data object with the needed key material and link
@@ -662,10 +587,9 @@ setup_introduce1_data(const hs_desc_intro_point_t *ip,
   tor_assert(subcredential);
   tor_assert(intro1_data);
 
-  /* Build the link specifiers from the extend information of the rendezvous
-   * circuit that we've picked previously. */
-  rp_lspecs = smartlist_new();
-  get_lspecs_from_node(rp_node, rp_lspecs);
+  /* Build the link specifiers from the node at the end of the rendezvous
+   * circuit that we opened for this introduction. */
+  rp_lspecs = node_get_link_specifier_smartlist(rp_node, 0);
   if (smartlist_len(rp_lspecs) == 0) {
     /* We can't rendezvous without link specifiers. */
     smartlist_free(rp_lspecs);
@@ -1044,9 +968,7 @@ hs_circ_handle_introduce2(const hs_service_t *service,
   ret = 0;
 
  done:
-  SMARTLIST_FOREACH(data.link_specifiers, link_specifier_t *, lspec,
-                    link_specifier_free(lspec));
-  smartlist_free(data.link_specifiers);
+  link_specifier_smartlist_free(data.link_specifiers);
   memwipe(&data, 0, sizeof(data));
   return ret;
 }

src/feature/hs/hs_client.c

@@ -546,13 +546,15 @@ find_desc_intro_point_by_legacy_id(const char *legacy_id,
   SMARTLIST_FOREACH_BEGIN(desc->encrypted_data.intro_points,
                           hs_desc_intro_point_t *, ip) {
     SMARTLIST_FOREACH_BEGIN(ip->link_specifiers,
-                            const hs_desc_link_specifier_t *, lspec) {
+                            const link_specifier_t *, lspec) {
       /* Not all tor node have an ed25519 identity key so we still rely on the
        * legacy identity digest. */
-      if (lspec->type != LS_LEGACY_ID) {
+      if (link_specifier_get_ls_type(lspec) != LS_LEGACY_ID) {
         continue;
       }
-      if (fast_memneq(legacy_id, lspec->u.legacy_id, DIGEST_LEN)) {
+      if (fast_memneq(legacy_id,
+                      link_specifier_getconstarray_un_legacy_id(lspec),
+                      DIGEST_LEN)) {
         break;
       }
       /* Found it. */
@@ -771,24 +773,13 @@ STATIC extend_info_t *
 desc_intro_point_to_extend_info(const hs_desc_intro_point_t *ip)
 {
   extend_info_t *ei;
-  smartlist_t *lspecs = smartlist_new();
 
   tor_assert(ip);
 
-  /* We first encode the descriptor link specifiers into the binary
-   * representation which is a trunnel object. */
-  SMARTLIST_FOREACH_BEGIN(ip->link_specifiers,
-                          const hs_desc_link_specifier_t *, desc_lspec) {
-    link_specifier_t *lspec = hs_desc_lspec_to_trunnel(desc_lspec);
-    smartlist_add(lspecs, lspec);
-  } SMARTLIST_FOREACH_END(desc_lspec);
-
   /* Explicitly put the direct connection option to 0 because this is client
    * side and there is no such thing as a non anonymous client. */
-  ei = hs_get_extend_info_from_lspecs(lspecs, &ip->onion_key, 0);
+  ei = hs_get_extend_info_from_lspecs(ip->link_specifiers, &ip->onion_key, 0);
 
-  SMARTLIST_FOREACH(lspecs, link_specifier_t *, ls, link_specifier_free(ls));
-  smartlist_free(lspecs);
   return ei;
 }
 

src/feature/hs/hs_common.c

@@ -1010,24 +1010,6 @@ hs_build_address(const ed25519_public_key_t *key, uint8_t version,
   tor_assert(hs_address_is_valid(addr_out));
 }
 
-/* Return a newly allocated copy of lspec. */
-link_specifier_t *
-hs_link_specifier_dup(const link_specifier_t *lspec)
-{
-  link_specifier_t *result = link_specifier_new();
-  memcpy(result, lspec, sizeof(*result));
-  /* The unrecognized field is a dynamic array so make sure to copy its
-   * content and not the pointer. */
-  link_specifier_setlen_un_unrecognized(
-                  result, link_specifier_getlen_un_unrecognized(lspec));
-  if (link_specifier_getlen_un_unrecognized(result)) {
-    memcpy(link_specifier_getarray_un_unrecognized(result),
-           link_specifier_getconstarray_un_unrecognized(lspec),
-           link_specifier_getlen_un_unrecognized(result));
-  }
-  return result;
-}
-
 /* From a given ed25519 public key pk and an optional secret, compute a
  * blinded public key and put it in blinded_pk_out. This is only useful to
  * the client side because the client only has access to the identity public
@@ -1698,6 +1680,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:
@@ -1731,6 +1719,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;
   }
 
@@ -1749,6 +1743,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;
   }
 
@@ -1756,7 +1754,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 "
@@ -1828,3 +1826,42 @@ hs_inc_rdv_stream_counter(origin_circuit_t *circ)
     tor_assert_nonfatal_unreached();
   }
 }
+
+/* Return a newly allocated link specifier object that is a copy of dst. */
+link_specifier_t *
+link_specifier_dup(const link_specifier_t *src)
+{
+  link_specifier_t *dup = NULL;
+  uint8_t *buf = NULL;
+
+  if (BUG(!src)) {
+    goto err;
+  }
+
+  ssize_t encoded_len_alloc = link_specifier_encoded_len(src);
+  if (BUG(encoded_len_alloc < 0)) {
+    goto err;
+  }
+
+  buf = tor_malloc_zero(encoded_len_alloc);
+  ssize_t encoded_len_data = link_specifier_encode(buf,
+                                                   encoded_len_alloc,
+                                                   src);
+  if (BUG(encoded_len_data < 0)) {
+    goto err;
+  }
+
+  ssize_t parsed_len = link_specifier_parse(&dup, buf, encoded_len_alloc);
+  if (BUG(parsed_len < 0)) {
+    goto err;
+  }
+
+  goto done;
+
+ err:
+  dup = NULL;
+
+ done:
+  tor_free(buf);
+  return dup;
+}

src/feature/hs/hs_common.h

@@ -217,8 +217,6 @@ uint64_t hs_get_time_period_num(time_t now);
 uint64_t hs_get_next_time_period_num(time_t now);
 time_t hs_get_start_time_of_next_time_period(time_t now);
 
-link_specifier_t *hs_link_specifier_dup(const link_specifier_t *lspec);
-
 MOCK_DECL(int, hs_in_period_between_tp_and_srv,
           (const networkstatus_t *consensus, time_t now));
 
@@ -262,6 +260,8 @@ extend_info_t *hs_get_extend_info_from_lspecs(const smartlist_t *lspecs,
                           const struct curve25519_public_key_t *onion_key,
                           int direct_conn);
 
+link_specifier_t *link_specifier_dup(const link_specifier_t *src);
+
 #ifdef HS_COMMON_PRIVATE
 
 STATIC void get_disaster_srv(uint64_t time_period_num, uint8_t *srv_out);

src/feature/hs/hs_descriptor.c

@@ -324,12 +324,11 @@ encode_link_specifiers(const smartlist_t *specs)
 
   link_specifier_list_set_n_spec(lslist, smartlist_len(specs));
 
-  SMARTLIST_FOREACH_BEGIN(specs, const hs_desc_link_specifier_t *,
+  SMARTLIST_FOREACH_BEGIN(specs, const link_specifier_t *,
                           spec) {
-    link_specifier_t *ls = hs_desc_lspec_to_trunnel(spec);
-    if (ls) {
-      link_specifier_list_add_spec(lslist, ls);
-    }
+    link_specifier_t *ls = link_specifier_dup(spec);
+    tor_assert(ls);
+    link_specifier_list_add_spec(lslist, ls);
   } SMARTLIST_FOREACH_END(spec);
 
   {
@@ -1190,52 +1189,22 @@ decode_link_specifiers(const char *encoded)
   results = smartlist_new();
 
   for (i = 0; i < link_specifier_list_getlen_spec(specs); i++) {
-    hs_desc_link_specifier_t *hs_spec;
     link_specifier_t *ls = link_specifier_list_get_spec(specs, i);
-    tor_assert(ls);
-
-    hs_spec = tor_malloc_zero(sizeof(*hs_spec));
-    hs_spec->type = link_specifier_get_ls_type(ls);
-    switch (hs_spec->type) {
-    case LS_IPV4:
-      tor_addr_from_ipv4h(&hs_spec->u.ap.addr,
-                          link_specifier_get_un_ipv4_addr(ls));
-      hs_spec->u.ap.port = link_specifier_get_un_ipv4_port(ls);
-      break;
-    case LS_IPV6:
-      tor_addr_from_ipv6_bytes(&hs_spec->u.ap.addr, (const char *)
-                               link_specifier_getarray_un_ipv6_addr(ls));
-      hs_spec->u.ap.port = link_specifier_get_un_ipv6_port(ls);
-      break;
-    case LS_LEGACY_ID:
-      /* Both are known at compile time so let's make sure they are the same
-       * else we can copy memory out of bound. */
-      tor_assert(link_specifier_getlen_un_legacy_id(ls) ==
-                 sizeof(hs_spec->u.legacy_id));
-      memcpy(hs_spec->u.legacy_id, link_specifier_getarray_un_legacy_id(ls),
-             sizeof(hs_spec->u.legacy_id));
-      break;
-    case LS_ED25519_ID:
-      /* Both are known at compile time so let's make sure they are the same
-       * else we can copy memory out of bound. */
-      tor_assert(link_specifier_getlen_un_ed25519_id(ls) ==
-                 sizeof(hs_spec->u.ed25519_id));
-      memcpy(hs_spec->u.ed25519_id,
-             link_specifier_getconstarray_un_ed25519_id(ls),
-             sizeof(hs_spec->u.ed25519_id));
-      break;
-    default:
-      tor_free(hs_spec);
+    if (BUG(!ls)) {
       goto err;
     }
-
-    smartlist_add(results, hs_spec);
+    link_specifier_t *ls_dup = link_specifier_dup(ls);
+    if (BUG(!ls_dup)) {
+      goto err;
+    }
+    smartlist_add(results, ls_dup);
   }
 
   goto done;
  err:
   if (results) {
-    SMARTLIST_FOREACH(results, hs_desc_link_specifier_t *, s, tor_free(s));
+    SMARTLIST_FOREACH(results, link_specifier_t *, s,
+                      link_specifier_free(s));
     smartlist_free(results);
     results = NULL;
   }
@@ -2878,8 +2847,8 @@ hs_desc_intro_point_free_(hs_desc_intro_point_t *ip)
     return;
   }
   if (ip->link_specifiers) {
-    SMARTLIST_FOREACH(ip->link_specifiers, hs_desc_link_specifier_t *,
-                      ls, hs_desc_link_specifier_free(ls));
+    SMARTLIST_FOREACH(ip->link_specifiers, link_specifier_t *,
+                      ls, link_specifier_free(ls));
     smartlist_free(ip->link_specifiers);
   }
   tor_cert_free(ip->auth_key_cert);
@@ -2972,69 +2941,6 @@ hs_desc_authorized_client_free_(hs_desc_authorized_client_t *client)
   tor_free(client);
 }
 
-/* Free the given descriptor link specifier. */
-void
-hs_desc_link_specifier_free_(hs_desc_link_specifier_t *ls)
-{
-  if (ls == NULL) {
-    return;
-  }
-  tor_free(ls);
-}
-
-/* Return a newly allocated descriptor link specifier using the given extend
- * info and requested type. Return NULL on error. */
-hs_desc_link_specifier_t *
-hs_desc_link_specifier_new(const extend_info_t *info, uint8_t type)
-{
-  hs_desc_link_specifier_t *ls = NULL;
-
-  tor_assert(info);
-
-  ls = tor_malloc_zero(sizeof(*ls));
-  ls->type = type;
-  switch (ls->type) {
-  case LS_IPV4:
-    if (info->addr.family != AF_INET) {
-      goto err;
-    }
-    tor_addr_copy(&ls->u.ap.addr, &info->addr);
-    ls->u.ap.port = info->port;
-    break;
-  case LS_IPV6:
-    if (info->addr.family != AF_INET6) {
-      goto err;
-    }
-    tor_addr_copy(&ls->u.ap.addr, &info->addr);
-    ls->u.ap.port = info->port;
-    break;
-  case LS_LEGACY_ID:
-    /* Bug out if the identity digest is not set */
-    if (BUG(tor_mem_is_zero(info->identity_digest,
-                            sizeof(info->identity_digest)))) {
-      goto err;
-    }
-    memcpy(ls->u.legacy_id, info->identity_digest, sizeof(ls->u.legacy_id));
-    break;
-  case LS_ED25519_ID:
-    /* ed25519 keys are optional for intro points */
-    if (ed25519_public_key_is_zero(&info->ed_identity)) {
-      goto err;
-    }
-    memcpy(ls->u.ed25519_id, info->ed_identity.pubkey,
-           sizeof(ls->u.ed25519_id));
-    break;
-  default:
-    /* Unknown type is code flow error. */
-    tor_assert(0);
-  }
-
-  return ls;
- err:
-  tor_free(ls);
-  return NULL;
-}
-
 /* From the given descriptor, remove and free every introduction point. */
 void
 hs_descriptor_clear_intro_points(hs_descriptor_t *desc)
@@ -3050,59 +2956,3 @@ hs_descriptor_clear_intro_points(hs_descriptor_t *desc)
     smartlist_clear(ips);
   }
 }
-
-/* From a descriptor link specifier object spec, returned a newly allocated
- * link specifier object that is the encoded representation of spec. Return
- * NULL on error. */
-link_specifier_t *
-hs_desc_lspec_to_trunnel(const hs_desc_link_specifier_t *spec)
-{
-  tor_assert(spec);
-
-  link_specifier_t *ls = link_specifier_new();
-  link_specifier_set_ls_type(ls, spec->type);
-
-  switch (spec->type) {
-  case LS_IPV4:
-    link_specifier_set_un_ipv4_addr(ls,
-                                    tor_addr_to_ipv4h(&spec->u.ap.addr));
-    link_specifier_set_un_ipv4_port(ls, spec->u.ap.port);
-    /* Four bytes IPv4 and two bytes port. */
-    link_specifier_set_ls_len(ls, sizeof(spec->u.ap.addr.addr.in_addr) +
-                                  sizeof(spec->u.ap.port));
-    break;
-  case LS_IPV6:
-  {
-    size_t addr_len = link_specifier_getlen_un_ipv6_addr(ls);
-    const uint8_t *in6_addr = tor_addr_to_in6_addr8(&spec->u.ap.addr);
-    uint8_t *ipv6_array = link_specifier_getarray_un_ipv6_addr(ls);
-    memcpy(ipv6_array, in6_addr, addr_len);
-    link_specifier_set_un_ipv6_port(ls, spec->u.ap.port);
-    /* Sixteen bytes IPv6 and two bytes port. */
-    link_specifier_set_ls_len(ls, addr_len + sizeof(spec->u.ap.port));
-    break;
-  }
-  case LS_LEGACY_ID:
-  {
-    size_t legacy_id_len = link_specifier_getlen_un_legacy_id(ls);
-    uint8_t *legacy_id_array = link_specifier_getarray_un_legacy_id(ls);
-    memcpy(legacy_id_array, spec->u.legacy_id, legacy_id_len);
-    link_specifier_set_ls_len(ls, legacy_id_len);
-    break;
-  }
-  case LS_ED25519_ID:
-  {
-    size_t ed25519_id_len = link_specifier_getlen_un_ed25519_id(ls);
-    uint8_t *ed25519_id_array = link_specifier_getarray_un_ed25519_id(ls);
-    memcpy(ed25519_id_array, spec->u.ed25519_id, ed25519_id_len);
-    link_specifier_set_ls_len(ls, ed25519_id_len);
-    break;
-  }
-  default:
-    tor_assert_nonfatal_unreached();
-    link_specifier_free(ls);
-    ls = NULL;
-  }
-
-  return ls;
-}

src/feature/hs/hs_descriptor.h

@@ -69,28 +69,10 @@ typedef enum {
   HS_DESC_AUTH_ED25519 = 1
 } hs_desc_auth_type_t;
 
-/* Link specifier object that contains information on how to extend to the
- * relay that is the address, port and handshake type. */
-typedef struct hs_desc_link_specifier_t {
-  /* Indicate the type of link specifier. See trunnel ed25519_cert
-   * specification. */
-  uint8_t type;
-
-  /* It must be one of these types, can't be more than one. */
-  union {
-    /* IP address and port of the relay use to extend. */
-    tor_addr_port_t ap;
-    /* Legacy identity. A 20-byte SHA1 identity fingerprint. */
-    uint8_t legacy_id[DIGEST_LEN];
-    /* ed25519 identity. A 32-byte key. */
-    uint8_t ed25519_id[ED25519_PUBKEY_LEN];
-  } u;
-} hs_desc_link_specifier_t;
-
 /* Introduction point information located in a descriptor. */
 typedef struct hs_desc_intro_point_t {
   /* Link specifier(s) which details how to extend to the relay. This list
-   * contains hs_desc_link_specifier_t object. It MUST have at least one. */
+   * contains link_specifier_t objects. It MUST have at least one. */
   smartlist_t *link_specifiers;
 
   /* Onion key of the introduction point used to extend to it for the ntor
@@ -261,12 +243,6 @@ void hs_desc_encrypted_data_free_(hs_desc_encrypted_data_t *desc);
 #define hs_desc_encrypted_data_free(desc) \
   FREE_AND_NULL(hs_desc_encrypted_data_t, hs_desc_encrypted_data_free_, (desc))
 
-void hs_desc_link_specifier_free_(hs_desc_link_specifier_t *ls);
-#define hs_desc_link_specifier_free(ls) \
-  FREE_AND_NULL(hs_desc_link_specifier_t, hs_desc_link_specifier_free_, (ls))
-
-hs_desc_link_specifier_t *hs_desc_link_specifier_new(
-                                  const extend_info_t *info, uint8_t type);
 void hs_descriptor_clear_intro_points(hs_descriptor_t *desc);
 
 MOCK_DECL(int,
@@ -299,9 +275,6 @@ void hs_desc_authorized_client_free_(hs_desc_authorized_client_t *client);
   FREE_AND_NULL(hs_desc_authorized_client_t, \
                 hs_desc_authorized_client_free_, (client))
 
-link_specifier_t *hs_desc_lspec_to_trunnel(
-                                   const hs_desc_link_specifier_t *spec);
-
 hs_desc_authorized_client_t *hs_desc_build_fake_authorized_client(void);
 void hs_desc_build_authorized_client(const uint8_t *subcredential,
                                      const curve25519_public_key_t *

src/feature/hs/hs_intropoint.c

@@ -601,8 +601,8 @@ hs_intropoint_clear(hs_intropoint_t *ip)
     return;
   }
   tor_cert_free(ip->auth_key_cert);
-  SMARTLIST_FOREACH(ip->link_specifiers, hs_desc_link_specifier_t *, ls,
-                    hs_desc_link_specifier_free(ls));
+  SMARTLIST_FOREACH(ip->link_specifiers, link_specifier_t *, ls,
+                    link_specifier_free(ls));
   smartlist_free(ip->link_specifiers);
   memset(ip, 0, sizeof(hs_intropoint_t));
 }

src/feature/hs/hs_service.c

@@ -280,9 +280,10 @@ describe_intro_point(const hs_service_intro_point_t *ip)
   const char *legacy_id = NULL;
 
   SMARTLIST_FOREACH_BEGIN(ip->base.link_specifiers,
-                          const hs_desc_link_specifier_t *, lspec) {
-    if (lspec->type == LS_LEGACY_ID) {
-      legacy_id = (const char *) lspec->u.legacy_id;
+                          const link_specifier_t *, lspec) {
+    if (link_specifier_get_ls_type(lspec) == LS_LEGACY_ID) {
+      legacy_id = (const char *)
+        link_specifier_getconstarray_un_legacy_id(lspec);
       break;
     }
   } SMARTLIST_FOREACH_END(lspec);
@@ -426,23 +427,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 +466,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 +489,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;
@@ -656,16 +624,16 @@ get_objects_from_ident(const hs_ident_circuit_t *ident,
  * encountered in the link specifier list. Return NULL if it can't be found.
  *
  * The caller does NOT have ownership of the object, the intro point does. */
-static hs_desc_link_specifier_t *
+static link_specifier_t *
 get_link_spec_by_type(const hs_service_intro_point_t *ip, uint8_t type)
 {
-  hs_desc_link_specifier_t *lnk_spec = NULL;
+  link_specifier_t *lnk_spec = NULL;
 
   tor_assert(ip);
 
   SMARTLIST_FOREACH_BEGIN(ip->base.link_specifiers,
-                          hs_desc_link_specifier_t *, ls) {
-    if (ls->type == type) {
+                          link_specifier_t *, ls) {
+    if (link_specifier_get_ls_type(ls) == type) {
       lnk_spec = ls;
       goto end;
     }
@@ -681,7 +649,7 @@ get_link_spec_by_type(const hs_service_intro_point_t *ip, uint8_t type)
 STATIC const node_t *
 get_node_from_intro_point(const hs_service_intro_point_t *ip)
 {
-  const hs_desc_link_specifier_t *ls;
+  const link_specifier_t *ls;
 
   tor_assert(ip);
 
@@ -690,7 +658,8 @@ get_node_from_intro_point(const hs_service_intro_point_t *ip)
     return NULL;
   }
   /* XXX In the future, we want to only use the ed25519 ID (#22173). */
-  return node_get_by_id((const char *) ls->u.legacy_id);
+  return node_get_by_id(
+    (const char *) link_specifier_getconstarray_un_legacy_id(ls));
 }
 
 /* Given a service intro point, return the extend_info_t for it. This can
@@ -1557,7 +1526,7 @@ remember_failing_intro_point(const hs_service_intro_point_t *ip,
                              hs_service_descriptor_t *desc, time_t now)
 {
   time_t *time_of_failure, *prev_ptr;
-  const hs_desc_link_specifier_t *legacy_ls;
+  const link_specifier_t *legacy_ls;
 
   tor_assert(ip);
   tor_assert(desc);
@@ -1566,22 +1535,13 @@ remember_failing_intro_point(const hs_service_intro_point_t *ip,
   *time_of_failure = now;
   legacy_ls = get_link_spec_by_type(ip, LS_LEGACY_ID);
   tor_assert(legacy_ls);
-  prev_ptr = digestmap_set(desc->intro_points.failed_id,
-                           (const char *) legacy_ls->u.legacy_id,
-                           time_of_failure);
+  prev_ptr = digestmap_set(
+    desc->intro_points.failed_id,
+    (const char *) link_specifier_getconstarray_un_legacy_id(legacy_ls),
+    time_of_failure);
   tor_free(prev_ptr);
 }
 
-/* Copy the descriptor link specifier object from src to dst. */
-static void
-link_specifier_copy(hs_desc_link_specifier_t *dst,
-                    const hs_desc_link_specifier_t *src)
-{
-  tor_assert(dst);
-  tor_assert(src);
-  memcpy(dst, src, sizeof(hs_desc_link_specifier_t));
-}
-
 /* Using a given descriptor signing keypair signing_kp, a service intro point
  * object ip and the time now, setup the content of an already allocated
  * descriptor intro desc_ip.
@@ -1616,9 +1576,14 @@ setup_desc_intro_point(const ed25519_keypair_t *signing_kp,
 
   /* Copy link specifier(s). */
   SMARTLIST_FOREACH_BEGIN(ip->base.link_specifiers,
-                          const hs_desc_link_specifier_t *, ls) {
-    hs_desc_link_specifier_t *copy = tor_malloc_zero(sizeof(*copy));
-    link_specifier_copy(copy, ls);
+                          const link_specifier_t *, ls) {
+    if (BUG(!ls)) {
+      goto done;
+    }
+    link_specifier_t *copy = link_specifier_dup(ls);
+    if (BUG(!copy)) {
+      goto done;
+    }
     smartlist_add(desc_ip->link_specifiers, copy);
   } SMARTLIST_FOREACH_END(ls);
 
@@ -2107,7 +2072,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;
@@ -2128,43 +2092,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;
 }
 

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,             \

src/feature/nodelist/nodelist.c

@@ -1189,6 +1189,102 @@ node_get_rsa_id_digest(const node_t *node)
   return (const uint8_t*)node->identity;
 }
 
+/* Returns a new smartlist with all possible link specifiers from node:
+ *  - legacy ID is mandatory thus MUST be present in node;
+ *  - include ed25519 link specifier if present in the node, and the node
+ *    supports ed25519 link authentication, and:
+ *    - if direct_conn is true, its link versions are compatible with us,
+ *    - if direct_conn is false, regardless of its link versions;
+ *  - include IPv4 link specifier, if the primary address is not IPv4, log a
+ *    BUG() warning, and return an empty smartlist;
+ *  - include IPv6 link specifier if present in the node.
+ *
+ * If node is NULL, returns an empty smartlist.
+ *
+ * The smartlist must be freed using link_specifier_smartlist_free(). */
+smartlist_t *
+node_get_link_specifier_smartlist(const node_t *node, bool direct_conn)
+{
+  link_specifier_t *ls;
+  tor_addr_port_t ap;
+  smartlist_t *lspecs = smartlist_new();
+
+  if (!node)
+    return lspecs;
+
+  /* Get the relay's IPv4 address. */
+  node_get_prim_orport(node, &ap);
+
+  /* We expect the node's primary address to be a valid IPv4 address.
+   * This conforms to the protocol, which requires either an IPv4 or IPv6
+   * address (or both). */
+  if (BUG(!tor_addr_is_v4(&ap.addr)) ||
+      BUG(!tor_addr_port_is_valid_ap(&ap, 0))) {
+    return lspecs;
+  }
+
+  ls = link_specifier_new();
+  link_specifier_set_ls_type(ls, LS_IPV4);
+  link_specifier_set_un_ipv4_addr(ls, tor_addr_to_ipv4h(&ap.addr));
+  link_specifier_set_un_ipv4_port(ls, ap.port);
+  /* Four bytes IPv4 and two bytes port. */
+  link_specifier_set_ls_len(ls, sizeof(ap.addr.addr.in_addr) +
+                            sizeof(ap.port));
+  smartlist_add(lspecs, ls);
+
+  /* Legacy ID is mandatory and will always be present in node. */
+  ls = link_specifier_new();
+  link_specifier_set_ls_type(ls, LS_LEGACY_ID);
+  memcpy(link_specifier_getarray_un_legacy_id(ls), node->identity,
+         link_specifier_getlen_un_legacy_id(ls));
+  link_specifier_set_ls_len(ls, link_specifier_getlen_un_legacy_id(ls));
+  smartlist_add(lspecs, ls);
+
+  /* ed25519 ID is only included if the node has it, and the node declares a
+   protocol version that supports ed25519 link authentication.
+   If direct_conn is true, we also require that the node's link version is
+   compatible with us. (Otherwise, we will be sending the ed25519 key
+   to another tor, which may support different link versions.) */
+  if (!ed25519_public_key_is_zero(&node->ed25519_id) &&
+      node_supports_ed25519_link_authentication(node, direct_conn)) {
+    ls = link_specifier_new();
+    link_specifier_set_ls_type(ls, LS_ED25519_ID);
+    memcpy(link_specifier_getarray_un_ed25519_id(ls), &node->ed25519_id,
+           link_specifier_getlen_un_ed25519_id(ls));
+    link_specifier_set_ls_len(ls, link_specifier_getlen_un_ed25519_id(ls));
+    smartlist_add(lspecs, ls);
+  }
+
+  /* Check for IPv6. If so, include it as well. */
+  if (node_has_ipv6_orport(node)) {
+    ls = link_specifier_new();
+    node_get_pref_ipv6_orport(node, &ap);
+    link_specifier_set_ls_type(ls, LS_IPV6);
+    size_t addr_len = link_specifier_getlen_un_ipv6_addr(ls);
+    const uint8_t *in6_addr = tor_addr_to_in6_addr8(&ap.addr);
+    uint8_t *ipv6_array = link_specifier_getarray_un_ipv6_addr(ls);
+    memcpy(ipv6_array, in6_addr, addr_len);
+    link_specifier_set_un_ipv6_port(ls, ap.port);
+    /* Sixteen bytes IPv6 and two bytes port. */
+    link_specifier_set_ls_len(ls, addr_len + sizeof(ap.port));
+    smartlist_add(lspecs, ls);
+  }
+
+  return lspecs;
+}
+
+/* Free a link specifier list. */
+void
+link_specifier_smartlist_free_(smartlist_t *ls_list)
+{
+  if (!ls_list)
+    return;
+
+  SMARTLIST_FOREACH(ls_list, link_specifier_t *, lspec,
+                    link_specifier_free(lspec));
+  smartlist_free(ls_list);
+}
+
 /** Return the nickname of <b>node</b>, or NULL if we can't find one. */
 const char *
 node_get_nickname(const node_t *node)

src/feature/nodelist/nodelist.h

@@ -77,6 +77,11 @@ int node_supports_v3_hsdir(const node_t *node);
 int node_supports_ed25519_hs_intro(const node_t *node);
 int node_supports_v3_rendezvous_point(const node_t *node);
 const uint8_t *node_get_rsa_id_digest(const node_t *node);
+smartlist_t *node_get_link_specifier_smartlist(const node_t *node,
+                                               bool direct_conn);
+void link_specifier_smartlist_free_(smartlist_t *ls_list);
+#define link_specifier_smartlist_free(ls_list) \
+  FREE_AND_NULL(smartlist_t, link_specifier_smartlist_free_, (ls_list))
 
 int node_has_ipv6_addr(const node_t *node);
 int node_has_ipv6_orport(const node_t *node);

src/test/hs_test_helpers.c

@@ -21,26 +21,35 @@ hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now,
   /* For a usable intro point we need at least two link specifiers: One legacy
    * keyid and one ipv4 */
   {
-    hs_desc_link_specifier_t *ls_legacy = tor_malloc_zero(sizeof(*ls_legacy));
-    hs_desc_link_specifier_t *ls_v4 = tor_malloc_zero(sizeof(*ls_v4));
-    ls_legacy->type = LS_LEGACY_ID;
-    memcpy(ls_legacy->u.legacy_id, "0299F268FCA9D55CD157976D39AE92B4B455B3A8",
-           DIGEST_LEN);
-    ls_v4->u.ap.port = 9001;
-    int family = tor_addr_parse(&ls_v4->u.ap.addr, addr);
+    tor_addr_t a;
+    tor_addr_make_unspec(&a);
+    link_specifier_t *ls_legacy = link_specifier_new();
+    link_specifier_t *ls_ip = link_specifier_new();
+    link_specifier_set_ls_type(ls_legacy, LS_LEGACY_ID);
+    memset(link_specifier_getarray_un_legacy_id(ls_legacy), 'C',
+           link_specifier_getlen_un_legacy_id(ls_legacy));
+    int family = tor_addr_parse(&a, addr);
     switch (family) {
     case AF_INET:
-          ls_v4->type = LS_IPV4;
+          link_specifier_set_ls_type(ls_ip, LS_IPV4);
+          link_specifier_set_un_ipv4_addr(ls_ip, tor_addr_to_ipv4h(&a));
+          link_specifier_set_un_ipv4_port(ls_ip, 9001);
           break;
         case AF_INET6:
-          ls_v4->type = LS_IPV6;
+          link_specifier_set_ls_type(ls_ip, LS_IPV6);
+          memcpy(link_specifier_getarray_un_ipv6_addr(ls_ip),
+                 tor_addr_to_in6_addr8(&a),
+                 link_specifier_getlen_un_ipv6_addr(ls_ip));
+          link_specifier_set_un_ipv6_port(ls_ip, 9001);
           break;
         default:
-          /* Stop the test, not suppose to have an error. */
-          tt_int_op(family, OP_EQ, AF_INET);
+          /* Stop the test, not supposed to have an error.
+           * Compare with -1 to show the actual family.
+           */
+          tt_int_op(family, OP_EQ, -1);
     }
     smartlist_add(ip->link_specifiers, ls_legacy);
-    smartlist_add(ip->link_specifiers, ls_v4);
+    smartlist_add(ip->link_specifiers, ls_ip);
   }
 
   ret = ed25519_keypair_generate(&auth_kp, 0);
@@ -153,8 +162,11 @@ 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,
@@ -202,7 +214,6 @@ void
 hs_helper_desc_equal(const hs_descriptor_t *desc1,
                      const hs_descriptor_t *desc2)
 {
-  char *addr1 = NULL, *addr2 = NULL;
   /* Plaintext data section. */
   tt_int_op(desc1->plaintext_data.version, OP_EQ,
             desc2->plaintext_data.version);
@@ -291,35 +302,57 @@ hs_helper_desc_equal(const hs_descriptor_t *desc1,
       tt_int_op(smartlist_len(ip1->link_specifiers), ==,
                 smartlist_len(ip2->link_specifiers));
       for (int j = 0; j < smartlist_len(ip1->link_specifiers); j++) {
-        hs_desc_link_specifier_t *ls1 = smartlist_get(ip1->link_specifiers, j),
-                                 *ls2 = smartlist_get(ip2->link_specifiers, j);
-        tt_int_op(ls1->type, ==, ls2->type);
-        switch (ls1->type) {
+        link_specifier_t *ls1 = smartlist_get(ip1->link_specifiers, j),
+                         *ls2 = smartlist_get(ip2->link_specifiers, j);
+        tt_int_op(link_specifier_get_ls_type(ls1), ==,
+                  link_specifier_get_ls_type(ls2));
+        switch (link_specifier_get_ls_type(ls1)) {
           case LS_IPV4:
+            {
+              uint32_t addr1 = link_specifier_get_un_ipv4_addr(ls1);
+              uint32_t addr2 = link_specifier_get_un_ipv4_addr(ls2);
+              tt_int_op(addr1, OP_EQ, addr2);
+              uint16_t port1 = link_specifier_get_un_ipv4_port(ls1);
+              uint16_t port2 = link_specifier_get_un_ipv4_port(ls2);
+              tt_int_op(port1, ==, port2);
+            }
+            break;
           case LS_IPV6:
             {
-              addr1 = tor_addr_to_str_dup(&ls1->u.ap.addr);
-              addr2 = tor_addr_to_str_dup(&ls2->u.ap.addr);
-              tt_str_op(addr1, OP_EQ, addr2);
-              tor_free(addr1);
-              tor_free(addr2);
-              tt_int_op(ls1->u.ap.port, ==, ls2->u.ap.port);
+              const uint8_t *addr1 =
+                link_specifier_getconstarray_un_ipv6_addr(ls1);
+              const uint8_t *addr2 =
+                link_specifier_getconstarray_un_ipv6_addr(ls2);
+              tt_int_op(link_specifier_getlen_un_ipv6_addr(ls1), OP_EQ,
+                        link_specifier_getlen_un_ipv6_addr(ls2));
+              tt_mem_op(addr1, OP_EQ, addr2,
+                        link_specifier_getlen_un_ipv6_addr(ls1));
+              uint16_t port1 = link_specifier_get_un_ipv6_port(ls1);
+              uint16_t port2 = link_specifier_get_un_ipv6_port(ls2);
+              tt_int_op(port1, ==, port2);
             }
             break;
           case LS_LEGACY_ID:
-            tt_mem_op(ls1->u.legacy_id, OP_EQ, ls2->u.legacy_id,
-                      sizeof(ls1->u.legacy_id));
+            {
+              const uint8_t *id1 =
+                link_specifier_getconstarray_un_legacy_id(ls1);
+              const uint8_t *id2 =
+                link_specifier_getconstarray_un_legacy_id(ls2);
+              tt_int_op(link_specifier_getlen_un_legacy_id(ls1), OP_EQ,
+                        link_specifier_getlen_un_legacy_id(ls2));
+              tt_mem_op(id1, OP_EQ, id2,
+                        link_specifier_getlen_un_legacy_id(ls1));
+            }
             break;
           default:
             /* Unknown type, caught it and print its value. */
-            tt_int_op(ls1->type, OP_EQ, -1);
+            tt_int_op(link_specifier_get_ls_type(ls1), OP_EQ, -1);
         }
       }
     }
   }
 
  done:
-  tor_free(addr1);
-  tor_free(addr2);
+  ;
 }
 

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 "

src/test/test_hs_client.c

@@ -403,6 +403,9 @@ test_client_pick_intro(void *arg)
   /* 2) Mark all intro points except _the chosen one_ as failed. Then query the
    *   desc and get a random intro: check that we got _the chosen one_. */
   {
+    /* Tell hs_get_extend_info_from_lspecs() to skip the private address check.
+     */
+    get_options_mutable()->ExtendAllowPrivateAddresses = 1;
     /* Pick the chosen intro point and get its ei */
     hs_desc_intro_point_t *chosen_intro_point =
       smartlist_get(desc->encrypted_data.intro_points, 0);
@@ -476,6 +479,7 @@ 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);
+      tt_assert(intro_ei);
       if (intro_ei) {
         const char *ptr;
         char ip_addr[TOR_ADDR_BUF_LEN];

src/test/test_hs_descriptor.c

@@ -178,115 +178,6 @@ test_descriptor_padding(void *arg)
   return;
 }
 
-static void
-test_link_specifier(void *arg)
-{
-  ssize_t ret;
-  hs_desc_link_specifier_t spec;
-  smartlist_t *link_specifiers = smartlist_new();
-  char buf[256];
-  char *b64 = NULL;
-  link_specifier_t *ls = NULL;
-
-  (void) arg;
-
-  /* Always this port. */
-  spec.u.ap.port = 42;
-  smartlist_add(link_specifiers, &spec);
-
-  /* Test IPv4 for starter. */
-  {
-    uint32_t ipv4;
-
-    spec.type = LS_IPV4;
-    ret = tor_addr_parse(&spec.u.ap.addr, "1.2.3.4");
-    tt_int_op(ret, OP_EQ, AF_INET);
-    b64 = encode_link_specifiers(link_specifiers);
-    tt_assert(b64);
-
-    /* Decode it and validate the format. */
-    ret = base64_decode(buf, sizeof(buf), b64, strlen(b64));
-    tt_int_op(ret, OP_GT, 0);
-    /* First byte is the number of link specifier. */
-    tt_int_op(get_uint8(buf), OP_EQ, 1);
-    ret = link_specifier_parse(&ls, (uint8_t *) buf + 1, ret - 1);
-    tt_int_op(ret, OP_EQ, 8);
-    /* Should be 2 bytes for port and 4 bytes for IPv4. */
-    tt_int_op(link_specifier_get_ls_len(ls), OP_EQ, 6);
-    ipv4 = link_specifier_get_un_ipv4_addr(ls);
-    tt_int_op(tor_addr_to_ipv4h(&spec.u.ap.addr), OP_EQ, ipv4);
-    tt_int_op(link_specifier_get_un_ipv4_port(ls), OP_EQ, spec.u.ap.port);
-
-    link_specifier_free(ls);
-    ls = NULL;
-    tor_free(b64);
-  }
-
-  /* Test IPv6. */
-  {
-    uint8_t ipv6[16];
-
-    spec.type = LS_IPV6;
-    ret = tor_addr_parse(&spec.u.ap.addr, "[1:2:3:4::]");
-    tt_int_op(ret, OP_EQ, AF_INET6);
-    b64 = encode_link_specifiers(link_specifiers);
-    tt_assert(b64);
-
-    /* Decode it and validate the format. */
-    ret = base64_decode(buf, sizeof(buf), b64, strlen(b64));
-    tt_int_op(ret, OP_GT, 0);
-    /* First byte is the number of link specifier. */
-    tt_int_op(get_uint8(buf), OP_EQ, 1);
-    ret = link_specifier_parse(&ls, (uint8_t *) buf + 1, ret - 1);
-    tt_int_op(ret, OP_EQ, 20);
-    /* Should be 2 bytes for port and 16 bytes for IPv6. */
-    tt_int_op(link_specifier_get_ls_len(ls), OP_EQ, 18);
-    for (unsigned int i = 0; i < sizeof(ipv6); i++) {
-      ipv6[i] = link_specifier_get_un_ipv6_addr(ls, i);
-    }
-    tt_mem_op(tor_addr_to_in6_addr8(&spec.u.ap.addr), OP_EQ, ipv6,
-              sizeof(ipv6));
-    tt_int_op(link_specifier_get_un_ipv6_port(ls), OP_EQ, spec.u.ap.port);
-
-    link_specifier_free(ls);
-    ls = NULL;
-    tor_free(b64);
-  }
-
-  /* Test legacy. */
-  {
-    uint8_t *id;
-
-    spec.type = LS_LEGACY_ID;
-    memset(spec.u.legacy_id, 'Y', sizeof(spec.u.legacy_id));
-    b64 = encode_link_specifiers(link_specifiers);
-    tt_assert(b64);
-
-    /* Decode it and validate the format. */
-    ret = base64_decode(buf, sizeof(buf), b64, strlen(b64));
-    tt_int_op(ret, OP_GT, 0);
-    /* First byte is the number of link specifier. */
-    tt_int_op(get_uint8(buf), OP_EQ, 1);
-    ret = link_specifier_parse(&ls, (uint8_t *) buf + 1, ret - 1);
-    /* 20 bytes digest + 1 byte type + 1 byte len. */
-    tt_int_op(ret, OP_EQ, 22);
-    tt_int_op(link_specifier_getlen_un_legacy_id(ls), OP_EQ, DIGEST_LEN);
-    /* Digest length is 20 bytes. */
-    tt_int_op(link_specifier_get_ls_len(ls), OP_EQ, DIGEST_LEN);
-    id = link_specifier_getarray_un_legacy_id(ls);
-    tt_mem_op(spec.u.legacy_id, OP_EQ, id, DIGEST_LEN);
-
-    link_specifier_free(ls);
-    ls = NULL;
-    tor_free(b64);
-  }
-
- done:
-  link_specifier_free(ls);
-  tor_free(b64);
-  smartlist_free(link_specifiers);
-}
-
 static void
 test_encode_descriptor(void *arg)
 {
@@ -932,8 +823,6 @@ struct testcase_t hs_descriptor[] = {
   /* Encoding tests. */
   { "cert_encoding", test_cert_encoding, TT_FORK,
     NULL, NULL },
-  { "link_specifier", test_link_specifier, TT_FORK,
-    NULL, NULL },
   { "encode_descriptor", test_encode_descriptor, TT_FORK,
     NULL, NULL },
   { "descriptor_padding", test_descriptor_padding, TT_FORK,

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);

src/test/test_hs_service.c

@@ -328,17 +328,18 @@ helper_create_service_with_clients(int num_clients)
 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);
+  link_specifier_t *ls;
+  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));
-  ls->type = LS_IPV4;
+  ls = link_specifier_new();
+  link_specifier_set_ls_type(ls, LS_IPV4);
   smartlist_add(ip->base.link_specifiers, ls);
   /* Add a second link specifier used by a test. */
-  ls = tor_malloc_zero(sizeof(*ls));
-  ls->type = LS_LEGACY_ID;
-  memset(ls->u.legacy_id, 'A', sizeof(ls->u.legacy_id));
+  ls = link_specifier_new();
+  link_specifier_set_ls_type(ls, LS_LEGACY_ID);
+  memset(link_specifier_getarray_un_legacy_id(ls), 'A',
+         link_specifier_getlen_un_legacy_id(ls));
   smartlist_add(ip->base.link_specifiers, ls);
 
   return ip;
@@ -811,10 +812,11 @@ test_helper_functions(void *arg)
     const node_t *node = get_node_from_intro_point(ip);
     tt_ptr_op(node, OP_EQ, &mock_node);
     SMARTLIST_FOREACH_BEGIN(ip->base.link_specifiers,
-                            hs_desc_link_specifier_t *, ls) {
-      if (ls->type == LS_LEGACY_ID) {
+                            link_specifier_t *, ls) {
+      if (link_specifier_get_ls_type(ls) == LS_LEGACY_ID) {
         /* Change legacy id in link specifier which is not the mock node. */
-        memset(ls->u.legacy_id, 'B', sizeof(ls->u.legacy_id));
+        memset(link_specifier_getarray_un_legacy_id(ls), 'B',
+               link_specifier_getlen_un_legacy_id(ls));
       }
     } SMARTLIST_FOREACH_END(ls);
     node = get_node_from_intro_point(ip);

src/trunnel/ed25519_cert.trunnel

@@ -28,12 +28,6 @@ const LS_IPV6 = 0x01;
 const LS_LEGACY_ID = 0x02;
 const LS_ED25519_ID = 0x03;
 
-// XXX hs_link_specifier_dup() violates the opaqueness of link_specifier_t by
-//  taking its sizeof(). If we ever want to turn on TRUNNEL_OPAQUE, or
-//  if we ever make link_specifier contain other types, we will
-//  need to refactor that function to do the copy by encoding and decoding the
-//  object.
-
 // amended from tor.trunnel
 struct link_specifier {
   u8 ls_type;