Re: [PATCH v2] ima: add a new CONFIG for loading arch-specific policies

[Mimi Zohar <zohar@linux.ibm.com>]

[Cc'ing Thomas Gleixner and x86 mailing list]

On Wed, 2020-03-04 at 08:14 +0100, Ard Biesheuvel wrote:
> On Wed, 4 Mar 2020 at 03:34, Nayna Jain <nayna@linux.ibm.com> wrote:
> >
> > Every time a new architecture defines the IMA architecture specific
> > functions - arch_ima_get_secureboot() and arch_ima_get_policy(), the IMA
> > include file needs to be updated. To avoid this "noise", this patch
> > defines a new IMA Kconfig IMA_SECURE_AND_OR_TRUSTED_BOOT option, allowing
> > the different architectures to select it.
> >
> > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> > Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: Philipp Rudo <prudo@linux.ibm.com>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> 
> Acked-by: Ard Biesheuvel <ardb@kernel.org>

Thanks, Ard.
> 
> for the x86 bits, but I'm not an x86 maintainer. Also, you may need to
> split this if you want to permit arch maintainers to pick up their
> parts individually.

Michael, Philipp, Thomas, do you prefer separate patches?

> 
> > ---
> > v2:
> > * Fixed the issue identified by Mimi. Thanks Mimi, Ard, Heiko and Michael for
> > discussing the fix.
> >
> >  arch/powerpc/Kconfig           | 1 +
> >  arch/s390/Kconfig              | 1 +
> >  arch/x86/Kconfig               | 1 +
> >  include/linux/ima.h            | 3 +--
> >  security/integrity/ima/Kconfig | 9 +++++++++
> >  5 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 497b7d0b2d7e..a5cfde432983 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -979,6 +979,7 @@ config PPC_SECURE_BOOT
> >         bool
> >         depends on PPC_POWERNV
> >         depends on IMA_ARCH_POLICY
> > +       select IMA_SECURE_AND_OR_TRUSTED_BOOT
> >         help
> >           Systems with firmware secure boot enabled need to define security
> >           policies to extend secure boot to the OS. This config allows a user
> > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> > index 8abe77536d9d..4a502fbcb800 100644
> > --- a/arch/s390/Kconfig
> > +++ b/arch/s390/Kconfig
> > @@ -195,6 +195,7 @@ config S390
> >         select ARCH_HAS_FORCE_DMA_UNENCRYPTED
> >         select SWIOTLB
> >         select GENERIC_ALLOCATOR
> > +       select IMA_SECURE_AND_OR_TRUSTED_BOOT if IMA_ARCH_POLICY
> >
> >
> >  config SCHED_OMIT_FRAME_POINTER
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index beea77046f9b..7f5bfaf0cbd2 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -230,6 +230,7 @@ config X86
> >         select VIRT_TO_BUS
> >         select X86_FEATURE_NAMES                if PROC_FS
> >         select PROC_PID_ARCH_STATUS             if PROC_FS
> > +       select IMA_SECURE_AND_OR_TRUSTED_BOOT   if EFI && IMA_ARCH_POLICY
> >
> >  config INSTRUCTION_DECODER
> >         def_bool y
> > diff --git a/include/linux/ima.h b/include/linux/ima.h
> > index 1659217e9b60..aefe758f4466 100644
> > --- a/include/linux/ima.h
> > +++ b/include/linux/ima.h
> > @@ -30,8 +30,7 @@ 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) \
> > -       || defined(CONFIG_PPC_SECURE_BOOT)
> > +#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
> >  extern bool arch_ima_get_secureboot(void);
> >  extern const char * const *arch_get_ima_policy(void);
> >  #else
> > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > index 3f3ee4e2eb0d..d17972aa413a 100644
> > --- a/security/integrity/ima/Kconfig
> > +++ b/security/integrity/ima/Kconfig
> > @@ -327,3 +327,12 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
> >         depends on IMA_MEASURE_ASYMMETRIC_KEYS
> >         depends on SYSTEM_TRUSTED_KEYRING
> >         default y
> > +
> > +config IMA_SECURE_AND_OR_TRUSTED_BOOT
> > +       bool
> > +       depends on IMA
> > +       depends on IMA_ARCH_POLICY
> 
> Doesn't the latter already depend on the former?

Yes, there's no need for the first.

Mimi
> 
> > +       default n
> > +       help
> > +          This option is selected by architectures to enable secure and/or
> > +          trusted boot based on IMA runtime policies.
> > --
> > 2.13.6
> >