Re: [Linux-ima-user] [RFC][PATCH][0/2] IMA

[Mimi Z. <zo...@li...>]

On Fri, 2010-04-16 at 17:12 +0200, Roberto Sassu wrote:
> On Thursday 15 April 2010 00:20:00 Mimi Zohar wrote:
> > On Wed, 2010-04-14 at 15:32 -0400, Eric Paris wrote:
> > > On Wed, Apr 14, 2010 at 5:51 AM, Roberto Sassu <rob...@po...> wrote:
> > > > Description of the issue:
> > > 
> > > > The parsing function is used only when custom rules must be loaded to replace
> > > > the default ones, but a different behavior can be observed in respect to the
> > > > first case: only one element is allocated and it is populated by the parsing
> > > > function that recognizes the tokens ' ' and '\n'.
> > > > So, having multiple lines in the file passed through the exported interface
> > > > doesn't lead to the same number of rules in the list, since only one allocated
> > > > item is available. If a directive spans two different lines, only the last
> > > > value is taken and the first one is overwritten.
> > > 
> > > I admit I haven't looked at the loading code before and it does seem
> > > 'special.'   It looks to me on first glance like the kernel accepts
> > > one rule per write() rather than one rule per line.  If you send it
> > > two rules in a single write you are going to get a rather
> > > unpredictable result.
> > 
> > Similar to smack, one rule per write.  Keeps it nice and simple.
> > 
> > > Your change is a good start to allow single writes which contain
> > > multiple rules.  I'm guessing that's a good idea, I though I think the
> > > parser should get a little smarter.  Does it ever make sense to have 2
> > > actions in a single rule, with the last one winning?  You also added
> > > support for some sort of commenting in the rule set didn't you?  That
> > > probably needs to be a lot stronger too....
> > > 
> > > I'm not acking or nacking here, just trying to understand exactly the
> > > problem we are trying to solve.  Loading rules using
> > > 
> > > cat rule > /sys/kernel/security/ima/policy
> > > 
> > > seems like a good direction to head.
> > 
> > hm, I was under the impression we wanted to minimize any
> > policy/configuration parsing done in the kernel.
> > 
> 
> In my thinking, actually the behaviour of the code that loads the policy through the /sys interface 
> is different from the one that handles hardcoded rules. In the last case these are stored as array of structures
> and each element is added to the list by the "list_add_tail" function. I past here the few lines of the code 
> involved:
> 
> void __init ima_init_policy(void)
> {
> 	int i, entries;
> 
> 	/* if !ima_use_tcb set entries = 0 so we load NO default rules */
> 	if (ima_use_tcb)
> 		entries = ARRAY_SIZE(default_rules);
> 	else
> 		entries = 0;
> 
> 	for (i = 0; i < entries; i++)
> 		list_add_tail(&default_rules[i].list, &measure_default_rules);
> 	ima_measure = &measure_default_rules;
> }

The original IMA code supported parsing multiple rules, but somewhere
along the way I was asked to clean the up code and told to minimize
kernel parsing. I personally don't have a problem with supporting a more
robust, complete policy parser. Perhaps someone could chime in here?

Thanks,

Mimi