test: Fix prop224 HS descriptor to use subcredential

We used to use NULL subcredential which is a terrible terrible idea.  Refactor
HS unittests to use subcredentials.

Also add some non-fatal asserts to make sure that we always use subcredentials
when decoding/encoding descs.

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

5 files changed, 52 insertions, 12 deletions

diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c
index 5a230759a4..6f304d6d2d 100644
--- a/src/or/hs_descriptor.c
+++ b/src/or/hs_descriptor.c
@@ -1006,6 +1006,11 @@ desc_encode_v3(const hs_descriptor_t *desc,
   tor_assert(encoded_out);
   tor_assert(desc->plaintext_data.version == 3);
 
+  if (BUG(desc->subcredential == NULL)) {
+    log_warn(LD_GENERAL, "Asked to encode desc with no subcred. No!");
+    goto err;
+  }
+
   /* Build the non-encrypted values. */
   {
     char *encoded_cert;
@@ -2261,7 +2266,7 @@ hs_desc_decode_descriptor(const char *encoded,
                           const uint8_t *subcredential,
                           hs_descriptor_t **desc_out)
 {
-  int ret;
+  int ret = -1;
   hs_descriptor_t *desc;
 
   tor_assert(encoded);
@@ -2269,10 +2274,13 @@ hs_desc_decode_descriptor(const char *encoded,
   desc = tor_malloc_zero(sizeof(hs_descriptor_t));
 
   /* Subcredentials are optional. */
-  if (subcredential) {
-    memcpy(desc->subcredential, subcredential, sizeof(desc->subcredential));
+  if (BUG(!subcredential)) {
+    log_warn(LD_GENERAL, "Tried to decrypt without subcred. Impossible!");
+    goto err;
   }
 
+  memcpy(desc->subcredential, subcredential, sizeof(desc->subcredential));
+
   ret = hs_desc_decode_plaintext(encoded, &desc->plaintext_data);
   if (ret < 0) {
     goto err;
diff --git a/src/test/hs_test_helpers.c b/src/test/hs_test_helpers.c
index 24d4a7e91a..2753d29078 100644
--- a/src/test/hs_test_helpers.c
+++ b/src/test/hs_test_helpers.c
@@ -6,6 +6,7 @@
 #include "test.h"
 #include "torcert.h"
 
+#include "hs_common.h"
 #include "hs_test_helpers.h"
 
 hs_desc_intro_point_t *
@@ -93,8 +94,7 @@ 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);
+  time_t now = approx_time();
   ed25519_keypair_t blinded_kp;
   hs_descriptor_t *descp = NULL, *desc = tor_malloc_zero(sizeof(*desc));
 
@@ -104,8 +104,9 @@ hs_helper_build_hs_desc_impl(unsigned int no_ip,
   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);
+  uint64_t current_time_period = hs_get_time_period_num(approx_time());
+  hs_build_blinded_keypair(signing_kp, NULL, 0,
+                           current_time_period, &blinded_kp);
   /* Copy only the public key into the descriptor. */
   memcpy(&desc->plaintext_data.blinded_pubkey, &blinded_kp.pubkey,
          sizeof(ed25519_public_key_t));
@@ -118,6 +119,9 @@ hs_helper_build_hs_desc_impl(unsigned int no_ip,
   desc->plaintext_data.revision_counter = 42;
   desc->plaintext_data.lifetime_sec = 3 * 60 * 60;
 
+  hs_get_subcredential(&signing_kp->pubkey, &blinded_kp.pubkey,
+                    desc->subcredential);
+
   /* Setup encrypted data section. */
   desc->encrypted_data.create2_ntor = 1;
   desc->encrypted_data.intro_auth_types = smartlist_new();
@@ -141,6 +145,21 @@ hs_helper_build_hs_desc_impl(unsigned int no_ip,
   return descp;
 }
 
+/** Helper function to get the HS subcredential using the identity keypair of
+ *  an HS. Used to decrypt descriptors in unittests. */
+void
+hs_helper_get_subcred_from_identity_keypair(ed25519_keypair_t *signing_kp,
+                                            uint8_t *subcred_out)
+{
+  ed25519_keypair_t blinded_kp;
+  uint64_t current_time_period = hs_get_time_period_num(approx_time());
+  hs_build_blinded_keypair(signing_kp, NULL, 0,
+                           current_time_period, &blinded_kp);
+
+  hs_get_subcredential(&signing_kp->pubkey, &blinded_kp.pubkey,
+                       subcred_out);
+}
+
 /* Build a descriptor with introduction points. */
 hs_descriptor_t *
 hs_helper_build_hs_desc_with_ip(const ed25519_keypair_t *signing_kp)
diff --git a/src/test/hs_test_helpers.h b/src/test/hs_test_helpers.h
index a7fedab136..05f5aa7b64 100644
--- a/src/test/hs_test_helpers.h
+++ b/src/test/hs_test_helpers.h
@@ -17,6 +17,9 @@ 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);
+void
+hs_helper_get_subcred_from_identity_keypair(ed25519_keypair_t *signing_kp,
+                                            uint8_t *subcred_out);
 
 #endif /* TOR_HS_TEST_HELPERS_H */
 
diff --git a/src/test/test_hs_cache.c b/src/test/test_hs_cache.c
index 40f50b322a..6c2addef9a 100644
--- a/src/test/test_hs_cache.c
+++ b/src/test/test_hs_cache.c
@@ -342,6 +342,7 @@ test_hsdir_revision_counter_check(void *arg)
   hs_descriptor_t *published_desc = NULL;
   char *published_desc_str = NULL;
 
+  uint8_t subcredential[DIGEST256_LEN];
   char *received_desc_str = NULL;
   hs_descriptor_t *received_desc = NULL;
 
@@ -378,9 +379,11 @@ test_hsdir_revision_counter_check(void *arg)
     const ed25519_public_key_t *blinded_key;
 
     blinded_key = &published_desc->plaintext_data.blinded_pubkey;
+    hs_get_subcredential(&signing_kp.pubkey, blinded_key, subcredential);
     received_desc_str = helper_fetch_desc_from_hsdir(blinded_key);
 
-    retval = hs_desc_decode_descriptor(received_desc_str,NULL, &received_desc);
+    retval = hs_desc_decode_descriptor(received_desc_str,
+                                       subcredential, &received_desc);
     tt_int_op(retval, ==, 0);
     tt_assert(received_desc);
 
@@ -412,7 +415,8 @@ test_hsdir_revision_counter_check(void *arg)
     blinded_key = &published_desc->plaintext_data.blinded_pubkey;
     received_desc_str = helper_fetch_desc_from_hsdir(blinded_key);
 
-    retval = hs_desc_decode_descriptor(received_desc_str,NULL, &received_desc);
+    retval = hs_desc_decode_descriptor(received_desc_str,
+                                       subcredential, &received_desc);
     tt_int_op(retval, ==, 0);
     tt_assert(received_desc);
 
diff --git a/src/test/test_hs_descriptor.c b/src/test/test_hs_descriptor.c
index d83f5e4c61..77bdd4be5e 100644
--- a/src/test/test_hs_descriptor.c
+++ b/src/test/test_hs_descriptor.c
@@ -296,6 +296,7 @@ test_decode_descriptor(void *arg)
   hs_descriptor_t *desc = NULL;
   hs_descriptor_t *decoded = NULL;
   hs_descriptor_t *desc_no_ip = NULL;
+  uint8_t subcredential[DIGEST256_LEN];
 
   (void) arg;
 
@@ -303,15 +304,18 @@ test_decode_descriptor(void *arg)
   tt_int_op(ret, ==, 0);
   desc = hs_helper_build_hs_desc_with_ip(&signing_kp);
 
+  hs_helper_get_subcred_from_identity_keypair(&signing_kp,
+                                              subcredential);
+
   /* Give some bad stuff to the decoding function. */
-  ret = hs_desc_decode_descriptor("hladfjlkjadf", NULL, &decoded);
+  ret = hs_desc_decode_descriptor("hladfjlkjadf", subcredential, &decoded);
   tt_int_op(ret, OP_EQ, -1);
 
   ret = hs_desc_encode_descriptor(desc, &signing_kp, &encoded);
   tt_int_op(ret, ==, 0);
   tt_assert(encoded);
 
-  ret = hs_desc_decode_descriptor(encoded, NULL, &decoded);
+  ret = hs_desc_decode_descriptor(encoded, subcredential, &decoded);
   tt_int_op(ret, ==, 0);
   tt_assert(decoded);
 
@@ -322,6 +326,8 @@ test_decode_descriptor(void *arg)
     ed25519_keypair_t signing_kp_no_ip;
     ret = ed25519_keypair_generate(&signing_kp_no_ip, 0);
     tt_int_op(ret, ==, 0);
+    hs_helper_get_subcred_from_identity_keypair(&signing_kp_no_ip,
+                                                subcredential);
     desc_no_ip = hs_helper_build_hs_desc_no_ip(&signing_kp_no_ip);
     tt_assert(desc_no_ip);
     tor_free(encoded);
@@ -329,7 +335,7 @@ test_decode_descriptor(void *arg)
     tt_int_op(ret, ==, 0);
     tt_assert(encoded);
     hs_descriptor_free(decoded);
-    ret = hs_desc_decode_descriptor(encoded, NULL, &decoded);
+    ret = hs_desc_decode_descriptor(encoded, subcredential, &decoded);
     tt_int_op(ret, ==, 0);
     tt_assert(decoded);
   }