* [PATCH] ima: defer arch_ima_get_secureboot() call to IMA init time
@ 2020-10-12 8:36 Ard Biesheuvel
2020-10-12 21:28 ` Mimi Zohar
0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2020-10-12 8:36 UTC (permalink / raw)
To: linux-efi
Cc: linux-integrity, linux-security-module, Ard Biesheuvel,
Chester Lin, Mimi Zohar, Dmitry Kasatkin, James Morris,
Serge E. Hallyn
Chester reports that it is necessary to introduce a new way to pass
the EFI secure boot status between the EFI stub and the core kernel
on ARM systems. The usual way of obtaining this information is by
checking the SecureBoot and SetupMode EFI variables, but this can
only be done after the EFI variable workqueue is created, which
occurs in a subsys_initcall(), whereas arch_ima_get_secureboot()
is called much earlier by the IMA framework.
However, the IMA framework itself is started as a late_initcall,
and the only reason the call to arch_ima_get_secureboot() occurs
so early is because it happens in the context of a __setup()
callback that parses the ima_appraise= command line parameter.
So let's refactor this code a little bit, by using a core_param()
callback to capture the command line argument, and deferring any
reasoning based on its contents to the IMA init routine.
Cc: Chester Lin <clin@suse.com>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Link: https://lore.kernel.org/linux-arm-kernel/20200904072905.25332-2-clin@suse.com/
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
include/linux/ima.h | 6 +++++
security/integrity/ima/ima_appraise.c | 24 +++++++++++---------
security/integrity/ima/ima_main.c | 1 +
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/include/linux/ima.h b/include/linux/ima.h
index d15100de6cdd..8aefee9c36ab 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -27,6 +27,12 @@ extern void ima_post_path_mknod(struct dentry *dentry);
extern int ima_file_hash(struct file *file, char *buf, size_t buf_size);
extern void ima_kexec_cmdline(int kernel_fd, const void *buf, int size);
+#ifdef CONFIG_IMA_APPRAISE_BOOTPARAM
+extern void ima_appraise_parse_cmdline(void);
+#else
+static inline void ima_appraise_parse_cmdline(void) {}
+#endif
+
#ifdef CONFIG_IMA_KEXEC
extern void ima_add_kexec_buffer(struct kimage *image);
#endif
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index b8848f53c8cc..a0135dbf2a64 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -16,26 +16,28 @@
#include "ima.h"
-static int __init default_appraise_setup(char *str)
-{
#ifdef CONFIG_IMA_APPRAISE_BOOTPARAM
+static char *ima_appraise_cmdline_default __initdata;
+core_param(ima_appraise, ima_appraise_cmdline_default, charp, 0);
+
+void __init ima_appraise_parse_cmdline(void)
+{
+ if (!ima_appraise_cmdline_default)
+ return;
if (arch_ima_get_secureboot()) {
pr_info("Secure boot enabled: ignoring ima_appraise=%s boot parameter option",
- str);
- return 1;
+ ima_appraise_cmdline_default);
+ return;
}
- if (strncmp(str, "off", 3) == 0)
+ if (strncmp(ima_appraise_cmdline_default, "off", 3) == 0)
ima_appraise = 0;
- else if (strncmp(str, "log", 3) == 0)
+ else if (strncmp(ima_appraise_cmdline_default, "log", 3) == 0)
ima_appraise = IMA_APPRAISE_LOG;
- else if (strncmp(str, "fix", 3) == 0)
+ else if (strncmp(ima_appraise_cmdline_default, "fix", 3) == 0)
ima_appraise = IMA_APPRAISE_FIX;
-#endif
- return 1;
}
-
-__setup("ima_appraise=", default_appraise_setup);
+#endif
/*
* is_ima_appraise_enabled - return appraise status
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 8a91711ca79b..31d72cf04768 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -850,6 +850,7 @@ static int __init init_ima(void)
{
int error;
+ ima_appraise_parse_cmdline();
ima_init_template_list();
hash_setup(CONFIG_IMA_DEFAULT_HASH);
error = ima_init();
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ima: defer arch_ima_get_secureboot() call to IMA init time
2020-10-12 8:36 [PATCH] ima: defer arch_ima_get_secureboot() call to IMA init time Ard Biesheuvel
@ 2020-10-12 21:28 ` Mimi Zohar
2020-10-13 6:59 ` Ard Biesheuvel
0 siblings, 1 reply; 3+ messages in thread
From: Mimi Zohar @ 2020-10-12 21:28 UTC (permalink / raw)
To: Ard Biesheuvel, linux-efi
Cc: linux-integrity, linux-security-module, Chester Lin,
Dmitry Kasatkin, James Morris, Serge E. Hallyn
Hi Ard,
On Mon, 2020-10-12 at 10:36 +0200, Ard Biesheuvel wrote:
> Chester reports that it is necessary to introduce a new way to pass
> the EFI secure boot status between the EFI stub and the core kernel
> on ARM systems. The usual way of obtaining this information is by
> checking the SecureBoot and SetupMode EFI variables, but this can
> only be done after the EFI variable workqueue is created, which
> occurs in a subsys_initcall(), whereas arch_ima_get_secureboot()
> is called much earlier by the IMA framework.
>
> However, the IMA framework itself is started as a late_initcall,
> and the only reason the call to arch_ima_get_secureboot() occurs
> so early is because it happens in the context of a __setup()
> callback that parses the ima_appraise= command line parameter.
>
> So let's refactor this code a little bit, by using a core_param()
> callback to capture the command line argument, and deferring any
> reasoning based on its contents to the IMA init routine.
Other than this patch needing to be on top of commit e4d7e2df3a09
("ima: limit secure boot feedback scope for appraise"), it looks good.
thanks,
Mimi
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ima: defer arch_ima_get_secureboot() call to IMA init time
2020-10-12 21:28 ` Mimi Zohar
@ 2020-10-13 6:59 ` Ard Biesheuvel
0 siblings, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2020-10-13 6:59 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-efi, linux-integrity, linux-security-module, Chester Lin,
Dmitry Kasatkin, James Morris, Serge E. Hallyn
On Mon, 12 Oct 2020 at 23:29, Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> Hi Ard,
>
> On Mon, 2020-10-12 at 10:36 +0200, Ard Biesheuvel wrote:
> > Chester reports that it is necessary to introduce a new way to pass
> > the EFI secure boot status between the EFI stub and the core kernel
> > on ARM systems. The usual way of obtaining this information is by
> > checking the SecureBoot and SetupMode EFI variables, but this can
> > only be done after the EFI variable workqueue is created, which
> > occurs in a subsys_initcall(), whereas arch_ima_get_secureboot()
> > is called much earlier by the IMA framework.
> >
> > However, the IMA framework itself is started as a late_initcall,
> > and the only reason the call to arch_ima_get_secureboot() occurs
> > so early is because it happens in the context of a __setup()
> > callback that parses the ima_appraise= command line parameter.
> >
> > So let's refactor this code a little bit, by using a core_param()
> > callback to capture the command line argument, and deferring any
> > reasoning based on its contents to the IMA init routine.
>
> Other than this patch needing to be on top of commit e4d7e2df3a09
> ("ima: limit secure boot feedback scope for appraise"), it looks good.
>
Thanks Mimi, I will rebase and resend.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-13 6:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-12 8:36 [PATCH] ima: defer arch_ima_get_secureboot() call to IMA init time Ard Biesheuvel
2020-10-12 21:28 ` Mimi Zohar
2020-10-13 6:59 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).