[libvirt] [PATCH] apparmor: support finer-grained ptrace checks

[Jamie Strandboge]

On Fri, 2017-09-22 at 17:46 +0200, Guido Günther wrote:

...

> What I don't understand yet is why we have in libvirt-lxc:
> 
> > diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc
> > index 4bfb503aa..0db137de0 100644
> > --- a/examples/apparmor/libvirt-lxc
> > +++ b/examples/apparmor/libvirt-lxc
> > @@ -3,6 +3,9 @@
> >    #include <abstractions/base>
> > 
> >    umount,
> > +  dbus,
> > +  signal,
> > +  ptrace,
> > 
> >    # ignore DENIED message on / remount
> >    deny mount options=(ro, remount) -> /,
> 
> Why does it need dbus and why can't we go with a restricted ptrace and
> signal specifications like in the libvirt-qemu case:
> 
> > +  signal (receive) peer=/usr/sbin/libvirtd,
> > +  ptrace (tracedby) peer=/usr/sbin/libvirtd,
> 
> maybe having to add /usr/lib/libvirt/libvirt_lxc into the mix. (I'd
> check myself but due to the lack of ustream support I can't). Maybe just
> an omission?

I'm not sure how the skew got there, but suspect it was simply that they were
developed at different times, perhaps on different distributions initially where
is might've looked consistent with the submitter's (perhaps distro-patched)
libvirt-qemu.

All that said, in the qemu case, we know that qemu is launching a kernel and
that accesses within the guest are mediated by whatever happens to be running in
there and libvirt-qemu policy is for hypervisor. For libvirt-lxc, the host's
kernel is mediating the access for the guest, so if that kernel has, for
example, signal mediation, since the container is confined by apparmor it will
need signal rules. Ideally we wouldn't use a bare signal rule, but instead
something like:

  signal peer=@{profile_name},
  signal (receive) peer=unconfined,
  signal (receive) peer=/usr/sbin/libvirtd,

As it happens, the base abstraction in AppArmor 2.9+ has the first two rules,
so, yes, we should be able to do as you suggest for signal and just have the
last one.

The base abstraction has similar rules for unix, but I don't think we need a
'unix (receive) peer=/usr/sbin/libvirtd,' in libvirt-lxc unless there is
something in the guest that libvirtd can connect to in some manner.

As for ptrace, it is a little different because the base abstraction in 2.9+ has
this:

  ptrace (tracedby),

So this rule is not actually needed:

  ptrace (tracedby) peer=/usr/sbin/libvirtd,

The base abstraction has nothing to say about dbus (there are other abstractions
that can be used, but libvirt-lxc is not using them). dbus rules are different
than the other three because it isn't the kernel that is enforcing them, but
dbus-daemon (dbus-daemon uses libapparmor to query the kernel to ask if an
access of a connecting process is allowed or not by asking if the the policy
that is loaded for the connecting process' security label (eg, profile name)
allows it, and then dbus-daemon will do the actual allow/deny. Therefore, if
dbus-daemon in the guest has apparmor support (and it is allowed to use sysfs
for the host's apparmor within the container's mount namespace), then dbus-
daemon will need some dbus rules. I don't know if libvirt-lxc exposing enough of
sysfs for this to work all work, but if it did I *suspect* (but haven't tested
this myself) that all that would be needed would be (the parentheses are needed
for this rule):

  dbus peer=(label=@{profile_name}), 

since everything in the container will run under the same profile name, ie, that
of the container. Like with 'unix', I don't think we'd need an extra rule for
'dbus (receive) peer=(label=/usr/sbin/libvirtd),' unless there is something in
the guest that libvirtd can connect to in some manner.

Hope this helps!

-- 
Jamie Strandboge             | http://www.canonical.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170922/686c3cb5/attachment-0001.sig>