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 e456e773b48b326e9a2dd04f466bd5b6ae403ac8..298ed578521910ceb7026ba734eadf63df488681 100644 (file)
--- a/src/lib/server/virtual_servers.c
+++ b/src/lib/server/virtual_servers.c
@@ -41,6 +41,7 @@ RCSID("$Id$")
 #include <freeradius-devel/io/master.h>
 #include <freeradius-devel/io/listen.h>
 
+#include <freeradius-devel/util/debug.h>
 #include <freeradius-devel/util/dict.h>
 #include <freeradius-devel/util/pair.h>
 #include <freeradius-devel/util/talloc.h>
@@ -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 3d1d236a82ff81642613331386b1c3f9e9940a84..34e439b284d7f1509d57122287d4bee2a7852161 100644 (file)
--- 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 3b4b693574a18aa54c80db906c8fa9e40be93231..5fd4dc6e89026dd93cffd5db031ccca292b9d8f9 100644 (file)
--- 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 <freeradius-devel/util/debug.h>
 
 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 35d53bac0dd9ea1cbef7e66a940935f89db7fac5..a53c488033b79978b72a54b59a0787c6faf1a019 100644 (file)
--- 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 <freeradius-devel/util/debug.h>
 
 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 5bdf05b7981604afab09a74907587001b2e69b2f..797f9bb22d80dcb5977956c6e1c1fb78428a0528 100644 (file)
--- 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 9fbe5cd673e6a19fc2b74d6de44fd780343e6657..b1ba5fe27c14cc31b241345da8ebc276734f6142 100644 (file)
--- 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 920a77051d4204b780289a6b1d13337be6ec366f..a835ba816acff65c4bf2a5a686d7fb08dec3391b 100644 (file)
--- 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 <freeradius-devel/util/debug.h>
 
 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 9629265110e94edd17e6ab27b3e59320afacccbb..ae1613eabf2f99fee4194928ec6914dbff65f241 100644 (file)
--- 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 c2a80c271be97810a2e4af9aea5c631f1f78f45a..a6395867f418da571a2ef619bb585a5cbbb5360c 100644 (file)
--- 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) },