[libvirt] [PATCH 0/3] Fix security driver integration for lock manager
This series ensures we correctly label the sanlock socket that gets leaked to QEMU
The virSecurityManagerSetFDLabel method is used to label file descriptors associated with disk images. There will shortly be a need to label other file descriptors in a different way. So the current name is ambiguous. Rename the method to virSecurityManagerSetImageFDLabel to clarify its purpose * src/libvirt_private.syms, src/qemu/qemu_migration.c, src/qemu/qemu_process.c, src/security/security_apparmor.c, src/security/security_dac.c, src/security/security_driver.h, src/security/security_manager.c, src/security/security_manager.h, src/security/security_selinux.c, src/security/security_stack.c: s/FDLabel/ImageFDLabel/ --- src/libvirt_private.syms | 2 +- src/qemu/qemu_migration.c | 4 ++-- src/qemu/qemu_process.c | 2 +- src/security/security_apparmor.c | 8 ++++---- src/security/security_dac.c | 8 ++++---- src/security/security_driver.h | 8 ++++---- src/security/security_manager.c | 10 +++++----- src/security/security_manager.h | 6 +++--- src/security/security_selinux.c | 8 ++++---- src/security/security_stack.c | 12 ++++++------ 10 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d9d3913..90725cd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -849,7 +849,7 @@ virSecurityManagerRestoreAllLabel; virSecurityManagerRestoreHostdevLabel; virSecurityManagerRestoreSavedStateLabel; virSecurityManagerSetAllLabel; -virSecurityManagerSetFDLabel; +virSecurityManagerSetImageFDLabel; virSecurityManagerSetImageLabel; virSecurityManagerSetHostdevLabel; virSecurityManagerSetProcessLabel; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index ca4a884..800b714 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2688,8 +2688,8 @@ qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm, * doesn't have to open() the file, so while we still have to * grant SELinux access, we can do it on fd and avoid cleanup * later, as well as skip futzing with cgroup. */ - if (virSecurityManagerSetFDLabel(driver->securityManager, vm, - compressor ? pipeFD[1] : fd) < 0) + if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm, + compressor ? pipeFD[1] : fd) < 0) goto cleanup; bypassSecurityDriver = true; } else { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7e2806b..e00d5a8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2727,7 +2727,7 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; } if (S_ISFIFO(stdin_sb.st_mode) && - virSecurityManagerSetFDLabel(driver->securityManager, vm, stdin_fd) < 0) + virSecurityManagerSetImageFDLabel(driver->securityManager, vm, stdin_fd) < 0) goto cleanup; } diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 4d77643..50a7383 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -757,9 +757,9 @@ AppArmorRestoreSavedStateLabel(virSecurityManagerPtr mgr, } static int -AppArmorSetFDLabel(virSecurityManagerPtr mgr, - virDomainObjPtr vm, - int fd) +AppArmorSetImageFDLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int fd) { int rc = -1; char *proc = NULL; @@ -818,5 +818,5 @@ virSecurityDriver virAppArmorSecurityDriver = { AppArmorSetSavedStateLabel, AppArmorRestoreSavedStateLabel, - AppArmorSetFDLabel, + AppArmorSetImageFDLabel, }; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 24b50e6..49bba5c 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -682,9 +682,9 @@ virSecurityDACClearSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, } static int -virSecurityDACSetFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainObjPtr vm ATTRIBUTE_UNUSED, - int fd ATTRIBUTE_UNUSED) +virSecurityDACSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + int fd ATTRIBUTE_UNUSED) { return 0; } @@ -725,5 +725,5 @@ virSecurityDriver virSecurityDriverDAC = { virSecurityDACSetSavedStateLabel, virSecurityDACRestoreSavedStateLabel, - virSecurityDACSetFDLabel, + virSecurityDACSetImageFDLabel, }; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 42dfcb8..6c6db3e 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -79,9 +79,9 @@ typedef int (*virSecurityDomainSetProcessLabel) (virSecurityManagerPtr mgr, virDomainObjPtr vm); typedef int (*virSecurityDomainSecurityVerify) (virSecurityManagerPtr mgr, virDomainDefPtr def); -typedef int (*virSecurityDomainSetFDLabel) (virSecurityManagerPtr mgr, - virDomainObjPtr vm, - int fd); +typedef int (*virSecurityDomainSetImageFDLabel) (virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int fd); struct _virSecurityDriver { size_t privateDataLen; @@ -117,7 +117,7 @@ struct _virSecurityDriver { virSecurityDomainSetSavedStateLabel domainSetSavedStateLabel; virSecurityDomainRestoreSavedStateLabel domainRestoreSavedStateLabel; - virSecurityDomainSetFDLabel domainSetSecurityFDLabel; + virSecurityDomainSetImageFDLabel domainSetSecurityImageFDLabel; }; virSecurityDriverPtr virSecurityDriverLookup(const char *name); diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 6f0becd..04159f4 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -326,12 +326,12 @@ int virSecurityManagerVerify(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerSetFDLabel(virSecurityManagerPtr mgr, - virDomainObjPtr vm, - int fd) +int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int fd) { - if (mgr->drv->domainSetSecurityFDLabel) - return mgr->drv->domainSetSecurityFDLabel(mgr, vm, fd); + if (mgr->drv->domainSetSecurityImageFDLabel) + return mgr->drv->domainSetSecurityImageFDLabel(mgr, vm, fd); virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 8d7c220..581957c 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -91,8 +91,8 @@ int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm); int virSecurityManagerVerify(virSecurityManagerPtr mgr, virDomainDefPtr def); -int virSecurityManagerSetFDLabel(virSecurityManagerPtr mgr, - virDomainObjPtr vm, - int fd); +int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int fd); #endif /* VIR_SECURITY_MANAGER_H__ */ diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 0ce999f..dc92ce6 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1209,9 +1209,9 @@ SELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr, } static int -SELinuxSetFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainObjPtr vm, - int fd) +SELinuxSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + int fd) { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; @@ -1255,5 +1255,5 @@ virSecurityDriver virSecurityDriverSELinux = { SELinuxSetSavedStateLabel, SELinuxRestoreSavedStateLabel, - SELinuxSetFDLabel, + SELinuxSetImageFDLabel, }; diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 64f745a..bec1626 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -370,16 +370,16 @@ virSecurityStackClearSocketLabel(virSecurityManagerPtr mgr, } static int -virSecurityStackSetFDLabel(virSecurityManagerPtr mgr, - virDomainObjPtr vm, - int fd) +virSecurityStackSetImageFDLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int fd) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); int rc = 0; - if (virSecurityManagerSetFDLabel(priv->secondary, vm, fd) < 0) + if (virSecurityManagerSetImageFDLabel(priv->secondary, vm, fd) < 0) rc = -1; - if (virSecurityManagerSetFDLabel(priv->primary, vm, fd) < 0) + if (virSecurityManagerSetImageFDLabel(priv->primary, vm, fd) < 0) rc = -1; return rc; @@ -420,5 +420,5 @@ virSecurityDriver virSecurityDriverStack = { virSecurityStackSetSavedStateLabel, virSecurityStackRestoreSavedStateLabel, - virSecurityStackSetFDLabel, + virSecurityStackSetImageFDLabel, }; -- 1.7.4.4
On 06/24/2011 09:09 AM, Daniel P. Berrange wrote:
The virSecurityManagerSetFDLabel method is used to label file descriptors associated with disk images. There will shortly be a need to label other file descriptors in a different way. So the current name is ambiguous. Rename the method to virSecurityManagerSetImageFDLabel to clarify its purpose
* src/libvirt_private.syms, src/qemu/qemu_migration.c, src/qemu/qemu_process.c, src/security/security_apparmor.c, src/security/security_dac.c, src/security/security_driver.h, src/security/security_manager.c, src/security/security_manager.h, src/security/security_selinux.c, src/security/security_stack.c: s/FDLabel/ImageFDLabel/ ---
Very mechanical. ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Fri, 2011-06-24 at 16:09 +0100, Daniel P. Berrange wrote:
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 4d77643..50a7383 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -757,9 +757,9 @@ AppArmorRestoreSavedStateLabel(virSecurityManagerPtr mgr, }
static int -AppArmorSetFDLabel(virSecurityManagerPtr mgr, - virDomainObjPtr vm, - int fd) +AppArmorSetImageFDLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int fd) { int rc = -1; char *proc = NULL; @@ -818,5 +818,5 @@ virSecurityDriver virAppArmorSecurityDriver = { AppArmorSetSavedStateLabel, AppArmorRestoreSavedStateLabel,
- AppArmorSetFDLabel, + AppArmorSetImageFDLabel, };
ACK -- Jamie Strandboge | http://www.canonical.com
Add a new security driver method for labelling an FD with the process label, rather than the image label * src/libvirt_private.syms, src/security/security_apparmor.c, src/security/security_dac.c, src/security/security_driver.h, src/security/security_manager.c, src/security/security_manager.h, src/security/security_selinux.c, src/security/security_stack.c: Add virSecurityManagerSetProcessFDLabel & impl --- src/libvirt_private.syms | 1 + src/security/security_apparmor.c | 29 +++++++++++++++++++++++++++++ src/security/security_dac.c | 9 +++++++++ src/security/security_driver.h | 4 ++++ src/security/security_manager.c | 11 +++++++++++ src/security/security_manager.h | 3 +++ src/security/security_selinux.c | 14 ++++++++++++++ src/security/security_stack.c | 18 ++++++++++++++++++ 8 files changed, 89 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 90725cd..2d3f9d8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -852,6 +852,7 @@ virSecurityManagerSetAllLabel; virSecurityManagerSetImageFDLabel; virSecurityManagerSetImageLabel; virSecurityManagerSetHostdevLabel; +virSecurityManagerSetProcessFDLabel; virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 50a7383..df8c66c 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -784,6 +784,34 @@ AppArmorSetImageFDLabel(virSecurityManagerPtr mgr, return reload_profile(mgr, vm, fd_path, true); } +static int +AppArmorSetProcessFDLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int fd) +{ + int rc = -1; + char *proc = NULL; + char *fd_path = NULL; + + const virSecurityLabelDefPtr secdef = &vm->def->seclabel; + + if (secdef->imagelabel == NULL) + return 0; + + if (virAsprintf(&proc, "/proc/self/fd/%d", fd) == -1) { + virReportOOMError(); + return rc; + } + + if (virFileResolveLink(proc, &fd_path) < 0) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("could not find path for descriptor")); + return rc; + } + + return reload_profile(mgr, vm, fd_path, true); +} + virSecurityDriver virAppArmorSecurityDriver = { 0, SECURITY_APPARMOR_NAME, @@ -819,4 +847,5 @@ virSecurityDriver virAppArmorSecurityDriver = { AppArmorRestoreSavedStateLabel, AppArmorSetImageFDLabel, + AppArmorSetProcessFDLabel, }; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 49bba5c..58d57ec 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -689,6 +689,14 @@ virSecurityDACSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return 0; } +static int +virSecurityDACSetProcessFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + int fd ATTRIBUTE_UNUSED) +{ + return 0; +} + virSecurityDriver virSecurityDriverDAC = { sizeof(virSecurityDACData), @@ -726,4 +734,5 @@ virSecurityDriver virSecurityDriverDAC = { virSecurityDACRestoreSavedStateLabel, virSecurityDACSetImageFDLabel, + virSecurityDACSetProcessFDLabel, }; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 6c6db3e..154f197 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -82,6 +82,9 @@ typedef int (*virSecurityDomainSecurityVerify) (virSecurityManagerPtr mgr, typedef int (*virSecurityDomainSetImageFDLabel) (virSecurityManagerPtr mgr, virDomainObjPtr vm, int fd); +typedef int (*virSecurityDomainSetProcessFDLabel) (virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int fd); struct _virSecurityDriver { size_t privateDataLen; @@ -118,6 +121,7 @@ struct _virSecurityDriver { virSecurityDomainRestoreSavedStateLabel domainRestoreSavedStateLabel; virSecurityDomainSetImageFDLabel domainSetSecurityImageFDLabel; + virSecurityDomainSetProcessFDLabel domainSetSecurityProcessFDLabel; }; virSecurityDriverPtr virSecurityDriverLookup(const char *name); diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 04159f4..6ae58dc 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -336,3 +336,14 @@ int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr, virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } + +int virSecurityManagerSetProcessFDLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int fd) +{ + if (mgr->drv->domainSetSecurityProcessFDLabel) + return mgr->drv->domainSetSecurityProcessFDLabel(mgr, vm, fd); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 581957c..8c3b8b2 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -94,5 +94,8 @@ int virSecurityManagerVerify(virSecurityManagerPtr mgr, int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm, int fd); +int virSecurityManagerSetProcessFDLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int fd); #endif /* VIR_SECURITY_MANAGER_H__ */ diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index dc92ce6..a022daa 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1221,6 +1221,19 @@ SELinuxSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return SELinuxFSetFilecon(fd, secdef->imagelabel); } +static int +SELinuxSetProcessFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + int fd) +{ + const virSecurityLabelDefPtr secdef = &vm->def->seclabel; + + if (secdef->label == NULL) + return 0; + + return SELinuxFSetFilecon(fd, secdef->label); +} + virSecurityDriver virSecurityDriverSELinux = { 0, SECURITY_SELINUX_NAME, @@ -1256,4 +1269,5 @@ virSecurityDriver virSecurityDriverSELinux = { SELinuxRestoreSavedStateLabel, SELinuxSetImageFDLabel, + SELinuxSetProcessFDLabel, }; diff --git a/src/security/security_stack.c b/src/security/security_stack.c index bec1626..b63e4c8 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -386,6 +386,23 @@ virSecurityStackSetImageFDLabel(virSecurityManagerPtr mgr, } +static int +virSecurityStackSetProcessFDLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int fd) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerSetProcessFDLabel(priv->secondary, vm, fd) < 0) + rc = -1; + if (virSecurityManagerSetProcessFDLabel(priv->primary, vm, fd) < 0) + rc = -1; + + return rc; +} + + virSecurityDriver virSecurityDriverStack = { sizeof(virSecurityStackData), "stack", @@ -421,4 +438,5 @@ virSecurityDriver virSecurityDriverStack = { virSecurityStackRestoreSavedStateLabel, virSecurityStackSetImageFDLabel, + virSecurityStackSetProcessFDLabel, }; -- 1.7.4.4
On 06/24/2011 09:09 AM, Daniel P. Berrange wrote:
Add a new security driver method for labelling an FD with the process label, rather than the image label
* src/libvirt_private.syms, src/security/security_apparmor.c, src/security/security_dac.c, src/security/security_driver.h, src/security/security_manager.c, src/security/security_manager.h, src/security/security_selinux.c, src/security/security_stack.c: Add virSecurityManagerSetProcessFDLabel & impl --- src/libvirt_private.syms | 1 + src/security/security_apparmor.c | 29 +++++++++++++++++++++++++++++ src/security/security_dac.c | 9 +++++++++ src/security/security_driver.h | 4 ++++ src/security/security_manager.c | 11 +++++++++++ src/security/security_manager.h | 3 +++ src/security/security_selinux.c | 14 ++++++++++++++ src/security/security_stack.c | 18 ++++++++++++++++++ 8 files changed, 89 insertions(+), 0 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Fri, 2011-06-24 at 16:09 +0100, Daniel P. Berrange wrote:
Add a new security driver method for labelling an FD with the process label, rather than the image label
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 90725cd..2d3f9d8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -852,6 +852,7 @@ virSecurityManagerSetAllLabel; virSecurityManagerSetImageFDLabel; virSecurityManagerSetImageLabel; virSecurityManagerSetHostdevLabel; +virSecurityManagerSetProcessFDLabel; virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 50a7383..df8c66c 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -784,6 +784,34 @@ AppArmorSetImageFDLabel(virSecurityManagerPtr mgr, return reload_profile(mgr, vm, fd_path, true); }
+static int +AppArmorSetProcessFDLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int fd) +{ + int rc = -1; + char *proc = NULL; + char *fd_path = NULL; + + const virSecurityLabelDefPtr secdef = &vm->def->seclabel; + + if (secdef->imagelabel == NULL) + return 0; + + if (virAsprintf(&proc, "/proc/self/fd/%d", fd) == -1) { + virReportOOMError(); + return rc; + } + + if (virFileResolveLink(proc, &fd_path) < 0) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("could not find path for descriptor")); + return rc; + } + + return reload_profile(mgr, vm, fd_path, true); +} + virSecurityDriver virAppArmorSecurityDriver = { 0, SECURITY_APPARMOR_NAME, @@ -819,4 +847,5 @@ virSecurityDriver virAppArmorSecurityDriver = { AppArmorRestoreSavedStateLabel,
AppArmorSetImageFDLabel, + AppArmorSetProcessFDLabel, };
ACK, though this and AppArmorSetImageFDLabel() are now identical and could therefore be refactored. I've made a note to check on this after the SetProcessFDLabel() changes are in place. -- Jamie Strandboge | http://www.canonical.com
On 06/24/2011 09:27 AM, Jamie Strandboge wrote:
@@ -819,4 +847,5 @@ virSecurityDriver virAppArmorSecurityDriver = { AppArmorRestoreSavedStateLabel,
AppArmorSetImageFDLabel, + AppArmorSetProcessFDLabel, };
Should we do a separate patch to make the security drivers use C99 named initialization, instead of C89 order-based, to match how most other driver callback structures are now set up? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
The libvirt sanlock plugin is intentionally leaking a file descriptor to QEMU. To enable QEMU to use this FD under SELinux, it must be labelled correctly. We dont want to use the svirt_image_t for this, since QEMU must not be allowed to actually use the FD. So instead we label it with svirt_t using virSecurityManagerSetProcessFDLabel * src/locking/domain_lock.c, src/locking/domain_lock.h, src/locking/lock_driver.h, src/locking/lock_driver_nop.c, src/locking/lock_driver_sanlock.c, src/locking/lock_manager.c, src/locking/lock_manager.h: Optionally pass an FD back to the hypervisor for security driver labelling * src/qemu/qemu_process.c: label the lock manager plugin FD with the process label --- src/locking/domain_lock.c | 11 ++++++----- src/locking/domain_lock.h | 3 ++- src/locking/lock_driver.h | 9 ++++++++- src/locking/lock_driver_nop.c | 4 ++-- src/locking/lock_driver_sanlock.c | 6 +++++- src/locking/lock_manager.c | 10 +++++++--- src/locking/lock_manager.h | 3 ++- src/qemu/qemu_process.c | 12 ++++++++++-- 8 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c index feb3f98..4544500 100644 --- a/src/locking/domain_lock.c +++ b/src/locking/domain_lock.c @@ -155,7 +155,8 @@ error: int virDomainLockProcessStart(virLockManagerPluginPtr plugin, virDomainObjPtr dom, - bool paused) + bool paused, + int *fd) { virLockManagerPtr lock; int ret; @@ -167,7 +168,7 @@ int virDomainLockProcessStart(virLockManagerPluginPtr plugin, if (paused) flags |= VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY; - ret = virLockManagerAcquire(lock, NULL, flags); + ret = virLockManagerAcquire(lock, NULL, flags, fd); virLockManagerFree(lock); @@ -200,7 +201,7 @@ int virDomainLockProcessResume(virLockManagerPluginPtr plugin, if (!(lock = virDomainLockManagerNew(plugin, dom, true))) return -1; - ret = virLockManagerAcquire(lock, state, 0); + ret = virLockManagerAcquire(lock, state, 0, NULL); virLockManagerFree(lock); return ret; @@ -236,7 +237,7 @@ int virDomainLockDiskAttach(virLockManagerPluginPtr plugin, if (virDomainLockManagerAddDisk(lock, disk) < 0) goto cleanup; - if (virLockManagerAcquire(lock, NULL, 0) < 0) + if (virLockManagerAcquire(lock, NULL, 0, NULL) < 0) goto cleanup; ret = 0; @@ -285,7 +286,7 @@ int virDomainLockLeaseAttach(virLockManagerPluginPtr plugin, if (virDomainLockManagerAddLease(lock, lease) < 0) goto cleanup; - if (virLockManagerAcquire(lock, NULL, 0) < 0) + if (virLockManagerAcquire(lock, NULL, 0, NULL) < 0) goto cleanup; ret = 0; diff --git a/src/locking/domain_lock.h b/src/locking/domain_lock.h index 40fadd4..dd35c7c 100644 --- a/src/locking/domain_lock.h +++ b/src/locking/domain_lock.h @@ -28,7 +28,8 @@ int virDomainLockProcessStart(virLockManagerPluginPtr plugin, virDomainObjPtr dom, - bool paused); + bool paused, + int *fd); int virDomainLockProcessPause(virLockManagerPluginPtr plugin, virDomainObjPtr dom, char **state); diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h index 3b90efa..94145cb 100644 --- a/src/locking/lock_driver.h +++ b/src/locking/lock_driver.h @@ -215,6 +215,7 @@ typedef int (*virLockDriverAddResource)(virLockManagerPtr man, * @manager: the lock manager context * @state: the current lock state * @flags: optional flags, currently unused + * @fd: optional return the leaked FD * * Start managing resources for the object. This * must be called from the PID that represents the @@ -223,11 +224,17 @@ typedef int (*virLockDriverAddResource)(virLockManagerPtr man, * The optional state contains information about the * locks previously held for the object. * + * The file descriptor returned in @fd is one that + * is intentionally leaked and should not be closed. + * It is returned so that it can be labelled by the + * security managers (if required). + * * Returns 0 on success, or -1 on failure */ typedef int (*virLockDriverAcquire)(virLockManagerPtr man, const char *state, - unsigned int flags); + unsigned int flags, + int *fd); /** * virLockDriverRelease: diff --git a/src/locking/lock_driver_nop.c b/src/locking/lock_driver_nop.c index 43374e4..0fab54c 100644 --- a/src/locking/lock_driver_nop.c +++ b/src/locking/lock_driver_nop.c @@ -67,9 +67,9 @@ static int virLockManagerNopAddResource(virLockManagerPtr lock ATTRIBUTE_UNUSED, static int virLockManagerNopAcquire(virLockManagerPtr lock ATTRIBUTE_UNUSED, const char *state ATTRIBUTE_UNUSED, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags ATTRIBUTE_UNUSED, + int *fd ATTRIBUTE_UNUSED) { - return 0; } diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index ad3a65a..76e6ddc 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -667,7 +667,8 @@ static int virLockManagerSanlockAddResource(virLockManagerPtr lock, static int virLockManagerSanlockAcquire(virLockManagerPtr lock, const char *state, - unsigned int flags) + unsigned int flags, + int *fd) { virLockManagerSanlockPrivatePtr priv = lock->privateData; struct sanlk_options *opt; @@ -788,6 +789,9 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr lock, VIR_FREE(res_args); } + if (fd) + *fd = sock; + return 0; error: diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c index c28ca86..4ee7f44 100644 --- a/src/locking/lock_manager.c +++ b/src/locking/lock_manager.c @@ -329,13 +329,17 @@ int virLockManagerAddResource(virLockManagerPtr lock, int virLockManagerAcquire(virLockManagerPtr lock, const char *state, - unsigned int flags) + unsigned int flags, + int *fd) { - VIR_DEBUG("lock=%p state='%s' flags=%u", lock, NULLSTR(state), flags); + VIR_DEBUG("lock=%p state='%s' flags=%u fd=%p", lock, NULLSTR(state), flags, fd); CHECK_MANAGER(drvAcquire, -1); - return lock->driver->drvAcquire(lock, state, flags); + if (fd) + *fd = -1; + + return lock->driver->drvAcquire(lock, state, flags, fd); } diff --git a/src/locking/lock_manager.h b/src/locking/lock_manager.h index 315c798..0fb3bb7 100644 --- a/src/locking/lock_manager.h +++ b/src/locking/lock_manager.h @@ -53,7 +53,8 @@ int virLockManagerAddResource(virLockManagerPtr manager, int virLockManagerAcquire(virLockManagerPtr manager, const char *state, - unsigned int flags); + unsigned int flags, + int *fd); int virLockManagerRelease(virLockManagerPtr manager, char **state, unsigned int flags); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e00d5a8..445dac9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2119,6 +2119,7 @@ static int qemuProcessHook(void *data) { struct qemuProcessHookData *h = data; int ret = -1; + int fd; /* Some later calls want pid present */ h->vm->pid = getpid(); @@ -2127,7 +2128,8 @@ static int qemuProcessHook(void *data) if (virDomainLockProcessStart(h->driver->lockManager, h->vm, /* QEMU is always pased initially */ - true) < 0) + true, + &fd) < 0) goto cleanup; if (qemuProcessLimits(h->driver) < 0) @@ -2149,10 +2151,16 @@ static int qemuProcessHook(void *data) if (qemuProcessInitNumaMemoryPolicy(h->vm) < 0) return -1; - VIR_DEBUG("Setting up security labeling"); + VIR_DEBUG("Setting up security labelling"); if (virSecurityManagerSetProcessLabel(h->driver->securityManager, h->vm) < 0) goto cleanup; + if (fd != -1) { + VIR_DEBUG("Setting up lock manager FD labelling"); + if (virSecurityManagerSetProcessFDLabel(h->driver->securityManager, h->vm, fd) < 0) + goto cleanup; + } + ret = 0; cleanup: -- 1.7.4.4
On 06/24/2011 09:09 AM, Daniel P. Berrange wrote:
The libvirt sanlock plugin is intentionally leaking a file descriptor to QEMU. To enable QEMU to use this FD under SELinux, it must be labelled correctly. We dont want to use the svirt_image_t for this, since QEMU must not be allowed to actually use the FD. So instead we label it with svirt_t using virSecurityManagerSetProcessFDLabel
* src/locking/domain_lock.c, src/locking/domain_lock.h, src/locking/lock_driver.h, src/locking/lock_driver_nop.c, src/locking/lock_driver_sanlock.c, src/locking/lock_manager.c, src/locking/lock_manager.h: Optionally pass an FD back to the hypervisor for security driver labelling * src/qemu/qemu_process.c: label the lock manager plugin FD with the process label
@@ -2149,10 +2151,16 @@ static int qemuProcessHook(void *data) if (qemuProcessInitNumaMemoryPolicy(h->vm) < 0) return -1;
- VIR_DEBUG("Setting up security labeling"); + VIR_DEBUG("Setting up security labelling");
Why the spelling change? Both spellings are valid, but I see 'labeling' in more places than labelling: http://www.googlefight.com/index.php?lang=en_GB&word1=labeling&word2=labelling ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Jamie Strandboge