From e0a74797bd3a8395b81e68ecfa7ada6e2b4be4c6 Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Fri, 14 Jun 2013 01:33:26 -0400 Subject: 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 4f88c55662..0fd3f2c1d5 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 73f404bdf7..63e0e8a32e 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 12945b4d65..059e00b994 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)); } -- cgit