From: Alan T. DeKok Date: Sun, 25 Jan 2026 21:15:34 +0000 (-0500) Subject: add and document dedup_key X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=73ff4e0d101eca30224813c163d4a9cb0e0782ce;p=thirdparty%2Ffreeradius-server.git add and document dedup_key --- diff --git a/doc/antora/modules/reference/pages/raddb/mods-available/eap.adoc b/doc/antora/modules/reference/pages/raddb/mods-available/eap.adoc index 676faca6a22..f1bfe6e0d80 100644 --- a/doc/antora/modules/reference/pages/raddb/mods-available/eap.adoc +++ b/doc/antora/modules/reference/pages/raddb/mods-available/eap.adoc @@ -42,6 +42,13 @@ it is enabled by OS and device vendors. +Session tracking has moved to the `session` section of a virtual server. + * timer_expire --> timeout + * max_sessions --> max + * dedup_key --> dedup_key + + + default_eap_type:: The default EAP submodule to invoke when an `EAP-Identity` response is received. @@ -153,7 +160,7 @@ may use the 'snakeoil' certificates. Included with the server in If these certificates have not been auto-generated: - cd raddb/certs + cd _confdir_/certs make These test certificates *SHOULD NOT* be used in a normal diff --git a/doc/antora/modules/reference/pages/raddb/sites-available/default.adoc b/doc/antora/modules/reference/pages/raddb/sites-available/default.adoc index e8fde7278fb..84e9309d785 100644 --- a/doc/antora/modules/reference/pages/raddb/sites-available/default.adoc +++ b/doc/antora/modules/reference/pages/raddb/sites-available/default.adoc @@ -133,6 +133,14 @@ max:: The maximum number of ongoing sessions ``` +max_rounds:: The maximum number of message exchanges +before the server gives up + +``` +# max_rounds = 40 + +``` + timeout:: How long to wait before expiring a session. @@ -144,6 +152,33 @@ state value is received. ``` # timeout = 15 + +``` + +dedup_key:: Key to enforce only one active session per +supplicant. + +Some supplicants may misbehave by starting many thousands +of EAP sessions, but never finishing them. These sessions +can cause the server to hit 'max_sessions' very quickly. +The 'timer_expire' configuration above does not help as +much as it could, because the old (duplicate) session +should be deleted as soon as the new one comes in. + +If you set the 'dedup_key' below, whenever the EAP module +starts a new session, it will check for a previous session +which has the same dedup key. If a previous session +is found, it is deleted. + +Setting this configuration item may cause issues if the +same device uses multiple EAP sessions at the same time. +But that device behavior should be rare to non-existent. + +The configuration item is commented out so that upgrades +do not change existing behavior. + +``` +# dedup_key = Calling-Station-Id } } diff --git a/raddb/mods-available/eap b/raddb/mods-available/eap index 1b2141a92de..33d77245199 100644 --- a/raddb/mods-available/eap +++ b/raddb/mods-available/eap @@ -46,6 +46,13 @@ eap { # # require_identity_realm = nai + # + # Session tracking has moved to the `session` section of a virtual server. + # * timer_expire --> timeout + # * max_sessions --> max + # * dedup_key --> dedup_key + # + # # default_eap_type:: The default EAP submodule to invoke when an `EAP-Identity` # response is received. diff --git a/raddb/sites-available/default b/raddb/sites-available/default index 28b75be9dec..5732c252097 100644 --- a/raddb/sites-available/default +++ b/raddb/sites-available/default @@ -121,6 +121,13 @@ server default { # # max = 4096 + + # + # max_rounds:: The maximum number of message exchanges + # before the server gives up + # +# max_rounds = 40 + # # timeout:: How long to wait before expiring a # session. @@ -132,6 +139,31 @@ server default { # state value is received. # # timeout = 15 + + # + # dedup_key:: Key to enforce only one active session per + # supplicant. + # + # Some supplicants may misbehave by starting many thousands + # of EAP sessions, but never finishing them. These sessions + # can cause the server to hit 'max_sessions' very quickly. + # The 'timer_expire' configuration above does not help as + # much as it could, because the old (duplicate) session + # should be deleted as soon as the new one comes in. + # + # If you set the 'dedup_key' below, whenever the EAP module + # starts a new session, it will check for a previous session + # which has the same dedup key. If a previous session + # is found, it is deleted. + # + # Setting this configuration item may cause issues if the + # same device uses multiple EAP sessions at the same time. + # But that device behavior should be rare to non-existent. + # + # The configuration item is commented out so that upgrades + # do not change existing behavior. + # +# dedup_key = Calling-Station-Id } } diff --git a/src/lib/server/state.c b/src/lib/server/state.c index d2d025c0bb7..68f3b016244 100644 --- a/src/lib/server/state.c +++ b/src/lib/server/state.c @@ -61,6 +61,7 @@ const conf_parser_t state_session_config[] = { { FR_CONF_OFFSET("max", fr_state_config_t, max_sessions), .dflt = "4096" }, { FR_CONF_OFFSET("max_rounds", fr_state_config_t, max_rounds), .dflt = "50" }, { FR_CONF_OFFSET("state_server_id", fr_state_config_t, server_id) }, + { FR_CONF_OFFSET("dedup_key", fr_state_config_t, dedup_key) }, CONF_PARSER_TERMINATOR }; @@ -70,7 +71,7 @@ const conf_parser_t state_session_config[] = { * */ typedef struct { - uint64_t id; //!< State number within state heap. + uint64_t id; //!< State number fr_rb_node_t node; //!< Entry in the state rbtree. union { /** Server ID components @@ -127,6 +128,9 @@ typedef struct { fr_dlist_head_t data; //!< Persistable request data, also parented by ctx. request_t *thawed; //!< The request that thawed this entry. + + fr_value_box_t const *dedup_key; //!< Key for dedup + fr_rb_node_t dedup_node; //!< Entry in the dedup rbtree } fr_state_entry_t; /** A child of a fr_state_entry_t @@ -155,6 +159,7 @@ struct fr_state_tree_s { fr_state_config_t config; //!< a local copy fr_rb_tree_t *tree; //!< rbtree used to lookup state value. + fr_rb_tree_t *dedup_tree; //!< rbtree used to do dedups fr_dlist_head_t to_expire; //!< Linked list of entries to free. pthread_mutex_t mutex; //!< Synchronisation mutex. @@ -179,6 +184,17 @@ static int8_t state_entry_cmp(void const *one, void const *two) return CMP(ret, 0); } +/** Compare two fr_state_entry_t based on their dedup key + * + */ +static int8_t state_dedup_cmp(void const *one, void const *two) +{ + fr_state_entry_t const *a = one, *b = two; + + return fr_value_box_cmp(a->dedup_key, b->dedup_key); +} + + /** Free the state tree * */ @@ -232,6 +248,25 @@ fr_state_tree_t *fr_state_tree_init(TALLOC_CTX *ctx, fr_dict_attr_t const *da, f state->config = *config; state->da = da; /* Remember which attribute we use to load/store state */ + /* + * Some systems may start a new session before closing + * out the old one. The dedup key lets us find + * pre-existing sessions, and close them out. + */ + if (config->dedup_key) { + if ((!tmpl_is_attr(config->dedup_key) && + !tmpl_is_xlat(config->dedup_key)) || + tmpl_needs_resolving(config->dedup_key)) { + fr_strerror_const("Invalid value for \"dedup_key\" - it must be an attribute reference or a simple expansion"); + return NULL; + } + + if (tmpl_async_required(config->dedup_key)) { + fr_strerror_const("Invalid value for \"dedup_key\" - it must be a simple expansion, and cannot query external systems such as databases"); + return NULL; + } + } + /* * Create a break in the contexts. * We still want this to be freed at the same time @@ -261,6 +296,14 @@ fr_state_tree_t *fr_state_tree_init(TALLOC_CTX *ctx, fr_dict_attr_t const *da, f } talloc_set_destructor(state, _state_tree_free); + if (config->dedup_key) { + state->dedup_tree = fr_rb_inline_talloc_alloc(state->tree, fr_state_entry_t, dedup_node, state_dedup_cmp, NULL); + if (!state->dedup_tree) { + talloc_free(state); + return NULL; + } + } + return state; } @@ -277,6 +320,7 @@ void state_entry_unlink(fr_state_tree_t *state, fr_state_entry_t *entry) fr_dlist_remove(&state->to_expire, entry); fr_rb_delete(state->tree, entry); + if (state->dedup_tree) fr_rb_delete(state->dedup_tree, entry); DEBUG4("State ID %" PRIu64 " unlinked", entry->id); } @@ -346,7 +390,8 @@ static void state_entry_fill(fr_state_entry_t *entry, fr_value_box_t const *vb) * @note Called with the mutex held. */ static fr_state_entry_t *state_entry_create(fr_state_tree_t *state, request_t *request, - fr_pair_list_t *reply_list, fr_state_entry_t *old) + fr_pair_list_t *reply_list, fr_state_entry_t *old, + fr_value_box_t const *dedup_key) { fr_time_t now = fr_time(); fr_pair_t *vp; @@ -357,16 +402,39 @@ static fr_state_entry_t *state_entry_create(fr_state_tree_t *state, request_t *r fr_dlist_head_t to_free; /* - * Shouldn't be in any lists if it's being reused + * If we have a previous entry, then it can't be in an + * expiry list, and it can't be in the list of states + * where we have sent a reply. */ fr_assert(!old || (!fr_dlist_entry_in_list(&old->expire_entry) && !fr_rb_node_inline_in_tree(&old->node))); + /* + * If we have a previous entry and a dedup_tree, then we + * must have a dedup key, AND the entry must be in the + * dedup tree. + */ + fr_assert(!old || !state->dedup_tree || (old->dedup_key && fr_rb_node_inline_in_tree(&old->dedup_node))); + + /* + * If there is an old entry, we can't have a dedup_key. + */ + fr_assert(!old || (old || !dedup_key)); + + /* + * We track a separate free list, as we have to check + * expiration with the mutex locked. But we want to free + * things with the mutex unlocked. + */ fr_dlist_init(&to_free, fr_state_entry_t, free_entry); /* - * Clean up expired entries + * Clean up expired entries which have not finished. If + * the request fails, then the corresponding entry is + * discarded. So the expiration list is only for entries + * which have been half-started, and then (many seconds + * later) haven't seen a "next" packet. */ for (entry = fr_dlist_head(&state->to_expire); entry != NULL; @@ -374,7 +442,10 @@ static fr_state_entry_t *state_entry_create(fr_state_tree_t *state, request_t *r (void)talloc_get_type_abort(entry, fr_state_entry_t); /* Allow examination */ next = fr_dlist_next(&state->to_expire, entry); /* Advance *before* potential unlinking */ - if (entry == old) continue; + /* + * It's active (and asserted so above), so it can't be in the expiry list. + */ + fr_assert(entry != old); /* * Too old, we can delete it. @@ -389,8 +460,6 @@ static fr_state_entry_t *state_entry_create(fr_state_tree_t *state, request_t *r break; } - state->timed_out += timed_out; - if (!old) { /* * We're inserting a new session. Limit the @@ -406,12 +475,27 @@ static fr_state_entry_t *state_entry_create(fr_state_tree_t *state, request_t *r * shouldn't be a problem. */ too_many = (fr_rb_num_elements(state->tree) >= state->config.max_sessions) && (timed_out == 0); + + /* + * If there is a previous session for the same dedup key, then remove the old one from + * the dedup tree. + */ + if (dedup_key) { + fr_state_entry_t *unfinished; + + unfinished = fr_rb_find(state->dedup_tree, &(fr_state_entry_t) { .dedup_key = dedup_key }); + if (unfinished) { + state_entry_unlink(state, unfinished); + fr_dlist_insert_tail(&to_free, unfinished); + } + } } PTHREAD_MUTEX_UNLOCK(&state->mutex); if (timed_out > 0) { RWDEBUG("Cleaning up %"PRIu64" timed out state entries", timed_out); + state->timed_out += timed_out; /* * Now free the unlinked entries. @@ -429,6 +513,7 @@ static fr_state_entry_t *state_entry_create(fr_state_tree_t *state, request_t *r } } else if (too_many) { + talloc_const_free(dedup_key); RERROR("Failed inserting state entry - At maximum ongoing session limit (%u)", state->config.max_sessions); return NULL; @@ -484,6 +569,8 @@ static fr_state_entry_t *state_entry_create(fr_state_tree_t *state, request_t *r size_t i; uint32_t hash; + if (dedup_key) entry->dedup_key = talloc_steal(entry, dedup_key); + /* * Get a bunch of random numbers. */ @@ -540,6 +627,7 @@ static fr_state_entry_t *state_entry_create(fr_state_tree_t *state, request_t *r PTHREAD_MUTEX_LOCK(&state->mutex); if (!fr_rb_insert(state->tree, entry)) { + fail_unlock: PTHREAD_MUTEX_UNLOCK(&state->mutex); RERROR("Failed inserting state entry - Insertion into state tree failed"); fail: @@ -548,6 +636,13 @@ static fr_state_entry_t *state_entry_create(fr_state_tree_t *state, request_t *r return NULL; } + /* + * Ensure that we can de-duplicate things if the supplicant is misbehaving. + */ + if (state->dedup_tree && !old) { + if (!fr_rb_insert(state->dedup_tree, entry)) goto fail_unlock; + } + /* * Link it to the end of the list, which is implicitly * ordered by cleanup time. @@ -721,6 +816,7 @@ int fr_state_store(fr_state_tree_t *state, request_t *request) fr_state_entry_t *entry, *old; fr_dlist_head_t data; fr_pair_t *state_ctx; + fr_value_box_t *dedup_key = NULL; old = request_data_get(request, state, 0); request_data_list_init(&data); @@ -742,15 +838,34 @@ int fr_state_store(fr_state_tree_t *state, request_t *request) #endif } + /* + * If there's a dedup tree, then we need to expand the + * key, but only if we don't already have a pre-existing state. + */ + if (state->dedup_tree && !old) { + fr_value_box_list_t list; + + fr_value_box_list_init(&list); + + if (tmpl_eval(NULL, &list, request, state->config.dedup_key) < 0) { + REDEBUG("Failed expanding dedup_key - not doing dedup"); + } else { + dedup_key = fr_value_box_list_pop_head(&list); + if (!dedup_key) { + RDEBUG("Failed expanding dedup_key - not doing dedup due to empty output"); + } + fr_value_box_list_talloc_free_head(&list); + } + } + MEM(state_ctx = request_state_replace(request, NULL)); /* * Reuses old if possible, and leaves the mutex unlocked on failure. */ PTHREAD_MUTEX_LOCK(&state->mutex); - entry = state_entry_create(state, request, &request->reply_pairs, old); + entry = state_entry_create(state, request, &request->reply_pairs, old, dedup_key); if (!entry) { - PTHREAD_MUTEX_UNLOCK(&state->mutex); talloc_free(request_state_replace(request, state_ctx)); request_data_restore(request, &data); /* Put it back again */ return -1; diff --git a/src/lib/server/state.h b/src/lib/server/state.h index 5bef6b9a656..c274fdd4485 100644 --- a/src/lib/server/state.h +++ b/src/lib/server/state.h @@ -33,6 +33,7 @@ extern "C" { #include #include #include +#include typedef struct fr_state_tree_s fr_state_tree_t; @@ -41,6 +42,7 @@ typedef struct { uint32_t max_rounds; //!< maximum number of rounds before we give up uint32_t context_id; //!< internal number to help keep state trees separate fr_time_delta_t timeout; //!< idle timeout + tmpl_t *dedup_key; //!< for tracking misbehaving supplicants uint8_t server_id; //!< for mangling State bool thread_safe; } fr_state_config_t;