| Message ID | 20201030060840.1810-4-clin@suse.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/3] efi: generalize efi_get_secureboot | expand |
On Fri, 30 Oct 2020 at 07:09, Chester Lin <clin@suse.com> wrote: > > Add arm64 IMA arch support. The code and arch policy is mainly inherited > from x86. > > Signed-off-by: Chester Lin <clin@suse.com> > --- > arch/arm64/Kconfig | 1 + > arch/arm64/kernel/Makefile | 2 ++ > arch/arm64/kernel/ima_arch.c | 43 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 46 insertions(+) > create mode 100644 arch/arm64/kernel/ima_arch.c > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index a42e8d13cc88..496a4a26afc6 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -201,6 +201,7 @@ config ARM64 > select SWIOTLB > select SYSCTL_EXCEPTION_TRACE > select THREAD_INFO_IN_TASK > + imply IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI > help > ARM 64-bit (AArch64) Linux support. > > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile > index bbaf0bc4ad60..0f6cbb50668c 100644 > --- a/arch/arm64/kernel/Makefile > +++ b/arch/arm64/kernel/Makefile > @@ -69,3 +69,5 @@ extra-y += $(head-y) vmlinux.lds > ifeq ($(CONFIG_DEBUG_EFI),y) > AFLAGS_head.o += -DVMLINUX_PATH="\"$(realpath $(objtree)/vmlinux)\"" > endif > + > +obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_arch.o > diff --git a/arch/arm64/kernel/ima_arch.c b/arch/arm64/kernel/ima_arch.c > new file mode 100644 > index 000000000000..564236d77adc > --- /dev/null > +++ b/arch/arm64/kernel/ima_arch.c > @@ -0,0 +1,43 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2018 IBM Corporation > + */ > +#include <linux/efi.h> > +#include <linux/module.h> > +#include <linux/ima.h> > + > +bool arch_ima_get_secureboot(void) > +{ > + static bool sb_enabled; > + static bool initialized; > + > + if (!initialized & efi_enabled(EFI_BOOT)) { > + sb_enabled = ima_get_efi_secureboot(); > + initialized = true; > + } > + > + return sb_enabled; > +} > + > +/* secure and trusted boot arch rules */ > +static const char * const sb_arch_rules[] = { > +#if !IS_ENABLED(CONFIG_KEXEC_SIG) > + "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig", > +#endif /* CONFIG_KEXEC_SIG */ > + "measure func=KEXEC_KERNEL_CHECK", > +#if !IS_ENABLED(CONFIG_MODULE_SIG) > + "appraise func=MODULE_CHECK appraise_type=imasig", > +#endif > + "measure func=MODULE_CHECK", > + NULL > +}; > + > +const char * const *arch_get_ima_policy(void) > +{ > + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) { > + if (IS_ENABLED(CONFIG_MODULE_SIG)) > + set_module_sig_enforced(); > + return sb_arch_rules; > + } > + return NULL; > +} > -- > 2.28.0 > Can we move all this stuff into security/integrity/ima/ima_efi.c instead?
On Fri, Oct 30, 2020 at 12:53:25PM +0100, Ard Biesheuvel wrote: > On Fri, 30 Oct 2020 at 07:09, Chester Lin <clin@suse.com> wrote: > > > > Add arm64 IMA arch support. The code and arch policy is mainly inherited > > from x86. > > > > Signed-off-by: Chester Lin <clin@suse.com> > > --- > > arch/arm64/Kconfig | 1 + > > arch/arm64/kernel/Makefile | 2 ++ > > arch/arm64/kernel/ima_arch.c | 43 ++++++++++++++++++++++++++++++++++++ > > 3 files changed, 46 insertions(+) > > create mode 100644 arch/arm64/kernel/ima_arch.c > > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > index a42e8d13cc88..496a4a26afc6 100644 > > --- a/arch/arm64/Kconfig > > +++ b/arch/arm64/Kconfig > > @@ -201,6 +201,7 @@ config ARM64 > > select SWIOTLB > > select SYSCTL_EXCEPTION_TRACE > > select THREAD_INFO_IN_TASK > > + imply IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI > > help > > ARM 64-bit (AArch64) Linux support. > > > > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile > > index bbaf0bc4ad60..0f6cbb50668c 100644 > > --- a/arch/arm64/kernel/Makefile > > +++ b/arch/arm64/kernel/Makefile > > @@ -69,3 +69,5 @@ extra-y += $(head-y) vmlinux.lds > > ifeq ($(CONFIG_DEBUG_EFI),y) > > AFLAGS_head.o += -DVMLINUX_PATH="\"$(realpath $(objtree)/vmlinux)\"" > > endif > > + > > +obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_arch.o > > diff --git a/arch/arm64/kernel/ima_arch.c b/arch/arm64/kernel/ima_arch.c > > new file mode 100644 > > index 000000000000..564236d77adc > > --- /dev/null > > +++ b/arch/arm64/kernel/ima_arch.c > > @@ -0,0 +1,43 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (C) 2018 IBM Corporation > > + */ > > +#include <linux/efi.h> > > +#include <linux/module.h> > > +#include <linux/ima.h> > > + > > +bool arch_ima_get_secureboot(void) > > +{ > > + static bool sb_enabled; > > + static bool initialized; > > + > > + if (!initialized & efi_enabled(EFI_BOOT)) { > > + sb_enabled = ima_get_efi_secureboot(); > > + initialized = true; > > + } > > + > > + return sb_enabled; > > +} > > + > > +/* secure and trusted boot arch rules */ > > +static const char * const sb_arch_rules[] = { > > +#if !IS_ENABLED(CONFIG_KEXEC_SIG) > > + "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig", > > +#endif /* CONFIG_KEXEC_SIG */ > > + "measure func=KEXEC_KERNEL_CHECK", > > +#if !IS_ENABLED(CONFIG_MODULE_SIG) > > + "appraise func=MODULE_CHECK appraise_type=imasig", > > +#endif > > + "measure func=MODULE_CHECK", > > + NULL > > +}; > > + > > +const char * const *arch_get_ima_policy(void) > > +{ > > + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) { > > + if (IS_ENABLED(CONFIG_MODULE_SIG)) > > + set_module_sig_enforced(); > > + return sb_arch_rules; > > + } > > + return NULL; > > +} > > -- > > 2.28.0 > > > > Can we move all this stuff into security/integrity/ima/ima_efi.c instead? > Actually I hesitated to move all this stuff into ima_efi.c when coding v3 because I haven't figured out a clear picture to achieve it. Since each architecture could still have different details to trigger secure boot detection and define their arch-specific rules [e.g. Having boot_params in x86_64 creates more conditions that need to be determined before calling get_sb_mode()], moving all this stuff seems to decrease the flexibility. Besides, it might also affect the consistency of ima_arch as well, for example, ppc and s390 still use these function prototypes defined in ima.h. Since these functions are already referred by non-EFI architectures, why don't we still reuse these prototypes? For example, we could remain a smaller arch_ima_get_secureboot() and the arch-specific rules but move the major part of arch_get_ima_policy() into ima_efi.c. For example, we could implement an efi_ima_policy() for arch_get_ima_policy() to call so that the arch_get_ima_policy() doesn't have to know some details such as checking conditions or calling set_module_sig_enforced(). Please feel free to let me know if any suggestions.
On Mon, 2020-11-02 at 15:20 +0800, Chester Lin wrote: > On Fri, Oct 30, 2020 at 12:53:25PM +0100, Ard Biesheuvel wrote: > > On Fri, 30 Oct 2020 at 07:09, Chester Lin <clin@suse.com> wrote: > > > > > > Add arm64 IMA arch support. The code and arch policy is mainly inherited > > > from x86. > > > > > > Signed-off-by: Chester Lin <clin@suse.com> > > > --- > > > arch/arm64/Kconfig | 1 + > > > arch/arm64/kernel/Makefile | 2 ++ > > > arch/arm64/kernel/ima_arch.c | 43 ++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 46 insertions(+) > > > create mode 100644 arch/arm64/kernel/ima_arch.c > > > > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > > index a42e8d13cc88..496a4a26afc6 100644 > > > --- a/arch/arm64/Kconfig > > > +++ b/arch/arm64/Kconfig > > > @@ -201,6 +201,7 @@ config ARM64 > > > select SWIOTLB > > > select SYSCTL_EXCEPTION_TRACE > > > select THREAD_INFO_IN_TASK > > > + imply IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI > > > help > > > ARM 64-bit (AArch64) Linux support. > > > > > > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile > > > index bbaf0bc4ad60..0f6cbb50668c 100644 > > > --- a/arch/arm64/kernel/Makefile > > > +++ b/arch/arm64/kernel/Makefile > > > @@ -69,3 +69,5 @@ extra-y += $(head-y) vmlinux.lds > > > ifeq ($(CONFIG_DEBUG_EFI),y) > > > AFLAGS_head.o += -DVMLINUX_PATH="\"$(realpath $(objtree)/vmlinux)\"" > > > endif > > > + > > > +obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_arch.o > > > diff --git a/arch/arm64/kernel/ima_arch.c b/arch/arm64/kernel/ima_arch.c > > > new file mode 100644 > > > index 000000000000..564236d77adc > > > --- /dev/null > > > +++ b/arch/arm64/kernel/ima_arch.c > > > @@ -0,0 +1,43 @@ > > > +// SPDX-License-Identifier: GPL-2.0+ > > > +/* > > > + * Copyright (C) 2018 IBM Corporation > > > + */ > > > +#include <linux/efi.h> > > > +#include <linux/module.h> > > > +#include <linux/ima.h> > > > + > > > +bool arch_ima_get_secureboot(void) > > > +{ > > > + static bool sb_enabled; > > > + static bool initialized; > > > + > > > + if (!initialized & efi_enabled(EFI_BOOT)) { > > > + sb_enabled = ima_get_efi_secureboot(); > > > + initialized = true; > > > + } > > > + > > > + return sb_enabled; > > > +} > > > + > > > +/* secure and trusted boot arch rules */ > > > +static const char * const sb_arch_rules[] = { > > > +#if !IS_ENABLED(CONFIG_KEXEC_SIG) > > > + "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig", > > > +#endif /* CONFIG_KEXEC_SIG */ > > > + "measure func=KEXEC_KERNEL_CHECK", > > > +#if !IS_ENABLED(CONFIG_MODULE_SIG) > > > + "appraise func=MODULE_CHECK appraise_type=imasig", > > > +#endif > > > + "measure func=MODULE_CHECK", > > > + NULL > > > +}; > > > + > > > +const char * const *arch_get_ima_policy(void) > > > +{ > > > + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) { > > > + if (IS_ENABLED(CONFIG_MODULE_SIG)) > > > + set_module_sig_enforced(); > > > + return sb_arch_rules; > > > + } > > > + return NULL; > > > +} > > > -- > > > 2.28.0 > > > > > > > Can we move all this stuff into security/integrity/ima/ima_efi.c instead? > > > Actually I hesitated to move all this stuff into ima_efi.c when coding v3 > because I haven't figured out a clear picture to achieve it. Since each > architecture could still have different details to trigger secure boot detection > and define their arch-specific rules [e.g. Having boot_params in x86_64 creates > more conditions that need to be determined before calling get_sb_mode()], moving > all this stuff seems to decrease the flexibility. Besides, it might also affect > the consistency of ima_arch as well, for example, ppc and s390 still use these > function prototypes defined in ima.h. Since these functions are already referred > by non-EFI architectures, why don't we still reuse these prototypes? For example, > we could remain a smaller arch_ima_get_secureboot() and the arch-specific rules > but move the major part of arch_get_ima_policy() into ima_efi.c. For example, > we could implement an efi_ima_policy() for arch_get_ima_policy() to call so that > the arch_get_ima_policy() doesn't have to know some details such as checking > conditions or calling set_module_sig_enforced(). > > Please feel free to let me know if any suggestions. Yes, that is the point and the reason for defining ima_efi.c and conditionally including it only for EFI systems. The existing ppc and s390 code should remain unaffected by this change. thanks, Mimi
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index a42e8d13cc88..496a4a26afc6 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -201,6 +201,7 @@ config ARM64 select SWIOTLB select SYSCTL_EXCEPTION_TRACE select THREAD_INFO_IN_TASK + imply IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI help ARM 64-bit (AArch64) Linux support. diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index bbaf0bc4ad60..0f6cbb50668c 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -69,3 +69,5 @@ extra-y += $(head-y) vmlinux.lds ifeq ($(CONFIG_DEBUG_EFI),y) AFLAGS_head.o += -DVMLINUX_PATH="\"$(realpath $(objtree)/vmlinux)\"" endif + +obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_arch.o diff --git a/arch/arm64/kernel/ima_arch.c b/arch/arm64/kernel/ima_arch.c new file mode 100644 index 000000000000..564236d77adc --- /dev/null +++ b/arch/arm64/kernel/ima_arch.c @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2018 IBM Corporation + */ +#include <linux/efi.h> +#include <linux/module.h> +#include <linux/ima.h> + +bool arch_ima_get_secureboot(void) +{ + static bool sb_enabled; + static bool initialized; + + if (!initialized & efi_enabled(EFI_BOOT)) { + sb_enabled = ima_get_efi_secureboot(); + initialized = true; + } + + return sb_enabled; +} + +/* secure and trusted boot arch rules */ +static const char * const sb_arch_rules[] = { +#if !IS_ENABLED(CONFIG_KEXEC_SIG) + "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig", +#endif /* CONFIG_KEXEC_SIG */ + "measure func=KEXEC_KERNEL_CHECK", +#if !IS_ENABLED(CONFIG_MODULE_SIG) + "appraise func=MODULE_CHECK appraise_type=imasig", +#endif + "measure func=MODULE_CHECK", + NULL +}; + +const char * const *arch_get_ima_policy(void) +{ + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) { + if (IS_ENABLED(CONFIG_MODULE_SIG)) + set_module_sig_enforced(); + return sb_arch_rules; + } + return NULL; +}
Add arm64 IMA arch support. The code and arch policy is mainly inherited from x86. Signed-off-by: Chester Lin <clin@suse.com> --- arch/arm64/Kconfig | 1 + arch/arm64/kernel/Makefile | 2 ++ arch/arm64/kernel/ima_arch.c | 43 ++++++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+) create mode 100644 arch/arm64/kernel/ima_arch.c