kexec.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] ima: add a knob to make IMA be able to be disabled
@ 2025-03-31  6:16 Baoquan He
  2025-03-31  6:22 ` Paul Menzel
  2025-03-31 12:15 ` Mimi Zohar
  0 siblings, 2 replies; 22+ messages in thread
From: Baoquan He @ 2025-03-31  6:16 UTC (permalink / raw)
  To: zohar; +Cc: linux-integrity, kexec, Baoquan He

It doesn't make sense to run IMA functionality in kdump kernel, and that
will cost extra memory. It would be great to allow IMA to be disabled on
purpose, e.g for kdump kernel.

Hence add a knob here to allow people to disable IMA if needed.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 security/integrity/ima/ima_main.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 28b8b0db6f9b..5d677d1389fe 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -38,11 +38,27 @@ int ima_appraise;
 
 int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
 static int hash_setup_done;
+static int ima_disabled = 0;
 
 static struct notifier_block ima_lsm_policy_notifier = {
 	.notifier_call = ima_lsm_policy_change,
 };
 
+static int __init ima_setup(char *str)
+{
+	if (strncmp(str, "off", 3) == 0)
+                ima_disabled = 1;
+        else if (strncmp(str, "on", 2) == 0)
+                ima_disabled = 0;
+        else
+                pr_err("invalid ima setup option: \"%s\" ", str);
+
+	return 1;
+}
+__setup("ima=", ima_setup);
+
+
+
 static int __init hash_setup(char *str)
 {
 	struct ima_template_desc *template_desc = ima_template_desc_current();
@@ -1176,6 +1192,11 @@ static int __init init_ima(void)
 {
 	int error;
 
+	if (ima_disabled) {
+		pr_info("IMA functionality is disabled on purpose!");
+		return 0;
+	}
+
 	ima_appraise_parse_cmdline();
 	ima_init_template_list();
 	hash_setup(CONFIG_IMA_DEFAULT_HASH);
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-03-31  6:16 [RFC PATCH] ima: add a knob to make IMA be able to be disabled Baoquan He
@ 2025-03-31  6:22 ` Paul Menzel
  2025-03-31  8:21   ` Baoquan He
  2025-03-31 12:15 ` Mimi Zohar
  1 sibling, 1 reply; 22+ messages in thread
From: Paul Menzel @ 2025-03-31  6:22 UTC (permalink / raw)
  To: Baoquan He; +Cc: zohar, linux-integrity, kexec

Dear Baoquan,


Thank you for your patch. I’d add the knob name to the commit message 
summary/title, so it shows up in `git log --oneline`.

Am 31.03.25 um 08:16 schrieb Baoquan He:
> It doesn't make sense to run IMA functionality in kdump kernel, and that
> will cost extra memory. It would be great to allow IMA to be disabled on
> purpose, e.g for kdump kernel.
> 
> Hence add a knob here to allow people to disable IMA if needed.

`initcall_blacklist=…` could be used already. I prefer a dedicated 
parameter too though.

> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>   security/integrity/ima/ima_main.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 28b8b0db6f9b..5d677d1389fe 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -38,11 +38,27 @@ int ima_appraise;
>   
>   int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
>   static int hash_setup_done;
> +static int ima_disabled = 0;
>   
>   static struct notifier_block ima_lsm_policy_notifier = {
>   	.notifier_call = ima_lsm_policy_change,
>   };
>   
> +static int __init ima_setup(char *str)
> +{
> +	if (strncmp(str, "off", 3) == 0)
> +                ima_disabled = 1;
> +        else if (strncmp(str, "on", 2) == 0)
> +                ima_disabled = 0;
> +        else
> +                pr_err("invalid ima setup option: \"%s\" ", str);

I’d add the allowed strings.

> +
> +	return 1;
> +}
> +__setup("ima=", ima_setup);
> +
> +
> +
>   static int __init hash_setup(char *str)
>   {
>   	struct ima_template_desc *template_desc = ima_template_desc_current();
> @@ -1176,6 +1192,11 @@ static int __init init_ima(void)
>   {
>   	int error;
>   
> +	if (ima_disabled) {
> +		pr_info("IMA functionality is disabled on purpose!");

… on Linux CLI.

> +		return 0;
> +	}
> +
>   	ima_appraise_parse_cmdline();
>   	ima_init_template_list();
>   	hash_setup(CONFIG_IMA_DEFAULT_HASH);


Kind regards,

Paul


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-03-31  6:22 ` Paul Menzel
@ 2025-03-31  8:21   ` Baoquan He
  0 siblings, 0 replies; 22+ messages in thread
From: Baoquan He @ 2025-03-31  8:21 UTC (permalink / raw)
  To: Paul Menzel; +Cc: zohar, linux-integrity, kexec

Hi Paul,

On 03/31/25 at 08:22am, Paul Menzel wrote:
> 
Thanks for your careful reviewing.
> 
> Thank you for your patch. I’d add the knob name to the commit message
> summary/title, so it shows up in `git log --oneline`.

Sounds great, will change.

> 
> Am 31.03.25 um 08:16 schrieb Baoquan He:
> > It doesn't make sense to run IMA functionality in kdump kernel, and that
> > will cost extra memory. It would be great to allow IMA to be disabled on
> > purpose, e.g for kdump kernel.
> > 
> > Hence add a knob here to allow people to disable IMA if needed.
> 
> `initcall_blacklist=…` could be used already. I prefer a dedicated parameter
> too though.

Yes, adding parameter can provide an explicit functionality to the
feature. While 'initcall_blacklist=' can't guarantee there won't be
dependency or connection between ima and other feature, and people could
add or change the connection anytime when userspace is using it but not
knowing the change.

> 
> > Signed-off-by: Baoquan He <bhe@redhat.com>
> > ---
> >   security/integrity/ima/ima_main.c | 21 +++++++++++++++++++++
> >   1 file changed, 21 insertions(+)
> > 
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 28b8b0db6f9b..5d677d1389fe 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -38,11 +38,27 @@ int ima_appraise;
> >   int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
> >   static int hash_setup_done;
> > +static int ima_disabled = 0;
> >   static struct notifier_block ima_lsm_policy_notifier = {
> >   	.notifier_call = ima_lsm_policy_change,
> >   };
> > +static int __init ima_setup(char *str)
> > +{
> > +	if (strncmp(str, "off", 3) == 0)
> > +                ima_disabled = 1;
> > +        else if (strncmp(str, "on", 2) == 0)
> > +                ima_disabled = 0;
> > +        else
> > +                pr_err("invalid ima setup option: \"%s\" ", str);
> 
> I’d add the allowed strings.

Sounds better, will change.

> 
> > +
> > +	return 1;
> > +}
> > +__setup("ima=", ima_setup);
> > +
> > +
> > +
> >   static int __init hash_setup(char *str)
> >   {
> >   	struct ima_template_desc *template_desc = ima_template_desc_current();
> > @@ -1176,6 +1192,11 @@ static int __init init_ima(void)
> >   {
> >   	int error;
> > +	if (ima_disabled) {
> > +		pr_info("IMA functionality is disabled on purpose!");
> 
> … on Linux CLI.

I may not get the suggestion in this place, could you be more specific?

> 
> > +		return 0;
> > +	}
> > +
> >   	ima_appraise_parse_cmdline();
> >   	ima_init_template_list();
> >   	hash_setup(CONFIG_IMA_DEFAULT_HASH);
> 
> 
> Kind regards,
> 
> Paul
> 



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-03-31  6:16 [RFC PATCH] ima: add a knob to make IMA be able to be disabled Baoquan He
  2025-03-31  6:22 ` Paul Menzel
@ 2025-03-31 12:15 ` Mimi Zohar
  2025-04-02  1:38   ` Coiby Xu
  1 sibling, 1 reply; 22+ messages in thread
From: Mimi Zohar @ 2025-03-31 12:15 UTC (permalink / raw)
  To: Baoquan He; +Cc: linux-integrity, kexec

On Mon, 2025-03-31 at 14:16 +0800, Baoquan He wrote:
> It doesn't make sense to run IMA functionality in kdump kernel, and that
> will cost extra memory. It would be great to allow IMA to be disabled on
> purpose, e.g for kdump kernel.
> 
> Hence add a knob here to allow people to disable IMA if needed.
> 
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  security/integrity/ima/ima_main.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 28b8b0db6f9b..5d677d1389fe 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -38,11 +38,27 @@ int ima_appraise;
>  
>  int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
>  static int hash_setup_done;
> +static int ima_disabled = 0;
>  
>  static struct notifier_block ima_lsm_policy_notifier = {
>  	.notifier_call = ima_lsm_policy_change,
>  };
>  
> +static int __init ima_setup(char *str)
> +{
> +	if (strncmp(str, "off", 3) == 0)
> +                ima_disabled = 1;
> +        else if (strncmp(str, "on", 2) == 0)
> +                ima_disabled = 0;
> +        else
> +                pr_err("invalid ima setup option: \"%s\" ", str);
> +
> +	return 1;
> +}
> +__setup("ima=", ima_setup);

I understand your wanting to disable IMA for Kdump, but this goes way beyond
that.  Please don't make it generic like this.

Please refer to ima_appraise_parse_cmdline().

Mimi

> +
> +
> +
>  static int __init hash_setup(char *str)
>  {
>  	struct ima_template_desc *template_desc = ima_template_desc_current();
> @@ -1176,6 +1192,11 @@ static int __init init_ima(void)
>  {
>  	int error;
>  
> +	if (ima_disabled) {
> +		pr_info("IMA functionality is disabled on purpose!");
> +		return 0;
> +	}
> +
>  	ima_appraise_parse_cmdline();
>  	ima_init_template_list();
>  	hash_setup(CONFIG_IMA_DEFAULT_HASH);



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-03-31 12:15 ` Mimi Zohar
@ 2025-04-02  1:38   ` Coiby Xu
  2025-04-02  1:47     ` RuiRui Yang
  0 siblings, 1 reply; 22+ messages in thread
From: Coiby Xu @ 2025-04-02  1:38 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: Baoquan He, linux-integrity, kexec

On Mon, Mar 31, 2025 at 08:15:08AM -0400, Mimi Zohar wrote:
>On Mon, 2025-03-31 at 14:16 +0800, Baoquan He wrote:
>> It doesn't make sense to run IMA functionality in kdump kernel, and that
>> will cost extra memory. It would be great to allow IMA to be disabled on
>> purpose, e.g for kdump kernel.
>>
>> Hence add a knob here to allow people to disable IMA if needed.
>>
>> Signed-off-by: Baoquan He <bhe@redhat.com>
>> ---
>>  security/integrity/ima/ima_main.c | 21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
>> index 28b8b0db6f9b..5d677d1389fe 100644
>> --- a/security/integrity/ima/ima_main.c
>> +++ b/security/integrity/ima/ima_main.c
>> @@ -38,11 +38,27 @@ int ima_appraise;
>>
>>  int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
>>  static int hash_setup_done;
>> +static int ima_disabled = 0;
>>
>>  static struct notifier_block ima_lsm_policy_notifier = {
>>  	.notifier_call = ima_lsm_policy_change,
>>  };
>>
>> +static int __init ima_setup(char *str)
>> +{
>> +	if (strncmp(str, "off", 3) == 0)
>> +                ima_disabled = 1;
>> +        else if (strncmp(str, "on", 2) == 0)
>> +                ima_disabled = 0;
>> +        else
>> +                pr_err("invalid ima setup option: \"%s\" ", str);
>> +
>> +	return 1;
>> +}
>> +__setup("ima=", ima_setup);
>
>I understand your wanting to disable IMA for Kdump, but this goes way beyond
>that.  Please don't make it generic like this.
>
>Please refer to ima_appraise_parse_cmdline().

Hi Mimi,

To save memory for kdump, it seems init_ima has been to be skipped thus
ima=off is necessary (ima_appraise=off won't serve the purpose). Or do
you have any specific concerns in mind?

>
>Mimi
>
>> +
>> +
>> +
>>  static int __init hash_setup(char *str)
>>  {
>>  	struct ima_template_desc *template_desc = ima_template_desc_current();
>> @@ -1176,6 +1192,11 @@ static int __init init_ima(void)
>>  {
>>  	int error;
>>
>> +	if (ima_disabled) {
>> +		pr_info("IMA functionality is disabled on purpose!");
>> +		return 0;
>> +	}
>> +
>>  	ima_appraise_parse_cmdline();
>>  	ima_init_template_list();
>>  	hash_setup(CONFIG_IMA_DEFAULT_HASH);
>
>

-- 
Best regards,
Coiby



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-04-02  1:38   ` Coiby Xu
@ 2025-04-02  1:47     ` RuiRui Yang
  2025-04-02  3:30       ` Mimi Zohar
  0 siblings, 1 reply; 22+ messages in thread
From: RuiRui Yang @ 2025-04-02  1:47 UTC (permalink / raw)
  To: Coiby Xu; +Cc: Mimi Zohar, Baoquan He, linux-integrity, kexec

On Wed, 2 Apr 2025 at 09:41, Coiby Xu <coxu@redhat.com> wrote:
>
> On Mon, Mar 31, 2025 at 08:15:08AM -0400, Mimi Zohar wrote:
> >On Mon, 2025-03-31 at 14:16 +0800, Baoquan He wrote:
> >> It doesn't make sense to run IMA functionality in kdump kernel, and that
> >> will cost extra memory. It would be great to allow IMA to be disabled on
> >> purpose, e.g for kdump kernel.
> >>
> >> Hence add a knob here to allow people to disable IMA if needed.
> >>
> >> Signed-off-by: Baoquan He <bhe@redhat.com>
> >> ---
> >>  security/integrity/ima/ima_main.c | 21 +++++++++++++++++++++
> >>  1 file changed, 21 insertions(+)
> >>
> >> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> >> index 28b8b0db6f9b..5d677d1389fe 100644
> >> --- a/security/integrity/ima/ima_main.c
> >> +++ b/security/integrity/ima/ima_main.c
> >> @@ -38,11 +38,27 @@ int ima_appraise;
> >>
> >>  int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
> >>  static int hash_setup_done;
> >> +static int ima_disabled = 0;
> >>
> >>  static struct notifier_block ima_lsm_policy_notifier = {
> >>      .notifier_call = ima_lsm_policy_change,
> >>  };
> >>
> >> +static int __init ima_setup(char *str)
> >> +{
> >> +    if (strncmp(str, "off", 3) == 0)
> >> +                ima_disabled = 1;
> >> +        else if (strncmp(str, "on", 2) == 0)
> >> +                ima_disabled = 0;
> >> +        else
> >> +                pr_err("invalid ima setup option: \"%s\" ", str);
> >> +
> >> +    return 1;
> >> +}
> >> +__setup("ima=", ima_setup);
> >
> >I understand your wanting to disable IMA for Kdump, but this goes way beyond
> >that.  Please don't make it generic like this.
> >
> >Please refer to ima_appraise_parse_cmdline().
>
> Hi Mimi,
>
> To save memory for kdump, it seems init_ima has been to be skipped thus
> ima=off is necessary (ima_appraise=off won't serve the purpose). Or do
> you have any specific concerns in mind?

I think as Mimi said see below logic enforces the IMA even with the
cmdline disabling, see ima_appraise_parse_cmdline:
if (sb_state) {
                if (!(appraisal_state & IMA_APPRAISE_ENFORCE))
                        pr_info("Secure boot enabled: ignoring
ima_appraise=%s option",
                                str);
        } else {
                ima_appraise = appraisal_state;
        }


>
> >
> >Mimi
> >
> >> +
> >> +
> >> +
> >>  static int __init hash_setup(char *str)
> >>  {
> >>      struct ima_template_desc *template_desc = ima_template_desc_current();
> >> @@ -1176,6 +1192,11 @@ static int __init init_ima(void)
> >>  {
> >>      int error;
> >>
> >> +    if (ima_disabled) {
> >> +            pr_info("IMA functionality is disabled on purpose!");
> >> +            return 0;
> >> +    }
> >> +
> >>      ima_appraise_parse_cmdline();
> >>      ima_init_template_list();
> >>      hash_setup(CONFIG_IMA_DEFAULT_HASH);
> >
> >
>
> --
> Best regards,
> Coiby
>
>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-04-02  1:47     ` RuiRui Yang
@ 2025-04-02  3:30       ` Mimi Zohar
  2025-04-02  8:43         ` Coiby Xu
  0 siblings, 1 reply; 22+ messages in thread
From: Mimi Zohar @ 2025-04-02  3:30 UTC (permalink / raw)
  To: RuiRui Yang, Coiby Xu; +Cc: Baoquan He, linux-integrity, kexec

On Wed, 2025-04-02 at 09:47 +0800, RuiRui Yang wrote:
> On Wed, 2 Apr 2025 at 09:41, Coiby Xu <coxu@redhat.com> wrote:
> > 
> > On Mon, Mar 31, 2025 at 08:15:08AM -0400, Mimi Zohar wrote:
> > > On Mon, 2025-03-31 at 14:16 +0800, Baoquan He wrote:
> > > > It doesn't make sense to run IMA functionality in kdump kernel, and that
> > > > will cost extra memory. It would be great to allow IMA to be disabled on
> > > > purpose, e.g for kdump kernel.
> > > > 
> > > > Hence add a knob here to allow people to disable IMA if needed.
> > > > 
> > > > Signed-off-by: Baoquan He <bhe@redhat.com>
> > > > ---
> > > >  security/integrity/ima/ima_main.c | 21 +++++++++++++++++++++
> > > >  1 file changed, 21 insertions(+)
> > > > 
> > > > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > > > index 28b8b0db6f9b..5d677d1389fe 100644
> > > > --- a/security/integrity/ima/ima_main.c
> > > > +++ b/security/integrity/ima/ima_main.c
> > > > @@ -38,11 +38,27 @@ int ima_appraise;
> > > > 
> > > >  int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
> > > >  static int hash_setup_done;
> > > > +static int ima_disabled = 0;
> > > > 
> > > >  static struct notifier_block ima_lsm_policy_notifier = {
> > > >      .notifier_call = ima_lsm_policy_change,
> > > >  };
> > > > 
> > > > +static int __init ima_setup(char *str)
> > > > +{
> > > > +    if (strncmp(str, "off", 3) == 0)
> > > > +                ima_disabled = 1;
> > > > +        else if (strncmp(str, "on", 2) == 0)
> > > > +                ima_disabled = 0;
> > > > +        else
> > > > +                pr_err("invalid ima setup option: \"%s\" ", str);
> > > > +
> > > > +    return 1;
> > > > +}
> > > > +__setup("ima=", ima_setup);
> > > 
> > > I understand your wanting to disable IMA for Kdump, but this goes way beyond
> > > that.  Please don't make it generic like this.
> > > 
> > > Please refer to ima_appraise_parse_cmdline().
> > 
> > Hi Mimi,
> > 
> > To save memory for kdump, it seems init_ima has been to be skipped thus
> > ima=off is necessary (ima_appraise=off won't serve the purpose). Or do
> > you have any specific concerns in mind?
> 
> I think as Mimi said see below logic enforces the IMA even with the
> cmdline disabling, see ima_appraise_parse_cmdline:
> if (sb_state) {
>                 if (!(appraisal_state & IMA_APPRAISE_ENFORCE))
>                         pr_info("Secure boot enabled: ignoring
> ima_appraise=%s option",
>                                 str);
>         } else {
>                 ima_appraise = appraisal_state;
>         }

Thanks, RuiRui.

Coiby, would disabling just IMA-measurement, as opposed to IMA-appraisal, save
sufficient memory for kdump?

> > > 
> > > > +
> > > > +
> > > > +
> > > >  static int __init hash_setup(char *str)
> > > >  {
> > > >      struct ima_template_desc *template_desc = ima_template_desc_current();
> > > > @@ -1176,6 +1192,11 @@ static int __init init_ima(void)
> > > >  {
> > > >      int error;
> > > > 
> > > > +    if (ima_disabled) {
> > > > +            pr_info("IMA functionality is disabled on purpose!");
> > > > +            return 0;
> > > > +    }
> > > > +
> > > >      ima_appraise_parse_cmdline();
> > > >      ima_init_template_list();
> > > >      hash_setup(CONFIG_IMA_DEFAULT_HASH);
> > > 
> > > 
> > 
> > --
> > Best regards,
> > Coiby
> > 
> > 
> 
> 



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-04-02  3:30       ` Mimi Zohar
@ 2025-04-02  8:43         ` Coiby Xu
  2025-04-02 11:25           ` Mimi Zohar
  2025-04-02 11:49           ` Baoquan He
  0 siblings, 2 replies; 22+ messages in thread
From: Coiby Xu @ 2025-04-02  8:43 UTC (permalink / raw)
  To: Mimi Zohar, RuiRui Yang; +Cc: Baoquan He, linux-integrity, kexec

On Tue, Apr 01, 2025 at 11:30:09PM -0400, Mimi Zohar wrote:
>On Wed, 2025-04-02 at 09:47 +0800, RuiRui Yang wrote:
[...]
>> > > that.  Please don't make it generic like this.
>> > >
>> > > Please refer to ima_appraise_parse_cmdline().
>> >
>> > Hi Mimi,
>> >
>> > To save memory for kdump, it seems init_ima has been to be skipped thus
>> > ima=off is necessary (ima_appraise=off won't serve the purpose). Or do
>> > you have any specific concerns in mind?
>>
>> I think as Mimi said see below logic enforces the IMA even with the
>> cmdline disabling, see ima_appraise_parse_cmdline:
>> if (sb_state) {
>>                 if (!(appraisal_state & IMA_APPRAISE_ENFORCE))
>>                         pr_info("Secure boot enabled: ignoring
>> ima_appraise=%s option",
>>                                 str);
>>         } else {
>>                 ima_appraise = appraisal_state;
>>         }

Thanks for pointing me to the above code! Note with the whole IMA
disabled as done by this patch, the above code will not run so IMA
(appraisal) won't be enforced.

>
>Thanks, RuiRui.
>

Mimi, so do I understand it correctly that your want IMA-appraisal to be
always enabled as long as secure boot is enabled even if users choose to
disable IMA? I wonder what security issue will it bring if this promise
gets broken considering other LSMs can SELinux can be disabled when
secure boot is enabled?

>Coiby, would disabling just IMA-measurement, as opposed to IMA-appraisal, save
>sufficient memory for kdump?

For disabling just IMA-measurement, do you mean not enabling any measure
rules?  The more memory reserved for the kdump kernel, the less memory
can be used by the 1st kernel. So from the perfective of kdump, we try
to make the memory footprint as smaller as possible. 

Baoquan, do you have any statistics about the memory overhead of IMA?


-- 
Best regards,
Coiby



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-04-02  8:43         ` Coiby Xu
@ 2025-04-02 11:25           ` Mimi Zohar
  2025-04-02 11:49           ` Baoquan He
  1 sibling, 0 replies; 22+ messages in thread
From: Mimi Zohar @ 2025-04-02 11:25 UTC (permalink / raw)
  To: Coiby Xu, RuiRui Yang; +Cc: Baoquan He, linux-integrity, kexec

On Wed, 2025-04-02 at 16:43 +0800, Coiby Xu wrote:
> > Coiby, would disabling just IMA-measurement, as opposed to IMA-appraisal, save
> > sufficient memory for kdump?
> 
> For disabling just IMA-measurement, do you mean not enabling any measure
> rules?  The more memory reserved for the kdump kernel, the less memory
> can be used by the 1st kernel. So from the perfective of kdump, we try
> to make the memory footprint as smaller as possible.

Yes, not enabling any "measure" rules and not restoring the kexec measurement
list.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-04-02  8:43         ` Coiby Xu
  2025-04-02 11:25           ` Mimi Zohar
@ 2025-04-02 11:49           ` Baoquan He
  2025-04-03 20:03             ` Mimi Zohar
  1 sibling, 1 reply; 22+ messages in thread
From: Baoquan He @ 2025-04-02 11:49 UTC (permalink / raw)
  To: Coiby Xu; +Cc: Mimi Zohar, RuiRui Yang, linux-integrity, kexec

On 04/02/25 at 04:43pm, Coiby Xu wrote:
> On Tue, Apr 01, 2025 at 11:30:09PM -0400, Mimi Zohar wrote:
> > On Wed, 2025-04-02 at 09:47 +0800, RuiRui Yang wrote:
> [...]
> > > > > that.  Please don't make it generic like this.
> > > > >
> > > > > Please refer to ima_appraise_parse_cmdline().
> > > >
> > > > Hi Mimi,
> > > >
> > > > To save memory for kdump, it seems init_ima has been to be skipped thus
> > > > ima=off is necessary (ima_appraise=off won't serve the purpose). Or do
> > > > you have any specific concerns in mind?
> > > 
> > > I think as Mimi said see below logic enforces the IMA even with the
> > > cmdline disabling, see ima_appraise_parse_cmdline:
> > > if (sb_state) {
> > >                 if (!(appraisal_state & IMA_APPRAISE_ENFORCE))
> > >                         pr_info("Secure boot enabled: ignoring
> > > ima_appraise=%s option",
> > >                                 str);
> > >         } else {
> > >                 ima_appraise = appraisal_state;
> > >         }
> 
> Thanks for pointing me to the above code! Note with the whole IMA
> disabled as done by this patch, the above code will not run so IMA
> (appraisal) won't be enforced.
> 
> > 
> > Thanks, RuiRui.
> > 
> 
> Mimi, so do I understand it correctly that your want IMA-appraisal to be
> always enabled as long as secure boot is enabled even if users choose to
> disable IMA? I wonder what security issue will it bring if this promise
> gets broken considering other LSMs can SELinux can be disabled when
> secure boot is enabled?
> 
> > Coiby, would disabling just IMA-measurement, as opposed to IMA-appraisal, save
> > sufficient memory for kdump?
> 
> For disabling just IMA-measurement, do you mean not enabling any measure
> rules?  The more memory reserved for the kdump kernel, the less memory
> can be used by the 1st kernel. So from the perfective of kdump, we try
> to make the memory footprint as smaller as possible.
> 
> Baoquan, do you have any statistics about the memory overhead of IMA?

I am getting a system to check that. I think there are two aspects of
IMA functionality we want to disable. One is disable the IMA-measurement
copying from 1st kernel to 2nd kernel, this is only needed by kexec
reboot; the other is IMA is not needed at all in kdump kernel, means we
don't want to call ima_init() to initialize
ima_keyring/crypto/template/digests/fs etc. 

With my shallow knowledge about IMA, I don't know how to imitate
appraisal cmdline to disable IMA partially in kdump kernel case.

One exmaple is 'cgroup_disable=memory' we have been doing to add into
kdump cmdline because mem_cgroup is not needed at all for kdump kernel.
We want to achieve that effect.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-04-02 11:49           ` Baoquan He
@ 2025-04-03 20:03             ` Mimi Zohar
  2025-04-07  1:34               ` Baoquan He
  0 siblings, 1 reply; 22+ messages in thread
From: Mimi Zohar @ 2025-04-03 20:03 UTC (permalink / raw)
  To: Baoquan He, Coiby Xu; +Cc: RuiRui Yang, linux-integrity, kexec

On Wed, 2025-04-02 at 19:49 +0800, Baoquan He wrote:
> On 04/02/25 at 04:43pm, Coiby Xu wrote:
> > On Tue, Apr 01, 2025 at 11:30:09PM -0400, Mimi Zohar wrote:
> > > On Wed, 2025-04-02 at 09:47 +0800, RuiRui Yang wrote:
> > [...]
> > > > > > that.  Please don't make it generic like this.
> > > > > > 
> > > > > > Please refer to ima_appraise_parse_cmdline().
> > > > > 
> > > > > Hi Mimi,
> > > > > 
> > > > > To save memory for kdump, it seems init_ima has been to be skipped thus
> > > > > ima=off is necessary (ima_appraise=off won't serve the purpose). Or do
> > > > > you have any specific concerns in mind?
> > > > 
> > > > I think as Mimi said see below logic enforces the IMA even with the
> > > > cmdline disabling, see ima_appraise_parse_cmdline:
> > > > if (sb_state) {
> > > >                 if (!(appraisal_state & IMA_APPRAISE_ENFORCE))
> > > >                         pr_info("Secure boot enabled: ignoring
> > > > ima_appraise=%s option",
> > > >                                 str);
> > > >         } else {
> > > >                 ima_appraise = appraisal_state;
> > > >         }
> > 
> > Thanks for pointing me to the above code! Note with the whole IMA
> > disabled as done by this patch, the above code will not run so IMA
> > (appraisal) won't be enforced.
> > 
> > > 
> > > Thanks, RuiRui.
> > > 
> > 
> > Mimi, so do I understand it correctly that your want IMA-appraisal to be
> > always enabled as long as secure boot is enabled even if users choose to
> > disable IMA? 

Secure boot is not the only reason.  Based on policy IMA-appraisal and EVM
calculate and store file hashes and HMAC's in their respective security xattrs.
Normally the usage of file hashes and HMAC's is limited to mutable files. 
Disabling IMA-appraisal could result in not properly updating the security
xattrs, which would result in not being able to verify the file's integrity on
reboot.

On systems where the RPM includes file signatures, file signatures of immutable
files can be safely restored.  Although it is possible to walk the filesystem(s)
"fixing" the xattrs of mutable files, it defeats the purpose.  "fix" mode should
only be enabled in a trusted environment.

> > I wonder what security issue will it bring if this promise
> > gets broken considering other LSMs can SELinux can be disabled when
> > secure boot is enabled?

The builtin IMA policy rules are not defined in terms of SELinux labels.  If the
initial IMA custom policy defines rules based on SELinux labels and SELinux is
not enabled, the policy will fail to be loaded.

> > > Coiby, would disabling just IMA-measurement, as opposed to IMA-appraisal, save
> > > sufficient memory for kdump?
> > 
> > For disabling just IMA-measurement, do you mean not enabling any measure
> > rules?  The more memory reserved for the kdump kernel, the less memory
> > can be used by the 1st kernel. So from the perfective of kdump, we try
> > to make the memory footprint as smaller as possible.

Got it.

> > Baoquan, do you have any statistics about the memory overhead of IMA?
> 
> I am getting a system to check that. I think there are two aspects of
> IMA functionality we want to disable. One is disable the IMA-measurement
> copying from 1st kernel to 2nd kernel, this is only needed by kexec
> reboot; the other is IMA is not needed at all in kdump kernel, means we
> don't want to call ima_init() to initialize
> ima_keyring/crypto/template/digests/fs etc. 
> 
> With my shallow knowledge about IMA, I don't know how to imitate
> appraisal cmdline to disable IMA partially in kdump kernel case.

The IMA policy controls how much or how little IMA measures and appraises.  Most
of the memory usage is the IMA measurement list, itself, and the per file cache
info.  (The per file cache info limits re-measuring or re-appraising files.)

Similarly my knowledge of kdump is very limited.  Is there a way for the kernel
to differentiate between kexec and kdump?  If we need a mechanism to disable
IMA-measurement, I'd *really* prefer it be limited to kdump.

thanks,

Mimi

> 
> One exmaple is 'cgroup_disable=memory' we have been doing to add into
> kdump cmdline because mem_cgroup is not needed at all for kdump kernel.
> We want to achieve that effect.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-04-03 20:03             ` Mimi Zohar
@ 2025-04-07  1:34               ` Baoquan He
  2025-04-07 11:46                 ` Mimi Zohar
  0 siblings, 1 reply; 22+ messages in thread
From: Baoquan He @ 2025-04-07  1:34 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: Coiby Xu, RuiRui Yang, linux-integrity, kexec

On 04/03/25 at 04:03pm, Mimi Zohar wrote:
> On Wed, 2025-04-02 at 19:49 +0800, Baoquan He wrote:
> > On 04/02/25 at 04:43pm, Coiby Xu wrote:
> > > On Tue, Apr 01, 2025 at 11:30:09PM -0400, Mimi Zohar wrote:
> > > > On Wed, 2025-04-02 at 09:47 +0800, RuiRui Yang wrote:
> > > [...]
> > > > > > > that.  Please don't make it generic like this.
> > > > > > > 
> > > > > > > Please refer to ima_appraise_parse_cmdline().
> > > > > > 
> > > > > > Hi Mimi,
> > > > > > 
> > > > > > To save memory for kdump, it seems init_ima has been to be skipped thus
> > > > > > ima=off is necessary (ima_appraise=off won't serve the purpose). Or do
> > > > > > you have any specific concerns in mind?
> > > > > 
> > > > > I think as Mimi said see below logic enforces the IMA even with the
> > > > > cmdline disabling, see ima_appraise_parse_cmdline:
> > > > > if (sb_state) {
> > > > >                 if (!(appraisal_state & IMA_APPRAISE_ENFORCE))
> > > > >                         pr_info("Secure boot enabled: ignoring
> > > > > ima_appraise=%s option",
> > > > >                                 str);
> > > > >         } else {
> > > > >                 ima_appraise = appraisal_state;
> > > > >         }
> > > 
> > > Thanks for pointing me to the above code! Note with the whole IMA
> > > disabled as done by this patch, the above code will not run so IMA
> > > (appraisal) won't be enforced.
> > > 
> > > > 
> > > > Thanks, RuiRui.
> > > > 
> > > 
> > > Mimi, so do I understand it correctly that your want IMA-appraisal to be
> > > always enabled as long as secure boot is enabled even if users choose to
> > > disable IMA? 
> 
> Secure boot is not the only reason.  Based on policy IMA-appraisal and EVM
> calculate and store file hashes and HMAC's in their respective security xattrs.
> Normally the usage of file hashes and HMAC's is limited to mutable files. 
> Disabling IMA-appraisal could result in not properly updating the security
> xattrs, which would result in not being able to verify the file's integrity on
> reboot.
> 
> On systems where the RPM includes file signatures, file signatures of immutable
> files can be safely restored.  Although it is possible to walk the filesystem(s)
> "fixing" the xattrs of mutable files, it defeats the purpose.  "fix" mode should
> only be enabled in a trusted environment.
> 
> > > I wonder what security issue will it bring if this promise
> > > gets broken considering other LSMs can SELinux can be disabled when
> > > secure boot is enabled?
> 
> The builtin IMA policy rules are not defined in terms of SELinux labels.  If the
> initial IMA custom policy defines rules based on SELinux labels and SELinux is
> not enabled, the policy will fail to be loaded.
> 
> > > > Coiby, would disabling just IMA-measurement, as opposed to IMA-appraisal, save
> > > > sufficient memory for kdump?
> > > 
> > > For disabling just IMA-measurement, do you mean not enabling any measure
> > > rules?  The more memory reserved for the kdump kernel, the less memory
> > > can be used by the 1st kernel. So from the perfective of kdump, we try
> > > to make the memory footprint as smaller as possible.
> 
> Got it.
> 
> > > Baoquan, do you have any statistics about the memory overhead of IMA?
> > 
> > I am getting a system to check that. I think there are two aspects of
> > IMA functionality we want to disable. One is disable the IMA-measurement
> > copying from 1st kernel to 2nd kernel, this is only needed by kexec
> > reboot; the other is IMA is not needed at all in kdump kernel, means we
> > don't want to call ima_init() to initialize
> > ima_keyring/crypto/template/digests/fs etc. 
> > 
> > With my shallow knowledge about IMA, I don't know how to imitate
> > appraisal cmdline to disable IMA partially in kdump kernel case.

Thanks for detailed explanations. Just back from holiday, sorry for late
reply.

> 
> The IMA policy controls how much or how little IMA measures and appraises.  Most
> of the memory usage is the IMA measurement list, itself, and the per file cache
> info.  (The per file cache info limits re-measuring or re-appraising files.)

In Steve Chen's kexec supporting ima patchset, kdump kernel loading
should skip ima_kexec buffers allocating and storing via checking if
(image->type == KEXEC_TYPE_CRASH).
> 
> Similarly my knowledge of kdump is very limited.  Is there a way for the kernel
> to differentiate between kexec and kdump?  If we need a mechanism to disable
> IMA-measurement, I'd *really* prefer it be limited to kdump.

Yes, function is_kdump_kernel() is provided for checking if the current
kernel is in kdump kernel.

As said in earlier reply, for kdump kernel, there are two things we
should do:
1) when loading 2nd kernel to prepare for switching, we should not
allocate buffer and store IMA measurement list;
2) when switched into kdump kernel, we should not call ima_init() to do
kinds of init which is useless.

My personnal opinion.

Thanks
Baoquan



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-04-07  1:34               ` Baoquan He
@ 2025-04-07 11:46                 ` Mimi Zohar
  2025-04-09  2:42                   ` Baoquan He
  0 siblings, 1 reply; 22+ messages in thread
From: Mimi Zohar @ 2025-04-07 11:46 UTC (permalink / raw)
  To: Baoquan He; +Cc: Coiby Xu, RuiRui Yang, linux-integrity, kexec

On Mon, 2025-04-07 at 09:34 +0800, Baoquan He wrote:
> On 04/03/25 at 04:03pm, Mimi Zohar wrote:
> > On Wed, 2025-04-02 at 19:49 +0800, Baoquan He wrote:
> > > On 04/02/25 at 04:43pm, Coiby Xu wrote:
> > > > On Tue, Apr 01, 2025 at 11:30:09PM -0400, Mimi Zohar wrote:
> > > > > On Wed, 2025-04-02 at 09:47 +0800, RuiRui Yang wrote:
> > > > [...]
> > > > > > > > that.  Please don't make it generic like this.
> > > > > > > > 
> > > > > > > > Please refer to ima_appraise_parse_cmdline().
> > > > > > > 
> > > > > > > Hi Mimi,
> > > > > > > 
> > > > > > > To save memory for kdump, it seems init_ima has been to be skipped thus
> > > > > > > ima=off is necessary (ima_appraise=off won't serve the purpose). Or do
> > > > > > > you have any specific concerns in mind?
> > > > > > 
> > > > > > I think as Mimi said see below logic enforces the IMA even with the
> > > > > > cmdline disabling, see ima_appraise_parse_cmdline:
> > > > > > if (sb_state) {
> > > > > >                 if (!(appraisal_state & IMA_APPRAISE_ENFORCE))
> > > > > >                         pr_info("Secure boot enabled: ignoring
> > > > > > ima_appraise=%s option",
> > > > > >                                 str);
> > > > > >         } else {
> > > > > >                 ima_appraise = appraisal_state;
> > > > > >         }
> > > > 
> > > > Thanks for pointing me to the above code! Note with the whole IMA
> > > > disabled as done by this patch, the above code will not run so IMA
> > > > (appraisal) won't be enforced.
> > > > 
> > > > > 
> > > > > Thanks, RuiRui.
> > > > > 
> > > > 
> > > > Mimi, so do I understand it correctly that your want IMA-appraisal to be
> > > > always enabled as long as secure boot is enabled even if users choose to
> > > > disable IMA? 
> > 
> > Secure boot is not the only reason.  Based on policy IMA-appraisal and EVM
> > calculate and store file hashes and HMAC's in their respective security xattrs.
> > Normally the usage of file hashes and HMAC's is limited to mutable files. 
> > Disabling IMA-appraisal could result in not properly updating the security
> > xattrs, which would result in not being able to verify the file's integrity on
> > reboot.
> > 
> > On systems where the RPM includes file signatures, file signatures of immutable
> > files can be safely restored.  Although it is possible to walk the filesystem(s)
> > "fixing" the xattrs of mutable files, it defeats the purpose.  "fix" mode should
> > only be enabled in a trusted environment.
> > 
> > > > I wonder what security issue will it bring if this promise
> > > > gets broken considering other LSMs can SELinux can be disabled when
> > > > secure boot is enabled?
> > 
> > The builtin IMA policy rules are not defined in terms of SELinux labels.  If the
> > initial IMA custom policy defines rules based on SELinux labels and SELinux is
> > not enabled, the policy will fail to be loaded.
> > 
> > > > > Coiby, would disabling just IMA-measurement, as opposed to IMA-appraisal, save
> > > > > sufficient memory for kdump?
> > > > 
> > > > For disabling just IMA-measurement, do you mean not enabling any measure
> > > > rules?  The more memory reserved for the kdump kernel, the less memory
> > > > can be used by the 1st kernel. So from the perfective of kdump, we try
> > > > to make the memory footprint as smaller as possible.
> > 
> > Got it.
> > 
> > > > Baoquan, do you have any statistics about the memory overhead of IMA?
> > > 
> > > I am getting a system to check that. I think there are two aspects of
> > > IMA functionality we want to disable. One is disable the IMA-measurement
> > > copying from 1st kernel to 2nd kernel, this is only needed by kexec
> > > reboot; the other is IMA is not needed at all in kdump kernel, means we
> > > don't want to call ima_init() to initialize
> > > ima_keyring/crypto/template/digests/fs etc. 
> > > 
> > > With my shallow knowledge about IMA, I don't know how to imitate
> > > appraisal cmdline to disable IMA partially in kdump kernel case.
> 
> Thanks for detailed explanations. Just back from holiday, sorry for late
> reply.
> 
> > 
> > The IMA policy controls how much or how little IMA measures and appraises.  Most
> > of the memory usage is the IMA measurement list, itself, and the per file cache
> > info.  (The per file cache info limits re-measuring or re-appraising files.)
> 
> In Steve Chen's kexec supporting ima patchset, kdump kernel loading
> should skip ima_kexec buffers allocating and storing via checking if
> (image->type == KEXEC_TYPE_CRASH).
> > 
> > Similarly my knowledge of kdump is very limited.  Is there a way for the kernel
> > to differentiate between kexec and kdump?  If we need a mechanism to disable
> > IMA-measurement, I'd *really* prefer it be limited to kdump.
> 
> Yes, function is_kdump_kernel() is provided for checking if the current
> kernel is in kdump kernel.
> 
> As said in earlier reply, for kdump kernel, there are two things we
> should do:
> 1) when loading 2nd kernel to prepare for switching, we should not
> allocate buffer and store IMA measurement list;
> 2) when switched into kdump kernel, we should not call ima_init() to do
> kinds of init which is useless.
> 
> My personnal opinion.

Thanks for pointing out the KEXEC_TYPE_CRASH check and is_kdump_kernel().  Both
changes sound reasonable.

Mimi



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-04-07 11:46                 ` Mimi Zohar
@ 2025-04-09  2:42                   ` Baoquan He
  2025-04-09 15:40                     ` Mimi Zohar
  0 siblings, 1 reply; 22+ messages in thread
From: Baoquan He @ 2025-04-09  2:42 UTC (permalink / raw)
  To: Mimi Zohar, chenste; +Cc: Coiby Xu, RuiRui Yang, linux-integrity, kexec

On 04/07/25 at 07:46am, Mimi Zohar wrote:
> On Mon, 2025-04-07 at 09:34 +0800, Baoquan He wrote:
> > On 04/03/25 at 04:03pm, Mimi Zohar wrote:
> > > On Wed, 2025-04-02 at 19:49 +0800, Baoquan He wrote:
> > > > On 04/02/25 at 04:43pm, Coiby Xu wrote:
> > > > > On Tue, Apr 01, 2025 at 11:30:09PM -0400, Mimi Zohar wrote:
> > > > > > On Wed, 2025-04-02 at 09:47 +0800, RuiRui Yang wrote:
> > > > > [...]
> > > > > > > > > that.  Please don't make it generic like this.
> > > > > > > > > 
> > > > > > > > > Please refer to ima_appraise_parse_cmdline().
> > > > > > > > 
> > > > > > > > Hi Mimi,
> > > > > > > > 
> > > > > > > > To save memory for kdump, it seems init_ima has been to be skipped thus
> > > > > > > > ima=off is necessary (ima_appraise=off won't serve the purpose). Or do
> > > > > > > > you have any specific concerns in mind?
> > > > > > > 
> > > > > > > I think as Mimi said see below logic enforces the IMA even with the
> > > > > > > cmdline disabling, see ima_appraise_parse_cmdline:
> > > > > > > if (sb_state) {
> > > > > > >                 if (!(appraisal_state & IMA_APPRAISE_ENFORCE))
> > > > > > >                         pr_info("Secure boot enabled: ignoring
> > > > > > > ima_appraise=%s option",
> > > > > > >                                 str);
> > > > > > >         } else {
> > > > > > >                 ima_appraise = appraisal_state;
> > > > > > >         }
> > > > > 
> > > > > Thanks for pointing me to the above code! Note with the whole IMA
> > > > > disabled as done by this patch, the above code will not run so IMA
> > > > > (appraisal) won't be enforced.
> > > > > 
> > > > > > 
> > > > > > Thanks, RuiRui.
> > > > > > 
> > > > > 
> > > > > Mimi, so do I understand it correctly that your want IMA-appraisal to be
> > > > > always enabled as long as secure boot is enabled even if users choose to
> > > > > disable IMA? 
> > > 
> > > Secure boot is not the only reason.  Based on policy IMA-appraisal and EVM
> > > calculate and store file hashes and HMAC's in their respective security xattrs.
> > > Normally the usage of file hashes and HMAC's is limited to mutable files. 
> > > Disabling IMA-appraisal could result in not properly updating the security
> > > xattrs, which would result in not being able to verify the file's integrity on
> > > reboot.
> > > 
> > > On systems where the RPM includes file signatures, file signatures of immutable
> > > files can be safely restored.  Although it is possible to walk the filesystem(s)
> > > "fixing" the xattrs of mutable files, it defeats the purpose.  "fix" mode should
> > > only be enabled in a trusted environment.
> > > 
> > > > > I wonder what security issue will it bring if this promise
> > > > > gets broken considering other LSMs can SELinux can be disabled when
> > > > > secure boot is enabled?
> > > 
> > > The builtin IMA policy rules are not defined in terms of SELinux labels.  If the
> > > initial IMA custom policy defines rules based on SELinux labels and SELinux is
> > > not enabled, the policy will fail to be loaded.
> > > 
> > > > > > Coiby, would disabling just IMA-measurement, as opposed to IMA-appraisal, save
> > > > > > sufficient memory for kdump?
> > > > > 
> > > > > For disabling just IMA-measurement, do you mean not enabling any measure
> > > > > rules?  The more memory reserved for the kdump kernel, the less memory
> > > > > can be used by the 1st kernel. So from the perfective of kdump, we try
> > > > > to make the memory footprint as smaller as possible.
> > > 
> > > Got it.
> > > 
> > > > > Baoquan, do you have any statistics about the memory overhead of IMA?
> > > > 
> > > > I am getting a system to check that. I think there are two aspects of
> > > > IMA functionality we want to disable. One is disable the IMA-measurement
> > > > copying from 1st kernel to 2nd kernel, this is only needed by kexec
> > > > reboot; the other is IMA is not needed at all in kdump kernel, means we
> > > > don't want to call ima_init() to initialize
> > > > ima_keyring/crypto/template/digests/fs etc. 
> > > > 
> > > > With my shallow knowledge about IMA, I don't know how to imitate
> > > > appraisal cmdline to disable IMA partially in kdump kernel case.
> > 
> > Thanks for detailed explanations. Just back from holiday, sorry for late
> > reply.
> > 
> > > 
> > > The IMA policy controls how much or how little IMA measures and appraises.  Most
> > > of the memory usage is the IMA measurement list, itself, and the per file cache
> > > info.  (The per file cache info limits re-measuring or re-appraising files.)
> > 
> > In Steve Chen's kexec supporting ima patchset, kdump kernel loading
> > should skip ima_kexec buffers allocating and storing via checking if
> > (image->type == KEXEC_TYPE_CRASH).
> > > 
> > > Similarly my knowledge of kdump is very limited.  Is there a way for the kernel
> > > to differentiate between kexec and kdump?  If we need a mechanism to disable
> > > IMA-measurement, I'd *really* prefer it be limited to kdump.
> > 
> > Yes, function is_kdump_kernel() is provided for checking if the current
> > kernel is in kdump kernel.
> > 
> > As said in earlier reply, for kdump kernel, there are two things we
> > should do:
> > 1) when loading 2nd kernel to prepare for switching, we should not
> > allocate buffer and store IMA measurement list;
> > 2) when switched into kdump kernel, we should not call ima_init() to do
> > kinds of init which is useless.
> > 
> > My personnal opinion.
> 
> Thanks for pointing out the KEXEC_TYPE_CRASH check and is_kdump_kernel().  Both
> changes sound reasonable.

Thanks for confirming. I will consider how to fix it accordingly. Maybe
after Steven's patches are merged. That would be great if the buffer
allocating and storing can be skiped for kdump in Steven's patch. While
I am worried that could disrupt the progress of Steven's patches.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-04-09  2:42                   ` Baoquan He
@ 2025-04-09 15:40                     ` Mimi Zohar
  2025-04-16  3:22                       ` Baoquan He
  0 siblings, 1 reply; 22+ messages in thread
From: Mimi Zohar @ 2025-04-09 15:40 UTC (permalink / raw)
  To: Baoquan He, chenste; +Cc: Coiby Xu, RuiRui Yang, linux-integrity, kexec

On Wed, 2025-04-09 at 10:42 +0800, Baoquan He wrote:
> On 04/07/25 at 07:46am, Mimi Zohar wrote:
> > On Mon, 2025-04-07 at 09:34 +0800, Baoquan He wrote:
> > > On 04/03/25 at 04:03pm, Mimi Zohar wrote:
> > > > On Wed, 2025-04-02 at 19:49 +0800, Baoquan He wrote:
> > > > > On 04/02/25 at 04:43pm, Coiby Xu wrote:
> > > > > > On Tue, Apr 01, 2025 at 11:30:09PM -0400, Mimi Zohar wrote:
> > > > > > > On Wed, 2025-04-02 at 09:47 +0800, RuiRui Yang wrote:
> > > > > > [...]
> > > > > > > > > > that.  Please don't make it generic like this.
> > > > > > > > > > 
> > > > > > > > > > Please refer to ima_appraise_parse_cmdline().
> > > > > > > > > 
> > > > > > > > > Hi Mimi,
> > > > > > > > > 
> > > > > > > > > To save memory for kdump, it seems init_ima has been to be skipped thus
> > > > > > > > > ima=off is necessary (ima_appraise=off won't serve the purpose). Or do
> > > > > > > > > you have any specific concerns in mind?
> > > > > > > > 
> > > > > > > > I think as Mimi said see below logic enforces the IMA even with the
> > > > > > > > cmdline disabling, see ima_appraise_parse_cmdline:
> > > > > > > > if (sb_state) {
> > > > > > > >                 if (!(appraisal_state & IMA_APPRAISE_ENFORCE))
> > > > > > > >                         pr_info("Secure boot enabled: ignoring
> > > > > > > > ima_appraise=%s option",
> > > > > > > >                                 str);
> > > > > > > >         } else {
> > > > > > > >                 ima_appraise = appraisal_state;
> > > > > > > >         }
> > > > > > 
> > > > > > Thanks for pointing me to the above code! Note with the whole IMA
> > > > > > disabled as done by this patch, the above code will not run so IMA
> > > > > > (appraisal) won't be enforced.
> > > > > > 
> > > > > > > 
> > > > > > > Thanks, RuiRui.
> > > > > > > 
> > > > > > 
> > > > > > Mimi, so do I understand it correctly that your want IMA-appraisal to be
> > > > > > always enabled as long as secure boot is enabled even if users choose to
> > > > > > disable IMA? 
> > > > 
> > > > Secure boot is not the only reason.  Based on policy IMA-appraisal and EVM
> > > > calculate and store file hashes and HMAC's in their respective security xattrs.
> > > > Normally the usage of file hashes and HMAC's is limited to mutable files. 
> > > > Disabling IMA-appraisal could result in not properly updating the security
> > > > xattrs, which would result in not being able to verify the file's integrity on
> > > > reboot.
> > > > 
> > > > On systems where the RPM includes file signatures, file signatures of immutable
> > > > files can be safely restored.  Although it is possible to walk the filesystem(s)
> > > > "fixing" the xattrs of mutable files, it defeats the purpose.  "fix" mode should
> > > > only be enabled in a trusted environment.
> > > > 
> > > > > > I wonder what security issue will it bring if this promise
> > > > > > gets broken considering other LSMs can SELinux can be disabled when
> > > > > > secure boot is enabled?
> > > > 
> > > > The builtin IMA policy rules are not defined in terms of SELinux labels.  If the
> > > > initial IMA custom policy defines rules based on SELinux labels and SELinux is
> > > > not enabled, the policy will fail to be loaded.
> > > > 
> > > > > > > Coiby, would disabling just IMA-measurement, as opposed to IMA-appraisal, save
> > > > > > > sufficient memory for kdump?
> > > > > > 
> > > > > > For disabling just IMA-measurement, do you mean not enabling any measure
> > > > > > rules?  The more memory reserved for the kdump kernel, the less memory
> > > > > > can be used by the 1st kernel. So from the perfective of kdump, we try
> > > > > > to make the memory footprint as smaller as possible.
> > > > 
> > > > Got it.
> > > > 
> > > > > > Baoquan, do you have any statistics about the memory overhead of IMA?
> > > > > 
> > > > > I am getting a system to check that. I think there are two aspects of
> > > > > IMA functionality we want to disable. One is disable the IMA-measurement
> > > > > copying from 1st kernel to 2nd kernel, this is only needed by kexec
> > > > > reboot; the other is IMA is not needed at all in kdump kernel, means we
> > > > > don't want to call ima_init() to initialize
> > > > > ima_keyring/crypto/template/digests/fs etc. 
> > > > > 
> > > > > With my shallow knowledge about IMA, I don't know how to imitate
> > > > > appraisal cmdline to disable IMA partially in kdump kernel case.
> > > 
> > > Thanks for detailed explanations. Just back from holiday, sorry for late
> > > reply.
> > > 
> > > > 
> > > > The IMA policy controls how much or how little IMA measures and appraises.  Most
> > > > of the memory usage is the IMA measurement list, itself, and the per file cache
> > > > info.  (The per file cache info limits re-measuring or re-appraising files.)
> > > 
> > > In Steve Chen's kexec supporting ima patchset, kdump kernel loading
> > > should skip ima_kexec buffers allocating and storing via checking if
> > > (image->type == KEXEC_TYPE_CRASH).
> > > > 
> > > > Similarly my knowledge of kdump is very limited.  Is there a way for the kernel
> > > > to differentiate between kexec and kdump?  If we need a mechanism to disable
> > > > IMA-measurement, I'd *really* prefer it be limited to kdump.
> > > 
> > > Yes, function is_kdump_kernel() is provided for checking if the current
> > > kernel is in kdump kernel.
> > > 
> > > As said in earlier reply, for kdump kernel, there are two things we
> > > should do:
> > > 1) when loading 2nd kernel to prepare for switching, we should not
> > > allocate buffer and store IMA measurement list;
> > > 2) when switched into kdump kernel, we should not call ima_init() to do
> > > kinds of init which is useless.
> > > 
> > > My personnal opinion.
> > 
> > Thanks for pointing out the KEXEC_TYPE_CRASH check and is_kdump_kernel().  Both
> > changes sound reasonable.
> 
> Thanks for confirming. I will consider how to fix it accordingly. Maybe
> after Steven's patches are merged. That would be great if the buffer
> allocating and storing can be skiped for kdump in Steven's patch. While
> I am worried that could disrupt the progress of Steven's patches.

Agreed, let's get Steven's patch set upstreamed and then make the kdump
exceptions.

- "ima: kexec: move IMA log copy from kexec load to execute" looks like it isn't
copying the IMA measurement list records (kexec_post_load), but the memory for
the IMA measurement list is being allocated (ima_alloc_kexec_file_buf).

- Do you really want to totally disable IMA for kdump or would disabling IMA-
measurement be sufficient?  Remember there's already an option to disable IMA-
appraisal.  Disabling just IMA-measurement would allow IMA-appraisal to continue
to work.  Meaning based on policy the integrity of files - executables, kernel
image, etc - could still be verified.

Without IMA-measurement:
- No adding records to the IMA measurement list
- No IMA measurement list pseudo securityfs files
- No extending the TPM

With IMA-appraisal:
- Integrity verification of files based on keys, keyrings
- Loading keys

Obviously my preference would be to add support to disable IMA-measurement in a
kdump environment.

thanks,

Mimi


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-04-09 15:40                     ` Mimi Zohar
@ 2025-04-16  3:22                       ` Baoquan He
  2025-04-28  3:48                         ` Coiby Xu
  0 siblings, 1 reply; 22+ messages in thread
From: Baoquan He @ 2025-04-16  3:22 UTC (permalink / raw)
  To: Mimi Zohar, Coiby Xu; +Cc: chenste, RuiRui Yang, linux-integrity, kexec

On 04/09/25 at 11:40am, Mimi Zohar wrote:
> On Wed, 2025-04-09 at 10:42 +0800, Baoquan He wrote:
......snip.. 
> > Thanks for confirming. I will consider how to fix it accordingly. Maybe
> > after Steven's patches are merged. That would be great if the buffer
> > allocating and storing can be skiped for kdump in Steven's patch. While
> > I am worried that could disrupt the progress of Steven's patches.
> 
> Agreed, let's get Steven's patch set upstreamed and then make the kdump
> exceptions.
> 
> - "ima: kexec: move IMA log copy from kexec load to execute" looks like it isn't
> copying the IMA measurement list records (kexec_post_load), but the memory for
> the IMA measurement list is being allocated (ima_alloc_kexec_file_buf).
> 
> - Do you really want to totally disable IMA for kdump or would disabling IMA-
> measurement be sufficient?  Remember there's already an option to disable IMA-
> appraisal.  Disabling just IMA-measurement would allow IMA-appraisal to continue
> to work.  Meaning based on policy the integrity of files - executables, kernel
> image, etc - could still be verified.
> 
> Without IMA-measurement:
> - No adding records to the IMA measurement list
> - No IMA measurement list pseudo securityfs files
> - No extending the TPM
> 
> With IMA-appraisal:
> - Integrity verification of files based on keys, keyrings
> - Loading keys

Currently, Kdump has no demand to do integrity verification based on
keys, keyrings, except of Coiby's LUKS support in kdump:

[PATCH v8 0/7] Support kdump with LUKS encryption by reusing LUKS volume keys
https://lore.kernel.org/all/20250207080818.129165-1-coxu@redhat.com/T/#u

I have talked to Coiby, he will do some investigations to see if loading
keys related to IMA or IMA-appraisal functionality is related to LUKS
support in kdump because the LUKS support in kdump also needs
store/restore keys/keyrings between normal kernel and kdump kernel.

> 
> Obviously my preference would be to add support to disable IMA-measurement in a
> kdump environment.
> 
> thanks,
> 
> Mimi
> 



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-04-16  3:22                       ` Baoquan He
@ 2025-04-28  3:48                         ` Coiby Xu
  2025-04-29 11:39                           ` Mimi Zohar
  0 siblings, 1 reply; 22+ messages in thread
From: Coiby Xu @ 2025-04-28  3:48 UTC (permalink / raw)
  To: Mimi Zohar, Baoquan He; +Cc: chenste, RuiRui Yang, linux-integrity, kexec

On Wed, Apr 16, 2025 at 11:22:52AM +0800, Baoquan He wrote:
>On 04/09/25 at 11:40am, Mimi Zohar wrote:
>> On Wed, 2025-04-09 at 10:42 +0800, Baoquan He wrote:
>......snip..
>> > Thanks for confirming. I will consider how to fix it accordingly. Maybe
>> > after Steven's patches are merged. That would be great if the buffer
>> > allocating and storing can be skiped for kdump in Steven's patch. While
>> > I am worried that could disrupt the progress of Steven's patches.
>>
>> Agreed, let's get Steven's patch set upstreamed and then make the kdump
>> exceptions.
>>
>> - "ima: kexec: move IMA log copy from kexec load to execute" looks like it isn't
>> copying the IMA measurement list records (kexec_post_load), but the memory for
>> the IMA measurement list is being allocated (ima_alloc_kexec_file_buf).
>>
>> - Do you really want to totally disable IMA for kdump or would disabling IMA-
>> measurement be sufficient?  Remember there's already an option to disable IMA-
>> appraisal.  Disabling just IMA-measurement would allow IMA-appraisal to continue
>> to work.  Meaning based on policy the integrity of files - executables, kernel
>> image, etc - could still be verified.
>>
>> Without IMA-measurement:
>> - No adding records to the IMA measurement list
>> - No IMA measurement list pseudo securityfs files
>> - No extending the TPM
>>
>> With IMA-appraisal:
>> - Integrity verification of files based on keys, keyrings
>> - Loading keys

Thanks for listing the impacts of disabling IMA measurement or
appraisal! 

kdump builds and loads its own initramfs as a cpio archive. After a
kernel crashes, the loaded initramfs will save the crashed kernel's
memory (vmcore) to specified location and then it will reboot the system
immediately. Since kdump merely copy files from existing fs into its
initramfs, I think at least majority of users don't need IMA for kdump.
In fact, currently IMA-appraisal doesn't work for kdump because cpio
doesn't support xattr. As for the issue of not properly updating the
security xattrs, I think in most of cases disabling IMA-appraisal in
kdump won't cause the trouble. If the vmcore is saved to a remote fs, the
local fs won't even be touched. If the vmcore is saved to local fs and
the appraisal rules cover the saved vmcore and the created logs files,
we only need to update the xattr file of three files. So from the
perspective of kdump, it's good to disable IMA for kdump to save memory.
Of course we can't rule out the possibility some users may want to need
IMA in kdump. So a flexible solution like providing a knob to allow
users to enable IMA in kdump will be the ideal solution.

Btw, recently, a colleague reminds me of an issue that the system hangs
because systemd fails to load incorrect /etc/ima/ima-policy. Of course,
we should ask users to verify the policy beforehand. But it's still
possible the booting process may be stopped by a strict, albeit
syntax-correct policy and users can't log into the system to fix the
policy. Do you think a knob to disable IMA is needed to address these
cases or is there a better solution?

>
>Currently, Kdump has no demand to do integrity verification based on
>keys, keyrings, except of Coiby's LUKS support in kdump:
>
>[PATCH v8 0/7] Support kdump with LUKS encryption by reusing LUKS volume keys
>https://lore.kernel.org/all/20250207080818.129165-1-coxu@redhat.com/T/#u
>
>I have talked to Coiby, he will do some investigations to see if loading
>keys related to IMA or IMA-appraisal functionality is related to LUKS
>support in kdump because the LUKS support in kdump also needs
>store/restore keys/keyrings between normal kernel and kdump kernel.

Thanks for reminding me about kdump LUKS support. IMA will create
keyring like .ima for loading keys and other components can still create
their own keyrings or use existing keyrings. And I can confirm kdump
LUKS support won't be affected. 

-- 
Best regards,
Coiby



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-04-28  3:48                         ` Coiby Xu
@ 2025-04-29 11:39                           ` Mimi Zohar
  2025-05-09  5:59                             ` Coiby Xu
  0 siblings, 1 reply; 22+ messages in thread
From: Mimi Zohar @ 2025-04-29 11:39 UTC (permalink / raw)
  To: Coiby Xu, Baoquan He; +Cc: chenste, RuiRui Yang, linux-integrity, kexec

On Mon, 2025-04-28 at 11:48 +0800, Coiby Xu wrote:
> On Wed, Apr 16, 2025 at 11:22:52AM +0800, Baoquan He wrote:
> > On 04/09/25 at 11:40am, Mimi Zohar wrote:
> > > On Wed, 2025-04-09 at 10:42 +0800, Baoquan He wrote:
> > ......snip..
> > > > Thanks for confirming. I will consider how to fix it accordingly. Maybe
> > > > after Steven's patches are merged. That would be great if the buffer
> > > > allocating and storing can be skiped for kdump in Steven's patch. While
> > > > I am worried that could disrupt the progress of Steven's patches.
> > > 
> > > Agreed, let's get Steven's patch set upstreamed and then make the kdump
> > > exceptions.
> > > 
> > > - "ima: kexec: move IMA log copy from kexec load to execute" looks like it
> > > isn't
> > > copying the IMA measurement list records (kexec_post_load), but the memory for
> > > the IMA measurement list is being allocated (ima_alloc_kexec_file_buf).
> > > 
> > > - Do you really want to totally disable IMA for kdump or would disabling IMA-
> > > measurement be sufficient?  Remember there's already an option to disable IMA-
> > > appraisal.  Disabling just IMA-measurement would allow IMA-appraisal to
> > > continue
> > > to work.  Meaning based on policy the integrity of files - executables, kernel
> > > image, etc - could still be verified.
> > > 
> > > Without IMA-measurement:
> > > - No adding records to the IMA measurement list
> > > - No IMA measurement list pseudo securityfs files
> > > - No extending the TPM
> > > 
> > > With IMA-appraisal:
> > > - Integrity verification of files based on keys, keyrings
> > > - Loading keys
> 
> Thanks for listing the impacts of disabling IMA measurement or
> appraisal! 
> 
> kdump builds and loads its own initramfs as a cpio archive. After a
> kernel crashes, the loaded initramfs will save the crashed kernel's
> memory (vmcore) to specified location and then it will reboot the system
> immediately. Since kdump merely copy files from existing fs into its
> initramfs, I think at least majority of users don't need IMA for kdump.

That's fine.

> In fact, currently IMA-appraisal doesn't work for kdump because cpio
> doesn't support xattr.

Although CPIO doesn't support xattrs, tmpfs supports xattrs.  I'm aware that some
store the security xattr information in a file and write it out as xattrs.

> As for the issue of not properly updating the
> security xattrs, I think in most of cases disabling IMA-appraisal in
> kdump won't cause the trouble. If the vmcore is saved to a remote fs, the
> local fs won't even be touched. If the vmcore is saved to local fs and
> the appraisal rules cover the saved vmcore and the created logs files,
> we only need to update the xattr file of three files. So from the
> perspective of kdump, it's good to disable IMA for kdump to save memory.

Remember my original concerns weren't about disabling IMA for kdump, but about not
limiting disabling IMA to just kdump.

> Of course we can't rule out the possibility some users may want to need
> IMA in kdump. So a flexible solution like providing a knob to allow
> users to enable IMA in kdump will be the ideal solution.

Agreed

> 
> Btw, recently, a colleague reminds me of an issue that the system hangs
> because systemd fails to load incorrect /etc/ima/ima-policy. Of course,
> we should ask users to verify the policy beforehand. But it's still
> possible the booting process may be stopped by a strict, albeit
> syntax-correct policy and users can't log into the system to fix the
> policy. Do you think a knob to disable IMA is needed to address these
> cases or is there a better solution?

Agreed a new policy should always be tested, before attempting to load it on boot. 
However there are situations when even a tested policy fails.  Commonly this occurs
when attempting to load an IMA policy based on IMA features that don't exist in an
older kernel.  This can normally be resolved by booting into the newer kernel and
"fixing" the policy.  Instead of disabling IMA, I would allow specifying on the boot
command line an alternate IMA policy (e.g. ima-policy.backup-version) to be loaded
as fallback.

> 
> > 
> > Currently, Kdump has no demand to do integrity verification based on
> > keys, keyrings, except of Coiby's LUKS support in kdump:
> > 
> > [PATCH v8 0/7] Support kdump with LUKS encryption by reusing LUKS volume keys
> > https://lore.kernel.org/all/20250207080818.129165-1-coxu@redhat.com/T/#u
> > 
> > I have talked to Coiby, he will do some investigations to see if loading
> > keys related to IMA or IMA-appraisal functionality is related to LUKS
> > support in kdump because the LUKS support in kdump also needs
> > store/restore keys/keyrings between normal kernel and kdump kernel.
> 
> Thanks for reminding me about kdump LUKS support. IMA will create
> keyring like .ima for loading keys and other components can still create
> their own keyrings or use existing keyrings. And I can confirm kdump
> LUKS support won't be affected. 

thanks,

Mimi



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-04-29 11:39                           ` Mimi Zohar
@ 2025-05-09  5:59                             ` Coiby Xu
  2025-05-09 13:03                               ` Mimi Zohar
  0 siblings, 1 reply; 22+ messages in thread
From: Coiby Xu @ 2025-05-09  5:59 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: Baoquan He, chenste, RuiRui Yang, linux-integrity, kexec

On Tue, Apr 29, 2025 at 07:39:17AM -0400, Mimi Zohar wrote:
>On Mon, 2025-04-28 at 11:48 +0800, Coiby Xu wrote:
>> On Wed, Apr 16, 2025 at 11:22:52AM +0800, Baoquan He wrote:
>> > On 04/09/25 at 11:40am, Mimi Zohar wrote:
>> > > On Wed, 2025-04-09 at 10:42 +0800, Baoquan He wrote:
>> > ......snip..
>> > > > Thanks for confirming. I will consider how to fix it accordingly. Maybe
>> > > > after Steven's patches are merged. That would be great if the buffer
>> > > > allocating and storing can be skiped for kdump in Steven's patch. While
>> > > > I am worried that could disrupt the progress of Steven's patches.
>> > >
>> > > Agreed, let's get Steven's patch set upstreamed and then make the kdump
>> > > exceptions.
>> > >
>> > > - "ima: kexec: move IMA log copy from kexec load to execute" looks like it
>> > > isn't
>> > > copying the IMA measurement list records (kexec_post_load), but the memory for
>> > > the IMA measurement list is being allocated (ima_alloc_kexec_file_buf).
>> > >
>> > > - Do you really want to totally disable IMA for kdump or would disabling IMA-
>> > > measurement be sufficient?  Remember there's already an option to disable IMA-
>> > > appraisal.  Disabling just IMA-measurement would allow IMA-appraisal to
>> > > continue
>> > > to work.  Meaning based on policy the integrity of files - executables, kernel
>> > > image, etc - could still be verified.
>> > >
>> > > Without IMA-measurement:
>> > > - No adding records to the IMA measurement list
>> > > - No IMA measurement list pseudo securityfs files
>> > > - No extending the TPM
>> > >
>> > > With IMA-appraisal:
>> > > - Integrity verification of files based on keys, keyrings
>> > > - Loading keys
>>
>> Thanks for listing the impacts of disabling IMA measurement or
>> appraisal!
>>
>> kdump builds and loads its own initramfs as a cpio archive. After a
>> kernel crashes, the loaded initramfs will save the crashed kernel's
>> memory (vmcore) to specified location and then it will reboot the system
>> immediately. Since kdump merely copy files from existing fs into its
>> initramfs, I think at least majority of users don't need IMA for kdump.
>
>That's fine.
>
>> In fact, currently IMA-appraisal doesn't work for kdump because cpio
>> doesn't support xattr.
>
>Although CPIO doesn't support xattrs, tmpfs supports xattrs.

Thanks for pointing it out! I thought tmpfs doesn't support xattrs
because ima_policy=tcb excludes TMPFS_MAGIC. There are still many
interesting questions for me to explore. For example, I don't understand
how 1st kernel's initramfs is shown as ramfs (which doesn't support
xattrs) but kdump intiramfs without the squashfs module is shown as
tmpfs.

>I'm aware that some
>store the security xattr information in a file and write it out as xattrs.

If the built initramfs as a CPIO file doesn't carry xattrs, the loaded
initramfs still doesn't have xattrs. I just found the initramfs could
opt to use squashfs or erofs which supports xattrs but currently it's
hardcoded to disable xattrs. For example, recently the dracut erorfs
module also follows squashfs to disable xattrs [1]. So in the near
future, I don't expect xattrs to be supported in kdump.

[1] https://github.com/dracut-ng/dracut-ng/pull

>
>> As for the issue of not properly updating the
>> security xattrs, I think in most of cases disabling IMA-appraisal in
>> kdump won't cause the trouble. If the vmcore is saved to a remote fs, the
>> local fs won't even be touched. If the vmcore is saved to local fs and
>> the appraisal rules cover the saved vmcore and the created logs files,
>> we only need to update the xattr file of three files. So from the
>> perspective of kdump, it's good to disable IMA for kdump to save memory.
>
>Remember my original concerns weren't about disabling IMA for kdump, but about not
>limiting disabling IMA to just kdump.

Thanks for reminding me about that! Baoquan will post a patch to only
disable IMA for kdump so we won't need to worry about these concerns:)

>
>> Of course we can't rule out the possibility some users may want to need
>> IMA in kdump. So a flexible solution like providing a knob to allow
>> users to enable IMA in kdump will be the ideal solution.
>
>Agreed
>
>>
>> Btw, recently, a colleague reminds me of an issue that the system hangs
>> because systemd fails to load incorrect /etc/ima/ima-policy. Of course,
>> we should ask users to verify the policy beforehand. But it's still
>> possible the booting process may be stopped by a strict, albeit
>> syntax-correct policy and users can't log into the system to fix the
>> policy. Do you think a knob to disable IMA is needed to address these
>> cases or is there a better solution?
>
>Agreed a new policy should always be tested, before attempting to load it on boot.
>However there are situations when even a tested policy fails.  Commonly this occurs
>when attempting to load an IMA policy based on IMA features that don't exist in an
>older kernel.  This can normally be resolved by booting into the newer kernel and
>"fixing" the policy.  Instead of disabling IMA, I would allow specifying on the boot
>command line an alternate IMA policy (e.g. ima-policy.backup-version) to be loaded
>as fallback.

Thanks for letting me know the case where an IMA policy may fails
because of an older kernel. I'll start a new thread to discuss how to
fix an booting failure issue since it can be decoupled from this patch.
Previously you reminded me that re-enabling IMA will require re-fixing
security.ima. Then I realize that changing IMA policy will also require
such re-fixing otherwise booting failure may also occur if it involves a
critical component. So I'll also ask your advice for this matter in the
new thread as well.

-- 
Best regards,
Coiby



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-05-09  5:59                             ` Coiby Xu
@ 2025-05-09 13:03                               ` Mimi Zohar
  2025-05-13  0:14                                 ` Coiby Xu
  0 siblings, 1 reply; 22+ messages in thread
From: Mimi Zohar @ 2025-05-09 13:03 UTC (permalink / raw)
  To: Coiby Xu, Roberto Sassu
  Cc: Baoquan He, chenste, RuiRui Yang, linux-integrity, kexec

[Cc'ing Roberto]

On Fri, 2025-05-09 at 13:59 +0800, Coiby Xu wrote:
> On Tue, Apr 29, 2025 at 07:39:17AM -0400, Mimi Zohar wrote:
> > On Mon, 2025-04-28 at 11:48 +0800, Coiby Xu wrote:
> > > On Wed, Apr 16, 2025 at 11:22:52AM +0800, Baoquan He wrote:
> > > > On 04/09/25 at 11:40am, Mimi Zohar wrote:
> > > > > On Wed, 2025-04-09 at 10:42 +0800, Baoquan He wrote:
> > > > ......snip..
> > > > > > Thanks for confirming. I will consider how to fix it accordingly. Maybe
> > > > > > after Steven's patches are merged. That would be great if the buffer
> > > > > > allocating and storing can be skiped for kdump in Steven's patch. While
> > > > > > I am worried that could disrupt the progress of Steven's patches.
> > > > > 
> > > > > Agreed, let's get Steven's patch set upstreamed and then make the kdump
> > > > > exceptions.
> > > > > 
> > > > > - "ima: kexec: move IMA log copy from kexec load to execute" looks like it
> > > > > isn't
> > > > > copying the IMA measurement list records (kexec_post_load), but the memory
> > > > > for
> > > > > the IMA measurement list is being allocated (ima_alloc_kexec_file_buf).
> > > > > 
> > > > > - Do you really want to totally disable IMA for kdump or would disabling
> > > > > IMA-
> > > > > measurement be sufficient?  Remember there's already an option to disable
> > > > > IMA-
> > > > > appraisal.  Disabling just IMA-measurement would allow IMA-appraisal to
> > > > > continue
> > > > > to work.  Meaning based on policy the integrity of files - executables,
> > > > > kernel
> > > > > image, etc - could still be verified.
> > > > > 
> > > > > Without IMA-measurement:
> > > > > - No adding records to the IMA measurement list
> > > > > - No IMA measurement list pseudo securityfs files
> > > > > - No extending the TPM
> > > > > 
> > > > > With IMA-appraisal:
> > > > > - Integrity verification of files based on keys, keyrings
> > > > > - Loading keys
> > > 
> > > Thanks for listing the impacts of disabling IMA measurement or
> > > appraisal!
> > > 
> > > kdump builds and loads its own initramfs as a cpio archive. After a
> > > kernel crashes, the loaded initramfs will save the crashed kernel's
> > > memory (vmcore) to specified location and then it will reboot the system
> > > immediately. Since kdump merely copy files from existing fs into its
> > > initramfs, I think at least majority of users don't need IMA for kdump.
> > 
> > That's fine.
> > 
> > > In fact, currently IMA-appraisal doesn't work for kdump because cpio
> > > doesn't support xattr.
> > 
> > Although CPIO doesn't support xattrs, tmpfs supports xattrs.
> 
> Thanks for pointing it out! I thought tmpfs doesn't support xattrs
> because ima_policy=tcb excludes TMPFS_MAGIC. There are still many
> interesting questions for me to explore. For example, I don't understand
> how 1st kernel's initramfs is shown as ramfs (which doesn't support
> xattrs) but kdump intiramfs without the squashfs module is shown as
> tmpfs.

Try adding "rootfstype=tmpfs" to the boot command line.  For a detailed explanation,
take a look at commit 21528c69a0d8 ("rootfs: Fix support for rootfstype= when root=
is given").

> 
> > I'm aware that some
> > store the security xattr information in a file and write it out as xattrs.
> 
> If the built initramfs as a CPIO file doesn't carry xattrs, the loaded
> initramfs still doesn't have xattrs. I just found the initramfs could
> opt to use squashfs or erofs which supports xattrs but currently it's
> hardcoded to disable xattrs. For example, recently the dracut erorfs
> module also follows squashfs to disable xattrs [1]. So in the near
> future, I don't expect xattrs to be supported in kdump.
> 
> [1] https://github.com/dracut-ng/dracut-ng/pull

Right, so the issue isn't the initramfs root filesystem, but CPIO.  Either CPIO
would need to be extended, which multiple people have attempted to do, or the xattrs
could be stored in a file and written out to the initramfs root filesystem.  In fact
Roberto's last attempts at adding CPIO xattr support did something like that.

https://lore.kernel.org/linux-integrity/20190523121803.21638-1-roberto.sassu@huawei.com/

Mimi



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-05-09 13:03                               ` Mimi Zohar
@ 2025-05-13  0:14                                 ` Coiby Xu
  2025-05-13  3:55                                   ` Gao Xiang
  0 siblings, 1 reply; 22+ messages in thread
From: Coiby Xu @ 2025-05-13  0:14 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Roberto Sassu, Baoquan He, chenste, RuiRui Yang, linux-integrity,
	kexec

On Fri, May 09, 2025 at 09:03:11AM -0400, Mimi Zohar wrote:
>[Cc'ing Roberto]
>
>On Fri, 2025-05-09 at 13:59 +0800, Coiby Xu wrote:
>> On Tue, Apr 29, 2025 at 07:39:17AM -0400, Mimi Zohar wrote:
>> > On Mon, 2025-04-28 at 11:48 +0800, Coiby Xu wrote:
>> > > On Wed, Apr 16, 2025 at 11:22:52AM +0800, Baoquan He wrote:
>> > > > On 04/09/25 at 11:40am, Mimi Zohar wrote:
>> > > > > On Wed, 2025-04-09 at 10:42 +0800, Baoquan He wrote:
>> > > > ......snip..
>> > > > > > Thanks for confirming. I will consider how to fix it accordingly. Maybe
>> > > > > > after Steven's patches are merged. That would be great if the buffer
>> > > > > > allocating and storing can be skiped for kdump in Steven's patch. While
>> > > > > > I am worried that could disrupt the progress of Steven's patches.
>> > > > >
>> > > > > Agreed, let's get Steven's patch set upstreamed and then make the kdump
>> > > > > exceptions.
>> > > > >
>> > > > > - "ima: kexec: move IMA log copy from kexec load to execute" looks like it
>> > > > > isn't
>> > > > > copying the IMA measurement list records (kexec_post_load), but the memory
>> > > > > for
>> > > > > the IMA measurement list is being allocated (ima_alloc_kexec_file_buf).
>> > > > >
>> > > > > - Do you really want to totally disable IMA for kdump or would disabling
>> > > > > IMA-
>> > > > > measurement be sufficient?  Remember there's already an option to disable
>> > > > > IMA-
>> > > > > appraisal.  Disabling just IMA-measurement would allow IMA-appraisal to
>> > > > > continue
>> > > > > to work.  Meaning based on policy the integrity of files - executables,
>> > > > > kernel
>> > > > > image, etc - could still be verified.
>> > > > >
>> > > > > Without IMA-measurement:
>> > > > > - No adding records to the IMA measurement list
>> > > > > - No IMA measurement list pseudo securityfs files
>> > > > > - No extending the TPM
>> > > > >
>> > > > > With IMA-appraisal:
>> > > > > - Integrity verification of files based on keys, keyrings
>> > > > > - Loading keys
>> > >
>> > > Thanks for listing the impacts of disabling IMA measurement or
>> > > appraisal!
>> > >
>> > > kdump builds and loads its own initramfs as a cpio archive. After a
>> > > kernel crashes, the loaded initramfs will save the crashed kernel's
>> > > memory (vmcore) to specified location and then it will reboot the system
>> > > immediately. Since kdump merely copy files from existing fs into its
>> > > initramfs, I think at least majority of users don't need IMA for kdump.
>> >
>> > That's fine.
>> >
>> > > In fact, currently IMA-appraisal doesn't work for kdump because cpio
>> > > doesn't support xattr.
>> >
>> > Although CPIO doesn't support xattrs, tmpfs supports xattrs.
>>
>> Thanks for pointing it out! I thought tmpfs doesn't support xattrs
>> because ima_policy=tcb excludes TMPFS_MAGIC. There are still many
>> interesting questions for me to explore. For example, I don't understand
>> how 1st kernel's initramfs is shown as ramfs (which doesn't support
>> xattrs) but kdump intiramfs without the squashfs module is shown as
>> tmpfs.
>
>Try adding "rootfstype=tmpfs" to the boot command line.  For a detailed explanation,
>take a look at commit 21528c69a0d8 ("rootfs: Fix support for rootfstype= when root=
>is given").

Thanks for pointing me to the above commit! I'll check it.

>
>>
>> > I'm aware that some
>> > store the security xattr information in a file and write it out as xattrs.
>>
>> If the built initramfs as a CPIO file doesn't carry xattrs, the loaded
>> initramfs still doesn't have xattrs. I just found the initramfs could
>> opt to use squashfs or erofs which supports xattrs but currently it's
>> hardcoded to disable xattrs. For example, recently the dracut erorfs
>> module also follows squashfs to disable xattrs [1]. So in the near
>> future, I don't expect xattrs to be supported in kdump.
>>
>> [1] https://github.com/dracut-ng/dracut-ng/pull

Sorry, I didn't notice the link is incomplete. It should be

[1] https://github.com/dracut-ng/dracut-ng/pull/1296

>
>Right, so the issue isn't the initramfs root filesystem, but CPIO.  Either CPIO
>would need to be extended, which multiple people have attempted to do, or the xattrs
>could be stored in a file and written out to the initramfs root filesystem.  In fact
>Roberto's last attempts at adding CPIO xattr support did something like that.
>
>https://lore.kernel.org/linux-integrity/20190523121803.21638-1-roberto.sassu@huawei.com/

Thanks for introducing Roberto's work to me!

>
>Mimi
>

-- 
Best regards,
Coiby



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH] ima: add a knob to make IMA be able to be disabled
  2025-05-13  0:14                                 ` Coiby Xu
@ 2025-05-13  3:55                                   ` Gao Xiang
  0 siblings, 0 replies; 22+ messages in thread
From: Gao Xiang @ 2025-05-13  3:55 UTC (permalink / raw)
  To: Coiby Xu
  Cc: Roberto Sassu, Baoquan He, chenste, RuiRui Yang, linux-integrity,
	kexec, Christian Brauner, Mimi Zohar

Hi Coiby,

On 2025/5/13 08:14, Coiby Xu wrote:
> On Fri, May 09, 2025 at 09:03:11AM -0400, Mimi Zohar wrote:

...

>>>
>>> > I'm aware that some
>>> > store the security xattr information in a file and write it out as xattrs.
>>>
>>> If the built initramfs as a CPIO file doesn't carry xattrs, the loaded
>>> initramfs still doesn't have xattrs. I just found the initramfs could
>>> opt to use squashfs or erofs which supports xattrs but currently it's
>>> hardcoded to disable xattrs. For example, recently the dracut erorfs
>>> module also follows squashfs to disable xattrs [1]. So in the near
>>> future, I don't expect xattrs to be supported in kdump.
>>>
>>> [1] https://github.com/dracut-ng/dracut-ng/pull
> 
> Sorry, I didn't notice the link is incomplete. It should be
> 
> [1] https://github.com/dracut-ng/dracut-ng/pull/1296
> 
>>
>> Right, so the issue isn't the initramfs root filesystem, but CPIO.  Either CPIO
>> would need to be extended, which multiple people have attempted to do, or the xattrs
>> could be stored in a file and written out to the initramfs root filesystem.  In fact
>> Roberto's last attempts at adding CPIO xattr support did something like that.
>>
>> https://lore.kernel.org/linux-integrity/20190523121803.21638-1-roberto.sassu@huawei.com/
> 
> Thanks for introducing Roberto's work to me!

I wrote some words on initramfs vs initrd erofs on related threads:
https://lore.kernel.org/r/934af3e3-3153-40c1-9a25-7a8d08fdb007@linux.alibaba.com
https://lore.kernel.org/r/582bc002-f0c8-4dbb-8fa5-4c10a479b518@linux.alibaba.com/T/#u

The CPIO standard doesn't support xattrs, also initramfs could cause
unnecessary unpacking.

But anyway it needs more work on this stuff too.

Thanks,
Gao Xiang

> 
>>
>> Mimi
>>
> 



^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2025-05-13  3:55 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-31  6:16 [RFC PATCH] ima: add a knob to make IMA be able to be disabled Baoquan He
2025-03-31  6:22 ` Paul Menzel
2025-03-31  8:21   ` Baoquan He
2025-03-31 12:15 ` Mimi Zohar
2025-04-02  1:38   ` Coiby Xu
2025-04-02  1:47     ` RuiRui Yang
2025-04-02  3:30       ` Mimi Zohar
2025-04-02  8:43         ` Coiby Xu
2025-04-02 11:25           ` Mimi Zohar
2025-04-02 11:49           ` Baoquan He
2025-04-03 20:03             ` Mimi Zohar
2025-04-07  1:34               ` Baoquan He
2025-04-07 11:46                 ` Mimi Zohar
2025-04-09  2:42                   ` Baoquan He
2025-04-09 15:40                     ` Mimi Zohar
2025-04-16  3:22                       ` Baoquan He
2025-04-28  3:48                         ` Coiby Xu
2025-04-29 11:39                           ` Mimi Zohar
2025-05-09  5:59                             ` Coiby Xu
2025-05-09 13:03                               ` Mimi Zohar
2025-05-13  0:14                                 ` Coiby Xu
2025-05-13  3:55                                   ` Gao Xiang

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).