[RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Song Liu]

While reading and testing LSM code, I found IMA/EVM consume per inode
storage even when they are not in use. Add options to diable them in
kernel command line. The logic and syntax is mostly borrowed from an
old serious [1].

[1] https://lore.kernel.org/lkml/cover.1398259638.git.d.kasatkin@samsung.com/

Song Liu (2):
  ima: Add kernel parameter to disable IMA
  evm: Add kernel parameter to disable EVM

 security/integrity/evm/evm.h       |  6 ++++++
 security/integrity/evm/evm_main.c  | 22 ++++++++++++++--------
 security/integrity/evm/evm_secfs.c |  3 ++-
 security/integrity/ima/ima_main.c  | 13 +++++++++++++
 4 files changed, 35 insertions(+), 9 deletions(-)

--
2.43.5

[RFC 1/2] ima: Add kernel parameter to disable IMA

[Song Liu]

This patch provides kernel parameter 'ima=off' that disables IMA.
This will reduce memory consumption by the ima when it is not needed.
Specifically, this saves one pointer per inode in the system.

Originally-by: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Signed-off-by: Song Liu <song@kernel.org>
---
 security/integrity/ima/ima_main.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 06132cf47016..21968c78f03f 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -1144,10 +1144,22 @@ static int ima_kernel_module_request(char *kmod_name)
 
 #endif /* CONFIG_INTEGRITY_ASYMMETRIC_KEYS */
 
+
+static int ima_mode = 1;
+static int __init ima_setup(char *str)
+{
+	if (strncmp(str, "off", 3) == 0)
+		ima_mode = 0;
+	return 1;
+}
+__setup("ima=", ima_setup);
+
 static int __init init_ima(void)
 {
 	int error;
 
+	if (!ima_mode)
+		return 0;
 	ima_appraise_parse_cmdline();
 	ima_init_template_list();
 	hash_setup(CONFIG_IMA_DEFAULT_HASH);
@@ -1217,6 +1229,7 @@ DEFINE_LSM(ima) = {
 	.name = "ima",
 	.init = init_ima_lsm,
 	.order = LSM_ORDER_LAST,
+	.enabled = &ima_mode,
 	.blobs = &ima_blob_sizes,
 };
 
-- 
2.43.5

[RFC 2/2] evm: Add kernel parameter to disable EVM

[Song Liu]

This patch provides kernel parameter 'evm=' that disables EVM.
This will reduce memory consumption by the EVM when it is not needed.
Specifically, this saves one evm_iint_cache per inode in the system.

Originally-by: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Signed-off-by: Song Liu <song@kernel.org>
---
 security/integrity/evm/evm.h       |  6 ++++++
 security/integrity/evm/evm_main.c  | 22 ++++++++++++++--------
 security/integrity/evm/evm_secfs.c |  3 ++-
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 51aba5a54275..64428c35e4cf 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -17,6 +17,10 @@
 
 #include "../integrity.h"
 
+#define EVM_MODE_OFF	0
+#define EVM_MODE_ON	1
+#define EVM_MODE_FIX	2
+
 #define EVM_INIT_HMAC	0x0001
 #define EVM_INIT_X509	0x0002
 #define EVM_ALLOW_METADATA_WRITES	0x0004
@@ -26,6 +30,8 @@
 #define EVM_INIT_MASK (EVM_INIT_HMAC | EVM_INIT_X509 | EVM_SETUP_COMPLETE | \
 		       EVM_ALLOW_METADATA_WRITES)
 
+extern int evm_mode;
+
 struct xattr_list {
 	struct list_head list;
 	char *name;
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 377e57e9084f..738c38f8190d 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -72,17 +72,19 @@ static struct xattr_list evm_config_default_xattrnames[] = {
 
 LIST_HEAD(evm_config_xattrnames);
 
-static int evm_fixmode __ro_after_init;
-static int __init evm_set_fixmode(char *str)
+int evm_mode __ro_after_init = EVM_MODE_ON;
+
+static int __init evm_setup(char *str)
 {
-	if (strncmp(str, "fix", 3) == 0)
-		evm_fixmode = 1;
+	if (strncmp(str, "off", 3) == 0)
+		evm_mode = EVM_MODE_OFF;
+	else if (strncmp(str, "fix", 3) == 0)
+		evm_mode = EVM_MODE_FIX;
 	else
 		pr_err("invalid \"%s\" mode", str);
-
-	return 1;
+	return 0;
 }
-__setup("evm=", evm_set_fixmode);
+__setup("evm=", evm_setup);
 
 static void __init evm_init_config(void)
 {
@@ -441,7 +443,7 @@ static enum integrity_status evm_verify_current_integrity(struct dentry *dentry)
 {
 	struct inode *inode = d_backing_inode(dentry);
 
-	if (!evm_key_loaded() || !S_ISREG(inode->i_mode) || evm_fixmode)
+	if (!evm_key_loaded() || !S_ISREG(inode->i_mode) || evm_mode == EVM_MODE_FIX)
 		return INTEGRITY_PASS;
 	return evm_verify_hmac(dentry, NULL, NULL, 0);
 }
@@ -1117,6 +1119,9 @@ static int __init init_evm(void)
 	int error;
 	struct list_head *pos, *q;
 
+	if (evm_mode == EVM_MODE_OFF)
+		return 0;
+
 	evm_init_config();
 
 	error = integrity_init_keyring(INTEGRITY_KEYRING_EVM);
@@ -1178,6 +1183,7 @@ DEFINE_LSM(evm) = {
 	.name = "evm",
 	.init = init_evm_lsm,
 	.order = LSM_ORDER_LAST,
+	.enabled = &evm_mode,
 	.blobs = &evm_blob_sizes,
 };
 
diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
index 9b907c2fee60..65f896cb838e 100644
--- a/security/integrity/evm/evm_secfs.c
+++ b/security/integrity/evm/evm_secfs.c
@@ -69,7 +69,8 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf,
 	unsigned int i;
 	int ret;
 
-	if (!capable(CAP_SYS_ADMIN) || (evm_initialized & EVM_SETUP_COMPLETE))
+	if (!capable(CAP_SYS_ADMIN) || (evm_initialized & EVM_SETUP_COMPLETE) ||
+	    evm_mode == EVM_MODE_OFF)
 		return -EPERM;
 
 	ret = kstrtouint_from_user(buf, count, 0, &i);
-- 
2.43.5

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Casey Schaufler]

On 12/17/2024 12:25 PM, Song Liu wrote:
> While reading and testing LSM code, I found IMA/EVM consume per inode
> storage even when they are not in use. Add options to diable them in
> kernel command line. The logic and syntax is mostly borrowed from an
> old serious [1].

Why not omit ima and evm from the lsm= parameter?

>
> [1] https://lore.kernel.org/lkml/cover.1398259638.git.d.kasatkin@samsung.com/
>
> Song Liu (2):
>   ima: Add kernel parameter to disable IMA
>   evm: Add kernel parameter to disable EVM
>
>  security/integrity/evm/evm.h       |  6 ++++++
>  security/integrity/evm/evm_main.c  | 22 ++++++++++++++--------
>  security/integrity/evm/evm_secfs.c |  3 ++-
>  security/integrity/ima/ima_main.c  | 13 +++++++++++++
>  4 files changed, 35 insertions(+), 9 deletions(-)
>
> --
> 2.43.5
>

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Paul Moore]

On Tue, Dec 17, 2024 at 4:29 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 12/17/2024 12:25 PM, Song Liu wrote:
> > While reading and testing LSM code, I found IMA/EVM consume per inode
> > storage even when they are not in use. Add options to diable them in
> > kernel command line. The logic and syntax is mostly borrowed from an
> > old serious [1].
>
> Why not omit ima and evm from the lsm= parameter?

Exactly.  Here is a link to the kernel documentation if anyone is
interested (search for "lsm"):

https://docs.kernel.org/admin-guide/kernel-parameters.html

It is worth mentioning that this works for all the LSMs.

> > [1] https://lore.kernel.org/lkml/cover.1398259638.git.d.kasatkin@samsung.com/
> >
> > Song Liu (2):
> >   ima: Add kernel parameter to disable IMA
> >   evm: Add kernel parameter to disable EVM
> >
> >  security/integrity/evm/evm.h       |  6 ++++++
> >  security/integrity/evm/evm_main.c  | 22 ++++++++++++++--------
> >  security/integrity/evm/evm_secfs.c |  3 ++-
> >  security/integrity/ima/ima_main.c  | 13 +++++++++++++
> >  4 files changed, 35 insertions(+), 9 deletions(-)
> >
> > --
> > 2.43.5

-- 
paul-moore.com

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Song Liu]

> On Dec 17, 2024, at 1:59 PM, Paul Moore <paul@paul-moore.com> wrote:
> 
> On Tue, Dec 17, 2024 at 4:29 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 12/17/2024 12:25 PM, Song Liu wrote:
>>> While reading and testing LSM code, I found IMA/EVM consume per inode
>>> storage even when they are not in use. Add options to diable them in
>>> kernel command line. The logic and syntax is mostly borrowed from an
>>> old serious [1].
>> 
>> Why not omit ima and evm from the lsm= parameter?
> 
> Exactly.  Here is a link to the kernel documentation if anyone is
> interested (search for "lsm"):
> 
> https://docs.kernel.org/admin-guide/kernel-parameters.html
> 
> It is worth mentioning that this works for all the LSMs.

I guess this is a bug that ima and evm do cannot be disabled
by (not being add to) lsm= parameter?

Thanks,
Song


> 
>>> [1] https://lore.kernel.org/lkml/cover.1398259638.git.d.kasatkin@samsung.com/
>>> 
>>> Song Liu (2):
>>>  ima: Add kernel parameter to disable IMA
>>>  evm: Add kernel parameter to disable EVM
>>> 
>>> security/integrity/evm/evm.h       |  6 ++++++
>>> security/integrity/evm/evm_main.c  | 22 ++++++++++++++--------
>>> security/integrity/evm/evm_secfs.c |  3 ++-
>>> security/integrity/ima/ima_main.c  | 13 +++++++++++++
>>> 4 files changed, 35 insertions(+), 9 deletions(-)
>>> 
>>> --
>>> 2.43.5
> 
> -- 
> paul-moore.com

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Song Liu]

> On Dec 17, 2024, at 2:04 PM, Song Liu <songliubraving@meta.com> wrote:
> 
> 
> 
>> On Dec 17, 2024, at 1:59 PM, Paul Moore <paul@paul-moore.com> wrote:
>> 
>> On Tue, Dec 17, 2024 at 4:29 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>> On 12/17/2024 12:25 PM, Song Liu wrote:
>>>> While reading and testing LSM code, I found IMA/EVM consume per inode
>>>> storage even when they are not in use. Add options to diable them in
>>>> kernel command line. The logic and syntax is mostly borrowed from an
>>>> old serious [1].
>>> 
>>> Why not omit ima and evm from the lsm= parameter?
>> 
>> Exactly.  Here is a link to the kernel documentation if anyone is
>> interested (search for "lsm"):
>> 
>> https://docs.kernel.org/admin-guide/kernel-parameters.html
>> 
>> It is worth mentioning that this works for all the LSMs.
> 
> I guess this is a bug that ima and evm do cannot be disabled
> by (not being add to) lsm= parameter?

If we use lsm= to control ima and evm, we will need the following
changes in ordered_lsm_parse(). We still need supporting logic
in ima and evm side, so that ima and evm are only initialized 
when they are in lsm=.  

Does this sound the right way forward?

Thanks,
Song





diff --git i/security/security.c w/security/security.c
index 09664e09fec9..00271be3b0c1 100644
--- i/security/security.c
+++ w/security/security.c
@@ -365,6 +365,9 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
                        if (strcmp(lsm->name, name) == 0) {
                                if (lsm->order == LSM_ORDER_MUTABLE)
                                        append_ordered_lsm(lsm, origin);
+                               else if (lsm->order == LSM_ORDER_LAST)
+                                       set_enabled(lsm, true);
+
                                found = true;
                        }
                }
@@ -386,7 +389,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)

        /* LSM_ORDER_LAST is always last. */
        for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
-               if (lsm->order == LSM_ORDER_LAST)
+               if (lsm->order == LSM_ORDER_LAST && is_enabled(lsm))
                        append_ordered_lsm(lsm, "   last");
        }

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Paul Moore]

On Tue, Dec 17, 2024 at 5:47 PM Song Liu <songliubraving@meta.com> wrote:
>
> If we use lsm= to control ima and evm, we will need the following
> changes in ordered_lsm_parse(). We still need supporting logic
> in ima and evm side, so that ima and evm are only initialized
> when they are in lsm=.
>
> Does this sound the right way forward?

Have you tested it?  What happens?  There is value in going through
the testing process, especially if you haven't played much with the
LSM code.

I'd also want to see a comment line in both places explaining why it
is necessary to mark the LSM as enabled prior to actually adding it to
@ordered_lsms.  Something along the lines of only parsing the
parameter once should be sufficient.

> diff --git i/security/security.c w/security/security.c
> index 09664e09fec9..00271be3b0c1 100644
> --- i/security/security.c
> +++ w/security/security.c
> @@ -365,6 +365,9 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>                         if (strcmp(lsm->name, name) == 0) {
>                                 if (lsm->order == LSM_ORDER_MUTABLE)
>                                         append_ordered_lsm(lsm, origin);
> +                               else if (lsm->order == LSM_ORDER_LAST)
> +                                       set_enabled(lsm, true);
> +
>                                 found = true;
>                         }
>                 }
> @@ -386,7 +389,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>
>         /* LSM_ORDER_LAST is always last. */
>         for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> -               if (lsm->order == LSM_ORDER_LAST)
> +               if (lsm->order == LSM_ORDER_LAST && is_enabled(lsm))
>                         append_ordered_lsm(lsm, "   last");
>         }

-- 
paul-moore.com

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Song Liu]

> On Dec 17, 2024, at 3:16 PM, Paul Moore <paul@paul-moore.com> wrote:
> 
> On Tue, Dec 17, 2024 at 5:47 PM Song Liu <songliubraving@meta.com> wrote:
>> 
>> If we use lsm= to control ima and evm, we will need the following
>> changes in ordered_lsm_parse(). We still need supporting logic
>> in ima and evm side, so that ima and evm are only initialized
>> when they are in lsm=.
>> 
>> Does this sound the right way forward?
> 
> Have you tested it?  What happens?  There is value in going through
> the testing process, especially if you haven't played much with the
> LSM code.

Yes, I tested both the original patches and the "lsm=xx" version. 

> 
> I'd also want to see a comment line in both places explaining why it
> is necessary to mark the LSM as enabled prior to actually adding it to
> @ordered_lsms.  Something along the lines of only parsing the
> parameter once should be sufficient.

Please see below for the explanation. I will add different words in 
the actual comments so they make more sense as comments

> 
>> diff --git i/security/security.c w/security/security.c
>> index 09664e09fec9..00271be3b0c1 100644
>> --- i/security/security.c
>> +++ w/security/security.c
>> @@ -365,6 +365,9 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>>                        if (strcmp(lsm->name, name) == 0) {
>>                                if (lsm->order == LSM_ORDER_MUTABLE)
>>                                        append_ordered_lsm(lsm, origin);
>> +                               else if (lsm->order == LSM_ORDER_LAST)
>> +                                       set_enabled(lsm, true);

We need a flag here, saying we want to enable the lsm. We cannot do 
append_ordered_lsm() yet, otherwise, it will not be "last". 

>> +
>>                                found = true;
>>                        }
>>                }
>> @@ -386,7 +389,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>> 
>>        /* LSM_ORDER_LAST is always last. */
>>        for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
>> -               if (lsm->order == LSM_ORDER_LAST)
>> +               if (lsm->order == LSM_ORDER_LAST && is_enabled(lsm))
>>                        append_ordered_lsm(lsm, "   last");

Before this change, lsm with order==LSM_ORDER_LAST is always considered
enabled, which is a bug (if I understand you and Casey correctly). 
To fix this, we need a flag from above saying we actually want to enable 
it. 

I personally think it is fine to use set_enabled() to set the flag. 
But I don't have a strong preference, we can add a different flag. 

Does this make sense?

Thanks,
Song

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Song Liu]

> On Dec 17, 2024, at 3:33 PM, Song Liu <songliubraving@meta.com> wrote:

[...]

> 
>>> +
>>>                               found = true;
>>>                       }
>>>               }
>>> @@ -386,7 +389,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>>> 
>>>       /* LSM_ORDER_LAST is always last. */
>>>       for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
>>> -               if (lsm->order == LSM_ORDER_LAST)
>>> +               if (lsm->order == LSM_ORDER_LAST && is_enabled(lsm))
>>>                       append_ordered_lsm(lsm, "   last");
> 
> Before this change, lsm with order==LSM_ORDER_LAST is always considered
> enabled, which is a bug (if I understand you and Casey correctly).

According to commit 42994ee3cd7298b27698daa6848ed7168e72d056, LSMs with 
order LSM_ORDER_LAST is expected to be always enabled:

"Similarly to LSM_ORDER_FIRST, LSMs with LSM_ORDER_LAST are always enabled
and put at the end of the LSM list, if selected in the kernel
configuration. "

Roberto, it feels weird to have two "last and always on" LSMs (ima and evm)
I guess this is not the expected behavior? At least, it appears to be a
surprise for Paul and Casey. 

I will send patch that allow enable/disable ima and evm with lsm= cmdline.
We can further discuss the topic with the patch. 

Thanks,
Song

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Casey Schaufler]

On 12/17/2024 10:41 PM, Song Liu wrote:
>> On Dec 17, 2024, at 3:33 PM, Song Liu <songliubraving@meta.com> wrote:
> [...]
>
>>>> +
>>>>                               found = true;
>>>>                       }
>>>>               }
>>>> @@ -386,7 +389,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>>>>
>>>>       /* LSM_ORDER_LAST is always last. */
>>>>       for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
>>>> -               if (lsm->order == LSM_ORDER_LAST)
>>>> +               if (lsm->order == LSM_ORDER_LAST && is_enabled(lsm))
>>>>                       append_ordered_lsm(lsm, "   last");
>> Before this change, lsm with order==LSM_ORDER_LAST is always considered
>> enabled, which is a bug (if I understand you and Casey correctly).
> According to commit 42994ee3cd7298b27698daa6848ed7168e72d056, LSMs with 
> order LSM_ORDER_LAST is expected to be always enabled:
>
> "Similarly to LSM_ORDER_FIRST, LSMs with LSM_ORDER_LAST are always enabled
> and put at the end of the LSM list, if selected in the kernel
> configuration. "
>
> Roberto, it feels weird to have two "last and always on" LSMs (ima and evm)
> I guess this is not the expected behavior? At least, it appears to be a
> surprise for Paul and Casey.

I can't speak for Paul, but having multiple "first" and "last" entries
comes as no surprise to me. We should probably have used LSM_ORDER_EARLY
and LSM_ORDER_LATE instead of LSM_ORDER_FIRST and LSM_ORDER_LAST. As for
"always on", I recall that being an artifact of compatibility for the
security= boot option.

> I will send patch that allow enable/disable ima and evm with lsm= cmdline.
> We can further discuss the topic with the patch. 
>
> Thanks,
> Song
>
>

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Song Liu]

> On Dec 18, 2024, at 8:42 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> 
> On 12/17/2024 10:41 PM, Song Liu wrote:
>>> On Dec 17, 2024, at 3:33 PM, Song Liu <songliubraving@meta.com> wrote:
>> [...]
>> 
>>>>> +
>>>>>                              found = true;
>>>>>                      }
>>>>>              }
>>>>> @@ -386,7 +389,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>>>>> 
>>>>>      /* LSM_ORDER_LAST is always last. */
>>>>>      for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
>>>>> -               if (lsm->order == LSM_ORDER_LAST)
>>>>> +               if (lsm->order == LSM_ORDER_LAST && is_enabled(lsm))
>>>>>                      append_ordered_lsm(lsm, "   last");
>>> Before this change, lsm with order==LSM_ORDER_LAST is always considered
>>> enabled, which is a bug (if I understand you and Casey correctly).
>> According to commit 42994ee3cd7298b27698daa6848ed7168e72d056, LSMs with 
>> order LSM_ORDER_LAST is expected to be always enabled:
>> 
>> "Similarly to LSM_ORDER_FIRST, LSMs with LSM_ORDER_LAST are always enabled
>> and put at the end of the LSM list, if selected in the kernel
>> configuration. "
>> 
>> Roberto, it feels weird to have two "last and always on" LSMs (ima and evm)
>> I guess this is not the expected behavior? At least, it appears to be a
>> surprise for Paul and Casey.
> 
> I can't speak for Paul, but having multiple "first" and "last" entries
> comes as no surprise to me. We should probably have used LSM_ORDER_EARLY
> and LSM_ORDER_LATE instead of LSM_ORDER_FIRST and LSM_ORDER_LAST. As for
> "always on", I recall that being an artifact of compatibility for the
> security= boot option.

Yes, _LATE makes more sense than _LAST. _LAST is a bit weird, but not
surprising. 

By "surprise to you and Paul", I meant the "always on" part. It appears
to me that both you and Paul believed that ima and evm are only enabled 
with proper lsm= cmdline. Or maybe I totally misunderstood your
comments?

Thanks,
Song

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Song Liu]

> On Dec 17, 2024, at 1:29 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> 
> On 12/17/2024 12:25 PM, Song Liu wrote:
>> While reading and testing LSM code, I found IMA/EVM consume per inode
>> storage even when they are not in use. Add options to diable them in
>> kernel command line. The logic and syntax is mostly borrowed from an
>> old serious [1].
> 
> Why not omit ima and evm from the lsm= parameter?

Both ima and evm have LSM_ORDER_LAST, so they are not controlled
by lsm= parameter. But we can probably change this behavior in 
ordered_lsm_parse(), so that ima and evm are controlled by lsm=. 

Thanks,
Song

> 
>> 
>> [1] https://lore.kernel.org/lkml/cover.1398259638.git.d.kasatkin@samsung.com/
>> 
>> Song Liu (2):
>>  ima: Add kernel parameter to disable IMA
>>  evm: Add kernel parameter to disable EVM
>> 
>> security/integrity/evm/evm.h       |  6 ++++++
>> security/integrity/evm/evm_main.c  | 22 ++++++++++++++--------
>> security/integrity/evm/evm_secfs.c |  3 ++-
>> security/integrity/ima/ima_main.c  | 13 +++++++++++++
>> 4 files changed, 35 insertions(+), 9 deletions(-)
>> 
>> --
>> 2.43.5
>> 

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Mimi Zohar]

On Tue, 2024-12-17 at 13:29 -0800, Casey Schaufler wrote:
> On 12/17/2024 12:25 PM, Song Liu wrote:
> > While reading and testing LSM code, I found IMA/EVM consume per inode
> > storage even when they are not in use. Add options to diable them in
> > kernel command line. The logic and syntax is mostly borrowed from an
> > old serious [1].
> 
> Why not omit ima and evm from the lsm= parameter?

Casey, Paul, always enabling IMA & EVM as the last LSMs, if configured, were the
conditions for making IMA and EVM LSMs.  Up to that point, only when an inode
was in policy did it consume any memory (rbtree).  I'm pretty sure you remember
the rather heated discussion(s).

Mimi

> 
> > 
> > [1] https://lore.kernel.org/lkml/cover.1398259638.git.d.kasatkin@samsung.com/
> > 
> > Song Liu (2):
> >   ima: Add kernel parameter to disable IMA
> >   evm: Add kernel parameter to disable EVM
> > 
> >  security/integrity/evm/evm.h       |  6 ++++++
> >  security/integrity/evm/evm_main.c  | 22 ++++++++++++++--------
> >  security/integrity/evm/evm_secfs.c |  3 ++-
> >  security/integrity/ima/ima_main.c  | 13 +++++++++++++
> >  4 files changed, 35 insertions(+), 9 deletions(-)
> > 
> > --
> > 2.43.5
> > 
> 

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Song Liu]

Hi Mimi, 

Thanks for your comments!

> On Dec 18, 2024, at 3:02 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Tue, 2024-12-17 at 13:29 -0800, Casey Schaufler wrote:
>> On 12/17/2024 12:25 PM, Song Liu wrote:
>>> While reading and testing LSM code, I found IMA/EVM consume per inode
>>> storage even when they are not in use. Add options to diable them in
>>> kernel command line. The logic and syntax is mostly borrowed from an
>>> old serious [1].
>> 
>> Why not omit ima and evm from the lsm= parameter?
> 
> Casey, Paul, always enabling IMA & EVM as the last LSMs, if configured, were the
> conditions for making IMA and EVM LSMs.  Up to that point, only when an inode
> was in policy did it consume any memory (rbtree).  I'm pretty sure you remember
> the rather heated discussion(s).

I didn't know about this history until today. I apologize if this 
RFC/PATCH is moving to the direction against the original agreement. 
I didn't mean to break any agreement. 

My motivation is actually the per inode memory consumption of IMA 
and EVM. Once enabled, EVM appends a whole struct evm_iint_cache to 
each inode via i_security. IMA is better on memory consumption, as 
it only adds a pointer to i_security. 

It appears to me that a way to disable IMA and EVM at boot time can 
be useful, especially for distro kernels. But I guess there are 
reasons to not allow this (thus the earlier agreement). Could you 
please share your thoughts on this?

Thanks,
Song

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Roberto Sassu]

On Wed, 2024-12-18 at 17:07 +0000, Song Liu wrote:
> Hi Mimi, 
> 
> Thanks for your comments!
> 
> > On Dec 18, 2024, at 3:02 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > On Tue, 2024-12-17 at 13:29 -0800, Casey Schaufler wrote:
> > > On 12/17/2024 12:25 PM, Song Liu wrote:
> > > > While reading and testing LSM code, I found IMA/EVM consume per inode
> > > > storage even when they are not in use. Add options to diable them in
> > > > kernel command line. The logic and syntax is mostly borrowed from an
> > > > old serious [1].
> > > 
> > > Why not omit ima and evm from the lsm= parameter?
> > 
> > Casey, Paul, always enabling IMA & EVM as the last LSMs, if configured, were the
> > conditions for making IMA and EVM LSMs.  Up to that point, only when an inode
> > was in policy did it consume any memory (rbtree).  I'm pretty sure you remember
> > the rather heated discussion(s).
> 
> I didn't know about this history until today. I apologize if this 
> RFC/PATCH is moving to the direction against the original agreement. 
> I didn't mean to break any agreement. 
> 
> My motivation is actually the per inode memory consumption of IMA 
> and EVM. Once enabled, EVM appends a whole struct evm_iint_cache to 
> each inode via i_security. IMA is better on memory consumption, as 
> it only adds a pointer to i_security. 
> 
> It appears to me that a way to disable IMA and EVM at boot time can 
> be useful, especially for distro kernels. But I guess there are 
> reasons to not allow this (thus the earlier agreement). Could you 
> please share your thoughts on this?

Hi Song

IMA/EVM cannot be always disabled for two reasons: (1) for secure and
trusted boot, IMA is expected to enforce architecture-specific
policies; (2) accidentally disabling them will cause modified files to
be rejected when IMA/EVM are turned on again.

If the requirements above are met, we are fine on disabling IMA/EVM.

As for reserving space in the inode security blob, please refer to this
discussion, where we reached the agreement:

https://lore.kernel.org/linux-integrity/CAHC9VhTTKac1o=RnQadu2xqdeKH8C_F+Wh4sY=HkGbCArwc8JQ@mail.gmail.com/

Thanks

Roberto

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Song Liu]

Hi Roberto, 

Thanks for sharing these information!

> On Dec 19, 2024, at 7:40 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:

[...]

>> I didn't know about this history until today. I apologize if this 
>> RFC/PATCH is moving to the direction against the original agreement. 
>> I didn't mean to break any agreement. 
>> 
>> My motivation is actually the per inode memory consumption of IMA 
>> and EVM. Once enabled, EVM appends a whole struct evm_iint_cache to 
>> each inode via i_security. IMA is better on memory consumption, as 
>> it only adds a pointer to i_security. 
>> 
>> It appears to me that a way to disable IMA and EVM at boot time can 
>> be useful, especially for distro kernels. But I guess there are 
>> reasons to not allow this (thus the earlier agreement). Could you 
>> please share your thoughts on this?
> 
> Hi Song
> 
> IMA/EVM cannot be always disabled for two reasons: (1) for secure and
> trusted boot, IMA is expected to enforce architecture-specific
> policies; (2) accidentally disabling them will cause modified files to
> be rejected when IMA/EVM are turned on again.
> 
> If the requirements above are met, we are fine on disabling IMA/EVM.

I probably missed something, but it appears to me IMA/EVM might be 
enabled in distro kernels, but the distro by default does not 
configure IMA/EVM, so they are not actually used. Did I misunderstand 
something?

> As for reserving space in the inode security blob, please refer to this
> discussion, where we reached the agreement:
> 
> https://lore.kernel.org/linux-integrity/CAHC9VhTTKac1o=RnQadu2xqdeKH8C_F+Wh4sY=HkGbCArwc8JQ@mail.gmail.com/

AFAICT, the benefit of i_security storage is its ability to be 
configured at boot time. If IMA/EVM cannot be disabled, it is 
better to add them to struct inode within a "#ifdef CONFIG_"
block. 

Thanks,
Song

Re: [RFC 0/2] ima: evm: Add kernel cmdline options to disable IMA/EVM

[Mimi Zohar]

On Thu, 2024-12-19 at 17:46 +0000, Song Liu wrote:
> Hi Roberto, 
> 
> Thanks for sharing these information!
> 
> > On Dec 19, 2024, at 7:40 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
> 
> [...]
> 
> > > I didn't know about this history until today. I apologize if this 
> > > RFC/PATCH is moving to the direction against the original agreement. 
> > > I didn't mean to break any agreement. 
> > > 
> > > My motivation is actually the per inode memory consumption of IMA 
> > > and EVM. Once enabled, EVM appends a whole struct evm_iint_cache to 
> > > each inode via i_security. IMA is better on memory consumption, as 
> > > it only adds a pointer to i_security. 
> > > 
> > > It appears to me that a way to disable IMA and EVM at boot time can 
> > > be useful, especially for distro kernels. But I guess there are 
> > > reasons to not allow this (thus the earlier agreement). Could you 
> > > please share your thoughts on this?
> > 
> > Hi Song
> > 
> > IMA/EVM cannot be always disabled for two reasons: (1) for secure and
> > trusted boot, IMA is expected to enforce architecture-specific
> > policies; (2) accidentally disabling them will cause modified files to
> > be rejected when IMA/EVM are turned on again.
> > 
> > If the requirements above are met, we are fine on disabling IMA/EVM.
> 
> I probably missed something, but it appears to me IMA/EVM might be 
> enabled in distro kernels, but the distro by default does not 
> configure IMA/EVM, so they are not actually used. Did I misunderstand 
> something?

If "CONFIG_IMA_ARCH_POLICY" is configured, then the architecture specific policy
is configured and loaded on boot.  For x86 and arm, the architecture specific
policy rules are defined in ima_efi.c.  On power, the rules are defined in
arch/powerpc/kernel/ima_arch.c.  On most systems, the currently enabled IMA
policy rules can be viewed by cat'ing <securityfs>/integrity/ima/policy.

For more information on IMA policies, refer to
https://ima-doc.readthedocs.io/en/latest/ima-policy.html#

Mimi

> 
> > As for reserving space in the inode security blob, please refer to this
> > discussion, where we reached the agreement:
> > 
> > https://lore.kernel.org/linux-integrity/CAHC9VhTTKac1o=RnQadu2xqdeKH8C_F+Wh4sY=HkGbCArwc8JQ@mail.gmail.com/
> 
> AFAICT, the benefit of i_security storage is its ability to be 
> configured at boot time. If IMA/EVM cannot be disabled, it is 
> better to add them to struct inode within a "#ifdef CONFIG_"
> block. 
> 
> Thanks,
> Song
>