[libvirt] Various apparmor related changes (part 1), version 2
Over the years there have been a bunch of changes to the apparmor profiles and/or virt-aa-helper which have been carried in Debian/Ubuntu but never made it upstream.
In an attempt to clean this up and generally improve the apparmor based environments, we (Christian and I) went over the changes, cleaned out cruft as much as possible and would be sending out hunks of changes to this list for upstream inclusion.
I hope doing multiple but smaller rounds of submissions will make it simpler to get those reviewed and hopefully accepted.
For the second version I added acks, merged the patches related to explicit device denials and local apparmor profiles, and split the 9p support one (holding back the part allowing link access for later or to be replaced by a safer solution). I also tried to improve the explanation in the description of patch #1 (virt-aa-helper: Ask for no deny rule for readonly disk elements). Thanks, Stefan
From: Serge Hallyn <serge.hallyn@ubuntu.com> Just because a disk element only requests read access doesn't mean there may not be another readwrite request. Using 'R' when creating the apparmor rule will prevent an implicit write-deny rule to be created alongside. This does not mean write is allowed but it would cause a denial message and probably more relevant, allows to add write access later. Bug-Ubuntu: https://bugs.launchpad.net/bugs/1554031 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- src/security/virt-aa-helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 5f5d1cd..d976a00 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -888,11 +888,11 @@ add_file_path(virDomainDiskDefPtr disk, if (depth == 0) { if (disk->src->readonly) - ret = vah_add_file(buf, path, "r"); + ret = vah_add_file(buf, path, "R"); else ret = vah_add_file(buf, path, "rw"); } else { - ret = vah_add_file(buf, path, "r"); + ret = vah_add_file(buf, path, "R"); } if (ret != 0) -- 2.7.4
From: Simon McVittie <smcv@debian.org> The split firmware and variables files introduced by https://bugs.debian.org/764918 are in a different directory for some reason. Let the virtual machine read both. Extended by Christian Ehrhardt to generalize FW test (simplifies additional testing on firmware files in future). Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Acked-by: Guido Günther <agx@sigxcpu.org> --- examples/apparmor/libvirt-qemu | 1 + src/security/virt-aa-helper.c | 1 + tests/virt-aa-helper-test | 24 ++++++++++++++++-------- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index a9020aa..e0988bb 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -70,6 +70,7 @@ /usr/share/vgabios/** r, /usr/share/seabios/** r, /usr/share/ovmf/** r, + /usr/share/OVMF/** r, # access PKI infrastructure /etc/pki/libvirt-vnc/** r, diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index d976a00..dd166c2 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -512,6 +512,7 @@ valid_path(const char *path, const bool readonly) "/vmlinuz", "/initrd", "/initrd.img", + "/usr/share/OVMF/", /* for OVMF images */ "/usr/share/ovmf/" /* for OVMF images */ }; /* override the above with these */ diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index 68e9399..73f3080 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -145,6 +145,20 @@ testme() { fi } +testfw() { + title="$1" + fwpath="$2" + + if [ -f "$fwpath" ]; then + sed -e "s,###UUID###,$uuid,g" \ + -e "s,###DISK###,$disk1,g" \ + -e "s,</os>,<loader readonly='yes' type='pflash'>$fwpath</loader></os>,g" "$template_xml" > "$test_xml" + testme "0" "$title" "-r -u $valid_uuid" "$test_xml" + else + echo "Skipping FW $title test. Could not find $fwpath" + fi +} + # Expected failures echo "Expected failures:" >$output testme "1" "invalid arg" "-z" @@ -291,14 +305,8 @@ sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</os>,<kernel>$tm touch "$tmpdir/kernel" testme "0" "kernel" "-r -u $valid_uuid" "$test_xml" -if [ -f /usr/share/ovmf/OVMF.fd ]; then - sed -e "s,###UUID###,$uuid,g" \ - -e "s,###DISK###,$disk1,g" \ - -e "s,</os>,<loader readonly='yes' type='pflash'>/usr/share/ovmf/OVMF.fd</loader></os>,g" "$template_xml" > "$test_xml" - testme "0" "ovmf" "-r -u $valid_uuid" "$test_xml" -else - echo "Skipping OVMF test. Could not find /usr/share/ovmf/OVMF.fd" -fi +testfw "ovmf (old path)" "/usr/share/ovmf/OVMF.fd" +testfw "OVMF (new path)" "/usr/share/OVMF/OVMF_CODE.fd" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</os>,<initrd>$tmpdir/initrd</initrd></os>,g" "$template_xml" > "$test_xml" touch "$tmpdir/initrd" -- 2.7.4
Hi Stefan, On Thu, May 18, 2017 at 10:53:40AM +0200, Stefan Bader wrote:
From: Simon McVittie <smcv@debian.org>
The split firmware and variables files introduced by https://bugs.debian.org/764918 are in a different directory for some reason. Let the virtual machine read both.
Extended by Christian Ehrhardt to generalize FW test (simplifies additional testing on firmware files in future).
If you want to credit this separately I suggest to split the ode that itroduces testfw into one commit (attributed to Christian) and the code that adds read access to OVMF into another one (attributed to Simon). Cheers, -- Guido
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Acked-by: Guido Günther <agx@sigxcpu.org> --- examples/apparmor/libvirt-qemu | 1 + src/security/virt-aa-helper.c | 1 + tests/virt-aa-helper-test | 24 ++++++++++++++++-------- 3 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index a9020aa..e0988bb 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -70,6 +70,7 @@ /usr/share/vgabios/** r, /usr/share/seabios/** r, /usr/share/ovmf/** r, + /usr/share/OVMF/** r,
# access PKI infrastructure /etc/pki/libvirt-vnc/** r, diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index d976a00..dd166c2 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -512,6 +512,7 @@ valid_path(const char *path, const bool readonly) "/vmlinuz", "/initrd", "/initrd.img", + "/usr/share/OVMF/", /* for OVMF images */ "/usr/share/ovmf/" /* for OVMF images */ }; /* override the above with these */ diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index 68e9399..73f3080 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -145,6 +145,20 @@ testme() { fi }
+testfw() { + title="$1" + fwpath="$2" + + if [ -f "$fwpath" ]; then + sed -e "s,###UUID###,$uuid,g" \ + -e "s,###DISK###,$disk1,g" \ + -e "s,</os>,<loader readonly='yes' type='pflash'>$fwpath</loader></os>,g" "$template_xml" > "$test_xml" + testme "0" "$title" "-r -u $valid_uuid" "$test_xml" + else + echo "Skipping FW $title test. Could not find $fwpath" + fi +} + # Expected failures echo "Expected failures:" >$output testme "1" "invalid arg" "-z" @@ -291,14 +305,8 @@ sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</os>,<kernel>$tm touch "$tmpdir/kernel" testme "0" "kernel" "-r -u $valid_uuid" "$test_xml"
-if [ -f /usr/share/ovmf/OVMF.fd ]; then - sed -e "s,###UUID###,$uuid,g" \ - -e "s,###DISK###,$disk1,g" \ - -e "s,</os>,<loader readonly='yes' type='pflash'>/usr/share/ovmf/OVMF.fd</loader></os>,g" "$template_xml" > "$test_xml" - testme "0" "ovmf" "-r -u $valid_uuid" "$test_xml" -else - echo "Skipping OVMF test. Could not find /usr/share/ovmf/OVMF.fd" -fi +testfw "ovmf (old path)" "/usr/share/ovmf/OVMF.fd" +testfw "OVMF (new path)" "/usr/share/OVMF/OVMF_CODE.fd"
sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</os>,<initrd>$tmpdir/initrd</initrd></os>,g" "$template_xml" > "$test_xml" touch "$tmpdir/initrd" -- 2.7.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 19.05.2017 09:46, Guido Günther wrote:
Hi Stefan, On Thu, May 18, 2017 at 10:53:40AM +0200, Stefan Bader wrote:
From: Simon McVittie <smcv@debian.org>
The split firmware and variables files introduced by https://bugs.debian.org/764918 are in a different directory for some reason. Let the virtual machine read both.
Extended by Christian Ehrhardt to generalize FW test (simplifies additional testing on firmware files in future).
If you want to credit this separately I suggest to split the ode that itroduces testfw into one commit (attributed to Christian) and the code that adds read access to OVMF into another one (attributed to Simon).
Though Simon already added some testing (just limited to the one addition made then). I guess I could re-submit Simon's patch as it was and create one additionally which only changes the testing (for future use). Which then the next (3/8) uses.
Cheers, -- Guido
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Acked-by: Guido Günther <agx@sigxcpu.org> --- examples/apparmor/libvirt-qemu | 1 + src/security/virt-aa-helper.c | 1 + tests/virt-aa-helper-test | 24 ++++++++++++++++-------- 3 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index a9020aa..e0988bb 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -70,6 +70,7 @@ /usr/share/vgabios/** r, /usr/share/seabios/** r, /usr/share/ovmf/** r, + /usr/share/OVMF/** r,
# access PKI infrastructure /etc/pki/libvirt-vnc/** r, diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index d976a00..dd166c2 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -512,6 +512,7 @@ valid_path(const char *path, const bool readonly) "/vmlinuz", "/initrd", "/initrd.img", + "/usr/share/OVMF/", /* for OVMF images */ "/usr/share/ovmf/" /* for OVMF images */ }; /* override the above with these */ diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index 68e9399..73f3080 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -145,6 +145,20 @@ testme() { fi }
+testfw() { + title="$1" + fwpath="$2" + + if [ -f "$fwpath" ]; then + sed -e "s,###UUID###,$uuid,g" \ + -e "s,###DISK###,$disk1,g" \ + -e "s,</os>,<loader readonly='yes' type='pflash'>$fwpath</loader></os>,g" "$template_xml" > "$test_xml" + testme "0" "$title" "-r -u $valid_uuid" "$test_xml" + else + echo "Skipping FW $title test. Could not find $fwpath" + fi +} + # Expected failures echo "Expected failures:" >$output testme "1" "invalid arg" "-z" @@ -291,14 +305,8 @@ sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</os>,<kernel>$tm touch "$tmpdir/kernel" testme "0" "kernel" "-r -u $valid_uuid" "$test_xml"
-if [ -f /usr/share/ovmf/OVMF.fd ]; then - sed -e "s,###UUID###,$uuid,g" \ - -e "s,###DISK###,$disk1,g" \ - -e "s,</os>,<loader readonly='yes' type='pflash'>/usr/share/ovmf/OVMF.fd</loader></os>,g" "$template_xml" > "$test_xml" - testme "0" "ovmf" "-r -u $valid_uuid" "$test_xml" -else - echo "Skipping OVMF test. Could not find /usr/share/ovmf/OVMF.fd" -fi +testfw "ovmf (old path)" "/usr/share/ovmf/OVMF.fd" +testfw "OVMF (new path)" "/usr/share/OVMF/OVMF_CODE.fd"
sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</os>,<initrd>$tmpdir/initrd</initrd></os>,g" "$template_xml" > "$test_xml" touch "$tmpdir/initrd" -- 2.7.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
From: William Grant <wgrant@ubuntu.com> Allow access to aarch64 UEFI images. Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Acked-by: Guido Günther <agx@sigxcpu.org> --- examples/apparmor/libvirt-qemu | 2 ++ src/security/virt-aa-helper.c | 4 +++- tests/virt-aa-helper-test | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index e0988bb..89466c9 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -71,6 +71,8 @@ /usr/share/seabios/** r, /usr/share/ovmf/** r, /usr/share/OVMF/** r, + /usr/share/AAVMF/** r, + /usr/share/qemu-efi/** r, # access PKI infrastructure /etc/pki/libvirt-vnc/** r, diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index dd166c2..a2d5c21 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -513,7 +513,9 @@ valid_path(const char *path, const bool readonly) "/initrd", "/initrd.img", "/usr/share/OVMF/", /* for OVMF images */ - "/usr/share/ovmf/" /* for OVMF images */ + "/usr/share/ovmf/", /* for OVMF images */ + "/usr/share/AAVMF/", /* for AAVMF images */ + "/usr/share/qemu-efi/" /* for AAVMF images */ }; /* override the above with these */ const char * const override[] = { diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index 73f3080..51072f6 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -307,6 +307,8 @@ testme "0" "kernel" "-r -u $valid_uuid" "$test_xml" testfw "ovmf (old path)" "/usr/share/ovmf/OVMF.fd" testfw "OVMF (new path)" "/usr/share/OVMF/OVMF_CODE.fd" +testfw "AAVMF" "/usr/share/AAVMF/AAVMF_CODE.fd" +testfw "qemu-efi" "/usr/share/qemu-efi/QEMU_EFI.fd" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</os>,<initrd>$tmpdir/initrd</initrd></os>,g" "$template_xml" > "$test_xml" touch "$tmpdir/initrd" -- 2.7.4
From: Felix Geyer <fgeyer@debian.org> Allow access to libnl-3 config files Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Acked-by: Guido Günther <agx@sigxcpu.org> --- 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 4a8f197..ee53c2c 100644 --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper @@ -16,6 +16,8 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { owner @{PROC}/[0-9]*/status r, @{PROC}/filesystems r, + /etc/libnl-3/classid r, + # for hostdev /sys/devices/ r, /sys/devices/** r, -- 2.7.4
On Thu, May 18, 2017 at 10:53:42AM +0200, Stefan Bader wrote:
From: Felix Geyer <fgeyer@debian.org>
Allow access to libnl-3 config files
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Acked-by: Guido Günther <agx@sigxcpu.org> --- 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 4a8f197..ee53c2c 100644 --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper @@ -16,6 +16,8 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { owner @{PROC}/[0-9]*/status r, @{PROC}/filesystems r,
+ /etc/libnl-3/classid r, + # for hostdev /sys/devices/ r, /sys/devices/** r, --
Pushed. Thanks! -- Guido
From: Felix Geyer <fgeyer@debian.org> Add explicit denies for disk devices to avoid cluttering dmesg with (acceptable) denials (merged with a second patch which added more disk device names). Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Acked-by: Guido Günther <agx@sigxcpu.org> --- examples/apparmor/usr.lib.libvirt.virt-aa-helper | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper b/examples/apparmor/usr.lib.libvirt.virt-aa-helper index ee53c2c..012080c 100644 --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper @@ -21,6 +21,15 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { # for hostdev /sys/devices/ r, /sys/devices/** r, + deny /dev/sd* r, + deny /dev/vd* r, + deny /dev/dm-* r, + deny /dev/drbd[0-9]* r, + deny /dev/dasd* r, + deny /dev/nvme* r, + deny /dev/zd[0-9]* r, + deny /dev/mapper/ r, + deny /dev/mapper/* r, /usr/{lib,lib64}/libvirt/virt-aa-helper mr, /{usr/,}sbin/apparmor_parser Ux, -- 2.7.4
On Thu, May 18, 2017 at 10:53:43AM +0200, Stefan Bader wrote:
From: Felix Geyer <fgeyer@debian.org>
Add explicit denies for disk devices to avoid cluttering dmesg with (acceptable) denials (merged with a second patch which added more disk device names).
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Acked-by: Guido Günther <agx@sigxcpu.org> --- examples/apparmor/usr.lib.libvirt.virt-aa-helper | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper b/examples/apparmor/usr.lib.libvirt.virt-aa-helper index ee53c2c..012080c 100644 --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper @@ -21,6 +21,15 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { # for hostdev /sys/devices/ r, /sys/devices/** r, + deny /dev/sd* r, + deny /dev/vd* r, + deny /dev/dm-* r, + deny /dev/drbd[0-9]* r, + deny /dev/dasd* r, + deny /dev/nvme* r, + deny /dev/zd[0-9]* r, + deny /dev/mapper/ r, + deny /dev/mapper/* r,
/usr/{lib,lib64}/libvirt/virt-aa-helper mr, /{usr/,}sbin/apparmor_parser Ux,
Pushed. Thanks! -- Guido
-- 2.7.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
From: Felix Geyer <fgeyer@debian.org> Local overrides is a feature Debian/Ubuntu libvirt provided for a while. This allows the user to have a non-conffile that he can use to extend the package delivered rules with extra content matching his special case. This change adds the include directives to the apparmor profiles for virt-aa-helper and libvirtd. Additionally extended the build environment to carry template local profiles and install them into the correct places. Without that the include directives would prevent the profile from loading. 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> --- examples/Makefile.am | 14 ++++++++++++++ examples/apparmor/local-usr.lib.libvirt.virt-aa-helper | 2 ++ examples/apparmor/local-usr.sbin.libvirtd | 2 ++ examples/apparmor/usr.lib.libvirt.virt-aa-helper | 3 +++ examples/apparmor/usr.sbin.libvirtd | 3 +++ 5 files changed, 24 insertions(+) create mode 100644 examples/apparmor/local-usr.lib.libvirt.virt-aa-helper create mode 100644 examples/apparmor/local-usr.sbin.libvirtd diff --git a/examples/Makefile.am b/examples/Makefile.am index 2956e14..16c7bf6 100644 --- a/examples/Makefile.am +++ b/examples/Makefile.am @@ -25,6 +25,8 @@ EXTRA_DIST = \ apparmor/libvirt-lxc \ apparmor/usr.lib.libvirt.virt-aa-helper \ apparmor/usr.sbin.libvirtd \ + apparmor/local-usr.sbin.libvirtd \ + apparmor/local-usr.lib.libvirt.virt-aa-helper \ lxcconvert/virt-lxc-convert \ polkit/libvirt-acl.rules \ $(wildcard $(srcdir)/systemtap/*.stp) \ @@ -74,6 +76,18 @@ apparmor_DATA = \ apparmor/usr.sbin.libvirtd \ $(NULL) +localdir = $(apparmordir)/local +local_DATA = \ + apparmor/local-usr.sbin.libvirtd \ + apparmor/local-usr.lib.libvirt.virt-aa-helper \ + $(NULL) + +install-data-hook: + mv $(DESTDIR)$(localdir)/local-usr.sbin.libvirtd \ + $(DESTDIR)$(localdir)/usr.sbin.libvirtd + mv $(DESTDIR)$(localdir)/local-usr.lib.libvirt.virt-aa-helper \ + $(DESTDIR)$(localdir)/usr.lib.libvirt.virt-aa-helper + abstractionsdir = $(apparmordir)/abstractions abstractions_DATA = \ apparmor/libvirt-qemu \ diff --git a/examples/apparmor/local-usr.lib.libvirt.virt-aa-helper b/examples/apparmor/local-usr.lib.libvirt.virt-aa-helper new file mode 100644 index 0000000..82c9c39 --- /dev/null +++ b/examples/apparmor/local-usr.lib.libvirt.virt-aa-helper @@ -0,0 +1,2 @@ +# Site-specific additions and overrides for usr.lib.libvirt.virt-aa-helper. +# For more details, please see /etc/apparmor.d/local/README. diff --git a/examples/apparmor/local-usr.sbin.libvirtd b/examples/apparmor/local-usr.sbin.libvirtd new file mode 100644 index 0000000..6e19f20 --- /dev/null +++ b/examples/apparmor/local-usr.sbin.libvirtd @@ -0,0 +1,2 @@ +# Site-specific additions and overrides for usr.sbin.libvirtd. +# For more details, please see /etc/apparmor.d/local/README. diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper b/examples/apparmor/usr.lib.libvirt.virt-aa-helper index 012080c..93ba74e 100644 --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper @@ -56,4 +56,7 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { /**.vmdk r, /**.[iI][sS][oO] r, /**/disk{,.*} r, + + # Site-specific additions and overrides. See local/README for details. + #include <local/usr.lib.libvirt.virt-aa-helper> } diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index 353b039..c37d5ee 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -85,4 +85,7 @@ /usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper rmix, } + + # Site-specific additions and overrides. See local/README for details. + #include <local/usr.sbin.libvirtd> } -- 2.7.4
From: Serge Hallyn <serge.hallyn@ubuntu.com> Add fowner and fsetid to libvirt-qemu profile. Bug-Ubuntu: https://bugs.launchpad.net/bugs/1378434 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 89466c9..f04ce04 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -13,6 +13,10 @@ capability setgid, capability setuid, + # for 9p + capability fsetid, + capability fowner, + network inet stream, network inet6 stream, -- 2.7.4
Mind you I'm not crazy about this. If this could be toggled with a default-off config option that would seem better than always giving these caps to libvirt-qemu. Quoting Stefan Bader (stefan.bader@canonical.com):
From: Serge Hallyn <serge.hallyn@ubuntu.com>
Add fowner and fsetid to libvirt-qemu profile.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1378434
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 89466c9..f04ce04 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -13,6 +13,10 @@ capability setgid, capability setuid,
+ # for 9p + capability fsetid, + capability fowner, + network inet stream, network inet6 stream,
-- 2.7.4
On Thu, May 18, 2017 at 11:21:54AM -0500, Serge E. Hallyn wrote:
Mind you I'm not crazy about this. If this could be toggled with a default-off config option that would seem better than always giving these caps to libvirt-qemu.
virt-aa-helper could add these if it detects a 9pfs file system. That would be better than always adding it. Cheers, -- Guido
Quoting Stefan Bader (stefan.bader@canonical.com):
From: Serge Hallyn <serge.hallyn@ubuntu.com>
Add fowner and fsetid to libvirt-qemu profile.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1378434
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 89466c9..f04ce04 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -13,6 +13,10 @@ capability setgid, capability setuid,
+ # for 9p + capability fsetid, + capability fowner, + network inet stream, network inet6 stream,
-- 2.7.4
Quoting Guido Günther (agx@sigxcpu.org):
On Thu, May 18, 2017 at 11:21:54AM -0500, Serge E. Hallyn wrote:
Mind you I'm not crazy about this. If this could be toggled with a default-off config option that would seem better than always giving these caps to libvirt-qemu.
virt-aa-helper could add these if it detects a 9pfs file system. That would be better than always adding it.
Agreed
Cheers, -- Guido
Quoting Stefan Bader (stefan.bader@canonical.com):
From: Serge Hallyn <serge.hallyn@ubuntu.com>
Add fowner and fsetid to libvirt-qemu profile.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1378434
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 89466c9..f04ce04 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -13,6 +13,10 @@ capability setgid, capability setuid,
+ # for 9p + capability fsetid, + capability fowner, + network inet stream, network inet6 stream,
-- 2.7.4
On 18.05.2017 21:40, Serge E. Hallyn wrote:
Quoting Guido Günther (agx@sigxcpu.org):
On Thu, May 18, 2017 at 11:21:54AM -0500, Serge E. Hallyn wrote:
Mind you I'm not crazy about this. If this could be toggled with a default-off config option that would seem better than always giving these caps to libvirt-qemu.
virt-aa-helper could add these if it detects a 9pfs file system. That would be better than always adding it.
Agreed
Ok, so at least for now, actually all 9p related changes should not be considered. Does the rest look ok (in particular 1/8 with the additional explanation)? -Stefan
Cheers, -- Guido
Quoting Stefan Bader (stefan.bader@canonical.com):
From: Serge Hallyn <serge.hallyn@ubuntu.com>
Add fowner and fsetid to libvirt-qemu profile.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1378434
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 89466c9..f04ce04 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -13,6 +13,10 @@ capability setgid, capability setuid,
+ # for 9p + capability fsetid, + capability fowner, + network inet stream, network inet6 stream,
-- 2.7.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
From: Serge Hallyn <serge.hallyn@ubuntu.com> Updates profile to allow running on ppc64el. Bug-Ubuntu: https://bugs.launchpad.net/bugs/1374554 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index f04ce04..2791dfc 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -77,6 +77,7 @@ /usr/share/OVMF/** r, /usr/share/AAVMF/** r, /usr/share/qemu-efi/** r, + /usr/share/slof/** r, # access PKI infrastructure /etc/pki/libvirt-vnc/** r, @@ -102,6 +103,7 @@ /usr/bin/qemu-system-or32 rmix, /usr/bin/qemu-system-ppc rmix, /usr/bin/qemu-system-ppc64 rmix, + /usr/bin/qemu-system-ppc64le rmix, /usr/bin/qemu-system-ppcemb rmix, /usr/bin/qemu-system-s390x rmix, /usr/bin/qemu-system-sh4 rmix, @@ -158,3 +160,8 @@ /etc/udev/udev.conf r, /sys/bus/ r, /sys/class/ r, + + # for ppc device-tree access + @{PROC}/device-tree/ r, + @{PROC}/device-tree/** r, + /sys/firmware/devicetree/** r, -- 2.7.4
On Thu, May 18, 2017 at 10:53:46AM +0200, Stefan Bader wrote:
From: Serge Hallyn <serge.hallyn@ubuntu.com>
Updates profile to allow running on ppc64el.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1374554
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- examples/apparmor/libvirt-qemu | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index f04ce04..2791dfc 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -77,6 +77,7 @@ /usr/share/OVMF/** r, /usr/share/AAVMF/** r, /usr/share/qemu-efi/** r, + /usr/share/slof/** r,
# access PKI infrastructure /etc/pki/libvirt-vnc/** r, @@ -102,6 +103,7 @@ /usr/bin/qemu-system-or32 rmix, /usr/bin/qemu-system-ppc rmix, /usr/bin/qemu-system-ppc64 rmix, + /usr/bin/qemu-system-ppc64le rmix, /usr/bin/qemu-system-ppcemb rmix, /usr/bin/qemu-system-s390x rmix, /usr/bin/qemu-system-sh4 rmix, @@ -158,3 +160,8 @@ /etc/udev/udev.conf r, /sys/bus/ r, /sys/class/ r, + + # for ppc device-tree access + @{PROC}/device-tree/ r, + @{PROC}/device-tree/** r, + /sys/firmware/devicetree/** r, -- 2.7.4
LGTM but I don't know much about PPC64, it's SLOF and where the device tree should be located. Cheers, -- Guido
On Fri, May 19, 2017 at 9:55 AM, Guido Günther <agx@sigxcpu.org> wrote:
LGTM but I don't know much about PPC64, it's SLOF and where the device tree should be located.
Hi those paths for SLOF are the default one for Debian/Ubuntu at least. $ dpkg -L qemu-slof /. /usr /usr/share /usr/share/doc /usr/share/doc/qemu-slof /usr/share/doc/qemu-slof/copyright /usr/share/doc/qemu-slof/changelog.Debian.gz /usr/share/slof /usr/share/slof/spapr-rtas.bin /usr/share/slof/slof.bin The other paths are kernel owned and match the current state: (the proc path is a symlink today, not sure if it was a real path back in history) $ ll /proc/device-tree lrwxrwxrwx 1 root root 29 Mai 19 08:59 /proc/device-tree -> /sys/firmware/devicetree/base/ $ ll /sys/firmware/devicetree/ total 0 drwxr-xr-x 3 root root 0 Mai 19 08:57 ./ drwxr-xr-x 4 root root 0 Mai 19 08:57 ../ drwxr-xr-x 51 root root 0 Mai 19 08:57 base/ (there is much more under this path, but the rule is ** for a reason) Hope that helps, Christian
On 19.05.2017 11:03, Christian Ehrhardt wrote:
On Fri, May 19, 2017 at 9:55 AM, Guido Günther <agx@sigxcpu.org <mailto:agx@sigxcpu.org>> wrote:
LGTM but I don't know much about PPC64, it's SLOF and where the device tree should be located.
Hi those paths for SLOF are the default one for Debian/Ubuntu at least. $ dpkg -L qemu-slof /. /usr /usr/share /usr/share/doc /usr/share/doc/qemu-slof /usr/share/doc/qemu-slof/copyright /usr/share/doc/qemu-slof/changelog.Debian.gz /usr/share/slof /usr/share/slof/spapr-rtas.bin /usr/share/slof/slof.bin
The other paths are kernel owned and match the current state: (the proc path is a symlink today, not sure if it was a real path back in history) $ ll /proc/device-tree lrwxrwxrwx 1 root root 29 Mai 19 08:59 /proc/device-tree -> /sys/firmware/devicetree/base/ $ ll /sys/firmware/devicetree/ total 0 drwxr-xr-x 3 root root 0 Mai 19 08:57 ./ drwxr-xr-x 4 root root 0 Mai 19 08:57 ../ drwxr-xr-x 51 root root 0 Mai 19 08:57 base/ (there is much more under this path, but the rule is ** for a reason)
Hope that helps,
Was that explanation sufficient? -Stefan
Christian
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, 2017-05-18 at 10:53 +0200, Stefan Bader wrote:
@@ -102,6 +103,7 @@ /usr/bin/qemu-system-or32 rmix, /usr/bin/qemu-system-ppc rmix, /usr/bin/qemu-system-ppc64 rmix, + /usr/bin/qemu-system-ppc64le rmix,
Why does Ubuntu have qemu-system-ppc64le at all? qemu-system-ppc64 can run both ppc64 and ppc64le guests just fine, it's emulating the same hardware and the endianness is decided at runtime AFAIK. Upstream QEMU doesn't seem to know anything about this: $ ./configure --target-list=ppc64le-softmmu ERROR: Unknown target name 'ppc64le-softmmu' and it's not in the Debian package, so it must have been added specifically for Ubuntu. -- Andrea Bolognani / Red Hat / Virtualization
On 22.05.2017 15:12, Andrea Bolognani wrote:
On Thu, 2017-05-18 at 10:53 +0200, Stefan Bader wrote:
@@ -102,6 +103,7 @@ /usr/bin/qemu-system-or32 rmix, /usr/bin/qemu-system-ppc rmix, /usr/bin/qemu-system-ppc64 rmix, + /usr/bin/qemu-system-ppc64le rmix,
Why does Ubuntu have qemu-system-ppc64le at all?
Hm, apparently it is not really there. It is a soft-link to qemu-system-ppc64. So that hunk really makes no sense because apparmor is following the link before checking permissions. I will refresh that one to drop the ppc64le path. -Stefan
qemu-system-ppc64 can run both ppc64 and ppc64le guests just fine, it's emulating the same hardware and the endianness is decided at runtime AFAIK.
Upstream QEMU doesn't seem to know anything about this:
$ ./configure --target-list=ppc64le-softmmu ERROR: Unknown target name 'ppc64le-softmmu'
and it's not in the Debian package, so it must have been added specifically for Ubuntu.
-- Andrea Bolognani / Red Hat / Virtualization
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Quoting Stefan Bader (stefan.bader@canonical.com):
Over the years there have been a bunch of changes to the apparmor profiles and/or virt-aa-helper which have been carried in Debian/Ubuntu but never made it upstream.
In an attempt to clean this up and generally improve the apparmor based environments, we (Christian and I) went over the changes, cleaned out cruft as much as possible and would be sending out hunks of changes to this list for upstream inclusion.
I hope doing multiple but smaller rounds of submissions will make it simpler to get those reviewed and hopefully accepted.
For the second version I added acks, merged the patches related to explicit device denials and local apparmor profiles, and split the 9p support one (holding back the part allowing link access for later or to be replaced by a safer solution). I also tried to improve the explanation in the description of patch #1 (virt-aa-helper: Ask for no deny rule for readonly disk elements).
Thanks, Stefan
Thanks, Acked-by: Serge Hallyn <serge@hallyn.com> I don't like the added capabilities in the one patch, but I'm not nacking it on that account. Still a toggle would be comforting. Make people who want 9p consciously sign in to the added privs.
participants (5)
-
Andrea Bolognani -
Christian Ehrhardt -
Guido Günther -
Serge E. Hallyn -
Stefan Bader