From 0cc18ef64cc184d3af0b87378be10f2a8a163fc6 Mon Sep 17 00:00:00 2001
From: David Goulet <dgoulet@torproject.org>
Date: Fri, 7 Apr 2017 10:28:57 -0400
Subject: [PATCH 1/3] test: Move duplicate HS test code and unify it

Create the hs_test_helpers.{c|h} files that contains helper functions to
create introduction point, descriptor and compare descriptor.

Used by both the hs cache and hs descriptor tests. Unify them to avoid code
duplication.

Also, this commit fixes the usage of the signing key that was wrongly used
when creating a cross signed certificate.

Signed-off-by: David Goulet <dgoulet@torproject.org>
---
 src/test/hs_test_helpers.c    | 244 ++++++++++++++++++++++++++++++++++
 src/test/hs_test_helpers.h    |  22 +++
 src/test/include.am           |   1 +
 src/test/test_hs_cache.c      | 103 ++------------
 src/test/test_hs_descriptor.c | 231 +-------------------------------
 5 files changed, 283 insertions(+), 318 deletions(-)
 create mode 100644 src/test/hs_test_helpers.c
 create mode 100644 src/test/hs_test_helpers.h

diff --git a/src/test/hs_test_helpers.c b/src/test/hs_test_helpers.c
new file mode 100644
index 0000000000..a17bf0f0a2
--- /dev/null
+++ b/src/test/hs_test_helpers.c
@@ -0,0 +1,244 @@
+/* Copyright (c) 2017, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#include "or.h"
+#include "crypto_ed25519.h"
+#include "test.h"
+#include "torcert.h"
+
+#include "hs_test_helpers.h"
+
+hs_desc_intro_point_t *
+hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now,
+                            const char *addr, int legacy)
+{
+  int ret;
+  ed25519_keypair_t auth_kp;
+  hs_desc_intro_point_t *intro_point = NULL;
+  hs_desc_intro_point_t *ip = tor_malloc_zero(sizeof(*ip));
+  ip->link_specifiers = smartlist_new();
+
+  {
+    hs_desc_link_specifier_t *ls = tor_malloc_zero(sizeof(*ls));
+    if (legacy) {
+      ls->type = LS_LEGACY_ID;
+      memcpy(ls->u.legacy_id, "0299F268FCA9D55CD157976D39AE92B4B455B3A8",
+             DIGEST_LEN);
+    } else {
+      ls->u.ap.port = 9001;
+      int family = tor_addr_parse(&ls->u.ap.addr, addr);
+      switch (family) {
+        case AF_INET:
+          ls->type = LS_IPV4;
+          break;
+        case AF_INET6:
+          ls->type = LS_IPV6;
+          break;
+        default:
+          /* Stop the test, not suppose to have an error. */
+          tt_int_op(family, OP_EQ, AF_INET);
+      }
+    }
+    smartlist_add(ip->link_specifiers, ls);
+  }
+
+  ret = ed25519_keypair_generate(&auth_kp, 0);
+  tt_int_op(ret, ==, 0);
+  ip->auth_key_cert = tor_cert_create(signing_kp, CERT_TYPE_AUTH_HS_IP_KEY,
+                                      &auth_kp.pubkey, now,
+                                      HS_DESC_CERT_LIFETIME,
+                                      CERT_FLAG_INCLUDE_SIGNING_KEY);
+  tt_assert(ip->auth_key_cert);
+
+  if (legacy) {
+    ip->enc_key.legacy = crypto_pk_new();
+    ip->enc_key_type = HS_DESC_KEY_TYPE_LEGACY;
+    tt_assert(ip->enc_key.legacy);
+    ret = crypto_pk_generate_key(ip->enc_key.legacy);
+    tt_int_op(ret, ==, 0);
+  } else {
+    ret = curve25519_keypair_generate(&ip->enc_key.curve25519, 0);
+    tt_int_op(ret, ==, 0);
+    ip->enc_key_type = HS_DESC_KEY_TYPE_CURVE25519;
+  }
+
+  intro_point = ip;
+ done:
+  return intro_point;
+}
+
+/* Return a valid hs_descriptor_t object. If no_ip is set, no introduction
+ * points are added. */
+static hs_descriptor_t *
+hs_helper_build_hs_desc_impl(unsigned int no_ip,
+                             const ed25519_keypair_t *signing_kp)
+{
+  int ret;
+  time_t now = time(NULL);
+  ed25519_keypair_t blinded_kp;
+  hs_descriptor_t *descp = NULL, *desc = tor_malloc_zero(sizeof(*desc));
+
+  desc->plaintext_data.version = HS_DESC_SUPPORTED_FORMAT_VERSION_MAX;
+
+  /* Copy only the public key into the descriptor. */
+  memcpy(&desc->plaintext_data.signing_pubkey, &signing_kp->pubkey,
+         sizeof(ed25519_public_key_t));
+
+  ret = ed25519_keypair_generate(&blinded_kp, 0);
+  tt_int_op(ret, ==, 0);
+  /* Copy only the public key into the descriptor. */
+  memcpy(&desc->plaintext_data.blinded_pubkey, &blinded_kp.pubkey,
+         sizeof(ed25519_public_key_t));
+
+  desc->plaintext_data.signing_key_cert =
+    tor_cert_create(&blinded_kp, CERT_TYPE_SIGNING_HS_DESC,
+                    &signing_kp->pubkey, now, 3600,
+                    CERT_FLAG_INCLUDE_SIGNING_KEY);
+  tt_assert(desc->plaintext_data.signing_key_cert);
+  desc->plaintext_data.revision_counter = 42;
+  desc->plaintext_data.lifetime_sec = 3 * 60 * 60;
+
+  /* Setup encrypted data section. */
+  desc->encrypted_data.create2_ntor = 1;
+  desc->encrypted_data.intro_auth_types = smartlist_new();
+  desc->encrypted_data.single_onion_service = 1;
+  smartlist_add(desc->encrypted_data.intro_auth_types, tor_strdup("ed25519"));
+  desc->encrypted_data.intro_points = smartlist_new();
+  if (!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));
+    smartlist_add(desc->encrypted_data.intro_points,
+              hs_helper_build_intro_point(signing_kp, now, "[2600::1]", 0));
+    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,
+              hs_helper_build_intro_point(signing_kp, now, "", 1));
+  }
+
+  descp = desc;
+ done:
+  return descp;
+}
+
+/* Build a descriptor with introduction points. */
+hs_descriptor_t *
+hs_helper_build_hs_desc_with_ip(const ed25519_keypair_t *signing_kp)
+{
+  return hs_helper_build_hs_desc_impl(0, signing_kp);
+}
+
+/* Build a descriptor without any introduction points. */
+hs_descriptor_t *
+hs_helper_build_hs_desc_no_ip(const ed25519_keypair_t *signing_kp)
+{
+  return hs_helper_build_hs_desc_impl(1, signing_kp);
+}
+
+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);
+  tt_uint_op(desc1->plaintext_data.lifetime_sec, OP_EQ,
+             desc2->plaintext_data.lifetime_sec);
+  tt_assert(tor_cert_eq(desc1->plaintext_data.signing_key_cert,
+                        desc2->plaintext_data.signing_key_cert));
+  tt_mem_op(desc1->plaintext_data.signing_pubkey.pubkey, OP_EQ,
+            desc2->plaintext_data.signing_pubkey.pubkey,
+            ED25519_PUBKEY_LEN);
+  tt_mem_op(desc1->plaintext_data.blinded_pubkey.pubkey, OP_EQ,
+            desc2->plaintext_data.blinded_pubkey.pubkey,
+            ED25519_PUBKEY_LEN);
+  tt_u64_op(desc1->plaintext_data.revision_counter, ==,
+            desc2->plaintext_data.revision_counter);
+
+  /* NOTE: We can't compare the encrypted blob because when encoding the
+   * descriptor, the object is immutable thus we don't update it with the
+   * encrypted blob. As contrast to the decoding process where we populate a
+   * descriptor object. */
+
+  /* Encrypted data section. */
+  tt_uint_op(desc1->encrypted_data.create2_ntor, ==,
+             desc2->encrypted_data.create2_ntor);
+
+  /* Authentication type. */
+  tt_int_op(!!desc1->encrypted_data.intro_auth_types, ==,
+            !!desc2->encrypted_data.intro_auth_types);
+  if (desc1->encrypted_data.intro_auth_types &&
+      desc2->encrypted_data.intro_auth_types) {
+    tt_int_op(smartlist_len(desc1->encrypted_data.intro_auth_types), ==,
+              smartlist_len(desc2->encrypted_data.intro_auth_types));
+    for (int i = 0;
+         i < smartlist_len(desc1->encrypted_data.intro_auth_types);
+         i++) {
+      tt_str_op(smartlist_get(desc1->encrypted_data.intro_auth_types, i),OP_EQ,
+                smartlist_get(desc2->encrypted_data.intro_auth_types, i));
+    }
+  }
+
+  /* Introduction points. */
+  {
+    tt_assert(desc1->encrypted_data.intro_points);
+    tt_assert(desc2->encrypted_data.intro_points);
+    tt_int_op(smartlist_len(desc1->encrypted_data.intro_points), ==,
+              smartlist_len(desc2->encrypted_data.intro_points));
+    for (int i=0; i < smartlist_len(desc1->encrypted_data.intro_points); i++) {
+      hs_desc_intro_point_t *ip1 = smartlist_get(desc1->encrypted_data
+                                                 .intro_points, i),
+                            *ip2 = smartlist_get(desc2->encrypted_data
+                                                 .intro_points, i);
+      tt_assert(tor_cert_eq(ip1->auth_key_cert, ip2->auth_key_cert));
+      tt_int_op(ip1->enc_key_type, OP_EQ, ip2->enc_key_type);
+      tt_assert(ip1->enc_key_type == HS_DESC_KEY_TYPE_LEGACY ||
+                ip1->enc_key_type == HS_DESC_KEY_TYPE_CURVE25519);
+      switch (ip1->enc_key_type) {
+        case HS_DESC_KEY_TYPE_LEGACY:
+          tt_int_op(crypto_pk_cmp_keys(ip1->enc_key.legacy,
+                                       ip2->enc_key.legacy), OP_EQ, 0);
+          break;
+        case HS_DESC_KEY_TYPE_CURVE25519:
+          tt_mem_op(ip1->enc_key.curve25519.pubkey.public_key, OP_EQ,
+                    ip2->enc_key.curve25519.pubkey.public_key,
+                    CURVE25519_PUBKEY_LEN);
+          break;
+      }
+
+      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) {
+          case LS_IPV4:
+          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);
+            }
+            break;
+          case LS_LEGACY_ID:
+            tt_mem_op(ls1->u.legacy_id, OP_EQ, ls2->u.legacy_id,
+                      sizeof(ls1->u.legacy_id));
+            break;
+          default:
+            /* Unknown type, caught it and print its value. */
+            tt_int_op(ls1->type, OP_EQ, -1);
+        }
+      }
+    }
+  }
+
+ done:
+  tor_free(addr1);
+  tor_free(addr2);
+}
+
diff --git a/src/test/hs_test_helpers.h b/src/test/hs_test_helpers.h
new file mode 100644
index 0000000000..a7fedab136
--- /dev/null
+++ b/src/test/hs_test_helpers.h
@@ -0,0 +1,22 @@
+/* Copyright (c) 2017, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#ifndef TOR_HS_TEST_HELPERS_H
+#define TOR_HS_TEST_HELPERS_H
+
+#include "ed25519_cert.h"
+#include "hs_descriptor.h"
+
+/* Set of functions to help build and test descriptors. */
+hs_desc_intro_point_t *hs_helper_build_intro_point(
+                          const ed25519_keypair_t *signing_kp, time_t now,
+                          const char *addr, int legacy);
+hs_descriptor_t *hs_helper_build_hs_desc_no_ip(
+                                 const ed25519_keypair_t *signing_kp);
+hs_descriptor_t *hs_helper_build_hs_desc_with_ip(
+                                 const ed25519_keypair_t *signing_kp);
+void hs_helper_desc_equal(const hs_descriptor_t *desc1,
+                          const hs_descriptor_t *desc2);
+
+#endif /* TOR_HS_TEST_HELPERS_H */
+
diff --git a/src/test/include.am b/src/test/include.am
index ee14d0651b..c166d3b0fe 100644
--- a/src/test/include.am
+++ b/src/test/include.am
@@ -69,6 +69,7 @@ src_test_AM_CPPFLAGS = -DSHARE_DATADIR="\"$(datadir)\"" \
 
 src_test_test_SOURCES = \
 	src/test/log_test_helpers.c \
+	src/test/hs_test_helpers.c \
 	src/test/rend_test_helpers.c \
 	src/test/test.c \
 	src/test/test_accounting.c \
diff --git a/src/test/test_hs_cache.c b/src/test/test_hs_cache.c
index bc4bfabde9..40f50b322a 100644
--- a/src/test/test_hs_cache.c
+++ b/src/test/test_hs_cache.c
@@ -15,96 +15,10 @@
 #include "directory.h"
 #include "connection.h"
 
+#include "hs_test_helpers.h"
 #include "test_helpers.h"
 #include "test.h"
 
-/* Build an intro point using a blinded key and an address. */
-static hs_desc_intro_point_t *
-helper_build_intro_point(const ed25519_keypair_t *blinded_kp,
-                         const char *addr)
-{
-  int ret;
-  ed25519_keypair_t auth_kp;
-  hs_desc_intro_point_t *intro_point = NULL;
-  hs_desc_intro_point_t *ip = tor_malloc_zero(sizeof(*ip));
-  ip->link_specifiers = smartlist_new();
-
-  {
-    hs_desc_link_specifier_t *ls = tor_malloc_zero(sizeof(*ls));
-    ls->u.ap.port = 9001;
-    int family = tor_addr_parse(&ls->u.ap.addr, addr);
-    switch (family) {
-    case AF_INET:
-      ls->type = LS_IPV4;
-      break;
-    case AF_INET6:
-      ls->type = LS_IPV6;
-      break;
-    default:
-      /* Stop the test, not suppose to have an error. */
-      tt_int_op(family, OP_EQ, AF_INET);
-    }
-    smartlist_add(ip->link_specifiers, ls);
-  }
-
-  ret = ed25519_keypair_generate(&auth_kp, 0);
-  tt_int_op(ret, ==, 0);
-  ip->auth_key_cert = tor_cert_create(blinded_kp, CERT_TYPE_AUTH_HS_IP_KEY,
-                                      &auth_kp.pubkey, time(NULL),
-                                      HS_DESC_CERT_LIFETIME,
-                                      CERT_FLAG_INCLUDE_SIGNING_KEY);
-  tt_assert(ip->auth_key_cert);
-
-  ret = curve25519_keypair_generate(&ip->enc_key.curve25519, 0);
-  tt_int_op(ret, ==, 0);
-  ip->enc_key_type = HS_DESC_KEY_TYPE_CURVE25519;
-  intro_point = ip;
- done:
-  return intro_point;
-}
-
-/* Return a valid hs_descriptor_t object. */
-static hs_descriptor_t *
-helper_build_hs_desc(uint64_t revision_counter, uint32_t lifetime,
-                     ed25519_public_key_t *signing_pubkey)
-{
-  int ret;
-  ed25519_keypair_t blinded_kp;
-  hs_descriptor_t *descp = NULL, *desc = tor_malloc_zero(sizeof(*desc));
-
-  desc->plaintext_data.version = HS_DESC_SUPPORTED_FORMAT_VERSION_MAX;
-
-  /* Copy only the public key into the descriptor. */
-  memcpy(&desc->plaintext_data.signing_pubkey, signing_pubkey,
-         sizeof(ed25519_public_key_t));
-
-  ret = ed25519_keypair_generate(&blinded_kp, 0);
-  tt_int_op(ret, ==, 0);
-  /* Copy only the public key into the descriptor. */
-  memcpy(&desc->plaintext_data.blinded_pubkey, &blinded_kp.pubkey,
-         sizeof(ed25519_public_key_t));
-
-  desc->plaintext_data.signing_key_cert =
-    tor_cert_create(&blinded_kp, CERT_TYPE_SIGNING_HS_DESC, signing_pubkey,
-                    time(NULL), 3600, CERT_FLAG_INCLUDE_SIGNING_KEY);
-  tt_assert(desc->plaintext_data.signing_key_cert);
-  desc->plaintext_data.revision_counter = revision_counter;
-  desc->plaintext_data.lifetime_sec = lifetime;
-
-  /* Setup encrypted data section. */
-  desc->encrypted_data.create2_ntor = 1;
-  desc->encrypted_data.intro_auth_types = smartlist_new();
-  smartlist_add(desc->encrypted_data.intro_auth_types, tor_strdup("ed25519"));
-  desc->encrypted_data.intro_points = smartlist_new();
-  /* Add an intro point. */
-  smartlist_add(desc->encrypted_data.intro_points,
-                helper_build_intro_point(&blinded_kp, "1.2.3.4"));
-
-  descp = desc;
- done:
-  return descp;
-}
-
 /* Static variable used to encoded the HSDir query. */
 static char query_b64[256];
 
@@ -141,7 +55,7 @@ test_directory(void *arg)
   /* Generate a valid descriptor with normal values. */
   ret = ed25519_keypair_generate(&signing_kp1, 0);
   tt_int_op(ret, ==, 0);
-  desc1 = helper_build_hs_desc(42, 3 * 60 * 60, &signing_kp1.pubkey);
+  desc1 = hs_helper_build_hs_desc_with_ip(&signing_kp1);
   tt_assert(desc1);
   ret = hs_desc_encode_descriptor(desc1, &signing_kp1, &desc1_str);
   tt_int_op(ret, OP_EQ, 0);
@@ -175,8 +89,10 @@ test_directory(void *arg)
     ret = ed25519_keypair_generate(&signing_kp_zero, 0);
     tt_int_op(ret, ==, 0);
     hs_descriptor_t *desc_zero_lifetime;
-    desc_zero_lifetime = helper_build_hs_desc(1, 0, &signing_kp_zero.pubkey);
+    desc_zero_lifetime = hs_helper_build_hs_desc_with_ip(&signing_kp_zero);
     tt_assert(desc_zero_lifetime);
+    desc_zero_lifetime->plaintext_data.revision_counter = 1;
+    desc_zero_lifetime->plaintext_data.lifetime_sec = 0;
     char *desc_zero_lifetime_str;
     ret = hs_desc_encode_descriptor(desc_zero_lifetime, &signing_kp_zero,
                                     &desc_zero_lifetime_str);
@@ -262,7 +178,7 @@ test_clean_as_dir(void *arg)
   /* Generate a valid descriptor with values. */
   ret = ed25519_keypair_generate(&signing_kp1, 0);
   tt_int_op(ret, ==, 0);
-  desc1 = helper_build_hs_desc(42, 3 * 60 * 60, &signing_kp1.pubkey);
+  desc1 = hs_helper_build_hs_desc_with_ip(&signing_kp1);
   tt_assert(desc1);
   ret = hs_desc_encode_descriptor(desc1, &signing_kp1, &desc1_str);
   tt_int_op(ret, OP_EQ, 0);
@@ -375,7 +291,7 @@ test_upload_and_download_hs_desc(void *arg)
     ed25519_keypair_t signing_kp;
     retval = ed25519_keypair_generate(&signing_kp, 0);
     tt_int_op(retval, ==, 0);
-    published_desc = helper_build_hs_desc(42, 3 * 60 * 60, &signing_kp.pubkey);
+    published_desc = hs_helper_build_hs_desc_with_ip(&signing_kp);
     tt_assert(published_desc);
     retval = hs_desc_encode_descriptor(published_desc, &signing_kp,
                                        &published_desc_str);
@@ -438,8 +354,7 @@ test_hsdir_revision_counter_check(void *arg)
   {
     retval = ed25519_keypair_generate(&signing_kp, 0);
     tt_int_op(retval, ==, 0);
-    published_desc = helper_build_hs_desc(1312, 3 * 60 * 60,
-                                          &signing_kp.pubkey);
+    published_desc = hs_helper_build_hs_desc_with_ip(&signing_kp);
     tt_assert(published_desc);
     retval = hs_desc_encode_descriptor(published_desc, &signing_kp,
                                        &published_desc_str);
@@ -470,7 +385,7 @@ test_hsdir_revision_counter_check(void *arg)
     tt_assert(received_desc);
 
     /* Check that the revision counter is correct */
-    tt_u64_op(received_desc->plaintext_data.revision_counter, ==, 1312);
+    tt_u64_op(received_desc->plaintext_data.revision_counter, ==, 42);
 
     hs_descriptor_free(received_desc);
     received_desc = NULL;
diff --git a/src/test/test_hs_descriptor.c b/src/test/test_hs_descriptor.c
index a1a1f14fb8..9b364dbc29 100644
--- a/src/test/test_hs_descriptor.c
+++ b/src/test/test_hs_descriptor.c
@@ -15,227 +15,10 @@
 #include "test.h"
 #include "torcert.h"
 
+#include "hs_test_helpers.h"
 #include "test_helpers.h"
 #include "log_test_helpers.h"
 
-static hs_desc_intro_point_t *
-helper_build_intro_point(const ed25519_keypair_t *blinded_kp, time_t now,
-                         const char *addr, int legacy)
-{
-  int ret;
-  ed25519_keypair_t auth_kp;
-  hs_desc_intro_point_t *intro_point = NULL;
-  hs_desc_intro_point_t *ip = tor_malloc_zero(sizeof(*ip));
-  ip->link_specifiers = smartlist_new();
-
-  {
-    hs_desc_link_specifier_t *ls = tor_malloc_zero(sizeof(*ls));
-    if (legacy) {
-      ls->type = LS_LEGACY_ID;
-      memcpy(ls->u.legacy_id, "0299F268FCA9D55CD157976D39AE92B4B455B3A8",
-             DIGEST_LEN);
-    } else {
-      ls->u.ap.port = 9001;
-      int family = tor_addr_parse(&ls->u.ap.addr, addr);
-      switch (family) {
-      case AF_INET:
-        ls->type = LS_IPV4;
-        break;
-      case AF_INET6:
-        ls->type = LS_IPV6;
-        break;
-      default:
-        /* Stop the test, not suppose to have an error. */
-        tt_int_op(family, OP_EQ, AF_INET);
-      }
-    }
-    smartlist_add(ip->link_specifiers, ls);
-  }
-
-  ret = ed25519_keypair_generate(&auth_kp, 0);
-  tt_int_op(ret, ==, 0);
-  ip->auth_key_cert = tor_cert_create(blinded_kp, CERT_TYPE_AUTH_HS_IP_KEY,
-                                      &auth_kp.pubkey, now,
-                                      HS_DESC_CERT_LIFETIME,
-                                      CERT_FLAG_INCLUDE_SIGNING_KEY);
-  tt_assert(ip->auth_key_cert);
-
-  if (legacy) {
-    ip->enc_key.legacy = crypto_pk_new();
-    ip->enc_key_type = HS_DESC_KEY_TYPE_LEGACY;
-    tt_assert(ip->enc_key.legacy);
-    ret = crypto_pk_generate_key(ip->enc_key.legacy);
-    tt_int_op(ret, ==, 0);
-  } else {
-    ret = curve25519_keypair_generate(&ip->enc_key.curve25519, 0);
-    tt_int_op(ret, ==, 0);
-    ip->enc_key_type = HS_DESC_KEY_TYPE_CURVE25519;
-  }
-
-  intro_point = ip;
- done:
-  return intro_point;
-}
-
-/* Return a valid hs_descriptor_t object. If no_ip is set, no introduction
- * points are added. */
-static hs_descriptor_t *
-helper_build_hs_desc(unsigned int no_ip, ed25519_public_key_t *signing_pubkey)
-{
-  int ret;
-  time_t now = time(NULL);
-  ed25519_keypair_t blinded_kp;
-  hs_descriptor_t *descp = NULL, *desc = tor_malloc_zero(sizeof(*desc));
-
-  desc->plaintext_data.version = HS_DESC_SUPPORTED_FORMAT_VERSION_MAX;
-
-  /* Copy only the public key into the descriptor. */
-  memcpy(&desc->plaintext_data.signing_pubkey, signing_pubkey,
-         sizeof(ed25519_public_key_t));
-
-  ret = ed25519_keypair_generate(&blinded_kp, 0);
-  tt_int_op(ret, ==, 0);
-  /* Copy only the public key into the descriptor. */
-  memcpy(&desc->plaintext_data.blinded_pubkey, &blinded_kp.pubkey,
-         sizeof(ed25519_public_key_t));
-
-  desc->plaintext_data.signing_key_cert =
-    tor_cert_create(&blinded_kp, CERT_TYPE_SIGNING_HS_DESC, signing_pubkey,
-                    now, 3600, CERT_FLAG_INCLUDE_SIGNING_KEY);
-  tt_assert(desc->plaintext_data.signing_key_cert);
-  desc->plaintext_data.revision_counter = 42;
-  desc->plaintext_data.lifetime_sec = 3 * 60 * 60;
-
-  /* Setup encrypted data section. */
-  desc->encrypted_data.create2_ntor = 1;
-  desc->encrypted_data.intro_auth_types = smartlist_new();
-  desc->encrypted_data.single_onion_service = 1;
-  smartlist_add(desc->encrypted_data.intro_auth_types, tor_strdup("ed25519"));
-  desc->encrypted_data.intro_points = smartlist_new();
-  if (!no_ip) {
-    /* Add four intro points. */
-    smartlist_add(desc->encrypted_data.intro_points,
-                helper_build_intro_point(&blinded_kp, now, "1.2.3.4", 0));
-    smartlist_add(desc->encrypted_data.intro_points,
-                helper_build_intro_point(&blinded_kp, now, "[2600::1]", 0));
-    smartlist_add(desc->encrypted_data.intro_points,
-                helper_build_intro_point(&blinded_kp, now, "3.2.1.4", 1));
-    smartlist_add(desc->encrypted_data.intro_points,
-                helper_build_intro_point(&blinded_kp, now, "", 1));
-  }
-
-  descp = desc;
- done:
-  return descp;
-}
-
-static void
-helper_compare_hs_desc(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);
-  tt_uint_op(desc1->plaintext_data.lifetime_sec, OP_EQ,
-             desc2->plaintext_data.lifetime_sec);
-  tt_assert(tor_cert_eq(desc1->plaintext_data.signing_key_cert,
-                        desc2->plaintext_data.signing_key_cert));
-  tt_mem_op(desc1->plaintext_data.signing_pubkey.pubkey, OP_EQ,
-            desc2->plaintext_data.signing_pubkey.pubkey,
-            ED25519_PUBKEY_LEN);
-  tt_mem_op(desc1->plaintext_data.blinded_pubkey.pubkey, OP_EQ,
-            desc2->plaintext_data.blinded_pubkey.pubkey,
-            ED25519_PUBKEY_LEN);
-  tt_u64_op(desc1->plaintext_data.revision_counter, ==,
-             desc2->plaintext_data.revision_counter);
-
-  /* NOTE: We can't compare the encrypted blob because when encoding the
-   * descriptor, the object is immutable thus we don't update it with the
-   * encrypted blob. As contrast to the decoding process where we populate a
-   * descriptor object. */
-
-  /* Encrypted data section. */
-  tt_uint_op(desc1->encrypted_data.create2_ntor, ==,
-             desc2->encrypted_data.create2_ntor);
-
-  /* Authentication type. */
-  tt_int_op(!!desc1->encrypted_data.intro_auth_types, ==,
-            !!desc2->encrypted_data.intro_auth_types);
-  if (desc1->encrypted_data.intro_auth_types &&
-      desc2->encrypted_data.intro_auth_types) {
-    tt_int_op(smartlist_len(desc1->encrypted_data.intro_auth_types), ==,
-              smartlist_len(desc2->encrypted_data.intro_auth_types));
-    for (int i = 0;
-         i < smartlist_len(desc1->encrypted_data.intro_auth_types);
-         i++) {
-      tt_str_op(smartlist_get(desc1->encrypted_data.intro_auth_types, i),OP_EQ,
-                smartlist_get(desc2->encrypted_data.intro_auth_types, i));
-    }
-  }
-
-  /* Introduction points. */
-  {
-    tt_assert(desc1->encrypted_data.intro_points);
-    tt_assert(desc2->encrypted_data.intro_points);
-    tt_int_op(smartlist_len(desc1->encrypted_data.intro_points), ==,
-              smartlist_len(desc2->encrypted_data.intro_points));
-    for (int i=0; i < smartlist_len(desc1->encrypted_data.intro_points); i++) {
-      hs_desc_intro_point_t *ip1 = smartlist_get(desc1->encrypted_data
-                                                 .intro_points, i),
-                            *ip2 = smartlist_get(desc2->encrypted_data
-                                                 .intro_points, i);
-      tt_assert(tor_cert_eq(ip1->auth_key_cert, ip2->auth_key_cert));
-      tt_int_op(ip1->enc_key_type, OP_EQ, ip2->enc_key_type);
-      tt_assert(ip1->enc_key_type == HS_DESC_KEY_TYPE_LEGACY ||
-                ip1->enc_key_type == HS_DESC_KEY_TYPE_CURVE25519);
-      switch (ip1->enc_key_type) {
-      case HS_DESC_KEY_TYPE_LEGACY:
-        tt_int_op(crypto_pk_cmp_keys(ip1->enc_key.legacy, ip2->enc_key.legacy),
-                  OP_EQ, 0);
-        break;
-      case HS_DESC_KEY_TYPE_CURVE25519:
-        tt_mem_op(ip1->enc_key.curve25519.pubkey.public_key, OP_EQ,
-                  ip2->enc_key.curve25519.pubkey.public_key,
-                  CURVE25519_PUBKEY_LEN);
-        break;
-      }
-
-      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) {
-        case LS_IPV4:
-        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);
-        }
-        break;
-        case LS_LEGACY_ID:
-          tt_mem_op(ls1->u.legacy_id, OP_EQ, ls2->u.legacy_id,
-                    sizeof(ls1->u.legacy_id));
-          break;
-        default:
-          /* Unknown type, caught it and print its value. */
-          tt_int_op(ls1->type, OP_EQ, -1);
-        }
-      }
-    }
-  }
-
- done:
-  tor_free(addr1);
-  tor_free(addr2);
-}
-
 /* Test certificate encoding put in a descriptor. */
 static void
 test_cert_encoding(void *arg)
@@ -494,7 +277,7 @@ test_encode_descriptor(void *arg)
 
   ret = ed25519_keypair_generate(&signing_kp, 0);
   tt_int_op(ret, ==, 0);
-  desc = helper_build_hs_desc(0, &signing_kp.pubkey);
+  desc = hs_helper_build_hs_desc_with_ip(&signing_kp);
   ret = hs_desc_encode_descriptor(desc, &signing_kp, &encoded);
   tt_int_op(ret, ==, 0);
   tt_assert(encoded);
@@ -518,7 +301,7 @@ test_decode_descriptor(void *arg)
 
   ret = ed25519_keypair_generate(&signing_kp, 0);
   tt_int_op(ret, ==, 0);
-  desc = helper_build_hs_desc(0, &signing_kp.pubkey);
+  desc = hs_helper_build_hs_desc_with_ip(&signing_kp);
 
   /* Give some bad stuff to the decoding function. */
   ret = hs_desc_decode_descriptor("hladfjlkjadf", NULL, &decoded);
@@ -532,14 +315,14 @@ test_decode_descriptor(void *arg)
   tt_int_op(ret, ==, 0);
   tt_assert(decoded);
 
-  helper_compare_hs_desc(desc, decoded);
+  hs_helper_desc_equal(desc, decoded);
 
   /* Decode a descriptor with _no_ introduction points. */
   {
     ed25519_keypair_t signing_kp_no_ip;
     ret = ed25519_keypair_generate(&signing_kp_no_ip, 0);
     tt_int_op(ret, ==, 0);
-    desc_no_ip = helper_build_hs_desc(1, &signing_kp_no_ip.pubkey);
+    desc_no_ip = hs_helper_build_hs_desc_no_ip(&signing_kp_no_ip);
     tt_assert(desc_no_ip);
     tor_free(encoded);
     ret = hs_desc_encode_descriptor(desc_no_ip, &signing_kp_no_ip, &encoded);
@@ -660,7 +443,7 @@ test_decode_intro_point(void *arg)
     char *line;
     ret = ed25519_keypair_generate(&signing_kp, 0);
     tt_int_op(ret, ==, 0);
-    desc = helper_build_hs_desc(0, &signing_kp.pubkey);
+    desc = hs_helper_build_hs_desc_with_ip(&signing_kp);
     tt_assert(desc);
     /* Only try to decode an incomplete introduction point section. */
     tor_asprintf(&line, "\n%s", intro_point);
@@ -690,7 +473,7 @@ test_decode_intro_point(void *arg)
     desc = NULL;
     ret = ed25519_keypair_generate(&signing_kp, 0);
     tt_int_op(ret, ==, 0);
-    desc = helper_build_hs_desc(0, &signing_kp.pubkey);
+    desc = hs_helper_build_hs_desc_with_ip(&signing_kp);
     const char *junk = "this is not a descriptor";
     ip = decode_introduction_point(desc, junk);
     tt_assert(!ip);

From 0958e3b208badb8f24c382e320e2a40d4ab5de86 Mon Sep 17 00:00:00 2001
From: David Goulet <dgoulet@torproject.org>
Date: Fri, 7 Apr 2017 10:57:27 -0400
Subject: [PATCH 2/3] test: Remove HS decode valid intro point test

The descriptor fields can't be validated properly during encoding because they
are signed by a descriptor signing key that we don't have in the unit test.

Removing the test case for now but ultimately we need an independent
implementation that can encode descriptor and test our decoding functions with
that.

Signed-off-by: David Goulet <dgoulet@torproject.org>
---
 src/test/test_hs_descriptor.c | 72 ++---------------------------------
 1 file changed, 3 insertions(+), 69 deletions(-)

diff --git a/src/test/test_hs_descriptor.c b/src/test/test_hs_descriptor.c
index 9b364dbc29..b4c58937c4 100644
--- a/src/test/test_hs_descriptor.c
+++ b/src/test/test_hs_descriptor.c
@@ -386,7 +386,7 @@ test_encrypted_data_len(void *arg)
 }
 
 static void
-test_decode_intro_point(void *arg)
+test_decode_invalid_intro_point(void *arg)
 {
   int ret;
   char *encoded_ip = NULL;
@@ -397,9 +397,6 @@ test_decode_intro_point(void *arg)
 
   (void) arg;
 
-  /* The following certificate expires in 2036. After that, one of the test
-   * will fail because of the expiry time. */
-
   /* Seperate pieces of a valid encoded introduction point. */
   const char *intro_point =
     "introduction-point AQIUMDI5OUYyNjhGQ0E5RDU1Q0QxNTc=";
@@ -412,13 +409,6 @@ test_decode_intro_point(void *arg)
     "-----END ED25519 CERT-----";
   const char *enc_key =
     "enc-key ntor bpZKLsuhxP6woDQ3yVyjm5gUKSk7RjfAijT2qrzbQk0=";
-  const char *enc_key_legacy =
-    "enc-key legacy\n"
-    "-----BEGIN RSA PUBLIC KEY-----\n"
-    "MIGJAoGBAO4bATcW8kW4h6RQQAKEgg+aXCpF4JwbcO6vGZtzXTDB+HdPVQzwqkbh\n"
-    "XzFM6VGArhYw4m31wcP1Z7IwULir7UMnAFd7Zi62aYfU6l+Y1yAoZ1wzu1XBaAMK\n"
-    "ejpwQinW9nzJn7c2f69fVke3pkhxpNdUZ+vplSA/l9iY+y+v+415AgMBAAE=\n"
-    "-----END RSA PUBLIC KEY-----";
   const char *enc_key_cert =
     "enc-key-certification\n"
     "-----BEGIN ED25519 CERT-----\n"
@@ -426,46 +416,6 @@ test_decode_intro_point(void *arg)
     "lbTt1DF5nKTE/gU3Fr8ZtlCIOhu1A+F5LM7fqCUupfesg0KTHwyIZOYQbJuM5/he\n"
     "/jDNyLy9woPJdjkxywaY2RPUxGjLYtMQV0E8PUxWyICV+7y52fTCYaKpYQw=\n"
     "-----END ED25519 CERT-----";
-  const char *enc_key_cert_legacy =
-    "enc-key-certification\n"
-    "-----BEGIN CROSSCERT-----\n"
-    "Sk28JnVolppHj2VLowJ2xWSFUZWtGqiPRjZPhLOugC0ACOhZgFPA5egeRDUXMM1U\n"
-    "Fn3c7Je0gJS6mVma5FzwlgwggeriF13UZcaT71vEAN/ZJXbxOfQVGMZ0rXuFpjUq\n"
-    "C8CvqmZIwEUaPE1nDFtmnTcucvNS1YQl9nsjH3ejbxc+4yqps/cXh46FmXsm5yz7\n"
-    "NZjBM9U1fbJhlNtOvrkf70K8bLk6\n"
-    "-----END CROSSCERT-----";
-
-  (void) enc_key_legacy;
-  (void) enc_key_cert_legacy;
-
-  /* Start by testing the "decode all intro points" function. */
-  {
-    char *line;
-    ret = ed25519_keypair_generate(&signing_kp, 0);
-    tt_int_op(ret, ==, 0);
-    desc = hs_helper_build_hs_desc_with_ip(&signing_kp);
-    tt_assert(desc);
-    /* Only try to decode an incomplete introduction point section. */
-    tor_asprintf(&line, "\n%s", intro_point);
-    ret = decode_intro_points(desc, &desc->encrypted_data, line);
-    tor_free(line);
-    tt_int_op(ret, ==, -1);
-
-    /* Decode one complete intro point. */
-    smartlist_t *lines = smartlist_new();
-    smartlist_add(lines, (char *) intro_point);
-    smartlist_add(lines, (char *) auth_key);
-    smartlist_add(lines, (char *) enc_key);
-    smartlist_add(lines, (char *) enc_key_cert);
-    encoded_ip = smartlist_join_strings(lines, "\n", 0, &len_out);
-    tt_assert(encoded_ip);
-    tor_asprintf(&line, "\n%s", encoded_ip);
-    tor_free(encoded_ip);
-    ret = decode_intro_points(desc, &desc->encrypted_data, line);
-    tor_free(line);
-    smartlist_free(lines);
-    tt_int_op(ret, ==, 0);
-  }
 
   /* Try to decode a junk string. */
   {
@@ -579,7 +529,7 @@ test_decode_intro_point(void *arg)
   /* Invalid enc-key invalid legacy. */
   {
     smartlist_t *lines = smartlist_new();
-    const char *bad_line = "enc-key legacy blah===";
+    const char *bad_line = "legacy-key blah===";
     /* Build intro point text. */
     smartlist_add(lines, (char *) intro_point);
     smartlist_add(lines, (char *) auth_key);
@@ -593,22 +543,6 @@ test_decode_intro_point(void *arg)
     smartlist_free(lines);
   }
 
-  /* Valid object. */
-  {
-    smartlist_t *lines = smartlist_new();
-    /* Build intro point text. */
-    smartlist_add(lines, (char *) intro_point);
-    smartlist_add(lines, (char *) auth_key);
-    smartlist_add(lines, (char *) enc_key);
-    smartlist_add(lines, (char *) enc_key_cert);
-    encoded_ip = smartlist_join_strings(lines, "\n", 0, &len_out);
-    tt_assert(encoded_ip);
-    ip = decode_introduction_point(desc, encoded_ip);
-    tt_assert(ip);
-    tor_free(encoded_ip);
-    smartlist_free(lines);
-  }
-
  done:
   hs_descriptor_free(desc);
   desc_intro_point_free(ip);
@@ -900,7 +834,7 @@ struct testcase_t hs_descriptor[] = {
     NULL, NULL },
   { "encrypted_data_len", test_encrypted_data_len, TT_FORK,
     NULL, NULL },
-  { "decode_intro_point", test_decode_intro_point, TT_FORK,
+  { "decode_invalid_intro_point", test_decode_invalid_intro_point, TT_FORK,
     NULL, NULL },
   { "decode_plaintext", test_decode_plaintext, TT_FORK,
     NULL, NULL },

From ae1d4cfdadb32f145f842e0ee943042eac428c93 Mon Sep 17 00:00:00 2001
From: David Goulet <dgoulet@torproject.org>
Date: Fri, 10 Feb 2017 14:24:54 -0500
Subject: [PATCH 3/3] prop224: Change encryption keys descriptor encoding

A descriptor only contains the curve25519 public key in the enc-key field so
the private key should not be in that data structure. The service data
structures will have access to the full keypair (#20657).

Furthermore, ticket #21871 has highlighted an issue in the proposal 224 about
the encryption key and legacy key being mutually exclusive. This is very wrong
and this commit fixes the code to follow the change to the proposal of that
ticket.

Signed-off-by: David Goulet <dgoulet@torproject.org>
---
 src/or/hs_descriptor.c        | 319 ++++++++++++++++++----------------
 src/or/hs_descriptor.h        |  37 ++--
 src/or/parsecommon.h          |   4 +-
 src/test/hs_test_helpers.c    |  53 +++---
 src/test/test_hs_descriptor.c |   2 +-
 5 files changed, 230 insertions(+), 185 deletions(-)

diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c
index 0480f63308..2a000f5002 100644
--- a/src/or/hs_descriptor.c
+++ b/src/or/hs_descriptor.c
@@ -79,7 +79,9 @@
 #define str_intro_point "introduction-point"
 #define str_ip_auth_key "auth-key"
 #define str_ip_enc_key "enc-key"
-#define str_ip_enc_key_cert "enc-key-certification"
+#define str_ip_enc_key_cert "enc-key-cert"
+#define str_ip_legacy_key "legacy-key"
+#define str_ip_legacy_key_cert "legacy-key-cert"
 #define str_intro_point_start "\n" str_intro_point " "
 /* Constant string value for the construction to encrypt the encrypted data
  * section. */
@@ -134,9 +136,10 @@ static token_rule_t hs_desc_encrypted_v3_token_table[] = {
 static token_rule_t hs_desc_intro_point_v3_token_table[] = {
   T1_START(str_intro_point, R3_INTRODUCTION_POINT, EQ(1), NO_OBJ),
   T1(str_ip_auth_key, R3_INTRO_AUTH_KEY, NO_ARGS, NEED_OBJ),
-  T1(str_ip_enc_key, R3_INTRO_ENC_KEY, ARGS, OBJ_OK),
-  T1_END(str_ip_enc_key_cert, R3_INTRO_ENC_KEY_CERTIFICATION,
-         NO_ARGS, NEED_OBJ),
+  T1(str_ip_enc_key, R3_INTRO_ENC_KEY, GE(2), OBJ_OK),
+  T1(str_ip_enc_key_cert, R3_INTRO_ENC_KEY_CERT, ARGS, OBJ_OK),
+  T01(str_ip_legacy_key, R3_INTRO_LEGACY_KEY, ARGS, NEED_KEY_1024),
+  T01(str_ip_legacy_key_cert, R3_INTRO_LEGACY_KEY_CERT, ARGS, OBJ_OK),
   END_OF_TABLE
 };
 
@@ -153,8 +156,12 @@ desc_intro_point_free(hs_desc_intro_point_t *ip)
     smartlist_free(ip->link_specifiers);
   }
   tor_cert_free(ip->auth_key_cert);
-  if (ip->enc_key_type == HS_DESC_KEY_TYPE_LEGACY) {
-    crypto_pk_free(ip->enc_key.legacy);
+  tor_cert_free(ip->enc_key_cert);
+  if (ip->legacy.key) {
+    crypto_pk_free(ip->legacy.key);
+  }
+  if (ip->legacy.cert.encoded) {
+    tor_free(ip->legacy.cert.encoded);
   }
   tor_free(ip);
 }
@@ -406,101 +413,68 @@ encode_link_specifiers(const smartlist_t *specs)
   return encoded_b64;
 }
 
-/* Encode an introduction point encryption key and return a newly allocated
- * string with it. On failure, return NULL. */
+/* Encode an introduction point legacy key and certificate. Return a newly
+ * allocated string with it. On failure, return NULL. */
 static char *
-encode_enc_key(const ed25519_public_key_t *sig_key,
-               const hs_desc_intro_point_t *ip)
+encode_legacy_key(const hs_desc_intro_point_t *ip)
 {
-  char *encoded = NULL;
-  time_t now = time(NULL);
+  char *key_str, b64_cert[256], *encoded = NULL;
+  size_t key_str_len;
 
-  tor_assert(sig_key);
   tor_assert(ip);
 
-  switch (ip->enc_key_type) {
-  case HS_DESC_KEY_TYPE_LEGACY:
-  {
-    char *key_str, b64_cert[256];
-    ssize_t cert_len;
-    size_t key_str_len;
-    uint8_t *cert_data = NULL;
-
-    /* Create cross certification cert. */
-    cert_len = tor_make_rsa_ed25519_crosscert(sig_key, ip->enc_key.legacy,
-                                              now + HS_DESC_CERT_LIFETIME,
-                                              &cert_data);
-    if (cert_len < 0) {
-      log_warn(LD_REND, "Unable to create legacy crosscert.");
-      goto err;
-    }
-    /* Encode cross cert. */
-    if (base64_encode(b64_cert, sizeof(b64_cert), (const char *) cert_data,
-                      cert_len, BASE64_ENCODE_MULTILINE) < 0) {
-      tor_free(cert_data);
-      log_warn(LD_REND, "Unable to encode legacy crosscert.");
-      goto err;
-    }
-    tor_free(cert_data);
-    /* Convert the encryption key to a string. */
-    if (crypto_pk_write_public_key_to_string(ip->enc_key.legacy, &key_str,
-                                             &key_str_len) < 0) {
-      log_warn(LD_REND, "Unable to encode legacy encryption key.");
-      goto err;
-    }
-    tor_asprintf(&encoded,
-                 "%s legacy\n%s"  /* Newline is added by the call above. */
-                 "%s\n"
-                 "-----BEGIN CROSSCERT-----\n"
-                 "%s"
-                 "-----END CROSSCERT-----",
-                 str_ip_enc_key, key_str,
-                 str_ip_enc_key_cert, b64_cert);
-    tor_free(key_str);
-    break;
+  /* Encode cross cert. */
+  if (base64_encode(b64_cert, sizeof(b64_cert),
+                    (const char *) ip->legacy.cert.encoded,
+                    ip->legacy.cert.len, BASE64_ENCODE_MULTILINE) < 0) {
+    log_warn(LD_REND, "Unable to encode legacy crosscert.");
+    goto done;
   }
-  case HS_DESC_KEY_TYPE_CURVE25519:
-  {
-    int signbit, ret;
-    char *encoded_cert, key_fp_b64[CURVE25519_BASE64_PADDED_LEN + 1];
-    ed25519_keypair_t curve_kp;
-
-    if (ed25519_keypair_from_curve25519_keypair(&curve_kp, &signbit,
-                                                &ip->enc_key.curve25519)) {
-      goto err;
-    }
-    tor_cert_t *cross_cert = tor_cert_create(&curve_kp,
-                                             CERT_TYPE_CROSS_HS_IP_KEYS,
-                                             sig_key, now,
-                                             HS_DESC_CERT_LIFETIME,
-                                             CERT_FLAG_INCLUDE_SIGNING_KEY);
-    memwipe(&curve_kp, 0, sizeof(curve_kp));
-    if (!cross_cert) {
-      goto err;
-    }
-    ret = tor_cert_encode_ed22519(cross_cert, &encoded_cert);
-    tor_cert_free(cross_cert);
-    if (ret) {
-      goto err;
-    }
-    if (curve25519_public_to_base64(key_fp_b64,
-                                    &ip->enc_key.curve25519.pubkey) < 0) {
-      tor_free(encoded_cert);
-      goto err;
-    }
-    tor_asprintf(&encoded,
-                 "%s ntor %s\n"
-                 "%s\n%s",
-                 str_ip_enc_key, key_fp_b64,
-                 str_ip_enc_key_cert, encoded_cert);
-    tor_free(encoded_cert);
-    break;
-  }
-  default:
-    tor_assert(0);
+  /* Convert the encryption key to PEM format NUL terminated. */
+  if (crypto_pk_write_public_key_to_string(ip->legacy.key, &key_str,
+                                           &key_str_len) < 0) {
+    log_warn(LD_REND, "Unable to encode legacy encryption key.");
+    goto done;
   }
+  tor_asprintf(&encoded,
+               "%s \n%s"  /* Newline is added by the call above. */
+               "%s\n"
+               "-----BEGIN CROSSCERT-----\n"
+               "%s"
+               "-----END CROSSCERT-----",
+               str_ip_legacy_key, key_str,
+               str_ip_legacy_key_cert, b64_cert);
+  tor_free(key_str);
 
- err:
+ done:
+  return encoded;
+}
+
+/* Encode an introduction point encryption key and certificate. Return a newly
+ * allocated string with it. On failure, return NULL. */
+static char *
+encode_enc_key(const hs_desc_intro_point_t *ip)
+{
+  char *encoded = NULL, *encoded_cert;
+  char key_b64[CURVE25519_BASE64_PADDED_LEN + 1];
+
+  tor_assert(ip);
+
+  /* Base64 encode the encryption key for the "enc-key" field. */
+  if (curve25519_public_to_base64(key_b64, &ip->enc_key) < 0) {
+    goto done;
+  }
+  if (tor_cert_encode_ed22519(ip->enc_key_cert, &encoded_cert) < 0) {
+    goto done;
+  }
+  tor_asprintf(&encoded,
+               "%s ntor %s\n"
+               "%s\n%s",
+               str_ip_enc_key, key_b64,
+               str_ip_enc_key_cert, encoded_cert);
+  tor_free(encoded_cert);
+
+ done:
   return encoded;
 }
 
@@ -535,7 +509,7 @@ encode_intro_point(const ed25519_public_key_t *sig_key,
 
   /* Encryption key encoding. */
   {
-    char *encoded_enc_key = encode_enc_key(sig_key, ip);
+    char *encoded_enc_key = encode_enc_key(ip);
     if (encoded_enc_key == NULL) {
       goto err;
     }
@@ -543,6 +517,18 @@ encode_intro_point(const ed25519_public_key_t *sig_key,
     tor_free(encoded_enc_key);
   }
 
+  /* Legacy key if any. */
+  if (ip->legacy.key != NULL) {
+    /* Strong requirement else the IP creation was badly done. */
+    tor_assert(ip->legacy.cert.encoded);
+    char *encoded_legacy_key = encode_legacy_key(ip);
+    if (encoded_legacy_key == NULL) {
+      goto err;
+    }
+    smartlist_add_asprintf(lines, "%s", encoded_legacy_key);
+    tor_free(encoded_legacy_key);
+  }
+
   /* Join them all in one blob of text. */
   encoded_ip = smartlist_join_strings(lines, "\n", 1, NULL);
 
@@ -1581,6 +1567,64 @@ desc_decrypt_all(const hs_descriptor_t *desc, char **decrypted_out)
   return decrypted_len;
 }
 
+/* Given the token tok for an intro point legacy key, the list of tokens, the
+ * introduction point ip being decoded and the descriptor desc from which it
+ * comes from, decode the legacy key and set the intro point object. Return 0
+ * on success else -1 on failure. */
+static int
+decode_intro_legacy_key(const directory_token_t *tok,
+                        smartlist_t *tokens,
+                        hs_desc_intro_point_t *ip,
+                        const hs_descriptor_t *desc)
+{
+  tor_assert(tok);
+  tor_assert(tokens);
+  tor_assert(ip);
+  tor_assert(desc);
+
+  if (!crypto_pk_public_exponent_ok(tok->key)) {
+    log_warn(LD_REND, "Introduction point legacy key is invalid");
+    goto err;
+  }
+  ip->legacy.key = crypto_pk_dup_key(tok->key);
+  /* Extract the legacy cross certification cert which MUST be present if we
+   * have a legacy key. */
+  tok = find_opt_by_keyword(tokens, R3_INTRO_LEGACY_KEY_CERT);
+  if (!tok) {
+    log_warn(LD_REND, "Introduction point legacy key cert is missing");
+    goto err;
+  }
+  tor_assert(tok->object_body);
+  if (strcmp(tok->object_type, "CROSSCERT")) {
+    /* Info level because this might be an unknown field that we should
+     * ignore. */
+    log_info(LD_REND, "Introduction point legacy encryption key "
+                      "cross-certification has an unknown format.");
+    goto err;
+  }
+  /* Keep a copy of the certificate. */
+  ip->legacy.cert.encoded = tor_memdup(tok->object_body, tok->object_size);
+  ip->legacy.cert.len = tok->object_size;
+  /* The check on the expiration date is for the entire lifetime of a
+   * certificate which is 24 hours. However, a descriptor has a maximum
+   * lifetime of 12 hours meaning we have a 12h difference between the two
+   * which ultimately accomodate the clock skewed client. */
+  if (rsa_ed25519_crosscert_check(ip->legacy.cert.encoded,
+                                  ip->legacy.cert.len, ip->legacy.key,
+                                  &desc->plaintext_data.signing_pubkey,
+                                  approx_time() - HS_DESC_CERT_LIFETIME)) {
+    log_warn(LD_REND, "Unable to check cross-certification on the "
+                      "introduction point legacy encryption key.");
+    ip->cross_certified = 0;
+    goto err;
+  }
+
+  /* Success. */
+  return 0;
+ err:
+  return -1;
+}
+
 /* Given the start of a section and the end of it, decode a single
  * introduction point from that section. Return a newly allocated introduction
  * point object containing the decoded data. Return NULL if the section can't
@@ -1591,7 +1635,6 @@ decode_introduction_point(const hs_descriptor_t *desc, const char *start)
   hs_desc_intro_point_t *ip = NULL;
   memarea_t *area = NULL;
   smartlist_t *tokens = NULL;
-  tor_cert_t *cross_cert = NULL;
   const directory_token_t *tok;
 
   tor_assert(desc);
@@ -1625,84 +1668,67 @@ decode_introduction_point(const hs_descriptor_t *desc, const char *start)
     log_warn(LD_REND, "Unexpected object type for introduction auth key");
     goto err;
   }
-
   /* Parse cert and do some validation. */
   if (cert_parse_and_validate(&ip->auth_key_cert, tok->object_body,
                               tok->object_size, CERT_TYPE_AUTH_HS_IP_KEY,
                               "introduction point auth-key") < 0) {
     goto err;
   }
+  /* Validate authentication certificate with descriptor signing key. */
+  if (tor_cert_checksig(ip->auth_key_cert,
+                        &desc->plaintext_data.signing_pubkey, 0) < 0) {
+    log_warn(LD_REND, "Invalid authentication key signature");
+    goto err;
+  }
 
-  /* Exactly one "enc-key" ... */
+  /* Exactly one "enc-key" SP "ntor" SP key NL */
   tok = find_by_keyword(tokens, R3_INTRO_ENC_KEY);
   if (!strcmp(tok->args[0], "ntor")) {
-    /* "enc-key" SP "ntor" SP key NL */
-    if (tok->n_args != 2 || tok->object_body) {
-      log_warn(LD_REND, "Introduction point ntor encryption key is invalid");
-      goto err;
-    }
+    /* This field is using GE(2) so for possible forward compatibility, we
+     * accept more fields but must be at least 2. */
+    tor_assert(tok->n_args >= 2);
 
-    if (curve25519_public_from_base64(&ip->enc_key.curve25519.pubkey,
-                                      tok->args[1]) < 0) {
-      log_warn(LD_REND, "Introduction point ntor encryption key is invalid");
+    if (curve25519_public_from_base64(&ip->enc_key, tok->args[1]) < 0) {
+      log_warn(LD_REND, "Introduction point ntor enc-key is invalid");
       goto err;
     }
-    ip->enc_key_type = HS_DESC_KEY_TYPE_CURVE25519;
-  } else if (!strcmp(tok->args[0], "legacy")) {
-    /* "enc-key" SP "legacy" NL key NL */
-    if (!tok->key) {
-      log_warn(LD_REND, "Introduction point legacy encryption key is "
-               "invalid");
-      goto err;
-    }
-    ip->enc_key.legacy = crypto_pk_dup_key(tok->key);
-    ip->enc_key_type = HS_DESC_KEY_TYPE_LEGACY;
   } else {
     /* Unknown key type so we can't use that introduction point. */
     log_warn(LD_REND, "Introduction point encryption key is unrecognized.");
     goto err;
   }
 
-  /* "enc-key-certification" NL certificate NL */
-  tok = find_by_keyword(tokens, R3_INTRO_ENC_KEY_CERTIFICATION);
+  /* Exactly once "enc-key-cert" NL certificate NL */
+  tok = find_by_keyword(tokens, R3_INTRO_ENC_KEY_CERT);
   tor_assert(tok->object_body);
   /* Do the cross certification. */
-  switch (ip->enc_key_type) {
-  case HS_DESC_KEY_TYPE_CURVE25519:
-  {
-    if (strcmp(tok->object_type, "ED25519 CERT")) {
+  if (strcmp(tok->object_type, "ED25519 CERT")) {
       log_warn(LD_REND, "Introduction point ntor encryption key "
                         "cross-certification has an unknown format.");
       goto err;
-    }
-    if (cert_parse_and_validate(&cross_cert, tok->object_body,
-                       tok->object_size, CERT_TYPE_CROSS_HS_IP_KEYS,
-                       "introduction point enc-key-certification") < 0) {
-      goto err;
-    }
-    break;
   }
-  case HS_DESC_KEY_TYPE_LEGACY:
-    if (strcmp(tok->object_type, "CROSSCERT")) {
-      log_warn(LD_REND, "Introduction point legacy encryption key "
-                        "cross-certification has an unknown format.");
-      goto err;
-    }
-    if (rsa_ed25519_crosscert_check((const uint8_t *) tok->object_body,
-          tok->object_size, ip->enc_key.legacy,
-          &desc->plaintext_data.signing_key_cert->signed_key,
-          approx_time()-86400)) {
-      log_warn(LD_REND, "Unable to check cross-certification on the "
-                        "introduction point legacy encryption key.");
-      goto err;
-    }
-    break;
-  default:
-    tor_assert(0);
-    break;
+  if (cert_parse_and_validate(&ip->enc_key_cert, tok->object_body,
+                              tok->object_size, CERT_TYPE_CROSS_HS_IP_KEYS,
+                              "introduction point enc-key-cert") < 0) {
+    goto err;
+  }
+  if (tor_cert_checksig(ip->enc_key_cert,
+                        &desc->plaintext_data.signing_pubkey, 0) < 0) {
+    log_warn(LD_REND, "Invalid encryption key signature");
+    goto err;
   }
   /* It is successfully cross certified. Flag the object. */
   ip->cross_certified = 1;
+
+  /* Do we have a "legacy-key" SP key NL ?*/
+  tok = find_opt_by_keyword(tokens, R3_INTRO_LEGACY_KEY);
+  if (tok) {
+    if (decode_intro_legacy_key(tok, tokens, ip, desc) < 0) {
+      goto err;
+    }
+  }
+
+  /* Introduction point has been parsed successfully. */
   goto done;
 
  err:
@@ -1710,7 +1736,6 @@ decode_introduction_point(const hs_descriptor_t *desc, const char *start)
   ip = NULL;
 
  done:
-  tor_cert_free(cross_cert);
   SMARTLIST_FOREACH(tokens, directory_token_t *, t, token_clear(t));
   smartlist_free(tokens);
   if (area) {
diff --git a/src/or/hs_descriptor.h b/src/or/hs_descriptor.h
index b7d512c06b..b8b94792de 100644
--- a/src/or/hs_descriptor.h
+++ b/src/or/hs_descriptor.h
@@ -58,12 +58,6 @@ typedef enum {
   HS_DESC_AUTH_ED25519 = 1
 } hs_desc_auth_type_t;
 
-/* Type of encryption key in the descriptor. */
-typedef enum {
-  HS_DESC_KEY_TYPE_LEGACY     = 1,
-  HS_DESC_KEY_TYPE_CURVE25519 = 2,
-} hs_desc_key_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 {
@@ -91,18 +85,29 @@ typedef struct hs_desc_intro_point_t {
    * the blinded key and in turn signs it. */
   tor_cert_t *auth_key_cert;
 
-  /* Encryption key type so we know which one to use in the union below. */
-  hs_desc_key_type_t enc_key_type;
+  /* Encryption key for the "ntor" type. */
+  curve25519_public_key_t enc_key;
 
-  /* Keys are mutually exclusive thus the union. */
-  union {
-    /* Encryption key used to encrypt request to hidden service. */
-    curve25519_keypair_t curve25519;
+  /* Certificate cross certifying the descriptor signing key by the encryption
+   * curve25519 key. This certificate contains the signing key and is of type
+   * CERT_TYPE_CROSS_HS_IP_KEYS [0B]. */
+  tor_cert_t *enc_key_cert;
 
-    /* Backward compat: RSA 1024 encryption key for legacy purposes.
-     * Mutually exclusive with enc_key. */
-    crypto_pk_t *legacy;
-  } enc_key;
+  /* (Optional): If this introduction point is a legacy one that is version <=
+   * 0.2.9.x (HSIntro=3), we use this extra key for the intro point to be able
+   * to relay the cells to the service correctly. */
+  struct {
+    /* RSA public key. */
+    crypto_pk_t *key;
+
+    /* Cross certified cert with the descriptor signing key (RSA->Ed). Because
+     * of the cross certification API, we need to keep the certificate binary
+     * blob and its length in order to properly encode it after. */
+    struct {
+      uint8_t *encoded;
+      size_t len;
+    } cert;
+  } legacy;
 
   /* True iff the introduction point has passed the cross certification. Upon
    * decoding an intro point, this must be true. */
diff --git a/src/or/parsecommon.h b/src/or/parsecommon.h
index f4974a9683..b9f1613457 100644
--- a/src/or/parsecommon.h
+++ b/src/or/parsecommon.h
@@ -162,7 +162,9 @@ typedef enum {
   R3_INTRODUCTION_POINT,
   R3_INTRO_AUTH_KEY,
   R3_INTRO_ENC_KEY,
-  R3_INTRO_ENC_KEY_CERTIFICATION,
+  R3_INTRO_ENC_KEY_CERT,
+  R3_INTRO_LEGACY_KEY,
+  R3_INTRO_LEGACY_KEY_CERT,
   R3_DESC_AUTH_TYPE,
   R3_DESC_AUTH_KEY,
   R3_DESC_AUTH_CLIENT,
diff --git a/src/test/hs_test_helpers.c b/src/test/hs_test_helpers.c
index a17bf0f0a2..3f0d6a9413 100644
--- a/src/test/hs_test_helpers.c
+++ b/src/test/hs_test_helpers.c
@@ -51,15 +51,36 @@ hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now,
   tt_assert(ip->auth_key_cert);
 
   if (legacy) {
-    ip->enc_key.legacy = crypto_pk_new();
-    ip->enc_key_type = HS_DESC_KEY_TYPE_LEGACY;
-    tt_assert(ip->enc_key.legacy);
-    ret = crypto_pk_generate_key(ip->enc_key.legacy);
+    ip->legacy.key = crypto_pk_new();
+    tt_assert(ip->legacy.key);
+    ret = crypto_pk_generate_key(ip->legacy.key);
     tt_int_op(ret, ==, 0);
-  } else {
-    ret = curve25519_keypair_generate(&ip->enc_key.curve25519, 0);
+    ssize_t cert_len = tor_make_rsa_ed25519_crosscert(
+                                    &signing_kp->pubkey, ip->legacy.key,
+                                    now + HS_DESC_CERT_LIFETIME,
+                                    &ip->legacy.cert.encoded);
+    tt_assert(ip->legacy.cert.encoded);
+    tt_u64_op(cert_len, OP_GT, 0);
+    ip->legacy.cert.len = cert_len;
+  }
+
+  /* Encryption key. */
+  {
+    int signbit;
+    curve25519_keypair_t curve25519_kp;
+    ed25519_keypair_t ed25519_kp;
+    tor_cert_t *cross_cert;
+
+    ret = curve25519_keypair_generate(&curve25519_kp, 0);
     tt_int_op(ret, ==, 0);
-    ip->enc_key_type = HS_DESC_KEY_TYPE_CURVE25519;
+    ed25519_keypair_from_curve25519_keypair(&ed25519_kp, &signbit,
+                                            &curve25519_kp);
+    cross_cert = tor_cert_create(signing_kp, CERT_TYPE_CROSS_HS_IP_KEYS,
+                                 &ed25519_kp.pubkey, time(NULL),
+                                 HS_DESC_CERT_LIFETIME,
+                                 CERT_FLAG_INCLUDE_SIGNING_KEY);
+    tt_assert(cross_cert);
+    ip->enc_key_cert = cross_cert;
   }
 
   intro_point = ip;
@@ -192,19 +213,11 @@ hs_helper_desc_equal(const hs_descriptor_t *desc1,
                             *ip2 = smartlist_get(desc2->encrypted_data
                                                  .intro_points, i);
       tt_assert(tor_cert_eq(ip1->auth_key_cert, ip2->auth_key_cert));
-      tt_int_op(ip1->enc_key_type, OP_EQ, ip2->enc_key_type);
-      tt_assert(ip1->enc_key_type == HS_DESC_KEY_TYPE_LEGACY ||
-                ip1->enc_key_type == HS_DESC_KEY_TYPE_CURVE25519);
-      switch (ip1->enc_key_type) {
-        case HS_DESC_KEY_TYPE_LEGACY:
-          tt_int_op(crypto_pk_cmp_keys(ip1->enc_key.legacy,
-                                       ip2->enc_key.legacy), OP_EQ, 0);
-          break;
-        case HS_DESC_KEY_TYPE_CURVE25519:
-          tt_mem_op(ip1->enc_key.curve25519.pubkey.public_key, OP_EQ,
-                    ip2->enc_key.curve25519.pubkey.public_key,
-                    CURVE25519_PUBKEY_LEN);
-          break;
+      if (ip1->legacy.key) {
+        tt_int_op(crypto_pk_cmp_keys(ip1->legacy.key, ip2->legacy.key),
+                  OP_EQ, 0);
+      } else {
+        tt_mem_op(&ip1->enc_key, OP_EQ, &ip2->enc_key, CURVE25519_PUBKEY_LEN);
       }
 
       tt_int_op(smartlist_len(ip1->link_specifiers), ==,
diff --git a/src/test/test_hs_descriptor.c b/src/test/test_hs_descriptor.c
index b4c58937c4..b1abe381d4 100644
--- a/src/test/test_hs_descriptor.c
+++ b/src/test/test_hs_descriptor.c
@@ -410,7 +410,7 @@ test_decode_invalid_intro_point(void *arg)
   const char *enc_key =
     "enc-key ntor bpZKLsuhxP6woDQ3yVyjm5gUKSk7RjfAijT2qrzbQk0=";
   const char *enc_key_cert =
-    "enc-key-certification\n"
+    "enc-key-cert\n"
     "-----BEGIN ED25519 CERT-----\n"
     "AQsACOhZAUpNvCZ1aJaaR49lS6MCdsVkhVGVrRqoj0Y2T4SzroAtAQAgBABFOcGg\n"
     "lbTt1DF5nKTE/gU3Fr8ZtlCIOhu1A+F5LM7fqCUupfesg0KTHwyIZOYQbJuM5/he\n"