[libvirt] [PATCH v2 0/3] virt-aa-helper: allow to add R/O files in restricted dirs
Hi, the purpose of these patches is to make it possible to put files in restricted_rw that are under a directory that is already in restricted. It was only possible to add the to overrides before so they ended up being rw. Sorry for the incomplete series this morning, hopefully this looks better. The tests pass here and I've added an additional patch checking the new file. Cheers, -- Guido Guido Günther (2): virt-aa-helper: document --probing and --dry-run virt-aa-helper: Simplify restriction logic intrigeri (1): virt-aa-helper: allow access to /usr/share/ovmf/ src/security/virt-aa-helper.c | 34 ++++++++++++++++++++++------------ tests/virt-aa-helper-test | 9 +++++++++ 2 files changed, 31 insertions(+), 12 deletions(-) -- 2.1.4
--- src/security/virt-aa-helper.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 4ce1e7a..178569e 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -107,12 +107,14 @@ vah_usage(void) " Options:\n" " -a | --add load profile\n" " -c | --create create profile from template\n" + " -d | --dry-run dry run\n" " -D | --delete unload and delete profile\n" " -f | --add-file <file> add file to profile\n" " -F | --append-file <file> append file to profile\n" " -r | --replace reload profile\n" " -R | --remove unload profile\n" " -h | --help this help\n" + " -p | --probing [0|1] allow disk format probing\n" " -u | --uuid <uuid> uuid (profile name)\n" "\n"), progname); -- 2.1.4
First check overrides, then read only files then restricted access itself. This allows us to mark files for read only access whose parents were already restricted for read write. Based on a proposal by Martin Kletzander --- src/security/virt-aa-helper.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 178569e..8e01bf6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -546,7 +546,9 @@ array_starts_with(const char *str, const char * const *arr, const long size) static int valid_path(const char *path, const bool readonly) { - int npaths, opaths; + int npaths; + int nropaths; + const char * const restricted[] = { "/bin/", "/etc/", @@ -596,19 +598,24 @@ valid_path(const char *path, const bool readonly) if (!virFileExists(path)) vah_warning(_("path does not exist, skipping file type checks")); - opaths = sizeof(override)/sizeof(*(override)); - - npaths = sizeof(restricted)/sizeof(*(restricted)); - if (array_starts_with(path, restricted, npaths) == 0 && - array_starts_with(path, override, opaths) != 0) - return 1; + /* overrides are always allowed */ + npaths = sizeof(override)/sizeof(*(override)); + if (array_starts_with(path, override, npaths) == 0) + return 0; - npaths = sizeof(restricted_rw)/sizeof(*(restricted_rw)); - if (!readonly) { - if (array_starts_with(path, restricted_rw, npaths) == 0) - return 1; + /* allow read only paths upfront */ + if (readonly) { + nropaths = sizeof(restricted_rw)/sizeof(*(restricted_rw)); + if (array_starts_with(path, restricted_rw, nropaths) == 0) + return 0; } + /* disallow RW acess to all paths in restricted and restriced_rw */ + npaths = sizeof(restricted)/sizeof(*(restricted)); + if ((array_starts_with(path, restricted, npaths) == 0 + || array_starts_with(path, restricted_rw, nropaths) == 0)) + return 1; + return 0; } -- 2.1.4
From: intrigeri <intrigeri@debian.org> We forbid access to /usr/share/, but (at least on Debian-based systems) the Open Virtual Machine Firmware files needed for booting UEFI virtual machines in QEMU live in /usr/share/ovmf/. Therefore, we need to add that directory to the list of read only paths. A similar patch was suggested by Jamie Strandboge <jamie@canonical.com> on https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1483071. --- src/security/virt-aa-helper.c | 3 ++- tests/virt-aa-helper-test | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 8e01bf6..f163fe7 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -572,7 +572,8 @@ valid_path(const char *path, const bool readonly) "/boot/", "/vmlinuz", "/initrd", - "/initrd.img" + "/initrd.img", + "/usr/share/ovmf/" /* for OVMF 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 caf2f97..1d03f5f 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -291,6 +291,15 @@ 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 + 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" testme "0" "initrd" "-r -u $valid_uuid" "$test_xml" -- 2.1.4
On Fri, Aug 21, 2015 at 03:31:59PM +0200, Guido Günther wrote:
Hi, the purpose of these patches is to make it possible to put files in restricted_rw that are under a directory that is already in restricted. It was only possible to add the to overrides before so they ended up being rw.
Sorry for the incomplete series this morning, hopefully this looks better. The tests pass here and I've added an additional patch checking the new file.
Cheers, -- Guido
I have no way to check for the virt-aa-helper test, so limited ACK from me.
Guido Günther (2): virt-aa-helper: document --probing and --dry-run virt-aa-helper: Simplify restriction logic
intrigeri (1): virt-aa-helper: allow access to /usr/share/ovmf/
src/security/virt-aa-helper.c | 34 ++++++++++++++++++++++------------ tests/virt-aa-helper-test | 9 +++++++++ 2 files changed, 31 insertions(+), 12 deletions(-)
-- 2.1.4
Hi, Martin Kletzander wrote (21 Aug 2015 21:44:49 GMT) :
I have no way to check for the virt-aa-helper test, so limited ACK from me.
I've applied this patch series on top of Debian unstable's 1.2.18-2, and I confirm that the test suite passes fine, and that I can now start a VM with OVMF. Thanks! Cheers, -- intrigeri
On Mon, Aug 24, 2015 at 11:01:34AM +0200, intrigeri wrote:
Hi,
Martin Kletzander wrote (21 Aug 2015 21:44:49 GMT) :
I have no way to check for the virt-aa-helper test, so limited ACK from me.
I've applied this patch series on top of Debian unstable's 1.2.18-2, and I confirm that the test suite passes fine, and that I can now start a VM with OVMF. Thanks!
I think this can be pushed then, thanks for checking that.
Cheers, -- intrigeri
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hi, On Mon, Aug 24, 2015 at 11:47:48AM +0200, Martin Kletzander wrote:
On Mon, Aug 24, 2015 at 11:01:34AM +0200, intrigeri wrote:
Hi,
Martin Kletzander wrote (21 Aug 2015 21:44:49 GMT) :
I have no way to check for the virt-aa-helper test, so limited ACK from me.
I've applied this patch series on top of Debian unstable's 1.2.18-2, and I confirm that the test suite passes fine, and that I can now start a VM with OVMF. Thanks!
I think this can be pushed then, thanks for checking that.
Pushed. Thanks! -- Guido
participants (3)
-
Guido Günther -
intrigeri -
Martin Kletzander