[libvirt] [PATCH v3 0/3] Correctly label migration TCP socket
With current libvirt and qemu, migration is not working if SELinux is in enforcing mode, since the TCP socket we pass to qemu is not labeled in a way that would allow qemu to read from it. After this patchset, migration works even in enforcing mode. Jiri Denemark (3): security: Rename SetSocketLabel APIs to SetDaemonSocketLabel security: Introduce SetSocketLabel qemu: Correctly label migration TCP socket src/libvirt_private.syms | 1 + src/qemu/qemu_migration.c | 5 +++- src/qemu/qemu_process.c | 3 +- src/security/security_dac.c | 11 +++++++++- src/security/security_driver.h | 3 ++ src/security/security_manager.c | 10 +++++++++ src/security/security_manager.h | 2 + src/security/security_nop.c | 7 ++++++ src/security/security_selinux.c | 42 +++++++++++++++++++++++++++++++++++++- src/security/security_stack.c | 17 +++++++++++++++ 10 files changed, 96 insertions(+), 5 deletions(-) -- 1.7.6.1
The APIs are designed to label a socket in a way that the libvirt daemon itself is able to access it (i.e., in SELinux the label is virtd_t based as opposed to svirt_* we use for labeling resources that need to be accessed by a vm). The new name reflects this. --- Notes: Version 3: - new patch src/libvirt_private.syms | 2 +- src/qemu/qemu_process.c | 3 ++- src/security/security_dac.c | 6 +++--- src/security/security_driver.h | 6 +++--- src/security/security_manager.c | 8 ++++---- src/security/security_manager.h | 4 ++-- src/security/security_nop.c | 6 +++--- src/security/security_selinux.c | 6 +++--- src/security/security_stack.c | 10 +++++----- 9 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0618b49..c3e33b4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -904,13 +904,13 @@ virSecurityManagerRestoreAllLabel; virSecurityManagerRestoreHostdevLabel; virSecurityManagerRestoreSavedStateLabel; virSecurityManagerSetAllLabel; +virSecurityManagerSetDaemonSocketLabel; virSecurityManagerSetImageFDLabel; virSecurityManagerSetImageLabel; virSecurityManagerSetHostdevLabel; virSecurityManagerSetProcessFDLabel; virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; -virSecurityManagerSetSocketLabel; virSecurityManagerVerify; # sexpr.h diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f691bbb..58b4d36 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -821,7 +821,8 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; - if (virSecurityManagerSetSocketLabel(driver->securityManager, vm) < 0) { + if (virSecurityManagerSetDaemonSocketLabel(driver->securityManager, + vm) < 0) { VIR_ERROR(_("Failed to set security context for monitor for %s"), vm->def->name); goto error; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 58d57ec..6df4087 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -667,8 +667,8 @@ virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, } static int -virSecurityDACSetSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainObjPtr vm ATTRIBUTE_UNUSED) +virSecurityDACSetDaemonSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) { return 0; } @@ -714,7 +714,7 @@ virSecurityDriver virSecurityDriverDAC = { virSecurityDACSetSecurityImageLabel, virSecurityDACRestoreSecurityImageLabel, - virSecurityDACSetSocketLabel, + virSecurityDACSetDaemonSocketLabel, virSecurityDACClearSocketLabel, virSecurityDACGenLabel, diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 154f197..73c8f04 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -41,8 +41,8 @@ typedef const char *(*virSecurityDriverGetDOI) (virSecurityManagerPtr mgr); typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr, virDomainObjPtr vm, virDomainDiskDefPtr disk); -typedef int (*virSecurityDomainSetSocketLabel) (virSecurityManagerPtr mgr, - virDomainObjPtr vm); +typedef int (*virSecurityDomainSetDaemonSocketLabel)(virSecurityManagerPtr mgr, + virDomainObjPtr vm); typedef int (*virSecurityDomainClearSocketLabel)(virSecurityManagerPtr mgr, virDomainObjPtr vm); typedef int (*virSecurityDomainSetImageLabel) (virSecurityManagerPtr mgr, @@ -101,7 +101,7 @@ struct _virSecurityDriver { virSecurityDomainSetImageLabel domainSetSecurityImageLabel; virSecurityDomainRestoreImageLabel domainRestoreSecurityImageLabel; - virSecurityDomainSetSocketLabel domainSetSecuritySocketLabel; + virSecurityDomainSetDaemonSocketLabel domainSetSecurityDaemonSocketLabel; virSecurityDomainClearSocketLabel domainClearSecuritySocketLabel; virSecurityDomainGenLabel domainGenSecurityLabel; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 6ae58dc..d30ebcf 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -160,11 +160,11 @@ int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, return -1; } -int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, - virDomainObjPtr vm) +int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) { - if (mgr->drv->domainSetSecuritySocketLabel) - return mgr->drv->domainSetSecuritySocketLabel(mgr, vm); + if (mgr->drv->domainSetSecurityDaemonSocketLabel) + return mgr->drv->domainSetSecurityDaemonSocketLabel(mgr, vm); virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 8c3b8b2..8d614a7 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -53,8 +53,8 @@ bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr); int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm, virDomainDiskDefPtr disk); -int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, - virDomainObjPtr vm); +int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm); int virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm); int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr, diff --git a/src/security/security_nop.c b/src/security/security_nop.c index 24d36fe..67d3ff6 100644 --- a/src/security/security_nop.c +++ b/src/security/security_nop.c @@ -53,8 +53,8 @@ static int virSecurityDomainRestoreImageLabelNop(virSecurityManagerPtr mgr ATTRI return 0; } -static int virSecurityDomainSetSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainObjPtr vm ATTRIBUTE_UNUSED) +static int virSecurityDomainSetDaemonSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) { return 0; } @@ -171,7 +171,7 @@ virSecurityDriver virSecurityDriverNop = { virSecurityDomainSetImageLabelNop, virSecurityDomainRestoreImageLabelNop, - virSecurityDomainSetSocketLabelNop, + virSecurityDomainSetDaemonSocketLabelNop, virSecurityDomainClearSocketLabelNop, virSecurityDomainGenLabelNop, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 5e6145f..f87c9a5 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1066,8 +1066,8 @@ SELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr, } static int -SELinuxSetSecuritySocketLabel(virSecurityManagerPtr mgr, - virDomainObjPtr vm) +SELinuxSetSecurityDaemonSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) { /* TODO: verify DOI */ const virSecurityLabelDefPtr secdef = &vm->def->seclabel; @@ -1312,7 +1312,7 @@ virSecurityDriver virSecurityDriverSELinux = { SELinuxSetSecurityImageLabel, SELinuxRestoreSecurityImageLabel, - SELinuxSetSecuritySocketLabel, + SELinuxSetSecurityDaemonSocketLabel, SELinuxClearSecuritySocketLabel, SELinuxGenSecurityLabel, diff --git a/src/security/security_stack.c b/src/security/security_stack.c index b63e4c8..404ff65 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -339,15 +339,15 @@ virSecurityStackGetProcessLabel(virSecurityManagerPtr mgr, static int -virSecurityStackSetSocketLabel(virSecurityManagerPtr mgr, - virDomainObjPtr vm) +virSecurityStackSetDaemonSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); int rc = 0; - if (virSecurityManagerSetSocketLabel(priv->secondary, vm) < 0) + if (virSecurityManagerSetDaemonSocketLabel(priv->secondary, vm) < 0) rc = -1; - if (virSecurityManagerSetSocketLabel(priv->primary, vm) < 0) + if (virSecurityManagerSetDaemonSocketLabel(priv->primary, vm) < 0) rc = -1; return rc; @@ -418,7 +418,7 @@ virSecurityDriver virSecurityDriverStack = { virSecurityStackSetSecurityImageLabel, virSecurityStackRestoreSecurityImageLabel, - virSecurityStackSetSocketLabel, + virSecurityStackSetDaemonSocketLabel, virSecurityStackClearSocketLabel, virSecurityStackGenLabel, -- 1.7.6.1
On Fri, Aug 26, 2011 at 10:23:46AM +0200, Jiri Denemark wrote:
The APIs are designed to label a socket in a way that the libvirt daemon itself is able to access it (i.e., in SELinux the label is virtd_t based as opposed to svirt_* we use for labeling resources that need to be accessed by a vm). The new name reflects this. --- Notes: Version 3: - new patch
src/libvirt_private.syms | 2 +- src/qemu/qemu_process.c | 3 ++- src/security/security_dac.c | 6 +++--- src/security/security_driver.h | 6 +++--- src/security/security_manager.c | 8 ++++---- src/security/security_manager.h | 4 ++-- src/security/security_nop.c | 6 +++--- src/security/security_selinux.c | 6 +++--- src/security/security_stack.c | 10 +++++----- 9 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0618b49..c3e33b4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -904,13 +904,13 @@ virSecurityManagerRestoreAllLabel; virSecurityManagerRestoreHostdevLabel; virSecurityManagerRestoreSavedStateLabel; virSecurityManagerSetAllLabel; +virSecurityManagerSetDaemonSocketLabel; virSecurityManagerSetImageFDLabel; virSecurityManagerSetImageLabel; virSecurityManagerSetHostdevLabel; virSecurityManagerSetProcessFDLabel; virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; -virSecurityManagerSetSocketLabel; virSecurityManagerVerify;
# sexpr.h diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f691bbb..58b4d36 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -821,7 +821,8 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1;
- if (virSecurityManagerSetSocketLabel(driver->securityManager, vm) < 0) { + if (virSecurityManagerSetDaemonSocketLabel(driver->securityManager, + vm) < 0) { VIR_ERROR(_("Failed to set security context for monitor for %s"), vm->def->name); goto error; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 58d57ec..6df4087 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -667,8 +667,8 @@ virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, }
static int -virSecurityDACSetSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainObjPtr vm ATTRIBUTE_UNUSED) +virSecurityDACSetDaemonSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) { return 0; } @@ -714,7 +714,7 @@ virSecurityDriver virSecurityDriverDAC = { virSecurityDACSetSecurityImageLabel, virSecurityDACRestoreSecurityImageLabel,
- virSecurityDACSetSocketLabel, + virSecurityDACSetDaemonSocketLabel, virSecurityDACClearSocketLabel,
virSecurityDACGenLabel, diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 154f197..73c8f04 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -41,8 +41,8 @@ typedef const char *(*virSecurityDriverGetDOI) (virSecurityManagerPtr mgr); typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr, virDomainObjPtr vm, virDomainDiskDefPtr disk); -typedef int (*virSecurityDomainSetSocketLabel) (virSecurityManagerPtr mgr, - virDomainObjPtr vm); +typedef int (*virSecurityDomainSetDaemonSocketLabel)(virSecurityManagerPtr mgr, + virDomainObjPtr vm); typedef int (*virSecurityDomainClearSocketLabel)(virSecurityManagerPtr mgr, virDomainObjPtr vm); typedef int (*virSecurityDomainSetImageLabel) (virSecurityManagerPtr mgr, @@ -101,7 +101,7 @@ struct _virSecurityDriver { virSecurityDomainSetImageLabel domainSetSecurityImageLabel; virSecurityDomainRestoreImageLabel domainRestoreSecurityImageLabel;
- virSecurityDomainSetSocketLabel domainSetSecuritySocketLabel; + virSecurityDomainSetDaemonSocketLabel domainSetSecurityDaemonSocketLabel; virSecurityDomainClearSocketLabel domainClearSecuritySocketLabel;
virSecurityDomainGenLabel domainGenSecurityLabel; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 6ae58dc..d30ebcf 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -160,11 +160,11 @@ int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, return -1; }
-int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, - virDomainObjPtr vm) +int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) { - if (mgr->drv->domainSetSecuritySocketLabel) - return mgr->drv->domainSetSecuritySocketLabel(mgr, vm); + if (mgr->drv->domainSetSecurityDaemonSocketLabel) + return mgr->drv->domainSetSecurityDaemonSocketLabel(mgr, vm);
virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 8c3b8b2..8d614a7 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -53,8 +53,8 @@ bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr); int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm, virDomainDiskDefPtr disk); -int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, - virDomainObjPtr vm); +int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm); int virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm); int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr, diff --git a/src/security/security_nop.c b/src/security/security_nop.c index 24d36fe..67d3ff6 100644 --- a/src/security/security_nop.c +++ b/src/security/security_nop.c @@ -53,8 +53,8 @@ static int virSecurityDomainRestoreImageLabelNop(virSecurityManagerPtr mgr ATTRI return 0; }
-static int virSecurityDomainSetSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainObjPtr vm ATTRIBUTE_UNUSED) +static int virSecurityDomainSetDaemonSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) { return 0; } @@ -171,7 +171,7 @@ virSecurityDriver virSecurityDriverNop = { virSecurityDomainSetImageLabelNop, virSecurityDomainRestoreImageLabelNop,
- virSecurityDomainSetSocketLabelNop, + virSecurityDomainSetDaemonSocketLabelNop, virSecurityDomainClearSocketLabelNop,
virSecurityDomainGenLabelNop, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 5e6145f..f87c9a5 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1066,8 +1066,8 @@ SELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr, }
static int -SELinuxSetSecuritySocketLabel(virSecurityManagerPtr mgr, - virDomainObjPtr vm) +SELinuxSetSecurityDaemonSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) { /* TODO: verify DOI */ const virSecurityLabelDefPtr secdef = &vm->def->seclabel; @@ -1312,7 +1312,7 @@ virSecurityDriver virSecurityDriverSELinux = { SELinuxSetSecurityImageLabel, SELinuxRestoreSecurityImageLabel,
- SELinuxSetSecuritySocketLabel, + SELinuxSetSecurityDaemonSocketLabel, SELinuxClearSecuritySocketLabel,
SELinuxGenSecurityLabel, diff --git a/src/security/security_stack.c b/src/security/security_stack.c index b63e4c8..404ff65 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -339,15 +339,15 @@ virSecurityStackGetProcessLabel(virSecurityManagerPtr mgr,
static int -virSecurityStackSetSocketLabel(virSecurityManagerPtr mgr, - virDomainObjPtr vm) +virSecurityStackSetDaemonSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); int rc = 0;
- if (virSecurityManagerSetSocketLabel(priv->secondary, vm) < 0) + if (virSecurityManagerSetDaemonSocketLabel(priv->secondary, vm) < 0) rc = -1; - if (virSecurityManagerSetSocketLabel(priv->primary, vm) < 0) + if (virSecurityManagerSetDaemonSocketLabel(priv->primary, vm) < 0) rc = -1;
return rc; @@ -418,7 +418,7 @@ virSecurityDriver virSecurityDriverStack = { virSecurityStackSetSecurityImageLabel, virSecurityStackRestoreSecurityImageLabel,
- virSecurityStackSetSocketLabel, + virSecurityStackSetDaemonSocketLabel, virSecurityStackClearSocketLabel,
virSecurityStackGenLabel,
ACK, this looks indeed as pure renaming, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Fri, Aug 26, 2011 at 10:23:46AM +0200, Jiri Denemark wrote:
The APIs are designed to label a socket in a way that the libvirt daemon itself is able to access it (i.e., in SELinux the label is virtd_t based as opposed to svirt_* we use for labeling resources that need to be accessed by a vm). The new name reflects this. --- Notes: Version 3: - new patch
src/libvirt_private.syms | 2 +- src/qemu/qemu_process.c | 3 ++- src/security/security_dac.c | 6 +++--- src/security/security_driver.h | 6 +++--- src/security/security_manager.c | 8 ++++---- src/security/security_manager.h | 4 ++-- src/security/security_nop.c | 6 +++--- src/security/security_selinux.c | 6 +++--- src/security/security_stack.c | 10 +++++----- 9 files changed, 26 insertions(+), 25 deletions(-)
Everyone always forgets about the poor apparmour driver since it isn't compiled in Fedora/RHEL.... :-( Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
This API labels all sockets created until ClearSocketLabel is called in a way that a vm can access them (i.e., they are labeled with svirt_t based label in SELinux). --- Notes: Version 3: - new patch src/libvirt_private.syms | 1 + src/security/security_dac.c | 9 +++++++++ src/security/security_driver.h | 3 +++ src/security/security_manager.c | 10 ++++++++++ src/security/security_manager.h | 2 ++ src/security/security_nop.c | 7 +++++++ src/security/security_selinux.c | 38 ++++++++++++++++++++++++++++++++++++++ src/security/security_stack.c | 17 +++++++++++++++++ 8 files changed, 87 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c3e33b4..2a453bc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -911,6 +911,7 @@ virSecurityManagerSetHostdevLabel; virSecurityManagerSetProcessFDLabel; virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; +virSecurityManagerSetSocketLabel; virSecurityManagerVerify; # sexpr.h diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 6df4087..e5465fc 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -675,6 +675,14 @@ virSecurityDACSetDaemonSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, static int +virSecurityDACSetSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + return 0; +} + + +static int virSecurityDACClearSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm ATTRIBUTE_UNUSED) { @@ -715,6 +723,7 @@ virSecurityDriver virSecurityDriverDAC = { virSecurityDACRestoreSecurityImageLabel, virSecurityDACSetDaemonSocketLabel, + virSecurityDACSetSocketLabel, virSecurityDACClearSocketLabel, virSecurityDACGenLabel, diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 73c8f04..94f27f8 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -43,6 +43,8 @@ typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr, virDomainDiskDefPtr disk); typedef int (*virSecurityDomainSetDaemonSocketLabel)(virSecurityManagerPtr mgr, virDomainObjPtr vm); +typedef int (*virSecurityDomainSetSocketLabel) (virSecurityManagerPtr mgr, + virDomainObjPtr vm); typedef int (*virSecurityDomainClearSocketLabel)(virSecurityManagerPtr mgr, virDomainObjPtr vm); typedef int (*virSecurityDomainSetImageLabel) (virSecurityManagerPtr mgr, @@ -102,6 +104,7 @@ struct _virSecurityDriver { virSecurityDomainRestoreImageLabel domainRestoreSecurityImageLabel; virSecurityDomainSetDaemonSocketLabel domainSetSecurityDaemonSocketLabel; + virSecurityDomainSetSocketLabel domainSetSecuritySocketLabel; virSecurityDomainClearSocketLabel domainClearSecuritySocketLabel; virSecurityDomainGenLabel domainGenSecurityLabel; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index d30ebcf..b2fd0d0 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -170,6 +170,16 @@ int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, return -1; } +int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + if (mgr->drv->domainSetSecuritySocketLabel) + return mgr->drv->domainSetSecuritySocketLabel(mgr, vm); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + int virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm) { diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 8d614a7..38342c2 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -55,6 +55,8 @@ int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, virDomainDiskDefPtr disk); int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm); +int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm); int virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm); int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr, diff --git a/src/security/security_nop.c b/src/security/security_nop.c index 67d3ff6..a68a6c0 100644 --- a/src/security/security_nop.c +++ b/src/security/security_nop.c @@ -59,6 +59,12 @@ static int virSecurityDomainSetDaemonSocketLabelNop(virSecurityManagerPtr mgr AT return 0; } +static int virSecurityDomainSetSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + return 0; +} + static int virSecurityDomainClearSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm ATTRIBUTE_UNUSED) { @@ -172,6 +178,7 @@ virSecurityDriver virSecurityDriverNop = { virSecurityDomainRestoreImageLabelNop, virSecurityDomainSetDaemonSocketLabelNop, + virSecurityDomainSetSocketLabelNop, virSecurityDomainClearSocketLabelNop, virSecurityDomainGenLabelNop, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index f87c9a5..cddbed5 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1137,6 +1137,43 @@ done: } static int +SELinuxSetSecuritySocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + const virSecurityLabelDefPtr secdef = &vm->def->seclabel; + int rc = -1; + + if (secdef->label == NULL) + return 0; + + if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("security label driver mismatch: " + "'%s' model configured for domain, but " + "hypervisor driver is '%s'."), + secdef->model, virSecurityManagerGetModel(mgr)); + goto done; + } + + VIR_DEBUG("Setting VM %s socket context %s", + vm->def->name, secdef->label); + if (setsockcreatecon(secdef->label) == -1) { + virReportSystemError(errno, + _("unable to set socket security context '%s'"), + secdef->label); + goto done; + } + + rc = 0; + +done: + if (security_getenforce() != 1) + rc = 0; + + return rc; +} + +static int SELinuxClearSecuritySocketLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm) { @@ -1313,6 +1350,7 @@ virSecurityDriver virSecurityDriverSELinux = { SELinuxRestoreSecurityImageLabel, SELinuxSetSecurityDaemonSocketLabel, + SELinuxSetSecuritySocketLabel, SELinuxClearSecuritySocketLabel, SELinuxGenSecurityLabel, diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 404ff65..f263f5b 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -355,6 +355,22 @@ virSecurityStackSetDaemonSocketLabel(virSecurityManagerPtr mgr, static int +virSecurityStackSetSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerSetSocketLabel(priv->secondary, vm) < 0) + rc = -1; + if (virSecurityManagerSetSocketLabel(priv->primary, vm) < 0) + rc = -1; + + return rc; +} + + +static int virSecurityStackClearSocketLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm) { @@ -419,6 +435,7 @@ virSecurityDriver virSecurityDriverStack = { virSecurityStackRestoreSecurityImageLabel, virSecurityStackSetDaemonSocketLabel, + virSecurityStackSetSocketLabel, virSecurityStackClearSocketLabel, virSecurityStackGenLabel, -- 1.7.6.1
On Fri, Aug 26, 2011 at 10:23:47AM +0200, Jiri Denemark wrote:
This API labels all sockets created until ClearSocketLabel is called in a way that a vm can access them (i.e., they are labeled with svirt_t based label in SELinux). --- Notes: Version 3: - new patch
src/libvirt_private.syms | 1 + src/security/security_dac.c | 9 +++++++++ src/security/security_driver.h | 3 +++ src/security/security_manager.c | 10 ++++++++++ src/security/security_manager.h | 2 ++ src/security/security_nop.c | 7 +++++++ src/security/security_selinux.c | 38 ++++++++++++++++++++++++++++++++++++++ src/security/security_stack.c | 17 +++++++++++++++++ 8 files changed, 87 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c3e33b4..2a453bc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -911,6 +911,7 @@ virSecurityManagerSetHostdevLabel; virSecurityManagerSetProcessFDLabel; virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; +virSecurityManagerSetSocketLabel; virSecurityManagerVerify;
# sexpr.h diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 6df4087..e5465fc 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -675,6 +675,14 @@ virSecurityDACSetDaemonSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
static int +virSecurityDACSetSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + return 0; +} + + +static int virSecurityDACClearSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm ATTRIBUTE_UNUSED) { @@ -715,6 +723,7 @@ virSecurityDriver virSecurityDriverDAC = { virSecurityDACRestoreSecurityImageLabel,
virSecurityDACSetDaemonSocketLabel, + virSecurityDACSetSocketLabel, virSecurityDACClearSocketLabel,
virSecurityDACGenLabel, diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 73c8f04..94f27f8 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -43,6 +43,8 @@ typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr, virDomainDiskDefPtr disk); typedef int (*virSecurityDomainSetDaemonSocketLabel)(virSecurityManagerPtr mgr, virDomainObjPtr vm); +typedef int (*virSecurityDomainSetSocketLabel) (virSecurityManagerPtr mgr, + virDomainObjPtr vm); typedef int (*virSecurityDomainClearSocketLabel)(virSecurityManagerPtr mgr, virDomainObjPtr vm); typedef int (*virSecurityDomainSetImageLabel) (virSecurityManagerPtr mgr, @@ -102,6 +104,7 @@ struct _virSecurityDriver { virSecurityDomainRestoreImageLabel domainRestoreSecurityImageLabel;
virSecurityDomainSetDaemonSocketLabel domainSetSecurityDaemonSocketLabel; + virSecurityDomainSetSocketLabel domainSetSecuritySocketLabel; virSecurityDomainClearSocketLabel domainClearSecuritySocketLabel;
virSecurityDomainGenLabel domainGenSecurityLabel; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index d30ebcf..b2fd0d0 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -170,6 +170,16 @@ int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, return -1; }
+int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + if (mgr->drv->domainSetSecuritySocketLabel) + return mgr->drv->domainSetSecuritySocketLabel(mgr, vm); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + int virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm) { diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 8d614a7..38342c2 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -55,6 +55,8 @@ int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, virDomainDiskDefPtr disk); int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm); +int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm); int virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm); int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr, diff --git a/src/security/security_nop.c b/src/security/security_nop.c index 67d3ff6..a68a6c0 100644 --- a/src/security/security_nop.c +++ b/src/security/security_nop.c @@ -59,6 +59,12 @@ static int virSecurityDomainSetDaemonSocketLabelNop(virSecurityManagerPtr mgr AT return 0; }
+static int virSecurityDomainSetSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + return 0; +} + static int virSecurityDomainClearSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm ATTRIBUTE_UNUSED) { @@ -172,6 +178,7 @@ virSecurityDriver virSecurityDriverNop = { virSecurityDomainRestoreImageLabelNop,
virSecurityDomainSetDaemonSocketLabelNop, + virSecurityDomainSetSocketLabelNop, virSecurityDomainClearSocketLabelNop,
virSecurityDomainGenLabelNop, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index f87c9a5..cddbed5 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1137,6 +1137,43 @@ done: }
static int +SELinuxSetSecuritySocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + const virSecurityLabelDefPtr secdef = &vm->def->seclabel; + int rc = -1; + + if (secdef->label == NULL) + return 0; + + if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("security label driver mismatch: " + "'%s' model configured for domain, but " + "hypervisor driver is '%s'."), + secdef->model, virSecurityManagerGetModel(mgr)); + goto done; + } + + VIR_DEBUG("Setting VM %s socket context %s", + vm->def->name, secdef->label); + if (setsockcreatecon(secdef->label) == -1) { + virReportSystemError(errno, + _("unable to set socket security context '%s'"), + secdef->label); + goto done; + } + + rc = 0; + +done: + if (security_getenforce() != 1) + rc = 0; + + return rc; +} + +static int SELinuxClearSecuritySocketLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm) { @@ -1313,6 +1350,7 @@ virSecurityDriver virSecurityDriverSELinux = { SELinuxRestoreSecurityImageLabel,
SELinuxSetSecurityDaemonSocketLabel, + SELinuxSetSecuritySocketLabel, SELinuxClearSecuritySocketLabel,
SELinuxGenSecurityLabel, diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 404ff65..f263f5b 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -355,6 +355,22 @@ virSecurityStackSetDaemonSocketLabel(virSecurityManagerPtr mgr,
static int +virSecurityStackSetSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerSetSocketLabel(priv->secondary, vm) < 0) + rc = -1; + if (virSecurityManagerSetSocketLabel(priv->primary, vm) < 0) + rc = -1; + + return rc; +} + + +static int virSecurityStackClearSocketLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm) { @@ -419,6 +435,7 @@ virSecurityDriver virSecurityDriverStack = { virSecurityStackRestoreSecurityImageLabel,
virSecurityStackSetDaemonSocketLabel, + virSecurityStackSetSocketLabel, virSecurityStackClearSocketLabel,
virSecurityStackGenLabel,
ACK, looks fine. My only concern would be about availability of setsockcreatecon() , hopefully it's supported on all systems where SELinux is detected, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Fri, Aug 26, 2011 at 10:23:47AM +0200, Jiri Denemark wrote:
This API labels all sockets created until ClearSocketLabel is called in a way that a vm can access them (i.e., they are labeled with svirt_t based label in SELinux). --- Notes: Version 3: - new patch
src/libvirt_private.syms | 1 + src/security/security_dac.c | 9 +++++++++ src/security/security_driver.h | 3 +++ src/security/security_manager.c | 10 ++++++++++ src/security/security_manager.h | 2 ++ src/security/security_nop.c | 7 +++++++ src/security/security_selinux.c | 38 ++++++++++++++++++++++++++++++++++++++ src/security/security_stack.c | 17 +++++++++++++++++ 8 files changed, 87 insertions(+), 0 deletions(-)
Again need a stub for apparmour ACK if that is added. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--- Notes: Version 3: - use virSecurityManagerSetSocketLabel/virSecurityManagerClearSocketLabel pair around virNetSocketNewConnectTCP to label the newly created socket with svirt_t Version 2: - use virSecurityManagerSetProcessFDLabel instead of virSecurityManagerSetImageFDLabel since the correct label for TCP sockets appears to be svirt_t and not svirt_image_t src/qemu/qemu_migration.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a38c0d9..3818d71 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1624,11 +1624,14 @@ static int doNativeMigrate(struct qemud_driver *driver, virReportOOMError(); goto cleanup; } + if (virSecurityManagerSetSocketLabel(driver->securityManager, vm) < 0) + goto cleanup; if (virNetSocketNewConnectTCP(uribits->server, tmp, &sock) == 0) { spec.dest.fd.qemu = virNetSocketDupFD(sock, true); virNetSocketFree(sock); } - if (spec.dest.fd.qemu == -1) + if (virSecurityManagerClearSocketLabel(driver->securityManager, vm) < 0 || + spec.dest.fd.qemu == -1) goto cleanup; } else { spec.destType = MIGRATION_DEST_HOST; -- 1.7.6.1
On Fri, Aug 26, 2011 at 10:23:48AM +0200, Jiri Denemark wrote:
--- Notes: Version 3: - use virSecurityManagerSetSocketLabel/virSecurityManagerClearSocketLabel pair around virNetSocketNewConnectTCP to label the newly created socket with svirt_t
Version 2: - use virSecurityManagerSetProcessFDLabel instead of virSecurityManagerSetImageFDLabel since the correct label for TCP sockets appears to be svirt_t and not svirt_image_t
src/qemu/qemu_migration.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a38c0d9..3818d71 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1624,11 +1624,14 @@ static int doNativeMigrate(struct qemud_driver *driver, virReportOOMError(); goto cleanup; } + if (virSecurityManagerSetSocketLabel(driver->securityManager, vm) < 0) + goto cleanup; if (virNetSocketNewConnectTCP(uribits->server, tmp, &sock) == 0) { spec.dest.fd.qemu = virNetSocketDupFD(sock, true); virNetSocketFree(sock); } - if (spec.dest.fd.qemu == -1) + if (virSecurityManagerClearSocketLabel(driver->securityManager, vm) < 0 || + spec.dest.fd.qemu == -1) goto cleanup; } else { spec.destType = MIGRATION_DEST_HOST;
ACK now, I feel more confident it it :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Fri, Aug 26, 2011 at 10:23:48AM +0200, Jiri Denemark wrote:
--- Notes: Version 3: - use virSecurityManagerSetSocketLabel/virSecurityManagerClearSocketLabel pair around virNetSocketNewConnectTCP to label the newly created socket with svirt_t
Version 2: - use virSecurityManagerSetProcessFDLabel instead of virSecurityManagerSetImageFDLabel since the correct label for TCP sockets appears to be svirt_t and not svirt_image_t
src/qemu/qemu_migration.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a38c0d9..3818d71 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1624,11 +1624,14 @@ static int doNativeMigrate(struct qemud_driver *driver, virReportOOMError(); goto cleanup; } + if (virSecurityManagerSetSocketLabel(driver->securityManager, vm) < 0) + goto cleanup; if (virNetSocketNewConnectTCP(uribits->server, tmp, &sock) == 0) { spec.dest.fd.qemu = virNetSocketDupFD(sock, true); virNetSocketFree(sock); } - if (spec.dest.fd.qemu == -1) + if (virSecurityManagerClearSocketLabel(driver->securityManager, vm) < 0 || + spec.dest.fd.qemu == -1) goto cleanup; } else { spec.destType = MIGRATION_DEST_HOST;
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Fri, Aug 26, 2011 at 10:23:45 +0200, Jiri Denemark wrote:
With current libvirt and qemu, migration is not working if SELinux is in enforcing mode, since the TCP socket we pass to qemu is not labeled in a way that would allow qemu to read from it.
After this patchset, migration works even in enforcing mode.
Jiri Denemark (3): security: Rename SetSocketLabel APIs to SetDaemonSocketLabel security: Introduce SetSocketLabel qemu: Correctly label migration TCP socket
Oops, thanks for spotting the missing part in apparmor driver. I fixed that (and installed libapparmor and compile-tested libvirt with it) and pushed the series. Jirka
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Jiri Denemark