[apparmor] Initial thoughts on profiling with signal and ptrace

[John Johansen]

On 03/24/2014 05:58 PM, Jamie Strandboge wrote:
> 
> Running the kernel and userspace from the dbus-dev ppa[1], I finally got around
> to profiling signal and ptrace with the current syntax. In general, I like it
> and it is straightforward (lack of 'label=' notwithstanding). However I have
> found myself repeating a couple of rules everywhere and I've barely gotten started.
> 
> = signal (send) set=("exists"), =
> 
> Curiously, this isn't used by all applications, but I did notice it was used by
> several gui applications (firefox, thunderbird, rhythmbox, a pygi application,
> Ubuntu Touch apps, ...).
> 
> Applications will use kill(2) or sigqueue(3) specifying '0' as the signal to see
> if a pid is running (interestingly, signal(7) didn't mention "exists"). In some
> ways I think this could probably be added to the base abstraction, but many
> applications don't use it (eg, rsyslog, qemu, dhclient, etc). Perhaps we should
> add this to the gnome abstraction and (possibly) the kde abstraction?
> 
> Interestingly, when running chromium-browser, I end up with these types of denials:
> apparmor="DENIED" operation="signal"
> profile="/usr/lib/thunderbird/thunderbird{,*[^s][^h]}" pid=13092
> comm="chromium-browse" requested_mask="receive" denied_mask="receive"
> signal=exists target="/usr/lib/chromium-browser/chromium-browser"
> 
ugh so that one is confusing, we could possible improve this
so the profile that is failing the access is
  /usr/lib/thunderbird/thunderbird{,*[^s][^h]}

It is failing to allow
  receive of an exists test

the process that is sending the signal
  /usr/lib/chromium-browser/chromium-browser


Basically this is the cross check, the reason comm="chromium-browse"
is both the send and receive check are being done in the context of the
chromium-browse processes.

So what could we change
  profile=...   except that is correct, it is the profile doing the denying

comm=   can't change that without moving the check outside of the chromium-browser
  context. And we can't do that because, we have 1 hook point into this.

target=...  We could rename this for the backcheck, as its not so much the target
  but the source

> With this in mind, perhaps the rule should be:
> 
>   signal (receive,send) set=("exists"),
> 
if we mean to allow the existence check every where then yes

> We could just have apparmor let this through and my initial thinking was that I
> thought I'd prefer that we not assume all applications need it, but after seeing
> how chromium-browser is behaving, I'm less sure mediating this one is useful (we
> could let it through be allow deny rules I guess-- would need to document it
> though).
> 
yeah I am not so fond of defaulting this to allow if we plan to mediate it ever.
If the question was default to allow within a profile (ie for signals not
crossing a process boundary) then I am more amenable to that.

Though that does run counter to how most MAC is generally set up, and is counter
to the decision from a previous IRC meeting so, while I am open to revisiting
this, it needs to be a broader discussion

> 
> = signal "<profile name>", =
> 
> Some applications like to send signals to themselves, for example, rsyslog and
> qemu. Eg:
> 
>   apparmor="DENIED" operation="signal" profile="/usr/sbin/rsyslogd" pid=700
>   comm="rsyslogd" requested_mask="receive" denied_mask="receive" signal=term
>   target="/usr/sbin/rsyslogd"
> 
>   apparmor="DENIED" operation="signal"
>   profile="libvirt-3a12a1f5-c8f5-c946-0357-021a3555aade" pid=8252
>   comm="qemu-system-x86" requested_mask="receive" denied_mask="receive"
>   signal=usr1 target="libvirt-3a12a1f5-c8f5-c946-0357-021a3555aade"
> 
>   apparmor="DENIED" operation="signal"
>   profile="libvirt-3a12a1f5-c8f5-c946-0357-021a3555aade" pid=8406
>   comm="qemu-system-x86" requested_mask="send" denied_mask="send" signal=usr1
>   target="libvirt-3a12a1f5-c8f5-c946-0357-021a3555aade"
> 
> For rsyslog, this is easy to fix with a rule like this:
> 
>   signal "/usr/sbin/rsyslogd",
> 
> However, adjusting libvirt is considerably more complicated because the profile
> name is autogenerated by virt-aa-helper based on the domain UUID. We would need
> to adjust virt-aa-helper to add a rule like this:
> 
>   signal "libvirt-3a12a1f5-c8f5-c946-0357-021a3555aade",
> 
> At first glance, this isn't difficult, but I was reminded that virt-aa-helper
> then needs to introspect the kernel to see if signal mediation is present at
> all, which I don't think we want virt-aa-helper or things like it to have to do.
> 
uh you lost me there, why does it need to introspect the kernel for this? As
long as the userspace tools support the signal rules (which is a separate issue
from the kernel supporting them), it can output a rule, and the parser will
correctly compile it when it is run.

If it is because libvirt maybe used with older apparmor userspaces, that is a
bit of a problem. Currently we don't have a good solution for this. The plan
going forward was to add a template rule that could be run after other rules
where matched which would allow the parser to throw away unknown rules.

Unfortunately that doesn't help with older userspaces, and we don't have that
ability even in the current parser.

> In all honesty, I was surprised that a process is not allowed to send signals to
> itself and found that it isn't allowed counter-intuitive (though I appreciate
> arguments to the contrary). I see a few options:
> 
It is standard MAC, explicitly list all allowed behaviors, and we do want to
support being able to deny sending certain signals etc to self. From the IRC
meeting the consensus was that default all to self was a bit surprising and
could result in missing perms that one might want to deny during profile dev.

> 1. leave it as is and add introspection code to virt-aa-helper
possible. Though I would instead suggest just having virt-aa-helper spit out
the right rules. And if there really needs to be some dynamic control just
have a file or config option.

> 2. allow processes to send themselves signals by default, but allow the use of
>    deny rules to remove them. We would need to cover this in documentation.
This change would need a broader discussion to happen, especially since it
is contrary to a previous meeting discussion about this behavior

> 3. have apparmor parser set a variable for the profile name so we can add rules
>    like 'signal "@{PROFILE_NAME}",'.
> 
This is possible, and wouldn't take much

> I prefer '2' as it is intuitive, makes profiling more straightforward and
Intuitive to who (just playing devil's advocate atm)?

In one sense it is intuitive that we want to allow an app to talk to it self.

In another it isn't so intuitive in that the profile will not contain the
full set of permissions it allows, and that you may miss out on permissions
you may want to deny during profiling because they are allowed by default.
The other argument against it is because it runs counter to the behavior or
other rules.

If we do it for signals we should do it for other forms of ipc as well, for
consistency. So the question arises about use of ptrace, sockets, pipes, ...

> doesn't make other applications like libvirt have to jump through unnecessary
> hoops. It allows for profilers to go more strict if desired (though not sure how
> useful it really is to say "I'm going to allow this app to send SIGUSR1 to
> itself but not SIGHUP").
> 
It reduces attack surface

> Based on the profiles I've done, if we decide on '1' or '3', I will definitely
> be adding 'signal "<my profile name>",' rules for Ubuntu Touch applications and
> have a feeling we'll be adding it a bunch of other places in Ubuntu. '3' is
> interesting and while it does allow us to put a generic rule in
> abstractions/libvirt-qemu such that virt-aa-helper doesn't have to be modified,
> it doesn't help with being counterintuitive.
> 
again counterintuitive for what/who, I think both ways have their perils

> If people violently object to '2', how hard would it be to do '3' in the short
> term? Even if we do '2', we probably still want '3' in the medium term since I
> think it could be useful.
> 
3 is easy. We "preseed" the variable substitution routine with a hard coded
check that returns the current profile name.  That may not be the ideal solution
long term but it can be done with just a few lines of code.

We just need to setting on what the variable name needs to be.