[apparmor] Initial thoughts on profiling with signal and ptrace

Tue Mar 25 03:46:38 UTC 2014

On 03/24/2014 08:40 PM, John Johansen wrote:
> 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
> 
Well, I understood the denial fine (though 'target' does feel slightly awkward
since chromium is the source, but it is possible to bend one's mind around
that), I just thought chromium's behavior odd and not sure how widespread this
will be. It seems to be only chromium, and chromium does a number of interesting
things with its sandbox.

> 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
> 
If renaming target is easy, I think that is perhaps worthwhile. It did take me a
minute to figure it out, but then I understood it well enough. Interestingly, I
almost felt like 'peer' was reasonable-- in terms of signals being sent and
received between two processes, peer almost worked in my mind....

>> 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
> 
Maybe. This was more an observation than an assertion.

>> 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.

There's both within and without, but I'm not overly concerned with this since
not all applications needed it and we could add the 'send' to an abstraction if
desired. Note that the denials I observed were mostly a single denial from a
confined application to unconfined (chromium is the outlier).

> 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
> 
I'm not looking to change things-- mostly I was commenting that we may want to
add something to an abstraction.

>>
>> = 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.
> 
The libvirt way is to query the system to see what it is capable of.
Compile-time or config file flags would almost certainly be rejected upstream
because all config options are technology agnostic (libvirt abstracts
everything). The apparmor driver uses the virt-aa-helper utility to generate
portions of the policy so it either needs to be smart (introspect) or not worry
about these types of rules, but there is also the libvirt-qemu abstraction that
we can leverage (see below).

>> 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.
> 
Sure, I get that; it just felt very odd (and it wasn't just me), but it is also
something I could get over.

>> 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
> 
Sure, but at a cost. Again, if people don't like '2', then fine, Ubuntu's policy
can tend to allow it if it makes sense.

>> 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
> 
Yes, and I tried to acknowledge that I too saw both sides.

>> 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.
> 
It sounds like this was all discussed rather extensively and the current
behavior is consistent with MAC systems and within itself. That's fine, we can
address the behavior in documentation.

I would like to have us setup the variable though, cause that will make it
easier for people and especially for libvirt since we can distro-patch the
libvirt-qemu abstraction to have a rule referencing this variable. Ubuntu can
then effectively leverage signal mediation for libvirt in 14.04.

-- 
Jamie Strandboge                 http://www.ubuntu.com/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 884 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20140324/e65fd9d8/attachment.pgp>