Messages in this thread | | | Subject | Re: [PATCH v5 2/2] powerpc: Add support to initialize ima policy rules | From | Nayna <> | Date | Thu, 5 Sep 2019 08:31:45 -0400 |
| |
On 09/02/2019 07:52 AM, Michael Ellerman wrote: > Hi Nayna,
Hi Michael,
> > Some more comments below. > > Nayna Jain <nayna@linux.ibm.com> writes: >> POWER secure boot relies on the kernel IMA security subsystem to >> perform the OS kernel image signature verification. > Again this is just a design choice we've made, it's not specified > anywhere or anything like that. And it only applies to bare metal secure > boot, at least so far. AIUI.
Yes. I will make it consistent to use "PowerNV".
>> Since each secure >> boot mode has different IMA policy requirements, dynamic definition of >> the policy rules based on the runtime secure boot mode of the system is >> required. On systems that support secure boot, but have it disabled, >> only measurement policy rules of the kernel image and modules are >> defined. > It's probably worth mentioning that we intend to use this in our > Linux-based boot loader, which uses kexec, and that's one of the reasons > why we're particularly interested in defining the rules for kexec?
Yes. Agreed. I will update patch description to add this.
> >> This patch defines the arch-specific implementation to retrieve the >> secure boot mode of the system and accordingly configures the IMA policy >> rules. >> >> This patch provides arch-specific IMA policies if PPC_SECURE_BOOT >> config is enabled. >> >> Signed-off-by: Nayna Jain <nayna@linux.ibm.com> >> --- >> arch/powerpc/Kconfig | 2 ++ >> arch/powerpc/kernel/Makefile | 2 +- >> arch/powerpc/kernel/ima_arch.c | 50 ++++++++++++++++++++++++++++++++++ >> include/linux/ima.h | 3 +- >> 4 files changed, 55 insertions(+), 2 deletions(-) >> create mode 100644 arch/powerpc/kernel/ima_arch.c >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index c902a39124dc..42109682b727 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -917,6 +917,8 @@ config PPC_SECURE_BOOT >> bool >> default n >> depends on PPC64 >> + depends on IMA >> + depends on IMA_ARCH_POLICY >> help >> Linux on POWER with firmware secure boot enabled needs to define >> security policies to extend secure boot to the OS.This config >> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile >> index d310ebb4e526..520b1c814197 100644 >> --- a/arch/powerpc/kernel/Makefile >> +++ b/arch/powerpc/kernel/Makefile >> @@ -157,7 +157,7 @@ endif >> obj-$(CONFIG_EPAPR_PARAVIRT) += epapr_paravirt.o epapr_hcalls.o >> obj-$(CONFIG_KVM_GUEST) += kvm.o kvm_emul.o >> >> -obj-$(CONFIG_PPC_SECURE_BOOT) += secboot.o >> +obj-$(CONFIG_PPC_SECURE_BOOT) += secboot.o ima_arch.o >> >> # Disable GCOV, KCOV & sanitizers in odd or sensitive code >> GCOV_PROFILE_prom_init.o := n >> diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c >> new file mode 100644 >> index 000000000000..ac90fac83338 >> --- /dev/null >> +++ b/arch/powerpc/kernel/ima_arch.c >> @@ -0,0 +1,50 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2019 IBM Corporation >> + * Author: Nayna Jain <nayna@linux.ibm.com> >> + * >> + * ima_arch.c >> + * - initialize ima policies for PowerPC Secure Boot >> + */ >> + >> +#include <linux/ima.h> >> +#include <asm/secboot.h> >> + >> +bool arch_ima_get_secureboot(void) >> +{ >> + return get_powerpc_secureboot(); >> +} >> + >> +/* >> + * File signature verification is not needed, include only measurements >> + */ >> +static const char *const default_arch_rules[] = { >> + "measure func=KEXEC_KERNEL_CHECK", >> + "measure func=MODULE_CHECK", >> + NULL >> +}; > The rules above seem fairly self explanatory. > >> + >> +/* Both file signature verification and measurements are needed */ >> +static const char *const sb_arch_rules[] = { >> + "measure func=KEXEC_KERNEL_CHECK template=ima-modsig", >> + "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig", >> +#if IS_ENABLED(CONFIG_MODULE_SIG) >> + "measure func=MODULE_CHECK", >> +#else >> + "measure func=MODULE_CHECK template=ima-modsig", >> + "appraise func=MODULE_CHECK appraise_type=imasig|modsig", >> +#endif > But these ones are not so obvious, at least to me who knows very little > about IMA. > > Can you add a one line comment to each of the ones in here saying what > it does and why we want it?
Sure.
> >> + NULL >> +}; >> + >> +/* >> + * On PowerPC, file measurements are to be added to the IMA measurement list >> + * irrespective of the secure boot state of the system. > Why? Just because we think it's useful? Would be good to provide some > further justification.
Sure. I will clarify this in the next version.
Thanks & Regards, - Nayna
| |