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