[apparmor] [patch 19/21] Add the ability to mediate signals.

[Seth Arnold]

On Mon, Mar 17, 2014 at 04:29:29PM -0700, john.johansen at canonical.com wrote:
> Add signal rules and make sure the parser encodes support for them
> if the supported feature set reports supporting them.
> 
> The current format of the signal rule is
> 
>   [audit] [deny] signal [<signal_perms>] [<signal_set>] <target_profile>,
> 
>   signal_perm  := 'send'|'receive'|'r'|'w'|'rw'
>   signal_perms := <signal_perm> | '(' <signal_perm> ([,]<signal_perm>)* ')'
>   signal := ("hup"|"int"|"quit"|"ill"|"trap"|"abrt"|"bus"|"fpe"|"kill"|
>              "usr1"|"segv"|"usr2"|"pipe"|"alrm"|"term"|"tkflt"|"chld"|

Note that the signal you've got in here as "tkflt" should actually be
"stkflt" here and throughout the code.

>              "cont"|"stop"|"stp"|"ttin"|"ttou"|"urg"|"xcpu"|"xfsz"|"vtalrm"|
>              "prof"|"winch"|"io"|"pwr"|"sys"|"emt"|"exists")
>   signal_set   := set=<signal> | '(' <signal> ([,]<signal>)* ')'
> 
> 
> it does not currently follow the peer=() format, and there is some question
> as to whether it should or not. Input welcome.

The peer=() stuff would feel so useless on these rules. I won't whine if
we don't do it, there's no "local" interfaces that could logically have
the same names, unlike dbus or networking.

> +rules:  rules opt_prefix signal_rule
> +	{
> +		if ($2.owner)
> +			yyerror(_("owner prefix not allowed on signal rules"));
> +		if ($2.deny && $2.audit) {
> +			$3->deny = 1;
> +		} else if ($2.deny) {
> +			$3->deny = 1;
> +			$3->audit = $3->mode;
> +		} else if ($2.audit) {
> +			$3->audit = $3->mode;
> +		}
> +		$1->rule_ents.push_back($3);
> +		$$ = $1;
> +	}

Why not support 'owner'? Granted it'd only make sense for root processes
and it is almost redundant with capability kill, but it seems like it
could be useful for someone.


> +void signal_rule::move_conditionals(struct cond_entry *conds)
> +{
> +	struct cond_entry *cond_ent;
> +
> +	list_for_each(conds, cond_ent) {
> +		/* for now disallow keyword 'in' (list) */
> +		if (!cond_ent->eq)
> +			yyerror("keyword \"in\" is not allowed in signal rules\n");
> +		/* use alternation instead????
> +		 * set=(kill,usr1) vs. set={kill,usr1}

Hrm. Dunno. Sorry.


> +int signal_rule::gen_policy_re(Profile &prof)
> +{
> +	std::ostringstream buffer;
> +	std::string buf;
> +
> +	pattern_t ptype;
> +	int pos;
> +
> +	/* Currently do not generate the rules if the kernel doesn't support
> +	 * it. We may want to switch this so that a compile could be
> +	 * used for full support on kernels that don't support the feature
> +	 */
> +	if (!kernel_supports_signal) {
> +		warn_once(prof.name);
> +		return RULE_NOT_SUPPORTED;
> +	}
> +
> +	if (signals.size() == 0) {
> +		/* not conditional on signal set, so will generate a label
> +		 * rule as well
> +		 */
> +		buffer << "(" << "\\x" << std::setfill('0') << std::setw(2) << std::hex << AA_CLASS_LABEL << "\\x" << AA_CLASS_SIGNAL << "|";
> +	}

Why the AA_CLASS_LABEL here?


Thanks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20140321/66ed2016/attachment.pgp>