[libvirt] [PATCH v3 0/2] expose baselabel for each sec model/virt type

Now each security model can define its own base label, that describes the default security context used by libvirt to run an hypervisor process. This information is exposed to users trough the host capabilities XML. *v3 major changes - support LXC - merge virSecurityDACSetUser and virSecurityDACSetGroup in virSecurityDACSetUserAndGroup - DAC sets the baselabel in virSecurityDACSetUserAndGroup - Use virDomainVirtTypeToString instead of hardcoding the name Giuseppe Scrivano (2): security: add new internal function "virSecurityManagerGetBaseLabel" capabilities: add baselabel per sec driver/virt type to secmodel docs/schemas/capability.rng | 8 ++++ src/conf/capabilities.c | 60 +++++++++++++++++++++++++++- src/conf/capabilities.h | 14 +++++++ src/libvirt_private.syms | 2 + src/lxc/lxc_conf.c | 10 ++++- src/qemu/qemu_conf.c | 21 ++++++++-- src/security/security_apparmor.c | 8 ++++ src/security/security_dac.c | 34 +++++++++++----- src/security/security_dac.h | 7 ++-- src/security/security_driver.h | 4 ++ src/security/security_manager.c | 22 +++++++++- src/security/security_manager.h | 2 + src/security/security_nop.c | 10 +++++ src/security/security_selinux.c | 12 ++++++ src/security/security_stack.c | 9 +++++ tests/capabilityschemadata/caps-qemu-kvm.xml | 2 + tests/capabilityschemadata/caps-test3.xml | 2 + 17 files changed, 204 insertions(+), 23 deletions(-) -- 1.8.3.1

virSecurityManagerGetBaseLabel queries the default settings used by a security model. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- src/libvirt_private.syms | 1 + src/security/security_apparmor.c | 8 ++++++++ src/security/security_dac.c | 34 ++++++++++++++++++++++++---------- src/security/security_dac.h | 7 +++---- src/security/security_driver.h | 4 ++++ src/security/security_manager.c | 22 ++++++++++++++++++++-- src/security/security_manager.h | 2 ++ src/security/security_nop.c | 10 ++++++++++ src/security/security_selinux.c | 12 ++++++++++++ src/security/security_stack.c | 9 +++++++++ 10 files changed, 93 insertions(+), 16 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 35f0f1b..aea7e94 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1033,6 +1033,7 @@ virSecurityDriverLookup; # security/security_manager.h virSecurityManagerClearSocketLabel; virSecurityManagerGenLabel; +virSecurityManagerGetBaseLabel; virSecurityManagerGetDOI; virSecurityManagerGetModel; virSecurityManagerGetMountOptions; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index adc9918..2d74cdd 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -931,6 +931,12 @@ AppArmorGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return opts; } +static const char * +AppArmorGetBaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + int virtType ATTRIBUTE_UNUSED) +{ + return NULL; +} virSecurityDriver virAppArmorSecurityDriver = { .privateDataLen = 0, @@ -972,4 +978,6 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSecurityTapFDLabel = AppArmorSetFDLabel, .domainGetSecurityMountOptions = AppArmorGetMountOptions, + + .getBaseLabel = AppArmoryGetBaseLabel, }; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 6876bd5..019c789 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -47,22 +47,25 @@ struct _virSecurityDACData { gid_t *groups; int ngroups; bool dynamicOwnership; + char *baselabel; }; -void -virSecurityDACSetUser(virSecurityManagerPtr mgr, - uid_t user) +/* returns -1 on error, 0 on success */ +int +virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr, + uid_t user, + gid_t group) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); priv->user = user; -} - -void -virSecurityDACSetGroup(virSecurityManagerPtr mgr, - gid_t group) -{ - virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); priv->group = group; + + if (virAsprintf(&priv->baselabel, "%u:%u", + (unsigned int) user, + (unsigned int) group) < 0) + return -1; + + return 0; } void @@ -217,6 +220,7 @@ virSecurityDACClose(virSecurityManagerPtr mgr) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); VIR_FREE(priv->groups); + VIR_FREE(priv->baselabel); return 0; } @@ -1170,6 +1174,14 @@ virSecurityDACGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return NULL; } +static const char * +virSecurityDACGetBaseLabel(virSecurityManagerPtr mgr, + int virt ATTRIBUTE_UNUSED) +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + return priv->baselabel; +} + virSecurityDriver virSecurityDriverDAC = { .privateDataLen = sizeof(virSecurityDACData), .name = SECURITY_DAC_NAME, @@ -1212,4 +1224,6 @@ virSecurityDriver virSecurityDriverDAC = { .domainSetSecurityTapFDLabel = virSecurityDACSetTapFDLabel, .domainGetSecurityMountOptions = virSecurityDACGetMountOptions, + + .getBaseLabel = virSecurityDACGetBaseLabel, }; diff --git a/src/security/security_dac.h b/src/security/security_dac.h index 02432a5..dbcf56f 100644 --- a/src/security/security_dac.h +++ b/src/security/security_dac.h @@ -25,10 +25,9 @@ extern virSecurityDriver virSecurityDriverDAC; -void virSecurityDACSetUser(virSecurityManagerPtr mgr, - uid_t user); -void virSecurityDACSetGroup(virSecurityManagerPtr mgr, - gid_t group); +int virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr, + uid_t user, + gid_t group); void virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr, bool dynamic); diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 8735558..ced1b92 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -46,6 +46,8 @@ typedef int (*virSecurityDriverClose) (virSecurityManagerPtr mgr); typedef const char *(*virSecurityDriverGetModel) (virSecurityManagerPtr mgr); typedef const char *(*virSecurityDriverGetDOI) (virSecurityManagerPtr mgr); +typedef const char *(*virSecurityDriverGetBaseLabel) (virSecurityManagerPtr mgr, + int virtType); typedef int (*virSecurityDriverPreFork) (virSecurityManagerPtr mgr); @@ -154,6 +156,8 @@ struct _virSecurityDriver { virSecurityDomainGetMountOptions domainGetSecurityMountOptions; virSecurityDomainSetHugepages domainSetSecurityHugepages; + + virSecurityDriverGetBaseLabel getBaseLabel; }; virSecurityDriverPtr virSecurityDriverLookup(const char *name, diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 92fb504..c4b8f10 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -146,8 +146,10 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, if (!mgr) return NULL; - virSecurityDACSetUser(mgr, user); - virSecurityDACSetGroup(mgr, group); + if (virSecurityDACSetUserAndGroup(mgr, user, group) < 0) { + virSecurityManagerDispose(mgr); + return NULL; + } virSecurityDACSetDynamicOwnership(mgr, dynamicOwnership); return mgr; @@ -273,6 +275,22 @@ virSecurityManagerGetModel(virSecurityManagerPtr mgr) return NULL; } +/* return NULL if a base label is not present */ +const char * +virSecurityManagerGetBaseLabel(virSecurityManagerPtr mgr, int virtType) +{ + if (mgr->drv->getBaseLabel) { + const char *ret; + virObjectLock(mgr); + ret = mgr->drv->getBaseLabel(mgr, virtType); + virObjectUnlock(mgr); + return ret; + } + + virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return NULL; +} + bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr) { return mgr->allowDiskFormatProbing; diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 9252830..81d3160 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -55,6 +55,8 @@ void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr); const char *virSecurityManagerGetDriver(virSecurityManagerPtr mgr); const char *virSecurityManagerGetDOI(virSecurityManagerPtr mgr); const char *virSecurityManagerGetModel(virSecurityManagerPtr mgr); +const char *virSecurityManagerGetBaseLabel(virSecurityManagerPtr mgr, int virtType); + bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr); bool virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr); bool virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr); diff --git a/src/security/security_nop.c b/src/security/security_nop.c index 233404c..73e1ac1 100644 --- a/src/security/security_nop.c +++ b/src/security/security_nop.c @@ -186,6 +186,14 @@ static char *virSecurityDomainGetMountOptionsNop(virSecurityManagerPtr mgr ATTRI return opts; } +static const char * +virSecurityGetBaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + int virtType ATTRIBUTE_UNUSED) +{ + return NULL; +} + + virSecurityDriver virSecurityDriverNop = { .privateDataLen = 0, .name = "none", @@ -226,4 +234,6 @@ virSecurityDriver virSecurityDriverNop = { .domainSetSecurityTapFDLabel = virSecurityDomainSetFDLabelNop, .domainGetSecurityMountOptions = virSecurityDomainGetMountOptionsNop, + + .getBaseLabel = virSecurityGetBaseLabel, }; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 38de060..1c2ea64 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1827,6 +1827,17 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def, } +static const char * +virSecuritySELinuxGetBaseLabel(virSecurityManagerPtr mgr, int virtType) +{ + virSecuritySELinuxDataPtr priv = virSecurityManagerGetPrivateData(mgr); + if (virtType == VIR_DOMAIN_VIRT_QEMU) + return priv->alt_domain_context; + else + return priv->domain_context; +} + + static int virSecuritySELinuxRestoreSecurityAllLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, @@ -2474,4 +2485,5 @@ virSecurityDriver virSecurityDriverSELinux = { .domainSetSecurityTapFDLabel = virSecuritySELinuxSetTapFDLabel, .domainGetSecurityMountOptions = virSecuritySELinuxGetSecurityMountOptions, + .getBaseLabel = virSecuritySELinuxGetBaseLabel, }; diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 0a0dc92..ff0f06b 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -555,6 +555,13 @@ virSecurityStackGetNested(virSecurityManagerPtr mgr) return list; } +static const char * +virSecurityStackGetBaseLabel(virSecurityManagerPtr mgr, int virtType) +{ + return virSecurityManagerGetBaseLabel(virSecurityStackGetPrimary(mgr), + virtType); +} + virSecurityDriver virSecurityDriverStack = { .privateDataLen = sizeof(virSecurityStackData), .name = "stack", @@ -599,4 +606,6 @@ virSecurityDriver virSecurityDriverStack = { .domainGetSecurityMountOptions = virSecurityStackGetMountOptions, .domainSetSecurityHugepages = virSecurityStackSetHugepages, + + .getBaseLabel = virSecurityStackGetBaseLabel, }; -- 1.8.3.1

On Fri, Sep 06, 2013 at 06:29:55PM +0200, Giuseppe Scrivano wrote:
virSecurityManagerGetBaseLabel queries the default settings used by a security model.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- src/libvirt_private.syms | 1 + src/security/security_apparmor.c | 8 ++++++++ src/security/security_dac.c | 34 ++++++++++++++++++++++++---------- src/security/security_dac.h | 7 +++---- src/security/security_driver.h | 4 ++++ src/security/security_manager.c | 22 ++++++++++++++++++++-- src/security/security_manager.h | 2 ++ src/security/security_nop.c | 10 ++++++++++ src/security/security_selinux.c | 12 ++++++++++++ src/security/security_stack.c | 9 +++++++++ 10 files changed, 93 insertions(+), 16 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 35f0f1b..aea7e94 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1033,6 +1033,7 @@ virSecurityDriverLookup; # security/security_manager.h virSecurityManagerClearSocketLabel; virSecurityManagerGenLabel; +virSecurityManagerGetBaseLabel; virSecurityManagerGetDOI; virSecurityManagerGetModel; virSecurityManagerGetMountOptions; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index adc9918..2d74cdd 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -931,6 +931,12 @@ AppArmorGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return opts; }
+static const char * +AppArmorGetBaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + int virtType ATTRIBUTE_UNUSED) +{ + return NULL; +}
virSecurityDriver virAppArmorSecurityDriver = { .privateDataLen = 0, @@ -972,4 +978,6 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSecurityTapFDLabel = AppArmorSetFDLabel,
.domainGetSecurityMountOptions = AppArmorGetMountOptions, + + .getBaseLabel = AppArmoryGetBaseLabel, }; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 6876bd5..019c789 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -47,22 +47,25 @@ struct _virSecurityDACData { gid_t *groups; int ngroups; bool dynamicOwnership; + char *baselabel; };
-void -virSecurityDACSetUser(virSecurityManagerPtr mgr, - uid_t user) +/* returns -1 on error, 0 on success */ +int +virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr, + uid_t user, + gid_t group) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); priv->user = user; -} - -void -virSecurityDACSetGroup(virSecurityManagerPtr mgr, - gid_t group) -{ - virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); priv->group = group; + + if (virAsprintf(&priv->baselabel, "%u:%u", + (unsigned int) user, + (unsigned int) group) < 0) + return -1; + + return 0; }
void @@ -217,6 +220,7 @@ virSecurityDACClose(virSecurityManagerPtr mgr) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); VIR_FREE(priv->groups); + VIR_FREE(priv->baselabel); return 0; }
@@ -1170,6 +1174,14 @@ virSecurityDACGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return NULL; }
+static const char * +virSecurityDACGetBaseLabel(virSecurityManagerPtr mgr, + int virt ATTRIBUTE_UNUSED) +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + return priv->baselabel; +} + virSecurityDriver virSecurityDriverDAC = { .privateDataLen = sizeof(virSecurityDACData), .name = SECURITY_DAC_NAME, @@ -1212,4 +1224,6 @@ virSecurityDriver virSecurityDriverDAC = { .domainSetSecurityTapFDLabel = virSecurityDACSetTapFDLabel,
.domainGetSecurityMountOptions = virSecurityDACGetMountOptions, + + .getBaseLabel = virSecurityDACGetBaseLabel, }; diff --git a/src/security/security_dac.h b/src/security/security_dac.h index 02432a5..dbcf56f 100644 --- a/src/security/security_dac.h +++ b/src/security/security_dac.h @@ -25,10 +25,9 @@
extern virSecurityDriver virSecurityDriverDAC;
-void virSecurityDACSetUser(virSecurityManagerPtr mgr, - uid_t user); -void virSecurityDACSetGroup(virSecurityManagerPtr mgr, - gid_t group); +int virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr, + uid_t user, + gid_t group);
It would be desirable to have this re-factoring done in a separate, prior, patch from that which adds the GetBaseLabel hook.
diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 92fb504..c4b8f10 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -146,8 +146,10 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, if (!mgr) return NULL;
- virSecurityDACSetUser(mgr, user); - virSecurityDACSetGroup(mgr, group); + if (virSecurityDACSetUserAndGroup(mgr, user, group) < 0) { + virSecurityManagerDispose(mgr); + return NULL; + }
Likewise this block
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 38de060..1c2ea64 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1827,6 +1827,17 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def, }
+static const char * +virSecuritySELinuxGetBaseLabel(virSecurityManagerPtr mgr, int virtType) +{ + virSecuritySELinuxDataPtr priv = virSecurityManagerGetPrivateData(mgr); + if (virtType == VIR_DOMAIN_VIRT_QEMU) + return priv->alt_domain_context;
alt_domain_context is not guaranteed to be present, so you need to have if (virtType == VIR_DOMAIN_VIRT_QEMU && priv->alt_domain_context) .... 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, Sep 06, 2013 at 06:29:55PM +0200, Giuseppe Scrivano wrote:
virSecurityManagerGetBaseLabel queries the default settings used by a security model.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- src/libvirt_private.syms | 1 + src/security/security_apparmor.c | 8 ++++++++ src/security/security_dac.c | 34 ++++++++++++++++++++++++---------- src/security/security_dac.h | 7 +++---- src/security/security_driver.h | 4 ++++ src/security/security_manager.c | 22 ++++++++++++++++++++-- src/security/security_manager.h | 2 ++ src/security/security_nop.c | 10 ++++++++++ src/security/security_selinux.c | 12 ++++++++++++ src/security/security_stack.c | 9 +++++++++ 10 files changed, 93 insertions(+), 16 deletions(-) diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 92fb504..c4b8f10 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -273,6 +275,22 @@ virSecurityManagerGetModel(virSecurityManagerPtr mgr) return NULL; }
+/* return NULL if a base label is not present */ +const char * +virSecurityManagerGetBaseLabel(virSecurityManagerPtr mgr, int virtType) +{ + if (mgr->drv->getBaseLabel) { + const char *ret; + virObjectLock(mgr); + ret = mgr->drv->getBaseLabel(mgr, virtType); + virObjectUnlock(mgr); + return ret; + } + + virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return NULL;
Per my reply to the 2nd patch, we need to remove thie virReportError method call. It is valid to not implement this method if no baselabel is required. 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 :|

Expand the "secmodel" XML fragment of "host" with a sequence of baselabel's which describe the default security context used by libvirt with a specific security model and virtualization type: <secmodel> <model>selinux</model> <doi>0</doi> <baselabel type='kvm'>system_u:system_r:svirt_t:s0</baselabel> <baselabel type='qemu'>system_u:system_r:svirt_t:s0</baselabel> </secmodel> <secmodel> <model>dac</model> <doi>0</doi> <baselabel type='kvm'>0:0</baselabel> <baselabel type='qemu'>0:0</baselabel> </secmodel> "baselabel" is driver-specific information, e.g. in the DAC security model, it indicates USER_ID:GROUP_ID. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- docs/schemas/capability.rng | 8 ++++ src/conf/capabilities.c | 60 +++++++++++++++++++++++++++- src/conf/capabilities.h | 14 +++++++ src/libvirt_private.syms | 1 + src/lxc/lxc_conf.c | 10 ++++- src/qemu/qemu_conf.c | 21 ++++++++-- tests/capabilityschemadata/caps-qemu-kvm.xml | 2 + tests/capabilityschemadata/caps-test3.xml | 2 + 8 files changed, 111 insertions(+), 7 deletions(-) diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 65c7c72..aee03d7 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -60,6 +60,14 @@ <element name='doi'> <text/> </element> + <zeroOrMore> + <element name='baselabel'> + <attribute name='type'> + <text/> + </attribute> + <text/> + </element> + </zeroOrMore> </interleave> </element> </define> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 1acc936..b0e2ff9 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -184,6 +184,20 @@ virCapabilitiesFreeNUMAInfo(virCapsPtr caps) } static void +virCapabilitiesFreeSecModel(virCapsHostSecModelPtr secmodel) +{ + size_t i; + for (i = 0; i < secmodel->nlabels; i++) { + VIR_FREE(secmodel->labels[i].type); + VIR_FREE(secmodel->labels[i].label); + } + + VIR_FREE(secmodel->labels); + VIR_FREE(secmodel->model); + VIR_FREE(secmodel->doi); +} + +static void virCapabilitiesDispose(void *object) { virCapsPtr caps = object; @@ -204,8 +218,7 @@ virCapabilitiesDispose(void *object) VIR_FREE(caps->host.migrateTrans); for (i = 0; i < caps->host.nsecModels; i++) { - VIR_FREE(caps->host.secModels[i].model); - VIR_FREE(caps->host.secModels[i].doi); + virCapabilitiesFreeSecModel(&caps->host.secModels[i]); } VIR_FREE(caps->host.secModels); @@ -507,6 +520,44 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest, } /** + * virCapabilitiesHostSecModelAddBaseLabel + * @secmodel: Security model to add a base label for + * @type: virtualization type + * @label: base label + * + * Returns non-zero on error. + */ +extern int +virCapabilitiesHostSecModelAddBaseLabel(virCapsHostSecModelPtr secmodel, + const char *type, + const char *label) +{ + char *t = NULL, *l = NULL; + + if (type == NULL || label == NULL) + return -1; + + if (VIR_STRDUP(t, type) < 0) + goto no_memory; + + if (VIR_STRDUP(l, label) < 0) + goto no_memory; + + if (VIR_EXPAND_N(secmodel->labels, secmodel->nlabels, 1) < 0) + goto no_memory; + + secmodel->labels[secmodel->nlabels - 1].type = t; + secmodel->labels[secmodel->nlabels - 1].label = l; + + return 0; + +no_memory: + VIR_FREE(l); + VIR_FREE(t); + return -1; +} + +/** * virCapabilitiesSupportsGuestArch: * @caps: capabilities to query * @arch: Architecture to search for @@ -826,6 +877,11 @@ virCapabilitiesFormatXML(virCapsPtr caps) caps->host.secModels[i].model); virBufferAsprintf(&xml, " <doi>%s</doi>\n", caps->host.secModels[i].doi); + for (j = 0; j < caps->host.secModels[i].nlabels; j++) { + virBufferAsprintf(&xml, " <baselabel type='%s'>%s</baselabel>\n", + caps->host.secModels[i].labels[j].type, + caps->host.secModels[i].labels[j].label); + } virBufferAddLit(&xml, " </secmodel>\n"); } diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 88ec454..5bc7bb5 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -104,11 +104,20 @@ struct _virCapsHostNUMACell { virCapsHostNUMACellCPUPtr cpus; }; +typedef struct _virCapsHostSecModelLabel virCapsHostSecModelLabel; +typedef virCapsHostSecModelLabel *virCapsHostSecModelLabelPtr; +struct _virCapsHostSecModelLabel { + char *type; + char *label; +}; + typedef struct _virCapsHostSecModel virCapsHostSecModel; typedef virCapsHostSecModel *virCapsHostSecModelPtr; struct _virCapsHostSecModel { char *model; char *doi; + size_t nlabels; + virCapsHostSecModelLabelPtr labels; }; typedef struct _virCapsHost virCapsHost; @@ -225,6 +234,11 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest, int toggle); extern int +virCapabilitiesHostSecModelAddBaseLabel(virCapsHostSecModelPtr secmodel, + const char *type, + const char *label); + +extern int virCapabilitiesSupportsGuestArch(virCapsPtr caps, virArch arch); extern int diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index aea7e94..cdf9e58 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -58,6 +58,7 @@ virCapabilitiesFormatXML; virCapabilitiesFreeMachines; virCapabilitiesFreeNUMAInfo; virCapabilitiesGetCpusForNodemask; +virCapabilitiesHostSecModelAddBaseLabel; virCapabilitiesNew; virCapabilitiesSetHostCPU; diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index c1cee3f..557191a 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -126,10 +126,13 @@ virCapsPtr virLXCDriverCapsInit(virLXCDriverPtr driver) if (driver) { /* Security driver data */ - const char *doi, *model; + const char *doi, *model, *label, *type; doi = virSecurityManagerGetDOI(driver->securityManager); model = virSecurityManagerGetModel(driver->securityManager); + label = virSecurityManagerGetBaseLabel(driver->securityManager, + VIR_DOMAIN_VIRT_LXC); + type = virDomainVirtTypeToString(VIR_DOMAIN_VIRT_LXC); /* Allocate the primary security driver for LXC. */ if (VIR_ALLOC(caps->host.secModels) < 0) goto error; @@ -138,6 +141,11 @@ virCapsPtr virLXCDriverCapsInit(virLXCDriverPtr driver) goto error; if (VIR_STRDUP(caps->host.secModels[0].doi, doi) < 0) goto error; + if (label && + virCapabilitiesHostSecModelAddBaseLabel(&caps->host.secModels[0], + type, + label) < 0) + goto error; VIR_DEBUG("Initialized caps for security driver \"%s\" with " "DOI \"%s\"", model, doi); diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 1f57f72..8daafc7 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -559,12 +559,15 @@ virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver) virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) { - size_t i; + size_t i, j; virCapsPtr caps; virSecurityManagerPtr *sec_managers = NULL; /* Security driver data */ - const char *doi, *model; + const char *doi, *model, *lbl, *type; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + const int virtTypes[] = {VIR_DOMAIN_VIRT_KVM, + VIR_DOMAIN_VIRT_QEMU, + VIR_DOMAIN_VIRT_LAST}; /* Basic host arch / guest machine capabilities */ if (!(caps = virQEMUCapsInit(driver->qemuCapsCache))) @@ -589,11 +592,21 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) goto error; for (i = 0; sec_managers[i]; i++) { + virCapsHostSecModelPtr sm = &caps->host.secModels[i]; doi = virSecurityManagerGetDOI(sec_managers[i]); model = virSecurityManagerGetModel(sec_managers[i]); - if (VIR_STRDUP(caps->host.secModels[i].model, model) < 0 || - VIR_STRDUP(caps->host.secModels[i].doi, doi) < 0) + if (VIR_STRDUP(sm->model, model) < 0 || + VIR_STRDUP(sm->doi, doi) < 0) goto error; + + for (j = 0; virtTypes[j] != VIR_DOMAIN_VIRT_LAST; j++) { + lbl = virSecurityManagerGetBaseLabel(sec_managers[i], virtTypes[j]); + type = virDomainVirtTypeToString(virtTypes[j]); + if (lbl && + virCapabilitiesHostSecModelAddBaseLabel(sm, type, lbl) < 0) + goto error; + } + VIR_DEBUG("Initialized caps for security driver \"%s\" with " "DOI \"%s\"", model, doi); } diff --git a/tests/capabilityschemadata/caps-qemu-kvm.xml b/tests/capabilityschemadata/caps-qemu-kvm.xml index 1fbc22b..066ec71 100644 --- a/tests/capabilityschemadata/caps-qemu-kvm.xml +++ b/tests/capabilityschemadata/caps-qemu-kvm.xml @@ -25,6 +25,8 @@ <secmodel> <model>selinux</model> <doi>0</doi> + <baselabel type='kvm'>system_u:system_r:svirt_t:s0</baselabel> + <baselabel type='qemu'>system_u:system_r:svirt_t:s0</baselabel> </secmodel> </host> diff --git a/tests/capabilityschemadata/caps-test3.xml b/tests/capabilityschemadata/caps-test3.xml index e6c56c5..d359f25 100644 --- a/tests/capabilityschemadata/caps-test3.xml +++ b/tests/capabilityschemadata/caps-test3.xml @@ -82,6 +82,8 @@ <secmodel> <model>dac</model> <doi>0</doi> + <baselabel type='kvm'>0:0</baselabel> + <baselabel type='qemu'>0:0</baselabel> </secmodel> </host> -- 1.8.3.1

On Fri, Sep 06, 2013 at 06:29:56PM +0200, Giuseppe Scrivano wrote:
Expand the "secmodel" XML fragment of "host" with a sequence of baselabel's which describe the default security context used by libvirt with a specific security model and virtualization type:
<secmodel> <model>selinux</model> <doi>0</doi> <baselabel type='kvm'>system_u:system_r:svirt_t:s0</baselabel> <baselabel type='qemu'>system_u:system_r:svirt_t:s0</baselabel>
s/svirt_t/svirt_tcg_t/ for the qemu example just to illustrate that it is sometimes diferent.
</secmodel> <secmodel> <model>dac</model> <doi>0</doi> <baselabel type='kvm'>0:0</baselabel> <baselabel type='qemu'>0:0</baselabel>
I'd suggest '107:107' for these examples since that's the usual ID for Fedora 'qemu' user.
</secmodel>
"baselabel" is driver-specific information, e.g. in the DAC security model, it indicates USER_ID:GROUP_ID.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- docs/schemas/capability.rng | 8 ++++ src/conf/capabilities.c | 60 +++++++++++++++++++++++++++- src/conf/capabilities.h | 14 +++++++ src/libvirt_private.syms | 1 + src/lxc/lxc_conf.c | 10 ++++- src/qemu/qemu_conf.c | 21 ++++++++-- tests/capabilityschemadata/caps-qemu-kvm.xml | 2 + tests/capabilityschemadata/caps-test3.xml | 2 + 8 files changed, 111 insertions(+), 7 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 1acc936..b0e2ff9 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -184,6 +184,20 @@ virCapabilitiesFreeNUMAInfo(virCapsPtr caps) }
static void +virCapabilitiesFreeSecModel(virCapsHostSecModelPtr secmodel) +{ + size_t i; + for (i = 0; i < secmodel->nlabels; i++) { + VIR_FREE(secmodel->labels[i].type); + VIR_FREE(secmodel->labels[i].label); + } + + VIR_FREE(secmodel->labels); + VIR_FREE(secmodel->model); + VIR_FREE(secmodel->doi); +}
For functions which don't actually free the passed-in pointer itself, we prefer to use 'Clear' instead of 'Free' in the name, to make it more obvious to people what the semantics are.
+ +static void virCapabilitiesDispose(void *object) { virCapsPtr caps = object; @@ -204,8 +218,7 @@ virCapabilitiesDispose(void *object) VIR_FREE(caps->host.migrateTrans);
for (i = 0; i < caps->host.nsecModels; i++) { - VIR_FREE(caps->host.secModels[i].model); - VIR_FREE(caps->host.secModels[i].doi); + virCapabilitiesFreeSecModel(&caps->host.secModels[i]); } VIR_FREE(caps->host.secModels);
@@ -507,6 +520,44 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest,
diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index c1cee3f..557191a 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -126,10 +126,13 @@ virCapsPtr virLXCDriverCapsInit(virLXCDriverPtr driver)
if (driver) { /* Security driver data */ - const char *doi, *model; + const char *doi, *model, *label, *type;
doi = virSecurityManagerGetDOI(driver->securityManager); model = virSecurityManagerGetModel(driver->securityManager); + label = virSecurityManagerGetBaseLabel(driver->securityManager, + VIR_DOMAIN_VIRT_LXC);
Hmm, the virSecurityManagerGetBaseLabel method can raise a VIR_ERR_NO_SUPPORT message if unsupported, which would be ignored here. It is none the less valid for this method to be not-implemented by a driver. Since I don't believe we have a need to report errors in this method, I think we should remove the call to virReportError in its impl.
+ type = virDomainVirtTypeToString(VIR_DOMAIN_VIRT_LXC); /* Allocate the primary security driver for LXC. */ if (VIR_ALLOC(caps->host.secModels) < 0) goto error; @@ -138,6 +141,11 @@ virCapsPtr virLXCDriverCapsInit(virLXCDriverPtr driver) goto error; if (VIR_STRDUP(caps->host.secModels[0].doi, doi) < 0) goto error; + if (label && + virCapabilitiesHostSecModelAddBaseLabel(&caps->host.secModels[0], + type, + label) < 0) + goto error;
VIR_DEBUG("Initialized caps for security driver \"%s\" with " "DOI \"%s\"", model, doi); diff --git a/tests/capabilityschemadata/caps-qemu-kvm.xml b/tests/capabilityschemadata/caps-qemu-kvm.xml index 1fbc22b..066ec71 100644 --- a/tests/capabilityschemadata/caps-qemu-kvm.xml +++ b/tests/capabilityschemadata/caps-qemu-kvm.xml @@ -25,6 +25,8 @@ <secmodel> <model>selinux</model> <doi>0</doi> + <baselabel type='kvm'>system_u:system_r:svirt_t:s0</baselabel> + <baselabel type='qemu'>system_u:system_r:svirt_t:s0</baselabel>
s/svirt_t/svirt_tcg_t/ in this example
</secmodel> </host>
diff --git a/tests/capabilityschemadata/caps-test3.xml b/tests/capabilityschemadata/caps-test3.xml index e6c56c5..d359f25 100644 --- a/tests/capabilityschemadata/caps-test3.xml +++ b/tests/capabilityschemadata/caps-test3.xml @@ -82,6 +82,8 @@ <secmodel> <model>dac</model> <doi>0</doi> + <baselabel type='kvm'>0:0</baselabel> + <baselabel type='qemu'>0:0</baselabel> </secmodel> </host>
s/0/107/ 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 :|

Giuseppe Scrivano <gscrivan@redhat.com> writes:
Now each security model can define its own base label, that describes the default security context used by libvirt to run an hypervisor process. This information is exposed to users trough the host capabilities XML.
*v3 major changes - support LXC - merge virSecurityDACSetUser and virSecurityDACSetGroup in virSecurityDACSetUserAndGroup - DAC sets the baselabel in virSecurityDACSetUserAndGroup - Use virDomainVirtTypeToString instead of hardcoding the name
Giuseppe Scrivano (2): security: add new internal function "virSecurityManagerGetBaseLabel" capabilities: add baselabel per sec driver/virt type to secmodel
ping

Giuseppe Scrivano <gscrivan@redhat.com> writes:
*v3 major changes - support LXC - merge virSecurityDACSetUser and virSecurityDACSetGroup in virSecurityDACSetUserAndGroup - DAC sets the baselabel in virSecurityDACSetUserAndGroup - Use virDomainVirtTypeToString instead of hardcoding the name
Giuseppe Scrivano (2): security: add new internal function "virSecurityManagerGetBaseLabel" capabilities: add baselabel per sec driver/virt type to secmodel
ping
someone had a chance to look at this series? Giuseppe

Giuseppe Scrivano <gscrivan@redhat.com> writes:
Now each security model can define its own base label, that describes the default security context used by libvirt to run an hypervisor process. This information is exposed to users trough the host capabilities XML.
*v3 major changes - support LXC - merge virSecurityDACSetUser and virSecurityDACSetGroup in virSecurityDACSetUserAndGroup - DAC sets the baselabel in virSecurityDACSetUserAndGroup - Use virDomainVirtTypeToString instead of hardcoding the name
I've ran a quick smoke test on top of the current HEAD and it seems to work, can someone please review it or tell me if it makes sense at all to have this information under "capabilities"? Thanks, Giuseppe

On Thu, Oct 17, 2013 at 02:55:04PM +0200, Giuseppe Scrivano wrote:
Giuseppe Scrivano <gscrivan@redhat.com> writes:
Now each security model can define its own base label, that describes the default security context used by libvirt to run an hypervisor process. This information is exposed to users trough the host capabilities XML.
*v3 major changes - support LXC - merge virSecurityDACSetUser and virSecurityDACSetGroup in virSecurityDACSetUserAndGroup - DAC sets the baselabel in virSecurityDACSetUserAndGroup - Use virDomainVirtTypeToString instead of hardcoding the name
I've ran a quick smoke test on top of the current HEAD and it seems to work, can someone please review it or tell me if it makes sense at all to have this information under "capabilities"?
Opps my bad, it was on my todo list to review this, but I totally forgot. Will do it now. 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 :|
participants (2)
-
Daniel P. Berrange
-
Giuseppe Scrivano