2020-02-26 19:11:19

by Nayna Jain

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

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 <[email protected]>
Signed-off-by: Nayna Jain <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Martin Schwidefsky <[email protected]>
Cc: Philipp Rudo <[email protected]>
Cc: Michael Ellerman <[email protected]>
---
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_NAMES if 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


2020-02-26 19:22:11

by Lakshmi Ramasubramanian

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

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

2020-02-26 20:37:05

by Mimi Zohar

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

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

2020-02-27 19:39:41

by Mimi Zohar

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

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

2020-03-02 14:49:44

by Mimi Zohar

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

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 <[email protected]>
> Signed-off-by: Nayna Jain <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Martin Schwidefsky <[email protected]>
> Cc: Philipp Rudo <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> ---
> 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_NAMES if 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.




2020-03-02 14:53:17

by Ard Biesheuvel

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

On Mon, 2 Mar 2020 at 15:48, Mimi Zohar <[email protected]> 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 <[email protected]>
> > Signed-off-by: Nayna Jain <[email protected]>
> > Cc: Ard Biesheuvel <[email protected]>
> > Cc: Martin Schwidefsky <[email protected]>
> > Cc: Philipp Rudo <[email protected]>
> > Cc: Michael Ellerman <[email protected]>
> > ---
> > 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_NAMES if 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.
>
>
>
>

2020-03-02 14:58:17

by Mimi Zohar

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

On Mon, 2020-03-02 at 15:52 +0100, Ard Biesheuvel wrote:
> On Mon, 2 Mar 2020 at 15:48, Mimi Zohar <[email protected]> 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 <[email protected]>
> > > Signed-off-by: Nayna Jain <[email protected]>
> > > Cc: Ard Biesheuvel <[email protected]>
> > > Cc: Martin Schwidefsky <[email protected]>
> > > Cc: Philipp Rudo <[email protected]>
> > > Cc: Michael Ellerman <[email protected]>
> > > ---
> > > 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_NAMES if 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

2020-03-02 21:21:50

by Heiko Carstens

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

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 <[email protected]> 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_NAMES if 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

2020-03-02 23:24:24

by Michael Ellerman

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

Mimi Zohar <[email protected]> writes:
> On Mon, 2020-03-02 at 15:52 +0100, Ard Biesheuvel wrote:
>> On Mon, 2 Mar 2020 at 15:48, Mimi Zohar <[email protected]> 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 <[email protected]>
>> > > Signed-off-by: Nayna Jain <[email protected]>
>> > > Cc: Ard Biesheuvel <[email protected]>
>> > > Cc: Martin Schwidefsky <[email protected]>
>> > > Cc: Philipp Rudo <[email protected]>
>> > > Cc: Michael Ellerman <[email protected]>
>> > > ---
>> > > 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_NAMES if 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