[libvirt] [PATCH] AppArmor: add rules needed with additional mediation features brought by Linux 4.14.
--- examples/apparmor/libvirt-qemu | 2 ++ examples/apparmor/usr.sbin.libvirtd | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index b341e31f42..5994a35042 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -16,6 +16,8 @@ network inet stream, network inet6 stream, + signal (receive) set=("term") peer=/usr/sbin/libvirtd, + /dev/net/tun rw, /dev/kvm rw, /dev/ptmx rw, diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index 819068ffc3..17b5ee38ff 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -30,6 +30,8 @@ # Needed for vfio capability sys_resource, + mount, + network inet stream, network inet dgram, network inet6 stream, @@ -37,11 +39,18 @@ network packet dgram, network packet raw, + network netlink raw, + network unix dgram, + network unix stream, + ptrace (trace) peer=unconfined, ptrace (trace) peer=/usr/sbin/libvirtd, ptrace (trace) peer=/usr/sbin/dnsmasq, ptrace (trace) peer=libvirt-*, + signal (send) set=("hup") peer=/usr/sbin/dnsmasq, + signal (send) set=("term") peer=libvirt-*, + # Very lenient profile for libvirtd since we want to first focus on confining # the guests. Guests will have a very restricted profile. / r, -- 2.15.0.rc2
[PATCH v2] AppArmor: add rules needed with additional mediation features Changes since v1: - remove unneeded "network unix" rules added by v1: they were only needed due to a bug in apparmor_parser, that was fixed in AppArmor 2.11.1 since then; - move the "network netlink raw" rule to honor previous sorting. Note that the "mount" rule is very broad. It could be replaced with a set of more specific rules in the future. A draft is available on https://bugzilla.opensuse.org/show_bug.cgi?id=1065123, that should be tested on various distros and configurations before it is submitted upstream. But let's not block on this and focus first on avoiding breakage when distros ship Linux 4.14: this is not a regression given so far we had no mount mediation at all (except in Ubuntu that carries out-of-tree patches).
From: intrigeri <intrigeri+libvirt@boum.org> --- examples/apparmor/libvirt-qemu | 2 ++ examples/apparmor/usr.sbin.libvirtd | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index b341e31f42..5994a35042 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -16,6 +16,8 @@ network inet stream, network inet6 stream, + signal (receive) set=("term") peer=/usr/sbin/libvirtd, + /dev/net/tun rw, /dev/kvm rw, /dev/ptmx rw, diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index 819068ffc3..eb24726e08 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -30,10 +30,13 @@ # Needed for vfio capability sys_resource, + mount, + network inet stream, network inet dgram, network inet6 stream, network inet6 dgram, + network netlink raw, network packet dgram, network packet raw, @@ -42,6 +45,9 @@ ptrace (trace) peer=/usr/sbin/dnsmasq, ptrace (trace) peer=libvirt-*, + signal (send) set=("hup") peer=/usr/sbin/dnsmasq, + signal (send) set=("term") peer=libvirt-*, + # Very lenient profile for libvirtd since we want to first focus on confining # the guests. Guests will have a very restricted profile. / r, -- 2.15.0.rc2
On Thu, 2017-10-26 at 10:22 +0000, intrigeri+libvirt@boum.org wrote:
From: intrigeri <intrigeri+libvirt@boum.org>
--- examples/apparmor/libvirt-qemu | 2 ++ examples/apparmor/usr.sbin.libvirtd | 6 ++++++ 2 files changed, 8 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index b341e31f42..5994a35042 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -16,6 +16,8 @@ network inet stream, network inet6 stream,
+ signal (receive) set=("term") peer=/usr/sbin/libvirtd,
I suggest this rule instead: signal (receive) peer=/usr/sbin/libvirtd, ie, let libvirtd send any signals it wants to its VMs.
/dev/net/tun rw, /dev/kvm rw, /dev/ptmx rw, diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index 819068ffc3..eb24726e08 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -30,10 +30,13 @@ # Needed for vfio capability sys_resource,
+ mount, +
This is interesting since the Ubuntu profile is missing mount rules. What specific denials/libvirt actions prompted this rule?
network inet stream, network inet dgram, network inet6 stream, network inet6 dgram, + network netlink raw,
This is fine.
network packet dgram, network packet raw,
@@ -42,6 +45,9 @@ ptrace (trace) peer=/usr/sbin/dnsmasq, ptrace (trace) peer=libvirt-*,
+ signal (send) set=("hup") peer=/usr/sbin/dnsmasq,
I suspect you are missing 'term' to support net-destroy. I suggest this instead: signal (send) peer=/usr/sbin/dnsmasq, Ie, let libvirtd send any signals to fully manage its dnsmasq.
+ signal (send) set=("term") peer=libvirt-*,
I suggest this instead: signal (send) peer=libvirt-*, Ie, let libvirtd send any signals to its VMs. I think you are missing this in libvirt-qemu: ptrace (readby, tracedby) peer=/usr/sbin/libvirtd, and this in usr.sbin.libvirtd: ptrace (read, trace) peer=libvirt-*, -- Jamie Strandboge | http://www.canonical.com
On Thu, 2017-10-26 at 08:39 -0500, Jamie Strandboge wrote:
On Thu, 2017-10-26 at 10:22 +0000, intrigeri+libvirt@boum.org wrote:
diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index 819068ffc3..eb24726e08 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -30,10 +30,13 @@ # Needed for vfio capability sys_resource,
+ mount, +
This is interesting since the Ubuntu profile is missing mount rules. What specific denials/libvirt actions prompted this rule?
Responding to myself now that I read the SUSE bug. I actually suggest using the fine-grained rules in the SUSE patch because it is much easier to add more rules for more access than to take them away. These rules are in the 'examples' directory so I think it is expected that a distribution may need to tailor them from time to time (hopefully upstreaming their changes! :). -- Jamie Strandboge | http://www.canonical.com
Hi, thanks Jamie for this review. All your suggestions make sense to me, I'll implement + test them and will re-submit as v3. Cheers, -- intrigeri
Changes since v2: - made signal rules broader, as suggested by Jamie Strandboge <jamie@canonical.com> and indeed my tests confirm v2 was too strict; - allowed libvirtd "ptrace (read)" on libvirt-* guests, as suggested by Jamie Strandboge <jamie@canonical.com> - added fine-grained mount rules written by openSUSE's Christian Boltz
From: intrigeri <intrigeri+libvirt@boum.org> --- examples/apparmor/libvirt-qemu | 4 ++++ examples/apparmor/usr.sbin.libvirtd | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 97dd2d45a9..9d487bf92f 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -16,6 +16,10 @@ network inet stream, network inet6 stream, + ptrace (readby, tracedby) peer=/usr/sbin/libvirtd, + + signal (receive) peer=/usr/sbin/libvirtd, + /dev/net/tun rw, /dev/kvm rw, /dev/ptmx rw, diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index 819068ffc3..d2831aa491 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -30,10 +30,13 @@ # Needed for vfio capability sys_resource, + mount, + network inet stream, network inet dgram, network inet6 stream, network inet6 dgram, + network netlink raw, network packet dgram, network packet raw, @@ -42,6 +45,9 @@ ptrace (trace) peer=/usr/sbin/dnsmasq, ptrace (trace) peer=libvirt-*, + signal (send) peer=/usr/sbin/dnsmasq, + signal (read, send) peer=libvirt-*, + # Very lenient profile for libvirtd since we want to first focus on confining # the guests. Guests will have a very restricted profile. / r, -- 2.15.0
On 11/05/2017 08:29 AM, intrigeri+libvirt@boum.org wrote:
From: intrigeri <intrigeri+libvirt@boum.org>
--- examples/apparmor/libvirt-qemu | 4 ++++ examples/apparmor/usr.sbin.libvirtd | 6 ++++++ 2 files changed, 10 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 97dd2d45a9..9d487bf92f 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -16,6 +16,10 @@ network inet stream, network inet6 stream,
+ ptrace (readby, tracedby) peer=/usr/sbin/libvirtd, + + signal (receive) peer=/usr/sbin/libvirtd, + /dev/net/tun rw, /dev/kvm rw, /dev/ptmx rw, diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index 819068ffc3..d2831aa491 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -30,10 +30,13 @@ # Needed for vfio capability sys_resource,
+ mount, +
I suppose this isn't needed here since it is removed in 2/2? Regards, Jim
network inet stream, network inet dgram, network inet6 stream, network inet6 dgram, + network netlink raw, network packet dgram, network packet raw,
@@ -42,6 +45,9 @@ ptrace (trace) peer=/usr/sbin/dnsmasq, ptrace (trace) peer=libvirt-*,
+ signal (send) peer=/usr/sbin/dnsmasq, + signal (read, send) peer=libvirt-*, + # Very lenient profile for libvirtd since we want to first focus on confining # the guests. Guests will have a very restricted profile. / r,
On Sun, 2017-11-05 at 15:29 +0000, intrigeri+libvirt@boum.org wrote:
From: intrigeri <intrigeri+libvirt@boum.org>
--- examples/apparmor/libvirt-qemu | 4 ++++ examples/apparmor/usr.sbin.libvirtd | 6 ++++++ 2 files changed, 10 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 97dd2d45a9..9d487bf92f 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -16,6 +16,10 @@ network inet stream, network inet6 stream,
+ ptrace (readby, tracedby) peer=/usr/sbin/libvirtd, + + signal (receive) peer=/usr/sbin/libvirtd, +
These LGTM
/dev/net/tun rw, /dev/kvm rw, /dev/ptmx rw, diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index 819068ffc3..d2831aa491 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -30,10 +30,13 @@ # Needed for vfio capability sys_resource,
+ mount, +
Yuck, but fixed in 2/2. Better might've been to skip this rule and add all the mount rules in 2/2.
network inet stream, network inet dgram, network inet6 stream, network inet6 dgram, + network netlink raw,
Looks fine. Almost certainly needed for udev.
network packet dgram, network packet raw,
@@ -42,6 +45,9 @@ ptrace (trace) peer=/usr/sbin/dnsmasq, ptrace (trace) peer=libvirt-*,
+ signal (send) peer=/usr/sbin/dnsmasq, + signal (read, send) peer=libvirt-*, +
LGTM, thanks! -- Jamie Strandboge | http://www.canonical.com
From: intrigeri <intrigeri+libvirt@boum.org> This set of rules was proposed by Christian Boltz <apparmor@cboltz.de> on https://bugzilla.opensuse.org/show_bug.cgi?id=1065123. --- examples/apparmor/usr.sbin.libvirtd | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index d2831aa491..8d61d154e1 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -30,7 +30,20 @@ # Needed for vfio capability sys_resource, - mount, + mount options=(rw,rslave) -> /, + mount options=(rw, nosuid) -> /{var/,}run/libvirt/qemu/*.dev/, + + mount options=(rw, move) /dev/ -> /{var/,}run/libvirt/qemu/*.dev/, + mount options=(rw, move) /dev/hugepages/ -> /{var/,}run/libvirt/qemu/*.hugepages/, + mount options=(rw, move) /dev/mqueue/ -> /{var/,}run/libvirt/qemu/*.mqueue/, + mount options=(rw, move) /dev/pts/ -> /{var/,}run/libvirt/qemu/*.pts/, + mount options=(rw, move) /dev/shm/ -> /{var/,}run/libvirt/qemu/*.shm/, + + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.dev/ -> /dev/, + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.hugepages/ -> /dev/hugepages/, + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.mqueue/ -> /dev/mqueue/, + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.pts/ -> /dev/pts/, + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.shm/ -> /dev/shm/, network inet stream, network inet dgram, -- 2.15.0
On Sun, 2017-11-05 at 15:29 +0000, intrigeri+libvirt@boum.org wrote:
From: intrigeri <intrigeri+libvirt@boum.org>
This set of rules was proposed by Christian Boltz <apparmor@cboltz.de
on https://bugzilla.opensuse.org/show_bug.cgi?id=1065123. --- examples/apparmor/usr.sbin.libvirtd | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index d2831aa491..8d61d154e1 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -30,7 +30,20 @@ # Needed for vfio capability sys_resource,
- mount, + mount options=(rw,rslave) -> /, + mount options=(rw, nosuid) -> /{var/,}run/libvirt/qemu/*.dev/, + + mount options=(rw, move) /dev/ -> /{var/,}run/libvirt/qemu/*.dev/, + mount options=(rw, move) /dev/hugepages/ -> /{var/,}run/libvirt/qemu/*.hugepages/, + mount options=(rw, move) /dev/mqueue/ -> /{var/,}run/libvirt/qemu/*.mqueue/, + mount options=(rw, move) /dev/pts/ -> /{var/,}run/libvirt/qemu/*.pts/, + mount options=(rw, move) /dev/shm/ -> /{var/,}run/libvirt/qemu/*.shm/, + + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.dev/ -> /dev/, + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.hugepages/ -> /dev/hugepages/, + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.mqueue/ -> /dev/mqueue/, + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.pts/ -> /dev/pts/, + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.shm/ -> /dev/shm/,
These all look fine. I suspect the rules may need to be fine-tuned for different distributions, but that's ok, this is example policy and we can iterate/adjust as needed. -- Jamie Strandboge | http://www.canonical.com
Changes since v3: - don't add in 1/2 blanket catch-all mount rule that 2/2 was replacing anyway
From: intrigeri <intrigeri+libvirt@boum.org> --- examples/apparmor/libvirt-qemu | 4 ++++ examples/apparmor/usr.sbin.libvirtd | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 064501f08e..73bdbae872 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -16,6 +16,10 @@ network inet stream, network inet6 stream, + ptrace (readby, tracedby) peer=/usr/sbin/libvirtd, + + signal (receive) peer=/usr/sbin/libvirtd, + /dev/net/tun rw, /dev/kvm rw, /dev/ptmx rw, diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index 819068ffc3..12b9d45bf0 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -34,6 +34,7 @@ network inet dgram, network inet6 stream, network inet6 dgram, + network netlink raw, network packet dgram, network packet raw, @@ -42,6 +43,9 @@ ptrace (trace) peer=/usr/sbin/dnsmasq, ptrace (trace) peer=libvirt-*, + signal (send) peer=/usr/sbin/dnsmasq, + signal (read, send) peer=libvirt-*, + # Very lenient profile for libvirtd since we want to first focus on confining # the guests. Guests will have a very restricted profile. / r, -- 2.15.0
From: intrigeri <intrigeri+libvirt@boum.org> This set of rules was proposed by Christian Boltz <apparmor@cboltz.de> on https://bugzilla.opensuse.org/show_bug.cgi?id=1065123. --- examples/apparmor/usr.sbin.libvirtd | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index 12b9d45bf0..8d61d154e1 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -30,6 +30,21 @@ # Needed for vfio capability sys_resource, + mount options=(rw,rslave) -> /, + mount options=(rw, nosuid) -> /{var/,}run/libvirt/qemu/*.dev/, + + mount options=(rw, move) /dev/ -> /{var/,}run/libvirt/qemu/*.dev/, + mount options=(rw, move) /dev/hugepages/ -> /{var/,}run/libvirt/qemu/*.hugepages/, + mount options=(rw, move) /dev/mqueue/ -> /{var/,}run/libvirt/qemu/*.mqueue/, + mount options=(rw, move) /dev/pts/ -> /{var/,}run/libvirt/qemu/*.pts/, + mount options=(rw, move) /dev/shm/ -> /{var/,}run/libvirt/qemu/*.shm/, + + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.dev/ -> /dev/, + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.hugepages/ -> /dev/hugepages/, + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.mqueue/ -> /dev/mqueue/, + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.pts/ -> /dev/pts/, + mount options=(rw, move) /{var/,}run/libvirt/qemu/*.shm/ -> /dev/shm/, + network inet stream, network inet dgram, network inet6 stream, -- 2.15.0
Hi, On Sun, Nov 19, 2017 at 02:57:32PM +0000, intrigeri+libvirt@boum.org wrote:
Changes since v3:
- don't add in 1/2 blanket catch-all mount rule that 2/2 was replacing anyway
Pushed now. Thanks! -- Guido
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (5)
-
Guido Günther -
intrigeri -
intrigeri+libvirt＠boum.org
-
Jamie Strandboge -
Jim Fehlig