[libvirt] [PATCH 1/2] virt-aa-helper: Simplify restriction logic

First check overrides, then read only files then restricted access itself. as proposed by Martin Kletzander --- src/security/virt-aa-helper.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 4ce1e7a..963cba6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -544,7 +544,7 @@ 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; const char * const restricted[] = { "/bin/", "/etc/", @@ -594,19 +594,20 @@ 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; + 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 (readonly) { if (array_starts_with(path, restricted_rw, npaths) == 0) - return 1; + return 0; } + npaths = sizeof(restricted)/sizeof(*(restricted)); + if (array_starts_with(path, restricted, npaths) != 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 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 963cba6..8475078 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -568,7 +568,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[] = { -- 2.1.4

Hi, this patchset breaks the test suite for me once applied on top of the debian/experimental branch (while the test suite passes fine without these patches there). Sorry, no time to look into it further today. Cheers, -- intrigeri

Hi, On Fri, Aug 21, 2015 at 11:44:11AM +0200, intrigeri wrote:
Hi,
this patchset breaks the test suite for me once applied on top of the debian/experimental branch (while the test suite passes fine without these patches there). Sorry, no time to look into it further today.
I can reproduce this here, dunno why deemed it passed before. Found one issue already... Cheers, -- Guido
participants (2)
-
Guido Günther
-
intrigeri