| Message ID | 1583289211-5420-1-git-send-email-nayna@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] ima: add a new CONFIG for loading arch-specific policies | expand |
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> 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 <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> > --- > 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 <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 > >
On Wed, 04 Mar 2020 07:55:38 -0500 Mimi Zohar <zohar@linux.ibm.com> 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@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? 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 <James.Bottomley@HansenPartnership.com> 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.
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.
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> --- 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(-)