On 03/21/2014 05:48 PM, Seth Arnold wrote:
> On Mon, Mar 17, 2014 at 04:29:29PM -0700, [email protected] 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.
>
heh, I really don't like the peer=() syntax and it really is pointless on
this type of rule. That said going without it introduces an inconsistency.
currently we have
signal (send,receive) set=(kill) /profile/foo,
which doesn't feel right either
signal (send,receive) set=(kill) label=/profile/foo,
might be better.
signal (send,receive) set=(kill) peer=(/profile/foo),
could work.
But I really dislike
signal (send,receive) set=(kill) peer=(label=/profile/foo),
I could be convinced to go with it for consistency but basically
we need to look at it a little more, and decide what we want to do.
>> +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.
>
Because the of way permission are structure in the policydb we can't support
owner yet, and have a consistent clean way to map to the newer permission
format.
It will come, but I delayed most of the extended permission work for this cycle
because its just more work (sure a lot of its done but more is needed), and more
to verify.
>
>> +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.
>
Its () because set can be multi-valued. I was messed up and thinking we only
get a single signal at a time, but we could theoretically allow for multiple
and it should still be okay.
Which means we might want to update the backend encoding a little bit to
support this if it ever is actually allowed.
>
>> +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?
>
there is a double encoding going on here. Because this rule isn't conditional
on the signal. Basically a straight up label check will allow the signal, as
well as going through the signal.
The reason for the split encoding is a little more complicated but it basically
works out to efficiency, and some compatibility with older rules having their
label check encoded not at the start.
--
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
