[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [tor] 02/03: test_hs_descriptor: Add a test case that fails without the fix for 40793
This is an automated email from the git hooks/post-receive script.
dgoulet pushed a commit to branch main
in repository tor.
commit 71b2958a624259ac4a10a67b76a12b0064f17616
Author: Micah Elizabeth Scott <beth@xxxxxxxxxxxxxx>
AuthorDate: Wed May 17 18:33:19 2023 -0700
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@xxxxxxxxxxxxxx>
---
src/feature/hs/hs_descriptor.c | 19 ++++++++++--
src/feature/hs/hs_descriptor.h | 7 +++++
src/test/hs_test_helpers.c | 12 ++++++++
src/test/test_hs_descriptor.c | 69 ++++++++++++++++++++++++++++++++++++++++++
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);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits