lkml.org 
[lkml]   [2012]   [Sep]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [RFC] module: signature infrastructure
    From
    On Tue, Sep 4, 2012 at 5:25 PM, Lucas De Marchi
    <lucas.de.marchi@gmail.com> wrote:
    > Hi Rusty,
    >
    > On Tue, Sep 4, 2012 at 2:55 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
    >> OK, I took a look at the module.c parts of David and Dmitry's patchsets,
    >> and didn't really like either, but I stole parts of David's to make
    >> this.
    >>
    >> So, here's the module.c part of module signing. I hope you two got time
    >> to discuss the signature format details? Mimi suggested a scheme where
    >> the private key would never be saved on disk (even temporarily), but I
    >> didn't see patches. Frankly it's something we can do later; let's aim
    >> at getting the format right for the next merge window.
    >>
    >> Cheers,
    >> Rusty.
    >>
    >> ---
    >> This patch doesn't compile: we need to implement:
    >>
    >> int mod_verify_sig(const void *mod, unsigned long modlen,
    >> const void *sig, unsigned long siglen,
    >> bool *sig_ok);
    >>
    >> Also, we need to actually append the signatures during build.
    >>
    >> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
    >> index ad7e2e5..9b2b8d3 100644
    >> --- a/Documentation/kernel-parameters.txt
    >> +++ b/Documentation/kernel-parameters.txt
    >> @@ -1582,6 +1582,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
    >> log everything. Information is printed at KERN_DEBUG
    >> so loglevel=8 may also need to be specified.
    >>
    >> + module.sig_enforce
    >> + [KNL] When CONFIG_MODULE_SIG is set, this means that
    >> + modules without (valid) signatures will fail to load.
    >> + Note that if CONFIG_MODULE_SIG_ENFORCE is set, that
    >> + is always true, so this option does nothing.
    >> +
    >> mousedev.tap_time=
    >> [MOUSE] Maximum time between finger touching and
    >> leaving touchpad surface for touch to be considered
    >> diff --git a/include/linux/module.h b/include/linux/module.h
    >> index fbcafe2..7760c6d 100644
    >> --- a/include/linux/module.h
    >> +++ b/include/linux/module.h
    >> @@ -21,6 +21,9 @@
    >> #include <linux/percpu.h>
    >> #include <asm/module.h>
    >>
    >> +/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
    >> +#define MODULE_SIG_STRING "~Module signature appended~\n"
    >> +
    >> /* Not Yet Implemented */
    >> #define MODULE_SUPPORTED_DEVICE(name)
    >>
    >> @@ -260,6 +263,11 @@ struct module
    >> const unsigned long *unused_gpl_crcs;
    >> #endif
    >>
    >> +#ifdef CONFIG_MODULE_SIG
    >> + /* Signature was verified. */
    >> + bool sig_ok;
    >> +#endif
    >> +
    >> /* symbols that will be GPL-only in the near future. */
    >> const struct kernel_symbol *gpl_future_syms;
    >> const unsigned long *gpl_future_crcs;
    >> diff --git a/init/Kconfig b/init/Kconfig
    >> index af6c7f8..7452e19 100644
    >> --- a/init/Kconfig
    >> +++ b/init/Kconfig
    >> @@ -1585,6 +1585,20 @@ config MODULE_SRCVERSION_ALL
    >> the version). With this option, such a "srcversion" field
    >> will be created for all modules. If unsure, say N.
    >>
    >> +config MODULE_SIG
    >> + bool "Module signature verification"
    >> + depends on MODULES
    >> + help
    >> + Check modules for valid signatures upon load: the signature
    >> + is simply appended to the module. For more information see
    >> + Documentation/module-signing.txt.
    >> +
    >> +config MODULE_SIG_FORCE
    >> + bool "Require modules to be validly signed"
    >> + depends on MODULE_SIG
    >> + help
    >> + Reject unsigned modules or signed modules for which we don't have a
    >> + key. Without this, such modules will simply taint the kernel.
    >> endif # MODULES
    >>
    >> config INIT_ALL_POSSIBLE
    >> diff --git a/kernel/module.c b/kernel/module.c
    >> index 4edbd9c..3cbd1a4 100644
    >> --- a/kernel/module.c
    >> +++ b/kernel/module.c
    >> @@ -102,6 +102,43 @@ static LIST_HEAD(modules);
    >> struct list_head *kdb_modules = &modules; /* kdb needs the list of modules */
    >> #endif /* CONFIG_KGDB_KDB */
    >>
    >> +#ifdef CONFIG_MODULE_SIG
    >> +#ifdef CONFIG_MODULE_SIG_ENFORCE
    >> +static bool sig_enforce = true;
    >> +#else
    >> +static bool sig_enforce = false;
    >> +
    >> +static int param_set_bool_enable_only(const char *val,
    >> + const struct kernel_param *kp)
    >> +{
    >> + int err;
    >> + bool test;
    >> + struct kernel_param dummy_kp = *kp;
    >> +
    >> + dummy_kp.arg = &test;
    >> +
    >> + err = param_set_bool(val, &dummy_kp);
    >> + if (err)
    >> + return err;
    >> +
    >> + /* Don't let them unset it once it's set! */
    >> + if (!test && sig_enforce)
    >> + return -EROFS;
    >> +
    >> + if (test)
    >> + sig_enforce = true;
    >> + return 0;
    >> +}
    >> +
    >> +static const struct kernel_param_ops param_ops_bool_enable_only = {
    >> + .set = param_set_bool_enable_only,
    >> + .get = param_get_bool,
    >> +};
    >> +#define param_check_bool_enable_only param_check_bool
    >> +
    >> +module_param(sig_enforce, bool_enable_only, 0644);
    >> +#endif /* !CONFIG_MODULE_SIG_ENFORCE */
    >> +#endif /* CONFIG_MODULE_SIG */
    >>
    >> /* Block module loading/unloading? */
    >> int modules_disabled = 0;
    >> @@ -136,6 +173,7 @@ struct load_info {
    >> unsigned long symoffs, stroffs;
    >> struct _ddebug *debug;
    >> unsigned int num_debug;
    >> + bool sig_ok;
    >> struct {
    >> unsigned int sym, str, mod, vers, info, pcpu;
    >> } index;
    >> @@ -2399,7 +2437,50 @@ static inline void kmemleak_load_module(const struct module *mod,
    >> }
    >> #endif
    >>
    >> -/* Sets info->hdr and info->len. */
    >> +#ifdef CONFIG_MODULE_SIG
    >> +static int module_sig_check(struct load_info *info,
    >> + void *mod, unsigned long *len)
    >> +{
    >> + int err;
    >> + unsigned long i, siglen;
    >> + char *sig = NULL;
    >> +
    >> + /* This is not a valid module: ELF header is larger anyway. */
    >> + if (*len < sizeof(MODULE_SIG_STRING))
    >> + return -ENOEXEC;
    >> +
    >> + for (i = 0; i < *len - (sizeof(MODULE_SIG_STRING)-1); i++) {
    >> + /* Our memcmp is dumb, speed it up a little. */
    >> + if (((char *)mod)[i] != MODULE_SIG_STRING[0])
    >> + continue;
    >
    > Since the signature is appended to the module, why don't you go
    > backwards, starting from *len - strlen(sizeof(MODULE_SIG_STRING)) and
    > making this first comparison?
    >
    > Or let the magic string as the last thing in the module and store the
    > signature length, too. In this case no scanning is needed
    >
    >
    > Lucas De Marchi
    >

    This is exactly like that here...

    http://git.kernel.org/?p=linux/kernel/git/kasatkin/linux-digsig.git;a=blob;f=security/integrity/module.c;h=b111bb400a248ab9b03a64ea373c88396f311649;hb=19eef6e4e529ccf2b3a6ab5c19dd40f2eaf8fcaf

    - Dmitry

    >
    >> + if (memcmp(mod, MODULE_SIG_STRING, strlen(MODULE_SIG_STRING)))
    >> + continue;
    >> +
    >> + sig = mod + i + strlen(MODULE_SIG_STRING);
    >> + siglen = *len - i - strlen(MODULE_SIG_STRING);
    >> + *len = i;
    >> + break;
    >> + }
    >> +
    >> + if (!sig)
    >> + err = 0;
    >> + else
    >> + err = mod_verify_sig(mod, len, sig, siglen, &info->sig_ok);
    >> +
    >> + /* Not having a signature is only an error if we're strict. */
    >> + if (!err && !info->sig_ok && sig_enforce)
    >> + err = -EKEYREJECTED;
    >> + return err;
    >> +}
    >> +#else /* !CONFIG_MODULE_SIG */
    >> +static int module_sig_check(struct load_info *info,
    >> + void *mod, unsigned long *len)
    >> +{
    >> + return 0;
    >> +}
    >> +#endif /* !CONFIG_MODULE_SIG */
    >> +
    >> +/* Sets info->hdr, info->len and info->sig_ok. */
    >> static int copy_and_check(struct load_info *info,
    >> const void __user *umod, unsigned long len,
    >> const char __user *uargs)
    >> @@ -2419,6 +2500,10 @@ static int copy_and_check(struct load_info *info,
    >> goto free_hdr;
    >> }
    >>
    >> + err = module_sig_check(info, hdr, &len);
    >> + if (err)
    >> + goto free_hdr;
    >> +
    >> /* Sanity checks against insmoding binaries or wrong arch,
    >> weird elf version */
    >> if (memcmp(hdr->e_ident, ELFMAG, SELFMAG) != 0
    >> @@ -2886,6 +2971,12 @@ static struct module *load_module(void __user *umod,
    >> goto free_copy;
    >> }
    >>
    >> +#ifdef CONFIG_MODULE_SIG
    >> + mod->sig_ok = info.sig_ok;
    >> + if (!mod->sig_ok)
    >> + add_taint_module(mod, TAINT_FORCED_MODULE);
    >> +#endif
    >> +
    >> /* Now module is in final location, initialize linked lists, etc. */
    >> err = module_unload_init(mod);
    >> if (err)
    >> --
    >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    >> the body of a message to majordomo@vger.kernel.org
    >> More majordomo info at http://vger.kernel.org/majordomo-info.html
    >> Please read the FAQ at http://www.tux.org/lkml/
    >
    >
    >
    > --
    >
    > Lucas De Marchi


    \
     
     \ /
      Last update: 2012-09-04 17:43    [W:2.834 / U:0.656 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site