test: Refactor HS tests to use the new ESTABLISH_INTRO cell code

Signed-off-by: David Goulet <dgoulet@torproject.org>

4 files changed, 200 insertions, 321 deletions

diff --git a/src/or/hs_service.c b/src/or/hs_service.c
index 293c17f6f1..4c7c642e11 100644
--- a/src/or/hs_service.c
+++ b/src/or/hs_service.c
@@ -297,7 +297,7 @@ service_free_all(void)
 }
 
 /* Free a given service intro point object. */
-static void
+STATIC void
 service_intro_point_free(hs_service_intro_point_t *ip)
 {
   if (!ip) {
@@ -322,7 +322,7 @@ service_intro_point_free_(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. On error, NULL is returned. */
-static hs_service_intro_point_t *
+STATIC hs_service_intro_point_t *
 service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy)
 {
   hs_desc_link_specifier_t *ls;
@@ -2747,159 +2747,6 @@ hs_service_free_all(void)
   service_free_all();
 }
 
-/* XXX We don't currently use these functions, apart from generating unittest
-   data. When we start implementing the service-side support for prop224 we
-   should revisit these functions and use them. */
-
-/** Given an ESTABLISH_INTRO <b>cell</b>, encode it and place its payload in
- *  <b>buf_out</b> which has size <b>buf_out_len</b>. Return the number of
- *  bytes written, or a negative integer if there was an error. */
-ssize_t
-get_establish_intro_payload(uint8_t *buf_out, size_t buf_out_len,
-                            const trn_cell_establish_intro_t *cell)
-{
-  ssize_t bytes_used = 0;
-
-  if (buf_out_len < RELAY_PAYLOAD_SIZE) {
-    return -1;
-  }
-
-  bytes_used = trn_cell_establish_intro_encode(buf_out, buf_out_len,
-                                              cell);
-  return bytes_used;
-}
-
-/* Set the cell extensions of <b>cell</b>. */
-static void
-set_trn_cell_extensions(trn_cell_establish_intro_t *cell)
-{
-  trn_cell_extension_t *trn_cell_extensions = trn_cell_extension_new();
-
-  /* For now, we don't use extensions at all. */
-  trn_cell_extensions->num = 0; /* It's already zeroed, but be explicit. */
-  trn_cell_establish_intro_set_extensions(cell, trn_cell_extensions);
-}
-
-/** Given the circuit handshake info in <b>circuit_key_material</b>, create and
- *  return an ESTABLISH_INTRO cell. Return NULL if something went wrong.  The
- *  returned cell is allocated on the heap and it's the responsibility of the
- *  caller to free it. */
-trn_cell_establish_intro_t *
-generate_establish_intro_cell(const uint8_t *circuit_key_material,
-                              size_t circuit_key_material_len)
-{
-  trn_cell_establish_intro_t *cell = NULL;
-  ssize_t encoded_len;
-
-  log_warn(LD_GENERAL,
-           "Generating ESTABLISH_INTRO cell (key_material_len: %u)",
-           (unsigned) circuit_key_material_len);
-
-  /* Generate short-term keypair for use in ESTABLISH_INTRO */
-  ed25519_keypair_t key_struct;
-  if (ed25519_keypair_generate(&key_struct, 0) < 0) {
-    goto err;
-  }
-
-  cell = trn_cell_establish_intro_new();
-
-  /* Set AUTH_KEY_TYPE: 2 means ed25519 */
-  trn_cell_establish_intro_set_auth_key_type(cell,
-                                             HS_INTRO_AUTH_KEY_TYPE_ED25519);
-
-  /* Set AUTH_KEY_LEN field */
-  /* Must also set byte-length of AUTH_KEY to match */
-  int auth_key_len = ED25519_PUBKEY_LEN;
-  trn_cell_establish_intro_set_auth_key_len(cell, auth_key_len);
-  trn_cell_establish_intro_setlen_auth_key(cell, auth_key_len);
-
-  /* Set AUTH_KEY field */
-  uint8_t *auth_key_ptr = trn_cell_establish_intro_getarray_auth_key(cell);
-  memcpy(auth_key_ptr, key_struct.pubkey.pubkey, auth_key_len);
-
-  /* No cell extensions needed */
-  set_trn_cell_extensions(cell);
-
-  /* Set signature size.
-     We need to do this up here, because _encode() needs it and we need to call
-     _encode() to calculate the MAC and signature.
-  */
-  int sig_len = ED25519_SIG_LEN;
-  trn_cell_establish_intro_set_sig_len(cell, sig_len);
-  trn_cell_establish_intro_setlen_sig(cell, sig_len);
-
-  /* XXX How to make this process easier and nicer? */
-
-  /* Calculate the cell MAC (aka HANDSHAKE_AUTH). */
-  {
-    /* To calculate HANDSHAKE_AUTH, we dump the cell in bytes, and then derive
-       the MAC from it. */
-    uint8_t cell_bytes_tmp[RELAY_PAYLOAD_SIZE] = {0};
-    uint8_t mac[TRUNNEL_SHA3_256_LEN];
-
-    encoded_len = trn_cell_establish_intro_encode(cell_bytes_tmp,
-                                                 sizeof(cell_bytes_tmp),
-                                                 cell);
-    if (encoded_len < 0) {
-      log_warn(LD_OR, "Unable to pre-encode ESTABLISH_INTRO cell.");
-      goto err;
-    }
-
-    /* sanity check */
-    tor_assert(encoded_len > ED25519_SIG_LEN + 2 + TRUNNEL_SHA3_256_LEN);
-
-    /* Calculate MAC of all fields before HANDSHAKE_AUTH */
-    crypto_mac_sha3_256(mac, sizeof(mac),
-                        circuit_key_material, circuit_key_material_len,
-                        cell_bytes_tmp,
-                        encoded_len -
-                          (ED25519_SIG_LEN + 2 + TRUNNEL_SHA3_256_LEN));
-    /* Write the MAC to the cell */
-    uint8_t *handshake_ptr =
-      trn_cell_establish_intro_getarray_handshake_mac(cell);
-    memcpy(handshake_ptr, mac, sizeof(mac));
-  }
-
-  /* Calculate the cell signature */
-  {
-    /* To calculate the sig we follow the same procedure as above. We first
-       dump the cell up to the sig, and then calculate the sig */
-    uint8_t cell_bytes_tmp[RELAY_PAYLOAD_SIZE] = {0};
-    ed25519_signature_t sig;
-
-    encoded_len = trn_cell_establish_intro_encode(cell_bytes_tmp,
-                                                 sizeof(cell_bytes_tmp),
-                                                 cell);
-    if (encoded_len < 0) {
-      log_warn(LD_OR, "Unable to pre-encode ESTABLISH_INTRO cell (2).");
-      goto err;
-    }
-
-    tor_assert(encoded_len > ED25519_SIG_LEN);
-
-    if (ed25519_sign_prefixed(&sig,
-                              cell_bytes_tmp,
-                              encoded_len -
-                                (ED25519_SIG_LEN + sizeof(cell->sig_len)),
-                              ESTABLISH_INTRO_SIG_PREFIX,
-                              &key_struct)) {
-      log_warn(LD_BUG, "Unable to gen signature for ESTABLISH_INTRO cell.");
-      goto err;
-    }
-
-    /* And write the signature to the cell */
-    uint8_t *sig_ptr = trn_cell_establish_intro_getarray_sig(cell);
-    memcpy(sig_ptr, sig.sig, sig_len);
-  }
-
-  /* We are done! Return the cell! */
-  return cell;
-
- err:
-  trn_cell_establish_intro_free(cell);
-  return NULL;
-}
-
 #ifdef TOR_UNIT_TESTS
 
 /* Return the global service map size. Only used by unit test. */
diff --git a/src/or/hs_service.h b/src/or/hs_service.h
index f678aa2c0e..fda2ebfc33 100644
--- a/src/or/hs_service.h
+++ b/src/or/hs_service.h
@@ -269,17 +269,6 @@ int hs_service_receive_introduce2(origin_circuit_t *circ,
                                   const uint8_t *payload,
                                   size_t payload_len);
 
-/* These functions are only used by unit tests and we need to expose them else
- * hs_service.o ends up with no symbols in libor.a which makes clang throw a
- * warning at compile time. See #21825. */
-
-trn_cell_establish_intro_t *
-generate_establish_intro_cell(const uint8_t *circuit_key_material,
-                              size_t circuit_key_material_len);
-ssize_t
-get_establish_intro_payload(uint8_t *buf, size_t buf_len,
-                            const trn_cell_establish_intro_t *cell);
-
 #ifdef HS_SERVICE_PRIVATE
 
 #ifdef TOR_UNIT_TESTS
@@ -295,6 +284,10 @@ STATIC hs_service_t *find_service(hs_service_ht *map,
                                   const ed25519_public_key_t *pk);
 STATIC void remove_service(hs_service_ht *map, hs_service_t *service);
 STATIC int register_service(hs_service_ht *map, hs_service_t *service);
+STATIC hs_service_intro_point_t *service_intro_point_new(
+                                         const extend_info_t *ei,
+                                         unsigned int is_legacy);
+STATIC void service_intro_point_free(hs_service_intro_point_t *ip);
 
 #endif /* TOR_UNIT_TESTS */
 
diff --git a/src/test/test_hs_cell.c b/src/test/test_hs_cell.c
index 0bb8c87355..9c963bcb16 100644
--- a/src/test/test_hs_cell.c
+++ b/src/test/test_hs_cell.c
@@ -14,6 +14,7 @@
 #include "log_test_helpers.h"
 
 #include "crypto_ed25519.h"
+#include "hs_cell.h"
 #include "hs_intropoint.h"
 #include "hs_service.h"
 
@@ -26,40 +27,44 @@ static void
 test_gen_establish_intro_cell(void *arg)
 {
   (void) arg;
-  ssize_t retval;
-  uint8_t circuit_key_material[DIGEST_LEN] = {0};
+  ssize_t ret;
+  char circ_nonce[DIGEST_LEN] = {0};
   uint8_t buf[RELAY_PAYLOAD_SIZE];
   trn_cell_establish_intro_t *cell_out = NULL;
   trn_cell_establish_intro_t *cell_in = NULL;
 
-  crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material));
+  crypto_rand(circ_nonce, sizeof(circ_nonce));
 
   /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we
      attempt to parse it. */
   {
-    cell_out = generate_establish_intro_cell(circuit_key_material,
-                                             sizeof(circuit_key_material));
-    tt_assert(cell_out);
-
-    retval = get_establish_intro_payload(buf, sizeof(buf), cell_out);
-    tt_int_op(retval, >=, 0);
+    cell_out = trn_cell_establish_intro_new();
+    /* We only need the auth key pair here. */
+    hs_service_intro_point_t *ip = service_intro_point_new(NULL, 0);
+    /* 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);
+    service_intro_point_free(ip);
+    tt_u64_op(ret, OP_GT, 0);
+
+    ret = trn_cell_establish_intro_encode(buf, sizeof(buf), cell_out);
+    tt_u64_op(ret, OP_GT, 0);
   }
 
   /* Parse it as the receiver */
   {
-    ssize_t parse_result = trn_cell_establish_intro_parse(&cell_in,
-                                                         buf, sizeof(buf));
-    tt_int_op(parse_result, >=, 0);
-
-    retval = verify_establish_intro_cell(cell_in,
-                                         circuit_key_material,
-                                         sizeof(circuit_key_material));
-    tt_int_op(retval, >=, 0);
+    ret = trn_cell_establish_intro_parse(&cell_in, buf, sizeof(buf));
+    tt_u64_op(ret, OP_GT, 0);
+
+    ret = verify_establish_intro_cell(cell_in,
+                                      (const uint8_t *) circ_nonce,
+                                      sizeof(circ_nonce));
+    tt_u64_op(ret, OP_EQ, 0);
   }
 
  done:
-  trn_cell_establish_intro_free(cell_out);
   trn_cell_establish_intro_free(cell_in);
+  trn_cell_establish_intro_free(cell_out);
 }
 
 /* Mocked ed25519_sign_prefixed() function that always fails :) */
@@ -81,22 +86,27 @@ static void
 test_gen_establish_intro_cell_bad(void *arg)
 {
   (void) arg;
+  ssize_t cell_len = 0;
   trn_cell_establish_intro_t *cell = NULL;
-  uint8_t circuit_key_material[DIGEST_LEN] = {0};
+  char circ_nonce[DIGEST_LEN] = {0};
+  hs_service_intro_point_t *ip = NULL;
 
   MOCK(ed25519_sign_prefixed, mock_ed25519_sign_prefixed);
 
-  crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material));
+  crypto_rand(circ_nonce, sizeof(circ_nonce));
 
   setup_full_capture_of_logs(LOG_WARN);
   /* Easiest way to make that function fail is to mock the
      ed25519_sign_prefixed() function and make it fail. */
-  cell = generate_establish_intro_cell(circuit_key_material,
-                                       sizeof(circuit_key_material));
-  expect_log_msg_containing("Unable to gen signature for "
+  cell = trn_cell_establish_intro_new();
+  tt_assert(cell);
+  ip = service_intro_point_new(NULL, 0);
+  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 "
                             "ESTABLISH_INTRO cell.");
   teardown_capture_of_logs();
-  tt_assert(!cell);
+  tt_u64_op(cell_len, OP_EQ, -1);
 
  done:
   trn_cell_establish_intro_free(cell);
diff --git a/src/test/test_hs_intropoint.c b/src/test/test_hs_intropoint.c
index 076d125ffc..09af10904b 100644
--- a/src/test/test_hs_intropoint.c
+++ b/src/test/test_hs_intropoint.c
@@ -17,21 +17,66 @@
 #include "log_test_helpers.h"
 
 #include "or.h"
+#include "circuitlist.h"
+#include "circuituse.h"
 #include "ht.h"
+#include "relay.h"
+#include "rendservice.h"
+
+#include "hs_cell.h"
+#include "hs_circuitmap.h"
+#include "hs_common.h"
+#include "hs_intropoint.h"
+#include "hs_service.h"
 
 /* Trunnel. */
 #include "hs/cell_establish_intro.h"
 #include "hs/cell_introduce1.h"
 #include "hs/cell_common.h"
-#include "hs_service.h"
-#include "hs_common.h"
-#include "hs_circuitmap.h"
-#include "hs_intropoint.h"
 
-#include "circuitlist.h"
-#include "circuituse.h"
-#include "rendservice.h"
-#include "relay.h"
+static size_t
+new_establish_intro_cell(const char *circ_nonce,
+                         trn_cell_establish_intro_t **cell_out)
+{
+  ssize_t cell_len = 0;
+  uint8_t buf[RELAY_PAYLOAD_SIZE] = {0};
+  trn_cell_establish_intro_t *cell = NULL;
+  hs_service_intro_point_t *ip = NULL;
+
+  /* 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);
+  tt_assert(ip);
+  cell_len = hs_cell_build_establish_intro(circ_nonce, ip, buf);
+  tt_u64_op(cell_len, OP_GT, 0);
+
+  cell_len = trn_cell_establish_intro_parse(&cell, buf, sizeof(buf));
+  tt_int_op(cell_len, OP_GT, 0);
+  tt_assert(cell);
+  *cell_out = cell;
+
+ done:
+  service_intro_point_free(ip);
+  return cell_len;
+}
+
+static ssize_t
+new_establish_intro_encoded_cell(const char *circ_nonce, uint8_t *cell_out)
+{
+  ssize_t cell_len = 0;
+  hs_service_intro_point_t *ip = NULL;
+
+  /* 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);
+  tt_assert(ip);
+  cell_len = hs_cell_build_establish_intro(circ_nonce, ip, cell_out);
+  tt_u64_op(cell_len, OP_GT, 0);
+
+ done:
+  service_intro_point_free(ip);
+  return cell_len;
+}
 
 /* Mock function to avoid networking in unittests */
 static int
@@ -122,29 +167,24 @@ static void
 test_establish_intro_wrong_purpose(void *arg)
 {
   int retval;
-  trn_cell_establish_intro_t *establish_intro_cell = NULL;
-  or_circuit_t *intro_circ = or_circuit_new(0,NULL);;
-  uint8_t cell_body[RELAY_PAYLOAD_SIZE];
   ssize_t cell_len = 0;
-  uint8_t circuit_key_material[DIGEST_LEN] = {0};
+  char circ_nonce[DIGEST_LEN] = {0};
+  uint8_t cell_body[RELAY_PAYLOAD_SIZE];
+  or_circuit_t *intro_circ = or_circuit_new(0,NULL);;
 
   (void)arg;
 
   /* Get the auth key of the intro point */
-  crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material));
-  memcpy(intro_circ->rend_circ_nonce, circuit_key_material, DIGEST_LEN);
+  crypto_rand(circ_nonce, sizeof(circ_nonce));
+  memcpy(intro_circ->rend_circ_nonce, circ_nonce, DIGEST_LEN);
 
   /* Set a bad circuit purpose!! :) */
   circuit_change_purpose(TO_CIRCUIT(intro_circ), CIRCUIT_PURPOSE_INTRO_POINT);
 
   /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we
      attempt to parse it. */
-  establish_intro_cell = generate_establish_intro_cell(circuit_key_material,
-                                           sizeof(circuit_key_material));
-  tt_assert(establish_intro_cell);
-  cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body),
-                                         establish_intro_cell);
-  tt_int_op(cell_len, >, 0);
+  cell_len = new_establish_intro_encoded_cell(circ_nonce, cell_body);
+  tt_u64_op(cell_len, OP_GT, 0);
 
   /* Receive the cell. Should fail. */
   setup_full_capture_of_logs(LOG_INFO);
@@ -154,18 +194,16 @@ test_establish_intro_wrong_purpose(void *arg)
   tt_int_op(retval, ==, -1);
 
  done:
-  trn_cell_establish_intro_free(establish_intro_cell);
   circuit_free(TO_CIRCUIT(intro_circ));
 }
 
 /* Prepare a circuit for accepting an ESTABLISH_INTRO cell */
 static void
-helper_prepare_circ_for_intro(or_circuit_t *circ,
-                              uint8_t *circuit_key_material)
+helper_prepare_circ_for_intro(or_circuit_t *circ, const char *circ_nonce)
 {
   /* Prepare the circuit for the incoming ESTABLISH_INTRO */
   circuit_change_purpose(TO_CIRCUIT(circ), CIRCUIT_PURPOSE_OR);
-  memcpy(circ->rend_circ_nonce, circuit_key_material, DIGEST_LEN);
+  memcpy(circ->rend_circ_nonce, circ_nonce, DIGEST_LEN);
 }
 
 /* Send an empty ESTABLISH_INTRO cell. Should fail. */
@@ -174,17 +212,17 @@ test_establish_intro_wrong_keytype(void *arg)
 {
   int retval;
   or_circuit_t *intro_circ = or_circuit_new(0,NULL);;
-  uint8_t circuit_key_material[DIGEST_LEN] = {0};
+  char circ_nonce[DIGEST_LEN] = {0};
 
-  (void)arg;
+  (void) arg;
 
   /* Get the auth key of the intro point */
-  crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material));
-  helper_prepare_circ_for_intro(intro_circ, circuit_key_material);
+  crypto_rand(circ_nonce, sizeof(circ_nonce));
+  helper_prepare_circ_for_intro(intro_circ, circ_nonce);
 
   /* Receive the cell. Should fail. */
   setup_full_capture_of_logs(LOG_INFO);
-  retval = hs_intro_received_establish_intro(intro_circ, (uint8_t*)"", 0);
+  retval = hs_intro_received_establish_intro(intro_circ, (uint8_t *) "", 0);
   expect_log_msg_containing("Empty ESTABLISH_INTRO cell.");
   teardown_capture_of_logs();
   tt_int_op(retval, ==, -1);
@@ -198,26 +236,21 @@ static void
 test_establish_intro_wrong_keytype2(void *arg)
 {
   int retval;
-  trn_cell_establish_intro_t *establish_intro_cell = NULL;
-  or_circuit_t *intro_circ = or_circuit_new(0,NULL);;
+  char circ_nonce[DIGEST_LEN] = {0};
   uint8_t cell_body[RELAY_PAYLOAD_SIZE];
   ssize_t cell_len = 0;
-  uint8_t circuit_key_material[DIGEST_LEN] = {0};
+  or_circuit_t *intro_circ = or_circuit_new(0,NULL);;
 
-  (void)arg;
+  (void) arg;
 
   /* Get the auth key of the intro point */
-  crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material));
-  helper_prepare_circ_for_intro(intro_circ, circuit_key_material);
+  crypto_rand(circ_nonce, sizeof(circ_nonce));
+  helper_prepare_circ_for_intro(intro_circ, circ_nonce);
 
   /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we
-     attempt to parse it. */
-  establish_intro_cell = generate_establish_intro_cell(circuit_key_material,
-                                           sizeof(circuit_key_material));
-  tt_assert(establish_intro_cell);
-  cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body),
-                                         establish_intro_cell);
-  tt_int_op(cell_len, >, 0);
+   * attempt to parse it. */
+  cell_len = new_establish_intro_encoded_cell(circ_nonce, cell_body);
+  tt_u64_op(cell_len, OP_GT, 0);
 
   /* Mutate the auth key type! :) */
   cell_body[0] = 42;
@@ -230,7 +263,6 @@ test_establish_intro_wrong_keytype2(void *arg)
   tt_int_op(retval, ==, -1);
 
  done:
-  trn_cell_establish_intro_free(establish_intro_cell);
   circuit_free(TO_CIRCUIT(intro_circ));
 }
 
@@ -239,26 +271,27 @@ static void
 test_establish_intro_wrong_mac(void *arg)
 {
   int retval;
-  trn_cell_establish_intro_t *establish_intro_cell = NULL;
-  or_circuit_t *intro_circ = or_circuit_new(0,NULL);;
-  uint8_t cell_body[RELAY_PAYLOAD_SIZE];
+  char circ_nonce[DIGEST_LEN] = {0};
   ssize_t cell_len = 0;
-  uint8_t circuit_key_material[DIGEST_LEN] = {0};
+  uint8_t cell_body[RELAY_PAYLOAD_SIZE];
+  trn_cell_establish_intro_t *cell = NULL;
+  or_circuit_t *intro_circ = or_circuit_new(0,NULL);;
 
-  (void)arg;
+  (void) arg;
 
   /* Get the auth key of the intro point */
-  crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material));
-  helper_prepare_circ_for_intro(intro_circ, circuit_key_material);
+  crypto_rand(circ_nonce, sizeof(circ_nonce));
+  helper_prepare_circ_for_intro(intro_circ, circ_nonce);
 
   /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we
-     attempt to parse it. */
-  establish_intro_cell = generate_establish_intro_cell(circuit_key_material,
-                                                sizeof(circuit_key_material));
-  tt_assert(establish_intro_cell);
+   * attempt to parse it. */
+  cell_len = new_establish_intro_cell(circ_nonce, &cell);
+  tt_u64_op(cell_len, OP_GT, 0);
+  tt_assert(cell);
+
   /* Mangle one byte of the MAC. */
   uint8_t *handshake_ptr =
-    trn_cell_establish_intro_getarray_handshake_mac(establish_intro_cell);
+    trn_cell_establish_intro_getarray_handshake_mac(cell);
   handshake_ptr[TRUNNEL_SHA3_256_LEN - 1]++;
   /* We need to resign the payload with that change. */
   {
@@ -269,26 +302,26 @@ test_establish_intro_wrong_mac(void *arg)
     retval = ed25519_keypair_generate(&key_struct, 0);
     tt_int_op(retval, OP_EQ, 0);
     uint8_t *auth_key_ptr =
-      trn_cell_establish_intro_getarray_auth_key(establish_intro_cell);
+      trn_cell_establish_intro_getarray_auth_key(cell);
     memcpy(auth_key_ptr, key_struct.pubkey.pubkey, ED25519_PUBKEY_LEN);
     /* Encode payload so we can sign it. */
-    cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body),
-                                           establish_intro_cell);
-    tt_int_op(cell_len, >, 0);
+    cell_len = trn_cell_establish_intro_encode(cell_body, sizeof(cell_body),
+                                               cell);
+    tt_int_op(cell_len, OP_GT, 0);
 
     retval = ed25519_sign_prefixed(&sig, cell_body,
                                    cell_len -
-                                   (ED25519_SIG_LEN +
-                                    sizeof(establish_intro_cell->sig_len)),
+                                   (ED25519_SIG_LEN + sizeof(cell->sig_len)),
                                    ESTABLISH_INTRO_SIG_PREFIX, &key_struct);
     tt_int_op(retval, OP_EQ, 0);
     /* And write the signature to the cell */
     uint8_t *sig_ptr =
-      trn_cell_establish_intro_getarray_sig(establish_intro_cell);
-    memcpy(sig_ptr, sig.sig, establish_intro_cell->sig_len);
+      trn_cell_establish_intro_getarray_sig(cell);
+    memcpy(sig_ptr, sig.sig, cell->sig_len);
     /* Re-encode with the new signature. */
-    cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body),
-                                           establish_intro_cell);
+    cell_len = trn_cell_establish_intro_encode(cell_body, sizeof(cell_body),
+                                               cell);
+    tt_int_op(cell_len, OP_GT, 0);
   }
 
   /* Receive the cell. Should fail because our MAC is wrong. */
@@ -299,7 +332,7 @@ test_establish_intro_wrong_mac(void *arg)
   tt_int_op(retval, ==, -1);
 
  done:
-  trn_cell_establish_intro_free(establish_intro_cell);
+  trn_cell_establish_intro_free(cell);
   circuit_free(TO_CIRCUIT(intro_circ));
 }
 
@@ -309,32 +342,32 @@ static void
 test_establish_intro_wrong_auth_key_len(void *arg)
 {
   int retval;
-  trn_cell_establish_intro_t *establish_intro_cell = NULL;
-  or_circuit_t *intro_circ = or_circuit_new(0,NULL);;
+  char circ_nonce[DIGEST_LEN] = {0};
   uint8_t cell_body[RELAY_PAYLOAD_SIZE];
   ssize_t cell_len = 0;
   size_t bad_auth_key_len = ED25519_PUBKEY_LEN - 1;
-  uint8_t circuit_key_material[DIGEST_LEN] = {0};
+  trn_cell_establish_intro_t *cell = NULL;
+  or_circuit_t *intro_circ = or_circuit_new(0,NULL);;
 
-  (void)arg;
+  (void) arg;
 
   /* Get the auth key of the intro point */
-  crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material));
-  helper_prepare_circ_for_intro(intro_circ, circuit_key_material);
+  crypto_rand(circ_nonce, sizeof(circ_nonce));
+  helper_prepare_circ_for_intro(intro_circ, circ_nonce);
 
   /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we
-     attempt to parse it. */
-  establish_intro_cell = generate_establish_intro_cell(circuit_key_material,
-                                               sizeof(circuit_key_material));
-  tt_assert(establish_intro_cell);
+   * attempt to parse it. */
+  cell_len = new_establish_intro_cell(circ_nonce, &cell);
+  tt_u64_op(cell_len, OP_GT, 0);
+  tt_assert(cell);
+
   /* Mangle the auth key length. */
-  trn_cell_establish_intro_set_auth_key_len(establish_intro_cell,
-                                           bad_auth_key_len);
-  trn_cell_establish_intro_setlen_auth_key(establish_intro_cell,
-                                          bad_auth_key_len);
-  cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body),
-                                         establish_intro_cell);
-  tt_int_op(cell_len, >, 0);
+  trn_cell_establish_intro_set_auth_key_len(cell, bad_auth_key_len);
+  trn_cell_establish_intro_setlen_auth_key(cell, bad_auth_key_len);
+  /* Encode cell. */
+  cell_len = trn_cell_establish_intro_encode(cell_body, sizeof(cell_body),
+                                             cell);
+  tt_int_op(cell_len, OP_GT, 0);
 
   /* Receive the cell. Should fail. */
   setup_full_capture_of_logs(LOG_INFO);
@@ -344,7 +377,7 @@ test_establish_intro_wrong_auth_key_len(void *arg)
   tt_int_op(retval, ==, -1);
 
  done:
-  trn_cell_establish_intro_free(establish_intro_cell);
+  trn_cell_establish_intro_free(cell);
   circuit_free(TO_CIRCUIT(intro_circ));
 }
 
@@ -354,30 +387,32 @@ static void
 test_establish_intro_wrong_sig_len(void *arg)
 {
   int retval;
-  trn_cell_establish_intro_t *establish_intro_cell = NULL;
-  or_circuit_t *intro_circ = or_circuit_new(0,NULL);;
+  char circ_nonce[DIGEST_LEN] = {0};
   uint8_t cell_body[RELAY_PAYLOAD_SIZE];
   ssize_t cell_len = 0;
   size_t bad_sig_len = ED25519_SIG_LEN - 1;
-  uint8_t circuit_key_material[DIGEST_LEN] = {0};
+  trn_cell_establish_intro_t *cell = NULL;
+  or_circuit_t *intro_circ = or_circuit_new(0,NULL);;
 
-  (void)arg;
+  (void) arg;
 
   /* Get the auth key of the intro point */
-  crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material));
-  helper_prepare_circ_for_intro(intro_circ, circuit_key_material);
+  crypto_rand(circ_nonce, sizeof(circ_nonce));
+  helper_prepare_circ_for_intro(intro_circ, circ_nonce);
 
   /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we
-     attempt to parse it. */
-  establish_intro_cell = generate_establish_intro_cell(circuit_key_material,
-                                               sizeof(circuit_key_material));
-  tt_assert(establish_intro_cell);
+   * attempt to parse it. */
+  cell_len = new_establish_intro_cell(circ_nonce, &cell);
+  tt_u64_op(cell_len, OP_GT, 0);
+  tt_assert(cell);
+
   /* Mangle the signature length. */
-  trn_cell_establish_intro_set_sig_len(establish_intro_cell, bad_sig_len);
-  trn_cell_establish_intro_setlen_sig(establish_intro_cell, bad_sig_len);
-  cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body),
-                                         establish_intro_cell);
-  tt_int_op(cell_len, >, 0);
+  trn_cell_establish_intro_set_sig_len(cell, bad_sig_len);
+  trn_cell_establish_intro_setlen_sig(cell, bad_sig_len);
+  /* Encode cell. */
+  cell_len = trn_cell_establish_intro_encode(cell_body, sizeof(cell_body),
+                                             cell);
+  tt_int_op(cell_len, OP_GT, 0);
 
   /* Receive the cell. Should fail. */
   setup_full_capture_of_logs(LOG_INFO);
@@ -387,7 +422,7 @@ test_establish_intro_wrong_sig_len(void *arg)
   tt_int_op(retval, ==, -1);
 
  done:
-  trn_cell_establish_intro_free(establish_intro_cell);
+  trn_cell_establish_intro_free(cell);
   circuit_free(TO_CIRCUIT(intro_circ));
 }
 
@@ -397,29 +432,24 @@ static void
 test_establish_intro_wrong_sig(void *arg)
 {
   int retval;
-  trn_cell_establish_intro_t *establish_intro_cell = NULL;
-  or_circuit_t *intro_circ = or_circuit_new(0,NULL);;
+  char circ_nonce[DIGEST_LEN] = {0};
   uint8_t cell_body[RELAY_PAYLOAD_SIZE];
   ssize_t cell_len = 0;
-  uint8_t circuit_key_material[DIGEST_LEN] = {0};
+  or_circuit_t *intro_circ = or_circuit_new(0,NULL);;
 
-  (void)arg;
+  (void) arg;
 
   /* Get the auth key of the intro point */
-  crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material));
-  helper_prepare_circ_for_intro(intro_circ, circuit_key_material);
+  crypto_rand(circ_nonce, sizeof(circ_nonce));
+  helper_prepare_circ_for_intro(intro_circ, circ_nonce);
 
   /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we
      attempt to parse it. */
-  establish_intro_cell = generate_establish_intro_cell(circuit_key_material,
-                                           sizeof(circuit_key_material));
-  tt_assert(establish_intro_cell);
-  cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body),
-                                         establish_intro_cell);
-  tt_int_op(cell_len, >, 0);
+  cell_len = new_establish_intro_encoded_cell(circ_nonce, cell_body);
+  tt_u64_op(cell_len, OP_GT, 0);
 
   /* Mutate the last byte (signature)! :) */
-  cell_body[cell_len-1]++;
+  cell_body[cell_len - 1]++;
 
   /* Receive the cell. Should fail. */
   setup_full_capture_of_logs(LOG_INFO);
@@ -429,7 +459,6 @@ test_establish_intro_wrong_sig(void *arg)
   tt_int_op(retval, ==, -1);
 
  done:
-  trn_cell_establish_intro_free(establish_intro_cell);
   circuit_free(TO_CIRCUIT(intro_circ));
 }
 
@@ -439,32 +468,32 @@ static trn_cell_establish_intro_t *
 helper_establish_intro_v3(or_circuit_t *intro_circ)
 {
   int retval;
-  trn_cell_establish_intro_t *establish_intro_cell = NULL;
+  char circ_nonce[DIGEST_LEN] = {0};
   uint8_t cell_body[RELAY_PAYLOAD_SIZE];
   ssize_t cell_len = 0;
-  uint8_t circuit_key_material[DIGEST_LEN] = {0};
+  trn_cell_establish_intro_t *cell = NULL;
 
   tt_assert(intro_circ);
 
   /* Prepare the circuit for the incoming ESTABLISH_INTRO */
-  crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material));
-  helper_prepare_circ_for_intro(intro_circ, circuit_key_material);
+  crypto_rand(circ_nonce, sizeof(circ_nonce));
+  helper_prepare_circ_for_intro(intro_circ, circ_nonce);
 
   /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we
-     attempt to parse it. */
-  establish_intro_cell = generate_establish_intro_cell(circuit_key_material,
-                                           sizeof(circuit_key_material));
-  tt_assert(establish_intro_cell);
-  cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body),
-                                         establish_intro_cell);
-  tt_int_op(cell_len, >, 0);
+   * attempt to parse it. */
+  cell_len = new_establish_intro_cell(circ_nonce, &cell);
+  tt_u64_op(cell_len, OP_GT, 0);
+  tt_assert(cell);
+  cell_len = trn_cell_establish_intro_encode(cell_body, sizeof(cell_body),
+                                             cell);
+  tt_int_op(cell_len, OP_GT, 0);
 
   /* Receive the cell */
   retval = hs_intro_received_establish_intro(intro_circ, cell_body, cell_len);
   tt_int_op(retval, ==, 0);
 
  done:
-  return establish_intro_cell;
+  return cell;
 }
 
 /* Helper function: Send a well-formed v2 ESTABLISH_INTRO cell to
@@ -476,22 +505,22 @@ helper_establish_intro_v2(or_circuit_t *intro_circ)
   int retval;
   uint8_t cell_body[RELAY_PAYLOAD_SIZE];
   ssize_t cell_len = 0;
-  uint8_t circuit_key_material[DIGEST_LEN] = {0};
+  char circ_nonce[DIGEST_LEN] = {0};
 
   tt_assert(intro_circ);
 
   /* Prepare the circuit for the incoming ESTABLISH_INTRO */
-  crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material));
-  helper_prepare_circ_for_intro(intro_circ, circuit_key_material);
+  crypto_rand(circ_nonce, sizeof(circ_nonce));
+  helper_prepare_circ_for_intro(intro_circ, circ_nonce);
 
   /* Send legacy establish_intro */
   key1 = pk_generate(0);
 
-  /* Use old circuit_key_material why not */
+  /* Use old circ_nonce why not */
   cell_len = rend_service_encode_establish_intro_cell(
                                            (char*)cell_body,
                                            sizeof(cell_body), key1,
-                                           (char *) circuit_key_material);
+                                           circ_nonce);
   tt_int_op(cell_len, >, 0);
 
   /* Receive legacy establish_intro */