[libvirt] [PATCH] 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> --- src/security/virt-aa-helper.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 35423b5..a097aa6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -592,19 +592,9 @@ 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) + } 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)); npaths = sizeof(restricted)/sizeof(*(restricted)); @@ -1101,6 +1091,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

Quoting Michal Dubiel (md@semihalf.com):
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> --- src/security/virt-aa-helper.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 35423b5..a097aa6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -592,19 +592,9 @@ 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) + } else if (stat(path, &sb) == -1) return -1;
Hi, Why keep this bit? sb is not used later in the fn, and you already know that access(2) didn't return ENOENT.
- switch (sb.st_mode & S_IFMT) { - case S_IFSOCK: - return 1; - break; - default: - break; - } - } - opaths = sizeof(override)/sizeof(*(override));
npaths = sizeof(restricted)/sizeof(*(restricted)); @@ -1101,6 +1091,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
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 06/19/2015 03:30 PM, Serge Hallyn wrote:
Quoting Michal Dubiel (md@semihalf.com):
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> --- src/security/virt-aa-helper.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 35423b5..a097aa6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -592,19 +592,9 @@ 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) + } else if (stat(path, &sb) == -1) return -1;
Hi,
Why keep this bit? sb is not used later in the fn, and you already know that access(2) didn't return ENOENT.
Besides by changing from : } else { if ((x) == -1)) return -1; to } else if ((x) == -1) { return -1; Everything else inside the else if after the return -1 becomes dead John
- switch (sb.st_mode & S_IFMT) { - case S_IFSOCK: - return 1; - break; - default: - break; - } - } - opaths = sizeof(override)/sizeof(*(override));
npaths = sizeof(restricted)/sizeof(*(restricted)); @@ -1101,6 +1091,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
-- 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

On 19 June 2015 at 21:30, Serge Hallyn <serge.hallyn@ubuntu.com> wrote:
Quoting Michal Dubiel (md@semihalf.com):
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> --- src/security/virt-aa-helper.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 35423b5..a097aa6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -592,19 +592,9 @@ 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) + } else if (stat(path, &sb) == -1) return -1;
Hi,
Why keep this bit? sb is not used later in the fn, and you already know that access(2) didn't return ENOENT.
You are right, it is not needed. Thanks for pointing this out. I will update the patch accordingly. Regards, Michal
participants (4)
-
John Ferlan
-
Michal Dubiel
-
Michał Dubiel
-
Serge Hallyn