[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 5/6] Cleanup sec driver error reporting to use virReportSystemError



* src/security_selinux.c: Use virReportSystemError whereever an
  errno is involved
* src/qemu_driver.c: Don't overwrite error message from the
  security driver
---
 src/qemu_driver.c      |   13 +++--
 src/security_selinux.c |  121 ++++++++++++++++++++++++++++--------------------
 2 files changed, 77 insertions(+), 57 deletions(-)

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index d75e28e..fad518c 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1695,6 +1695,8 @@ static int qemuDomainSetHostdevUSBOwnershipActor(virConnectPtr conn,
 {
     struct qemuFileOwner *owner = opaque;
 
+    VIR_DEBUG("Setting ownership on %s to %d:%d", file, owner->uid, owner->gid);
+
     if (chown(file, owner->uid, owner->gid) < 0) {
         virReportSystemError(conn, errno, _("cannot set ownership on %s"), file);
         return -1;
@@ -1736,6 +1738,8 @@ static int qemuDomainSetHostdevPCIOwnershipActor(virConnectPtr conn,
 {
     struct qemuFileOwner *owner = opaque;
 
+    VIR_DEBUG("Setting ownership on %s to %d:%d", file, owner->uid, owner->gid);
+
     if (chown(file, owner->uid, owner->gid) < 0) {
         virReportSystemError(conn, errno, _("cannot set ownership on %s"), file);
         return -1;
@@ -1900,18 +1904,15 @@ static int qemudSecurityHook(void *data) {
     if (qemuAddToCgroup(h->driver, h->vm->def) < 0)
         return -1;
 
-    if (qemudDomainSetSecurityLabel(h->conn, h->driver, h->vm) < 0) {
-        qemudReportError(h->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                         "%s", _("Failed to set security label"));
+    if (qemudDomainSetSecurityLabel(h->conn, h->driver, h->vm) < 0)
         return -1;
-    }
 
     if (h->driver->privileged) {
-        DEBUG("Dropping privileges of VM to %d:%d", h->driver->user, h->driver->group);
-
         if (qemuDomainSetAllDeviceOwnership(h->conn, h->driver, h->vm->def, 0) < 0)
             return -1;
 
+        DEBUG("Dropping privileges of VM to %d:%d", h->driver->user, h->driver->group);
+
         if (h->driver->group) {
             if (setregid(h->driver->group, h->driver->group) < 0) {
                 virReportSystemError(NULL, errno,
diff --git a/src/security_selinux.c b/src/security_selinux.c
index 5b7b038..bc295b1 100644
--- a/src/security_selinux.c
+++ b/src/security_selinux.c
@@ -106,24 +106,21 @@ SELinuxInitialize(virConnectPtr conn)
 {
     char *ptr = NULL;
     int fd = 0;
-    char ebuf[1024];
 
     virRandomInitialize(time(NULL) ^ getpid());
 
     fd = open(selinux_virtual_domain_context_path(), O_RDONLY);
     if (fd < 0) {
-        virSecurityReportError(conn, VIR_ERR_ERROR,
-                               _("%s: cannot open SELinux virtual domain context file %s: %s"),
-                               __func__,selinux_virtual_domain_context_path(),
-                               virStrerror(errno, ebuf, sizeof ebuf));
+        virReportSystemError(conn, errno,
+                             _("cannot open SELinux virtual domain context file '%s'"),
+                             selinux_virtual_domain_context_path());
         return -1;
     }
 
     if (saferead(fd, default_domain_context, sizeof(default_domain_context)) < 0) {
-       virSecurityReportError(conn, VIR_ERR_ERROR,
-                               _("%s: cannot read SELinux virtual domain context file %s: %s"),
-                               __func__,selinux_virtual_domain_context_path(),
-                               virStrerror(errno, ebuf, sizeof ebuf));
+        virReportSystemError(conn, errno,
+                             _("cannot read SELinux virtual domain context file %s"),
+                             selinux_virtual_domain_context_path());
         close(fd);
         return -1;
     }
@@ -133,18 +130,16 @@ SELinuxInitialize(virConnectPtr conn)
     *ptr = '\0';
 
     if ((fd = open(selinux_virtual_image_context_path(), O_RDONLY)) < 0) {
-        virSecurityReportError(conn, VIR_ERR_ERROR,
-                               _("%s: cannot open SELinux virtual image context file %s: %s"),
-                               __func__,selinux_virtual_image_context_path(),
-                               virStrerror(errno, ebuf, sizeof ebuf));
+        virReportSystemError(conn, errno,
+                             _("cannot open SELinux virtual image context file %s"),
+                             selinux_virtual_image_context_path());
         return -1;
     }
 
     if (saferead(fd, default_image_context, sizeof(default_image_context)) < 0) {
-        virSecurityReportError(conn, VIR_ERR_ERROR,
-                               _("%s: cannot read SELinux virtual image context file %s: %s"),
-                               __func__,selinux_virtual_image_context_path(),
-                               virStrerror(errno, ebuf, sizeof ebuf));
+        virReportSystemError(conn, errno,
+                             _("cannot read SELinux virtual image context file %s"),
+                             selinux_virtual_image_context_path());
         close(fd);
         return -1;
     }
@@ -232,10 +227,8 @@ SELinuxReserveSecurityLabel(virConnectPtr conn,
     const char *mcs;
 
     if (getpidcon(vm->pid, &pctx) == -1) {
-        char ebuf[1024];
-        virSecurityReportError(conn, VIR_ERR_ERROR, _("%s: error calling "
-                               "getpidcon(): %s"), __func__,
-                               virStrerror(errno, ebuf, sizeof ebuf));
+        virReportSystemError(conn, errno,
+                             _("unable to get PID %d security context"), vm->pid);
         return -1;
     }
 
@@ -286,17 +279,16 @@ SELinuxGetSecurityLabel(virConnectPtr conn,
     security_context_t ctx;
 
     if (getpidcon(vm->pid, &ctx) == -1) {
-        char ebuf[1024];
-        virSecurityReportError(conn, VIR_ERR_ERROR, _("%s: error calling "
-                               "getpidcon(): %s"), __func__,
-                               virStrerror(errno, ebuf, sizeof ebuf));
+        virReportSystemError(conn, errno,
+                             _("unable to get PID %d security context"),
+                             vm->pid);
         return -1;
     }
 
     if (strlen((char *) ctx) >= VIR_SECURITY_LABEL_BUFLEN) {
         virSecurityReportError(conn, VIR_ERR_ERROR,
-                               _("%s: security label exceeds "
-                               "maximum lenth: %d"), __func__,
+                               _("security label exceeds "
+                                 "maximum lenth: %d"),
                                VIR_SECURITY_LABEL_BUFLEN - 1);
         return -1;
     }
@@ -306,10 +298,8 @@ SELinuxGetSecurityLabel(virConnectPtr conn,
 
     sec->enforcing = security_getenforce();
     if (sec->enforcing == -1) {
-        char ebuf[1024];
-        virSecurityReportError(conn, VIR_ERR_ERROR, _("%s: error calling "
-                               "security_getenforce(): %s"), __func__,
-                               virStrerror(errno, ebuf, sizeof ebuf));
+        virReportSystemError(conn, errno, "%s",
+                             _("error calling security_getenforce()"));
         return -1;
     }
 
@@ -319,7 +309,6 @@ SELinuxGetSecurityLabel(virConnectPtr conn,
 static int
 SELinuxSetFilecon(virConnectPtr conn, const char *path, char *tcon)
 {
-    char ebuf[1024];
     security_context_t econ;
 
     VIR_INFO("Setting SELinux context on '%s' to '%s'", path, tcon);
@@ -343,14 +332,14 @@ SELinuxSetFilecon(virConnectPtr conn, const char *path, char *tcon)
          * virt_use_{nfs,usb,pci}  boolean tunables to allow it...
          */
         if (setfilecon_errno != EOPNOTSUPP) {
-            virSecurityReportError(conn, VIR_ERR_ERROR,
-                                 _("%s: unable to set security context "
-                                   "'\%s\' on %s: %s."), __func__,
-                                 tcon,
-                                 path,
-                                 virStrerror(errno, ebuf, sizeof ebuf));
+            virReportSystemError(conn, setfilecon_errno,
+                                 _("unable to set security context '%s' on '%s'"),
+                                 tcon, path);
             if (security_getenforce() == 1)
                 return -1;
+        } else {
+            VIR_INFO("Setting security context '%s' on '%s' not supported",
+                     tcon, path);
         }
     }
     return 0;
@@ -366,6 +355,8 @@ SELinuxRestoreSecurityFileLabel(virConnectPtr conn,
     int err;
     char *newpath = NULL;
 
+    VIR_INFO("Restoring SELinux context on '%s'", path);
+
     if ((err = virFileResolveLink(path, &newpath)) < 0) {
         virReportSystemError(conn, err,
                              _("cannot resolve symlink %s"), path);
@@ -440,6 +431,17 @@ SELinuxSetSecurityPCILabel(virConnectPtr conn,
 }
 
 static int
+SELinuxSetSecurityUSBLabel(virConnectPtr conn,
+                           usbDevice *dev ATTRIBUTE_UNUSED,
+                           const char *file, void *opaque)
+{
+    virDomainObjPtr vm = opaque;
+    const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
+
+    return SELinuxSetFilecon(conn, file, secdef->imagelabel);
+}
+
+static int
 SELinuxSetSecurityHostdevLabel(virConnectPtr conn,
                                virDomainObjPtr vm,
                                virDomainHostdevDefPtr dev)
@@ -451,8 +453,24 @@ SELinuxSetSecurityHostdevLabel(virConnectPtr conn,
         return 0;
 
     switch (dev->source.subsys.type) {
-    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
-        break;
+    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
+        if (dev->source.subsys.u.usb.bus && dev->source.subsys.u.usb.device) {
+            usbDevice *usb = usbGetDevice(conn,
+                                          dev->source.subsys.u.usb.bus,
+                                          dev->source.subsys.u.usb.device);
+
+            if (!usb)
+                goto done;
+
+            ret = usbDeviceFileIterate(conn, usb, SELinuxSetSecurityUSBLabel, vm);
+            usbFreeDevice(conn, usb);
+
+            break;
+        } else {
+            /* XXX deal with product/vendor better */
+            ret = 0;
+        }
+    }
 
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
         pciDevice *pci = pciGetDevice(conn,
@@ -554,6 +572,9 @@ SELinuxRestoreSecurityLabel(virConnectPtr conn,
     const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
     int i;
     int rc = 0;
+
+    VIR_DEBUG("Restoring security label on %s", vm->def->name);
+
     if (secdef->imagelabel) {
         for (i = 0 ; i < vm->def->nhostdevs ; i++) {
             if (SELinuxRestoreSecurityHostdevLabel(conn, vm->def->hostdevs[i]) < 0)
@@ -597,25 +618,23 @@ SELinuxSetSecurityLabel(virConnectPtr conn,
     /* TODO: verify DOI */
     const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
     int i;
-    char ebuf[1024];
 
     if (!STREQ(drv->name, secdef->model)) {
         virSecurityReportError(conn, VIR_ERR_ERROR,
-                               _("%s: security label driver mismatch: "
-                                 "\'%s\' model configured for domain, but "
-                                 "hypervisor driver is \'%s\'."),
-                                 __func__, secdef->model, drv->name);
+                               _("security label driver mismatch: "
+                                 "'%s' model configured for domain, but "
+                                 "hypervisor driver is '%s'."),
+                               secdef->model, drv->name);
         if (security_getenforce() == 1)
-                return -1;
+            return -1;
     }
 
     if (setexeccon(secdef->label) == -1) {
-        virSecurityReportError(conn, VIR_ERR_ERROR,
-                               _("%s: unable to set security context "
-                               "'\%s\': %s."), __func__, secdef->label,
-                               virStrerror(errno, ebuf, sizeof ebuf));
+        virReportSystemError(conn, errno,
+                             _("unable to set security context '%s'"),
+                             secdef->label);
         if (security_getenforce() == 1)
-                return -1;
+            return -1;
     }
 
     if (secdef->imagelabel) {
-- 
1.6.2.5


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]