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

[Tushar Sugandhi <tusharsu@linux.microsoft.com>]

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@linux.microsoft.com/ 


 >>>> "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