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: Philipp Rudo <[email protected]>
Cc: Michael Ellerman <[email protected]>
---
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
+ default n
+ help
+ This option is selected by architectures to enable secure and/or
+ trusted boot based on IMA runtime policies.
--
2.13.6
On Wed, 4 Mar 2020 at 03:34, Nayna Jain <[email protected]> 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: Philipp Rudo <[email protected]>
> Cc: Michael Ellerman <[email protected]>
Acked-by: Ard Biesheuvel <[email protected]>
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.
> ---
> 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?
> + default n
> + help
> + This option is selected by architectures to enable secure and/or
> + trusted boot based on IMA runtime policies.
> --
> 2.13.6
>
On Tue, 2020-03-03 at 21:33 -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: Philipp Rudo <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> ---
> 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
> + default n
You can't do this: a symbol designed to be selected can't depend on
other symbols because Kconfig doesn't see the dependencies during
select. We even have a doc for this now:
Documentation/kbuild/Kconfig.select-break
The only way to get this to work would be to have the long name symbol
select both IMA and IMA_ARCH_POLICY, which doesn't seem to be what you
want either.
Looking at what you're trying to do, I think making the symbol
independent of IMA and IMA_ARCH_POLICY is the correct thing, then
enforce the dependencies inside the outer #ifdef, but I haven't looked
deeply at the code.
James
On Tue, 2020-03-03 at 23:43 -0800, James Bottomley wrote:
> On Tue, 2020-03-03 at 21:33 -0500, Nayna Jain wrote:
> > 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
>
> You can't do this: a symbol designed to be selected can't depend on
> other symbols because Kconfig doesn't see the dependencies during
> select. We even have a doc for this now:
>
> Documentation/kbuild/Kconfig.select-break
The document is discussing a circular dependency, where C selects B.
IMA_SECURE_AND_OR_TRUSTED_BOOT is not selecting anything, but is
being selected. All of the Kconfig's are now dependent on
IMA_ARCH_POLICY being enabled before selecting
IMA_SECURE_AND_OR_TRUSTED_BOOT.
As Ard pointed out, both IMA and IMA_ARCH_POLICY are not needed, as
IMA_ARCH_POLICY is already dependent on IMA.
Mimi
[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 <[email protected]> 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: Philipp Rudo <[email protected]>
> > Cc: Michael Ellerman <[email protected]>
>
> Acked-by: Ard Biesheuvel <[email protected]>
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
> >
On Wed, 04 Mar 2020 07:55:38 -0500
Mimi Zohar <[email protected]> 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 <[email protected]> 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: Philipp Rudo <[email protected]>
> > > Cc: Michael Ellerman <[email protected]>
> >
> > Acked-by: Ard Biesheuvel <[email protected]>
>
> 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
> > >
>
On Wed, 2020-03-04 at 07:35 -0500, Mimi Zohar wrote:
> On Tue, 2020-03-03 at 23:43 -0800, James Bottomley wrote:
> > On Tue, 2020-03-03 at 21:33 -0500, Nayna Jain wrote:
> > > 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
> >
> > You can't do this: a symbol designed to be selected can't depend on
> > other symbols because Kconfig doesn't see the dependencies during
> > select. We even have a doc for this now:
> >
> > Documentation/kbuild/Kconfig.select-break
>
> The document is discussing a circular dependency, where C selects B.
> IMA_SECURE_AND_OR_TRUSTED_BOOT is not selecting anything, but is
> being selected. All of the Kconfig's are now dependent on
> IMA_ARCH_POLICY being enabled before selecting
> IMA_SECURE_AND_OR_TRUSTED_BOOT.
>
> As Ard pointed out, both IMA and IMA_ARCH_POLICY are not needed, as
> IMA_ARCH_POLICY is already dependent on IMA.
Then removing them is fine, if they're not necessary ... you just can't
select a symbol with dependencies because the two Kconfig mechanisms
don't mix.
James
James Bottomley <[email protected]> writes:
> On Wed, 2020-03-04 at 07:35 -0500, Mimi Zohar wrote:
>> On Tue, 2020-03-03 at 23:43 -0800, James Bottomley wrote:
>> > On Tue, 2020-03-03 at 21:33 -0500, Nayna Jain wrote:
>> > > 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
>> >
>> > You can't do this: a symbol designed to be selected can't depend on
>> > other symbols because Kconfig doesn't see the dependencies during
>> > select. We even have a doc for this now:
>> >
>> > Documentation/kbuild/Kconfig.select-break
>>
>> The document is discussing a circular dependency, where C selects B.
>> IMA_SECURE_AND_OR_TRUSTED_BOOT is not selecting anything, but is
>> being selected. All of the Kconfig's are now dependent on
>> IMA_ARCH_POLICY being enabled before selecting
>> IMA_SECURE_AND_OR_TRUSTED_BOOT.
>>
>> As Ard pointed out, both IMA and IMA_ARCH_POLICY are not needed, as
>> IMA_ARCH_POLICY is already dependent on IMA.
>
> Then removing them is fine, if they're not necessary ... you just can't
> select a symbol with dependencies because the two Kconfig mechanisms
> don't mix.
You can safely select something if the selector has the same or stricter
set of dependencies than the selectee. And in this case that's true.
config IMA_SECURE_AND_OR_TRUSTED_BOOT
bool
depends on IMA
depends on IMA_ARCH_POLICY
powerpc:
depends on IMA_ARCH_POLICY
select IMA_SECURE_AND_OR_TRUSTED_BOOT
s390: select IMA_SECURE_AND_OR_TRUSTED_BOOT if IMA_ARCH_POLICY
x86: select IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI && IMA_ARCH_POLICY
But that's not to say it's the best solution, because you have to ensure
the arch code has the right set of dependencies.
I think this is actually a perfect case for using imply. We want the
arch code to indicate it wants IMA_SECURE_..., but only if all the IMA
related dependencies are met.
I think the patch below should work.
For example:
$ grep PPC_SECURE_BOOT .config
CONFIG_PPC_SECURE_BOOT=y
$ ./scripts/config -d CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
$ grep IMA_SECURE .config
# CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT is not set
$ make oldconfig
scripts/kconfig/conf --oldconfig Kconfig
#
# configuration written to .config
#
$ grep IMA_SECURE .config
CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT=y
$ ./scripts/config -d CONFIG_IMA_ARCH_POLICY
$ grep -e IMA_ARCH_POLICY -e IMA_SECURE .config
# CONFIG_IMA_ARCH_POLICY is not set
CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT=y
$ make olddefconfig
scripts/kconfig/conf --olddefconfig Kconfig
#
# configuration written to .config
#
$ grep -e IMA_ARCH_POLICY -e IMA_SECURE .config
# CONFIG_IMA_ARCH_POLICY is not set
$
cheers
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 497b7d0b2d7e..5b9f1cba2a44 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
+ imply 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..59c216af6264 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
+ imply IMA_SECURE_AND_OR_TRUSTED_BOOT
config SCHED_OMIT_FRAME_POINTER
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index beea77046f9b..92204a486d97 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
+ imply 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..5ba4ae040fd8 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -327,3 +327,10 @@ 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_ARCH_POLICY
+ help
+ This option is selected by architectures to enable secure and/or
+ trusted boot based on IMA runtime policies.