Disable all disk probing in QEMU driver & add config option to re-enable

Disk format probing is now disabled by default. A new config
option in /etc/qemu/qemu.conf will re-enable it for existing
deployments where this causes trouble

src/qemu/libvirtd_qemu.aug

@@ -40,6 +40,7 @@ module Libvirtd_qemu =
                  | bool_entry "relaxed_acs_check"
                  | bool_entry "vnc_allow_host_audio"
                  | bool_entry "clear_emulator_capabilities"
+                 | bool_entry "allow_disk_format_probing"
 
    (* Each enty in the config is one of the following three ... *)
    let entry = vnc_entry

src/qemu/qemu.conf

@@ -187,3 +187,15 @@
 # exploit the privileges and possibly do damage to the host.
 #
 # clear_emulator_capabilities = 1
+
+
+
+# If allow_disk_format_probing is enabled, libvirt will probe disk
+# images to attempt to identify their format, when not otherwise
+# specified in the XML. This is disabled by default.
+#
+# WARNING: Enabling probing is a security hole in almost all
+# deployments. It is strongly recommended that users update their
+# guest XML <disk> elements to include  <driver type='XXXX'/>
+# elements instead of enabling this option.
+# allow_disk_format_probing = 1

src/qemu/qemu_conf.c

@@ -365,6 +365,10 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
     CHECK_TYPE ("clear_emulator_capabilities", VIR_CONF_LONG);
     if (p) driver->clearEmulatorCapabilities = p->l;
 
+    p = virConfGetValue (conf, "allow_disk_format_probing");
+    CHECK_TYPE ("allow_disk_format_probing", VIR_CONF_LONG);
+    if (p) driver->allowDiskFormatProbing = p->l;
+
     virConfFree (conf);
     return 0;
 }

src/qemu/qemu_conf.h

@@ -141,6 +141,7 @@ struct qemud_driver {
     unsigned int relaxedACS : 1;
     unsigned int vncAllowHostAudio : 1;
     unsigned int clearEmulatorCapabilities : 1;
+    unsigned int allowDiskFormatProbing : 1;
 
     virCapsPtr caps;
 

src/qemu/qemu_driver.c

@@ -1326,7 +1326,8 @@ qemudSecurityInit(struct qemud_driver *qemud_drv)
     qemuSecurityDACSetDriver(qemud_drv);
 
     ret = virSecurityDriverStartup(&security_drv,
-                                   qemud_drv->securityDriverName);
+                                   qemud_drv->securityDriverName,
+                                   qemud_drv->allowDiskFormatProbing);
     if (ret == -1) {
         VIR_ERROR0(_("Failed to start security driver"));
         return -1;
@@ -3074,11 +3075,12 @@ static int qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
 }
 
 
-static int qemuSetupDiskCgroup(virCgroupPtr cgroup,
+static int qemuSetupDiskCgroup(struct qemud_driver *driver,
+                               virCgroupPtr cgroup,
                                virDomainDiskDefPtr disk)
 {
     return virDomainDiskDefForeachPath(disk,
-                                       true,
+                                       driver->allowDiskFormatProbing,
                                        true,
                                        qemuSetupDiskPathAllow,
                                        cgroup);
@@ -3113,11 +3115,12 @@ static int qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
 }
 
 
-static int qemuTeardownDiskCgroup(virCgroupPtr cgroup,
+static int qemuTeardownDiskCgroup(struct qemud_driver *driver,
+                                  virCgroupPtr cgroup,
                                   virDomainDiskDefPtr disk)
 {
     return virDomainDiskDefForeachPath(disk,
-                                       true,
+                                       driver->allowDiskFormatProbing,
                                        true,
                                        qemuTeardownDiskPathDeny,
                                        cgroup);
@@ -3184,7 +3187,7 @@ static int qemuSetupCgroup(struct qemud_driver *driver,
         }
 
         for (i = 0; i < vm->def->ndisks ; i++) {
-            if (qemuSetupDiskCgroup(cgroup, vm->def->disks[i]) < 0)
+            if (qemuSetupDiskCgroup(driver, cgroup, vm->def->disks[i]) < 0)
                 goto cleanup;
         }
 
@@ -8037,7 +8040,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
                                 vm->def->name);
                 goto endjob;
             }
-            if (qemuSetupDiskCgroup(cgroup, dev->data.disk) < 0)
+            if (qemuSetupDiskCgroup(driver, cgroup, dev->data.disk) < 0)
                 goto endjob;
         }
 
@@ -8082,7 +8085,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
             /* Fallthrough */
         }
         if (ret != 0 && cgroup) {
-            if (qemuTeardownDiskCgroup(cgroup, dev->data.disk) < 0)
+            if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0)
                 VIR_WARN("Failed to teardown cgroup for disk path %s",
                          NULLSTR(dev->data.disk->src));
         }
@@ -8282,7 +8285,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
                                 vm->def->name);
                 goto endjob;
             }
-            if (qemuSetupDiskCgroup(cgroup, dev->data.disk) < 0)
+            if (qemuSetupDiskCgroup(driver, cgroup, dev->data.disk) < 0)
                 goto endjob;
         }
 
@@ -8305,7 +8308,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
         }
 
         if (ret != 0 && cgroup) {
-            if (qemuTeardownDiskCgroup(cgroup, dev->data.disk) < 0)
+            if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0)
                 VIR_WARN("Failed to teardown cgroup for disk path %s",
                          NULLSTR(dev->data.disk->src));
         }
@@ -8433,7 +8436,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver,
         VIR_WARN("Unable to restore security label on %s", dev->data.disk->src);
 
     if (cgroup != NULL) {
-        if (qemuTeardownDiskCgroup(cgroup, dev->data.disk) < 0)
+        if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0)
             VIR_WARN("Failed to teardown cgroup for disk path %s",
                      NULLSTR(dev->data.disk->src));
     }
@@ -8497,7 +8500,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver,
         VIR_WARN("Unable to restore security label on %s", dev->data.disk->src);
 
     if (cgroup != NULL) {
-        if (qemuTeardownDiskCgroup(cgroup, dev->data.disk) < 0)
+        if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0)
             VIR_WARN("Failed to teardown cgroup for disk path %s",
                      NULLSTR(dev->data.disk->src));
     }
@@ -9676,8 +9679,15 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
             goto cleanup;
         }
     } else {
+        if (driver->allowDiskFormatProbing) {
             if ((format = virStorageFileProbeFormat(disk->src)) < 0)
                 goto cleanup;
+        } else {
+            qemuReportError(VIR_ERR_INTERNAL_ERROR,
+                            _("no disk format for %s and probing is disabled"),
+                            disk->src);
+            goto cleanup;
+        }
     }
 
     if (virStorageFileGetMetadataFromFD(path, fd,

src/qemu/qemu_security_dac.c

@@ -117,7 +117,7 @@ qemuSecurityDACSetSecurityImageLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED,
         return 0;
 
     return virDomainDiskDefForeachPath(disk,
-                                       true,
+                                       driver->allowDiskFormatProbing,
                                        false,
                                        qemuSecurityDACSetSecurityFileLabel,
                                        NULL);

src/qemu/test_libvirtd_qemu.aug

@@ -101,6 +101,8 @@ relaxed_acs_check = 1
 vnc_allow_host_audio = 1
 
 clear_emulator_capabilities = 0
+
+allow_disk_format_probing = 1
 "
 
    test Libvirtd_qemu.lns get conf =
@@ -212,3 +214,5 @@ clear_emulator_capabilities = 0
 { "vnc_allow_host_audio" = "1" }
 { "#empty" }
 { "clear_emulator_capabilities" = "0" }
+{ "#empty" }
+{ "allow_disk_format_probing" = "1" }

src/security/security_apparmor.c

@@ -157,6 +157,8 @@ load_profile(virSecurityDriverPtr drv,
     char *xml = NULL;
     int pipefd[2];
     pid_t child;
+    const char *probe = virSecurityDriverGetAllowDiskFormatProbing(drv)
+        ? "1" : "0";
 
     if (pipe(pipefd) < -1) {
         virReportSystemError(errno, "%s", _("unable to create pipe"));
@@ -172,19 +174,19 @@ load_profile(virSecurityDriverPtr drv,
 
     if (create) {
         const char *const argv[] = {
-            VIRT_AA_HELPER, "-c", "-u", profile, NULL
+            VIRT_AA_HELPER, "-p", probe, "-c", "-u", profile, NULL
         };
         ret = virExec(argv, NULL, NULL, &child,
                       pipefd[0], NULL, NULL, VIR_EXEC_NONE);
     } else if (fn) {
         const char *const argv[] = {
-            VIRT_AA_HELPER, "-r", "-u", profile, "-f", fn, NULL
+            VIRT_AA_HELPER, "-p", probe, "-r", "-u", profile, "-f", fn, NULL
         };
         ret = virExec(argv, NULL, NULL, &child,
                       pipefd[0], NULL, NULL, VIR_EXEC_NONE);
     } else {
         const char *const argv[] = {
-            VIRT_AA_HELPER, "-r", "-u", profile, NULL
+            VIRT_AA_HELPER, "-p", probe, "-r", "-u", profile, NULL
         };
         ret = virExec(argv, NULL, NULL, &child,
                       pipefd[0], NULL, NULL, VIR_EXEC_NONE);
@@ -347,9 +349,11 @@ AppArmorSecurityDriverProbe(void)
  * currently not used.
  */
 static int
-AppArmorSecurityDriverOpen(virSecurityDriverPtr drv)
+AppArmorSecurityDriverOpen(virSecurityDriverPtr drv,
+                           bool allowDiskFormatProbing)
 {
     virSecurityDriverSetDOI(drv, SECURITY_APPARMOR_VOID_DOI);
+    virSecurityDriverSetAllowDiskFormatProbing(drv, allowDiskFormatProbing);
     return 0;
 }
 

src/security/security_driver.c

@@ -56,7 +56,8 @@ virSecurityDriverVerify(virDomainDefPtr def)
 
 int
 virSecurityDriverStartup(virSecurityDriverPtr *drv,
-                         const char *name)
+                         const char *name,
+                         bool allowDiskFormatProbing)
 {
     unsigned int i;
 
@@ -72,7 +73,7 @@ virSecurityDriverStartup(virSecurityDriverPtr *drv,
         switch (tmp->probe()) {
         case SECURITY_DRIVER_ENABLE:
             virSecurityDriverInit(tmp);
-            if (tmp->open(tmp) == -1) {
+            if (tmp->open(tmp, allowDiskFormatProbing) == -1) {
                 return -1;
             } else {
                 *drv = tmp;
@@ -125,3 +126,14 @@ virSecurityDriverGetModel(virSecurityDriverPtr drv)
 {
     return drv->name;
 }
+
+void virSecurityDriverSetAllowDiskFormatProbing(virSecurityDriverPtr drv,
+                                                bool allowDiskFormatProbing)
+{
+    drv->_private.allowDiskFormatProbing = allowDiskFormatProbing;
+}
+
+bool virSecurityDriverGetAllowDiskFormatProbing(virSecurityDriverPtr drv)
+{
+    return drv->_private.allowDiskFormatProbing;
+}

src/security/security_driver.h

@@ -33,7 +33,8 @@ typedef struct _virSecurityDriverState virSecurityDriverState;
 typedef virSecurityDriverState *virSecurityDriverStatePtr;
 
 typedef virSecurityDriverStatus (*virSecurityDriverProbe) (void);
-typedef int (*virSecurityDriverOpen) (virSecurityDriverPtr drv);
+typedef int (*virSecurityDriverOpen) (virSecurityDriverPtr drv,
+                                      bool allowDiskFormatProbing);
 typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityDriverPtr drv,
                                                    virDomainObjPtr vm,
                                                    virDomainDiskDefPtr disk);
@@ -102,12 +103,14 @@ struct _virSecurityDriver {
      */
     struct {
         char doi[VIR_SECURITY_DOI_BUFLEN];
+        bool allowDiskFormatProbing;
     } _private;
 };
 
 /* Global methods */
 int virSecurityDriverStartup(virSecurityDriverPtr *drv,
-                             const char *name);
+                             const char *name,
+                             bool allowDiskFormatProbing);
 
 int
 virSecurityDriverVerify(virDomainDefPtr def);
@@ -120,7 +123,10 @@ virSecurityDriverVerify(virDomainDefPtr def);
 void virSecurityDriverInit(virSecurityDriverPtr drv);
 int virSecurityDriverSetDOI(virSecurityDriverPtr drv,
                             const char *doi);
+void virSecurityDriverSetAllowDiskFormatProbing(virSecurityDriverPtr drv,
+                                                bool allowDiskFormatProbing);
 const char *virSecurityDriverGetDOI(virSecurityDriverPtr drv);
 const char *virSecurityDriverGetModel(virSecurityDriverPtr drv);
+bool virSecurityDriverGetAllowDiskFormatProbing(virSecurityDriverPtr drv);
 
 #endif /* __VIR_SECURITY_H__ */

src/security/security_selinux.c

@@ -266,13 +266,15 @@ SELinuxSecurityDriverProbe(void)
 }
 
 static int
-SELinuxSecurityDriverOpen(virSecurityDriverPtr drv)
+SELinuxSecurityDriverOpen(virSecurityDriverPtr drv,
+                          bool allowDiskFormatProbing)
 {
     /*
      * Where will the DOI come from?  SELinux configuration, or qemu
      * configuration? For the moment, we'll just set it to "0".
      */
     virSecurityDriverSetDOI(drv, SECURITY_SELINUX_VOID_DOI);
+    virSecurityDriverSetAllowDiskFormatProbing(drv, allowDiskFormatProbing);
     return SELinuxInitialize();
 }
 
@@ -467,18 +469,19 @@ SELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk,
 }
 
 static int
-SELinuxSetSecurityImageLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED,
+SELinuxSetSecurityImageLabel(virSecurityDriverPtr drv,
                              virDomainObjPtr vm,
                              virDomainDiskDefPtr disk)
 
 {
     const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
+    bool allowDiskFormatProbing = virSecurityDriverGetAllowDiskFormatProbing(drv);
 
     if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC)
         return 0;
 
     return virDomainDiskDefForeachPath(disk,
-                                       true,
+                                       allowDiskFormatProbing,
                                        false,
                                        SELinuxSetSecurityFileLabel,
                                        secdef);

src/security/virt-aa-helper.c

@@ -40,6 +40,7 @@
 static char *progname;
 
 typedef struct {
+    bool allowDiskFormatProbing;
     char uuid[PROFILE_NAME_SIZE];       /* UUID of vm */
     bool dryrun;                /* dry run */
     char cmd;                   /* 'c'   create
@@ -844,7 +845,7 @@ get_files(vahControl * ctl)
 
     for (i = 0; i < ctl->def->ndisks; i++) {
         int ret = virDomainDiskDefForeachPath(ctl->def->disks[i],
-                                              true,
+                                              ctl->allowDiskFormatProbing,
                                               false,
                                               add_file_path,
                                               &buf);
@@ -943,6 +944,7 @@ vahParseArgv(vahControl * ctl, int argc, char **argv)
 {
     int arg, idx = 0;
     struct option opt[] = {
+        {"probing", 1, 0, 'p' },
         {"add", 0, 0, 'a'},
         {"create", 0, 0, 'c'},
         {"dryrun", 0, 0, 'd'},
@@ -991,6 +993,12 @@ vahParseArgv(vahControl * ctl, int argc, char **argv)
                     PROFILE_NAME_SIZE) == NULL)
                     vah_error(ctl, 1, "error copying UUID");
                 break;
+            case 'p':
+                if (STREQ(optarg, "1"))
+                    ctl->allowDiskFormatProbing = true;
+                else
+                    ctl->allowDiskFormatProbing = false;
+                break;
             default:
                 vah_error(ctl, 1, "unsupported option");
                 break;

tests/seclabeltest.c

@@ -15,7 +15,7 @@ main (int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED)
     const char *doi, *model;
     virSecurityDriverPtr security_drv;
 
-    ret = virSecurityDriverStartup (&security_drv, "selinux");
+    ret = virSecurityDriverStartup (&security_drv, "selinux", false);
     if (ret == -1)
     {
         fprintf (stderr, "Failed to start security driver");