From dac2b936e77f6c76c11f162e4b175492e4803acb Mon Sep 17 00:00:00 2001

From: Daniel P. Berrange <berrange@redhat.com>

Date: Tue, 15 Jun 2010 17:58:58 +0100

Subject: [PATCH 08/11] 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       |    1 +

 src/qemu/qemu.conf               |   12 ++++++++++++

 src/qemu/qemu_conf.c             |    4 ++++

 src/qemu/qemu_conf.h             |    1 +

 src/qemu/qemu_driver.c           |   36 +++++++++++++++++++++++-------------

 src/qemu/qemu_security_dac.c     |    2 +-

 src/qemu/test_libvirtd_qemu.aug  |    4 ++++

 src/security/security_apparmor.c |   12 ++++++++----

 src/security/security_driver.c   |   16 ++++++++++++++--

 src/security/security_driver.h   |   10 ++++++++--

 src/security/security_selinux.c  |    9 ++++++---

 src/security/virt-aa-helper.c    |   10 +++++++++-

 tests/seclabeltest.c             |    2 +-

 13 files changed, 92 insertions(+), 27 deletions(-)

diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug

index 7c9f271..47d0525 100644

--- a/src/qemu/libvirtd_qemu.aug

+++ b/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

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf

index 93934f3..dc8eb83 100644

--- a/src/qemu/qemu.conf

+++ b/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

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c

index 988220b..3ba48bf 100644

--- a/src/qemu/qemu_conf.c

+++ b/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;

 }

diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h

index ab5f158..30e9f20 100644

--- a/src/qemu/qemu_conf.h

+++ b/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;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c

index 616547c..3c479c5 100644

--- a/src/qemu/qemu_driver.c

+++ b/src/qemu/qemu_driver.c

@@ -1322,7 +1322,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;

@@ -3070,11 +3071,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);

@@ -3109,11 +3111,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);

@@ -3180,7 +3183,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;

         }

@@ -8033,7 +8036,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;

         }

@@ -8078,7 +8081,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));

         }

@@ -8278,7 +8281,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;

         }

@@ -8301,7 +8304,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));

         }

@@ -8429,7 +8432,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));

     }

@@ -8493,7 +8496,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));

     }

@@ -9672,8 +9675,15 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,

             goto cleanup;

         }

     } else {

-        if ((format = virStorageFileProbeFormat(disk->src)) < 0)

+        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,

diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c

index 0bbcf69..55dc0c6 100644

--- a/src/qemu/qemu_security_dac.c

+++ b/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);

diff --git a/src/qemu/test_libvirtd_qemu.aug b/src/qemu/test_libvirtd_qemu.aug

index 3326cc5..f0c4a0d 100644

--- a/src/qemu/test_libvirtd_qemu.aug

+++ b/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" }

diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c

index cb5c739..c5f9829 100644

--- a/src/security/security_apparmor.c

+++ b/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;

 }

diff --git a/src/security/security_driver.c b/src/security/security_driver.c

index aac9f78..9e32fa4 100644

--- a/src/security/security_driver.c

+++ b/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;

+}

diff --git a/src/security/security_driver.h b/src/security/security_driver.h

index 61c9eb0..d768f32 100644

--- a/src/security/security_driver.h

+++ b/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__ */

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c

index cc3812b..a9dd836 100644

--- a/src/security/security_selinux.c

+++ b/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);

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c

index 9ed0cd3..521545d 100644

--- a/src/security/virt-aa-helper.c

+++ b/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;

diff --git a/tests/seclabeltest.c b/tests/seclabeltest.c

index 26d1f86..ef3f026 100644

--- a/tests/seclabeltest.c

+++ b/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");

-- 

1.7.1.1