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

[Roberto S. <rob...@po...>]

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;
}


> Mimi
> 
>