Messages in this thread | | | Subject | Re: [PATCH v5 6/7] IMA: add critical_data to the built-in policy rules | From | Lakshmi Ramasubramanian <> | Date | Fri, 6 Nov 2020 15:51:51 -0800 |
| |
On 11/6/20 7:37 AM, Lakshmi Ramasubramanian wrote:
Hi Mimi,
> >> Hi Lakshmi, Tushar, >> >> This patch defines a new critical_data builtin policy. Please update >> the Subject line. >> >> On Sun, 2020-11-01 at 14:26 -0800, Tushar Sugandhi wrote: >>> From: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> >>> >>> The IMA hook to measure kernel critical data, namely >>> ima_measure_critical_data(), could be called before a custom IMA policy >>> is loaded. For example, SELinux calls ima_measure_critical_data() to >>> measure its state and policy when they are initialized. This occurs >>> before a custom IMA policy is loaded, and hence IMA hook will not >>> measure the data. A built-in policy is therefore needed to measure >>> critical data provided by callers before a custom IMA policy is loaded. >> >> ^Define a new critical data builtin policy to allow measuring early >> kernel integrity critical data before a custom IMA policy is loaded. > > I will add the above line in the patch description. > >> >> Either remove the references to SELinux or move this patch after the >> subsequent patch which measures SELinux critical data. > > I will remove the reference to SELinux. > I think it would be better to have this patch before the SELinux > measurement patch. > >> >>> >>> Add CRITICAL_DATA to built-in IMA rules if the kernel command line >>> contains "ima_policy=critical_data". Set the IMA template for this rule >>> to "ima-buf" since ima_measure_critical_data() measures a buffer. >>> >>> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> >> >>> --- >>> security/integrity/ima/ima_policy.c | 32 +++++++++++++++++++++++++++++ >>> 1 file changed, 32 insertions(+) >>> >>> 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.
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(); >>> } >> >
| |