krb5 commit: Provide plugin module ordering guarantees
Greg Hudson
ghudson at MIT.EDU
Thu Jun 27 02:01:20 EDT 2013
https://github.com/krb5/krb5/commit/e0a74797bd3a8395b81e68ecfa7ada6e2b4be4c6
commit e0a74797bd3a8395b81e68ecfa7ada6e2b4be4c6
Author: Greg Hudson <ghudson at mit.edu>
Date: Fri Jun 14 01:33:26 2013 -0400
Provide plugin module ordering guarantees
Rewrite the plugin internals so that modules have a well-defined
order--either the order of enable_only tags, or dynamic modules
followed by the built-in modules in order of registration.
ticket: 7665 (new)
doc/admin/conf_files/krb5_conf.rst | 6 +
src/include/k5-int.h | 14 +-
src/lib/krb5/krb/plugin.c | 453 +++++++++++++++++++++---------------
3 files changed, 273 insertions(+), 200 deletions(-)
diff --git a/doc/admin/conf_files/krb5_conf.rst b/doc/admin/conf_files/krb5_conf.rst
index 4f88c55..0fd3f2c 100644
--- a/doc/admin/conf_files/krb5_conf.rst
+++ b/doc/admin/conf_files/krb5_conf.rst
@@ -659,6 +659,12 @@ All subsections support the same tags:
absolute path, it will be treated as relative to the
**plugin_base_dir** value from :ref:`libdefaults`.
+For pluggable interfaces where module order matters, modules
+registered with a **module** tag normally come first, in the order
+they are registered, followed by built-in modules in the order they
+are documented below. If **enable_only** tags are used, then the
+order of those tags overrides the normal module order.
+
The following subsections are currently supported within the [plugins]
section:
diff --git a/src/include/k5-int.h b/src/include/k5-int.h
index 73f404b..63e0e8a 100644
--- a/src/include/k5-int.h
+++ b/src/include/k5-int.h
@@ -1057,21 +1057,11 @@ krb5_authdata_free_internal(krb5_context kcontext,
* and krb5.conf man page.
*/
-/*
- * A linked list entry mapping a module name to a module initvt function. The
- * entry may also include a dynamic object handle so that it can be released
- * when the context is destroyed.
- */
-struct plugin_mapping {
- char *modname;
- krb5_plugin_initvt_fn module;
- struct plugin_file_handle *dyn_handle;
- struct plugin_mapping *next;
-};
+struct plugin_mapping;
/* Holds krb5_context information about each pluggable interface. */
struct plugin_interface {
- struct plugin_mapping *modules;
+ struct plugin_mapping **modules;
krb5_boolean configured;
};
diff --git a/src/lib/krb5/krb/plugin.c b/src/lib/krb5/krb/plugin.c
index 12945b4..059e00b 100644
--- a/src/lib/krb5/krb/plugin.c
+++ b/src/lib/krb5/krb/plugin.c
@@ -26,6 +26,27 @@
#include "k5-int.h"
+/*
+ * A plugin_mapping structure maps a module name to a built-in or dynamic
+ * module. modname is always present; the other three fields can be in four
+ * different states:
+ *
+ * - If dyn_path and dyn_handle are null but module is set, the mapping is to a
+ * built-in module.
+ * - If dyn_path is set but dyn_handle and module are null, the mapping is to a
+ * dynamic module which hasn't been loaded yet.
+ * - If all three fields are set, the mapping is to a dynamic module which has
+ * been loaded and is ready to use.
+ * - If all three fields are null, the mapping is to a dynamic module which
+ * failed to load and should be ignored.
+ */
+struct plugin_mapping {
+ char *modname;
+ char *dyn_path;
+ struct plugin_file_handle *dyn_handle;
+ krb5_plugin_initvt_fn module;
+};
+
const char *interface_names[] = {
"pwqual",
"kadm5_hook",
@@ -44,69 +65,96 @@ get_interface(krb5_context context, int id)
return &context->plugins[id];
}
-/* Release the memory associated with the linked list entry map. */
+/* Release the memory associated with the mapping list entry map. */
static void
free_plugin_mapping(struct plugin_mapping *map)
{
if (map == NULL)
return;
free(map->modname);
+ free(map->dyn_path);
if (map->dyn_handle != NULL)
krb5int_close_plugin(map->dyn_handle);
free(map);
}
+static void
+free_mapping_list(struct plugin_mapping **list)
+{
+ struct plugin_mapping **mp;
+
+ for (mp = list; mp != NULL && *mp != NULL; mp++)
+ free_plugin_mapping(*mp);
+ free(list);
+}
+
+/* Construct a plugin mapping object. path may be NULL (for a built-in
+ * module), or may be relative to the plugin base directory. */
+static krb5_error_code
+make_plugin_mapping(krb5_context context, const char *name, size_t namelen,
+ const char *path, krb5_plugin_initvt_fn module,
+ struct plugin_mapping **map_out)
+{
+ krb5_error_code ret;
+ struct plugin_mapping *map = NULL;
+
+ /* Create the mapping entry. */
+ map = k5alloc(sizeof(*map), &ret);
+ if (map == NULL)
+ return ret;
+
+ map->modname = k5memdup0(name, namelen, &ret);
+ if (map->modname == NULL)
+ goto oom;
+ if (path != NULL) {
+ if (k5_path_join(context->plugin_base_dir, path, &map->dyn_path))
+ goto oom;
+ }
+ map->module = module;
+ *map_out = map;
+ return 0;
+
+oom:
+ free_plugin_mapping(map);
+ return ENOMEM;
+}
+
/*
- * Register a mapping from modname to module. On success, dyn_handle is
- * remembered in the mapping and will be released when the mapping is
- * overwritten or the context is destroyed.
+ * Register a mapping from modname to either dyn_path (for an auto-registered
+ * dynamic module) or to module (for a builtin module). dyn_path may be
+ * relative to the plugin base directory.
*/
static krb5_error_code
register_module(krb5_context context, struct plugin_interface *interface,
- const char *modname, krb5_plugin_initvt_fn module,
- struct plugin_file_handle *dyn_handle)
+ const char *modname, const char *dyn_path,
+ krb5_plugin_initvt_fn module)
{
- struct plugin_mapping *map, **pmap;
-
- /* If a mapping already exists for modname, remove it. */
- for (pmap = &interface->modules; *pmap != NULL; pmap = &(*pmap)->next) {
- map = *pmap;
- if (strcmp(map->modname, modname) == 0) {
- *pmap = map->next;
- free_plugin_mapping(map);
- break;
- }
- }
+ struct plugin_mapping **list;
+ size_t count;
- /* Create a new mapping structure. */
- map = malloc(sizeof(*map));
- if (map == NULL)
- return ENOMEM;
- map->modname = strdup(modname);
- if (map->modname == NULL) {
- free(map);
+ /* Allocate list space for another element and a terminator. */
+ list = interface->modules;
+ for (count = 0; list != NULL && list[count] != NULL; count++);
+ list = realloc(interface->modules, (count + 2) * sizeof(*list));
+ if (list == NULL)
return ENOMEM;
- }
- map->module = module;
- map->dyn_handle = dyn_handle;
+ list[count] = list[count + 1] = NULL;
+ interface->modules = list;
- /* Chain it into the list. */
- map->next = interface->modules;
- interface->modules = map;
- return 0;
+ /* Create a new mapping structure and add it to the list. */
+ return make_plugin_mapping(context, modname, strlen(modname), dyn_path,
+ module, &list[count]);
}
-/* Parse a profile module string of the form "modname:modpath" into its
- * component parts. */
+/* Parse a profile module string of the form "modname:modpath" into a mapping
+ * entry. */
static krb5_error_code
parse_modstr(krb5_context context, const char *modstr,
- char **modname, char **modpath)
+ struct plugin_mapping **map_out)
{
const char *sep;
- char *name = NULL, *path = NULL;
- *modname = NULL;
- *modpath = NULL;
+ *map_out = NULL;
sep = strchr(modstr, ':');
if (sep == NULL) {
@@ -115,23 +163,8 @@ parse_modstr(krb5_context context, const char *modstr,
return KRB5_PLUGIN_BAD_MODULE_SPEC;
}
- /* Copy the module name. */
- name = malloc(sep - modstr + 1);
- if (name == NULL)
- return ENOMEM;
- memcpy(name, modstr, sep - modstr);
- name[sep - modstr] = '\0';
-
- /* Copy the module path. */
- path = strdup(sep + 1);
- if (path == NULL) {
- free(name);
- return ENOMEM;
- }
-
- *modname = name;
- *modpath = path;
- return 0;
+ return make_plugin_mapping(context, modstr, sep - modstr, sep + 1, NULL,
+ map_out);
}
/* Return true if value is found in list. */
@@ -145,108 +178,126 @@ find_in_list(char **list, const char *value)
return FALSE;
}
-/* Return true if module is not filtered out by enable or disable lists. */
-static krb5_boolean
-module_enabled(const char *modname, char **enable, char **disable)
+/* Get the list of values for the profile variable varname in the section for
+ * interface id, or NULL if no values are set. */
+static krb5_error_code
+get_profile_var(krb5_context context, int id, const char *varname, char ***out)
{
- return ((enable == NULL || find_in_list(enable, modname)) &&
- (disable == NULL || !find_in_list(disable, modname)));
-}
+ krb5_error_code ret;
+ const char *path[4];
-/* Remove any registered modules whose names are filtered out. */
-static void
-filter_builtins(krb5_context context, struct plugin_interface *interface,
- char **enable, char **disable)
-{
- struct plugin_mapping *map, **pmap;
-
- pmap = &interface->modules;
- while (*pmap != NULL) {
- map = *pmap;
- if (!module_enabled(map->modname, enable, disable)) {
- *pmap = map->next;
- free_plugin_mapping(map);
- } else
- pmap = &map->next;
- }
+ *out = NULL;
+ path[0] = KRB5_CONF_PLUGINS;
+ path[1] = interface_names[id];
+ path[2] = varname;
+ path[3] = NULL;
+ ret = profile_get_values(context->profile, path, out);
+ return (ret == PROF_NO_RELATION) ? 0 : ret;
}
+/* Expand *list_inout to contain the mappings from modstrs, followed by the
+ * existing built-in module mappings. */
static krb5_error_code
-register_dyn_module(krb5_context context, struct plugin_interface *interface,
- const char *iname, const char *modname, const char *path)
+make_full_list(krb5_context context, char **modstrs,
+ struct plugin_mapping ***list_inout)
{
- krb5_error_code ret;
- char *symname = NULL;
- struct plugin_file_handle *handle = NULL;
- void (*initvt_fn)();
+ krb5_error_code ret = 0;
+ size_t count, pos, i, j;
+ struct plugin_mapping **list, **mp;
+ char **mod;
+
+ /* Allocate space for all of the modules plus a null terminator. */
+ for (count = 0; modstrs[count] != NULL; count++);
+ for (mp = *list_inout; mp != NULL && *mp != NULL; mp++, count++);
+ list = calloc(count + 1, sizeof(*list));
+ if (list == NULL)
+ return ENOMEM;
- /* Construct the initvt symbol name for this interface and module. */
- if (asprintf(&symname, "%s_%s_initvt", iname, modname) < 0) {
- symname = NULL;
- ret = ENOMEM;
- goto cleanup;
+ /* Parse each profile module entry and store it in the list. */
+ for (mod = modstrs, pos = 0; *mod != NULL; mod++, pos++) {
+ ret = parse_modstr(context, *mod, &list[pos]);
+ if (ret != 0) {
+ free_mapping_list(list);
+ return ret;
+ }
}
- /* Open the plugin and resolve the initvt symbol. */
- ret = krb5int_open_plugin(path, &handle, &context->err);
- if (ret != 0)
- goto cleanup;
- ret = krb5int_get_plugin_func(handle, symname, &initvt_fn, &context->err);
- if (ret != 0)
- goto cleanup;
-
- /* Create a mapping for the module. */
- ret = register_module(context, interface, modname,
- (krb5_plugin_initvt_fn)initvt_fn, handle);
- if (ret != 0)
- goto cleanup;
- handle = NULL; /* Now owned by the module mapping. */
+ /* Cannibalize the old list of built-in modules. */
+ for (mp = *list_inout; mp != NULL && *mp != NULL; mp++, pos++)
+ list[pos] = *mp;
+ assert(pos == count);
+
+ /* Filter out duplicates, preferring earlier entries to later ones. */
+ for (i = 0, pos = 0; i < count; i++) {
+ for (j = 0; j < pos; j++) {
+ if (strcmp(list[i]->modname, list[j]->modname) == 0) {
+ free_plugin_mapping(list[i]);
+ break;
+ }
+ }
+ if (j == pos)
+ list[pos++] = list[i];
+ }
+ list[pos] = NULL;
-cleanup:
- free(symname);
- if (handle != NULL)
- krb5int_close_plugin(handle);
- return ret;
+ free(*list_inout);
+ *list_inout = list;
+ return 0;
}
-/* Register the plugin module given by the profile string mod, if enabled
- * according to the values of enable and disable. */
-static krb5_error_code
-register_dyn_mapping(krb5_context context, struct plugin_interface *interface,
- const char *iname, const char *modstr, char **enable,
- char **disable)
+/* Remove any entries from list which match values in disabled. */
+static void
+remove_disabled_modules(struct plugin_mapping **list, char **disable)
{
- krb5_error_code ret;
- char *modname = NULL, *modpath = NULL, *fullpath = NULL;
+ struct plugin_mapping **in, **out;
+
+ out = list;
+ for (in = list; *in != NULL; in++) {
+ if (find_in_list(disable, (*in)->modname))
+ free_plugin_mapping(*in);
+ else
+ *out++ = *in;
+ }
+ *out = NULL;
+}
- /* Parse out the module name and path, and make sure it is enabled. */
- ret = parse_modstr(context, modstr, &modname, &modpath);
- if (ret != 0)
- goto cleanup;
- /* Treat non-absolute modpaths as relative to the plugin base directory. */
- ret = k5_path_join(context->plugin_base_dir, modpath, &fullpath);
- if (ret != 0)
- goto cleanup;
- if (!module_enabled(modname, enable, disable))
- goto cleanup;
- ret = register_dyn_module(context, interface, iname, modname, fullpath);
+/* Modify list to include only the entries matching strings in enable, in
+ * the order they are listed there. */
+static void
+filter_enabled_modules(struct plugin_mapping **list, char **enable)
+{
+ size_t count, i, pos = 0;
+ struct plugin_mapping *tmp;
+
+ /* Count the number of existing entries. */
+ for (count = 0; list[count] != NULL; count++);
+
+ /* For each string in enable, look for a matching module. */
+ for (; *enable != NULL; enable++) {
+ for (i = pos; i < count; i++) {
+ if (strcmp(list[i]->modname, *enable) == 0) {
+ /* Swap the matching module into the next result position. */
+ tmp = list[pos];
+ list[pos++] = list[i];
+ list[i] = tmp;
+ break;
+ }
+ }
+ }
-cleanup:
- free(modname);
- free(modpath);
- free(fullpath);
- return ret;
+ /* Free all mappings which didn't match and terminate the list. */
+ for (i = pos; i < count; i++)
+ free_plugin_mapping(list[i]);
+ list[pos] = NULL;
}
-/* Ensure that a plugin interface is configured. id is assumed to be valid. */
+/* Ensure that a plugin interface is configured. id must be valid. */
static krb5_error_code
configure_interface(krb5_context context, int id)
{
krb5_error_code ret;
struct plugin_interface *interface = &context->plugins[id];
- const char *iname = interface_names[id];
- char **modules = NULL, **enable = NULL, **disable = NULL, **mod;
- static const char *path[4];
+ char **modstrs = NULL, **enable = NULL, **disable = NULL;
if (interface->configured)
return 0;
@@ -255,59 +306,93 @@ configure_interface(krb5_context context, int id)
assert(sizeof(interface_names) / sizeof(*interface_names) ==
PLUGIN_NUM_INTERFACES);
- /* Read the configuration variables for this interface. */
- path[0] = KRB5_CONF_PLUGINS;
- path[1] = iname;
- path[2] = KRB5_CONF_MODULE;
- path[3] = NULL;
- ret = profile_get_values(context->profile, path, &modules);
- if (ret != 0 && ret != PROF_NO_RELATION)
+ /* Get profile variables for this interface. */
+ ret = get_profile_var(context, id, KRB5_CONF_MODULE, &modstrs);
+ if (ret)
goto cleanup;
- path[2] = KRB5_CONF_ENABLE_ONLY;
- ret = profile_get_values(context->profile, path, &enable);
- if (ret != 0 && ret != PROF_NO_RELATION)
+ ret = get_profile_var(context, id, KRB5_CONF_DISABLE, &disable);
+ if (ret)
goto cleanup;
- path[2] = KRB5_CONF_DISABLE;
- ret = profile_get_values(context->profile, path, &disable);
- if (ret != 0 && ret != PROF_NO_RELATION)
+ ret = get_profile_var(context, id, KRB5_CONF_ENABLE_ONLY, &enable);
+ if (ret)
goto cleanup;
- /* Remove built-in modules which are filtered out by configuration. */
- filter_builtins(context, interface, enable, disable);
-
- /* Create mappings for dynamic modules which aren't filtered out. */
- for (mod = modules; mod && *mod; mod++) {
- ret = register_dyn_mapping(context, interface, iname, *mod,
- enable, disable);
- if (ret != 0)
- return ret;
+ /* Create the full list of dynamic and built-in modules. */
+ if (modstrs != NULL) {
+ ret = make_full_list(context, modstrs, &interface->modules);
+ if (ret)
+ goto cleanup;
}
- ret = 0;
+ /* Remove disabled modules. */
+ if (disable != NULL)
+ remove_disabled_modules(interface->modules, disable);
+
+ /* Filter and re-order the list according to enable-modules. */
+ if (enable != NULL)
+ filter_enabled_modules(interface->modules, enable);
+
cleanup:
- profile_free_list(modules);
+ profile_free_list(modstrs);
profile_free_list(enable);
profile_free_list(disable);
return ret;
}
+/* If map is for a dynamic module which hasn't been loaded yet, attempt to load
+ * it. Only try to load a module once. */
+static void
+load_if_needed(krb5_context context, struct plugin_mapping *map,
+ const char *iname)
+{
+ char *symname = NULL;
+ struct plugin_file_handle *handle;
+ void (*initvt_fn)();
+
+ if (map->module != NULL || map->dyn_path == NULL)
+ return;
+ if (asprintf(&symname, "%s_%s_initvt", iname, map->modname) < 0)
+ return;
+ if (krb5int_open_plugin(map->dyn_path, &handle, &context->err))
+ goto err;
+ if (krb5int_get_plugin_func(handle, symname, &initvt_fn, &context->err))
+ goto err;
+ free(symname);
+ map->dyn_handle = handle;
+ map->module = (krb5_plugin_initvt_fn)initvt_fn;
+ return;
+
+err:
+ /* Clean up, and also null out map->dyn_path so we don't try again. */
+ if (handle != NULL)
+ krb5int_close_plugin(handle);
+ free(symname);
+ free(map->dyn_path);
+ map->dyn_path = NULL;
+}
+
krb5_error_code
k5_plugin_load(krb5_context context, int interface_id, const char *modname,
krb5_plugin_initvt_fn *module)
{
krb5_error_code ret;
struct plugin_interface *interface = get_interface(context, interface_id);
- struct plugin_mapping *map;
+ struct plugin_mapping **mp, *map;
if (interface == NULL)
return EINVAL;
ret = configure_interface(context, interface_id);
if (ret != 0)
return ret;
- for (map = interface->modules; map != NULL; map = map->next) {
+ for (mp = interface->modules; mp != NULL && *mp != NULL; mp++) {
+ map = *mp;
if (strcmp(map->modname, modname) == 0) {
- *module = map->module;
- return 0;
+ load_if_needed(context, map, interface_names[interface_id]);
+ if (map->module != NULL) {
+ *module = map->module;
+ return 0;
+ }
+ break;
}
}
krb5_set_error_message(context, KRB5_PLUGIN_NAME_NOTFOUND,
@@ -322,7 +407,7 @@ k5_plugin_load_all(krb5_context context, int interface_id,
{
krb5_error_code ret;
struct plugin_interface *interface = get_interface(context, interface_id);
- struct plugin_mapping *map;
+ struct plugin_mapping **mp, *map;
krb5_plugin_initvt_fn *list;
size_t count;
@@ -333,18 +418,20 @@ k5_plugin_load_all(krb5_context context, int interface_id,
return ret;
/* Count the modules and allocate a list to hold them. */
- count = 0;
- for (map = interface->modules; map != NULL; map = map->next)
- count++;
- list = malloc((count + 1) * sizeof(*list));
+ mp = interface->modules;
+ for (count = 0; mp != NULL && mp[count] != NULL; count++);
+ list = calloc(count + 1, sizeof(*list));
if (list == NULL)
return ENOMEM;
/* Place each module's initvt function into list. */
count = 0;
- for (map = interface->modules; map != NULL; map = map->next)
- list[count++] = map->module;
- list[count] = NULL;
+ for (mp = interface->modules; mp != NULL && *mp != NULL; mp++) {
+ map = *mp;
+ load_if_needed(context, map, interface_names[interface_id]);
+ if (map->module != NULL)
+ list[count++] = map->module;
+ }
*modules = list;
return 0;
@@ -370,7 +457,7 @@ k5_plugin_register(krb5_context context, int interface_id, const char *modname,
if (interface->configured)
return EINVAL;
- return register_module(context, interface, modname, module, NULL);
+ return register_module(context, interface, modname, NULL, module);
}
krb5_error_code
@@ -384,12 +471,10 @@ k5_plugin_register_dyn(krb5_context context, int interface_id,
/* Disallow registering plugins after load. */
if (interface == NULL || interface->configured)
return EINVAL;
- if (asprintf(&path, "%s/%s/%s%s", context->plugin_base_dir, modsubdir,
- modname, PLUGIN_EXT) < 0)
- return ENOMEM;
- ret = register_dyn_module(context, interface,
- interface_names[interface_id], modname, path);
+ if (asprintf(&path, "%s/%s%s", modsubdir, modname, PLUGIN_EXT) < 0)
+ return ENOMEM;
+ ret = register_module(context, interface, modname, path, NULL);
free(path);
return ret;
}
@@ -398,16 +483,8 @@ void
k5_plugin_free_context(krb5_context context)
{
int i;
- struct plugin_interface *interface;
- struct plugin_mapping *map, *next;
-
- for (i = 0; i < PLUGIN_NUM_INTERFACES; i++) {
- interface = &context->plugins[i];
- for (map = interface->modules; map != NULL; map = next) {
- next = map->next;
- free_plugin_mapping(map);
- }
- interface->modules = NULL;
- interface->configured = FALSE;
- }
+
+ for (i = 0; i < PLUGIN_NUM_INTERFACES; i++)
+ free_mapping_list(context->plugins[i].modules);
+ memset(context->plugins, 0, sizeof(context->plugins));
}
More information about the cvs-krb5
mailing list