From 46e473f43ee6aa920a779d37f7d2a28da64df383 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Fri, 3 Feb 2023 02:11:10 +0000 Subject: [PATCH] Prop#329 Pool: Avoid sharing Guards and Middles between circuits. Conflux must not use the same Guard for each leg; nor the same middle for each leg. --- src/core/or/circuitbuild.c | 24 ++++--- src/core/or/circuitbuild.h | 3 +- src/core/or/conflux_pool.c | 123 ++++++++++++++++++++++++++++++++ src/core/or/conflux_pool.h | 5 ++ src/feature/client/entrynodes.c | 42 +++++++++-- src/feature/client/entrynodes.h | 14 +++- 6 files changed, 193 insertions(+), 18 deletions(-) diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index 743d67acde..d6e022e7fa 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -45,6 +45,7 @@ #include "core/or/command.h" #include "core/or/connection_edge.h" #include "core/or/connection_or.h" +#include "core/or/conflux_pool.h" #include "core/or/extendinfo.h" #include "core/or/onion.h" #include "core/or/ocirc_event.h" @@ -89,7 +90,8 @@ static int circuit_send_first_onion_skin(origin_circuit_t *circ); static int circuit_build_no_more_hops(origin_circuit_t *circ); static int circuit_send_intermediate_onion_skin(origin_circuit_t *circ, crypt_path_t *hop); -static const node_t *choose_good_middle_server(uint8_t purpose, +static const node_t *choose_good_middle_server(const origin_circuit_t *, + uint8_t purpose, cpath_build_state_t *state, crypt_path_t *head, int cur_len); @@ -2313,7 +2315,8 @@ build_vanguard_middle_exclude_list(uint8_t purpose, * hop, based on already chosen nodes. */ static smartlist_t * -build_middle_exclude_list(uint8_t purpose, +build_middle_exclude_list(const origin_circuit_t *circ, + uint8_t purpose, cpath_build_state_t *state, crypt_path_t *head, int cur_len) @@ -2330,6 +2333,9 @@ build_middle_exclude_list(uint8_t purpose, excluded = smartlist_new(); + // Exclude other middles on pending and built conflux circs + conflux_add_middles_to_exclude_list(circ, excluded); + /* For non-vanguard circuits, add the exit and its family to the exclude list * (note that the exit/last hop is always chosen first in * circuit_establish_circuit()). */ @@ -2423,7 +2429,8 @@ pick_vanguard_middle_node(const or_options_t *options, * family, and make sure we don't duplicate any previous nodes or their * families. */ static const node_t * -choose_good_middle_server(uint8_t purpose, +choose_good_middle_server(const origin_circuit_t * circ, + uint8_t purpose, cpath_build_state_t *state, crypt_path_t *head, int cur_len) @@ -2438,7 +2445,7 @@ choose_good_middle_server(uint8_t purpose, log_debug(LD_CIRC, "Contemplating intermediate hop #%d: random choice.", cur_len+1); - excluded = build_middle_exclude_list(purpose, state, head, cur_len); + excluded = build_middle_exclude_list(circ, purpose, state, head, cur_len); flags |= cpath_build_state_to_crn_flags(state); flags |= cpath_build_state_to_crn_ipv6_extend_flag(state, cur_len); @@ -2483,7 +2490,8 @@ choose_good_middle_server(uint8_t purpose, * guard worked or not. */ const node_t * -choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state, +choose_good_entry_server(const origin_circuit_t *circ, + uint8_t purpose, cpath_build_state_t *state, circuit_guard_state_t **guard_state_out) { const node_t *choice; @@ -2505,7 +2513,7 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state, /* This request is for an entry server to use for a regular circuit, * and we use entry guard nodes. Just return one of the guard nodes. */ tor_assert(guard_state_out); - return guards_choose_guard(state, purpose, guard_state_out); + return guards_choose_guard(circ, state, purpose, guard_state_out); } excluded = smartlist_new(); @@ -2551,7 +2559,7 @@ onion_extend_cpath(origin_circuit_t *circ) if (cur_len == state->desired_path_len - 1) { /* Picking last node */ info = extend_info_dup(state->chosen_exit); } else if (cur_len == 0) { /* picking first node */ - const node_t *r = choose_good_entry_server(purpose, state, + const node_t *r = choose_good_entry_server(circ, purpose, state, &circ->guard_state); if (r) { /* If we're a client, use the preferred address rather than the @@ -2564,7 +2572,7 @@ onion_extend_cpath(origin_circuit_t *circ) } } else { const node_t *r = - choose_good_middle_server(purpose, state, circ->cpath, cur_len); + choose_good_middle_server(circ, purpose, state, circ->cpath, cur_len); if (r) { info = extend_info_from_node(r, 0, false); } diff --git a/src/core/or/circuitbuild.h b/src/core/or/circuitbuild.h index 4235ee96b2..c76259fc29 100644 --- a/src/core/or/circuitbuild.h +++ b/src/core/or/circuitbuild.h @@ -57,7 +57,8 @@ const char *build_state_get_exit_nickname(cpath_build_state_t *state); struct circuit_guard_state_t; -const node_t *choose_good_entry_server(uint8_t purpose, +const node_t *choose_good_entry_server(const origin_circuit_t *circ, + uint8_t purpose, cpath_build_state_t *state, struct circuit_guard_state_t **guard_state_out); void circuit_upgrade_circuits_from_guard_wait(void); diff --git a/src/core/or/conflux_pool.c b/src/core/or/conflux_pool.c index 66a405d4c1..c1b522fa67 100644 --- a/src/core/or/conflux_pool.c +++ b/src/core/or/conflux_pool.c @@ -31,6 +31,8 @@ #include "core/or/crypt_path_st.h" #include "core/or/or_circuit_st.h" #include "core/or/origin_circuit_st.h" +#include "core/or/extend_info_st.h" +#include "core/or/conflux_st.h" #include "feature/nodelist/nodelist.h" @@ -1092,6 +1094,127 @@ conflux_launch_leg(const uint8_t *nonce) return false; } +/** + * Add the identity digest of the guard nodes of all legs of the conflux + * circuit. + * + * This function checks both pending and linked conflux circuits. + */ +void +conflux_add_guards_to_exclude_list(const origin_circuit_t *orig_circ, + smartlist_t *excluded) +{ + tor_assert(orig_circ); + tor_assert(excluded); + + /* Ease our lives. */ + const circuit_t *circ = TO_CIRCUIT(orig_circ); + + /* Ignore if this is not conflux related. */ + if (!CIRCUIT_IS_CONFLUX(circ)) { + return; + } + + /* When building a circuit, we should not have a conflux object + * ourselves (though one may exist elsewhere). */ + tor_assert(!circ->conflux); + + /* Getting here without a nonce is a code flow issue. */ + if (BUG(!circ->conflux_pending_nonce)) { + return; + } + + /* A linked set exists, use it. */ + const conflux_t *cfx = linked_pool_get(circ->conflux_pending_nonce, true); + if (cfx) { + CONFLUX_FOR_EACH_LEG_BEGIN(cfx, leg) { + const origin_circuit_t *ocirc = CONST_TO_ORIGIN_CIRCUIT(leg->circ); + smartlist_add(excluded, + tor_memdup(ocirc->cpath->extend_info->identity_digest, + DIGEST_LEN)); + } CONFLUX_FOR_EACH_LEG_END(leg); + } + + /* An unlinked set might exist for this nonce, if so, add the second hop of + * the existing legs to the exclusion list. */ + unlinked_circuits_t *unlinked = + unlinked_pool_get(circ->conflux_pending_nonce, true); + if (unlinked) { + tor_assert(unlinked->is_client); + SMARTLIST_FOREACH_BEGIN(unlinked->legs, leg_t *, leg) { + /* Convert to origin circ and get cpath */ + const origin_circuit_t *ocirc = CONST_TO_ORIGIN_CIRCUIT(leg->circ); + smartlist_add(excluded, + tor_memdup(ocirc->cpath->extend_info->identity_digest, + DIGEST_LEN)); + } SMARTLIST_FOREACH_END(leg); + } +} + +/** + * Add the identity digest of the middle nodes of all legs of the conflux + * circuit. + * + * This function checks both pending and linked conflux circuits. + * + * XXX: The add guard and middle could be merged since it is the exact same + * code except for the cpath position and the identity digest vs node_t in + * the list. We could use an extra param indicating guard or middle. */ +void +conflux_add_middles_to_exclude_list(const origin_circuit_t *orig_circ, + smartlist_t *excluded) +{ + tor_assert(orig_circ); + tor_assert(excluded); + + /* Ease our lives. */ + const circuit_t *circ = TO_CIRCUIT(orig_circ); + + /* Ignore if this is not conflux related. */ + if (!CIRCUIT_IS_CONFLUX(circ)) { + return; + } + + /* When building a circuit, we should not have a conflux object + * ourselves (though one may exist elsewhere). */ + tor_assert(!circ->conflux); + + /* Getting here without a nonce is a code flow issue. */ + if (BUG(!circ->conflux_pending_nonce)) { + return; + } + + /* A linked set exists, use it. */ + const conflux_t *cfx = linked_pool_get(circ->conflux_pending_nonce, true); + if (cfx) { + CONFLUX_FOR_EACH_LEG_BEGIN(cfx, leg) { + const origin_circuit_t *ocirc = CONST_TO_ORIGIN_CIRCUIT(leg->circ); + node_t *node = node_get_mutable_by_id( + ocirc->cpath->next->extend_info->identity_digest); + if (node) { + smartlist_add(excluded, node); + } + } CONFLUX_FOR_EACH_LEG_END(leg); + } + + /* An unlinked set might exist for this nonce, if so, add the second hop of + * the existing legs to the exclusion list. */ + unlinked_circuits_t *unlinked = + unlinked_pool_get(circ->conflux_pending_nonce, true); + if (unlinked) { + tor_assert(unlinked->is_client); + SMARTLIST_FOREACH_BEGIN(unlinked->legs, leg_t *, leg) { + /* Convert to origin circ and get cpath */ + const origin_circuit_t *ocirc = CONST_TO_ORIGIN_CIRCUIT(leg->circ); + node_t *node = node_get_mutable_by_id( + ocirc->cpath->next->extend_info->identity_digest); + if (node) { + smartlist_add(excluded, node); + } + } SMARTLIST_FOREACH_END(leg); + } +} + /** The given circuit is conflux pending and has closed. This deletes the leg * from the set, attempt to finalize it and relaunch a new leg. If the set is * empty after removing this leg, it is deleted. */ diff --git a/src/core/or/conflux_pool.h b/src/core/or/conflux_pool.h index 4276049485..547b4d3974 100644 --- a/src/core/or/conflux_pool.h +++ b/src/core/or/conflux_pool.h @@ -21,6 +21,11 @@ void conflux_predict_new(time_t now); bool conflux_launch_leg(const uint8_t *nonce); +void conflux_add_guards_to_exclude_list(const origin_circuit_t *circ, + smartlist_t *excluded); +void conflux_add_middles_to_exclude_list(const origin_circuit_t *circ, + smartlist_t *excluded); + void conflux_circuit_has_closed(circuit_t *circ); void conflux_circuit_has_opened(origin_circuit_t *orig_circ); void conflux_circuit_about_to_free(circuit_t *circ); diff --git a/src/feature/client/entrynodes.c b/src/feature/client/entrynodes.c index b078382e76..4783faf9dd 100644 --- a/src/feature/client/entrynodes.c +++ b/src/feature/client/entrynodes.c @@ -126,6 +126,7 @@ #include "core/or/circuitlist.h" #include "core/or/circuitstats.h" #include "core/or/circuituse.h" +#include "core/or/conflux_pool.h" #include "core/or/policies.h" #include "feature/client/bridges.h" #include "feature/client/circpathbias.h" @@ -151,6 +152,8 @@ #include "core/or/origin_circuit_st.h" #include "app/config/or_state_st.h" +#include "core/or/conflux_util.h" + /** A list of existing guard selection contexts. */ static smartlist_t *guard_contexts = NULL; /** The currently enabled guard selection context. */ @@ -1588,6 +1591,19 @@ guard_create_exit_restriction(const uint8_t *exit_id) return rst; } +/* Allocate and return a new exit guard restriction that excludes all current + * and pending conflux guards */ +STATIC entry_guard_restriction_t * +guard_create_conflux_restriction(const origin_circuit_t *circ) +{ + entry_guard_restriction_t *rst = NULL; + rst = tor_malloc_zero(sizeof(entry_guard_restriction_t)); + rst->type = RST_EXCL_LIST; + rst->excluded = smartlist_new(); + conflux_add_guards_to_exclude_list(circ, rst->excluded); + return rst; +} + /** If we have fewer than this many possible usable guards, don't set * MD-availability-based restrictions: we might denylist all of them. */ #define MIN_GUARDS_FOR_MD_RESTRICTION 10 @@ -1680,6 +1696,8 @@ entry_guard_obeys_restriction(const entry_guard_t *guard, return guard_obeys_exit_restriction(guard, rst); } else if (rst->type == RST_OUTDATED_MD_DIRSERVER) { return guard_obeys_md_dirserver_restriction(guard); + } else if (rst->type == RST_EXCL_LIST) { + return !smartlist_contains_digest(rst->excluded, guard->identity); } tor_assert_nonfatal_unreached(); @@ -2427,6 +2445,11 @@ entry_guard_has_higher_priority(entry_guard_t *a, entry_guard_t *b) STATIC void entry_guard_restriction_free_(entry_guard_restriction_t *rst) { + if (rst && rst->excluded) { + SMARTLIST_FOREACH(rst->excluded, void *, g, + tor_free(g)); + smartlist_free(rst->excluded); + } tor_free(rst); } @@ -3780,7 +3803,8 @@ guards_update_all(void) /** Helper: pick a guard for a circuit, with whatever algorithm is used. */ const node_t * -guards_choose_guard(cpath_build_state_t *state, +guards_choose_guard(const origin_circuit_t *circ, + cpath_build_state_t *state, uint8_t purpose, circuit_guard_state_t **guard_state_out) { @@ -3788,14 +3812,18 @@ guards_choose_guard(cpath_build_state_t *state, const uint8_t *exit_id = NULL; entry_guard_restriction_t *rst = NULL; - /* Only apply restrictions if we have a specific exit node in mind, and only - * if we are not doing vanguard circuits: we don't want to apply guard - * restrictions to vanguard circuits. */ - if (state && !circuit_should_use_vanguards(purpose) && + /* If we this is a conflux circuit, build an exclusion list for it. */ + if (CIRCUIT_IS_CONFLUX(TO_CIRCUIT(circ))) { + rst = guard_create_conflux_restriction(circ); + /* Don't allow connecting back to the exit if there is one */ + if (state && (exit_id = build_state_get_exit_rsa_id(state))) { + /* add the exit_id to the excluded list */ + smartlist_add(rst->excluded, tor_memdup(exit_id, DIGEST_LEN)); + } + } else if (state && !circuit_should_use_vanguards(purpose) && (exit_id = build_state_get_exit_rsa_id(state))) { /* We're building to a targeted exit node, so that node can't be - * chosen as our guard for this circuit. Remember that fact in a - * restriction. */ + * chosen as our guard for this circuit, unless we're vanguards. */ rst = guard_create_exit_restriction(exit_id); tor_assert(rst); } diff --git a/src/feature/client/entrynodes.h b/src/feature/client/entrynodes.h index 08fd7cf745..2a94775430 100644 --- a/src/feature/client/entrynodes.h +++ b/src/feature/client/entrynodes.h @@ -294,7 +294,9 @@ typedef enum guard_restriction_type_t { /* Don't pick the same guard node as our exit node (or its family) */ RST_EXIT_NODE = 0, /* Don't pick dirguards that have previously shown to be outdated */ - RST_OUTDATED_MD_DIRSERVER = 1 + RST_OUTDATED_MD_DIRSERVER = 1, + /* Don't pick guards if they are in the exclusion list */ + RST_EXCL_LIST = 2, } guard_restriction_type_t; /** @@ -312,6 +314,10 @@ struct entry_guard_restriction_t { * digest must not equal this; and it must not be in the same family as any * node with this digest. */ uint8_t exclude_id[DIGEST_LEN]; + + /* In the case of RST_EXCL_LIST, any identity digests in this list + * must not be used. */ + smartlist_t *excluded; }; /** @@ -337,7 +343,8 @@ struct circuit_guard_state_t { /* Common entry points for old and new guard code */ int guards_update_all(void); -const node_t *guards_choose_guard(cpath_build_state_t *state, +const node_t *guards_choose_guard(const origin_circuit_t *circ, + cpath_build_state_t *state, uint8_t purpose, circuit_guard_state_t **guard_state_out); const node_t *guards_choose_dirguard(uint8_t dir_purpose, @@ -597,6 +604,9 @@ STATIC entry_guard_restriction_t *guard_create_exit_restriction( STATIC entry_guard_restriction_t *guard_create_dirserver_md_restriction(void); +STATIC entry_guard_restriction_t * guard_create_conflux_restriction( + const origin_circuit_t *circ); + STATIC void entry_guard_restriction_free_(entry_guard_restriction_t *rst); #define entry_guard_restriction_free(rst) \ FREE_AND_NULL(entry_guard_restriction_t, \