Messages in this thread | | | Subject | Re: [PATCH v5 6/7] IMA: add critical_data to the built-in policy rules | From | Lakshmi Ramasubramanian <> | Date | Mon, 9 Nov 2020 09:24:04 -0800 |
| |
On 11/8/20 7:46 AM, Mimi Zohar wrote: > Hi Lakshmi, > > On Fri, 2020-11-06 at 15:51 -0800, Lakshmi Ramasubramanian wrote: >> >>>>> diff --git a/security/integrity/ima/ima_policy.c >>>>> b/security/integrity/ima/ima_policy.c >>>>> index ec99e0bb6c6f..dc8fe969d3fe 100644 >>>>> --- a/security/integrity/ima/ima_policy.c >>>>> +++ b/security/integrity/ima/ima_policy.c >>>> >>>>> @@ -875,6 +884,29 @@ void __init ima_init_policy(void) >>>>> ARRAY_SIZE(default_appraise_rules), >>>>> IMA_DEFAULT_POLICY); >>>>> + if (ima_use_critical_data) { >>>>> + template = lookup_template_desc("ima-buf"); >>>>> + if (!template) { >>>>> + ret = -EINVAL; >>>>> + goto out; >>>>> + } >>>>> + >>>>> + ret = template_desc_init_fields(template->fmt, >>>>> + &(template->fields), >>>>> + &(template->num_fields)); >>>> >>>> The default IMA template when measuring buffer data is "ima_buf". Is >>>> there a reason for allocating and initializing it here and not >>>> deferring it until process_buffer_measurement()? >>>> >>> >>> You are right - good catch. >>> I will remove the above and validate. >>> >> >> process_buffer_measurement() allocates and initializes "ima-buf" >> template only when the parameter "func" is NONE. Currently, only >> ima_check_blacklist() passes NONE for func when calling >> process_buffer_measurement(). >> >> If "func" is anything other than NONE, ima_match_policy() picks >> the default IMA template if the IMA policy rule does not specify a template. >> >> We need to add "ima-buf" in the built-in policy for critical_data so >> that the default template is not used for buffer measurement. >> >> Please let me know if I am missing something. >> > > Let's explain a bit further what is happening and why. As you said > ima_get_action() returns the template format, which may be the default > IMA template or the specific IMA policy rule template format. This > works properly for both the arch specific and custom policies, but not > for builtin policies, because the policy rules may contain a rule > specific .template field. When the rules don't contain a rule > specific template field, they default to the IMA default template. In > the case of builtin policies, the policy rules cannot contain the > .template field. > > The default template field for process_buffer_measurement() should > always be "ima-buf", not the default IMA template format. Let's fix > this prior to this patch. > > Probably something like this: > - In addition to initializing the default IMA template, initialize the > "ima-buf" template. Maybe something similiar to > ima_template_desc_current(). > - Set the default in process_buffer_measurement() to "ima-buf", before > calling ima_get_action(). > - modify ima_match_policy() so that the default policy isn't reset when > already specified. >
Sure Mimi - I will try this out and update.
thanks, -lakshmi
> > >>>> >>>>> + if (ret) >>>>> + goto out; >>>>> + >>>>> + critical_data_rules[0].template = template; >>>>> + add_rules(critical_data_rules, >>>>> + ARRAY_SIZE(critical_data_rules), >>>>> + IMA_DEFAULT_POLICY); >>>>> + } >>>>> + >>>>> +out: >>>>> + if (ret) >>>>> + pr_err("%s failed, result: %d\n", __func__, ret); >>>>> + >>>>> ima_update_policy_flag(); >>>>> } >>>> >>> >>
| |