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

[Philipp Rudo]

On Wed, 04 Mar 2020 07:55:38 -0500
Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote:

> [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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx>
> > > Signed-off-by: Nayna Jain <nayna@xxxxxxxxxxxxx>
> > > Cc: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > > Cc: Philipp Rudo <prudo@xxxxxxxxxxxxx>
> > > Cc: Michael Ellerman <mpe@xxxxxxxxxxxxxx>  
> > 
> > Acked-by: Ard Biesheuvel <ardb@xxxxxxxxxx>  
> 
> 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?

I don't think splitting this patch makes sense. Otherwise you would break the
build for all architectures until they picked up their line of code.

I'm fine with the patch as is.

Thanks
Philipp

> >   
> > > ---
> > > 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
> > >  
>