[PATCH v2 0/5] Further Debian/Ubuntu Apparmor Delta
Hi, I don't even remember which number of submissions that is #5 maybe? Anyway - I'm hereby continuing to bring Debian and Ubuntu apparmor Delta into upstream libvirt. I have kept out all patches that are either Distro-specific or we ran into trouble/discussions in the past. But there are enough left for a new submission. I have kept the most-original (read the earliest - as some patches appeared in Ubuntu and later with a different Author in Debian) patch author that I could find intact and git-send-email should auto-cc them. I added some more bug links and descriptions so one can understand the case a commit tries to fix without knowing too much context. Update since v1: - drop a few commits that in discussion turned out to be not/no-more needed - fixed a few typos - added the ack's that I received by Jamie Strandboge Christian Ehrhardt (1): apparmor: let qemu load old shared objects after upgrades Jamie Strandboge (1): apparmor: read only access to overcommit_memory Sam Hartman (1): apparmor: allow default pki path Stefan Bader (2): apparmor: allow libvirtd to call pygrub apparmor: qemu access to @{PROC}/*/auxv for hw_cap src/security/apparmor/libvirt-qemu | 10 ++++++++++ src/security/apparmor/usr.sbin.libvirtd.in | 1 + 2 files changed, 11 insertions(+) -- 2.27.0
From: Sam Hartman <hartmans@debian.org> /etc/pki/qemu is a pki path recommended by qemu tls docs [1] and one that can cause issues with spice connections when missing. Add the path to the allowed list of pki paths to fix the issue. Note: this is active in Debian/Ubuntu [1] for quite a while already. [1]: https://www.qemu.org/docs/master/system/tls.html [2]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=930100 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Acked-by: Jamie Strandboge <jamie@canonical.com> --- src/security/apparmor/libvirt-qemu | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 1a4b226612..2d08d6f7ad 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -94,6 +94,8 @@ /etc/pki/CA/* r, /etc/pki/libvirt{,-spice,-vnc}/ r, /etc/pki/libvirt{,-spice,-vnc}/** r, + /etc/pki/qemu/ r, + /etc/pki/qemu/** r, # the various binaries /usr/bin/kvm rmix, -- 2.27.0
From: Stefan Bader <stefan.bader@canonical.com> When using xen through libxl in Debian/Ubuntu it needs to be able to call pygrub. This is placed in a versioned path like /usr/lib/xen-4.11/bin. In theory the rule could be more strict by rendering the libexec_dir setting pkg-config can derive from libbxen-dev. But that would make particular libvirt/xen packages version-depend on each other. It seems more reasonable to avoid these versioned dependencies and use a wildcard rule instead as it is already in place for libxl-save-helper. Note: This change was in Debian [1] and Ubuntu [2] for quite some time already. [1]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=931768 [2]: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1326003 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Acked-by: Jamie Strandboge <jamie@canonical.com> --- src/security/apparmor/usr.sbin.libvirtd.in | 1 + 1 file changed, 1 insertion(+) diff --git a/src/security/apparmor/usr.sbin.libvirtd.in b/src/security/apparmor/usr.sbin.libvirtd.in index 1e137039e9..312fa4b6d1 100644 --- a/src/security/apparmor/usr.sbin.libvirtd.in +++ b/src/security/apparmor/usr.sbin.libvirtd.in @@ -86,6 +86,7 @@ profile libvirtd @sbindir@/libvirtd flags=(attach_disconnected) { /usr/{lib,lib64}/xen-common/bin/xen-toolstack PUx, /usr/{lib,lib64}/xen/bin/* Ux, /usr/lib/xen-*/bin/libxl-save-helper PUx, + /usr/lib/xen-*/bin/pygrub PUx, /usr/{lib,lib64,lib/qemu,libexec}/vhost-user-gpu PUx, # Required by nwfilter_ebiptables_driver.c:ebiptablesWriteToTempFile() to -- 2.27.0
From: Jamie Strandboge <jamie@ubuntu.com> Allow qemu to read @{PROC}/sys/vm/overcommit_memory. This is read on guest start-up and (as read-only) not a critical secret that has to stay hidden. Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Signed-off-by: Jamie Strandboge <jamie@ubuntu.com> --- src/security/apparmor/libvirt-qemu | 1 + 1 file changed, 1 insertion(+) diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index 2d08d6f7ad..b132cf0226 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -32,6 +32,7 @@ # only modify its comm value or those in its thread group. owner @{PROC}/@{pid}/task/@{tid}/comm rw, @{PROC}/sys/kernel/cap_last_cap r, + @{PROC}/sys/vm/overcommit_memory r, # For hostdev access. The actual devices will be added dynamically /sys/bus/usb/devices/ r, -- 2.27.0
From: Stefan Bader <stefan.bader@canonical.com> On some architectures (ppc, s390x, sparc, arm) qemu will read auxv to detect hardware capabilities via qemu_getauxval. Allow that access read-only for the entry owned by the current qemu process. Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Acked-by: Jamie Strandboge <jamie@canonical.com> --- src/security/apparmor/libvirt-qemu | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index b132cf0226..ae3db68f82 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -33,6 +33,8 @@ owner @{PROC}/@{pid}/task/@{tid}/comm rw, @{PROC}/sys/kernel/cap_last_cap r, @{PROC}/sys/vm/overcommit_memory r, + # detect hardware capabilities via qemu_getauxval + owner @{PROC}/*/auxv r, # For hostdev access. The actual devices will be added dynamically /sys/bus/usb/devices/ r, -- 2.27.0
Since [1] qemu can after upgrade fall back to pre-upgrade modules to still be able to dynamically load qemu-module based features. The paths for these modules are pre-defined by the code and should be allowed to be mapped and loaded from which will allow packagers avoiding the inability of late feature load [2] after package upgrades. [1]: https://github.com/qemu/qemu/commit/bd83c861 [2]: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1847361 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Acked-by: Jamie Strandboge <jamie@canonical.com> --- src/security/apparmor/libvirt-qemu | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index ae3db68f82..a03e9e2c94 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -169,6 +169,11 @@ /usr/{lib,lib64}/qemu/*.so mr, /usr/lib/@{multiarch}/qemu/*.so mr, + # let qemu load old shared objects after upgrades (LP: #1847361) + /{var/,}run/qemu/*/*.so mr, + # but explicitly deny writing to these files + audit deny /{var/,}run/qemu/*/*.so w, + # swtpm /{usr/,}bin/swtpm rmix, /usr/{lib,lib64}/libswtpm_libtpms.so mr, -- 2.27.0
On Tue, 2020-08-04 at 17:32 +0200, Christian Ehrhardt wrote:
Hi, I don't even remember which number of submissions that is #5 maybe? Anyway - I'm hereby continuing to bring Debian and Ubuntu apparmor Delta into upstream libvirt.
Thanks, I really appreciate the effort :)
I have kept out all patches that are either Distro-specific or we ran into trouble/discussions in the past. But there are enough left for a new submission.
Fair enough - let's get the uncontroversial stuff merged first, then worry about the more complicated case separately. Anyway, I'm absolutely not an AppArmor expert but the pointers you provide along with the various changes and the discussion around v1, along with the fact that these patches have been shipped in Debian and Ubuntu for so long, are convincing enough in my book, so Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
On Tue, 2020-08-04 at 23:56 +0200, Andrea Bolognani wrote:
Anyway, I'm absolutely not an AppArmor expert but the pointers you provide along with the various changes and the discussion around v1, along with the fact that these patches have been shipped in Debian and Ubuntu for so long, are convincing enough in my book, so
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
You don't seem to have pushed these yet. I can do that for you if you want, but since you are in the GitLab group with "Developer" role you should be able to do that on your own. -- Andrea Bolognani / Red Hat / Virtualization
On Fri, Aug 7, 2020 at 2:59 PM Andrea Bolognani <abologna@redhat.com> wrote:
On Tue, 2020-08-04 at 23:56 +0200, Andrea Bolognani wrote:
Anyway, I'm absolutely not an AppArmor expert but the pointers you provide along with the various changes and the discussion around v1, along with the fact that these patches have been shipped in Debian and Ubuntu for so long, are convincing enough in my book, so
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
You don't seem to have pushed these yet. I can do that for you if you want, but since you are in the GitLab group with "Developer" role you should be able to do that on your own.
Thanks for the offer, I planned to push these today giving people who would look more likely to review on the weekend a chance as well. Now pushed with all the review/ack tags I got on these changes. --
Andrea Bolognani / Red Hat / Virtualization
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd
participants (2)
-
Andrea Bolognani -
Christian Ehrhardt