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