Hi Eric, Nayna,
On Wed, 2018-09-26 at 17:52 +0530, Nayna Jain wrote:
> From: Eric Richter <erichte@xxxxxxxxxxxxxxxxxx>
> This patch implements an example arch-specific IMA policy for x86 to
> enable measurement and appraisal of any kernel image loaded for kexec,
> when CONFIG_KEXEC_VERIFY_SIG is not enabled.
>
> For systems with CONFIG_KEXEC_VERIFY_SIG enabled, only the measurement
> rule is enabled, not the IMA-appraisal rule.
The patch itself looks good, but this patch description explains
"what" the patch is doing, not "why". Missing is the motivation for
the patch.
Mimi
>
> Signed-off-by: Eric Richter <erichte@xxxxxxxxxxxxxxxxxx>
> - Removed the policy KEXEC_ORIG_KERNEL_CHECK which was defined to
> disable the kexec_load syscall.
> - arch_get_ima_policy() uses arch_ima_get_secureboot() to get secureboot
> state
> Signed-off-by: Nayna Jain <nayna@xxxxxxxxxxxxxxxxxx>
> ---
> arch/x86/kernel/ima_arch.c | 18 ++++++++++++++++++
> include/linux/ima.h | 4 ++++
> security/integrity/ima/Kconfig | 8 ++++++++
> 3 files changed, 30 insertions(+)
>
> diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c
> index bb5a88d2b271..245976e49a55 100644
> --- a/arch/x86/kernel/ima_arch.c
> +++ b/arch/x86/kernel/ima_arch.c
> @@ -15,3 +15,21 @@ bool arch_ima_get_secureboot(void)
> else
> return false;
> }
> +
> +/* arch rules for audit and user mode */
> +static const char * const sb_arch_rules[] = {
> +#ifndef CONFIG_KEXEC_VERIFY_SIG
> + "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig",
> +#endif /* CONFIG_KEXEC_VERIFY_SIG */
> + "measure func=KEXEC_KERNEL_CHECK",
> + NULL
> +};
> +
> +#ifdef CONFIG_IMA_ARCH_POLICY
> +const char * const *arch_get_ima_policy(void)
> +{
> + if (arch_ima_get_secureboot())
> + return sb_arch_rules;
> + return NULL;
> +}
> +#endif
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 350fa957f8a6..dabd3abdf671 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -39,10 +39,14 @@ static inline bool arch_ima_get_secureboot(void)
> }
> #endif
>
> +#if defined(CONFIG_X86) && defined(CONFIG_IMA_ARCH_POLICY)
> +extern const char * const *arch_get_ima_policy(void);
> +#else
> static inline const char * const *arch_get_ima_policy(void)
> {
> return NULL;
> }
> +#endif
>
> #else
> static inline int ima_bprm_check(struct linux_binprm *bprm)
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 13b446328dda..97609a76aa14 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -157,6 +157,14 @@ config IMA_APPRAISE
> <http://linux-ima.sourceforge.net>
> If unsure, say N.
>
> +config IMA_ARCH_POLICY
> + bool "Enable loading an IMA architecture specific policy"
> + depends on KEXEC_VERIFY_SIG || IMA_APPRAISE && INTEGRITY_ASYMMETRIC_KEYS
> + default n
> + help
> + This option enables loading an IMA architecture specific policy
> + based on run time secure boot flags.
> +
> config IMA_APPRAISE_BUILD_POLICY
> bool "IMA build time configured policy rules"
> depends on IMA_APPRAISE && INTEGRITY_ASYMMETRIC_KEYS
