From dac2b936e77f6c76c11f162e4b175492e4803acb Mon Sep 17 00:00:00 2001 From: Daniel P. Berrange 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 elements to include +# 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