From: Arran Cudbard-Bell Date: Fri, 25 Jul 2025 18:31:05 +0000 (-0700) Subject: Ensure process modules in virtual servers have a specific name Closes #5626 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b53c4db6dd4eea60bdf8882cfe2b312ba9938bd1;p=thirdparty%2Ffreeradius-server.git Ensure process modules in virtual servers have a specific name Closes #5626 This stops people using the wrong virtual server with EAP modules --- diff --git a/src/lib/server/virtual_servers.c b/src/lib/server/virtual_servers.c index e456e773b4..298ed57852 100644 --- a/src/lib/server/virtual_servers.c +++ b/src/lib/server/virtual_servers.c @@ -41,6 +41,7 @@ RCSID("$Id$") #include #include +#include #include #include #include @@ -1006,6 +1007,7 @@ int virtual_server_cf_parse(UNUSED TALLOC_CTX *ctx, void *out, UNUSED void *pare CONF_ITEM *ci, UNUSED conf_parser_t const *rule) { virtual_server_t const *vs; + virtual_server_cf_parse_uctx_t const *uctx = rule->uctx; vs = virtual_server_find(cf_pair_value(cf_item_to_pair(ci))); if (!vs) { @@ -1013,6 +1015,52 @@ int virtual_server_cf_parse(UNUSED TALLOC_CTX *ctx, void *out, UNUSED void *pare return -1; } + /* + * Validation checks + */ + if (uctx) { + /* + * Check the module name + * + * FIXME: ...at some point in the distance future. Names + * are icky, we should compare based on dl_module_t, but + * ordering issues make this difficult currently. + */ + if (uctx->process_module_name) { + /* catch users doing stupid things */ + if (strcmp(uctx->process_module_name, vs->process_mi->module->name) != 0) { + cf_log_err(ci, "virtual server \"%s\" must be of type \"%s\", " + "got type \"%s\"", + cf_pair_value(cf_item_to_pair(ci)), + uctx->process_module_name, vs->process_mi->module->name); + return -1; + } + } + + /* + * It's theoretically possible for the same module to be used + * with multiple namespaces, so we need this check too. + */ + if (uctx->required_dict) { + fr_dict_t *required_dict = *uctx->required_dict; + + if (!fr_cond_assert_msg(required_dict != NULL, + "dict not resolved before virtual server reference")) { + goto done; + } + + if (required_dict != *vs->process_module->dict) { + cf_log_err(ci, "virtual server \"%s\" must use namespace \"%s\", " + "got namespace \"%s\"", + cf_pair_value(cf_item_to_pair(ci)), + fr_dict_root(required_dict)->name, + fr_dict_root(*vs->process_module->dict)->name); + return -1; + } + } + } + +done: *((CONF_SECTION **)out) = vs->server_cs; return 0; diff --git a/src/lib/server/virtual_servers.h b/src/lib/server/virtual_servers.h index 3d1d236a82..34e439b284 100644 --- a/src/lib/server/virtual_servers.h +++ b/src/lib/server/virtual_servers.h @@ -89,6 +89,34 @@ virtual_server_t const *virtual_server_find(char const *name) CC_HINT(nonnull); virtual_server_t const *virtual_server_by_child(CONF_ITEM const *ci) CC_HINT(nonnull); +/** Additional validation rules for virtual server lookup + * + * This is used to ensure that the virtual server we find matches + * the requirements of the caller. + * + * This should be passed in the rule calling this function: + @code{.c} + { FR_CONF_OFFSET_TYPE_FLAGS("virtual_server", FR_TYPE_VOID, 0, module_conf_t, virtual_server), + .func = virtual_server_cf_parse, + .uctx = &(virtual_server_cf_parse_uctx_t){ .process_module_name = "eap_aka"} }, + @endcode + * + * @note process_module_name is the name field in the exported symbol of the module. + * + * The double dereference is to work around compile time constant issues. + */ +typedef struct { + char const *process_module_name; //!< Virtual server we find must use this + ///< this process module. This is a string + ///< identifier as the process modules are + ///< often not loaded at the same time as the modules + ///< and this makes ordering issues easier to deal with. + ///< Ignored if NULL. + + fr_dict_t **required_dict; //!< Virtual server we find must have this + ///< dictionary. Ignored if NULL +} virtual_server_cf_parse_uctx_t; + int virtual_server_cf_parse(TALLOC_CTX *ctx, void *out, void *parent, CONF_ITEM *ci, conf_parser_t const *rule) CC_HINT(nonnull(2,4)); /** @} */ diff --git a/src/modules/rlm_eap/types/rlm_eap_aka/rlm_eap_aka.c b/src/modules/rlm_eap/types/rlm_eap_aka/rlm_eap_aka.c index 3b4b693574..5fd4dc6e89 100644 --- a/src/modules/rlm_eap/types/rlm_eap_aka/rlm_eap_aka.c +++ b/src/modules/rlm_eap/types/rlm_eap_aka/rlm_eap_aka.c @@ -37,7 +37,11 @@ RCSID("$Id$") #include static conf_parser_t submodule_config[] = { - { FR_CONF_OFFSET_TYPE_FLAGS("virtual_server", FR_TYPE_VOID, 0, eap_aka_sim_module_conf_t, virtual_server), .func = virtual_server_cf_parse }, + { FR_CONF_OFFSET_TYPE_FLAGS("virtual_server", FR_TYPE_VOID, 0, eap_aka_sim_module_conf_t, virtual_server), + .func = virtual_server_cf_parse, + .uctx = &(virtual_server_cf_parse_uctx_t){ + .process_module_name = "eap_aka", + } }, { FR_CONF_OFFSET_IS_SET("prefer_aka_prime", FR_TYPE_BOOL, 0, eap_aka_sim_module_conf_t, aka.send_at_bidding_prefer_prime ), .dflt = "no" }, CONF_PARSER_TERMINATOR diff --git a/src/modules/rlm_eap/types/rlm_eap_aka_prime/rlm_eap_aka_prime.c b/src/modules/rlm_eap/types/rlm_eap_aka_prime/rlm_eap_aka_prime.c index 35d53bac0d..a53c488033 100644 --- a/src/modules/rlm_eap/types/rlm_eap_aka_prime/rlm_eap_aka_prime.c +++ b/src/modules/rlm_eap/types/rlm_eap_aka_prime/rlm_eap_aka_prime.c @@ -37,7 +37,9 @@ RCSID("$Id$") #include static conf_parser_t submodule_config[] = { - { FR_CONF_OFFSET_TYPE_FLAGS("virtual_server", FR_TYPE_VOID, 0, eap_aka_sim_module_conf_t, virtual_server), .func = virtual_server_cf_parse }, + { FR_CONF_OFFSET_TYPE_FLAGS("virtual_server", FR_TYPE_VOID, 0, eap_aka_sim_module_conf_t, virtual_server), + .func = virtual_server_cf_parse, + .uctx = &(virtual_server_cf_parse_uctx_t){ .process_module_name = "eap_aka_prime"} }, CONF_PARSER_TERMINATOR }; diff --git a/src/modules/rlm_eap/types/rlm_eap_fast/rlm_eap_fast.c b/src/modules/rlm_eap/types/rlm_eap_fast/rlm_eap_fast.c index 5bdf05b798..797f9bb22d 100644 --- a/src/modules/rlm_eap/types/rlm_eap_fast/rlm_eap_fast.c +++ b/src/modules/rlm_eap/types/rlm_eap_fast/rlm_eap_fast.c @@ -46,7 +46,7 @@ typedef struct { char const *default_provisioning_method_name; int default_provisioning_method; - char const *virtual_server; //!< Virtual server to use for processing + virtual_server_t *virtual_server; //!< Virtual server to use for processing //!< inner EAP method. CONF_SECTION *server_cs; char const *cipher_list; //!< cipher list specific to EAP-FAST @@ -67,7 +67,10 @@ static conf_parser_t submodule_config[] = { { FR_CONF_OFFSET("default_provisioning_eap_type", rlm_eap_fast_t, default_provisioning_method_name), .dflt = "mschapv2" }, - { FR_CONF_OFFSET_FLAGS("virtual_server", CONF_FLAG_REQUIRED | CONF_FLAG_NOT_EMPTY, rlm_eap_fast_t, virtual_server) }, + { FR_CONF_OFFSET_TYPE_FLAGS("virtual_server", FR_TYPE_VOID, CONF_FLAG_REQUIRED | CONF_FLAG_NOT_EMPTY, rlm_eap_fast_t, virtual_server), + .func = virtual_server_cf_parse, + .uctx = &(virtual_server_cf_parse_uctx_t){ .process_module_name = "eap_fast"} }, + { FR_CONF_OFFSET("cipher_list", rlm_eap_fast_t, cipher_list) }, { FR_CONF_OFFSET("require_client_cert", rlm_eap_fast_t, req_client_cert), .dflt = "no" }, @@ -629,19 +632,8 @@ static int mod_instantiate(module_inst_ctx_t const *mctx) { rlm_eap_fast_t *inst = talloc_get_type_abort(mctx->mi->data, rlm_eap_fast_t); CONF_SECTION *conf = mctx->mi->conf; - virtual_server_t const *virtual_server = virtual_server_find(inst->virtual_server); - - if (!virtual_server) { - cf_log_err_by_child(mctx->mi->conf, "virtual_server", "Unknown virtual server '%s'", - inst->virtual_server); - return -1; - } - inst->server_cs = virtual_server_cs(virtual_server); - if (!inst->server_cs) { - cf_log_err_by_child(conf, "virtual_server", "Virtual server \"%s\" missing", inst->virtual_server); - return -1; - } + inst->server_cs = virtual_server_cs(inst->virtual_server); inst->default_provisioning_method = eap_name2type(inst->default_provisioning_method_name); if (!inst->default_provisioning_method) { diff --git a/src/modules/rlm_eap/types/rlm_eap_peap/rlm_eap_peap.c b/src/modules/rlm_eap/types/rlm_eap_peap/rlm_eap_peap.c index 9fbe5cd673..b1ba5fe27c 100644 --- a/src/modules/rlm_eap/types/rlm_eap_peap/rlm_eap_peap.c +++ b/src/modules/rlm_eap/types/rlm_eap_peap/rlm_eap_peap.c @@ -39,7 +39,7 @@ typedef struct { bool copy_request_to_tunnel; //!< Use SOME of the request attributes from outside of the //!< tunneled session in the tunneled request. - char const *virtual_server; //!< Virtual server for inner tunnel session. + virtual_server_t *virtual_server; //!< Virtual server for inner tunnel session. CONF_SECTION *server_cs; bool req_client_cert; //!< Do we do require a client cert? @@ -52,8 +52,9 @@ static conf_parser_t submodule_config[] = { { FR_CONF_DEPRECATED("use_tunneled_reply", rlm_eap_peap_t, NULL), .dflt = "no" }, - { FR_CONF_OFFSET_FLAGS("virtual_server", CONF_FLAG_REQUIRED | CONF_FLAG_NOT_EMPTY, rlm_eap_peap_t, virtual_server) }, - + { FR_CONF_OFFSET_TYPE_FLAGS("virtual_server", FR_TYPE_VOID, CONF_FLAG_REQUIRED | CONF_FLAG_NOT_EMPTY, rlm_eap_peap_t, virtual_server), + .func = virtual_server_cf_parse, + .uctx = &(virtual_server_cf_parse_uctx_t){ .process_module_name = "eap_peap"} }, { FR_CONF_OFFSET("require_client_cert", rlm_eap_peap_t, req_client_cert), .dflt = "no" }, CONF_PARSER_TERMINATOR @@ -380,18 +381,8 @@ static int mod_instantiate(module_inst_ctx_t const *mctx) { rlm_eap_peap_t *inst = talloc_get_type_abort(mctx->mi->data, rlm_eap_peap_t); CONF_SECTION *conf = mctx->mi->conf; - virtual_server_t const *virtual_server = virtual_server_find(inst->virtual_server); - - if (!virtual_server) { - cf_log_err_by_child(conf, "virtual_server", "Unknown virtual server '%s'", inst->virtual_server); - return -1; - } - inst->server_cs = virtual_server_cs(virtual_server); - if (!inst->server_cs) { - cf_log_err_by_child(conf, "virtual_server", "Virtual server \"%s\" missing", inst->virtual_server); - return -1; - } + inst->server_cs = virtual_server_cs(inst->virtual_server); /* * Read tls configuration, either from group given by 'tls' diff --git a/src/modules/rlm_eap/types/rlm_eap_sim/rlm_eap_sim.c b/src/modules/rlm_eap/types/rlm_eap_sim/rlm_eap_sim.c index 920a77051d..a835ba816a 100644 --- a/src/modules/rlm_eap/types/rlm_eap_sim/rlm_eap_sim.c +++ b/src/modules/rlm_eap/types/rlm_eap_sim/rlm_eap_sim.c @@ -37,7 +37,9 @@ RCSID("$Id$") #include static conf_parser_t submodule_config[] = { - { FR_CONF_OFFSET_TYPE_FLAGS("virtual_server", FR_TYPE_VOID, CONF_FLAG_REQUIRED, eap_aka_sim_module_conf_t, virtual_server), .func = virtual_server_cf_parse }, + { FR_CONF_OFFSET_TYPE_FLAGS("virtual_server", FR_TYPE_VOID, CONF_FLAG_REQUIRED, eap_aka_sim_module_conf_t, virtual_server), + .func = virtual_server_cf_parse, + .uctx = &(virtual_server_cf_parse_uctx_t){ .process_module_name = "eap_sim"} }, CONF_PARSER_TERMINATOR }; diff --git a/src/modules/rlm_eap/types/rlm_eap_ttls/rlm_eap_ttls.c b/src/modules/rlm_eap/types/rlm_eap_ttls/rlm_eap_ttls.c index 9629265110..ae1613eabf 100644 --- a/src/modules/rlm_eap/types/rlm_eap_ttls/rlm_eap_ttls.c +++ b/src/modules/rlm_eap/types/rlm_eap_ttls/rlm_eap_ttls.c @@ -37,8 +37,8 @@ typedef struct { /* * TLS configuration */ - char const *tls_conf_name; - fr_tls_conf_t *tls_conf; + char const *tls_conf_name; + fr_tls_conf_t *tls_conf; /* * RFC 5281 (TTLS) says that the length field MUST NOT be @@ -49,18 +49,18 @@ typedef struct { * RFC, we add the option here. If set to "no", it sends * the length field in ONLY the first fragment. */ - bool include_length; + bool include_length; /* * Virtual server for inner tunnel session. */ - char const *virtual_server; - CONF_SECTION *server_cs; + virtual_server_t *virtual_server; + CONF_SECTION *server_cs; /* * Do we do require a client cert? */ - bool req_client_cert; + bool req_client_cert; } rlm_eap_ttls_t; @@ -68,7 +68,9 @@ static conf_parser_t submodule_config[] = { { FR_CONF_OFFSET("tls", rlm_eap_ttls_t, tls_conf_name) }, { FR_CONF_DEPRECATED("copy_request_to_tunnel", rlm_eap_ttls_t, NULL), .dflt = "no" }, { FR_CONF_DEPRECATED("use_tunneled_reply", rlm_eap_ttls_t, NULL), .dflt = "no" }, - { FR_CONF_OFFSET_FLAGS("virtual_server", CONF_FLAG_REQUIRED | CONF_FLAG_NOT_EMPTY, rlm_eap_ttls_t, virtual_server) }, + { FR_CONF_OFFSET_TYPE_FLAGS("virtual_server", FR_TYPE_VOID, CONF_FLAG_REQUIRED | CONF_FLAG_NOT_EMPTY, rlm_eap_ttls_t, virtual_server), + .func = virtual_server_cf_parse, + .uctx = &(virtual_server_cf_parse_uctx_t){ .process_module_name = "eap_ttls"} }, { FR_CONF_OFFSET("include_length", rlm_eap_ttls_t, include_length), .dflt = "yes" }, { FR_CONF_OFFSET("require_client_cert", rlm_eap_ttls_t, req_client_cert), .dflt = "no" }, CONF_PARSER_TERMINATOR @@ -300,18 +302,8 @@ static int mod_instantiate(module_inst_ctx_t const *mctx) { rlm_eap_ttls_t *inst = talloc_get_type_abort(mctx->mi->data, rlm_eap_ttls_t); CONF_SECTION *conf = mctx->mi->conf; - virtual_server_t const *virtual_server = virtual_server_find(inst->virtual_server); - if (!virtual_server) { - cf_log_err_by_child(conf, "virtual_server", "Unknown virtual server '%s'", inst->virtual_server); - return -1; - } - - inst->server_cs = virtual_server_cs(virtual_server); - if (!inst->server_cs) { - cf_log_err_by_child(conf, "virtual_server", "Virtual server \"%s\" missing", inst->virtual_server); - return -1; - } + inst->server_cs = virtual_server_cs(inst->virtual_server); /* * Read tls configuration, either from group given by 'tls' diff --git a/src/modules/rlm_ocsp/conf.c b/src/modules/rlm_ocsp/conf.c index c2a80c271b..a6395867f4 100644 --- a/src/modules/rlm_ocsp/conf.c +++ b/src/modules/rlm_ocsp/conf.c @@ -2,7 +2,9 @@ static conf_parser_t ocsp_config[] = { { FR_CONF_OFFSET("enable", fr_tls_ocsp_conf_t, enable), .dflt = "no" }, - { FR_CONF_OFFSET("virtual_server", fr_tls_ocsp_conf_t, cache_server) }, + { FR_CONF_OFFSET_TYPE_FLAGS("virtual_server", FR_TYPE_VOID, CONF_FLAG_REQUIRED | CONF_FLAG_NOT_EMPTY, fr_tls_ocsp_conf_t, virtual_server), + .func = virtual_server_cf_parse, + .uctx = &(virtual_server_cf_parse_uctx_t){ .process_module_name = "ocsp"} }, { FR_CONF_OFFSET("override_cert_url", fr_tls_ocsp_conf_t, override_url), .dflt = "no" }, { FR_CONF_OFFSET("url", fr_tls_ocsp_conf_t, url) },