* [PATCH v2] x86/ima: require signed kernel modules @ 2019-02-13 12:17 Mimi Zohar 2019-02-14 17:58 ` Luis Chamberlain 2019-03-07 22:27 ` Matthew Garrett 0 siblings, 2 replies; 8+ messages in thread From: Mimi Zohar @ 2019-02-13 12:17 UTC (permalink / raw) To: linux-integrity Cc: linux-security-module, linux-kernel, Jessica Yu, Luis Chamberlain, David Howells, Seth Forshee, Bruno E . O . Meneguele, Mimi Zohar Require signed kernel modules on systems with secure boot mode enabled. Requiring appended kernel module signatures may be configured, enabled on the boot command line, or with this patch enabled in secure boot mode. This patch defines set_module_sig_enforced(). To coordinate between appended kernel module signatures and IMA signatures, only define an IMA MODULE_CHECK policy rule if CONFIG_MODULE_SIG is not enabled. Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> --- Changelog: - Removed new "sig_required" flag and associated functions, directly set sig_enforce. arch/x86/kernel/ima_arch.c | 9 ++++++++- include/linux/module.h | 1 + kernel/module.c | 5 +++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c index e47cd9390ab4..3fb9847f1cad 100644 --- a/arch/x86/kernel/ima_arch.c +++ b/arch/x86/kernel/ima_arch.c @@ -64,12 +64,19 @@ static const char * const sb_arch_rules[] = { "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig", #endif /* CONFIG_KEXEC_VERIFY_SIG */ "measure func=KEXEC_KERNEL_CHECK", +#if !IS_ENABLED(CONFIG_MODULE_SIG) + "appraise func=MODULE_CHECK appraise_type=imasig", +#endif + "measure func=MODULE_CHECK", NULL }; const char * const *arch_get_ima_policy(void) { - if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) { + if (IS_ENABLED(CONFIG_MODULE_SIG)) + set_module_sig_enforced(); return sb_arch_rules; + } return NULL; } diff --git a/include/linux/module.h b/include/linux/module.h index 8fa38d3e7538..75e2a5c24a2b 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -660,6 +660,7 @@ static inline bool is_livepatch_module(struct module *mod) #endif /* CONFIG_LIVEPATCH */ bool is_module_sig_enforced(void); +void set_module_sig_enforced(void); #else /* !CONFIG_MODULES... */ diff --git a/kernel/module.c b/kernel/module.c index 2ad1b5239910..4cb5b733fb18 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -286,6 +286,11 @@ bool is_module_sig_enforced(void) } EXPORT_SYMBOL(is_module_sig_enforced); +void set_module_sig_enforced(void) +{ + sig_enforce = true; +} + /* Block module loading/unloading? */ int modules_disabled = 0; core_param(nomodule, modules_disabled, bint, 0); -- 2.7.5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] x86/ima: require signed kernel modules 2019-02-13 12:17 [PATCH v2] x86/ima: require signed kernel modules Mimi Zohar @ 2019-02-14 17:58 ` Luis Chamberlain 2019-02-14 18:47 ` Mimi Zohar 2019-03-07 22:27 ` Matthew Garrett 1 sibling, 1 reply; 8+ messages in thread From: Luis Chamberlain @ 2019-02-14 17:58 UTC (permalink / raw) To: Mimi Zohar Cc: linux-integrity, linux-security-module, linux-kernel, Jessica Yu, David Howells, Seth Forshee, Bruno E . O . Meneguele On Wed, Feb 13, 2019 at 07:17:59AM -0500, Mimi Zohar wrote: > Require signed kernel modules on systems with secure boot mode enabled. > > Requiring appended kernel module signatures may be configured, enabled > on the boot command line, or with this patch enabled in secure boot > mode. But only if IMA is enabled? If so, should this statement be true if IMA is disabled? Either way, this is not clear from the commit log and code, can the commit log be clear if set_module_sig_enforced() will be set if IMA is disabled but secure boot mode enabled? > This patch defines set_module_sig_enforced(). > > To coordinate between appended kernel module signatures and IMA > signatures, only define an IMA MODULE_CHECK policy rule if > CONFIG_MODULE_SIG is not enabled. > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > --- > > Changelog: > - Removed new "sig_required" flag and associated functions, directly set > sig_enforce. > > arch/x86/kernel/ima_arch.c | 9 ++++++++- > include/linux/module.h | 1 + > kernel/module.c | 5 +++++ > 3 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c > index e47cd9390ab4..3fb9847f1cad 100644 > --- a/arch/x86/kernel/ima_arch.c > +++ b/arch/x86/kernel/ima_arch.c > @@ -64,12 +64,19 @@ static const char * const sb_arch_rules[] = { > "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig", > #endif /* CONFIG_KEXEC_VERIFY_SIG */ > "measure func=KEXEC_KERNEL_CHECK", > +#if !IS_ENABLED(CONFIG_MODULE_SIG) > + "appraise func=MODULE_CHECK appraise_type=imasig", > +#endif > + "measure func=MODULE_CHECK", > NULL > }; > > const char * const *arch_get_ima_policy(void) > { > - if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) > + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) { > + if (IS_ENABLED(CONFIG_MODULE_SIG)) > + set_module_sig_enforced(); > return sb_arch_rules; > + } > return NULL; > } > diff --git a/include/linux/module.h b/include/linux/module.h > index 8fa38d3e7538..75e2a5c24a2b 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -660,6 +660,7 @@ static inline bool is_livepatch_module(struct module *mod) > #endif /* CONFIG_LIVEPATCH */ > > bool is_module_sig_enforced(void); > +void set_module_sig_enforced(void); > > #else /* !CONFIG_MODULES... */ I think you need the !CONFIG_MODULES definition of set_module_sig_enforced() then... > diff --git a/kernel/module.c b/kernel/module.c > index 2ad1b5239910..4cb5b733fb18 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -286,6 +286,11 @@ bool is_module_sig_enforced(void) > } > EXPORT_SYMBOL(is_module_sig_enforced); > > +void set_module_sig_enforced(void) > +{ > + sig_enforce = true; > +} The export is not needed as it is bool eh? Luis ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] x86/ima: require signed kernel modules 2019-02-14 17:58 ` Luis Chamberlain @ 2019-02-14 18:47 ` Mimi Zohar 0 siblings, 0 replies; 8+ messages in thread From: Mimi Zohar @ 2019-02-14 18:47 UTC (permalink / raw) To: Luis Chamberlain Cc: linux-integrity, linux-security-module, linux-kernel, Jessica Yu, David Howells, Seth Forshee, Bruno E . O . Meneguele On Thu, 2019-02-14 at 09:58 -0800, Luis Chamberlain wrote: > On Wed, Feb 13, 2019 at 07:17:59AM -0500, Mimi Zohar wrote: > > Require signed kernel modules on systems with secure boot mode enabled. > > > > Requiring appended kernel module signatures may be configured, enabled > > on the boot command line, or with this patch enabled in secure boot > > mode. > > But only if IMA is enabled? The patch subject line indicates this is for IMA, but sure I can amend the patch description, making it clearer. > If so, should this statement be true if > IMA is disabled? This patch coordinates the PE and IMA signatures so that both signature types aren't required. Only if "CONFIG_KEXEC_VERIFY_SIGNATURE" is not enabled, is an IMA policy rule defined. A custom IMA policy can still define an IMA kexec rule, requiring an IMA signature, even if the PE signature is required. For the case when IMA is disabled and PE signatures are required, then there isn't a problem. The issue is when neither signature verification method is enabled. I'll leave that for someone else to address. > > Either way, this is not clear from the commit log and code, can the > commit log be clear if set_module_sig_enforced() will be set if > IMA is disabled but secure boot mode enabled? > > > This patch defines set_module_sig_enforced(). > > > > To coordinate between appended kernel module signatures and IMA > > signatures, only define an IMA MODULE_CHECK policy rule if > > CONFIG_MODULE_SIG is not enabled. > > > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > > --- > > > > Changelog: > > - Removed new "sig_required" flag and associated functions, directly set > > sig_enforce. > > > > arch/x86/kernel/ima_arch.c | 9 ++++++++- > > include/linux/module.h | 1 + > > kernel/module.c | 5 +++++ > > 3 files changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c > > index e47cd9390ab4..3fb9847f1cad 100644 > > --- a/arch/x86/kernel/ima_arch.c > > +++ b/arch/x86/kernel/ima_arch.c > > @@ -64,12 +64,19 @@ static const char * const sb_arch_rules[] = { > > "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig", > > #endif /* CONFIG_KEXEC_VERIFY_SIG */ > > "measure func=KEXEC_KERNEL_CHECK", > > +#if !IS_ENABLED(CONFIG_MODULE_SIG) > > + "appraise func=MODULE_CHECK appraise_type=imasig", > > +#endif > > + "measure func=MODULE_CHECK", > > NULL > > }; > > > > const char * const *arch_get_ima_policy(void) > > { > > - if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) > > + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) { > > + if (IS_ENABLED(CONFIG_MODULE_SIG)) > > + set_module_sig_enforced(); > > return sb_arch_rules; > > + } > > return NULL; > > } > > diff --git a/include/linux/module.h b/include/linux/module.h > > index 8fa38d3e7538..75e2a5c24a2b 100644 > > --- a/include/linux/module.h > > +++ b/include/linux/module.h > > @@ -660,6 +660,7 @@ static inline bool is_livepatch_module(struct module *mod) > > #endif /* CONFIG_LIVEPATCH */ > > > > bool is_module_sig_enforced(void); > > +void set_module_sig_enforced(void); > > > > #else /* !CONFIG_MODULES... */ > > I think you need the !CONFIG_MODULES definition of set_module_sig_enforced() > then... Good catch, thanks. > > > diff --git a/kernel/module.c b/kernel/module.c > > index 2ad1b5239910..4cb5b733fb18 100644 > > --- a/kernel/module.c > > +++ b/kernel/module.c > > @@ -286,6 +286,11 @@ bool is_module_sig_enforced(void) > > } > > EXPORT_SYMBOL(is_module_sig_enforced); > > > > +void set_module_sig_enforced(void) > > +{ > > + sig_enforce = true; > > +} > > The export is not needed as it is bool eh? IMA is builtin, so it doesn't need to be exported. Mimi ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] x86/ima: require signed kernel modules 2019-02-13 12:17 [PATCH v2] x86/ima: require signed kernel modules Mimi Zohar 2019-02-14 17:58 ` Luis Chamberlain @ 2019-03-07 22:27 ` Matthew Garrett 2019-03-07 22:34 ` Mimi Zohar 1 sibling, 1 reply; 8+ messages in thread From: Matthew Garrett @ 2019-03-07 22:27 UTC (permalink / raw) To: Mimi Zohar Cc: linux-integrity, LSM List, Linux Kernel Mailing List, Jessica Yu, Luis Chamberlain, David Howells, Seth Forshee, Bruno E . O . Meneguele On Wed, Feb 13, 2019 at 4:18 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > - if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) > + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) { > + if (IS_ENABLED(CONFIG_MODULE_SIG)) > + set_module_sig_enforced(); > return sb_arch_rules; Linus previously pushed back on having the lockdown features automatically enabled on secure boot systems. Why are we doing the same in IMA? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] x86/ima: require signed kernel modules 2019-03-07 22:27 ` Matthew Garrett @ 2019-03-07 22:34 ` Mimi Zohar 2019-03-07 22:36 ` Matthew Garrett 0 siblings, 1 reply; 8+ messages in thread From: Mimi Zohar @ 2019-03-07 22:34 UTC (permalink / raw) To: Matthew Garrett Cc: linux-integrity, LSM List, Linux Kernel Mailing List, Jessica Yu, Luis Chamberlain, David Howells, Seth Forshee, Bruno E . O . Meneguele On Thu, 2019-03-07 at 14:27 -0800, Matthew Garrett wrote: > On Wed, Feb 13, 2019 at 4:18 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > - if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) > > + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) { > > + if (IS_ENABLED(CONFIG_MODULE_SIG)) > > + set_module_sig_enforced(); > > return sb_arch_rules; > > Linus previously pushed back on having the lockdown features > automatically enabled on secure boot systems. Why are we doing the > same in IMA? IMA-appraisal is extending the "secure boot" concept to the running system. Mimi ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] x86/ima: require signed kernel modules 2019-03-07 22:34 ` Mimi Zohar @ 2019-03-07 22:36 ` Matthew Garrett 2019-03-07 22:41 ` Mimi Zohar 0 siblings, 1 reply; 8+ messages in thread From: Matthew Garrett @ 2019-03-07 22:36 UTC (permalink / raw) To: Mimi Zohar Cc: linux-integrity, LSM List, Linux Kernel Mailing List, Jessica Yu, Luis Chamberlain, David Howells, Seth Forshee, Bruno E . O . Meneguele On Thu, Mar 7, 2019 at 2:34 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > On Thu, 2019-03-07 at 14:27 -0800, Matthew Garrett wrote: > > On Wed, Feb 13, 2019 at 4:18 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > > - if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) > > > + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) { > > > + if (IS_ENABLED(CONFIG_MODULE_SIG)) > > > + set_module_sig_enforced(); > > > return sb_arch_rules; > > > > Linus previously pushed back on having the lockdown features > > automatically enabled on secure boot systems. Why are we doing the > > same in IMA? > > IMA-appraisal is extending the "secure boot" concept to the running > system. Right, but how is this different to what Linus was objecting to? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] x86/ima: require signed kernel modules 2019-03-07 22:36 ` Matthew Garrett @ 2019-03-07 22:41 ` Mimi Zohar 2019-03-07 22:45 ` Matthew Garrett 0 siblings, 1 reply; 8+ messages in thread From: Mimi Zohar @ 2019-03-07 22:41 UTC (permalink / raw) To: Matthew Garrett Cc: linux-integrity, LSM List, Linux Kernel Mailing List, Jessica Yu, Luis Chamberlain, David Howells, Seth Forshee, Bruno E . O . Meneguele On Thu, 2019-03-07 at 14:36 -0800, Matthew Garrett wrote: > On Thu, Mar 7, 2019 at 2:34 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > > > On Thu, 2019-03-07 at 14:27 -0800, Matthew Garrett wrote: > > > On Wed, Feb 13, 2019 at 4:18 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > > > - if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) > > > > + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) { > > > > + if (IS_ENABLED(CONFIG_MODULE_SIG)) > > > > + set_module_sig_enforced(); > > > > return sb_arch_rules; > > > > > > Linus previously pushed back on having the lockdown features > > > automatically enabled on secure boot systems. Why are we doing the > > > same in IMA? > > > > IMA-appraisal is extending the "secure boot" concept to the running > > system. > > Right, but how is this different to what Linus was objecting to? Both Andy Lutomirski and Linus objected to limiting the "lockdown" patch set to secure boot enabled systems. Mimi ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] x86/ima: require signed kernel modules 2019-03-07 22:41 ` Mimi Zohar @ 2019-03-07 22:45 ` Matthew Garrett 0 siblings, 0 replies; 8+ messages in thread From: Matthew Garrett @ 2019-03-07 22:45 UTC (permalink / raw) To: Mimi Zohar Cc: linux-integrity, LSM List, Linux Kernel Mailing List, Jessica Yu, Luis Chamberlain, David Howells, Seth Forshee, Bruno E . O . Meneguele On Thu, Mar 7, 2019 at 2:41 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > On Thu, 2019-03-07 at 14:36 -0800, Matthew Garrett wrote: > > Right, but how is this different to what Linus was objecting to? > > Both Andy Lutomirski and Linus objected to limiting the "lockdown" > patch set to secure boot enabled systems. No, Linus objected to it being automatically enabled when secure boot was enabled. It was always possible to enable it at boot on any platform. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-03-07 22:46 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-02-13 12:17 [PATCH v2] x86/ima: require signed kernel modules Mimi Zohar 2019-02-14 17:58 ` Luis Chamberlain 2019-02-14 18:47 ` Mimi Zohar 2019-03-07 22:27 ` Matthew Garrett 2019-03-07 22:34 ` Mimi Zohar 2019-03-07 22:36 ` Matthew Garrett 2019-03-07 22:41 ` Mimi Zohar 2019-03-07 22:45 ` Matthew Garrett
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).