Re: [PATCH v3] x86/ima: require signed kernel modules

From: Jessica Yu <jeyu@kernel.org>
To: Mimi Zohar <zohar@linux.ibm.com>
Cc: linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Luis Chamberlain <mcgrof@kernel.org>,
	David Howells <dhowells@redhat.com>,
	Seth Forshee <seth.forshee@canonical.com>,
	"Bruno E . O . Meneguele" <bmeneg@redhat.com>
Subject: Re: [PATCH v3] x86/ima: require signed kernel modules
Date: Mon, 18 Feb 2019 10:43:14 +0100	[thread overview]
Message-ID: <20190218094313.GA9296@linux-8ccs> (raw)
In-Reply-To: <1550249418-7986-1-git-send-email-zohar@linux.ibm.com>

+++ Mimi Zohar [15/02/19 11:50 -0500]:
>Have the IMA architecture specific policy require signed kernel modules
>on systems with secure boot mode enabled; and coordinate the different
>signature verification methods, so only one signature is required.
>
>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.  A custom IMA policy may still define
>and require an IMA signature.
>
>Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>

Acked-by: Jessica Yu <jeyu@kernel.org>

Thanks!

>---
> arch/x86/kernel/ima_arch.c | 9 ++++++++-
> include/linux/module.h     | 5 +++++
> kernel/module.c            | 5 +++++
> 3 files changed, 18 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..5aaa9359adc8 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... */
>
>@@ -780,6 +781,10 @@ static inline bool is_module_sig_enforced(void)
> 	return false;
> }
>
>+static inline void set_module_sig_enforced(void)
>+{
>+}
>+
> /* Dereference module function descriptor */
> static inline
> void *dereference_module_function_descriptor(struct module *mod, void *ptr)
>diff --git a/kernel/module.c b/kernel/module.c
>index 2ad1b5239910..73cada04bd24 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
>