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@torproject.org 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@torproject.org --- 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);