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

[libvirt] Problem with the current svirt patch



The current svirt patch relabels all disk to the image_t:MCS, which is incorrect. Read Only Disks and Sharable Disks should not be labeled.

Also when libvirt is completed running the image it needs to relabel the image back to something sane. Right now it is labeling everything imagelabel:s0, including phisical disk partitions. I considered two ways of labeling the "disk" back. We can either grab the label when libvirt starts and change it back to this label when ever an image completes or we can ask the system what the label should be. (matcpathcon). I originally coded up the first, but quickly realized if anything went wrong with libvirt labeling like a crash, the labels on disk could be wrong. And libvirt would continuously set them to this wrong label. With matchpathcon, libvirt will at least set them to something sane.

So this patch Removes labeling of readonly and shared disks and restores the images label to the system default when the image completes.

I would really like to get this in ASAP. Since currently libvirt is relabeing the cdrom to virt_image_t when it is complete as well as physical disks.

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index e73bd3a..7d84344 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -3756,7 +3756,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
                 goto cleanup;
             }
             if (driver->securityDriver)
-                driver->securityDriver->domainSetSecurityImageLabel(dom->conn, vm, dev);
+                driver->securityDriver->domainSetSecurityImageLabel(dom->conn, vm, dev->data.disk);
             break;
 
         default:
@@ -3892,7 +3892,7 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
          dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)) {
         ret = qemudDomainDetachPciDiskDevice(dom->conn, vm, dev);
         if (driver->securityDriver)
-            driver->securityDriver->domainRestoreSecurityImageLabel(dom->conn, vm, dev);
+            driver->securityDriver->domainRestoreSecurityImageLabel(dom->conn, dev->data.disk);
     }
     else
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
diff --git a/src/security.h b/src/security.h
index ac8d690..8cc2c17 100644
--- a/src/security.h
+++ b/src/security.h
@@ -32,11 +32,10 @@ typedef virSecurityDriverStatus (*virSecurityDriverProbe) (void);
 typedef int (*virSecurityDriverOpen) (virConnectPtr conn,
                                       virSecurityDriverPtr drv);
 typedef int (*virSecurityDomainRestoreImageLabel) (virConnectPtr conn,
-                                                   virDomainObjPtr vm,
-                                                   virDomainDeviceDefPtr dev);
+                                                   virDomainDiskDefPtr disk);
 typedef int (*virSecurityDomainSetImageLabel) (virConnectPtr conn,
                                                virDomainObjPtr vm,
-                                               virDomainDeviceDefPtr dev);
+                                               virDomainDiskDefPtr disk);
 typedef int (*virSecurityDomainGenLabel) (virConnectPtr conn,
                                           virDomainObjPtr sec);
 typedef int (*virSecurityDomainGetLabel) (virConnectPtr conn,
diff --git a/src/security_selinux.c b/src/security_selinux.c
index 9e6a442..bbe4bac 100644
--- a/src/security_selinux.c
+++ b/src/security_selinux.c
@@ -269,7 +269,7 @@ SELinuxGetSecurityLabel(virConnectPtr conn,
 }
 
 static int
-SELinuxSetFilecon(virConnectPtr conn, char *path, char *tcon)
+SELinuxSetFilecon(virConnectPtr conn, const char *path, char *tcon)
 {
     char ebuf[1024];
 
@@ -288,28 +288,47 @@ SELinuxSetFilecon(virConnectPtr conn, char *path, char *tcon)
 
 static int
 SELinuxRestoreSecurityImageLabel(virConnectPtr conn,
-                                 virDomainObjPtr vm,
-                                 virDomainDeviceDefPtr dev)
+                                 virDomainDiskDefPtr disk)
 {
-    const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
+    struct stat buf;
+    security_context_t fcon = NULL;
+    int rc = -1;
+    char *newpath = NULL;
+    const char *path = disk->src;
 
-    if (secdef->imagelabel) {
-        return SELinuxSetFilecon(conn, dev->data.disk->src, default_image_context);
+    if (disk->readonly || disk->shared) return 0;
+
+    if (lstat(path, &buf) != 0) return -1;
+
+    if (S_ISLNK(buf.st_mode)) {
+        newpath = calloc(1, buf.st_size + 1);
+        if (!newpath) return -1;
+
+        if (readlink(path, newpath, buf.st_size) < 0) goto err;
+        path = newpath;
+        if (stat(path, &buf) != 0) goto err;
     }
-    return 0;
+
+    if (matchpathcon(path, buf.st_mode, &fcon) == 0)  {
+        rc = SELinuxSetFilecon(conn, path, fcon);
+    }
+err:
+    free(fcon);
+    free(newpath);
+    return rc;
 }
 
 static int
 SELinuxSetSecurityImageLabel(virConnectPtr conn,
                              virDomainObjPtr vm,
-                             virDomainDeviceDefPtr dev)
+                             virDomainDiskDefPtr disk)
 
 {
     const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
 
-    if (secdef->imagelabel) {
-        return SELinuxSetFilecon(conn, dev->data.disk->src, secdef->imagelabel);
-    }
+    if (secdef->imagelabel)
+        return SELinuxSetFilecon(conn, disk->src, secdef->imagelabel);
+
     return 0;
 }
 
@@ -322,7 +341,7 @@ SELinuxRestoreSecurityLabel(virConnectPtr conn,
     int rc = 0;
     if (secdef->imagelabel) {
         for (i = 0 ; i < vm->def->ndisks ; i++) {
-            if (SELinuxSetFilecon(conn, vm->def->disks[i]->src, default_image_context) < 0)
+            if (SELinuxRestoreSecurityImageLabel(conn, vm->def->disks[i]) < 0)
                 rc = -1;
         }
         VIR_FREE(secdef->model);
@@ -368,16 +387,11 @@ SELinuxSetSecurityLabel(virConnectPtr conn,
 
     if (secdef->imagelabel) {
         for (i = 0 ; i < vm->def->ndisks ; i++) {
-            if(setfilecon(vm->def->disks[i]->src, secdef->imagelabel) < 0) {
-                virSecurityReportError(conn, VIR_ERR_ERROR,
-                                       _("%s: unable to set security context "
-                                         "'\%s\' on %s: %s."), __func__,
-                                       secdef->imagelabel,
-                                       vm->def->disks[i]->src,
-                                       virStrerror(errno, ebuf, sizeof ebuf));
-                if (security_getenforce() == 1)
-                    return -1;
-            }
+            if (vm->def->disks[i]->readonly ||
+                vm->def->disks[i]->shared) continue;
+
+            if (SELinuxSetSecurityImageLabel(conn, vm, vm->def->disks[i]) < 0)
+                return -1;
         }
     }
 

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