test_hs_descriptor: Add a test case that fails without the fix for 40793

This adds a bit more to hs_descriptor/test_decode_descriptor, mostly
testing pow-params and triggering the tor_assert() in issue #40793.

There was no mechanism for adding arbitrary test strings to the
encrypted portion of the desc without duplicating encode logic. One
option might be to publicize get_inner_encrypted_layer_plaintext enough
to add a mock implementation. In this patch I opt for what seems like
the simplest solution, at the cost of a small amount of #ifdef noise.
The unpacked descriptor grows a new test-only member that's used for
dropping arbitrary data in at encode time.

Signed-off-by: Micah Elizabeth Scott <beth@torproject.org>

4 files changed, 104 insertions, 3 deletions

diff --git a/src/feature/hs/hs_descriptor.c b/src/feature/hs/hs_descriptor.c
index 7b519e4c78..93fc1cf674 100644
--- a/src/feature/hs/hs_descriptor.c
+++ b/src/feature/hs/hs_descriptor.c
@@ -771,6 +771,13 @@ get_inner_encrypted_layer_plaintext(const hs_descriptor_t *desc)
     smartlist_add_asprintf(lines, "%s %d\n", str_create2_formats,
                            ONION_HANDSHAKE_TYPE_NTOR);
 
+#ifdef TOR_UNIT_TESTS
+    if (desc->encrypted_data.test_extra_plaintext) {
+      smartlist_add(lines,
+                    tor_strdup(desc->encrypted_data.test_extra_plaintext));
+    }
+#endif
+
     if (desc->encrypted_data.intro_auth_types &&
         smartlist_len(desc->encrypted_data.intro_auth_types)) {
       /* Put the authentication-required line. */
@@ -2817,9 +2824,15 @@ hs_desc_encode_descriptor,(const hs_descriptor_t *desc,
   }
 
   /* Try to decode what we just encoded. Symmetry is nice!, but it is
-   * symmetric only if the client auth is disabled. That is, the descriptor
-   * cookie will be NULL. */
-  if (!descriptor_cookie) {
+   * symmetric only if the client auth is disabled (That is, the descriptor
+   * cookie will be NULL) and the test-only mock plaintext isn't in use. */
+  bool do_round_trip_test = !descriptor_cookie;
+#ifdef TOR_UNIT_TESTS
+  if (desc->encrypted_data.test_extra_plaintext) {
+    do_round_trip_test = false;
+  }
+#endif
+  if (do_round_trip_test) {
     ret = hs_desc_decode_descriptor(*encoded_out, &desc->subcredential,
                                     NULL, NULL);
     if (BUG(ret != HS_DESC_DECODE_OK)) {
diff --git a/src/feature/hs/hs_descriptor.h b/src/feature/hs/hs_descriptor.h
index c89dc0b580..ca87972de1 100644
--- a/src/feature/hs/hs_descriptor.h
+++ b/src/feature/hs/hs_descriptor.h
@@ -177,6 +177,13 @@ typedef struct hs_desc_encrypted_data_t {
 
   /** A list of intro points. Contains hs_desc_intro_point_t objects. */
   smartlist_t *intro_points;
+
+#ifdef TOR_UNIT_TESTS
+  /** In unit tests only, we can include additional arbitrary plaintext.
+   * This is used to test parser validation by adding invalid inner data to
+   * descriptors that are otherwise correct and correctly encrypted. */
+  const char *test_extra_plaintext;
+#endif
 } hs_desc_encrypted_data_t;
 
 /** The superencrypted data section of a descriptor. Obviously the data in
diff --git a/src/test/hs_test_helpers.c b/src/test/hs_test_helpers.c
index 20b225ba4a..4ef2398095 100644
--- a/src/test/hs_test_helpers.c
+++ b/src/test/hs_test_helpers.c
@@ -354,6 +354,18 @@ hs_helper_desc_equal(const hs_descriptor_t *desc1,
     }
   }
 
+  /* Proof of Work DoS mitigation options */
+  tt_int_op(!!desc1->encrypted_data.pow_params, OP_EQ,
+            !!desc2->encrypted_data.pow_params);
+  if (desc1->encrypted_data.pow_params && desc2->encrypted_data.pow_params) {
+    hs_pow_desc_params_t *params1 = desc1->encrypted_data.pow_params;
+    hs_pow_desc_params_t *params2 = desc2->encrypted_data.pow_params;
+    tt_int_op(params1->type, OP_EQ, params2->type);
+    tt_mem_op(params1->seed, OP_EQ, params2->seed, HS_POW_SEED_LEN);
+    tt_int_op(params1->suggested_effort, OP_EQ, params2->suggested_effort);
+    tt_int_op(params1->expiration_time, OP_EQ, params2->expiration_time);
+  }
+
   /* Introduction points. */
   {
     tt_assert(desc1->encrypted_data.intro_points);
diff --git a/src/test/test_hs_descriptor.c b/src/test/test_hs_descriptor.c
index 469e3c39f9..d96048a0f6 100644
--- a/src/test/test_hs_descriptor.c
+++ b/src/test/test_hs_descriptor.c
@@ -364,6 +364,75 @@ test_decode_descriptor(void *arg)
     hs_helper_desc_equal(desc, decoded);
   }
 
+  /* Decode a descriptor without auth clients, and with PoW data added via
+   * test_extra_plaintext to test both the normal case of PoW decoding and the
+   * extra plaintext mechanism itself. */
+  {
+    tor_assert(!desc->encrypted_data.pow_params);
+
+    char pow_seed_base64[HS_POW_SEED_LEN*2];
+    uint8_t pow_seed[HS_POW_SEED_LEN];
+    crypto_strongest_rand(pow_seed, sizeof pow_seed);
+    tt_int_op(base64_encode_nopad(pow_seed_base64, sizeof pow_seed_base64,
+                                  pow_seed, sizeof pow_seed), OP_GT, 0);
+
+    time_t expiration_time = time(NULL);
+    char time_buf[ISO_TIME_LEN + 1];
+    format_iso_time_nospace(time_buf, expiration_time);
+
+    const unsigned suggested_effort = 123456;
+    char *extra_plaintext = NULL;
+    tor_asprintf(&extra_plaintext,
+                "pow-params v1 %s %u %s\n",
+                pow_seed_base64, suggested_effort, time_buf);
+
+    tor_free(encoded);
+    desc->encrypted_data.test_extra_plaintext = extra_plaintext;
+    ret = hs_desc_encode_descriptor(desc, &signing_kp, NULL, &encoded);
+    tor_free(extra_plaintext);
+    desc->encrypted_data.test_extra_plaintext = extra_plaintext;
+
+    tt_int_op(ret, OP_EQ, 0);
+    tt_assert(encoded);
+
+    desc->encrypted_data.pow_params =
+      tor_malloc_zero(sizeof(hs_pow_desc_params_t));
+    desc->encrypted_data.pow_params->type = HS_POW_DESC_V1;
+    memcpy(desc->encrypted_data.pow_params->seed, pow_seed, HS_POW_SEED_LEN);
+    desc->encrypted_data.pow_params->suggested_effort = suggested_effort;
+    desc->encrypted_data.pow_params->expiration_time = expiration_time;
+
+    hs_descriptor_free(decoded);
+    ret = hs_desc_decode_descriptor(encoded, &subcredential, NULL, &decoded);
+    tt_int_op(ret, OP_EQ, HS_DESC_DECODE_OK);
+    tt_assert(decoded);
+
+    hs_helper_desc_equal(desc, decoded);
+
+    tor_free(desc->encrypted_data.pow_params);
+  }
+
+  /* Now a version of the above that's expected to fail. This reproduces the
+   * issue from ticket tor#40793, in which pow_params gets too few parameters
+   * but this would cause an assert instead of an early validation fail.
+   * Make sure it fails to parse. Prior to the fix for #40793 this fails
+   * an assertion instead. */
+  {
+    tor_free(encoded);
+    tor_assert(!desc->encrypted_data.pow_params);
+    desc->encrypted_data.test_extra_plaintext = "pow-params v1 a a\n";
+    ret = hs_desc_encode_descriptor(desc, &signing_kp, NULL, &encoded);
+    desc->encrypted_data.test_extra_plaintext = NULL;
+
+    tt_int_op(ret, OP_EQ, 0);
+    tt_assert(encoded);
+
+    hs_descriptor_free(decoded);
+    ret = hs_desc_decode_descriptor(encoded, &subcredential, NULL, &decoded);
+    tt_int_op(ret, OP_EQ, HS_DESC_DECODE_ENCRYPTED_ERROR);
+    tt_assert(!decoded);
+  }
+
  done:
   hs_descriptor_free(desc);
   hs_descriptor_free(desc_no_ip);