Re: [PATCH v5 5/7] IMA: validate supported kernel data sources before measurement

[Tushar Sugandhi]

On 2020-11-06 6:01 a.m., Mimi Zohar wrote:
> Hi Tushar,
>
> On Sun, 2020-11-01 at 14:26 -0800, Tushar Sugandhi wrote:
> > Currently, IMA does not restrict random data sources from measuring
> > their data using ima_measure_critical_data(). Any kernel data source can
> > call the function, and it's data will get measured as long as the input
> > event_data_source is part of the IMA policy - CRITICAL_DATA+data_sources.
> >
> > To ensure that only data from supported sources are measured, the kernel
> > subsystem name needs to be added to a compile-time list of supported
> > sources (an "allowed list of components"). IMA then validates the input
> > parameter - "event_data_source" passed to ima_measure_critical_data()
> > against this allowed list at run-time.
> >
> > This compile-time list must be updated when kernel subsystems are
> > updated to measure their data using IMA.
> >
> > Provide an infrastructure for kernel data sources to be added to
> > IMA's supported data sources list at compile-time. Update
> > ima_measure_critical_data() to validate, at run-time, that the data
> > source is supported before measuring the data coming from that source.
>
> For those interested in limiting which critical data to measure, the
> "data sources" IMA policy rule option already does that.   Why is this
> needed?
>
> thanks,
>
> Mimi

This wasn’t part of the initial series. And I wasn’t convinced if it was
really needed. :) I added it based on the feedback in v2 of this
series. (pasted below for reference[1]).

Maybe I misunderstood your feedback at that time.

*Question*
Could you please let me know if you want us to remove this patch?


[1] From v2 of this series:
https://patchwork.kernel.org/project/linux-integrity/patch/20200821182107.5328-3-tusharsu@xxxxxxxxxxxxxxxxxxx/ 


>>>> "keyrings=" isn't bounded because keyrings can be created by 
userspace.
>>>> Perhaps keyring names has a minimum/maximum length.  IMA isn't
>>>> measuring userspace construsts.  Shouldn't the list of critical data
>>>> being measured be bounded and verified?
>>> The comment is not entirely clear.
>>> Do you mean there should be some sort of allow_list in IMA, against
>>> which the values in "data_sources=" should be vetted? And if the
>>> value is present in the IMA allow_list, then only the measurements for
>>> that data source are allowed?
>>>
>>> Or do you mean something else?
>>
>> Yes, something along those lines.  Does the list of critical data need
>> to be vetted?  And if so, against what?
> I am thinking of having an enum and string array - just like ima_hooks
> and ima_hooks_measure_str in ima.h.
> And any new kernel component that would support generic IMA measurements
> in future would have to add itself to the enum/array.
> And the param *event_data_source in ima_measure_critical_data() will be
> vetted against the above enum/string array.
>
> I will implement it in the next iteration, and hopefully the vetting
> workflow will be more clear.
>
> ~Tushar
>>
>> Mimi