From 00d9e0d252687110189ea5a1ed0dce99a7984681 Mon Sep 17 00:00:00 2001 From: Micah Elizabeth Scott Date: Tue, 14 Mar 2023 20:45:36 -0700 Subject: [PATCH] hs_pow: Define seed_head as uint8_t[4] instead of uint32_t This is more consistent with the specification, and it's much less confusing with endianness. This resolves the underlying cause of the earlier byte-swap. This patch itself does not change the wire protocol at all, it's just tidying up the types we use at the trunnel layer. Signed-off-by: Micah Elizabeth Scott --- src/feature/hs/hs_cell.c | 6 ++- src/feature/hs/hs_pow.c | 38 +++++++++-------- src/feature/hs/hs_pow.h | 18 ++++---- src/feature/hs/hs_service.c | 6 +-- src/test/test_hs_pow.c | 2 +- src/test/test_hs_pow_slow.c | 11 +++-- src/trunnel/hs/cell_introduce1.c | 57 ++++++++++++++++++-------- src/trunnel/hs/cell_introduce1.h | 33 +++++++++++---- src/trunnel/hs/cell_introduce1.trunnel | 6 ++- 9 files changed, 114 insertions(+), 63 deletions(-) diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index 190f0f3b5a..dce360fd91 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -405,8 +405,9 @@ build_introduce_pow_extension(const hs_pow_solution_t *pow_solution, &pow_solution->nonce, TRUNNEL_POW_NONCE_LEN); trn_cell_extension_pow_set_pow_effort(pow_ext, pow_solution->effort); - trn_cell_extension_pow_set_pow_seed(pow_ext, pow_solution->seed_head); + memcpy(trn_cell_extension_pow_getarray_pow_seed(pow_ext), + pow_solution->seed_head, TRUNNEL_POW_SEED_HEAD_LEN); memcpy(trn_cell_extension_pow_getarray_pow_solution(pow_ext), &pow_solution->equix_solution, TRUNNEL_POW_SOLUTION_LEN); @@ -829,7 +830,8 @@ handle_introduce2_encrypted_cell_pow_extension(const hs_service_t *service, /* Effort E */ sol.effort = trn_cell_extension_pow_get_pow_effort(pow); /* Seed C */ - sol.seed_head = trn_cell_extension_pow_get_pow_seed(pow); + memcpy(&sol.seed_head, trn_cell_extension_pow_getconstarray_pow_seed(pow), + HS_POW_SEED_HEAD_LEN); /* Nonce N */ memcpy(&sol.nonce, trn_cell_extension_pow_getconstarray_pow_nonce(pow), HS_POW_NONCE_LEN); diff --git a/src/feature/hs/hs_pow.c b/src/feature/hs/hs_pow.c index e41f271fec..77fec689e4 100644 --- a/src/feature/hs/hs_pow.c +++ b/src/feature/hs/hs_pow.c @@ -26,8 +26,10 @@ /** Cache entry for (nonce, seed) replay protection. */ typedef struct nonce_cache_entry_t { HT_ENTRY(nonce_cache_entry_t) node; - uint8_t nonce[HS_POW_NONCE_LEN]; - uint32_t seed_head; + struct { + uint8_t nonce[HS_POW_NONCE_LEN]; + uint8_t seed_head[HS_POW_SEED_HEAD_LEN]; + } bytes; } nonce_cache_entry_t; /** Return true if the two (nonce, seed) replay cache entries are the same */ @@ -35,15 +37,14 @@ static inline int nonce_cache_entries_eq_(const struct nonce_cache_entry_t *entry1, const struct nonce_cache_entry_t *entry2) { - return fast_memeq(entry1->nonce, entry2->nonce, HS_POW_NONCE_LEN) && - entry1->seed_head == entry2->seed_head; + return fast_memeq(&entry1->bytes, &entry2->bytes, sizeof entry1->bytes); } /** Hash function to hash the (nonce, seed) tuple entry. */ static inline unsigned nonce_cache_entry_hash_(const struct nonce_cache_entry_t *ent) { - return (unsigned)siphash24g(ent->nonce, HS_POW_NONCE_LEN) + ent->seed_head; + return (unsigned)siphash24g(&ent->bytes, sizeof ent->bytes); } static HT_HEAD(nonce_cache_table_ht, nonce_cache_entry_t) @@ -62,7 +63,7 @@ static int nonce_cache_entry_has_seed(nonce_cache_entry_t *ent, void *data) { /* Returning nonzero makes HT_FOREACH_FN remove the element from the HT */ - return ent->seed_head == *(uint32_t *)data; + return fast_memeq(ent->bytes.seed_head, data, HS_POW_SEED_HEAD_LEN); } /** Helper: Increment a given nonce and set it in the challenge at the right @@ -186,7 +187,8 @@ hs_pow_solve(const hs_pow_desc_params_t *pow_params, /* Store the effort E. */ pow_solution_out->effort = effort; /* We only store the first 4 bytes of the seed C. */ - pow_solution_out->seed_head = tor_ntohl(get_uint32(pow_params->seed)); + memcpy(pow_solution_out->seed_head, pow_params->seed, + sizeof(pow_solution_out->seed_head)); /* Store the solution S */ memcpy(&pow_solution_out->equix_solution, sol, sizeof(pow_solution_out->equix_solution)); @@ -235,11 +237,11 @@ hs_pow_verify(const hs_pow_service_state_t *pow_state, /* Find a valid seed C that starts with the seed head. Fail if no such seed * exists. */ - if (tor_ntohl(get_uint32(pow_state->seed_current)) - == pow_solution->seed_head) { + if (fast_memeq(pow_state->seed_current, pow_solution->seed_head, + HS_POW_SEED_HEAD_LEN)) { seed = pow_state->seed_current; - } else if (tor_ntohl(get_uint32(pow_state->seed_previous)) - == pow_solution->seed_head) { + } else if (fast_memeq(pow_state->seed_previous, pow_solution->seed_head, + HS_POW_SEED_HEAD_LEN)) { seed = pow_state->seed_previous; } else { log_warn(LD_REND, "Seed head didn't match either seed."); @@ -247,8 +249,9 @@ hs_pow_verify(const hs_pow_service_state_t *pow_state, } /* Fail if N = POW_NONCE is present in the replay cache. */ - memcpy(search.nonce, pow_solution->nonce, HS_POW_NONCE_LEN); - search.seed_head = pow_solution->seed_head; + memcpy(search.bytes.nonce, pow_solution->nonce, HS_POW_NONCE_LEN); + memcpy(search.bytes.seed_head, pow_solution->seed_head, + HS_POW_SEED_HEAD_LEN); entry = HT_FIND(nonce_cache_table_ht, &nonce_cache_table, &search); if (entry) { log_warn(LD_REND, "Found (nonce, seed) tuple in the replay cache."); @@ -283,8 +286,9 @@ hs_pow_verify(const hs_pow_service_state_t *pow_state, /* Add the (nonce, seed) tuple to the replay cache. */ entry = tor_malloc_zero(sizeof(nonce_cache_entry_t)); - memcpy(entry->nonce, pow_solution->nonce, HS_POW_NONCE_LEN); - entry->seed_head = pow_solution->seed_head; + memcpy(entry->bytes.nonce, pow_solution->nonce, HS_POW_NONCE_LEN); + memcpy(entry->bytes.seed_head, pow_solution->seed_head, + HS_POW_SEED_HEAD_LEN); HT_INSERT(nonce_cache_table_ht, &nonce_cache_table, entry); done: @@ -296,11 +300,11 @@ hs_pow_verify(const hs_pow_service_state_t *pow_state, /** Remove entries from the (nonce, seed) replay cache which are for the seed * beginning with seed_head. */ void -hs_pow_remove_seed_from_cache(uint32_t seed) +hs_pow_remove_seed_from_cache(const uint8_t *seed_head) { /* If nonce_cache_entry_has_seed returns 1, the entry is removed. */ HT_FOREACH_FN(nonce_cache_table_ht, &nonce_cache_table, - nonce_cache_entry_has_seed, &seed); + nonce_cache_entry_has_seed, (void*)seed_head); } /** Free a given PoW service state. */ diff --git a/src/feature/hs/hs_pow.h b/src/feature/hs/hs_pow.h index 6eb03ef64e..3d7018ab30 100644 --- a/src/feature/hs/hs_pow.h +++ b/src/feature/hs/hs_pow.h @@ -27,6 +27,8 @@ #define HS_POW_HASH_LEN 4 /** Length of random seed used in the PoW scheme. */ #define HS_POW_SEED_LEN 32 +/** Length of seed identification heading in the PoW scheme. */ +#define HS_POW_SEED_HEAD_LEN 4 /** Length of an effort value */ #define HS_POW_EFFORT_LEN sizeof(uint32_t) /** Length of a PoW challenge. Construction as per prop327 is: @@ -45,7 +47,7 @@ typedef struct hs_pow_desc_params_t { /** Type of PoW system being used. */ hs_pow_desc_type_t type; - /** Random 32-byte seed used as input the the PoW hash function. Decoded? */ + /** Random 32-byte seed used as input the the PoW hash function */ uint8_t seed[HS_POW_SEED_LEN]; /** Specifies effort value that clients should aim for when contacting the @@ -110,14 +112,14 @@ typedef struct hs_pow_service_state_t { /* Struct to store a solution to the PoW challenge. */ typedef struct hs_pow_solution_t { - /* The 16 byte nonce used in the solution. */ + /* The nonce chosen to satisfy the PoW challenge's conditions. */ uint8_t nonce[HS_POW_NONCE_LEN]; - /* The effort used in the solution. */ + /* The effort used in this solution. */ uint32_t effort; - /* The first four bytes of the seed used in the solution. */ - uint32_t seed_head; + /* A prefix of the seed used in this solution, so it can be identified. */ + uint8_t seed_head[HS_POW_SEED_HEAD_LEN]; /* The Equi-X solution used in the solution. */ equix_solution equix_solution; @@ -133,7 +135,7 @@ int hs_pow_solve(const hs_pow_desc_params_t *pow_params, int hs_pow_verify(const hs_pow_service_state_t *pow_state, const hs_pow_solution_t *pow_solution); -void hs_pow_remove_seed_from_cache(uint32_t seed); +void hs_pow_remove_seed_from_cache(const uint8_t *seed_head); void hs_pow_free_service_state(hs_pow_service_state_t *state); int hs_pow_queue_work(uint32_t intro_circ_identifier, @@ -162,9 +164,9 @@ hs_pow_verify(const hs_pow_service_state_t *pow_state, } static inline void -hs_pow_remove_seed_from_cache(uint32_t seed) +hs_pow_remove_seed_from_cache(const uint8_t *seed_head) { - (void)seed; + (void)seed_head; } static inline void diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index a9070024cb..44d39e6525 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -2650,7 +2650,7 @@ rotate_pow_seeds(hs_service_t *service, time_t now) /* Before we overwrite the previous seed lets scrub entries corresponding * to it in the nonce replay cache. */ - hs_pow_remove_seed_from_cache(get_uint32(pow_state->seed_previous)); + hs_pow_remove_seed_from_cache(pow_state->seed_previous); /* Keep track of the current seed that we are now rotating. */ memcpy(pow_state->seed_previous, pow_state->seed_current, HS_POW_SEED_LEN); @@ -2658,8 +2658,8 @@ rotate_pow_seeds(hs_service_t *service, time_t now) /* Generate a new random seed to use from now on. Make sure the seed head * is different to that of the previous seed. The following while loop * will run at least once as the seeds will initially be equal. */ - while (get_uint32(pow_state->seed_previous) == - get_uint32(pow_state->seed_current)) { + while (fast_memeq(pow_state->seed_previous, pow_state->seed_current, + HS_POW_SEED_HEAD_LEN)) { crypto_rand((char *)pow_state->seed_current, HS_POW_SEED_LEN); } diff --git a/src/test/test_hs_pow.c b/src/test/test_hs_pow.c index e2111478fc..877f022a79 100644 --- a/src/test/test_hs_pow.c +++ b/src/test/test_hs_pow.c @@ -382,7 +382,7 @@ test_hs_pow_vectors(void *arg) sol_hex, 2 * sizeof solution.equix_solution), OP_EQ, HS_POW_EQX_SOL_LEN); - solution.seed_head = tor_ntohl(get_uint32(pow_state->seed_previous)); + memcpy(solution.seed_head, pow_state->seed_previous, HS_POW_SEED_HEAD_LEN); /* Try to encode 'solution' into a relay cell */ diff --git a/src/test/test_hs_pow_slow.c b/src/test/test_hs_pow_slow.c index 716501dffd..0c83922646 100644 --- a/src/test/test_hs_pow_slow.c +++ b/src/test/test_hs_pow_slow.c @@ -26,16 +26,14 @@ testing_one_hs_pow_solution(const hs_pow_solution_t *ref_solution, int retval = -1; hs_pow_solution_t sol_buffer; hs_pow_service_state_t *s = tor_malloc_zero(sizeof(hs_pow_service_state_t)); - uint32_t seed_head; memcpy(s->seed_previous, seed, HS_POW_SEED_LEN); - memcpy(&seed_head, seed, sizeof seed_head); const unsigned num_variants = 10; const unsigned num_attempts = 3; for (unsigned variant = 0; variant < num_variants; variant++) { - hs_pow_remove_seed_from_cache(seed_head); + hs_pow_remove_seed_from_cache(seed); for (unsigned attempt = 0; attempt < num_attempts; attempt++) { int expected = -1; @@ -211,15 +209,16 @@ test_hs_pow_vectors(void *arg) sizeof solution.equix_solution, sol_hex, 2 * sizeof solution.equix_solution), OP_EQ, HS_POW_EQX_SOL_LEN); - solution.seed_head = tor_ntohl(get_uint32(params.seed)); + memcpy(solution.seed_head, params.seed, HS_POW_SEED_HEAD_LEN); memset(&output, 0xaa, sizeof output); testing_enable_prefilled_rng(rng_bytes, HS_POW_NONCE_LEN); tt_int_op(0, OP_EQ, hs_pow_solve(¶ms, &output)); testing_disable_prefilled_rng(); - tt_int_op(solution.seed_head, OP_EQ, output.seed_head); - tt_mem_op(&solution.nonce, OP_EQ, &output.nonce, + tt_mem_op(solution.seed_head, OP_EQ, output.seed_head, + sizeof output.seed_head); + tt_mem_op(solution.nonce, OP_EQ, output.nonce, sizeof output.nonce); tt_mem_op(&solution.equix_solution, OP_EQ, &output.equix_solution, sizeof output.equix_solution); diff --git a/src/trunnel/hs/cell_introduce1.c b/src/trunnel/hs/cell_introduce1.c index 03943568e7..27a62f83de 100644 --- a/src/trunnel/hs/cell_introduce1.c +++ b/src/trunnel/hs/cell_introduce1.c @@ -134,16 +134,41 @@ trn_cell_extension_pow_set_pow_effort(trn_cell_extension_pow_t *inp, uint32_t va inp->pow_effort = val; return 0; } -uint32_t -trn_cell_extension_pow_get_pow_seed(const trn_cell_extension_pow_t *inp) +size_t +trn_cell_extension_pow_getlen_pow_seed(const trn_cell_extension_pow_t *inp) +{ + (void)inp; return TRUNNEL_POW_SEED_HEAD_LEN; +} + +uint8_t +trn_cell_extension_pow_get_pow_seed(trn_cell_extension_pow_t *inp, size_t idx) +{ + trunnel_assert(idx < TRUNNEL_POW_SEED_HEAD_LEN); + return inp->pow_seed[idx]; +} + +uint8_t +trn_cell_extension_pow_getconst_pow_seed(const trn_cell_extension_pow_t *inp, size_t idx) +{ + return trn_cell_extension_pow_get_pow_seed((trn_cell_extension_pow_t*)inp, idx); +} +int +trn_cell_extension_pow_set_pow_seed(trn_cell_extension_pow_t *inp, size_t idx, uint8_t elt) +{ + trunnel_assert(idx < TRUNNEL_POW_SEED_HEAD_LEN); + inp->pow_seed[idx] = elt; + return 0; +} + +uint8_t * +trn_cell_extension_pow_getarray_pow_seed(trn_cell_extension_pow_t *inp) { return inp->pow_seed; } -int -trn_cell_extension_pow_set_pow_seed(trn_cell_extension_pow_t *inp, uint32_t val) +const uint8_t * +trn_cell_extension_pow_getconstarray_pow_seed(const trn_cell_extension_pow_t *inp) { - inp->pow_seed = val; - return 0; + return (const uint8_t *)trn_cell_extension_pow_getarray_pow_seed((trn_cell_extension_pow_t*)inp); } size_t trn_cell_extension_pow_getlen_pow_solution(const trn_cell_extension_pow_t *inp) @@ -211,8 +236,8 @@ trn_cell_extension_pow_encoded_len(const trn_cell_extension_pow_t *obj) /* Length of u32 pow_effort */ result += 4; - /* Length of u32 pow_seed */ - result += 4; + /* Length of u8 pow_seed[TRUNNEL_POW_SEED_HEAD_LEN] */ + result += TRUNNEL_POW_SEED_HEAD_LEN; /* Length of u8 pow_solution[TRUNNEL_POW_SOLUTION_LEN] */ result += TRUNNEL_POW_SOLUTION_LEN; @@ -264,12 +289,12 @@ trn_cell_extension_pow_encode(uint8_t *output, const size_t avail, const trn_cel trunnel_set_uint32(ptr, trunnel_htonl(obj->pow_effort)); written += 4; ptr += 4; - /* Encode u32 pow_seed */ + /* Encode u8 pow_seed[TRUNNEL_POW_SEED_HEAD_LEN] */ trunnel_assert(written <= avail); - if (avail - written < 4) + if (avail - written < TRUNNEL_POW_SEED_HEAD_LEN) goto truncated; - trunnel_set_uint32(ptr, trunnel_htonl(obj->pow_seed)); - written += 4; ptr += 4; + memcpy(ptr, obj->pow_seed, TRUNNEL_POW_SEED_HEAD_LEN); + written += TRUNNEL_POW_SEED_HEAD_LEN; ptr += TRUNNEL_POW_SEED_HEAD_LEN; /* Encode u8 pow_solution[TRUNNEL_POW_SOLUTION_LEN] */ trunnel_assert(written <= avail); @@ -330,10 +355,10 @@ trn_cell_extension_pow_parse_into(trn_cell_extension_pow_t *obj, const uint8_t * obj->pow_effort = trunnel_ntohl(trunnel_get_uint32(ptr)); remaining -= 4; ptr += 4; - /* Parse u32 pow_seed */ - CHECK_REMAINING(4, truncated); - obj->pow_seed = trunnel_ntohl(trunnel_get_uint32(ptr)); - remaining -= 4; ptr += 4; + /* Parse u8 pow_seed[TRUNNEL_POW_SEED_HEAD_LEN] */ + CHECK_REMAINING(TRUNNEL_POW_SEED_HEAD_LEN, truncated); + memcpy(obj->pow_seed, ptr, TRUNNEL_POW_SEED_HEAD_LEN); + remaining -= TRUNNEL_POW_SEED_HEAD_LEN; ptr += TRUNNEL_POW_SEED_HEAD_LEN; /* Parse u8 pow_solution[TRUNNEL_POW_SOLUTION_LEN] */ CHECK_REMAINING(TRUNNEL_POW_SOLUTION_LEN, truncated); diff --git a/src/trunnel/hs/cell_introduce1.h b/src/trunnel/hs/cell_introduce1.h index 827c107b6b..b81c562343 100644 --- a/src/trunnel/hs/cell_introduce1.h +++ b/src/trunnel/hs/cell_introduce1.h @@ -23,13 +23,14 @@ struct link_specifier_st; #define TRUNNEL_EXT_TYPE_POW 2 #define TRUNNEL_POW_NONCE_LEN 16 #define TRUNNEL_POW_SOLUTION_LEN 16 +#define TRUNNEL_POW_SEED_HEAD_LEN 4 #define TRUNNEL_POW_VERSION_EQUIX 1 #if !defined(TRUNNEL_OPAQUE) && !defined(TRUNNEL_OPAQUE_TRN_CELL_EXTENSION_POW) struct trn_cell_extension_pow_st { uint8_t pow_version; uint8_t pow_nonce[TRUNNEL_POW_NONCE_LEN]; uint32_t pow_effort; - uint32_t pow_seed; + uint8_t pow_seed[TRUNNEL_POW_SEED_HEAD_LEN]; uint8_t pow_solution[TRUNNEL_POW_SOLUTION_LEN]; uint8_t trunnel_error_code_; }; @@ -148,15 +149,31 @@ uint32_t trn_cell_extension_pow_get_pow_effort(const trn_cell_extension_pow_t *i * return -1 and set the error code on 'inp' on failure. */ int trn_cell_extension_pow_set_pow_effort(trn_cell_extension_pow_t *inp, uint32_t val); -/** Return the value of the pow_seed field of the - * trn_cell_extension_pow_t in 'inp' +/** Return the (constant) length of the array holding the pow_seed + * field of the trn_cell_extension_pow_t in 'inp'. */ -uint32_t trn_cell_extension_pow_get_pow_seed(const trn_cell_extension_pow_t *inp); -/** Set the value of the pow_seed field of the - * trn_cell_extension_pow_t in 'inp' to 'val'. Return 0 on success; - * return -1 and set the error code on 'inp' on failure. +size_t trn_cell_extension_pow_getlen_pow_seed(const trn_cell_extension_pow_t *inp); +/** Return the element at position 'idx' of the fixed array field + * pow_seed of the trn_cell_extension_pow_t in 'inp'. */ -int trn_cell_extension_pow_set_pow_seed(trn_cell_extension_pow_t *inp, uint32_t val); +uint8_t trn_cell_extension_pow_get_pow_seed(trn_cell_extension_pow_t *inp, size_t idx); +/** As trn_cell_extension_pow_get_pow_seed, but take and return a + * const pointer + */ +uint8_t trn_cell_extension_pow_getconst_pow_seed(const trn_cell_extension_pow_t *inp, size_t idx); +/** Change the element at position 'idx' of the fixed array field + * pow_seed of the trn_cell_extension_pow_t in 'inp', so that it will + * hold the value 'elt'. + */ +int trn_cell_extension_pow_set_pow_seed(trn_cell_extension_pow_t *inp, size_t idx, uint8_t elt); +/** Return a pointer to the TRUNNEL_POW_SEED_HEAD_LEN-element array + * field pow_seed of 'inp'. + */ +uint8_t * trn_cell_extension_pow_getarray_pow_seed(trn_cell_extension_pow_t *inp); +/** As trn_cell_extension_pow_get_pow_seed, but take and return a + * const pointer + */ +const uint8_t * trn_cell_extension_pow_getconstarray_pow_seed(const trn_cell_extension_pow_t *inp); /** Return the (constant) length of the array holding the pow_solution * field of the trn_cell_extension_pow_t in 'inp'. */ diff --git a/src/trunnel/hs/cell_introduce1.trunnel b/src/trunnel/hs/cell_introduce1.trunnel index a92fc76ab5..cf8a291c26 100644 --- a/src/trunnel/hs/cell_introduce1.trunnel +++ b/src/trunnel/hs/cell_introduce1.trunnel @@ -89,6 +89,8 @@ const TRUNNEL_EXT_TYPE_POW = 0x02; const TRUNNEL_POW_NONCE_LEN = 16; const TRUNNEL_POW_SOLUTION_LEN = 16; +const TRUNNEL_POW_SEED_HEAD_LEN = 4; + /* Version 1 is based on Equi-X scheme. */ const TRUNNEL_POW_VERSION_EQUIX = 0x01; @@ -102,8 +104,8 @@ struct trn_cell_extension_pow { /* Effort */ u32 pow_effort; - /* First 4 bytes of the seed. */ - u32 pow_seed; + /* Identifiable prefix from the seed. */ + u8 pow_seed[TRUNNEL_POW_SEED_HEAD_LEN]; /* Solution. */ u8 pow_solution[TRUNNEL_POW_SOLUTION_LEN];