[libvirt] [PATCH 0/3] caps: expose the user and group owner of the HV

virt-manager has no way to known the user/group that run the HV process without requiring the user to manually set it trough setup.py or assuming "root" as default value. This series extends the Guest capabilities with "hv_user" and "hv_group", to inform the libvirt user about the user/group (when qemu is used) that run the HV process. Giuseppe Scrivano (3): caps: new internal function "virCapabilitiesSetHvUserAndGroup" qemu: set hv_user/hv_group caps to the user/group which runs qemu caps: add tests and documentation for hv_user and hv_group docs/schemas/capability.rng | 10 +++++++ src/conf/capabilities.c | 46 +++++++++++++++++++++++++++++++ src/conf/capabilities.h | 7 +++++ src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 3 ++ tests/capabilityschemadata/caps-test2.xml | 4 +++ 6 files changed, 71 insertions(+) -- 1.8.3.1

It allows to set the optional elements "hv_user" and "hv_group" for a guest. hv_user and hv_group can be used to specify the user/group which own the hypervisor. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- src/conf/capabilities.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/capabilities.h | 7 +++++++ src/libvirt_private.syms | 1 + 3 files changed, 54 insertions(+) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 1acc936..ffb1db5 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -169,6 +169,9 @@ virCapabilitiesFreeGuest(virCapsGuestPtr guest) virCapabilitiesFreeGuestFeature(guest->features[i]); VIR_FREE(guest->features); + VIR_FREE(guest->hvUser); + VIR_FREE(guest->hvGroup); + VIR_FREE(guest); } @@ -470,6 +473,43 @@ error: /** + * virCapabilitiesSetHvUserAndGroup: + * @guest: guest to set HV user:group for. + * @user: name of the user which owns the hypervisor. Use NULL to unset. + * @group: name of the group which owns the hypervisor. Use NULL to unset. + * + * Set the user and the group which own the hypervisor. + */ +int +virCapabilitiesSetHvUserAndGroup(virCapsGuestPtr guest, + const char *user, + const char *group) +{ + char *u = NULL; + char *g = NULL; + + if (user && VIR_STRDUP(u, user) < 0) + goto no_memory; + + if (group && VIR_STRDUP(g, group) < 0) + goto no_memory; + + VIR_FREE(guest->hvUser); + VIR_FREE(guest->hvGroup); + + guest->hvUser = u; + guest->hvGroup = g; + + return 0; + +no_memory: + VIR_FREE(u); + VIR_FREE(g); + return -1; +} + + +/** * virCapabilitiesAddGuestFeature: * @guest: guest to associate feature with * @name: name of feature ('pae', 'acpi', 'apic') @@ -903,6 +943,12 @@ virCapabilitiesFormatXML(virCapsPtr caps) virBufferAddLit(&xml, " </features>\n"); } + if (caps->guests[i]->hvUser) + virBufferAsprintf(&xml, " <hv_user>%s</hv_user>\n", + caps->guests[i]->hvUser); + if (caps->guests[i]->hvGroup) + virBufferAsprintf(&xml, " <hv_group>%s</hv_group>\n", + caps->guests[i]->hvGroup); virBufferAddLit(&xml, " </guest>\n\n"); } diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 88ec454..a6fe7e0 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -80,6 +80,8 @@ typedef struct _virCapsGuest virCapsGuest; typedef virCapsGuest *virCapsGuestPtr; struct _virCapsGuest { char *ostype; + char *hvUser; + char *hvGroup; virCapsGuestArch arch; size_t nfeatures; size_t nfeatures_max; @@ -218,6 +220,11 @@ virCapabilitiesAddGuestDomain(virCapsGuestPtr guest, int nmachines, virCapsGuestMachinePtr *machines); +extern int +virCapabilitiesSetHvUserAndGroup(virCapsGuestPtr guest, + const char *user, + const char *group); + extern virCapsGuestFeaturePtr virCapabilitiesAddGuestFeature(virCapsGuestPtr guest, const char *name, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c25a61f..41219d6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -60,6 +60,7 @@ virCapabilitiesFreeNUMAInfo; virCapabilitiesGetCpusForNodemask; virCapabilitiesNew; virCapabilitiesSetHostCPU; +virCapabilitiesSetHvUserAndGroup; # conf/cpu_conf.h -- 1.8.3.1

Use QEMU_USER and QEMU_GROUP, that are passed to libvirt at configure-time, as values for "hv_user" and "hv_group" in the guest capabilities. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- src/qemu/qemu_capabilities.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ffbede7..d536554 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -749,6 +749,9 @@ virQEMUCapsInitGuest(virCapsPtr caps, machines)) == NULL) goto error; + if (virCapabilitiesSetHvUserAndGroup(guest, QEMU_USER, QEMU_GROUP) < 0) + goto error; + machines = NULL; nmachines = 0; -- 1.8.3.1

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- docs/schemas/capability.rng | 10 ++++++++++ tests/capabilityschemadata/caps-test2.xml | 4 ++++ 2 files changed, 14 insertions(+) diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 65c7c72..1a65142 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -225,6 +225,16 @@ <optional> <ref name='features'/> </optional> + <optional> + <element name='hv_user'> + <ref name='genericName'/> + </element> + </optional> + <optional> + <element name='hv_group'> + <ref name='genericName'/> + </element> + </optional> </element> </define> diff --git a/tests/capabilityschemadata/caps-test2.xml b/tests/capabilityschemadata/caps-test2.xml index a99c1b8..4a57e44 100644 --- a/tests/capabilityschemadata/caps-test2.xml +++ b/tests/capabilityschemadata/caps-test2.xml @@ -76,6 +76,8 @@ <acpi default='on' toggle='yes'/> <apic default='on' toggle='no'/> </features> + <hv_user>foo</hv_user> + <hv_group>bar</hv_group> </guest> <guest> @@ -117,6 +119,8 @@ <acpi default='on' toggle='yes'/> <apic default='on' toggle='no'/> </features> + <hv_user>ba-r</hv_user> + <hv_group>baz_</hv_group> </guest> </capabilities> -- 1.8.3.1

On Tue, Aug 27, 2013 at 02:00:03AM +0200, Giuseppe Scrivano wrote:
virt-manager has no way to known the user/group that run the HV process without requiring the user to manually set it trough setup.py or assuming "root" as default value.
This series extends the Guest capabilities with "hv_user" and "hv_group", to inform the libvirt user about the user/group (when qemu is used) that run the HV process.
This is not a suitably generic approach. The 'user' and 'group' associated with QEMU are default settings used by the DAC security driver if the user does not specify the user and group themselves in the <seclabel> XML. Similarly with the SELinux security driver there is a default label used (svirt_t), if the user does not specify the label themselves. So to expose anything in the XML we should be providing a way to describe the default label for any security driver. Also there can actually be multiple default security labels depending on the virt type usd (kvm vs tcg) eg where the capabilities has <secmodel> <model>selinux</model> <doi>0</doi> </secmodel> <secmodel> <model>dac</model> <doi>0</doi> </secmodel> We should look at extending it to be something like <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_tcg_t:s0</baselabel> </secmodel> <secmodel> <model>dac</model> <doi>0</doi> <baselabel type='kvm'>107:107</baselabel> <baselabel type='qemu'>107:107</baselabel> </secmodel> NB, I used 'baselabel' to mirror the same 'baselabel' naming used in the domain XML <seclabel> element. 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 :|

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. Giuseppe Scrivano (3): security: add new internal function "virSecurityManagerGetBaseLabel" capabilities: add baselabel per sec driver/virt type to secmodel capabilities: document and test "<baselabel>" docs/schemas/capability.rng | 8 ++++ src/conf/capabilities.c | 60 +++++++++++++++++++++++++++- src/conf/capabilities.h | 14 +++++++ src/libvirt_private.syms | 2 + src/qemu/qemu_conf.c | 11 +++-- src/security/security_apparmor.c | 7 ++++ src/security/security_dac.c | 26 +++++++++++- src/security/security_driver.h | 3 ++ src/security/security_manager.c | 15 +++++++ src/security/security_manager.h | 2 + src/security/security_nop.c | 9 +++++ src/security/security_selinux.c | 9 +++++ src/security/security_stack.c | 8 ++++ tests/capabilityschemadata/caps-qemu-kvm.xml | 2 + tests/capabilityschemadata/caps-test3.xml | 2 + 15 files changed, 172 insertions(+), 6 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 | 7 +++++++ src/security/security_dac.c | 26 +++++++++++++++++++++++++- src/security/security_driver.h | 3 +++ src/security/security_manager.c | 15 +++++++++++++++ src/security/security_manager.h | 2 ++ src/security/security_nop.c | 9 +++++++++ src/security/security_selinux.c | 9 +++++++++ src/security/security_stack.c | 8 ++++++++ 9 files changed, 79 insertions(+), 1 deletion(-) 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..6f95ce5 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -931,6 +931,11 @@ AppArmorGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return opts; } +static const char * +AppArmorGetBaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return ""; +} virSecurityDriver virAppArmorSecurityDriver = { .privateDataLen = 0, @@ -972,4 +977,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..d5e93fa 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -47,6 +47,7 @@ struct _virSecurityDACData { gid_t *groups; int ngroups; bool dynamicOwnership; + char *baselabel; }; void @@ -217,6 +218,7 @@ virSecurityDACClose(virSecurityManagerPtr mgr) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); VIR_FREE(priv->groups); + VIR_FREE(priv->baselabel); return 0; } @@ -1114,8 +1116,9 @@ virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); - if (!secdef || !seclabel) + if (!secdef || !seclabel) { return -1; + } if (secdef->label) ignore_value(virStrcpy(seclabel->label, secdef->label, @@ -1170,6 +1173,25 @@ virSecurityDACGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return NULL; } +static const char * +virSecurityDACGetBaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + uid_t user; + gid_t group; + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + if (priv->baselabel) + return priv->baselabel; + + if (virGetUserID(QEMU_USER, &user) < 0 || + virGetGroupID(QEMU_GROUP, &group) < 0 || + virAsprintf(&priv->baselabel, "%u:%u", + (unsigned int) priv->user, + (unsigned int) priv->group) < 0) + return NULL; + + return priv->baselabel; +} + virSecurityDriver virSecurityDriverDAC = { .privateDataLen = sizeof(virSecurityDACData), .name = SECURITY_DAC_NAME, @@ -1212,4 +1234,6 @@ virSecurityDriver virSecurityDriverDAC = { .domainSetSecurityTapFDLabel = virSecurityDACSetTapFDLabel, .domainGetSecurityMountOptions = virSecurityDACGetMountOptions, + + .getBaseLabel = virSecurityDACGetBaseLabel, }; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 8735558..64bd307 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -46,6 +46,7 @@ typedef int (*virSecurityDriverClose) (virSecurityManagerPtr mgr); typedef const char *(*virSecurityDriverGetModel) (virSecurityManagerPtr mgr); typedef const char *(*virSecurityDriverGetDOI) (virSecurityManagerPtr mgr); +typedef const char *(*virSecurityDriverGetBaseLabel) (virSecurityManagerPtr mgr); typedef int (*virSecurityDriverPreFork) (virSecurityManagerPtr mgr); @@ -154,6 +155,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..8535c8e 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -273,6 +273,21 @@ virSecurityManagerGetModel(virSecurityManagerPtr mgr) return NULL; } +const char * +virSecurityManagerGetBaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + if (mgr->drv->getBaseLabel) { + const char *ret; + virObjectLock(mgr); + ret = mgr->drv->getBaseLabel(mgr); + 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..381cfc9 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); + 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..c0d0f08 100644 --- a/src/security/security_nop.c +++ b/src/security/security_nop.c @@ -186,6 +186,13 @@ static char *virSecurityDomainGetMountOptionsNop(virSecurityManagerPtr mgr ATTRI return opts; } +static const char * +virSecurityGetBaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return ""; +} + + virSecurityDriver virSecurityDriverNop = { .privateDataLen = 0, .name = "none", @@ -226,4 +233,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..d7cafc6 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1827,6 +1827,14 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def, } +static const char * +virSecuritySELinuxGetBaseLabel(virSecurityManagerPtr mgr) +{ + virSecuritySELinuxDataPtr priv = virSecurityManagerGetPrivateData(mgr); + return priv->domain_context; +} + + static int virSecuritySELinuxRestoreSecurityAllLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, @@ -2474,4 +2482,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..d704dd9 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -555,6 +555,12 @@ virSecurityStackGetNested(virSecurityManagerPtr mgr) return list; } +static const char * +virSecurityStackGetBaseLabel(virSecurityManagerPtr mgr) +{ + return virSecurityManagerGetBaseLabel(virSecurityStackGetPrimary(mgr)); +} + virSecurityDriver virSecurityDriverStack = { .privateDataLen = sizeof(virSecurityStackData), .name = "stack", @@ -599,4 +605,6 @@ virSecurityDriver virSecurityDriverStack = { .domainGetSecurityMountOptions = virSecurityStackGetMountOptions, .domainSetSecurityHugepages = virSecurityStackSetHugepages, + + .getBaseLabel = virSecurityStackGetBaseLabel, }; -- 1.8.3.1

On Thu, Sep 05, 2013 at 01:49:43PM +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 | 7 +++++++ src/security/security_dac.c | 26 +++++++++++++++++++++++++- src/security/security_driver.h | 3 +++ src/security/security_manager.c | 15 +++++++++++++++ src/security/security_manager.h | 2 ++ src/security/security_nop.c | 9 +++++++++ src/security/security_selinux.c | 9 +++++++++ src/security/security_stack.c | 8 ++++++++ 9 files changed, 79 insertions(+), 1 deletion(-) +static const char * +AppArmorGetBaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return ""; +}
I wonder if we should just return NULL here. I don't think we need to be able to report errors other than "no base label", so I think using NULL for that is sufficient.
@@ -1170,6 +1173,25 @@ virSecurityDACGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return NULL; }
+static const char * +virSecurityDACGetBaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + uid_t user; + gid_t group; + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + if (priv->baselabel) + return priv->baselabel; + + if (virGetUserID(QEMU_USER, &user) < 0 || + virGetGroupID(QEMU_GROUP, &group) < 0 || + virAsprintf(&priv->baselabel, "%u:%u", + (unsigned int) priv->user, + (unsigned int) priv->group) < 0) + return NULL;
It would be better to initialize the 'pribv->baselabel' when we first set the user/group, so that this getter does not have side effects.
+typedef const char *(*virSecurityDriverGetBaseLabel) (virSecurityManagerPtr mgr);
We need to be able to pass in 'int virttype' here...
+static const char * +virSecuritySELinuxGetBaseLabel(virSecurityManagerPtr mgr) +{ + virSecuritySELinuxDataPtr priv = virSecurityManagerGetPrivateData(mgr); + return priv->domain_context;
....So that here we can do if (virttype == VIR_DOMAIN_VIRT_QEMU) return priv->alt_domain_context else return priv->domain_context Regards, 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> --- src/conf/capabilities.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++-- src/conf/capabilities.h | 14 +++++++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_conf.c | 11 ++++++--- 4 files changed, 81 insertions(+), 5 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); +} + +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/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 1f57f72..71dbb27 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -563,7 +563,7 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) virCapsPtr caps; virSecurityManagerPtr *sec_managers = NULL; /* Security driver data */ - const char *doi, *model; + const char *doi, *model, *label; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); /* Basic host arch / guest machine capabilities */ @@ -589,11 +589,16 @@ 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) + label = virSecurityManagerGetBaseLabel(sec_managers[i]); + if (VIR_STRDUP(sm->model, model) < 0 || + VIR_STRDUP(sm->doi, doi) < 0 || + virCapabilitiesHostSecModelAddBaseLabel(sm, "kvm", label) < 0 || + virCapabilitiesHostSecModelAddBaseLabel(sm, "qemu", label) < 0) goto error; + VIR_DEBUG("Initialized caps for security driver \"%s\" with " "DOI \"%s\"", model, doi); } -- 1.8.3.1

On Thu, Sep 05, 2013 at 01:49:44PM +0200, Giuseppe Scrivano wrote:
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) + label = virSecurityManagerGetBaseLabel(sec_managers[i]);
You need to call this twice, once for kvm and once for qemu since different labels will be used
+ if (VIR_STRDUP(sm->model, model) < 0 || + VIR_STRDUP(sm->doi, doi) < 0 || + virCapabilitiesHostSecModelAddBaseLabel(sm, "kvm", label) < 0 || + virCapabilitiesHostSecModelAddBaseLabel(sm, "qemu", label) < 0)
And pass in the different labels here. Also, you need to update the lxc_driver code to set this metadata since it uses sVirt too. 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 :|

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- docs/schemas/capability.rng | 8 ++++++++ tests/capabilityschemadata/caps-qemu-kvm.xml | 2 ++ tests/capabilityschemadata/caps-test3.xml | 2 ++ 3 files changed, 12 insertions(+) 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/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 Thu, Sep 05, 2013 at 01:49:45PM +0200, Giuseppe Scrivano wrote:
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- docs/schemas/capability.rng | 8 ++++++++ tests/capabilityschemadata/caps-qemu-kvm.xml | 2 ++ tests/capabilityschemadata/caps-test3.xml | 2 ++ 3 files changed, 12 insertions(+)
It would be preferrable to have thse changes in the same commit that you changed conf/capabilities.{c,h}. And then have the changes to the qemu & lxc drivers separate from the changes to the core capabilities code. 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 Thu, Sep 05, 2013 at 01:49:42PM +0200, Giuseppe Scrivano wrote:
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.
Just a quick note for future reference. When posting v2/v3/etc versions of a patch series, we generally prefer them to be posted as new top level threads, rather than setting In-reply-to against the original posting. The reason is we've found people a more likely to miss new postings when posted as replies, and it also makes it clearer that the new posting is a self-contained series 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