[libvirt] [PATCH 0/3] Fix security driver integration for lock manager - Devel - Libvirt List Archives

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

December

[libvirt] [PATCH 0/3] Fix security driver integration for lock manager

[libvirt] [PATCH] Move load of...

[libvirt] [PATCH] docs: document...

Daniel P. Berrange

Friday, 24 June 2011 Fri, 24 Jun '11

10:09 a.m.

This series ensures we correctly label the sanlock socket that gets leaked to QEMU

Reply

Show replies by date

Daniel P. Berrange

Friday, 24 June Fri, 24 Jun

10:09 a.m.

New subject: [libvirt] [PATCH 1/3] Rename virSecurityManagerSetFDLabel method

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

Reply

Eric Blake

10:15 a.m.

New subject: [libvirt] [PATCH 1/3] Rename virSecurityManagerSetFDLabel method

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(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Reply

Jamie Strandboge

10:19 a.m.

New subject: [libvirt] [PATCH 1/3] Rename virSecurityManagerSetFDLabel method

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

Reply

Daniel P. Berrange

10:09 a.m.

New subject: [libvirt] [PATCH 2/3] Add a virSecurityManagerSetProcessFDLabel

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

Reply

Eric Blake

10:21 a.m.

New subject: [libvirt] [PATCH 2/3] Add a virSecurityManagerSetProcessFDLabel

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(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Reply

Jamie Strandboge

10:27 a.m.

New subject: [libvirt] [PATCH 2/3] Add a virSecurityManagerSetProcessFDLabel

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

Reply

Eric Blake

10:32 a.m.

New subject: [libvirt] [PATCH 2/3] Add a virSecurityManagerSetProcessFDLabel

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(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Reply

Daniel P. Berrange

10:09 a.m.

New subject: [libvirt] [PATCH 3/3] Ensure sanlock socket is labelled with the VM process label

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

Reply

Eric Blake

10:26 a.m.

New subject: [libvirt] [PATCH 3/3] Ensure sanlock socket is labelled with the VM process label

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&wo... ACK. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Reply

4900

days inactive

4900

days old

devel@lists.libvirt.org

Manage subscription

9 comments

3 participants

tags (0)

participants (3)

Daniel P. Berrange
Eric Blake
Jamie Strandboge