hs: abolish hs_desc_link_specifier_t

The previous commits for 23576 confused hs_desc_link_specifier_t
and link_specifier_t. Removing hs_desc_link_specifier_t fixes this
confusion.

Fixes bug 22781; bugfix on 0.3.2.1-alpha.

8 files changed, 96 insertions, 236 deletions

diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c
index 597982b34e..e24520da47 100644
--- a/src/feature/hs/hs_cell.c
+++ b/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 = hs_link_specifier_dup(lspec);
+    if (BUG(!lspec_dup)) {
+      goto done;
+    }
+    smartlist_add(data->link_specifiers, lspec_dup);
   }
 
   /* Success. */
diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c
index d4eee50bb7..9d0dee7124 100644
--- a/src/feature/hs/hs_client.c
+++ b/src/feature/hs/hs_client.c
@@ -528,13 +528,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. */
@@ -753,24 +755,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;
 }
 
diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c
index 5d8f54230f..a1dc0a6290 100644
--- a/src/feature/hs/hs_common.c
+++ b/src/feature/hs/hs_common.c
@@ -1843,3 +1843,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;
+}
diff --git a/src/feature/hs/hs_common.h b/src/feature/hs/hs_common.h
index a44505930a..544e63e596 100644
--- a/src/feature/hs/hs_common.h
+++ b/src/feature/hs/hs_common.h
@@ -262,6 +262,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);
diff --git a/src/feature/hs/hs_descriptor.c b/src/feature/hs/hs_descriptor.c
index b09d50e010..8f7bdf86ef 100644
--- a/src/feature/hs/hs_descriptor.c
+++ b/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;
-}
diff --git a/src/feature/hs/hs_descriptor.h b/src/feature/hs/hs_descriptor.h
index 04a8e16d63..dbe0cb1c94 100644
--- a/src/feature/hs/hs_descriptor.h
+++ b/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 *
diff --git a/src/feature/hs/hs_intropoint.c b/src/feature/hs/hs_intropoint.c
index b28a5c2b80..c9cd3a0419 100644
--- a/src/feature/hs/hs_intropoint.c
+++ b/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));
 }
diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c
index 6e4da8856a..8cd863eef4 100644
--- a/src/feature/hs/hs_service.c
+++ b/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);
@@ -623,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;
     }
@@ -648,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);
 
@@ -657,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
@@ -1523,7 +1525,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);
@@ -1532,22 +1534,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.
@@ -1582,9 +1575,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);