On 09/20/2017 08:57 AM, Jim Fehlig wrote: > On 09/20/2017 12:51 AM, Guido Günther wrote: > > Hi Jim, > > On Mon, Sep 18, 2017 at 02:06:13PM -0600, Jim Fehlig wrote: > > > Kernel 4.13 introduced finer-grained ptrace checks > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/c... > > > > > > > > > When Apparmor is enabled and libvirtd is confined, attempting to start > > > a domain fails > > > > > > virsh start test > > > error: Failed to start domain test > > > error: internal error: child reported: Kernel does not provide mount > > >         namespace: Permission denied > > > > > > The audit log contains > > > > > > type=AVC msg=audit(1505466699.828:534): apparmor="DENIED" > > > operation="ptrace" profile="/usr/sbin/libvirtd" pid=6621 > > > comm="libvirtd" requested_mask="trace" denied_mask="trace" > > > peer="/usr/sbin/libvirtd" > > > > It seems access to /proc/<pid>/tasks already requires trace permissions. > > > > > > > > It was also noticed that simply connecting to libvirtd (e.g. virsh list) > > > resulted in the following entries in the audit log > > > > > > type=AVC msg=audit(1505755799.975:65): apparmor="DENIED" > > > operation="ptrace" profile="/usr/sbin/libvirtd" pid=1418 > > > comm="libvirtd" requested_mask="trace" denied_mask="trace" > > > peer="unconfined" > > > type=AVC msg=audit(1505755799.976:66): apparmor="DENIED" > > > operation="ptrace" profile="/usr/sbin/libvirtd" pid=1418 > > > comm="libvirtd" requested_mask="trace" denied_mask="trace" > > > peer="unconfined" > > > > > > Both Apparmor denials can be fixed by adding ptrace rules to the > > > libvirtd profile. The new rules only grant trace permission. > > > > I'm seeing the same denials with 4.13 (4.13.1-1~exp1 (2017-09-11) in > > Debian) but the proposed profile change does not fix the vm start issue > > for me. I can't tell why atm, will have to look into this in more detail > > at the WE. > > I have other problems when running with 'security_default_confined = 1' > in qemu.conf, but the changes allow starting unconfined domains. > > Cedric remembered this old thread > > https://www.redhat.com/archives/libvir-list/2014-October/msg00011.html > > Some of those changes have been merged, but the ptrace, dbus, signal, > etc. have not. I used Stefan's changes to the libvirtd profile but still > see the same issue with confined domains I dug a bit further in that thread to find Stefan's most recent version of the patches https://www.redhat.com/archives/libvir-list/2014-October/msg00556.html I took the ptrace, dbus, signal, etc. changes out of patch 2 and used the attached patch to successfully start confined domains. Since a few years have passed, I'm not sure if patch 1 is still relevant. IIUC, it allows to conditionalize profile content based on apparmor version, which patch 2 uses to add some stuff if version >= 2.9. 2.9 has been out for a while...

From e3bb609812776b30acfc0349b25b2e4d539c45c2 Mon Sep 17 00:00:00 2001 From: Jim Fehlig <jfehlig(a)suse.com&gt; Date: Mon, 18 Sep 2017 13:41:26 -0600 Subject: [PATCH] apparmor: support ptrace checks Kernel 4.13 introduced finer-grained ptrace checks https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/c... When Apparmor is enabled and libvirtd is confined, attempting to start a domain fails virsh start test error: Failed to start domain test error: internal error: child reported: Kernel does not provide mount namespace: Permission denied The audit log contains type=AVC msg=audit(1505466699.828:534): apparmor="DENIED" operation="ptrace" profile="/usr/sbin/libvirtd" pid=6621 comm="libvirtd" requested_mask="trace" denied_mask="trace" peer="/usr/sbin/libvirtd" It was also noticed that simply connecting to libvirtd (e.g. virsh list) resulted in the following entries in the audit log type=AVC msg=audit(1505755799.975:65): apparmor="DENIED" operation="ptrace" profile="/usr/sbin/libvirtd" pid=1418 comm="libvirtd" requested_mask="trace" denied_mask="trace" peer="unconfined" type=AVC msg=audit(1505755799.976:66): apparmor="DENIED" operation="ptrace" profile="/usr/sbin/libvirtd" pid=1418 comm="libvirtd" requested_mask="trace" denied_mask="trace" peer="unconfined" Both Apparmor denials can be fixed by supporting ptrace in the libvirtd, qemu, and lxc profiles. While at it, also add support for dbus, signal, and unix. Resolves: https://bugzilla.suse.com/show_bug.cgi?id=1058847 Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- examples/apparmor/libvirt-lxc | 3 +++ examples/apparmor/libvirt-qemu | 3 +++ examples/apparmor/usr.sbin.libvirtd | 6 ++++++ 3 files changed, 12 insertions(+) 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) -> /, diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index dcfb1a598..6a4a2335a 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -170,6 +170,9 @@ @{PROC}/device-tree/** r, /sys/firmware/devicetree/** r, + signal (receive) peer=/usr/sbin/libvirtd, + ptrace (tracedby) peer=/usr/sbin/libvirtd,

+ # for gathering information about available host resources /sys/devices/system/cpu/ r, /sys/devices/system/node/ r, diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index acb59e071..9aadba411 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -36,6 +36,12 @@ network inet6 dgram, network packet dgram, network packet raw, + network netlink, + + dbus bus=system, + signal, + ptrace,

+ unix, # Very lenient profile for libvirtd since we want to first focus on confining # the guests. Guests will have a very restricted profile. -- 2.14.1

On Fri, 2017-09-22 at 14:52 +0200, Guido Günther wrote: > > + ptrace, > > ^^^^^^^ > > This single line is enough to make things work for me on 4.13. AFAIK > dbus mediation is not upstream yet and I think unix socket and signal > support is neither. Should we drop these for now (the syntax and > behaviour might change while things are being upsreamed)? Note that if you are upstreaming profile changes for ptrace, you may as well add them for signal and dbus because an apparmor parser that can understand 'ptrace' can understand the other two. The parser is designed to deal with kernels that don't have the full set of apparmor capabilities. The policy syntax for all of these rules should not change as part of upstreaming dbus and unix. 'unix' is probably ok to add because support for it was added to the parser in devel releases of AppArmor within 6 months of ptrace and signal. 'dbus', 'ptrace', 'signal' and 'unix' were officially introduced in 2.9[1]. By adding 'ptrace' you are saying AppArmor 2.9 is required, therefore, the other 3 are parseable.

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) -> /,

+ signal (receive) peer=/usr/sbin/libvirtd, + ptrace (tracedby) peer=/usr/sbin/libvirtd,

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!

Hi Jim, On Wed, Sep 20, 2017 at 11:17:06AM -0600, Jim Fehlig wrote: > On 09/20/2017 08:57 AM, Jim Fehlig wrote: >> On 09/20/2017 12:51 AM, Guido Günther wrote: >>> Hi Jim, >>> On Mon, Sep 18, 2017 at 02:06:13PM -0600, Jim Fehlig wrote: >>>> Kernel 4.13 introduced finer-grained ptrace checks >>>> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/c... >>>> >>>> >>>> When Apparmor is enabled and libvirtd is confined, attempting to start >>>> a domain fails >>>> >>>> virsh start test >>>> error: Failed to start domain test >>>> error: internal error: child reported: Kernel does not provide mount >>>>         namespace: Permission denied >>>> >>>> The audit log contains >>>> >>>> type=AVC msg=audit(1505466699.828:534): apparmor="DENIED" >>>> operation="ptrace" profile="/usr/sbin/libvirtd" pid=6621 >>>> comm="libvirtd" requested_mask="trace" denied_mask="trace" >>>> peer="/usr/sbin/libvirtd" >>> >>> It seems access to /proc/<pid>/tasks already requires trace permissions. >>> >>>> >>>> It was also noticed that simply connecting to libvirtd (e.g. virsh list) >>>> resulted in the following entries in the audit log >>>> >>>> type=AVC msg=audit(1505755799.975:65): apparmor="DENIED" >>>> operation="ptrace" profile="/usr/sbin/libvirtd" pid=1418 >>>> comm="libvirtd" requested_mask="trace" denied_mask="trace" >>>> peer="unconfined" >>>> type=AVC msg=audit(1505755799.976:66): apparmor="DENIED" >>>> operation="ptrace" profile="/usr/sbin/libvirtd" pid=1418 >>>> comm="libvirtd" requested_mask="trace" denied_mask="trace" >>>> peer="unconfined" >>>> >>>> Both Apparmor denials can be fixed by adding ptrace rules to the >>>> libvirtd profile. The new rules only grant trace permission. >>> >>> I'm seeing the same denials with 4.13 (4.13.1-1~exp1 (2017-09-11) in >>> Debian) but the proposed profile change does not fix the vm start issue >>> for me. I can't tell why atm, will have to look into this in more detail >>> at the WE. >> >> I have other problems when running with 'security_default_confined = 1' >> in qemu.conf, but the changes allow starting unconfined domains. >> >> Cedric remembered this old thread >> >> https://www.redhat.com/archives/libvir-list/2014-October/msg00011.html >> >> Some of those changes have been merged, but the ptrace, dbus, signal, >> etc. have not. I used Stefan's changes to the libvirtd profile but still >> see the same issue with confined domains > > I dug a bit further in that thread to find Stefan's most recent version of > the patches > > https://www.redhat.com/archives/libvir-list/2014-October/msg00556.html > > I took the ptrace, dbus, signal, etc. changes out of patch 2 and used the > attached patch to successfully start confined domains. > > Since a few years have passed, I'm not sure if patch 1 is still relevant. > IIUC, it allows to conditionalize profile content based on apparmor version, > which patch 2 uses to add some stuff if version >= 2.9. 2.9 has been out for > a while... Great, this helps a lot! > From e3bb609812776b30acfc0349b25b2e4d539c45c2 Mon Sep 17 00:00:00 2001 > From: Jim Fehlig <jfehlig(a)suse.com&gt; > Date: Mon, 18 Sep 2017 13:41:26 -0600 > Subject: [PATCH] apparmor: support ptrace checks > > Kernel 4.13 introduced finer-grained ptrace checks > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/c... > > When Apparmor is enabled and libvirtd is confined, attempting to start > a domain fails > > virsh start test > error: Failed to start domain test > error: internal error: child reported: Kernel does not provide mount > namespace: Permission denied > > The audit log contains > > type=AVC msg=audit(1505466699.828:534): apparmor="DENIED" > operation="ptrace" profile="/usr/sbin/libvirtd" pid=6621 > comm="libvirtd" requested_mask="trace" denied_mask="trace" > peer="/usr/sbin/libvirtd" > > It was also noticed that simply connecting to libvirtd (e.g. virsh list) > resulted in the following entries in the audit log > > type=AVC msg=audit(1505755799.975:65): apparmor="DENIED" > operation="ptrace" profile="/usr/sbin/libvirtd" pid=1418 > comm="libvirtd" requested_mask="trace" denied_mask="trace" > peer="unconfined" > type=AVC msg=audit(1505755799.976:66): apparmor="DENIED" > operation="ptrace" profile="/usr/sbin/libvirtd" pid=1418 > comm="libvirtd" requested_mask="trace" denied_mask="trace" > peer="unconfined" > > Both Apparmor denials can be fixed by supporting ptrace in the > libvirtd, qemu, and lxc profiles. While at it, also add support > for dbus, signal, and unix. > > Resolves: https://bugzilla.suse.com/show_bug.cgi?id=1058847 > Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; > --- > examples/apparmor/libvirt-lxc | 3 +++ > examples/apparmor/libvirt-qemu | 3 +++ > examples/apparmor/usr.sbin.libvirtd | 6 ++++++ > 3 files changed, 12 insertions(+) > > 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, Can't we get a long with a ptrace (tracedby) peer=/usr/sbin/libvirtd, here too?

> > # ignore DENIED message on / remount > deny mount options=(ro, remount) -> /, > diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu > index dcfb1a598..6a4a2335a 100644 > --- a/examples/apparmor/libvirt-qemu > +++ b/examples/apparmor/libvirt-qemu > @@ -170,6 +170,9 @@ > @{PROC}/device-tree/** r, > /sys/firmware/devicetree/** r, > > + signal (receive) peer=/usr/sbin/libvirtd, > + ptrace (tracedby) peer=/usr/sbin/libvirtd, I would have expected to need the ptrace part but don't (see below).

> + > # for gathering information about available host resources > /sys/devices/system/cpu/ r, > /sys/devices/system/node/ r, > diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd > index acb59e071..9aadba411 100644 > --- a/examples/apparmor/usr.sbin.libvirtd > +++ b/examples/apparmor/usr.sbin.libvirtd > @@ -36,6 +36,12 @@ > network inet6 dgram, > network packet dgram, > network packet raw, > + network netlink, > + > + dbus bus=system, > + signal, > + ptrace, ^^^^^^^ This single line is enough to make things work for me on 4.13.