From: Arran Cudbard-Bell Date: Mon, 28 May 2018 11:50:36 +0000 (+0600) Subject: Move state tree init and configuration to proto_radius_auth X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b805fbc6011cd9f3b4c3fd6642188013df5cedb8;p=thirdparty%2Ffreeradius-server.git Move state tree init and configuration to proto_radius_auth We should add a mechanism for sharing the state trees between listeners in future. --- diff --git a/raddb/radrelay.conf.in b/raddb/radrelay.conf.in index 6b455c16e3d..e06181a29e7 100644 --- a/raddb/radrelay.conf.in +++ b/raddb/radrelay.conf.in @@ -55,7 +55,6 @@ pidfile = ${run_dir}/${name}.pid # max_request_time = 30 cleanup_delay = 5 -max_requests = 65536 # # Logging section. diff --git a/raddb/sites-available/default b/raddb/sites-available/default index aed1a8e63d2..f931d08369e 100644 --- a/raddb/sites-available/default +++ b/raddb/sites-available/default @@ -266,43 +266,63 @@ server default { # needed for other packet types. # Access-Request { - # Log the full User-Name attribute, as it was found in the request. - # - # allowed values: {no, yes} - # - stripped_names = no + log { + # Log the full User-Name attribute, as it was found in the request. + # + # allowed values: {no, yes} + # + stripped_names = no - # Log authentication requests to the log file. - # - # allowed values: {no, yes} - # - auth = no + # Log authentication requests to the log file. + # + # allowed values: {no, yes} + # + auth = no - # Log passwords with the authentication requests. - # - # auth_badpass - logs password if it's rejected - # auth_goodpass - logs password if it's correct - # - # allowed values: {no, yes} - # - auth_badpass = no - auth_goodpass = no + # Log passwords with the authentication requests. + # + # auth_badpass - logs password if it's rejected + # auth_goodpass - logs password if it's correct + # + # allowed values: {no, yes} + # + auth_badpass = no + auth_goodpass = no + + # Log additional text at the end of the "Login OK" messages. + # for these to work, the "auth" and "auth_goodpass" or "auth_badpass" + # configurations above have to be set to "yes". + # + # The strings below are dynamically expanded, which means that + # you can put anything you want in them. However, note that + # this expansion can be slow, and can negatively impact server + # performance. + # +# msg_goodpass = "" +# msg_badpass = "" + + # The message when the user exceeds the Simultaneous-Use limit. + # + msg_denied = "You are already logged in - access denied" + } - # Log additional text at the end of the "Login OK" messages. - # for these to work, the "auth" and "auth_goodpass" or "auth_badpass" - # configurations above have to be set to "yes". # - # The strings below are dynamically expanded, which means that - # you can put anything you want in them. However, note that - # this expansion can be slow, and can negatively impact server - # performance. + # Controls how ongoing (multi-round) sessions are handled + # This is primarily useful for EAP to control the number + # of EAP authentication attempts that can occur concurrently. # -# msg_goodpass = "" -# msg_badpass = "" + session { + # The maximum number of ongoing sessions + # +# max = 4096 - # The message when the user exceeds the Simultaneous-Use limit. - # - msg_denied = "You are already logged in - access denied" + # Timeout (in seconds) - How long to wait before expiring a + # session. The timer starts when a response with a state + # value is sent, and stops when a request containing the + # previously sent state value is received. + # +# timeout = 15 + } } } diff --git a/src/include/radiusd.h b/src/include/radiusd.h index 4565d780d99..957695b3eba 100644 --- a/src/include/radiusd.h +++ b/src/include/radiusd.h @@ -111,8 +111,6 @@ typedef struct main_config { uint32_t max_request_time; //!< How long a request can be processed for before //!< timing out. - uint32_t continuation_timeout; //!< How long to wait before cleaning up state entries. - uint32_t max_requests; uint32_t num_networks; //!< number of network threads uint32_t num_workers; //!< number of network threads @@ -296,7 +294,6 @@ struct rad_request { #define SECONDS_PER_DAY 86400 #define MAX_REQUEST_TIME 30 #define CLEANUP_DELAY 5 -#define MAX_REQUESTS 256 #define RETRY_DELAY 5 #define RETRY_COUNT 3 #define DEAD_TIME 120 diff --git a/src/include/state.h b/src/include/state.h index f2d1eb52ab0..57bd8843584 100644 --- a/src/include/state.h +++ b/src/include/state.h @@ -31,9 +31,8 @@ extern "C" { #endif typedef struct fr_state_tree_t fr_state_tree_t; -extern fr_state_tree_t *global_state; -fr_state_tree_t *fr_state_tree_init(TALLOC_CTX *ctx, uint32_t max_sessions, uint32_t timeout); +fr_state_tree_t *fr_state_tree_init(TALLOC_CTX *ctx, fr_dict_attr_t const *da, uint32_t max_sessions, uint32_t timeout); void fr_state_discard(fr_state_tree_t *state, REQUEST *request); diff --git a/src/main/mainconfig.c b/src/main/mainconfig.c index 83246e7c944..f08906bb12b 100644 --- a/src/main/mainconfig.c +++ b/src/main/mainconfig.c @@ -169,8 +169,6 @@ static const CONF_PARSER server_config[] = { { FR_CONF_POINTER("panic_action", FR_TYPE_STRING, &main_config.panic_action) }, { FR_CONF_POINTER("hostname_lookups", FR_TYPE_BOOL, &fr_dns_lookups), .dflt = "no" }, { FR_CONF_POINTER("max_request_time", FR_TYPE_UINT32, &main_config.max_request_time), .dflt = STRINGIFY(MAX_REQUEST_TIME) }, - { FR_CONF_POINTER("continuation_timeout", FR_TYPE_UINT32, &main_config.continuation_timeout), .dflt = "15" }, - { FR_CONF_POINTER("max_requests", FR_TYPE_UINT32, &main_config.max_requests), .dflt = STRINGIFY(MAX_REQUESTS) }, { FR_CONF_POINTER("pidfile", FR_TYPE_STRING, &main_config.pid_file), .dflt = "${run_dir}/radiusd.pid"}, { FR_CONF_POINTER("debug_level", FR_TYPE_UINT32, &main_config.debug_level), .dflt = "0" }, diff --git a/src/main/radiusd.c b/src/main/radiusd.c index eb2ecd596aa..3d1f32c8b0e 100644 --- a/src/main/radiusd.c +++ b/src/main/radiusd.c @@ -633,11 +633,6 @@ int main(int argc, char *argv[]) */ if (log_global_init(&default_log, main_config.daemonize) < 0) fr_exit(EXIT_FAILURE); - /* - * Initialise the state rbtree (used to link multiple rounds of challenges). - */ - global_state = fr_state_tree_init(autofree, main_config.max_requests * 2, main_config.continuation_timeout); - /* * Start the network / worker threads. */ @@ -830,8 +825,6 @@ int main(int argc, char *argv[]) */ log_global_free(); - talloc_free(global_state); /* Free state entries */ - cleanup: /* * Free xlat instance data, and call any detach methods diff --git a/src/main/state.c b/src/main/state.c index ce29ffeeb7c..9f8513ef81d 100644 --- a/src/main/state.c +++ b/src/main/state.c @@ -118,9 +118,9 @@ struct fr_state_tree_t { fr_state_entry_t *head, *tail; //!< Entries to expire. uint32_t timeout; //!< How long to wait before cleaning up state entires. pthread_mutex_t mutex; //!< Synchronisation mutex. -}; -fr_state_tree_t *global_state = NULL; + fr_dict_attr_t const *da; //!< State attribute used. +}; #define PTHREAD_MUTEX_LOCK if (main_config.spawn_workers) pthread_mutex_lock #define PTHREAD_MUTEX_UNLOCK if (main_config.spawn_workers) pthread_mutex_unlock @@ -166,19 +166,21 @@ static int _state_tree_free(fr_state_tree_t *state) */ talloc_free(state->tree); - if (state == global_state) global_state = NULL; - return 0; } /** Initialise a new state tree * - * @param ctx to link the lifecycle of the state tree to. - * @param max_sessions we track state for. - * @param timeout How long to wait before cleaning up entries. - * @return a new state tree or NULL on failure. + * @param[in] ctx to link the lifecycle of the state tree to. + * @param[in] da Attribute used to store and retrieve state from. + * @param[in] max_sessions we track state for. + * @param[in] timeout How long to wait before cleaning up entries. + * @return + * - A new state tree. + * - NULL on failure. */ -fr_state_tree_t *fr_state_tree_init(TALLOC_CTX *ctx, uint32_t max_sessions, uint32_t timeout) +fr_state_tree_t *fr_state_tree_init(TALLOC_CTX *ctx, fr_dict_attr_t const *da, + uint32_t max_sessions, uint32_t timeout) { fr_state_tree_t *state; @@ -215,6 +217,8 @@ fr_state_tree_t *fr_state_tree_init(TALLOC_CTX *ctx, uint32_t max_sessions, uint } talloc_set_destructor(state, _state_tree_free); + state->da = da; /* Remember which attribute we use to load/store state */ + return state; } @@ -401,7 +405,7 @@ static fr_state_entry_t *state_entry_create(fr_state_tree_t *state, REQUEST *req * int the reply, we use that in preference to the * old state. */ - vp = fr_pair_find_by_num(packet->vps, 0, FR_STATE, TAG_ANY); + vp = fr_pair_find_by_da(packet->vps, state->da, TAG_ANY); if (vp) { if (DEBUG_ENABLED && (vp->vp_length > sizeof(entry->state))) { WARN("State too long, will be truncated. Expected <= %zd bytes, got %zu bytes", @@ -443,7 +447,7 @@ static fr_state_entry_t *state_entry_create(fr_state_tree_t *state, REQUEST *req */ entry->state_comp.server_id = main_config.state_server_id; - vp = fr_pair_afrom_num(packet, 0, FR_STATE); + vp = fr_pair_afrom_da(packet, state->da); fr_pair_value_memcpy(vp, entry->state, sizeof(entry->state)); fr_pair_add(&packet->vps, vp); } @@ -462,7 +466,7 @@ static fr_state_entry_t *state_entry_create(fr_state_tree_t *state, REQUEST *req RERROR("Failed inserting state entry (post alloc) - At maximum ongoing session limit (%u)", state->max_sessions); failed: - fr_pair_delete_by_num(&packet->vps, 0, FR_STATE, TAG_ANY); + fr_pair_delete_by_da(&packet->vps, state->da); talloc_free(entry); return NULL; } @@ -508,7 +512,7 @@ static fr_state_entry_t *state_entry_find(fr_state_tree_t *state, REQUEST *reque VALUE_PAIR *vp; fr_state_entry_t *entry, my_entry; - vp = fr_pair_find_by_num(packet->vps, 0, FR_STATE, TAG_ANY); + vp = fr_pair_find_by_da(packet->vps, state->da, TAG_ANY); if (!vp) return NULL; if (vp->vp_length != sizeof(my_entry.state)) return NULL; @@ -578,7 +582,7 @@ void fr_state_to_request(fr_state_tree_t *state, REQUEST *request) /* * No State, don't do anything. */ - if (!fr_pair_find_by_num(request->packet->vps, 0, FR_STATE, TAG_ANY)) { + if (!fr_pair_find_by_da(request->packet->vps, state->da, TAG_ANY)) { RDEBUG3("No &request:State attribute, can't restore &session-state"); if (request->seq_start == 0) request->seq_start = request->number; /* Need check for fake requests */ return; diff --git a/src/main/unit_test_module.c b/src/main/unit_test_module.c index d8b86dc2289..6f4a0d8047b 100644 --- a/src/main/unit_test_module.c +++ b/src/main/unit_test_module.c @@ -81,6 +81,7 @@ static fr_dict_attr_t const *attr_packet_type; static fr_dict_attr_t const *attr_response_packet_type; static fr_dict_attr_t const *attr_chap_password; static fr_dict_attr_t const *attr_digest_attributes; +static fr_dict_attr_t const *attr_state; static fr_dict_attr_t const *attr_user_name; static fr_dict_attr_t const *attr_user_password; @@ -107,6 +108,7 @@ fr_dict_attr_autoload_t unit_test_module_dict_attr[] = { { .out = &attr_response_packet_type, .name = "Response-Packet-Type", .type = FR_TYPE_UINT32, .dict = &dict_freeradius }, { .out = &attr_chap_password, .name = "CHAP-Password", .type = FR_TYPE_OCTETS, .dict = &dict_radius }, { .out = &attr_digest_attributes, .name = "Digest-Attributes", .type = FR_TYPE_OCTETS, .dict = &dict_radius }, + { .out = &attr_state, .name = "State", .type = FR_TYPE_OCTETS, .dict = &dict_radius }, { .out = &attr_user_name, .name = "User-Name", .type = FR_TYPE_STRING, .dict = &dict_radius }, { .out = &attr_user_password, .name = "User-Password", .type = FR_TYPE_STRING, .dict = &dict_radius }, { NULL } @@ -942,7 +944,7 @@ int main(int argc, char *argv[]) if (modules_thread_instantiate(thread_ctx, main_config.config, el) < 0) goto exit_failure; if (xlat_thread_instantiate(thread_ctx) < 0) goto exit_failure; - state = fr_state_tree_init(NULL, main_config.max_requests * 2, 10); + state = fr_state_tree_init(autofree, attr_state, 256, 10); /* * Set the panic action (if required) diff --git a/src/modules/proto_radius/proto_radius_auth.c b/src/modules/proto_radius/proto_radius_auth.c index c8ef0cc760e..f9c4816b675 100644 --- a/src/modules/proto_radius/proto_radius_auth.c +++ b/src/modules/proto_radius/proto_radius_auth.c @@ -44,13 +44,25 @@ typedef struct { char const *denied_msg; //!< Additional text to append if the user is already logged //!< in (simultaneous use check failed). + + uint32_t session_timeout; //!< Maximum time between the last response and next request. + uint32_t max_session; //!< Maximum ongoing session allowed. + + fr_state_tree_t *state_tree; //!< State tree to link multiple requests/responses. } proto_radius_auth_t; -static const CONF_PARSER proto_radius_auth_config[] = { - { FR_CONF_OFFSET("log_stripped_names", FR_TYPE_BOOL, proto_radius_auth_t, log_stripped_names), .dflt = "no" }, - { FR_CONF_OFFSET("log_auth", FR_TYPE_BOOL, proto_radius_auth_t, log_auth), .dflt = "no" }, - { FR_CONF_OFFSET("log_auth_badpass", FR_TYPE_BOOL, proto_radius_auth_t, log_auth_badpass), .dflt = "no" }, - { FR_CONF_OFFSET("log_auth_goodpass", FR_TYPE_BOOL,proto_radius_auth_t, log_auth_goodpass), .dflt = "no" }, +static const CONF_PARSER session_config[] = { + { FR_CONF_OFFSET("timeout", FR_TYPE_UINT32, proto_radius_auth_t, session_timeout), .dflt = "15" }, + { FR_CONF_OFFSET("max", FR_TYPE_UINT32, proto_radius_auth_t, max_session), .dflt = "4096" }, + + CONF_PARSER_TERMINATOR +}; + +static const CONF_PARSER log_config[] = { + { FR_CONF_OFFSET("stripped_names", FR_TYPE_BOOL, proto_radius_auth_t, log_stripped_names), .dflt = "no" }, + { FR_CONF_OFFSET("auth", FR_TYPE_BOOL, proto_radius_auth_t, log_auth), .dflt = "no" }, + { FR_CONF_OFFSET("auth_badpass", FR_TYPE_BOOL, proto_radius_auth_t, log_auth_badpass), .dflt = "no" }, + { FR_CONF_OFFSET("auth_goodpass", FR_TYPE_BOOL,proto_radius_auth_t, log_auth_goodpass), .dflt = "no" }, { FR_CONF_OFFSET("msg_badpass", FR_TYPE_STRING, proto_radius_auth_t, auth_badpass_msg) }, { FR_CONF_OFFSET("msg_goodpass", FR_TYPE_STRING, proto_radius_auth_t, auth_goodpass_msg) }, { FR_CONF_OFFSET("msg_denied", FR_TYPE_STRING, proto_radius_auth_t, denied_msg), .dflt = "You are already logged in - access denied" }, @@ -58,6 +70,14 @@ static const CONF_PARSER proto_radius_auth_config[] = { CONF_PARSER_TERMINATOR }; +static const CONF_PARSER proto_radius_auth_config[] = { + { FR_CONF_POINTER("log", FR_TYPE_SUBSECTION, NULL), .subcs = (void const *) log_config }, + + { FR_CONF_POINTER("session", FR_TYPE_SUBSECTION, NULL), .subcs = (void const *) session_config }, + + CONF_PARSER_TERMINATOR +}; + static fr_dict_t *dict_freeradius; static fr_dict_t *dict_radius; @@ -219,7 +239,7 @@ static fr_io_final_t mod_process(void const *instance, REQUEST *request, fr_io_a /* * Grab the VPS and data associated with the State attribute. */ - if (!request->parent) fr_state_to_request(global_state, request); + if (!request->parent) fr_state_to_request(inst->state_tree, request); /* * Push the conf section into the unlang stack. @@ -535,12 +555,12 @@ static fr_io_final_t mod_process(void const *instance, REQUEST *request, fr_io_a /* * We can't create a valid response */ - if (fr_request_to_state(global_state, request) < 0) { + if (fr_request_to_state(inst->state_tree, request) < 0) { request->reply->code = FR_CODE_DO_NOT_RESPOND; return FR_IO_REPLY; } } else { - fr_state_discard(global_state, request); + fr_state_discard(inst->state_tree, request); } } @@ -572,25 +592,9 @@ static fr_io_final_t mod_process(void const *instance, REQUEST *request, fr_io_a return FR_IO_REPLY; } - -static int mod_bootstrap(UNUSED void *instance, CONF_SECTION *process_app_cs) -{ - CONF_SECTION *listen_cs = cf_item_to_section(cf_parent(process_app_cs)); - CONF_SECTION *server_cs; - - rad_assert(process_app_cs); - rad_assert(listen_cs); - - server_cs = cf_item_to_section(cf_parent(listen_cs)); - rad_assert(strcmp(cf_section_name1(server_cs), "server") == 0); - - if (virtual_server_section_attribute_define(server_cs, "authenticate", attr_auth_type) < 0) return -1; - - return 0; -} - -static int mod_instantiate(UNUSED void *instance, CONF_SECTION *process_app_cs) +static int mod_instantiate(void *instance, CONF_SECTION *process_app_cs) { + proto_radius_auth_t *inst = instance; CONF_SECTION *listen_cs = cf_item_to_section(cf_parent(process_app_cs)); CONF_SECTION *server_cs; CONF_SECTION *subcs = NULL; @@ -617,6 +621,24 @@ static int mod_instantiate(UNUSED void *instance, CONF_SECTION *process_app_cs) } } + inst->state_tree = fr_state_tree_init(inst, attr_state, inst->max_session, inst->session_timeout); + + return 0; +} + +static int mod_bootstrap(UNUSED void *instance, CONF_SECTION *process_app_cs) +{ + CONF_SECTION *listen_cs = cf_item_to_section(cf_parent(process_app_cs)); + CONF_SECTION *server_cs; + + rad_assert(process_app_cs); + rad_assert(listen_cs); + + server_cs = cf_item_to_section(cf_parent(listen_cs)); + rad_assert(strcmp(cf_section_name1(server_cs), "server") == 0); + + if (virtual_server_section_attribute_define(server_cs, "authenticate", attr_auth_type) < 0) return -1; + return 0; } diff --git a/src/modules/proto_tacacs/proto_tacacs.c b/src/modules/proto_tacacs/proto_tacacs.c index 4a2648398c3..916b883271b 100644 --- a/src/modules/proto_tacacs/proto_tacacs.c +++ b/src/modules/proto_tacacs/proto_tacacs.c @@ -30,6 +30,26 @@ #include +typedef struct { + uint32_t session_timeout; //!< Maximum time between rounds. + uint32_t max_sessions; //!< Maximum ongoing sessions. + + fr_state_tree_t *state_tree; +} proto_tacacs_t; + +static const CONF_PARSER sessions_config[] = { + { FR_CONF_OFFSET("timeout", FR_TYPE_UINT32, proto_tacacs_t, session_timeout), .dflt = "15" }, + { FR_CONF_OFFSET("max", FR_TYPE_UINT32, proto_tacacs_t, max_sessions), .dflt = "4096" }, + + CONF_PARSER_TERMINATOR +}; + +static const CONF_PARSER proto_tacacs_config[] = { + { FR_CONF_POINTER("sessions", FR_TYPE_SUBSECTION, NULL), .subcs = (void const *) sessions_config }, + + CONF_PARSER_TERMINATOR +}; + static fr_dict_t *dict_freeradius; static fr_dict_t *dict_radius; static fr_dict_t *dict_tacacs; @@ -236,7 +256,9 @@ static void tacacs_running(REQUEST *request, fr_state_signal_t action) /* FIXME only for seq_id greater than 1 */ if (tacacs_type(request->packet) == TAC_PLUS_AUTHEN) { state_add(request, request->packet); - fr_state_to_request(global_state, request); +#ifdef TACACS_HAS_BEEN_MIGRATED + fr_state_to_request(inst->state_tree, request); +#endif } RDEBUG("Running 'recv %s' from file %s", cf_section_name2(unlang), cf_filename(unlang)); @@ -250,8 +272,10 @@ static void tacacs_running(REQUEST *request, fr_state_signal_t action) if (request->master_state == REQUEST_STOP_PROCESSING) { stop_processing: +#ifdef TACACS_HAS_BEEN_MIGRATED if (tacacs_type(request->packet) == TAC_PLUS_AUTHEN) - fr_state_discard(global_state, request); + fr_state_discard(inst->state_tree, request); +#endif goto done; } @@ -413,7 +437,9 @@ send_reply: case TAC_PLUS_AUTHEN_STATUS_RESTART: case TAC_PLUS_AUTHEN_STATUS_ERROR: case TAC_PLUS_AUTHEN_STATUS_FOLLOW: - fr_state_discard(global_state, request); +#ifdef TACACS_HAS_BEEN_MIGRATED + fr_state_discard(inst->state_tree, request); +#endif break; default: vp = fr_pair_find_by_da(request->packet->vps, @@ -426,7 +452,9 @@ send_reply: /* authentication would continue but seq_no cannot continue */ if (vp->vp_uint8 == 253) { RWARN("Sequence number would wrap, restarting authentication"); - fr_state_discard(global_state, request); +#ifdef TACACS_HAS_BEEN_MIGRATED + fr_state_discard(inst->state_tree, request); +#endif fr_pair_list_free(&request->reply->vps); MEM(pair_update_reply(&vp, attr_tacacs_authentication_status) >= 0); @@ -434,12 +462,18 @@ send_reply: } else { state_add(request, request->reply); request->reply->code = 1; /* FIXME: util.c:request_verify() */ - fr_request_to_state(global_state, request); +#ifdef TACACS_HAS_BEEN_MIGRATED + fr_request_to_state(inst->state_tree, request); +#endif } } - } else { - fr_state_discard(global_state, request); + } +#ifdef TACACS_HAS_BEEN_MIGRATED + else { + fr_state_discard(inst->state_tree, request); + } +#endif } if (RDEBUG_ENABLED) tacacs_packet_debug(request, request->reply, false); @@ -638,7 +672,9 @@ rad_protocol_t proto_tacacs = { .magic = RLM_MODULE_INIT, .load = mod_load, .unload = mod_unload, - .inst_size = sizeof(listen_socket_t), + .config = proto_tacacs_config, + .inst_size = sizeof(proto_tacacs_t), + .transports = TRANSPORT_TCP, .tls = false, .compile = tacacs_listen_compile, diff --git a/src/tests/eapol_test/config/servers.conf b/src/tests/eapol_test/config/servers.conf index 84340b88689..2afe896d009 100644 --- a/src/tests/eapol_test/config/servers.conf +++ b/src/tests/eapol_test/config/servers.conf @@ -13,11 +13,6 @@ security { allow_vulnerable_openssl = yes } -# -# Max outstanding requests -# -max_requests = 10000 - # # References by some modules for default thread pool configuration #