| Message ID | 1570497267-13672-3-git-send-email-nayna@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc: Enabling IMA arch specific secure boot policies | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (6edfc6487b474fe01857dc3f1a9cd701bb9b21c8) |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 0 warnings, 1 checks, 58 lines checked |
On Mon, 2019-10-07 at 21:14 -0400, Nayna Jain wrote: > PowerNV systems uses kernel based bootloader, thus its secure boot > implementation uses kernel IMA security subsystem to verify the kernel > before kexec. ^use a Linux based bootloader, which rely on the IMA subsystem to enforce different secure boot modes. > Since the verification policy might differ based on the > secure boot mode of the system, the policies are defined at runtime. ^the policies need to be defined at runtime. > > This patch implements the arch-specific support to define the IMA policy > rules based on the runtime secure boot mode of the system. > > 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 | 33 +++++++++++++++++++++++++++++++++ > include/linux/ima.h | 3 ++- > 4 files changed, 38 insertions(+), 2 deletions(-) > create mode 100644 arch/powerpc/kernel/ima_arch.c > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index b4a221886fcf..deb19ec6ba3d 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -938,6 +938,8 @@ config PPC_SECURE_BOOT > prompt "Enable secure boot support" > bool > depends on PPC_POWERNV > + depends on IMA > + depends on IMA_ARCH_POLICY As IMA_ARCH_POLICY is dependent on IMA, I don't see a need for depending on both IMA and IMA_ARCH_POLICY. Mimi
Mimi Zohar <zohar@linux.ibm.com> writes: > On Mon, 2019-10-07 at 21:14 -0400, Nayna Jain wrote: ... >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index b4a221886fcf..deb19ec6ba3d 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -938,6 +938,8 @@ config PPC_SECURE_BOOT >> prompt "Enable secure boot support" >> bool >> depends on PPC_POWERNV >> + depends on IMA >> + depends on IMA_ARCH_POLICY > > As IMA_ARCH_POLICY is dependent on IMA, I don't see a need for > depending on both IMA and IMA_ARCH_POLICY. I agree. And what we actually depend on is the arch part, so it should depend on just IMA_ARCH_POLICY. cheers
Nayna Jain <nayna@linux.ibm.com> writes: > PowerNV systems uses kernel based bootloader, thus its secure boot > implementation uses kernel IMA security subsystem to verify the kernel > before kexec. Since the verification policy might differ based on the > secure boot mode of the system, the policies are defined at runtime. > > This patch implements the arch-specific support to define the IMA policy > rules based on the runtime secure boot mode of the system. > > This patch provides arch-specific IMA policies if PPC_SECURE_BOOT > config is enabled. ... > diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c > new file mode 100644 > index 000000000000..c22d82965eb4 > --- /dev/null > +++ b/arch/powerpc/kernel/ima_arch.c > @@ -0,0 +1,33 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2019 IBM Corporation > + * Author: Nayna Jain > + */ > + > +#include <linux/ima.h> > +#include <asm/secure_boot.h> > + > +bool arch_ima_get_secureboot(void) > +{ > + return is_powerpc_os_secureboot_enabled(); > +} > + > +/* Defines IMA appraise rules for secureboot */ > +static const char *const arch_rules[] = { > + "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig", > +#if !IS_ENABLED(CONFIG_MODULE_SIG_FORCE) > + "appraise func=MODULE_CHECK appraise_type=imasig|modsig", > +#endif This confuses me. If I spell it out we get: #if IS_ENABLED(CONFIG_MODULE_SIG_FORCE) // nothing #else "appraise func=MODULE_CHECK appraise_type=imasig|modsig", #endif Which is just: #ifdef CONFIG_MODULE_SIG_FORCE // nothing #else "appraise func=MODULE_CHECK appraise_type=imasig|modsig", #endif But CONFIG_MODULE_SIG_FORCE enabled says that we *do* require modules to have a valid signature. Isn't that the inverse of what the rules say? Presumably I'm misunderstanding something :) cheers
On 10/15/2019 07:29 AM, Michael Ellerman wrote: > Nayna Jain <nayna@linux.ibm.com> writes: >> PowerNV systems uses kernel based bootloader, thus its secure boot >> implementation uses kernel IMA security subsystem to verify the kernel >> before kexec. Since the verification policy might differ based on the >> secure boot mode of the system, the policies are defined at runtime. >> >> This patch implements the arch-specific support to define the IMA policy >> rules based on the runtime secure boot mode of the system. >> >> This patch provides arch-specific IMA policies if PPC_SECURE_BOOT >> config is enabled. > ... >> diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c >> new file mode 100644 >> index 000000000000..c22d82965eb4 >> --- /dev/null >> +++ b/arch/powerpc/kernel/ima_arch.c >> @@ -0,0 +1,33 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2019 IBM Corporation >> + * Author: Nayna Jain >> + */ >> + >> +#include <linux/ima.h> >> +#include <asm/secure_boot.h> >> + >> +bool arch_ima_get_secureboot(void) >> +{ >> + return is_powerpc_os_secureboot_enabled(); >> +} >> + >> +/* Defines IMA appraise rules for secureboot */ >> +static const char *const arch_rules[] = { >> + "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig", >> +#if !IS_ENABLED(CONFIG_MODULE_SIG_FORCE) >> + "appraise func=MODULE_CHECK appraise_type=imasig|modsig", >> +#endif > This confuses me. > > If I spell it out we get: > > #if IS_ENABLED(CONFIG_MODULE_SIG_FORCE) > // nothing > #else > "appraise func=MODULE_CHECK appraise_type=imasig|modsig", > #endif > > Which is just: > > #ifdef CONFIG_MODULE_SIG_FORCE > // nothing > #else > "appraise func=MODULE_CHECK appraise_type=imasig|modsig", > #endif > > But CONFIG_MODULE_SIG_FORCE enabled says that we *do* require modules to > have a valid signature. Isn't that the inverse of what the rules say? > > Presumably I'm misunderstanding something :) To avoid duplicate signature verification as much as possible, the IMA policy rule is added only if CONFIG_MODULE_SIG_FORCE is not enabled. CONFIG_MODULE_SIG_FORCE is part of modules support. IMA signature verification is based on policy. Thanks & Regards, - Nayna
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index b4a221886fcf..deb19ec6ba3d 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -938,6 +938,8 @@ config PPC_SECURE_BOOT prompt "Enable secure boot support" bool depends on PPC_POWERNV + depends on IMA + depends on IMA_ARCH_POLICY help Systems with firmware secure boot enabled needs to define security policies to extend secure boot to the OS. This config allows user diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile index e2a54fa240ac..e8eb2955b7d5 100644 --- a/arch/powerpc/kernel/Makefile +++ b/arch/powerpc/kernel/Makefile @@ -161,7 +161,7 @@ ifneq ($(CONFIG_PPC_POWERNV)$(CONFIG_PPC_SVM),) obj-y += ucall.o endif -obj-$(CONFIG_PPC_SECURE_BOOT) += secure_boot.o +obj-$(CONFIG_PPC_SECURE_BOOT) += secure_boot.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..c22d82965eb4 --- /dev/null +++ b/arch/powerpc/kernel/ima_arch.c @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019 IBM Corporation + * Author: Nayna Jain + */ + +#include <linux/ima.h> +#include <asm/secure_boot.h> + +bool arch_ima_get_secureboot(void) +{ + return is_powerpc_os_secureboot_enabled(); +} + +/* Defines IMA appraise rules for secureboot */ +static const char *const arch_rules[] = { + "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig", +#if !IS_ENABLED(CONFIG_MODULE_SIG_FORCE) + "appraise func=MODULE_CHECK appraise_type=imasig|modsig", +#endif + NULL +}; + +/* + * Returns the relevant IMA arch policies based on the system secureboot state. + */ +const char *const *arch_get_ima_policy(void) +{ + if (is_powerpc_os_secureboot_enabled()) + return arch_rules; + + return NULL; +} diff --git a/include/linux/ima.h b/include/linux/ima.h index 1c37f17f7203..6d904754d858 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -29,7 +29,8 @@ extern void ima_kexec_cmdline(const void *buf, int size); extern void ima_add_kexec_buffer(struct kimage *image); #endif -#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) +#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \ + || defined(CONFIG_PPC_SECURE_BOOT) extern bool arch_ima_get_secureboot(void); extern const char * const *arch_get_ima_policy(void); #else
PowerNV systems uses kernel based bootloader, thus its secure boot implementation uses kernel IMA security subsystem to verify the kernel before kexec. Since the verification policy might differ based on the secure boot mode of the system, the policies are defined at runtime. This patch implements the arch-specific support to define the IMA policy rules based on the runtime secure boot mode of the system. 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 | 33 +++++++++++++++++++++++++++++++++ include/linux/ima.h | 3 ++- 4 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 arch/powerpc/kernel/ima_arch.c