[libvirt] [PATCH 00/12] Various apparmor related changes (part )
Hi, this is a continuation of the ongoing effort to feed back Ubuntu apparmor Delta on libvirt to the community (or to sort out remaining todos or to keep them distro specific). In that it is a follow on to: - https://www.redhat.com/archives/libvir-list/2017-May/msg00630.html - https://www.redhat.com/archives/libvir-list/2017-May/msg00887.html I punted those we had discussions on and decided to rework in the last rounds out of the submission. But in exchange pulled in some more changes we had that are now ready for discussion. That way I prepared the next set of 12 changes which I hereby submit for your consideration into 3.11. Christian Ehrhardt (3): apparmor, libvirt-qemu: add default pki path of lbvirt-spice apparmor, libvirt-qemu: add generic base vfio device apparmor, libvirt-qemu: qemu won't call qemu-nbd Jamie Strandboge (5): apparmor, libvirt-qemu: Allow read access to sysfs system info apparmor, libvirt-qemu: Allow qemu-block-extra libraries apparmor, libvirtd: Allow ixr to /var/lib/libvirt/virtd* apparmor, virt-aa-helper: Allow access to ecryptfs files apparmor, virt-aa-helper: Allow access to /sys/bus/usb/devices Serge Hallyn (3): apparmor, libvirt-qemu: Allow use of sgabios apparmor, libvirt-qemu: Allow read access to max_mem_regions apparmor, libvirt-qemu: Allow access to hugepage mounts Stefan Bader (1): apparmor, libvirt-qemu: Silence lttng related deny messages examples/apparmor/libvirt-qemu | 26 +++++++++++++++++++++++- examples/apparmor/usr.lib.libvirt.virt-aa-helper | 6 ++++++ examples/apparmor/usr.sbin.libvirtd | 4 ++++ 3 files changed, 35 insertions(+), 1 deletion(-) -- 2.7.4
From: Serge Hallyn <serge.hallyn@ubuntu.com> Bug-Ubuntu: https://bugs.launchpad.net/bugs/1393548 Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index d4fad85..77c72a5 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -81,6 +81,7 @@ /usr/share/proll/** r, /usr/share/vgabios/** r, /usr/share/seabios/** r, + /usr/share/misc/sgabios.bin r, /usr/share/ovmf/** r, /usr/share/OVMF/** r, /usr/share/AAVMF/** r, -- 2.7.4
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
From: Serge Hallyn <serge.hallyn@ubuntu.com>
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1393548
Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 1 + 1 file changed, 1 insertion(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index d4fad85..77c72a5 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -81,6 +81,7 @@ /usr/share/proll/** r, /usr/share/vgabios/** r, /usr/share/seabios/** r, + /usr/share/misc/sgabios.bin r, /usr/share/ovmf/** r, /usr/share/OVMF/** r, /usr/share/AAVMF/** r,
+1 to apply -- Jamie Strandboge | http://www.canonical.com
Jamie Strandboge:
--- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -81,6 +81,7 @@ /usr/share/proll/** r, /usr/share/vgabios/** r, /usr/share/seabios/** r, + /usr/share/misc/sgabios.bin r, /usr/share/ovmf/** r, /usr/share/OVMF/** r, /usr/share/AAVMF/** r,
+1 to apply
+1 as well :)
From: Stefan Bader <stefan.bader@canonical.com> Prevent denial messages related to attempted reads on lttng files from spamming the logs. Bug-Ubuntu: https://bugs.launchpad.net/bugs/1432644 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 77c72a5..651d841 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -191,3 +191,7 @@ /sys/devices/system/node/ r, /sys/devices/system/node/node[0-9]*/meminfo r, /sys/module/vhost/parameters/max_mem_regions r, + + # silence refusals to open lttng files (see LP: #1432644) + deny /dev/shm/lttng-ust-wait-* r, + deny /run/shm/lttng-ust-wait-* r, -- 2.7.4
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
From: Stefan Bader <stefan.bader@canonical.com>
Prevent denial messages related to attempted reads on lttng files from spamming the logs.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1432644
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 77c72a5..651d841 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -191,3 +191,7 @@ /sys/devices/system/node/ r, /sys/devices/system/node/node[0-9]*/meminfo r, /sys/module/vhost/parameters/max_mem_regions r, + + # silence refusals to open lttng files (see LP: #1432644) + deny /dev/shm/lttng-ust-wait-* r, + deny /run/shm/lttng-ust-wait-* r,
+1 to apply. These are noisy and not needed by typical guests. -- Jamie Strandboge | http://www.canonical.com
Hi, Christian Ehrhardt:
--- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -191,3 +191,7 @@ /sys/devices/system/node/ r, /sys/devices/system/node/node[0-9]*/meminfo r, /sys/module/vhost/parameters/max_mem_regions r, + + # silence refusals to open lttng files (see LP: #1432644) + deny /dev/shm/lttng-ust-wait-* r, + deny /run/shm/lttng-ust-wait-* r,
In principle this looks OK to me but I wonder if this is the sweet spot regarding admin UX. I've skimmed over the Ubuntu bug report but found it confusing as it mixes breakage caused by the fact we deny such access (which apparently does not happen anymore otherwise you would not be proposing these deny rules) with log flooding issues (that will be fixed by the proposed rules). So I'm afraid I need to ask an executive summary :) Under which circumstances do we log these denials? I'd like to make sure we're not creating the following situation: - In most practical cases we don't even try to access these files, so don't log denials, and then these rules are not useful. - In the rare(r) case when the admin actually enables LTT-ng debugging, with these added rules it'll be hard to discover why it does not work. Thanks in advance! Cheers, -- intrigeri
On Wed, Dec 20, 2017 at 10:30 AM, intrigeri <intrigeri+libvirt@boum.org> wrote:
Hi,
Christian Ehrhardt:
--- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -191,3 +191,7 @@ /sys/devices/system/node/ r, /sys/devices/system/node/node[0-9]*/meminfo r, /sys/module/vhost/parameters/max_mem_regions r, + + # silence refusals to open lttng files (see LP: #1432644) + deny /dev/shm/lttng-ust-wait-* r, + deny /run/shm/lttng-ust-wait-* r,
In principle this looks OK to me but I wonder if this is the sweet spot regarding admin UX.
I've skimmed over the Ubuntu bug report but found it confusing as it mixes breakage caused by the fact we deny such access (which apparently does not happen anymore otherwise you would not be proposing these deny rules) with log flooding issues (that will be fixed by the proposed rules).
So I'm afraid I need to ask an executive summary :) Under which circumstances do we log these denials?
I'd like to make sure we're not creating the following situation:
- In most practical cases we don't even try to access these files, so don't log denials, and then these rules are not useful.
- In the rare(r) case when the admin actually enables LTT-ng debugging, with these added rules it'll be hard to discover why it does not work.
Great point intrigeri! #1 At least as far as my history analysis went this was triggered by ceph having the support for lttng enabled. Not by actually (trying to) enable the LTT-ng tracking. While being disabled in ceph package since then it could show up in a similar manner from almost any other source. #2 OTOH I never have seen any complains on LTT-ng not working in the virt stack for the years carrying this delta. So either it is not an issue to those using LTT-ng or no one (statistically) uses it on virt-hosts in a case that would require it to get these access. Especially due to #1 IMHO I'd tend to add the denies as the flooding hits people not explicitly enabling/caring about LTT-ng. It would be great if instead of allow/deny we had the option to "deny but report once" - like a ratelimit, but we don't.
Thanks in advance!
Cheers, -- intrigeri
-- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
Christian Ehrhardt:
Great point intrigeri!
#1 At least as far as my history analysis went this was triggered by ceph having the support for lttng enabled. Not by actually (trying to) enable the LTT-ng tracking. While being disabled in ceph package since then it could show up in a similar manner from almost any other source.
#2 OTOH I never have seen any complains on LTT-ng not working in the virt stack for the years carrying this delta. So either it is not an issue to those using LTT-ng or no one (statistically) uses it on virt-hosts in a case that would require it to get these access.
Especially due to #1 IMHO I'd tend to add the denies as the flooding hits people not explicitly enabling/caring about LTT-ng. It would be great if instead of allow/deny we had the option to "deny but report once" - like a ratelimit, but we don't.
OK, why not then. My only remaining concern is that someone who wants to enable LTT-ng for their VMs (and somehow manages to guess that these two new rules break it) has to edit the libvirt-qemu abstraction directly: AFAIK there's no way to override them via a local/ include, because deny rules take precedence over allow rules. But anyway, we don't have any local/ include set up for this abstraction on Debian/Ubuntu currently, so for all practical matters it does not make a big difference. Thus, +1 for applying. And then let's keep our awareness level high and be ready to revert if we get bad feedback about it from !Ubuntu users :) Cheers, -- intrigeri
On Thu, Dec 21, 2017 at 11:50 AM, intrigeri <intrigeri+libvirt@boum.org> wrote:
Christian Ehrhardt:
Great point intrigeri!
#1 At least as far as my history analysis went this was triggered by ceph having the support for lttng enabled. Not by actually (trying to) enable the LTT-ng tracking. While being disabled in ceph package since then it could show up in a similar manner from almost any other source.
#2 OTOH I never have seen any complains on LTT-ng not working in the virt stack for the years carrying this delta. So either it is not an issue to those using LTT-ng or no one (statistically) uses it on virt-hosts in a case that would require it to get these access.
Especially due to #1 IMHO I'd tend to add the denies as the flooding hits people not explicitly enabling/caring about LTT-ng. It would be great if instead of allow/deny we had the option to "deny but report once" - like a ratelimit, but we don't.
OK, why not then. My only remaining concern is that someone who wants to enable LTT-ng for their VMs (and somehow manages to guess that these two new rules break it) has to edit the libvirt-qemu abstraction directly: AFAIK there's no way to override them via a local/ include, because deny rules take precedence over allow rules. But anyway, we don't have any local/ include set up for this abstraction on Debian/Ubuntu currently, so for all practical matters it does not make a big difference.
Thus, +1 for applying.
Thanks
And then let's keep our awareness level high and be ready to revert if we get bad feedback about it from !Ubuntu users :)
Ack
From: Jamie Strandboge <jamie@ubuntu.com> Newer qemu wants to read /sys/devices/system/node/ /sys/devices/system/cpu/ /sys/devices/system/node/node[0-9]*/meminfo Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 651d841..b9e45bd 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -34,6 +34,10 @@ owner @{PROC}/@{pid}/task/@{tid}/comm rw, @{PROC}/sys/kernel/cap_last_cap r, + /sys/devices/system/node/ r, + /sys/devices/system/node/node[0-9]*/meminfo r, + /sys/devices/system/cpu/ r, + # For hostdev access. The actual devices will be added dynamically /sys/bus/usb/devices/ r, /sys/devices/**/usb[0-9]*/** r, -- 2.7.4
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
From: Jamie Strandboge <jamie@ubuntu.com>
Newer qemu wants to read /sys/devices/system/node/ /sys/devices/system/cpu/ /sys/devices/system/node/node[0-9]*/meminfo
Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 651d841..b9e45bd 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -34,6 +34,10 @@ owner @{PROC}/@{pid}/task/@{tid}/comm rw, @{PROC}/sys/kernel/cap_last_cap r,
+ /sys/devices/system/node/ r, + /sys/devices/system/node/node[0-9]*/meminfo r, + /sys/devices/system/cpu/ r, +
These read accesses are fine. +1 -- Jamie Strandboge | http://www.canonical.com
Hi, Christian Ehrhardt:
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 651d841..b9e45bd 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -34,6 +34,10 @@ owner @{PROC}/@{pid}/task/@{tid}/comm rw, @{PROC}/sys/kernel/cap_last_cap r,
+ /sys/devices/system/node/ r, + /sys/devices/system/node/node[0-9]*/meminfo r, + /sys/devices/system/cpu/ r, + # For hostdev access. The actual devices will be added dynamically /sys/bus/usb/devices/ r, /sys/devices/**/usb[0-9]*/** r,
I think I've already upstream'ed this 4 months ago: commit e7f5d627f93c1c71260d2a795a1227b16b0d3186. Maybe rebase your patch series on top of the current upstream master branch? :) Cheers, -- intrigeri
On Wed, Dec 20, 2017 at 10:35 AM, intrigeri <intrigeri+libvirt@boum.org> wrote:
Hi,
Christian Ehrhardt:
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 651d841..b9e45bd 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -34,6 +34,10 @@ owner @{PROC}/@{pid}/task/@{tid}/comm rw, @{PROC}/sys/kernel/cap_last_cap r,
+ /sys/devices/system/node/ r, + /sys/devices/system/node/node[0-9]*/meminfo r, + /sys/devices/system/cpu/ r, + # For hostdev access. The actual devices will be added dynamically /sys/bus/usb/devices/ r, /sys/devices/**/usb[0-9]*/** r,
I think I've already upstream'ed this 4 months ago: commit e7f5d627f93c1c71260d2a795a1227b16b0d3186.
You did, thank you!
Maybe rebase your patch series on top of the current upstream master branch? :)
I rebased my branch before submission, but since my change was at another context it didn't break to make it obvious. I checked for duplicates in an extra tree that was on an old state of about 5 months ago :-/ TL;DR ignore this commit - already done
From: Serge Hallyn <serge.hallyn@ubuntu.com> Allows read access to /sys/module/vhost/parameters/max_mem_regions. Bug-Ubuntu: https://bugs.launchpad.net/bugs/1531564 Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 2 ++ 1 file changed, 2 insertions(+) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index b9e45bd..91d0e02 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -38,6 +38,8 @@ /sys/devices/system/node/node[0-9]*/meminfo r, /sys/devices/system/cpu/ r, + /sys/module/vhost/parameters/max_mem_regions r, + # For hostdev access. The actual devices will be added dynamically /sys/bus/usb/devices/ r, /sys/devices/**/usb[0-9]*/** r, -- 2.7.4
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
From: Serge Hallyn <serge.hallyn@ubuntu.com>
Allows read access to /sys/module/vhost/parameters/max_mem_regions.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1531564
Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index b9e45bd..91d0e02 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -38,6 +38,8 @@ /sys/devices/system/node/node[0-9]*/meminfo r, /sys/devices/system/cpu/ r,
+ /sys/module/vhost/parameters/max_mem_regions r, +
+1 to apply -- Jamie Strandboge | http://www.canonical.com
On Wed, Dec 20, 2017 at 10:35 AM, intrigeri <intrigeri@boum.org> wrote:
Christian Ehrhardt:
Allows read access to /sys/module/vhost/parameters/max_mem_regions.
Same as patch 03, already done back in August.
Yes, thank for doing so (also same reason)! TL;DR ignore this commit in the series - already done
From: Jamie Strandboge <jamie@ubuntu.com> Allows (multi-arch enabled) access to libraries under the /usr/lib/@{multiarch}/qemu/*.so path in the Debian/Ubuntu qemu-block-extra package. Bug-Ubuntu: https://bugs.launchpad.net/bugs/1554761 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- examples/apparmor/libvirt-qemu | 3 +++ 1 file changed, 3 insertions(+) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 91d0e02..912b4ac 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -161,6 +161,9 @@ /usr/{lib,lib64}/qemu/block-curl.so mr, /usr/{lib,lib64}/qemu/block-rbd.so mr, + # for Debian/Ubuntu qemu-block-extra (LP: #1554761) + /usr/lib/@{multiarch}/qemu/*.so rm, + # for use by libvirt-vnc (LP: #901272) /etc/pki/CA/ r, /etc/pki/CA/* r, -- 2.7.4
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
From: Jamie Strandboge <jamie@ubuntu.com>
Allows (multi-arch enabled) access to libraries under the /usr/lib/@{multiarch}/qemu/*.so path in the Debian/Ubuntu qemu-block-extra package.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1554761
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- examples/apparmor/libvirt-qemu | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 91d0e02..912b4ac 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -161,6 +161,9 @@ /usr/{lib,lib64}/qemu/block-curl.so mr, /usr/{lib,lib64}/qemu/block-rbd.so mr,
+ # for Debian/Ubuntu qemu-block-extra (LP: #1554761) + /usr/lib/@{multiarch}/qemu/*.so rm, +
+1 as is (though s/rm/mr/ for consistency), but on my system I see block-curl.so, block-isci.so and block-rdb.so. I think it probably makes to adjust this rule block to simply be: /usr/{lib,lib64}/qemu/*.so mr, /usr/lib/@{multiarch}/qemu/*.so mr, Ie, rather than limiting the libraries that qemu can mmap that are in its system library directory, allow qemu access to all of them and then mediate the accesses those libraries need in policy. -- Jamie Strandboge | http://www.canonical.com
On Tue, Dec 19, 2017 at 5:09 PM, Jamie Strandboge <jamie@canonical.com> wrote:
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
From: Jamie Strandboge <jamie@ubuntu.com>
Allows (multi-arch enabled) access to libraries under the /usr/lib/@{multiarch}/qemu/*.so path in the Debian/Ubuntu qemu-block-extra package.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1554761
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- examples/apparmor/libvirt-qemu | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 91d0e02..912b4ac 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -161,6 +161,9 @@ /usr/{lib,lib64}/qemu/block-curl.so mr, /usr/{lib,lib64}/qemu/block-rbd.so mr,
+ # for Debian/Ubuntu qemu-block-extra (LP: #1554761) + /usr/lib/@{multiarch}/qemu/*.so rm, +
+1 as is (though s/rm/mr/ for consistency),
ack
but on my system I see block-curl.so, block-isci.so and block-rdb.so. I think it probably makes to adjust this rule block to simply be:
Yeah the number of those so's can change anyway. The upper path is mostly for rpm systems, but e.g. SuSe is rpm+apparmor so your suggestion is great.
/usr/{lib,lib64}/qemu/*.so mr, /usr/lib/@{multiarch}/qemu/*.so mr,
From: Jamie Strandboge <jamie@ubuntu.com> Allows (multi-arch enabled) access to libraries under the /usr/lib/@{multiarch}/qemu/*.so path in the Debian/Ubuntu qemu-block-extra package and all such libs for the paths of rpm qemu-block-* packages. Bug-Ubuntu: https://bugs.launchpad.net/bugs/1554761 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- examples/apparmor/libvirt-qemu | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 91d0e02..34a564f 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -158,8 +158,9 @@ /usr/bin/qemu-sparc64 rmix, /usr/bin/qemu-unicore32 rmix, /usr/bin/qemu-x86_64 rmix, - /usr/{lib,lib64}/qemu/block-curl.so mr, - /usr/{lib,lib64}/qemu/block-rbd.so mr, + # for Debian/Ubuntu qemu-block-extra / RPMs qemu-block-* (LP: #1554761) + /usr/{lib,lib64}/qemu/*.so mr, + /usr/lib/@{multiarch}/qemu/*.so mr, # for use by libvirt-vnc (LP: #901272) /etc/pki/CA/ r, -- 2.7.4
Christian Ehrhardt:
From: Jamie Strandboge <jamie@ubuntu.com>
Allows (multi-arch enabled) access to libraries under the /usr/lib/@{multiarch}/qemu/*.so path in the Debian/Ubuntu qemu-block-extra package and all such libs for the paths of rpm qemu-block-* packages.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1554761
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- examples/apparmor/libvirt-qemu | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 91d0e02..34a564f 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -158,8 +158,9 @@ /usr/bin/qemu-sparc64 rmix, /usr/bin/qemu-unicore32 rmix, /usr/bin/qemu-x86_64 rmix, - /usr/{lib,lib64}/qemu/block-curl.so mr, - /usr/{lib,lib64}/qemu/block-rbd.so mr, + # for Debian/Ubuntu qemu-block-extra / RPMs qemu-block-* (LP: #1554761) + /usr/{lib,lib64}/qemu/*.so mr, + /usr/lib/@{multiarch}/qemu/*.so mr,
+1
On Wed, 2017-12-20 at 08:41 +0100, Christian Ehrhardt wrote:
From: Jamie Strandboge <jamie@ubuntu.com>
Allows (multi-arch enabled) access to libraries under the /usr/lib/@{multiarch}/qemu/*.so path in the Debian/Ubuntu qemu-block-extra package and all such libs for the paths of rpm qemu-block-* packages.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1554761
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- examples/apparmor/libvirt-qemu | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 91d0e02..34a564f 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -158,8 +158,9 @@ /usr/bin/qemu-sparc64 rmix, /usr/bin/qemu-unicore32 rmix, /usr/bin/qemu-x86_64 rmix, - /usr/{lib,lib64}/qemu/block-curl.so mr, - /usr/{lib,lib64}/qemu/block-rbd.so mr, + # for Debian/Ubuntu qemu-block-extra / RPMs qemu-block-* (LP: #1554761) + /usr/{lib,lib64}/qemu/*.so mr, + /usr/lib/@{multiarch}/qemu/*.so mr,
+1 to apply. Thanks for the update. :) -- Jamie Strandboge | http://www.canonical.com
From: Serge Hallyn <serge.hallyn@ubuntu.com> Allows owner access to hugepage mounts (both, the old and new systemd variant). Bug-Ubuntu: https://bugs.launchpad.net/bugs/1250216 Bug-Ubuntu: https://bugs.launchpad.net/bugs/1524737 Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 912b4ac..bb30530 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -184,6 +184,10 @@ /sys/bus/ r, /sys/class/ r, + # for access to hugepages (LP: #1250216 and LP: #1524737) + owner "/run/hugepages/kvm/libvirt/qemu/**" rw, + owner "/dev/hugepages/libvirt/qemu/**" rw, + # for rbd /etc/ceph/ceph.conf r, -- 2.7.4
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
From: Serge Hallyn <serge.hallyn@ubuntu.com>
Allows owner access to hugepage mounts (both, the old and new systemd variant).
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1250216 Bug-Ubuntu: https://bugs.launchpad.net/bugs/1524737
Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 912b4ac..bb30530 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -184,6 +184,10 @@ /sys/bus/ r, /sys/class/ r,
+ # for access to hugepages (LP: #1250216 and LP: #1524737) + owner "/run/hugepages/kvm/libvirt/qemu/**" rw, + owner "/dev/hugepages/libvirt/qemu/**" rw, +
These rules are not vm-specific. I'm not familiar with the hugepages feature in libvirt, but the rule suggests that libvirtd will create files in these directories per-vm, and then the vm uses what libvirtd created for it. I see virSecurityManagerSetHugepages() in security_manager.c, is it possible that these rules can be removed and vm-specific ones added dynamically with virt-aa-helper? -- Jamie Strandboge | http://www.canonical.com
On Tue, Dec 19, 2017 at 5:21 PM, Jamie Strandboge <jamie@canonical.com> wrote:
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
From: Serge Hallyn <serge.hallyn@ubuntu.com>
Allows owner access to hugepage mounts (both, the old and new systemd variant).
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1250216 Bug-Ubuntu: https://bugs.launchpad.net/bugs/1524737
Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 912b4ac..bb30530 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -184,6 +184,10 @@ /sys/bus/ r, /sys/class/ r,
+ # for access to hugepages (LP: #1250216 and LP: #1524737) + owner "/run/hugepages/kvm/libvirt/qemu/**" rw, + owner "/dev/hugepages/libvirt/qemu/**" rw, +
These rules are not vm-specific. I'm not familiar with the hugepages feature in libvirt, but the rule suggests that libvirtd will create files in these directories per-vm, and then the vm uses what libvirtd created for it. I see virSecurityManagerSetHugepages() in security_manager.c,
is it possible that these rules can be removed and vm-specific ones added dynamically with virt-aa-helper?
I agree - and in general it would also be nice to do a per guest rule with a full path to keep the guests away from each other. I see different ways to go thou: #1 - per security_apparmor.c The particular function you mention was generalized by [1] and further reused in [3]. The creation of the per guest paths is in [2]. So essentially it comes down to a call to domainSetPathLabel with the path as an argument. But then security_apparmor.c so far does not implement domainSetPathLabel at all. If we add a function for domainSetPathLabel it could call reload_profile with the path (as others in security_apparmor.c do). But this path is in use by other places as well: - qemuDomainWriteMasterKeyFile - qemuProcessMakeDir OTOH domainSetPathLabel is used in those places for the reson of it's meaning "add this to the scope of the security label right?" So we might after all want to add all these anyway? If that is so, an (untested) change could be like: --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -953,6 +953,13 @@ AppArmorSetSavedStateLabel(virSecurityManagerPtr mgr, return reload_profile(mgr, def, savefile, true); } +static int +AppArmorSetPathLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + const char *path) +{ + return reload_profile(mgr, def, path, true); +} static int AppArmorRestoreSavedStateLabel(virSecurityManagerPtr mgr, @@ -1045,6 +1052,8 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSavedStateLabel = AppArmorSetSavedStateLabel, .domainRestoreSavedStateLabel = AppArmorRestoreSavedStateLabel, + .domainSetPathLabel = AppArmorSetPathLabel, + .domainSetSecurityImageFDLabel = AppArmorSetFDLabel, .domainSetSecurityTapFDLabel = AppArmorSetFDLabel, #2 in virt-aa-helper with XML parsing The next option would to try to detect if hugepages are used from the xml description in virt-aa-helper. #3 unconditional per domain rule And finally we could avoid too much (?over?)engineering and unconditionally add this in virt-aa-helper (at the cost of breaking if paths generated [2] ever change): owner "/dev/hugepages/libvirt/qemu/<domainName>" rw, IMHO - [1] or [3] - please let me know your opinions before I follow any of those paths. [1]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=ce937d37105c2fc95638f4dc0d... [2]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=f55afd83b1338e17eae7ec83b7... [3]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=eff2b2edb147572b6d8f701f4b... [4]: https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/security/virt-aa-helper.... -- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
On Wed, 2017-12-20 at 14:43 +0100, Christian Ehrhardt wrote:
On Tue, Dec 19, 2017 at 5:21 PM, Jamie Strandboge <jamie@canonical.co m> wrote:
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
From: Serge Hallyn <serge.hallyn@ubuntu.com>
Allows owner access to hugepage mounts (both, the old and new systemd variant).
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1250216 Bug-Ubuntu: https://bugs.launchpad.net/bugs/1524737
Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 912b4ac..bb30530 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -184,6 +184,10 @@ /sys/bus/ r, /sys/class/ r,
+ # for access to hugepages (LP: #1250216 and LP: #1524737) + owner "/run/hugepages/kvm/libvirt/qemu/**" rw, + owner "/dev/hugepages/libvirt/qemu/**" rw, +
These rules are not vm-specific. I'm not familiar with the hugepages feature in libvirt, but the rule suggests that libvirtd will create files in these directories per-vm, and then the vm uses what libvirtd created for it. I see virSecurityManagerSetHugepages() in security_manager.c,
is it possible that these rules can be removed and vm-specific ones added dynamically with virt-aa-helper?
I agree - and in general it would also be nice to do a per guest rule with a full path to keep the guests away from each other. I see different ways to go thou:
#1 - per security_apparmor.c The particular function you mention was generalized by [1] and further reused in [3]. The creation of the per guest paths is in [2].
So essentially it comes down to a call to domainSetPathLabel with the path as an argument. But then security_apparmor.c so far does not implement domainSetPathLabel at all.
If we add a function for domainSetPathLabel it could call reload_profile with the path (as others in security_apparmor.c do). But this path is in use by other places as well: - qemuDomainWriteMasterKeyFile - qemuProcessMakeDir
OTOH domainSetPathLabel is used in those places for the reson of it's meaning "add this to the scope of the security label right?" So we might after all want to add all these anyway?
If that is so, an (untested) change could be like: --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -953,6 +953,13 @@ AppArmorSetSavedStateLabel(virSecurityManagerPtr mgr, return reload_profile(mgr, def, savefile, true); }
+static int +AppArmorSetPathLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + const char *path) +{ + return reload_profile(mgr, def, path, true); +}
static int AppArmorRestoreSavedStateLabel(virSecurityManagerPtr mgr, @@ -1045,6 +1052,8 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSavedStateLabel = AppArmorSetSavedStateLabel, .domainRestoreSavedStateLabel = AppArmorRestoreSavedStateLabel,
+ .domainSetPathLabel = AppArmorSetPathLabel, + .domainSetSecurityImageFDLabel = AppArmorSetFDLabel, .domainSetSecurityTapFDLabel = AppArmorSetFDLabel,
#2 in virt-aa-helper with XML parsing The next option would to try to detect if hugepages are used from the xml description in virt-aa-helper.
#3 unconditional per domain rule And finally we could avoid too much (?over?)engineering and unconditionally add this in virt-aa-helper (at the cost of breaking if paths generated [2] ever change): owner "/dev/hugepages/libvirt/qemu/<domainName>" rw,
IMHO - [1] or [3] - please let me know your opinions before I follow any of those paths.
[1]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=ce937d37105c2f
Or a combination of 2 and 3. Ie, if hugepages is enabled, add the rule in 3. To me, 1 feels most correct cause while the other two fix hugepages, there seem to be lurking bugs since we aren't implementing domainSetPathLabel. -- Jamie Strandboge | http://www.canonical.com
On Wed, Dec 20, 2017 at 3:34 PM, Jamie Strandboge <jamie@canonical.com> wrote:
On Wed, 2017-12-20 at 14:43 +0100, Christian Ehrhardt wrote:
On Tue, Dec 19, 2017 at 5:21 PM, Jamie Strandboge <jamie@canonical.co m> wrote:
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
From: Serge Hallyn <serge.hallyn@ubuntu.com>
Allows owner access to hugepage mounts (both, the old and new systemd variant).
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1250216 Bug-Ubuntu: https://bugs.launchpad.net/bugs/1524737
Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 912b4ac..bb30530 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -184,6 +184,10 @@ /sys/bus/ r, /sys/class/ r,
+ # for access to hugepages (LP: #1250216 and LP: #1524737) + owner "/run/hugepages/kvm/libvirt/qemu/**" rw, + owner "/dev/hugepages/libvirt/qemu/**" rw, +
These rules are not vm-specific. I'm not familiar with the hugepages feature in libvirt, but the rule suggests that libvirtd will create files in these directories per-vm, and then the vm uses what libvirtd created for it. I see virSecurityManagerSetHugepages() in security_manager.c,
is it possible that these rules can be removed and vm-specific ones added dynamically with virt-aa-helper?
I agree - and in general it would also be nice to do a per guest rule with a full path to keep the guests away from each other. I see different ways to go thou:
#1 - per security_apparmor.c The particular function you mention was generalized by [1] and further reused in [3]. The creation of the per guest paths is in [2].
So essentially it comes down to a call to domainSetPathLabel with the path as an argument. But then security_apparmor.c so far does not implement domainSetPathLabel at all.
If we add a function for domainSetPathLabel it could call reload_profile with the path (as others in security_apparmor.c do). But this path is in use by other places as well: - qemuDomainWriteMasterKeyFile - qemuProcessMakeDir
OTOH domainSetPathLabel is used in those places for the reson of it's meaning "add this to the scope of the security label right?" So we might after all want to add all these anyway?
If that is so, an (untested) change could be like: --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -953,6 +953,13 @@ AppArmorSetSavedStateLabel(virSecurityManagerPtr mgr, return reload_profile(mgr, def, savefile, true); }
+static int +AppArmorSetPathLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + const char *path) +{ + return reload_profile(mgr, def, path, true); +}
static int AppArmorRestoreSavedStateLabel(virSecurityManagerPtr mgr, @@ -1045,6 +1052,8 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSavedStateLabel = AppArmorSetSavedStateLabel, .domainRestoreSavedStateLabel = AppArmorRestoreSavedStateLabel,
+ .domainSetPathLabel = AppArmorSetPathLabel, + .domainSetSecurityImageFDLabel = AppArmorSetFDLabel, .domainSetSecurityTapFDLabel = AppArmorSetFDLabel,
#2 in virt-aa-helper with XML parsing The next option would to try to detect if hugepages are used from the xml description in virt-aa-helper.
#3 unconditional per domain rule And finally we could avoid too much (?over?)engineering and unconditionally add this in virt-aa-helper (at the cost of breaking if paths generated [2] ever change): owner "/dev/hugepages/libvirt/qemu/<domainName>" rw,
IMHO - [1] or [3] - please let me know your opinions before I follow any of those paths.
[1]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=ce937d37105c2f
Or a combination of 2 and 3. Ie, if hugepages is enabled, add the rule in 3.
To me, 1 feels most correct cause while the other two fix hugepages, there seem to be lurking bugs since we aren't implementing domainSetPathLabel.
I work on #1 a while and I think we can do a lot good here. Yet while I'm convinced at the changes this is currently a debugging nightmare. I guess it wants to become a 2018 submission. So for now keep this hugepage patch out of consideration when looking at applying all those with many +1's. I'll hopefully come back somewhen in 2018 with a dynamic handling of hugepages (and several more actually).
-- Jamie Strandboge | http://www.canonical.com
[...]
To me, 1 feels most correct cause while the other two fix hugepages, there seem to be lurking bugs since we aren't implementing domainSetPathLabel.
I work on #1 a while and I think we can do a lot good here. Yet while I'm convinced at the changes this is currently a debugging nightmare. I guess it wants to become a 2018 submission.
Note: I'll not user reply-to onto this thread to keep threading somewhat sane. Also the new submission, while inspired by this discussion, is a totally separate thing.
So for now keep this hugepage patch out of consideration when looking at applying all those with many +1's.
So as I expected the hugepage patch of this series will be covered by the new series that I submit. But I wanted to ask for all the others changes in the current series here to be considered - most have one or two acks already. Let me know if more is needed to commit them.
On Wed, 2018-01-03 at 17:55 +0100, Christian Ehrhardt wrote:
[...]
To me, 1 feels most correct cause while the other two fix hugepages, there seem to be lurking bugs since we aren't implementing domainSetPathLabel.
I work on #1 a while and I think we can do a lot good here. Yet while I'm convinced at the changes this is currently a debugging nightmare. I guess it wants to become a 2018 submission.
Note: I'll not user reply-to onto this thread to keep threading somewhat sane. Also the new submission, while inspired by this discussion, is a totally separate thing.
So for now keep this hugepage patch out of consideration when looking at applying all those with many +1's.
So as I expected the hugepage patch of this series will be covered by the new series that I submit. But I wanted to ask for all the others changes in the current series here to be considered - most have one or two acks already. Let me know if more is needed to commit them.
This is the only patch from the series that I haven't pushed or ignored... What should be done with this one? -- Cedric
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Jan 9, 2018 at 11:02 AM, Cedric Bosdonnat <cbosdonnat@suse.com> wrote:
On Wed, 2018-01-03 at 17:55 +0100, Christian Ehrhardt wrote:
[...]
To me, 1 feels most correct cause while the other two fix hugepages, there seem to be lurking bugs since we aren't implementing domainSetPathLabel.
I work on #1 a while and I think we can do a lot good here. Yet while I'm convinced at the changes this is currently a debugging nightmare. I guess it wants to become a 2018 submission.
Note: I'll not user reply-to onto this thread to keep threading somewhat sane. Also the new submission, while inspired by this discussion, is a totally separate thing.
So for now keep this hugepage patch out of consideration when looking at applying all those with many +1's.
So as I expected the hugepage patch of this series will be covered by the new series that I submit. But I wanted to ask for all the others changes in the current series here to be considered - most have one or two acks already. Let me know if more is needed to commit them.
This is the only patch from the series that I haven't pushed or ignored... What should be done with this one?
On this change here please do nothing - this is the one superseded by the new series that is in review since early january. Michael already had feedback there which I'm looking into shortly. Thank you so much for committing the acked patches of this series!
Adding the PKI path that is used as default suggestion in src/qemu/qemu.conf If people use non-default paths they should use local overrides but the suggested defaults we should open up. This is the default path as referenced by src/qemu/qemu.conf in libvirt. Bug-Ubuntu: https://bugs.launchpad.net/bugs/1690140 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- examples/apparmor/libvirt-qemu | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index bb30530..5d811f9 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -170,6 +170,10 @@ /etc/pki/libvirt/ r, /etc/pki/libvirt/** r, + # for use by libvirt-spice (LP: #1690140) + /etc/pki/libvirt-spice/ r, + /etc/pki/libvirt-spice/** r, + # for save and resume /{usr/,}bin/dash rmix, /{usr/,}bin/dd rmix, -- 2.7.4
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
Adding the PKI path that is used as default suggestion in src/qemu/qemu.conf If people use non-default paths they should use local overrides but the suggested defaults we should open up.
This is the default path as referenced by src/qemu/qemu.conf in libvirt.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1690140
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- examples/apparmor/libvirt-qemu | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index bb30530..5d811f9 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -170,6 +170,10 @@ /etc/pki/libvirt/ r, /etc/pki/libvirt/** r,
+ # for use by libvirt-spice (LP: #1690140) + /etc/pki/libvirt-spice/ r, + /etc/pki/libvirt-spice/** r, +
+1 to apply -- Jamie Strandboge | http://www.canonical.com
Jamie Strandboge:
+ # for use by libvirt-spice (LP: #1690140) + /etc/pki/libvirt-spice/ r, + /etc/pki/libvirt-spice/** r,
+1 to apply
+1 as well, although I'd prefer some minor refactoring to merge this with the 2 already existing libvirt-vnc PKI sections (that were added in two different places in the file 7 years apart, but apparently are about the exact same use case). Something like this should allow replacing these two existing sections and the third one you're proposing we add: # for use by libvirt-vnc and libvirt-spice (LP: #901272, #1690140) /etc/pki/CA/ r, /etc/pki/CA/* r, /etc/pki/libvirt{,-spice,-vnc}/ r, /etc/pki/libvirt{,-spice,-vnc}/** r, What do you think? Cheers, -- intrigeri
On Wed, Dec 20, 2017 at 10:45 AM, intrigeri <intrigeri+libvirt@boum.org> wrote:
Jamie Strandboge:
+ # for use by libvirt-spice (LP: #1690140) + /etc/pki/libvirt-spice/ r, + /etc/pki/libvirt-spice/** r,
+1 to apply
+1 as well, although I'd prefer some minor refactoring to merge this with the 2 already existing libvirt-vnc PKI sections (that were added in two different places in the file 7 years apart, but apparently are about the exact same use case).
Something like this should allow replacing these two existing sections and the third one you're proposing we add:
# for use by libvirt-vnc and libvirt-spice (LP: #901272, #1690140) /etc/pki/CA/ r, /etc/pki/CA/* r, /etc/pki/libvirt{,-spice,-vnc}/ r, /etc/pki/libvirt{,-spice,-vnc}/** r,
What do you think?
Yes I like to take the opportunity to make this more readable in one place while adding -spice. Thanks for the suggestion! Submitting as a v2 in reply ...
Adding the PKI path that is used as default suggestion in src/qemu/qemu.conf If people use non-default paths they should use local overrides but the suggested defaults we should open up. This is the default path as referenced by src/qemu/qemu.conf in libvirt. While doing so merge the several places we have to cover PKI access into one. Bug-Ubuntu: https://bugs.launchpad.net/bugs/1690140 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- examples/apparmor/libvirt-qemu | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index fa2b753..f206f6c 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -88,8 +88,11 @@ /usr/share/qemu-efi/** r, /usr/share/slof/** r, - # access PKI infrastructure - /etc/pki/libvirt-vnc/** r, + # pki for libvirt-vnc and libvirt-spice (LP: #901272, #1690140) + /etc/pki/CA/ r, + /etc/pki/CA/* r, + /etc/pki/libvirt{,-spice,-vnc}/ r, + /etc/pki/libvirt{,-spice,-vnc}/** r, # the various binaries /usr/bin/kvm rmix, @@ -156,12 +159,6 @@ /usr/{lib,lib64}/qemu/*.so mr, /usr/lib/@{multiarch}/qemu/*.so mr, - # for use by libvirt-vnc (LP: #901272) - /etc/pki/CA/ r, - /etc/pki/CA/* r, - /etc/pki/libvirt/ r, - /etc/pki/libvirt/** r, - # for save and resume /{usr/,}bin/dash rmix, /{usr/,}bin/dd rmix, -- 2.7.4
Christian Ehrhardt:
Adding the PKI path that is used as default suggestion in src/qemu/qemu.conf If people use non-default paths they should use local overrides but the suggested defaults we should open up.
This is the default path as referenced by src/qemu/qemu.conf in libvirt.
While doing so merge the several places we have to cover PKI access into one.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1690140
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Looks good, thanks for the refactoring ⇒ +1 for applying.
On Wed, 2017-12-20 at 12:41 +0100, Christian Ehrhardt wrote:
Adding the PKI path that is used as default suggestion in src/qemu/qemu.conf If people use non-default paths they should use local overrides but the suggested defaults we should open up.
This is the default path as referenced by src/qemu/qemu.conf in libvirt.
While doing so merge the several places we have to cover PKI access into one.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1690140
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- examples/apparmor/libvirt-qemu | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index fa2b753..f206f6c 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -88,8 +88,11 @@ /usr/share/qemu-efi/** r, /usr/share/slof/** r,
- # access PKI infrastructure - /etc/pki/libvirt-vnc/** r, + # pki for libvirt-vnc and libvirt-spice (LP: #901272, #1690140) + /etc/pki/CA/ r, + /etc/pki/CA/* r, + /etc/pki/libvirt{,-spice,-vnc}/ r, + /etc/pki/libvirt{,-spice,-vnc}/** r,
# the various binaries /usr/bin/kvm rmix, @@ -156,12 +159,6 @@ /usr/{lib,lib64}/qemu/*.so mr, /usr/lib/@{multiarch}/qemu/*.so mr,
- # for use by libvirt-vnc (LP: #901272) - /etc/pki/CA/ r, - /etc/pki/CA/* r, - /etc/pki/libvirt/ r, - /etc/pki/libvirt/** r, - # for save and resume /{usr/,}bin/dash rmix, /{usr/,}bin/dd rmix,
+1 to apply. Thanks for the patch and intrigeri for the feedback. -- Jamie Strandboge | http://www.canonical.com
vfio devices are generated on the fly, but the generic base is missing. The base vfio has not much functionality but to provide a custom container by opening this path. See https://www.kernel.org/doc/Documentation/vfio.txt for more. Current access by qemu is "wr": [ 2652.756712] audit: type=1400 audit(1491303691.719:25): apparmor="DENIED" operation="open" profile="libvirt-17a61b87-5132-497c-b928-421ac2ee0c8a" name="/dev/vfio/vfio" pid=8486 comm="qemu-system-x86" requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=0 Bug-Ubuntu: https://bugs.launchpad.net/bugs/1678322 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 3 +++ 1 file changed, 3 insertions(+) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 5d811f9..eb4d58c 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -212,3 +212,6 @@ # silence refusals to open lttng files (see LP: #1432644) deny /dev/shm/lttng-ust-wait-* r, deny /run/shm/lttng-ust-wait-* r, + + # for vfio (LP: #1678322) + /dev/vfio/vfio rw, -- 2.7.4
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
vfio devices are generated on the fly, but the generic base is missing.
The base vfio has not much functionality but to provide a custom container by opening this path. See https://www.kernel.org/doc/Documentation/vfio.txt for more.
Current access by qemu is "wr": [ 2652.756712] audit: type=1400 audit(1491303691.719:25): apparmor="DENIED" operation="open" profile="libvirt-17a61b87-5132-497c-b928-421ac2ee0c8a" name="/dev/vfio/vfio" pid=8486 comm="qemu-system-x86" requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=0
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1678322
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 5d811f9..eb4d58c 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -212,3 +212,6 @@ # silence refusals to open lttng files (see LP: #1432644) deny /dev/shm/lttng-ust-wait-* r, deny /run/shm/lttng-ust-wait-* r, + + # for vfio (LP: #1678322) + /dev/vfio/vfio rw,
Why not just also add this rule iff there is a vfio-specific device rule? Ie, just add this along with the vfio device rule in virt-aa- helper instead of giving all VMs access when it isn't needed. -- Jamie Strandboge | http://www.canonical.com
On Tue, Dec 19, 2017 at 5:26 PM, Jamie Strandboge <jamie@canonical.com> wrote:
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
vfio devices are generated on the fly, but the generic base is missing.
The base vfio has not much functionality but to provide a custom container by opening this path. See https://www.kernel.org/doc/Documentation/vfio.txt for more.
Current access by qemu is "wr": [ 2652.756712] audit: type=1400 audit(1491303691.719:25): apparmor="DENIED" operation="open" profile="libvirt-17a61b87-5132-497c-b928-421ac2ee0c8a" name="/dev/vfio/vfio" pid=8486 comm="qemu-system-x86" requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=0
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1678322
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 5d811f9..eb4d58c 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -212,3 +212,6 @@ # silence refusals to open lttng files (see LP: #1432644) deny /dev/shm/lttng-ust-wait-* r, deny /run/shm/lttng-ust-wait-* r, + + # for vfio (LP: #1678322) + /dev/vfio/vfio rw,
Why not just also add this rule iff there is a vfio-specific device rule? Ie, just add this along with the vfio device rule in virt-aa- helper instead of giving all VMs access when it isn't needed.
Good suggestion, but I realized that - other than the per-device rules that I knew - we already do so for the base dev as well [1]. Thanks for making me aware. One thing that puzzles me still is that I hit this in 2017 on a rather new libvirt. Maybe on that case the detection was broken and thereby the rule missing - I'll go back and take a look again and might come up with a fix for that logic then. TL;DR - this is just old garbage, I'm dropping this on my end - yay cleanup ! [1]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=74e86b6b2521881808bb93290b...
-- Jamie Strandboge | http://www.canonical.com
On Wed, Dec 20, 2017 at 12:58 PM, Christian Ehrhardt < christian.ehrhardt@canonical.com> wrote:
On Tue, Dec 19, 2017 at 5:26 PM, Jamie Strandboge <jamie@canonical.com> wrote:
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
vfio devices are generated on the fly, but the generic base is missing.
The base vfio has not much functionality but to provide a custom container by opening this path. See https://www.kernel.org/doc/Documentation/vfio.txt for more.
Current access by qemu is "wr": [ 2652.756712] audit: type=1400 audit(1491303691.719:25): apparmor="DENIED" operation="open" profile="libvirt-17a61b87-5132-497c-b928-421ac2ee0c8a" name="/dev/vfio/vfio" pid=8486 comm="qemu-system-x86" requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=0
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1678322
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 5d811f9..eb4d58c 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -212,3 +212,6 @@ # silence refusals to open lttng files (see LP: #1432644) deny /dev/shm/lttng-ust-wait-* r, deny /run/shm/lttng-ust-wait-* r, + + # for vfio (LP: #1678322) + /dev/vfio/vfio rw,
Why not just also add this rule iff there is a vfio-specific device rule? Ie, just add this along with the vfio device rule in virt-aa- helper instead of giving all VMs access when it isn't needed.
Good suggestion, but I realized that - other than the per-device rules that I knew - we already do so for the base dev as well [1]. Thanks for making me aware.
One thing that puzzles me still is that I hit this in 2017 on a rather new libvirt. Maybe on that case the detection was broken and thereby the rule missing - I'll go back and take a look again and might come up with a fix for that logic then.
TL;DR - this is just old garbage, I'm dropping this on my end - yay cleanup !
I finally found why we still need the change, giving a TL;DR here and then submitting a refreshed patch. If a guest description has no hostdev's defined, then in the current virt-aa-helper code it will not add /dev/vfio/vfio nor any per vfio device rules. That works for guests that have a) no hostdevs b) hostdevs defined in the guest xml It even works for c) hostdevs defined in the guest xml, later more hostdevs hotplugged But there is another case d) guest has no hostdev in guest xml, later hostdevs get hotplugged In case (d) virt-aa-helper did not add the rule since at the time there was no need. And the way libvirt/qemu currently play together on vfio hotplug leads to the deny of /dev/vfio/vfio as any labeling hooks or similar I have seen are too late. If we allow the rather safe /dev/vfio/vfio which is not giving you any device access anyway (see kernel doc) we are good. In that case qemu will be allowed to open vfio to get its own vfio space. Later libvirt will catch all that is needed and a labeling call lets virt-aa-helper add the right vfio device. So we end up with the proposed safe allowance here in the global profile. And the actual device only being in the per guest profile. /dev/vfio/93 re-submitting the patch in a few minutes ...
[1]: https://libvirt.org/git/?p=libvirt.git;a=commit;h= 74e86b6b2521881808bb93290bcebcb469ab7820
-- Jamie Strandboge | http://www.canonical.com
-- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
While libvirtd might do so, qemu itself as a guest will not need to call qemu-nbd so remove it from the profile. Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index eb4d58c..2bffb04 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -144,7 +144,6 @@ /usr/bin/qemu-mipsel rmix, /usr/bin/qemu-mipsn32 rmix, /usr/bin/qemu-mipsn32el rmix, - /usr/bin/qemu-nbd rmix, /usr/bin/qemu-or32 rmix, /usr/bin/qemu-ppc rmix, /usr/bin/qemu-ppc64 rmix, -- 2.7.4
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
While libvirtd might do so, qemu itself as a guest will not need to call qemu-nbd so remove it from the profile.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 1 - 1 file changed, 1 deletion(-)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index eb4d58c..2bffb04 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -144,7 +144,6 @@ /usr/bin/qemu-mipsel rmix, /usr/bin/qemu-mipsn32 rmix, /usr/bin/qemu-mipsn32el rmix, - /usr/bin/qemu-nbd rmix, /usr/bin/qemu-or32 rmix, /usr/bin/qemu-ppc rmix, /usr/bin/qemu-ppc64 rmix,
Nice catch. +1 to apply. -- Jamie Strandboge | http://www.canonical.com
From: Jamie Strandboge <jamie@ubuntu.com> This is required for the ebtables functionality added in libvirt 0.8.0. Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/usr.sbin.libvirtd | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index 8d61d15..2b6b33a 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -76,6 +76,10 @@ /usr/{lib,lib64}/xen/bin/* Ux, /usr/lib/xen-*/bin/libxl-save-helper PUx, + # Required by nwfilter_ebiptables_driver.c:ebiptablesWriteToTempFile() to + # write and run an ebtables script. + /var/lib/libvirt/virtd* ixr, + # force the use of virt-aa-helper audit deny /{usr/,}sbin/apparmor_parser rwxl, audit deny /etc/apparmor.d/libvirt/** wxl, -- 2.7.4
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
From: Jamie Strandboge <jamie@ubuntu.com>
This is required for the ebtables functionality added in libvirt 0.8.0.
Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/usr.sbin.libvirtd | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index 8d61d15..2b6b33a 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -76,6 +76,10 @@ /usr/{lib,lib64}/xen/bin/* Ux, /usr/lib/xen-*/bin/libxl-save-helper PUx,
+ # Required by nwfilter_ebiptables_driver.c:ebiptablesWriteToTempFile() to + # write and run an ebtables script. + /var/lib/libvirt/virtd* ixr, +
s/write and run/read and run/, then +1 to apply. The ixr rule makes it so anything that is executed in there inherits this profile. This profile is of course super-lenient, but the ix means if in the future we choose to go more strict, the virtd scripts will also be affected. -- Jamie Strandboge | http://www.canonical.com
From: Jamie Strandboge <jamie@ubuntu.com> Bug-Ubuntu: https://bugs.launchpad.net/bugs/591769 Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/usr.lib.libvirt.virt-aa-helper | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper b/examples/apparmor/usr.lib.libvirt.virt-aa-helper index bd6181d..d63c844 100644 --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper @@ -47,6 +47,10 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { audit deny @{HOME}/bin/** mrwkl, @{HOME}/ r, @{HOME}/** r, + # Alow access to ecryptfs files (LP: #591769) + @{HOME}/.Private/** mrwlk, + @{HOMEDIRS}/.ecryptfs/*/.Private/** mrwlk, + /var/lib/libvirt/images/ r, /var/lib/libvirt/images/** r, /{media,mnt,opt,srv}/** r, -- 2.7.4
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
From: Jamie Strandboge <jamie@ubuntu.com>
Bug-Ubuntu: https://bugs.launchpad.net/bugs/591769
Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/usr.lib.libvirt.virt-aa-helper | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper b/examples/apparmor/usr.lib.libvirt.virt-aa-helper index bd6181d..d63c844 100644 --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper @@ -47,6 +47,10 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { audit deny @{HOME}/bin/** mrwkl, @{HOME}/ r, @{HOME}/** r, + # Alow access to ecryptfs files (LP: #591769) + @{HOME}/.Private/** mrwlk, + @{HOMEDIRS}/.ecryptfs/*/.Private/** mrwlk, +
Hrmm, these rules were never meant to last as long as they have. That said, they are already a part of the AppArmor base abstraction (using owner match though) and virt-aa-helper uses '#include <abstractions/base>'. Are these rules still needed considering the base abstraction? I imagine at worst virt-aa-helper would only need 'r' for some of these... -- Jamie Strandboge | http://www.canonical.com
Jamie Strandboge:
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
+ # Alow access to ecryptfs files (LP: #591769) + @{HOME}/.Private/** mrwlk, + @{HOMEDIRS}/.ecryptfs/*/.Private/** mrwlk,
Hrmm, these rules were never meant to last as long as they have. That said, they are already a part of the AppArmor base abstraction (using owner match though) and virt-aa-helper uses '#include <abstractions/base>'. Are these rules still needed considering the base abstraction? I imagine at worst virt-aa-helper would only need 'r' for some of these...
I concur with Jamie: I'd rather can avoid spreading copies of these rules around if we can. Cheers, -- intrigeri
On Wed, Dec 20, 2017 at 10:50 AM, intrigeri <intrigeri+libvirt@boum.org> wrote:
Jamie Strandboge:
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
+ # Alow access to ecryptfs files (LP: #591769) + @{HOME}/.Private/** mrwlk, + @{HOMEDIRS}/.ecryptfs/*/.Private/** mrwlk,
Hrmm, these rules were never meant to last as long as they have. That said, they are already a part of the AppArmor base abstraction (using owner match though) and virt-aa-helper uses '#include <abstractions/base>'. Are these rules still needed considering the base abstraction? I imagine at worst virt-aa-helper would only need 'r' for some of these...
I concur with Jamie: I'd rather can avoid spreading copies of these rules around if we can.
Checked as well - no more needed. Thanks for the hint I missed that those were moved into the base abstraction.
From: Jamie Strandboge <jamie@ubuntu.com> Required to generate correct profiles when using usb passthrough. Bug-Ubuntu: https://bugs.launchpad.net/bugs/565691 Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/usr.lib.libvirt.virt-aa-helper | 2 ++ 1 file changed, 2 insertions(+) diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper b/examples/apparmor/usr.lib.libvirt.virt-aa-helper index d63c844..de92291 100644 --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper @@ -22,6 +22,8 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { # for hostdev /sys/devices/ r, /sys/devices/** r, + /sys/bus/usb/devices/ r, + /sys/bus/usb/devices/** r, deny /dev/sd* r, deny /dev/vd* r, deny /dev/dm-* r, -- 2.7.4
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
From: Jamie Strandboge <jamie@ubuntu.com>
Required to generate correct profiles when using usb passthrough.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/565691
Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/usr.lib.libvirt.virt-aa-helper | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper b/examples/apparmor/usr.lib.libvirt.virt-aa-helper index d63c844..de92291 100644 --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper @@ -22,6 +22,8 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { # for hostdev /sys/devices/ r, /sys/devices/** r, + /sys/bus/usb/devices/ r, + /sys/bus/usb/devices/** r, deny /dev/sd* r, deny /dev/vd* r, deny /dev/dm-* r,
Note that only the first rule is needed since /sys/bus/usb/devices/ is a symlink farm (at least these days). AppArmor resolves symlinks before applying mediation so the glob rule won't ever match anything. +1 for first rule. -- Jamie Strandboge | http://www.canonical.com
Hi, Jamie Strandboge:
On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote:
examples/apparmor/usr.lib.libvirt.virt-aa-helper | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper b/examples/apparmor/usr.lib.libvirt.virt-aa-helper index d63c844..de92291 100644 --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper @@ -22,6 +22,8 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { # for hostdev /sys/devices/ r, /sys/devices/** r, + /sys/bus/usb/devices/ r, + /sys/bus/usb/devices/** r, deny /dev/sd* r, deny /dev/vd* r, deny /dev/dm-* r,
Note that only the first rule is needed since /sys/bus/usb/devices/ is a symlink farm (at least these days). AppArmor resolves symlinks before applying mediation so the glob rule won't ever match anything.
+1 for first rule.
+1 for the first rule as well.
From: Jamie Strandboge <jamie@ubuntu.com> Required to generate correct profiles when using usb passthrough. Bug-Ubuntu: https://bugs.launchpad.net/bugs/565691 Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Acked-by: Jamie Strandboge <jamie@ubuntu.com> Acked-by: Intrigeri <intrigeri@boum.org> --- examples/apparmor/usr.lib.libvirt.virt-aa-helper | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper b/examples/apparmor/usr.lib.libvirt.virt-aa-helper index d63c844..6a6558e 100644 --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper @@ -22,6 +22,7 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { # for hostdev /sys/devices/ r, /sys/devices/** r, + /sys/bus/usb/devices/ r, deny /dev/sd* r, deny /dev/vd* r, deny /dev/dm-* r, -- 2.7.4
On Wed, 2017-12-20 at 11:56 +0100, Christian Ehrhardt wrote:
From: Jamie Strandboge <jamie@ubuntu.com>
Required to generate correct profiles when using usb passthrough.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/565691
Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Acked-by: Jamie Strandboge <jamie@ubuntu.com> Acked-by: Intrigeri <intrigeri@boum.org> --- examples/apparmor/usr.lib.libvirt.virt-aa-helper | 1 + 1 file changed, 1 insertion(+)
diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper b/examples/apparmor/usr.lib.libvirt.virt-aa-helper index d63c844..6a6558e 100644 --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper @@ -22,6 +22,7 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { # for hostdev /sys/devices/ r, /sys/devices/** r, + /sys/bus/usb/devices/ r,
+1 to apply -- Jamie Strandboge | http://www.canonical.com
I beg your pardon - too much open edit's at once - should have been "part 3" in the subject :-)
participants (5)
-
Cedric Bosdonnat -
Christian Ehrhardt -
intrigeri -
intrigeri -
Jamie Strandboge