diff mbox series

[v3,3/3] arm64/ima: add ima_arch support

Message ID 20201030060840.1810-4-clin@suse.com
State New
Headers show
Series add ima_arch support for ARM64 | expand

Commit Message

Chester Lin Oct. 30, 2020, 6:08 a.m. UTC
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

Comments

Ard Biesheuvel Oct. 30, 2020, 11:53 a.m. UTC | #1
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?
Chester Lin Nov. 2, 2020, 7:20 a.m. UTC | #2
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.
Mimi Zohar Nov. 2, 2020, 12:13 p.m. UTC | #3
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 mbox series

Patch

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;
+}