[libvirt] [PATCHv2] virt-aa-helper: Fix permissions for vhost-user socket files
QEMU working in vhost-user mode communicates with the other end (i.e. some virtual router application) via unix domain sockets. This requires that permissions for the socket files are correctly written into /etc/apparmor.d/libvirt/libvirt-UUID.files. Signed-off-by: Michal Dubiel <md@semihalf.com> --- Changes since v1: - Removed unnecessary stat() call and dead 'else' block src/security/virt-aa-helper.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 35423b5..f39932e 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -32,7 +32,6 @@ #include <unistd.h> #include <errno.h> #include <sys/types.h> -#include <sys/stat.h> #include <fcntl.h> #include <getopt.h> #include <sys/utsname.h> @@ -542,7 +541,6 @@ array_starts_with(const char *str, const char * const *arr, const long size) static int valid_path(const char *path, const bool readonly) { - struct stat sb; int npaths, opaths; const char * const restricted[] = { "/bin/", @@ -592,17 +590,6 @@ valid_path(const char *path, const bool readonly) if (!virFileExists(path)) { vah_warning(_("path does not exist, skipping file type checks")); - } else { - if (stat(path, &sb) == -1) - return -1; - - switch (sb.st_mode & S_IFMT) { - case S_IFSOCK: - return 1; - break; - default: - break; - } } opaths = sizeof(override)/sizeof(*(override)); @@ -1101,6 +1088,18 @@ get_files(vahControl * ctl) } } + for (i = 0; i < ctl->def->nnets; i++) { + if (ctl->def->nets[i] && + ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && + ctl->def->nets[i]->data.vhostuser) { + virDomainChrSourceDefPtr vhu = ctl->def->nets[i]->data.vhostuser; + + if (vah_add_file_chardev(&buf, vhu->data.nix.path, "rw", + vhu->type) != 0) + goto cleanup; + } + } + if (ctl->def->virtType == VIR_DOMAIN_VIRT_KVM) { for (i = 0; i < ctl->def->nnets; i++) { virDomainNetDefPtr net = ctl->def->nets[i]; -- 1.9.1
On 22.06.2015 12:47, Michal Dubiel wrote:
QEMU working in vhost-user mode communicates with the other end (i.e. some virtual router application) via unix domain sockets. This requires that permissions for the socket files are correctly written into /etc/apparmor.d/libvirt/libvirt-UUID.files.
Signed-off-by: Michal Dubiel <md@semihalf.com> --- Changes since v1: - Removed unnecessary stat() call and dead 'else' block
src/security/virt-aa-helper.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 35423b5..f39932e 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -32,7 +32,6 @@ #include <unistd.h> #include <errno.h> #include <sys/types.h> -#include <sys/stat.h> #include <fcntl.h> #include <getopt.h> #include <sys/utsname.h> @@ -542,7 +541,6 @@ array_starts_with(const char *str, const char * const *arr, const long size) static int valid_path(const char *path, const bool readonly) { - struct stat sb; int npaths, opaths; const char * const restricted[] = { "/bin/", @@ -592,17 +590,6 @@ valid_path(const char *path, const bool readonly)
if (!virFileExists(path)) { vah_warning(_("path does not exist, skipping file type checks")); - } else { - if (stat(path, &sb) == -1) - return -1; - - switch (sb.st_mode & S_IFMT) { - case S_IFSOCK: - return 1; - break; - default: - break; - } }
This leaves a one line body to the if(). Therefore 'syntax-check' is sad. With that fixed I'm inclined to ACK the patch. But I'm not too familiar with AppArmor, so unless somebody else gives another ACK, I'll push this after the release.
opaths = sizeof(override)/sizeof(*(override)); @@ -1101,6 +1088,18 @@ get_files(vahControl * ctl) } }
+ for (i = 0; i < ctl->def->nnets; i++) { + if (ctl->def->nets[i] && + ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && + ctl->def->nets[i]->data.vhostuser) { + virDomainChrSourceDefPtr vhu = ctl->def->nets[i]->data.vhostuser; + + if (vah_add_file_chardev(&buf, vhu->data.nix.path, "rw", + vhu->type) != 0) + goto cleanup; + } + } + if (ctl->def->virtType == VIR_DOMAIN_VIRT_KVM) { for (i = 0; i < ctl->def->nnets; i++) { virDomainNetDefPtr net = ctl->def->nets[i];
Michal
Hi Michal, I've sent v3 of the patch with the curly braces removed from one line 'if' blok. Sorry for that. Regards, Michal On 30 June 2015 at 16:41, Michal Privoznik <mprivozn@redhat.com> wrote:
On 22.06.2015 12:47, Michal Dubiel wrote:
QEMU working in vhost-user mode communicates with the other end (i.e. some virtual router application) via unix domain sockets. This requires that permissions for the socket files are correctly written into /etc/apparmor.d/libvirt/libvirt-UUID.files.
Signed-off-by: Michal Dubiel <md@semihalf.com> --- Changes since v1: - Removed unnecessary stat() call and dead 'else' block
src/security/virt-aa-helper.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 35423b5..f39932e 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -32,7 +32,6 @@ #include <unistd.h> #include <errno.h> #include <sys/types.h> -#include <sys/stat.h> #include <fcntl.h> #include <getopt.h> #include <sys/utsname.h> @@ -542,7 +541,6 @@ array_starts_with(const char *str, const char * const *arr, const long size) static int valid_path(const char *path, const bool readonly) { - struct stat sb; int npaths, opaths; const char * const restricted[] = { "/bin/", @@ -592,17 +590,6 @@ valid_path(const char *path, const bool readonly)
if (!virFileExists(path)) { vah_warning(_("path does not exist, skipping file type checks")); - } else { - if (stat(path, &sb) == -1) - return -1; - - switch (sb.st_mode & S_IFMT) { - case S_IFSOCK: - return 1; - break; - default: - break; - } }
This leaves a one line body to the if(). Therefore 'syntax-check' is sad. With that fixed I'm inclined to ACK the patch. But I'm not too familiar with AppArmor, so unless somebody else gives another ACK, I'll push this after the release.
opaths = sizeof(override)/sizeof(*(override)); @@ -1101,6 +1088,18 @@ get_files(vahControl * ctl) } }
+ for (i = 0; i < ctl->def->nnets; i++) { + if (ctl->def->nets[i] && + ctl->def->nets[i]->type ==
VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
+ ctl->def->nets[i]->data.vhostuser) { + virDomainChrSourceDefPtr vhu = ctl->def->nets[i]->data.vhostuser; + + if (vah_add_file_chardev(&buf, vhu->data.nix.path, "rw", + vhu->type) != 0) + goto cleanup; + } + } + if (ctl->def->virtType == VIR_DOMAIN_VIRT_KVM) { for (i = 0; i < ctl->def->nnets; i++) { virDomainNetDefPtr net = ctl->def->nets[i];
Michal
participants (3)
-
Michal Dubiel -
Michal Privoznik -
Michał Dubiel