On Tue, Nov 6, 2012 at 10:03 AM, Milan Broz <mbroz@xxxxxxxxxx> wrote: > The whitelist option allows setup of hardened systems, only > modules on whitelist are allowed to load. > > This patch adds "whitelist" option and definition to libkmod > allowing to work with whitelist (similar to blakclist, except > whitelist, once defined, is unconditional). > > Without defined whitelist, there is no change. > > If at least one whitelist command is specified, module loading > is limited to module matching loaded whitelist. > > For more context see bug > https://bugzilla.redhat.com/show_bug.cgi?id=560084 > --- > Makefile.am | 4 +- > libkmod/docs/libkmod-sections.txt | 1 + > libkmod/libkmod-config.c | 72 ++++++++++++- > libkmod/libkmod-module.c | 43 +++++++- > libkmod/libkmod-private.h | 3 +- > libkmod/libkmod.h | 3 + > libkmod/libkmod.sym | 5 + > man/modprobe.d.xml | 27 +++++ > .../test-whitelist/etc/modprobe.d/modprobe.conf | 2 + > testsuite/test-whitelist.c | 114 ++++++++++++++++++++ > tools/modprobe.c | 12 ++- > 11 files changed, 275 insertions(+), 11 deletions(-) > create mode 100644 testsuite/rootfs-pristine/test-whitelist/etc/modprobe.d/modprobe.conf > create mode 100644 testsuite/test-whitelist.c > > diff --git a/Makefile.am b/Makefile.am > index e4c1a83..03aa421 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -176,7 +176,7 @@ testsuite_libtestsuite_la_LIBADD = -lrt > > TESTSUITE = testsuite/test-init testsuite/test-testsuite testsuite/test-loaded \ > testsuite/test-modinfo testsuite/test-alias testsuite/test-new-module \ > - testsuite/test-modprobe testsuite/test-blacklist \ > + testsuite/test-modprobe testsuite/test-blacklist testsuite/test-whitelist \ > testsuite/test-dependencies testsuite/test-depmod > > check_PROGRAMS = $(TESTSUITE) > @@ -198,6 +198,8 @@ testsuite_test_modprobe_LDADD = $(TESTSUITE_LDADD) > testsuite_test_modprobe_CPPFLAGS = $(TESTSUITE_CPPFLAGS) > testsuite_test_blacklist_LDADD = $(TESTSUITE_LDADD) > testsuite_test_blacklist_CPPFLAGS = $(TESTSUITE_CPPFLAGS) > +testsuite_test_whitelist_LDADD = $(TESTSUITE_LDADD) > +testsuite_test_whitelist_CPPFLAGS = $(TESTSUITE_CPPFLAGS) > testsuite_test_dependencies_LDADD = $(TESTSUITE_LDADD) > testsuite_test_dependencies_CPPFLAGS = $(TESTSUITE_CPPFLAGS) > testsuite_test_depmod_LDADD = $(TESTSUITE_LDADD) > diff --git a/libkmod/docs/libkmod-sections.txt b/libkmod/docs/libkmod-sections.txt > index e59ab7a..bf7c0d9 100644 > --- a/libkmod/docs/libkmod-sections.txt > +++ b/libkmod/docs/libkmod-sections.txt > @@ -31,6 +31,7 @@ kmod_list_prev > <FILE>libkmod-config</FILE> > kmod_config_iter > kmod_config_get_blacklists > +kmod_config_get_whitelists > kmod_config_get_install_commands > kmod_config_get_remove_commands > kmod_config_get_aliases > diff --git a/libkmod/libkmod-config.c b/libkmod/libkmod-config.c > index 398468e..f1f1181 100644 > --- a/libkmod/libkmod-config.c > +++ b/libkmod/libkmod-config.c > @@ -56,7 +56,7 @@ struct kmod_softdep { > unsigned int n_post; > }; > > -const char *kmod_blacklist_get_modname(const struct kmod_list *l) > +const char *kmod_list_get_modname(const struct kmod_list *l) humn... this is confusing. We had a "kmod_list" namespace for managing the lists. Any other option? filter/filterlist maybe? > { > return l->data; > } > @@ -303,6 +303,38 @@ static void kmod_config_free_blacklist(struct kmod_config *config, > config->blacklists = kmod_list_remove(l); > } > > +static int kmod_config_add_whitelist(struct kmod_config *config, > + const char *modname) > +{ > + char *p; > + struct kmod_list *list; > + > + DBG(config->ctx, "modname=%s\n", modname); > + > + p = strdup(modname); > + if (!p) > + goto oom_error_init; > + > + list = kmod_list_append(config->whitelists, p); > + if (!list) > + goto oom_error; > + config->whitelists = list; > + return 0; > + > +oom_error: > + free(p); > +oom_error_init: > + ERR(config->ctx, "out-of-memory modname=%s\n", modname); > + return -ENOMEM; > +} > + > +static void kmod_config_free_whitelist(struct kmod_config *config, > + struct kmod_list *l) > +{ > + free(l->data); > + config->whitelists = kmod_list_remove(l); > +} > + > static int kmod_config_add_softdep(struct kmod_config *config, > const char *modname, > const char *line) > @@ -634,6 +666,14 @@ static int kmod_config_parse(struct kmod_config *config, int fd, > > kmod_config_add_blacklist(config, > underscores(ctx, modname)); > + } else if (streq(cmd, "whitelist")) { > + char *modname = strtok_r(NULL, "\t ", &saveptr); > + > + if (modname == NULL) > + goto syntax_error; > + > + kmod_config_add_whitelist(config, > + underscores(ctx, modname)); > } else if (streq(cmd, "options")) { > char *modname = strtok_r(NULL, "\t ", &saveptr); > char *options = strtok_r(NULL, "\0", &saveptr); > @@ -703,6 +743,9 @@ void kmod_config_free(struct kmod_config *config) > while (config->blacklists) > kmod_config_free_blacklist(config, config->blacklists); > > + while (config->whitelists) > + kmod_config_free_whitelist(config, config->whitelists); > + > while (config->options) > kmod_config_free_options(config, config->options); > > @@ -955,6 +998,7 @@ enum config_type { > CONFIG_TYPE_ALIAS, > CONFIG_TYPE_OPTION, > CONFIG_TYPE_SOFTDEP, > + CONFIG_TYPE_WHITELIST, > }; > > struct kmod_config_iter { > @@ -987,7 +1031,11 @@ static struct kmod_config_iter *kmod_config_iter_new(const struct kmod_ctx* ctx, > switch (type) { > case CONFIG_TYPE_BLACKLIST: > iter->list = config->blacklists; > - iter->get_key = kmod_blacklist_get_modname; > + iter->get_key = kmod_list_get_modname; > + break; > + case CONFIG_TYPE_WHITELIST: > + iter->list = config->whitelists; > + iter->get_key = kmod_list_get_modname; > break; > case CONFIG_TYPE_INSTALL: > iter->list = config->install_commands; > @@ -1046,6 +1094,26 @@ KMOD_EXPORT struct kmod_config_iter *kmod_config_get_blacklists(const struct kmo > } > > /** > + * kmod_config_get_whitelists: > + * @ctx: kmod library context > + * > + * Retrieve an iterator to deal with the whitelist maintained inside the > + * library. See kmod_config_iter_get_key(), kmod_config_iter_get_value() and > + * kmod_config_iter_next(). At least one call to kmod_config_iter_next() must > + * be made to initialize the iterator and check if it's valid. > + * > + * Returns: a new iterator over the whitelists or NULL on failure. Free it > + * with kmod_config_iter_free_iter(). > + */ > +KMOD_EXPORT struct kmod_config_iter *kmod_config_get_whitelists(const struct kmod_ctx *ctx) > +{ > + if (ctx == NULL) > + return NULL; > + > + return kmod_config_iter_new(ctx, CONFIG_TYPE_WHITELIST); > +} > + > +/** > * kmod_config_get_install_commands: > * @ctx: kmod library context > * > diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c > index 0d87ce1..cf68fae 100644 > --- a/libkmod/libkmod-module.c > +++ b/libkmod/libkmod-module.c > @@ -850,7 +850,7 @@ static bool module_is_blacklisted(struct kmod_module *mod) > const struct kmod_list *l; > > kmod_list_foreach(l, bl) { > - const char *modname = kmod_blacklist_get_modname(l); > + const char *modname = kmod_list_get_modname(l); > > if (streq(modname, mod->name)) > return true; > @@ -859,6 +859,26 @@ static bool module_is_blacklisted(struct kmod_module *mod) > return false; > } > > +static bool module_is_not_whitelisted(struct kmod_module *mod) module_is_whitelisted > +{ > + struct kmod_ctx *ctx = mod->ctx; > + const struct kmod_config *config = kmod_get_config(ctx); > + const struct kmod_list *wl = config->whitelists; > + const struct kmod_list *l; > + > + if (!wl) > + return false; > + > + kmod_list_foreach(l, wl) { > + const char *modname = kmod_list_get_modname(l); > + > + if (streq(modname, mod->name)) > + return false; > + } > + > + return true; and don't forget to change the returns :-) > +} > + > /** > * kmod_module_apply_filter > * @ctx: kmod library context > @@ -897,6 +917,10 @@ KMOD_EXPORT int kmod_module_apply_filter(const struct kmod_ctx *ctx, > if ((filter_type & KMOD_FILTER_BUILTIN) && mod->builtin) > continue; > > + if ((filter_type & KMOD_FILTER_WHITELIST) && > + module_is_not_whitelisted(mod)) > + continue; > + > node = kmod_list_append(*output, mod); > if (node == NULL) > goto fail; > @@ -1143,7 +1167,7 @@ static int kmod_module_get_probe_list(struct kmod_module *mod, > * output or in dry-run mode. > * > * Insert a module in Linux kernel resolving dependencies, soft dependencies, > - * install commands and applying blacklist. > + * install commands and applying blacklist and whitelist. > * > * If @run_install is NULL, this function will fork and exec by calling > * system(3). Don't pass a NULL argument in @run_install if your binary is > @@ -1163,6 +1187,7 @@ KMOD_EXPORT int kmod_module_probe_insert_module(struct kmod_module *mod, > const char *options)) > { > struct kmod_list *list = NULL, *l; > + struct kmod_list *filtered = NULL; > struct probe_insert_cb cb; > int err; > > @@ -1195,8 +1220,20 @@ KMOD_EXPORT int kmod_module_probe_insert_module(struct kmod_module *mod, > if (err < 0) > return err; > > + /* Removes modules NOT in whitelist */ > + err = kmod_module_apply_filter(mod->ctx, > + KMOD_FILTER_WHITELIST, list, &filtered); > + if (err < 0) > + return err; > + > + kmod_module_unref_list(list); > + if (filtered == NULL) > + return KMOD_PROBE_APPLY_WHITELIST; > + > + list = filtered; > + filtered = NULL; So this is applied to all modules, including the dependencies. Why doesn't it follow the blacklist naming of adding a _ALL suffix? Besides that. If the user explicitly put a module in a whitelist - shouldn't it be allowed to load even if the module then starts depending on a new module? Let's say in kernel X.Y we had mod-bla-a, mod-bla-b, mod-bla-c. And in kernel X.(Y+1) some functionality were put together in mod-bla-common and the others started to depend on it. You will not load mod-bla-a regardless if it is in a blacklist. That would be a surprising effect, much more than "hey, this new module is loaded and it wasn't in previous kernel". > + > if (flags & KMOD_PROBE_APPLY_BLACKLIST_ALL) { > - struct kmod_list *filtered = NULL; > if you do this remove the blank line, too. > err = kmod_module_apply_filter(mod->ctx, > KMOD_FILTER_BLACKLIST, list, &filtered); > diff --git a/libkmod/libkmod-private.h b/libkmod/libkmod-private.h > index 4760733..0651d52 100644 > --- a/libkmod/libkmod-private.h > +++ b/libkmod/libkmod-private.h > @@ -102,6 +102,7 @@ struct kmod_config { > struct kmod_ctx *ctx; > struct kmod_list *aliases; > struct kmod_list *blacklists; > + struct kmod_list *whitelists; > struct kmod_list *options; > struct kmod_list *remove_commands; > struct kmod_list *install_commands; > @@ -112,7 +113,7 @@ struct kmod_config { > > int kmod_config_new(struct kmod_ctx *ctx, struct kmod_config **config, const char * const *config_paths) __attribute__((nonnull(1, 2,3))); > void kmod_config_free(struct kmod_config *config) __attribute__((nonnull(1))); > -const char *kmod_blacklist_get_modname(const struct kmod_list *l) __attribute__((nonnull(1))); > +const char *kmod_list_get_modname(const struct kmod_list *l) __attribute__((nonnull(1))); > const char *kmod_alias_get_name(const struct kmod_list *l) __attribute__((nonnull(1))); > const char *kmod_alias_get_modname(const struct kmod_list *l) __attribute__((nonnull(1))); > const char *kmod_option_get_options(const struct kmod_list *l) __attribute__((nonnull(1))); > diff --git a/libkmod/libkmod.h b/libkmod/libkmod.h > index d03ab19..18721f3 100644 > --- a/libkmod/libkmod.h > +++ b/libkmod/libkmod.h > @@ -106,6 +106,7 @@ struct kmod_list *kmod_list_last(const struct kmod_list *list); > */ > struct kmod_config_iter; > struct kmod_config_iter *kmod_config_get_blacklists(const struct kmod_ctx *ctx); > +struct kmod_config_iter *kmod_config_get_whitelists(const struct kmod_ctx *ctx); > struct kmod_config_iter *kmod_config_get_install_commands(const struct kmod_ctx *ctx); > struct kmod_config_iter *kmod_config_get_remove_commands(const struct kmod_ctx *ctx); > struct kmod_config_iter *kmod_config_get_aliases(const struct kmod_ctx *ctx); > @@ -162,12 +163,14 @@ enum kmod_probe { > KMOD_PROBE_APPLY_BLACKLIST_ALL = 0x10000, > KMOD_PROBE_APPLY_BLACKLIST = 0x20000, > KMOD_PROBE_APPLY_BLACKLIST_ALIAS_ONLY = 0x40000, > + KMOD_PROBE_APPLY_WHITELIST = 0x80000, > }; > > /* Flags to kmod_module_apply_filter() */ > enum kmod_filter { > KMOD_FILTER_BLACKLIST = 0x00001, > KMOD_FILTER_BUILTIN = 0x00002, > + KMOD_FILTER_WHITELIST = 0x00004, > }; > > int kmod_module_remove_module(struct kmod_module *mod, unsigned int flags); > diff --git a/libkmod/libkmod.sym b/libkmod/libkmod.sym > index 854d257..28c8462 100644 > --- a/libkmod/libkmod.sym > +++ b/libkmod/libkmod.sym > @@ -86,3 +86,8 @@ LIBKMOD_6 { > global: > kmod_module_apply_filter; > } LIBKMOD_5; > + > +LIBKMOD_11 { > +global: > + kmod_config_get_whitelists; > +} LIBKMOD_6; > diff --git a/man/modprobe.d.xml b/man/modprobe.d.xml > index dc19b23..2140c9b 100644 > --- a/man/modprobe.d.xml > +++ b/man/modprobe.d.xml > @@ -112,6 +112,33 @@ > </listitem> > </varlistentry> > <varlistentry> > + <term>whitelist <replaceable>modulename</replaceable> > + </term> > + <listitem> > + <para> > + If at least one <command>whitelist</command> command is specified, > + module loading and scripts specified > + using <command>install</command> are limited to module names > + matching a glob <replaceable>pattern</replaceable> specified in one > + of the <command>whitelist</command> commands. > + </para> > + <para> > + You can prepare whitelist of currently loaded modules e.g. by this > + command: > + </para> > + <para> > + lsmod | tail -n +2 | cut -d ' ' -f 1 | sed 's/^/whitelist /' > + </para> Please remove the example. I don't want to encourage people to use it. > + <para> > + Note that the <command>whitelist</command> command is not a direct > + opposite of the <command>blacklist</command> command. > + The <command>blacklist</command> command affects the selection > + of a module to be loaded or <command>install</command> script to > + be performed. > + </para> This means that if you have a file /etc/modprobe.d/bla.conf that contains 1 single whitelist will turn all other config files moot. Then for each install command in *other files* you need also a whitelist. Confusing... to say the least. And I need more strong words here saying to take care with this option, especially because it can render the machine unbootable in a kernel upgrade: because a module changed its name or because it started to depend on a new one. There's also no way to easily check if a module name in a whitelist became obsolete. > + </listitem> > + </varlistentry> > + <varlistentry> > <term>install <replaceable>modulename</replaceable> <replaceable>command...</replaceable> > </term> > <listitem> > diff --git a/testsuite/rootfs-pristine/test-whitelist/etc/modprobe.d/modprobe.conf b/testsuite/rootfs-pristine/test-whitelist/etc/modprobe.d/modprobe.conf > new file mode 100644 > index 0000000..9dc0ced > --- /dev/null > +++ b/testsuite/rootfs-pristine/test-whitelist/etc/modprobe.d/modprobe.conf > @@ -0,0 +1,2 @@ > +whitelist floppy > +whitelist pcspkr > diff --git a/testsuite/test-whitelist.c b/testsuite/test-whitelist.c > new file mode 100644 > index 0000000..49a3082 > --- /dev/null > +++ b/testsuite/test-whitelist.c > @@ -0,0 +1,114 @@ > +/* > + * Copyright (C) 2011-2012 ProFUSION embedded systems > + * Copyright (C) 2012 Red Hat, Inc. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <stddef.h> > +#include <errno.h> > +#include <unistd.h> > +#include <inttypes.h> > +#include <string.h> > +#include <libkmod.h> > + > +/* good luck bulding a kmod_list outside of the library... makes this blacklist > + * function rather pointless */ I realized this was copied from test-blacklist.c, but there's no need to keep this comment. It's this way by design. User is not supposed to build lists from outside the library. You can do this as a first step, removing from test-blacklist.c if you want. > +#include <libkmod-private.h> > + > +/* FIXME: hack, change name so we don't clash */ > +#undef ERR > +#include "testsuite.h" > + > +static int whitelist_1(const struct test *t) > +{ > + struct kmod_ctx *ctx; > + struct kmod_list *list = NULL, *l, *filtered; > + struct kmod_module *mod; > + int err; > + size_t len = 0; > + > + const char *names[] = { "pcspkr", "pcspkr2", "floppy", "ext4", NULL }; > + const char **name; > + > + ctx = kmod_new(NULL, NULL); > + if (ctx == NULL) > + exit(EXIT_FAILURE); > + > + for(name = names; *name; name++) { > + err = kmod_module_new_from_name(ctx, *name, &mod); > + if (err < 0) > + goto fail_lookup; > + list = kmod_list_append(list, mod); > + } > + > + err = kmod_module_apply_filter(ctx, KMOD_FILTER_WHITELIST, list, > + &filtered); > + if (err < 0) { > + ERR("Could not filter: %s\n", strerror(-err)); > + goto fail; > + } > + if (filtered == NULL) { > + ERR("All modules were filtered out!\n"); > + goto fail; > + } > + > + kmod_list_foreach(l, filtered) { > + const char *modname; > + mod = kmod_module_get_module(l); > + modname = kmod_module_get_name(mod); > + /* These are not on whitelist, must be rejected */ > + if (strcmp("pcspkr2", modname) == 0 || strcmp("ext4", modname) == 0) > + goto fail; > + /* These are on whitelist, must be in list */ > + if (strcmp("pcspkr", modname) && strcmp("floppy", modname)) > + goto fail; > + len++; > + kmod_module_unref(mod); > + } > + > + if (len != 2) > + goto fail; > + > + kmod_module_unref_list(filtered); > + kmod_module_unref_list(list); > + kmod_unref(ctx); > + > + return EXIT_SUCCESS; > + > +fail: > + kmod_module_unref_list(list); > +fail_lookup: > + kmod_unref(ctx); > + return EXIT_FAILURE; > +} > +static const struct test swhitelist_1 = { > + .name = "whitelist_1", > + .description = "check if modules are correctly whitelisted", > + .func = whitelist_1, > + .config = { > + [TC_ROOTFS] = TESTSUITE_ROOTFS "test-whitelist/", > + }, > + .need_spawn = true, > +}; > + > +static const struct test *tests[] = { > + &swhitelist_1, > + NULL, > +}; > + > +TESTSUITE_MAIN(tests); > diff --git a/tools/modprobe.c b/tools/modprobe.c > index 58f6df9..81b8442 100644 > --- a/tools/modprobe.c > +++ b/tools/modprobe.c > @@ -638,10 +638,14 @@ static int insmod(struct kmod_ctx *ctx, const char *alias, > extra_options, NULL, NULL, show); > } > > - if (err >= 0) > - /* ignore flag return values such as a mod being blacklisted */ > - err = 0; > - else { > + if (err >= 0) { > + if (err & KMOD_PROBE_APPLY_WHITELIST) > + ERR("could not insert '%s': Module not on whitelist\n", > + kmod_module_get_name(mod)); > + else > + /* ignore flag return values such as a mod being blacklisted */ > + err = 0; > + } else { > switch (err) { > case -EEXIST: > ERR("could not insert '%s': Module already in kernel\n", > -- Lucas De Marchi -- To unsubscribe from this list: send the line "unsubscribe linux-modules" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
