On Tue, Nov 6, 2012 at 10:03 AM, Milan Broz <[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
