Use cgroups for block device whitelisting in QEMU guests

* src/qemu_driver.c: Set a restrictive block device whitelist for
  all QEMU guests. Update whitelist when hotplugging disks.
* src/cgroup.h, src/cgroup.c: Add some more convenience methods
  for dealing with block device whitelists.

diff --git a/src/cgroup.c b/src/cgroup.c
index 1a80a204198403e5be094a193e87a3fab4526f68..35fedadf7b24ce0be3265ddbd088f2ee1dbd9dc0 100644 (file)
--- a/src/cgroup.c
+++ b/src/cgroup.c
@@ -736,10 +736,7 @@ int virCgroupDenyAllDevices(virCgroupPtr group)
  *
  * Returns: 0 on success
  */
-int virCgroupAllowDevice(virCgroupPtr group,
-                         char type,
-                         int major,
-                         int minor)
+int virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor)
 {
     int rc;
     char *devstr = NULL;
@@ -768,9 +765,7 @@ out:
  *
  * Returns: 0 on success
  */
-int virCgroupAllowDeviceMajor(virCgroupPtr group,
-                              char type,
-                              int major)
+int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major)
 {
     int rc;
     char *devstr = NULL;
@@ -790,6 +785,108 @@ int virCgroupAllowDeviceMajor(virCgroupPtr group,
     return rc;
 }
 
+/**
+ * virCgroupAllowDevicePath:
+ *
+ * @group: The cgroup to allow the device for
+ * @path: the device to allow
+ *
+ * Queries the type of device and its major/minor number, and
+ * adds that to the cgroup ACL
+ *
+ * Returns: 0 on success
+ */
+int virCgroupAllowDevicePath(virCgroupPtr group, const char *path)
+{
+    struct stat sb;
+
+    if (stat(path, &sb) < 0)
+        return -errno;
+
+    if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode))
+        return -EINVAL;
+
+    return virCgroupAllowDevice(group,
+                                S_ISCHR(sb.st_mode) ? 'c' : 'b',
+                                major(sb.st_rdev),
+                                minor(sb.st_rdev));
+}
+
+/**
+ * virCgroupDenyDevice:
+ *
+ * @group: The cgroup to deny a device for
+ * @type: The device type (i.e., 'c' or 'b')
+ * @major: The major number of the device
+ * @minor: The minor number of the device
+ *
+ * Returns: 0 on success
+ */
+int virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor)
+{
+    int rc;
+    char *devstr = NULL;
+
+    if (virAsprintf(&devstr, "%c %i:%i rwm", type, major, minor) == -1) {
+        rc = -ENOMEM;
+        goto out;
+    }
+
+    rc = virCgroupSetValueStr(group,
+                              VIR_CGROUP_CONTROLLER_DEVICES,
+                              "devices.deny",
+                              devstr);
+out:
+    VIR_FREE(devstr);
+
+    return rc;
+}
+
+/**
+ * virCgroupDenyDeviceMajor:
+ *
+ * @group: The cgroup to deny an entire device major type for
+ * @type: The device type (i.e., 'c' or 'b')
+ * @major: The major number of the device type
+ *
+ * Returns: 0 on success
+ */
+int virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int major)
+{
+    int rc;
+    char *devstr = NULL;
+
+    if (virAsprintf(&devstr, "%c %i:* rwm", type, major) == -1) {
+        rc = -ENOMEM;
+        goto out;
+    }
+
+    rc = virCgroupSetValueStr(group,
+                              VIR_CGROUP_CONTROLLER_DEVICES,
+                              "devices.deny",
+                              devstr);
+ out:
+    VIR_FREE(devstr);
+
+    return rc;
+}
+
+int virCgroupDenyDevicePath(virCgroupPtr group, const char *path)
+{
+    struct stat sb;
+
+    if (stat(path, &sb) < 0)
+        return -errno;
+
+    if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode))
+        return -EINVAL;
+
+    return virCgroupDenyDevice(group,
+                               S_ISCHR(sb.st_mode) ? 'c' : 'b',
+                               major(sb.st_rdev),
+                               minor(sb.st_rdev));
+}
+
 int virCgroupSetCpuShares(virCgroupPtr group, unsigned long long shares)
 {
     return virCgroupSetValueU64(group,

diff --git a/src/cgroup.h b/src/cgroup.h
index f452e2d84477dd398746af5c810009d721522092..efc337074e949b7d02b0513804a08ea325418e83 100644 (file)
--- a/src/cgroup.h
+++ b/src/cgroup.h
@@ -38,6 +38,18 @@ int virCgroupAllowDevice(virCgroupPtr group,
 int virCgroupAllowDeviceMajor(virCgroupPtr group,
                               char type,
                               int major);
+int virCgroupAllowDevicePath(virCgroupPtr group,
+                             const char *path);
+
+int virCgroupDenyDevice(virCgroupPtr group,
+                        char type,
+                        int major,
+                        int minor);
+int virCgroupDenyDeviceMajor(virCgroupPtr group,
+                             char type,
+                             int major);
+int virCgroupDenyDevicePath(virCgroupPtr group,
+                            const char *path);
 
 int virCgroupSetCpuShares(virCgroupPtr group, unsigned long long shares);
 int virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares);

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 6e3b75bc436c8f96800793a4a3879b296b2405c1..9a181c515486230219ce185b6a5b5d8b8a8adf84 100644 (file)
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1388,12 +1388,24 @@ error:
     return -1;
 }
 
+static const char *const defaultDeviceACL[] = {
+    "/dev/null", "/dev/full", "/dev/zero",
+    "/dev/random", "/dev/urandom",
+    "/dev/ptmx", "/dev/kvm", "/dev/kqemu",
+    "/dev/rtc", "/dev/hpet", "/dev/net/tun",
+    NULL,
+};
+#define DEVICE_PTY_MAJOR 136
+#define DEVICE_SND_MAJOR 116
+
 static int qemuSetupCgroup(virConnectPtr conn,
                            struct qemud_driver *driver,
                            virDomainObjPtr vm)
 {
     virCgroupPtr cgroup = NULL;
     int rc;
+    unsigned int i;
+    const char *const *deviceACL = defaultDeviceACL;
 
     if (driver->cgroup == NULL)
         return 0; /* Not supported, so claim success */
@@ -1406,6 +1418,62 @@ static int qemuSetupCgroup(virConnectPtr conn,
         goto cleanup;
     }
 
+    rc = virCgroupDenyAllDevices(cgroup);
+    if (rc != 0) {
+        if (rc == -EPERM) {
+            VIR_WARN0("Group devices ACL is not accessible, disabling whitelisting");
+            goto done;
+        }
+
+        virReportSystemError(conn, -rc,
+                             _("Unable to deny all devices for %s"), vm->def->name);
+        goto cleanup;
+    }
+
+    for (i = 0; i < vm->def->ndisks ; i++) {
+        if (vm->def->disks[i]->type != VIR_DOMAIN_DISK_TYPE_BLOCK ||
+            vm->def->disks[i]->src == NULL)
+            continue;
+
+        rc = virCgroupAllowDevicePath(cgroup,
+                                      vm->def->disks[i]->src);
+        if (rc != 0) {
+            virReportSystemError(conn, -rc,
+                                 _("Unable to allow device %s for %s"),
+                                 vm->def->disks[i]->src, vm->def->name);
+            goto cleanup;
+        }
+    }
+
+    rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR);
+    if (rc != 0) {
+        virReportSystemError(conn, -rc, "%s",
+                             _("unable to allow /dev/pts/ devices"));
+        goto cleanup;
+    }
+
+    if (vm->def->nsounds) {
+        rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR);
+        if (rc != 0) {
+            virReportSystemError(conn, -rc, "%s",
+                                 _("unable to allow /dev/snd/ devices"));
+            goto cleanup;
+        }
+    }
+
+    for (i = 0; deviceACL[i] != NULL ; i++) {
+        rc = virCgroupAllowDevicePath(cgroup,
+                                      deviceACL[i]);
+        if (rc < 0 &&
+            rc != -ENOENT) {
+            virReportSystemError(conn, -rc,
+                                 _("unable to allow device %s"),
+                                 deviceACL[i]);
+            goto cleanup;
+        }
+    }
+
+done:
     virCgroupFree(&cgroup);
     return 0;
 
@@ -4836,6 +4904,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
     virDomainObjPtr vm;
     virDomainDeviceDefPtr dev = NULL;
     unsigned int qemuCmdFlags;
+    virCgroupPtr cgroup = NULL;
     int ret = -1;
 
     qemuDriverLock(driver);
@@ -4865,6 +4934,27 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
         goto cleanup;
 
     if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
+        if (driver->cgroup != NULL) {
+            if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) !=0 ) {
+                qemudReportError(dom->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                                 _("Unable to find cgroup for %s\n"),
+                                 vm->def->name);
+                goto cleanup;
+            }
+            if (dev->data.disk->src != NULL &&
+                dev->data.disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK &&
+                virCgroupAllowDevicePath(cgroup,
+                                         dev->data.disk->src) < 0) {
+                qemudReportError(dom->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                                 _("unable to allow device %s"),
+                                 dev->data.disk->src);
+                goto cleanup;
+            }
+        }
+
+        if (driver->securityDriver)
+            driver->securityDriver->domainSetSecurityImageLabel(dom->conn, vm, dev->data.disk);
+
         switch (dev->data.disk->device) {
         case VIR_DOMAIN_DISK_DEVICE_CDROM:
         case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
@@ -4893,7 +4983,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
                 qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
                                  _("disk bus '%s' cannot be hotplugged."),
                                  virDomainDiskBusTypeToString(dev->data.disk->bus));
-                goto cleanup;
+                /* fallthrough */
             }
             break;
 
@@ -4901,7 +4991,11 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
             qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
                              _("disk device type '%s' cannot be hotplugged"),
                              virDomainDiskDeviceTypeToString(dev->data.disk->device));
-            goto cleanup;
+            /* Fallthrough */
+        }
+        if (ret != 0) {
+            virCgroupDenyDevicePath(cgroup,
+                                    dev->data.disk->src);
         }
     } else if (dev->type == VIR_DOMAIN_DEVICE_NET) {
         ret = qemudDomainAttachNetDevice(dom->conn, vm, dev, qemuCmdFlags);
@@ -4923,6 +5017,9 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
         ret = -1;
 
 cleanup:
+    if (cgroup)
+        virCgroupFree(&cgroup);
+
     if (ret < 0) {
         if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 1) < 0)
             VIR_WARN0("Fail to restore disk device ownership");