From: "Kasatkin, Dmitry" <dmitry.kasatkin@intel.com>
To: Lucas De Marchi <lucas.de.marchi@gmail.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>,
David Howells <dhowells@redhat.com>,
zohar@linux.vnet.ibm.com, jmorris@namei.org,
keyrings@linux-nfs.org, linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [RFC] module: signature infrastructure
Date: Tue, 4 Sep 2012 18:04:12 +0300 [thread overview]
Message-ID: <CALLzPKYM33W-6hL-eEuHn886MCovVGanQ6x_F8UTHrgc_WaJpg@mail.gmail.com> (raw)
In-Reply-To: <CAKi4VALezG7VccXJfwvzr2UZrmAjCUp9eZRjGwqq6cWcExNqXA@mail.gmail.com>
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
next prev parent reply other threads:[~2012-09-04 15:04 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-16 1:34 [PATCH 00/25] Crypto keys and module signing David Howells
2012-08-16 1:34 ` [PATCH 01/25] KEYS: Add payload preparsing opportunity prior to key instantiate or update David Howells
2012-08-16 1:34 ` [PATCH 02/25] MPILIB: Provide count_leading/trailing_zeros() based on arch functions David Howells
2012-09-10 7:13 ` Kasatkin, Dmitry
2012-09-13 5:14 ` James Morris
2012-09-13 14:09 ` Kasatkin, Dmitry
2012-08-16 1:34 ` [PATCH 03/25] KEYS: Create a key type that can be used for general cryptographic operations David Howells
2012-08-16 1:34 ` [PATCH 04/25] KEYS: Add signature verification facility David Howells
2012-08-16 1:35 ` [PATCH 05/25] KEYS: Asymmetric public-key algorithm crypto key subtype David Howells
2012-08-16 1:35 ` [PATCH 06/25] MPILIB: Reinstate mpi_cmp[_ui]() and export for RSA signature verification David Howells
2012-08-16 1:35 ` [PATCH 07/25] KEYS: RSA: Implement signature verification algorithm [PKCS#1 / RFC3447] David Howells
2012-08-16 1:35 ` [PATCH 08/25] KEYS: RSA: Fix signature verification for shorter signatures David Howells
2012-08-16 1:35 ` [PATCH 09/25] PGPLIB: PGP definitions (RFC 4880) David Howells
2012-08-16 1:36 ` [PATCH 10/25] PGPLIB: Basic packet parser David Howells
2012-08-16 1:36 ` [PATCH 11/25] PGPLIB: Signature parser David Howells
2012-08-16 1:36 ` [PATCH 12/25] KEYS: PGP data parser David Howells
2012-08-16 1:36 ` [PATCH 13/25] KEYS: PGP-based public key signature verification David Howells
2012-08-16 1:36 ` [PATCH 14/25] KEYS: PGP format signature parser David Howells
2012-08-16 1:36 ` [PATCH 15/25] KEYS: Provide PGP key description autogeneration David Howells
2012-08-16 1:37 ` [PATCH 16/25] KEYS: Provide a function to load keys from a PGP keyring blob David Howells
2012-08-16 1:37 ` [PATCH 17/25] MODSIGN: Provide gitignore and make clean rules for extra files David Howells
2012-08-16 1:37 ` [PATCH 18/25] MODSIGN: Provide Documentation and Kconfig options David Howells
2012-08-16 1:37 ` [PATCH 19/25] MODSIGN: Sign modules during the build process David Howells
2012-08-16 1:37 ` [PATCH 20/25] MODSIGN: Provide module signing public keys to the kernel David Howells
2012-08-31 14:33 ` Michal Marek
2012-08-16 1:38 ` [PATCH 21/25] MODSIGN: Module signature verification David Howells
2012-08-16 1:38 ` [PATCH 22/25] MODSIGN: Automatically generate module signing keys if missing David Howells
2012-08-16 1:38 ` [PATCH 23/25] MODSIGN: Panic the kernel if FIPS is enabled upon module signing failure David Howells
2012-08-16 1:38 ` [PATCH 24/25] MODSIGN: Allow modules to be signed with an unknown key unless enforcing David Howells
2012-08-16 1:38 ` [PATCH 25/25] MODSIGN: Fix documentation of signed-nokey behavior when not enforcing David Howells
2012-08-21 5:04 ` [PATCH 00/25] Crypto keys and module signing Rusty Russell
2012-08-22 10:50 ` David Howells
2012-08-22 11:52 ` Mimi Zohar
2012-08-22 16:07 ` Kasatkin, Dmitry
2012-09-04 5:55 ` [RFC] module: signature infrastructure Rusty Russell
2012-09-04 12:07 ` Kasatkin, Dmitry
2012-09-04 12:21 ` Kasatkin, Dmitry
2012-09-04 13:40 ` Mimi Zohar
2012-09-05 0:29 ` Rusty Russell
2012-09-05 13:34 ` Mimi Zohar
2012-09-06 2:05 ` Rusty Russell
2012-09-04 14:25 ` Lucas De Marchi
2012-09-04 15:04 ` Kasatkin, Dmitry [this message]
2012-09-05 0:19 ` Rusty Russell
2012-09-05 23:41 ` Lucas De Marchi
2012-09-06 7:55 ` Rusty Russell
2012-09-04 22:51 ` David Howells
2012-09-04 23:17 ` Kasatkin, Dmitry
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CALLzPKYM33W-6hL-eEuHn886MCovVGanQ6x_F8UTHrgc_WaJpg@mail.gmail.com \
--to=dmitry.kasatkin@intel.com \
--cc=dhowells@redhat.com \
--cc=jmorris@namei.org \
--cc=keyrings@linux-nfs.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=lucas.de.marchi@gmail.com \
--cc=rusty@rustcorp.com.au \
--cc=zohar@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).