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

2020-03-02 Thread Michael Ellerman
Mimi Zohar  writes:
> On Mon, 2020-03-02 at 15:52 +0100, Ard Biesheuvel wrote:
>> On Mon, 2 Mar 2020 at 15:48, Mimi Zohar  wrote:
>> >
>> > On Wed, 2020-02-26 at 14:10 -0500, Nayna Jain 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 
>> > > Signed-off-by: Nayna Jain 
>> > > Cc: Ard Biesheuvel 
>> > > Cc: Martin Schwidefsky 
>> > > Cc: Philipp Rudo 
>> > > Cc: Michael Ellerman 
>> > > ---
>> > >  arch/powerpc/Kconfig   | 2 +-
>> > >  arch/s390/Kconfig  | 1 +
>> > >  arch/x86/Kconfig   | 1 +
>> > >  include/linux/ima.h| 3 +--
>> > >  security/integrity/ima/Kconfig | 9 +
>> > >  5 files changed, 13 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> > > index 497b7d0b2d7e..b8ce1b995633 100644
>> > > --- a/arch/powerpc/Kconfig
>> > > +++ b/arch/powerpc/Kconfig
>> > > @@ -246,6 +246,7 @@ config PPC
>> > >   select SYSCTL_EXCEPTION_TRACE
>> > >   select THREAD_INFO_IN_TASK
>> > >   select VIRT_TO_BUS  if !PPC64
>> > > + select IMA_SECURE_AND_OR_TRUSTED_BOOT   if PPC_SECURE_BOOT
>> > >   #
>> > >   # Please keep this list sorted alphabetically.
>> > >   #
>> > > @@ -978,7 +979,6 @@ config PPC_SECURE_BOOT
>> > >   prompt "Enable secure boot support"
>> > >   bool
>> > >   depends on PPC_POWERNV
>> > > - depends on IMA_ARCH_POLICY
>> > >   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..90ff3633ade6 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
>> > >
>> > >
>> > >  config SCHED_OMIT_FRAME_POINTER
>> > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> > > index beea77046f9b..cafa66313fe2 100644
>> > > --- a/arch/x86/Kconfig
>> > > +++ b/arch/x86/Kconfig
>> > > @@ -230,6 +230,7 @@ config X86
>> > >   select VIRT_TO_BUS
>> > >   select X86_FEATURE_NAMESif PROC_FS
>> > >   select PROC_PID_ARCH_STATUS if PROC_FS
>> > > + select IMA_SECURE_AND_OR_TRUSTED_BOOT   if EFI
>> >
>> > Not everyone is interested in enabling IMA or requiring IMA runtime
>> > policies.  With this patch, enabling IMA_ARCH_POLICY is therefore
>> > still left up to the person building the kernel.  As a result, I'm
>> > seeing the following warning, which is kind of cool.
>> >
>> > WARNING: unmet direct dependencies detected for
>> > IMA_SECURE_AND_OR_TRUSTED_BOOT
>> >   Depends on [n]: INTEGRITY [=y] && IMA [=y] && IMA_ARCH_POLICY [=n]
>> >   Selected by [y]:
>> >   - X86 [=y] && EFI [=y]
>> >
>> > Ard, Michael, Martin, just making sure this type of warning is
>> > acceptable before upstreaming this patch.  I would appreciate your
>> > tags.
>> >
>> 
>> Ehm, no, warnings like these are not really acceptable. It means there
>> is an inconsistency in the way the Kconfig dependencies are defined.
>> 
>> Does this help:
>> 
>>   select IMA_SECURE_AND_OR_TRUSTED_BOOT   if EFI && IMA_ARCH_POLICY
>> 
>> ?
>
> Yes, that's fine for x86.  Michael, Martin, do you want something
> similar or would you prefer actually selecting IMA_ARCH_POLICY?

For powerpc this should be all we need:

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 497b7d0b2d7e..a5cfde432983 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -976,12 +976,13 @@ config PPC_MEM_KEYS
 
 config PPC_SECURE_BOOT
prompt "Enable secure boot support"
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
  to enable OS secure boot on systems that have firmware support for
  it. If in doubt say N.
 

cheers


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

2020-03-02 Thread Heiko Carstens
On Mon, Mar 02, 2020 at 09:56:58AM -0500, Mimi Zohar wrote:
> On Mon, 2020-03-02 at 15:52 +0100, Ard Biesheuvel wrote:
> > On Mon, 2 Mar 2020 at 15:48, Mimi Zohar  wrote:
> > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > > index beea77046f9b..cafa66313fe2 100644
> > > > --- a/arch/x86/Kconfig
> > > > +++ b/arch/x86/Kconfig
> > > > @@ -230,6 +230,7 @@ config X86
> > > >   select VIRT_TO_BUS
> > > >   select X86_FEATURE_NAMESif PROC_FS
> > > >   select PROC_PID_ARCH_STATUS if PROC_FS
> > > > + select IMA_SECURE_AND_OR_TRUSTED_BOOT   if EFI
> > >
> > > Not everyone is interested in enabling IMA or requiring IMA runtime
> > > policies.  With this patch, enabling IMA_ARCH_POLICY is therefore
> > > still left up to the person building the kernel.  As a result, I'm
> > > seeing the following warning, which is kind of cool.
> > >
> > > WARNING: unmet direct dependencies detected for
> > > IMA_SECURE_AND_OR_TRUSTED_BOOT
> > >   Depends on [n]: INTEGRITY [=y] && IMA [=y] && IMA_ARCH_POLICY [=n]
> > >   Selected by [y]:
> > >   - X86 [=y] && EFI [=y]
> > >
> > > Ard, Michael, Martin, just making sure this type of warning is
> > > acceptable before upstreaming this patch.  I would appreciate your
> > > tags.
> > >
> > 
> > Ehm, no, warnings like these are not really acceptable. It means there
> > is an inconsistency in the way the Kconfig dependencies are defined.
> > 
> > Does this help:
> > 
> >   select IMA_SECURE_AND_OR_TRUSTED_BOOT   if EFI && IMA_ARCH_POLICY
> > 
> > ?
> 
> Yes, that's fine for x86.  Michael, Martin, do you want something
> similar or would you prefer actually selecting IMA_ARCH_POLICY?

For s390 something like

select IMA_SECURE_AND_OR_TRUSTED_BOOT if IMA_ARCH_POLICY

should be fine.

Thanks,
Heiko



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

2020-03-02 Thread Mimi Zohar
On Mon, 2020-03-02 at 15:52 +0100, Ard Biesheuvel wrote:
> On Mon, 2 Mar 2020 at 15:48, Mimi Zohar  wrote:
> >
> > On Wed, 2020-02-26 at 14:10 -0500, Nayna Jain 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 
> > > Signed-off-by: Nayna Jain 
> > > Cc: Ard Biesheuvel 
> > > Cc: Martin Schwidefsky 
> > > Cc: Philipp Rudo 
> > > Cc: Michael Ellerman 
> > > ---
> > >  arch/powerpc/Kconfig   | 2 +-
> > >  arch/s390/Kconfig  | 1 +
> > >  arch/x86/Kconfig   | 1 +
> > >  include/linux/ima.h| 3 +--
> > >  security/integrity/ima/Kconfig | 9 +
> > >  5 files changed, 13 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > > index 497b7d0b2d7e..b8ce1b995633 100644
> > > --- a/arch/powerpc/Kconfig
> > > +++ b/arch/powerpc/Kconfig
> > > @@ -246,6 +246,7 @@ config PPC
> > >   select SYSCTL_EXCEPTION_TRACE
> > >   select THREAD_INFO_IN_TASK
> > >   select VIRT_TO_BUS  if !PPC64
> > > + select IMA_SECURE_AND_OR_TRUSTED_BOOT   if PPC_SECURE_BOOT
> > >   #
> > >   # Please keep this list sorted alphabetically.
> > >   #
> > > @@ -978,7 +979,6 @@ config PPC_SECURE_BOOT
> > >   prompt "Enable secure boot support"
> > >   bool
> > >   depends on PPC_POWERNV
> > > - depends on IMA_ARCH_POLICY
> > >   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..90ff3633ade6 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
> > >
> > >
> > >  config SCHED_OMIT_FRAME_POINTER
> > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > index beea77046f9b..cafa66313fe2 100644
> > > --- a/arch/x86/Kconfig
> > > +++ b/arch/x86/Kconfig
> > > @@ -230,6 +230,7 @@ config X86
> > >   select VIRT_TO_BUS
> > >   select X86_FEATURE_NAMESif PROC_FS
> > >   select PROC_PID_ARCH_STATUS if PROC_FS
> > > + select IMA_SECURE_AND_OR_TRUSTED_BOOT   if EFI
> >
> > Not everyone is interested in enabling IMA or requiring IMA runtime
> > policies.  With this patch, enabling IMA_ARCH_POLICY is therefore
> > still left up to the person building the kernel.  As a result, I'm
> > seeing the following warning, which is kind of cool.
> >
> > WARNING: unmet direct dependencies detected for
> > IMA_SECURE_AND_OR_TRUSTED_BOOT
> >   Depends on [n]: INTEGRITY [=y] && IMA [=y] && IMA_ARCH_POLICY [=n]
> >   Selected by [y]:
> >   - X86 [=y] && EFI [=y]
> >
> > Ard, Michael, Martin, just making sure this type of warning is
> > acceptable before upstreaming this patch.  I would appreciate your
> > tags.
> >
> 
> Ehm, no, warnings like these are not really acceptable. It means there
> is an inconsistency in the way the Kconfig dependencies are defined.
> 
> Does this help:
> 
>   select IMA_SECURE_AND_OR_TRUSTED_BOOT   if EFI && IMA_ARCH_POLICY
> 
> ?

Yes, that's fine for x86.  Michael, Martin, do you want something
similar or would you prefer actually selecting IMA_ARCH_POLICY?

Mimi



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

2020-03-02 Thread Ard Biesheuvel
On Mon, 2 Mar 2020 at 15:48, Mimi Zohar  wrote:
>
> On Wed, 2020-02-26 at 14:10 -0500, Nayna Jain 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 
> > Signed-off-by: Nayna Jain 
> > Cc: Ard Biesheuvel 
> > Cc: Martin Schwidefsky 
> > Cc: Philipp Rudo 
> > Cc: Michael Ellerman 
> > ---
> >  arch/powerpc/Kconfig   | 2 +-
> >  arch/s390/Kconfig  | 1 +
> >  arch/x86/Kconfig   | 1 +
> >  include/linux/ima.h| 3 +--
> >  security/integrity/ima/Kconfig | 9 +
> >  5 files changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 497b7d0b2d7e..b8ce1b995633 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -246,6 +246,7 @@ config PPC
> >   select SYSCTL_EXCEPTION_TRACE
> >   select THREAD_INFO_IN_TASK
> >   select VIRT_TO_BUS  if !PPC64
> > + select IMA_SECURE_AND_OR_TRUSTED_BOOT   if PPC_SECURE_BOOT
> >   #
> >   # Please keep this list sorted alphabetically.
> >   #
> > @@ -978,7 +979,6 @@ config PPC_SECURE_BOOT
> >   prompt "Enable secure boot support"
> >   bool
> >   depends on PPC_POWERNV
> > - depends on IMA_ARCH_POLICY
> >   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..90ff3633ade6 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
> >
> >
> >  config SCHED_OMIT_FRAME_POINTER
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index beea77046f9b..cafa66313fe2 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -230,6 +230,7 @@ config X86
> >   select VIRT_TO_BUS
> >   select X86_FEATURE_NAMESif PROC_FS
> >   select PROC_PID_ARCH_STATUS if PROC_FS
> > + select IMA_SECURE_AND_OR_TRUSTED_BOOT   if EFI
>
> Not everyone is interested in enabling IMA or requiring IMA runtime
> policies.  With this patch, enabling IMA_ARCH_POLICY is therefore
> still left up to the person building the kernel.  As a result, I'm
> seeing the following warning, which is kind of cool.
>
> WARNING: unmet direct dependencies detected for
> IMA_SECURE_AND_OR_TRUSTED_BOOT
>   Depends on [n]: INTEGRITY [=y] && IMA [=y] && IMA_ARCH_POLICY [=n]
>   Selected by [y]:
>   - X86 [=y] && EFI [=y]
>
> Ard, Michael, Martin, just making sure this type of warning is
> acceptable before upstreaming this patch.  I would appreciate your
> tags.
>

Ehm, no, warnings like these are not really acceptable. It means there
is an inconsistency in the way the Kconfig dependencies are defined.

Does this help:

  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
> > + default n
> > + help
> > +This option is selected by architectures to enable secure and/or
> > +trusted boot based on IMA runtime policies.
>
>
>
>


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

2020-03-02 Thread Mimi Zohar
On Wed, 2020-02-26 at 14:10 -0500, Nayna Jain 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 
> Signed-off-by: Nayna Jain 
> Cc: Ard Biesheuvel 
> Cc: Martin Schwidefsky 
> Cc: Philipp Rudo 
> Cc: Michael Ellerman 
> ---
>  arch/powerpc/Kconfig   | 2 +-
>  arch/s390/Kconfig  | 1 +
>  arch/x86/Kconfig   | 1 +
>  include/linux/ima.h| 3 +--
>  security/integrity/ima/Kconfig | 9 +
>  5 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 497b7d0b2d7e..b8ce1b995633 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -246,6 +246,7 @@ config PPC
>   select SYSCTL_EXCEPTION_TRACE
>   select THREAD_INFO_IN_TASK
>   select VIRT_TO_BUS  if !PPC64
> + select IMA_SECURE_AND_OR_TRUSTED_BOOT   if PPC_SECURE_BOOT
>   #
>   # Please keep this list sorted alphabetically.
>   #
> @@ -978,7 +979,6 @@ config PPC_SECURE_BOOT
>   prompt "Enable secure boot support"
>   bool
>   depends on PPC_POWERNV
> - depends on IMA_ARCH_POLICY
>   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..90ff3633ade6 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
>  
>  
>  config SCHED_OMIT_FRAME_POINTER
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index beea77046f9b..cafa66313fe2 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -230,6 +230,7 @@ config X86
>   select VIRT_TO_BUS
>   select X86_FEATURE_NAMESif PROC_FS
>   select PROC_PID_ARCH_STATUS if PROC_FS
> + select IMA_SECURE_AND_OR_TRUSTED_BOOT   if EFI

Not everyone is interested in enabling IMA or requiring IMA runtime
policies.  With this patch, enabling IMA_ARCH_POLICY is therefore
still left up to the person building the kernel.  As a result, I'm
seeing the following warning, which is kind of cool.

WARNING: unmet direct dependencies detected for
IMA_SECURE_AND_OR_TRUSTED_BOOT
  Depends on [n]: INTEGRITY [=y] && IMA [=y] && IMA_ARCH_POLICY [=n]
  Selected by [y]:
  - X86 [=y] && EFI [=y]

Ard, Michael, Martin, just making sure this type of warning is
acceptable before upstreaming this patch.  I would appreciate your
tags.

thanks!

Mimi

>  
>  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
> + default n
> + help
> +This option is selected by architectures to enable secure and/or
> +trusted boot based on IMA runtime policies.






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

2020-02-27 Thread Mimi Zohar
On Wed, 2020-02-26 at 15:36 -0500, Mimi Zohar wrote:
> On Wed, 2020-02-26 at 11:21 -0800, Lakshmi Ramasubramanian wrote:
> > Hi Nayna,
> > 
> > > +
> > > +config IMA_SECURE_AND_OR_TRUSTED_BOOT
> > > + bool
> > > + depends on IMA
> > > + depends on IMA_ARCH_POLICY
> > > + default n
> > > + help
> > > +This option is selected by architectures to enable secure and/or
> > > +trusted boot based on IMA runtime policies.
> > > 
> > 
> > Why is the default for this new config "n"?
> > Is there any reason to not turn on this config if both IMA and 
> > IMA_ARCH_POLICY are set to y?
> 
> Good catch.  Having "IMA_SECURE_AND_OR_TRUSTED_BOOT" depend on
> "IMA_ARCH_POLICY" doesn't make sense.  "IMA_ARCH_POLICY" needs to be
> selected.

After discussing this some more with Nayna, the new Kconfig indicates
that the architecture defines the arch_ima_get_secureboot() and
arch_get_ima_policy() functions, but doesn't automatically enable
IMA_ARCH_POLICY.  The decision to enable IMA_ARCH_POLICY is left up to
whoever is building the kernel.  The patch, at least this aspect of
it, is correct.

Mimi



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

2020-02-26 Thread Mimi Zohar
On Wed, 2020-02-26 at 11:21 -0800, Lakshmi Ramasubramanian wrote:
> Hi Nayna,
> 
> > +
> > +config IMA_SECURE_AND_OR_TRUSTED_BOOT
> > +   bool
> > +   depends on IMA
> > +   depends on IMA_ARCH_POLICY
> > +   default n
> > +   help
> > +  This option is selected by architectures to enable secure and/or
> > +  trusted boot based on IMA runtime policies.
> > 
> 
> Why is the default for this new config "n"?
> Is there any reason to not turn on this config if both IMA and 
> IMA_ARCH_POLICY are set to y?

Good catch.  Having "IMA_SECURE_AND_OR_TRUSTED_BOOT" depend on
"IMA_ARCH_POLICY" doesn't make sense.  "IMA_ARCH_POLICY" needs to be
selected.

thanks,

Mimi



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

2020-02-26 Thread Nayna Jain
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 
Signed-off-by: Nayna Jain 
Cc: Ard Biesheuvel 
Cc: Martin Schwidefsky 
Cc: Philipp Rudo 
Cc: Michael Ellerman 
---
 arch/powerpc/Kconfig   | 2 +-
 arch/s390/Kconfig  | 1 +
 arch/x86/Kconfig   | 1 +
 include/linux/ima.h| 3 +--
 security/integrity/ima/Kconfig | 9 +
 5 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 497b7d0b2d7e..b8ce1b995633 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -246,6 +246,7 @@ config PPC
select SYSCTL_EXCEPTION_TRACE
select THREAD_INFO_IN_TASK
select VIRT_TO_BUS  if !PPC64
+   select IMA_SECURE_AND_OR_TRUSTED_BOOT   if PPC_SECURE_BOOT
#
# Please keep this list sorted alphabetically.
#
@@ -978,7 +979,6 @@ config PPC_SECURE_BOOT
prompt "Enable secure boot support"
bool
depends on PPC_POWERNV
-   depends on IMA_ARCH_POLICY
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..90ff3633ade6 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
 
 
 config SCHED_OMIT_FRAME_POINTER
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index beea77046f9b..cafa66313fe2 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -230,6 +230,7 @@ config X86
select VIRT_TO_BUS
select X86_FEATURE_NAMESif PROC_FS
select PROC_PID_ARCH_STATUS if PROC_FS
+   select IMA_SECURE_AND_OR_TRUSTED_BOOT   if EFI
 
 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
+   default n
+   help
+  This option is selected by architectures to enable secure and/or
+  trusted boot based on IMA runtime policies.
-- 
2.18.1



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

2020-02-26 Thread Lakshmi Ramasubramanian

Hi Nayna,


+
+config IMA_SECURE_AND_OR_TRUSTED_BOOT
+   bool
+   depends on IMA
+   depends on IMA_ARCH_POLICY
+   default n
+   help
+  This option is selected by architectures to enable secure and/or
+  trusted boot based on IMA runtime policies.



Why is the default for this new config "n"?
Is there any reason to not turn on this config if both IMA and 
IMA_ARCH_POLICY are set to y?


thanks,
 -lakshmi