[apparmor] [PATCH 2/4] parser: Add support for DBus rules

[John Johansen]

On 07/29/2013 03:40 PM, Seth Arnold wrote:
> On Sat, Jul 27, 2013 at 02:45:16AM -0700, Tyler Hicks wrote:
>> This patch implements the parsing of DBus rules.
>>

<< snip >>

>> +}
>> +
>> +static void copy_conditionals(struct dbus_entry *ent, struct cond_entry *conds)
>> +{
>> +	struct cond_entry *cond_ent;
>> +
>> +	list_for_each(conds, cond_ent) {
>> +		char **ent_member = NULL;
>> +
>> +		/* for now disallow keyword 'in' (list) */
>> +		if (!cond_ent->eq)
>> +			yyerror("keyword \"in\" is not allowed in dbus rules\n");
>> +		if (list_len(cond_ent->vals) > 1)
>> +			yyerror("dbus conditional \"%s\" only supports a single value\n",
>> +				cond_ent->name);
>> +
>> +		if (strcmp(cond_ent->name, "bus") == 0) {
>> +			ent_member = &ent->bus;
>> +		} else if (strcmp(cond_ent->name, "name") == 0) {
>> +			ent_member = &ent->name;
>> +		} else if (strcmp(cond_ent->name, "label") == 0) {
>> +			ent_member = &ent->peer_label;
>> +		} else if (strcmp(cond_ent->name, "path") == 0) {
>> +			ent_member = &ent->path;
>> +		} else if (strcmp(cond_ent->name, "interface") == 0) {
>> +			ent_member = &ent->interface;
>> +		} else if (strcmp(cond_ent->name, "member") == 0) {
>> +			ent_member = &ent->member;
>> +		} else {
>> +			yyerror("invalid dbus conditional \"%s\"\n",
>> +				cond_ent->name);
>> +		}
>> +
>> +		if (ent_member) {
>> +			if (*ent_member) {
>> +				yyerror("dbus conditional \"%s\" can only be specified once\n",
>> +					cond_ent->name);
>> +			}
>> +
>> +			(*ent_member) = cond_ent->vals->value;
>> +		}
>> +
>> +		cond_ent->vals->value = NULL;
>> +	}
>> +}
> 
> This is a bit intricate :) I've read through it a few times and I'm
> not convinced I understand it. It seems a strange place to put the 'in'
> check, and the pointer handling seems more complicated than ideal. I'm
> sorry though, I don't have a concrete suggestion for improvement.
> 

The 'in' check is handled here because the code is using a common parsing
pattern routine, to pick up the entries.  So either this needs to be
handled in the common parsing code by setting up/passing in context,
or in post.

This is just handling the semantic check post rule match. And the check
is here instead of have a large code block hanging off of the yacc rule.

As for pushing the semantics down into the generic pattern, this is
difficult to do with yacc/bison. You can pass context, but we currently
don't, and the context can only be updated once a certain match is
achieved and reduced. So handling setting up meaningful context for
this to be pushed into the generic pattern is probably more work than
its worth.

There will be some patches to start push profile/block context down but
this is will be less work and a lot more useful.

As for the pointer handling sure its more complicated than you might
like but isn't that always the case with pointers? I don't see a better
way without standard templates to achieve the abstraction here


<< snip >>

>> +dbus_perm: TOK_VALUE
>> +	{
>> +		if (strcmp($1, "bind") == 0)
>> +			$$ = AA_DBUS_BIND;
>> +		else if (strcmp($1, "send") == 0 || strcmp($1, "write") == 0)
>> +			$$ = AA_DBUS_SEND;
>> +		else if (strcmp($1, "receive") == 0 || strcmp($1, "read") == 0)
>> +			$$ = AA_DBUS_RECEIVE;
> 
> Is this code still needed? You've got explicit tokens lower down...
> 
sadly yes, though perhaps we could abstract this more.

This is the difference between a bare keyword

  dbus send blah...,

and the keyword showing up in a permission list

  dbus (send receive) blah....

>> +		else if ($1) {
>> +			parse_dbus_mode($1, &$$, 1);
>> +		} else
>> +			$$ = 0;
>> +
>> +		if ($1)
>> +			free($1);
>> +	}
>> +	| TOK_BIND { $$ = AA_DBUS_BIND; }
>> +	| TOK_SEND { $$ = AA_DBUS_SEND; }
>> +	| TOK_RECEIVE { $$ = AA_DBUS_RECEIVE; }
>> +	| TOK_READ { $$ = AA_DBUS_RECEIVE; }
>> +	| TOK_WRITE { $$ = AA_DBUS_SEND; }
>> +	| TOK_MODE
>> +	{
>> +		parse_dbus_mode($1, &$$, 1);
>> +	}
>> +
>> +dbus_perms: { /* nothing */ $$ = 0; }
>> +	| dbus_perms dbus_perm { $$ = $1 | $2; }
>> +	| dbus_perms TOK_COMMA dbus_perm { $$ = $1 | $3; }
>> +
>> +opt_dbus_perm: { /* nothing */ $$ = 0; }
>> +	| dbus_perm  { $$ = $1; }
>> +	| TOK_OPENPAREN dbus_perms TOK_CLOSEPAREN { $$ = $2; }
>> +
>> +dbus_rule: TOK_DBUS opt_dbus_perm opt_conds opt_cond_list TOK_END_OF_RULE
>> +	{
>> +		struct dbus_entry *ent;
>> +
>> +		ent = new_dbus_entry($2, $3, $4);
>> +		if (!ent) {
>> +			yyerror(_("Memory allocation error."));
>> +		}
>> +		$$ = ent;
>> +	}
>> +
>>  hat_start: TOK_CARET {}
>>  	| TOK_HAT {}
>>  
> 
> Thanks!
> 
> 
>