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

[Jessica Yu]

+++ 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@xxxxxxxxxxxxx>

Acked-by: Jessica Yu <jeyu@xxxxxxxxxx>

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