Restrict the acceptable char set for module names

diff --git a/src/lib/server/module.c b/src/lib/server/module.c
index 9b1f2d5f17ff828f164ff85a9ba7980a4d48ddd9..3fd90e5db68f0a8cac3ea2750bab60d1ee061db8 100644 (file)
--- a/src/lib/server/module.c
+++ b/src/lib/server/module.c
@@ -701,14 +701,30 @@ char const *module_instance_root_prefix_str(module_instance_t const *mi)
 /** Avoid boilerplate when setting the module instance name
  *
  */
-char const *module_instance_name_from_conf(CONF_SECTION *conf)
+fr_slen_t module_instance_name_from_conf(char const **name, CONF_SECTION *conf)
 {
-       char const *name2;
+       char const      *name2;
+       char const      *inst_name;
+       fr_slen_t       slen;
 
        name2 = cf_section_name2(conf);
-       if (name2) return name2;
+       if (name2) {
+               inst_name = name2;
+               goto done;
+       }
+
+       inst_name = cf_section_name1(conf);
+done:
+       slen = module_instance_name_valid(inst_name);
+       if (slen < 0) {
+               cf_log_perr(conf, "Invalid module configuration");
+               *name = NULL;
+               return slen;
+       }
 
-       return cf_section_name1(conf);
+       *name = inst_name;
+
+       return 0;
 }
 
 /** Covert a CONF_SECTION into parsed module instance data
@@ -1537,6 +1553,58 @@ void module_instance_data_alloc(module_data_pool_t *pool_out, void **out,
        *out = data;
 }
 
+/** Check to see if a module instance name is valid
+ *
+ * @note On failure the error message may be retrieved with fr_strerror().
+ *
+ * @param[in] inst_name                to check.
+ *
+ * @return
+ *     - 0 on success.
+ *     - Negative value on error indicating the position of the bad char.
+ */
+fr_slen_t module_instance_name_valid(char const *inst_name)
+{
+       static bool const inst_allowed_chars[UINT8_MAX + 1] = {
+               ['-'] = true, ['/'] = true, ['_'] = true,
+               ['0'] = true, ['1'] = true, ['2'] = true, ['3'] = true, ['4'] = true,
+               ['5'] = true, ['6'] = true, ['7'] = true, ['8'] = true, ['9'] = true,
+               ['A'] = true, ['B'] = true, ['C'] = true, ['D'] = true, ['E'] = true,
+               ['F'] = true, ['G'] = true, ['H'] = true, ['I'] = true, ['J'] = true,
+               ['K'] = true, ['L'] = true, ['M'] = true, ['N'] = true, ['O'] = true,
+               ['P'] = true, ['Q'] = true, ['R'] = true, ['S'] = true, ['T'] = true,
+               ['U'] = true, ['V'] = true, ['W'] = true, ['X'] = true, ['Y'] = true,
+               ['Z'] = true,
+               ['a'] = true, ['b'] = true, ['c'] = true, ['d'] = true, ['e'] = true,
+               ['f'] = true, ['g'] = true, ['h'] = true, ['i'] = true, ['j'] = true,
+               ['k'] = true, ['l'] = true, ['m'] = true, ['n'] = true, ['o'] = true,
+               ['p'] = true, ['q'] = true, ['r'] = true, ['s'] = true, ['t'] = true,
+               ['u'] = true, ['v'] = true, ['w'] = true, ['x'] = true, ['y'] = true,
+               ['z'] = true
+       };
+
+       /*
+        *      [] are used for dynamic module selection.
+        *      . is used as a method and submodule separator.
+        *      Quoting and other characters would just confuse the parser in too many
+        *      instances so they're disallowed too.
+        */
+       {
+               size_t len = strlen(inst_name);
+
+               for (size_t i = 0; i < len; i++) {
+                       if (!inst_allowed_chars[(uint8_t)inst_name[i]]) {
+                               fr_strerror_printf("Instance name \"%s\" contains an invalid character.  "
+                                                  "Valid characters are [0-9a-zA-Z/_-]", inst_name);
+                               return -(i + 1);
+                       }
+               }
+       }
+
+       return 0;
+}
+
+
 /** Allocate a new module and add it to a module list for later bootstrap/instantiation
  *
  * - Load the module shared library.

diff --git a/src/lib/server/module.h b/src/lib/server/module.h
index 05380981aa4ca294f87b270ae7df47756eaa39a1..4b38912e6a6ba67425f69af0eacc79774b0dc808 100644 (file)
--- a/src/lib/server/module.h
+++ b/src/lib/server/module.h
@@ -415,7 +415,7 @@ int                 module_instance_data_unprotect(module_instance_t const *mi);
  *
  * @{
  */
-char const             *module_instance_name_from_conf(CONF_SECTION *conf);
+fr_slen_t              module_instance_name_from_conf(char const **name, CONF_SECTION *conf);
 
 int                    module_instance_conf_parse(module_instance_t *mi, CONF_SECTION *conf);
 
@@ -464,6 +464,8 @@ int                 module_bootstrap(module_instance_t *mi) CC_HINT(nonnull) CC_HINT(warn_unus
 
 int                    modules_bootstrap(module_list_t const *ml) CC_HINT(nonnull) CC_HINT(warn_unused_result);
 
+fr_slen_t              module_instance_name_valid(char const *inst_name) CC_HINT(nonnull);
+
 module_instance_t      *module_instance_copy(module_list_t *dst, module_instance_t const *src, char const *inst_name)
                        CC_HINT(nonnull(1,2)) CC_HINT(warn_unused_result);
 

diff --git a/src/lib/server/module_rlm.c b/src/lib/server/module_rlm.c
index 647463dfc9a5929e82dc2cb68f8518a4f39f0af4..663922f81b629c45d3052d51d494d53d5aa0cedc 100644 (file)
--- a/src/lib/server/module_rlm.c
+++ b/src/lib/server/module_rlm.c
@@ -966,6 +966,7 @@ int modules_rlm_instantiate(void)
 static int module_conf_parse(module_list_t *ml, CONF_SECTION *mod_conf)
 {
        char const              *name;
+       char const              *inst_name;
        module_instance_t       *mi = NULL;
        CONF_SECTION            *actions;
 
@@ -1001,7 +1002,9 @@ static int module_conf_parse(module_list_t *ml, CONF_SECTION *mod_conf)
                return 0;
        }
 
-       mi = module_instance_alloc(ml, NULL, DL_MODULE_TYPE_MODULE, name, module_instance_name_from_conf(mod_conf), 0);
+       if (module_instance_name_from_conf(&inst_name, mod_conf) < 0) goto invalid_name;
+
+       mi = module_instance_alloc(ml, NULL, DL_MODULE_TYPE_MODULE, name, inst_name, 0);
        if (unlikely(mi == NULL)) {
                cf_log_perr(mod_conf, "Failed loading module");
                return -1;

diff --git a/src/lib/server/virtual_servers.c b/src/lib/server/virtual_servers.c
index f3f6e9c3a9aa54cf3f50066534e063238f452515..729100b64a4d05e56334583d7ed861858e9a7379 100644 (file)
--- a/src/lib/server/virtual_servers.c
+++ b/src/lib/server/virtual_servers.c
@@ -229,6 +229,7 @@ static int namespace_on_read(TALLOC_CTX *ctx, UNUSED void *out, UNUSED void *par
        module_instance_t               *mi;
        char const                      *namespace;
        char                            *module_name, *p, *end;
+       char const                      *inst_name;
        fr_process_module_t const       *process;
 
        fr_cond_assert_msg(process_modules,
@@ -244,6 +245,7 @@ static int namespace_on_read(TALLOC_CTX *ctx, UNUSED void *out, UNUSED void *par
             p < end;
             p++) if (*p == '-') *p = '_';
 
+       if (module_instance_name_from_conf(&inst_name, server_cs) < 0) return -1;
 
        /*
         *      The module being loaded is the namespace with all '-'
@@ -252,7 +254,7 @@ static int namespace_on_read(TALLOC_CTX *ctx, UNUSED void *out, UNUSED void *par
         *      The instance name is the virtual server name.
         */
        mi = module_instance_alloc(process_modules, NULL, DL_MODULE_TYPE_PROCESS,
-                                  module_name, module_instance_name_from_conf(server_cs),
+                                  module_name, inst_name,
                                   0);
        talloc_free(module_name);
        if (mi == NULL) {
@@ -473,14 +475,17 @@ static int listen_parse(UNUSED TALLOC_CTX *ctx, void *out, UNUSED void *parent,
        inst_name = cf_section_name2(listener_cs);
        if (!inst_name) inst_name = mod_name;
 
-       MEM(qual_inst_name = talloc_asprintf(NULL, "%s.%s", cf_section_name2(server_cs), inst_name));
-       mi = module_instance_alloc(proto_modules, NULL, DL_MODULE_TYPE_PROTO, mod_name, qual_inst_name, 0);
-       talloc_free(qual_inst_name);
-       if (!mi) {
+       if (module_instance_name_valid(inst_name) < 0) {
        error:
                cf_log_err(listener_cs, "Failed loading listener");
                return -1;
        }
+
+       MEM(qual_inst_name = talloc_asprintf(NULL, "%s.%s", cf_section_name2(server_cs), inst_name));
+       mi = module_instance_alloc(proto_modules, NULL, DL_MODULE_TYPE_PROTO, mod_name, qual_inst_name, 0);
+       talloc_free(qual_inst_name);
+       if (!mi) goto error;
+
        if (unlikely(module_instance_conf_parse(mi, listener_cs) < 0)) goto error;
 
        if (DEBUG_ENABLED4) cf_log_debug(ci, "Loading %s listener into %p", inst_name, out);

diff --git a/src/listen/detail/proto_detail.c b/src/listen/detail/proto_detail.c
index 6de5f743c58ccf2e42f1cfb54271b2717f41b61d..49e39ca3b0da020906e5491cf39cc5024be66df9 100644 (file)
--- a/src/listen/detail/proto_detail.c
+++ b/src/listen/detail/proto_detail.c
@@ -163,8 +163,9 @@ static int transport_parse(TALLOC_CTX *ctx, void *out, void *parent, CONF_ITEM *
         *      If we're not loading the work submodule directly, then try to load it here.
         */
        if (strcmp(inst->io_submodule->module->dl->name, "proto_detail_work") != 0) {
-               CONF_SECTION *transport_cs;
-               module_instance_t *mi;
+               CONF_SECTION            *transport_cs;
+               module_instance_t       *mi;
+               char const              *inst_name;
 
                inst->work_submodule = NULL;
 
@@ -181,6 +182,8 @@ static int transport_parse(TALLOC_CTX *ctx, void *out, void *parent, CONF_ITEM *
                        }
                }
 
+               if (module_instance_name_from_conf(&inst_name, transport_cs) < 0) return -1;
+
                /*
                 *      This *should* get bootstrapped at some point after this module
                 *      as it's inserted into the three the caller is iterating over.
@@ -190,7 +193,7 @@ static int transport_parse(TALLOC_CTX *ctx, void *out, void *parent, CONF_ITEM *
                 *      of that list.
                 */
                inst->work_submodule = module_instance_alloc(mi->ml, mi, DL_MODULE_TYPE_SUBMODULE,
-                                                            "work", module_instance_name_from_conf(transport_cs), 0);
+                                                            "work", inst_name, 0);
                if (inst->work_submodule == NULL) {
                error:
                        cf_log_perr(mi->conf, "Failed to load proto_detail_work");