[libvirt] [PATCH] fix parsing security labels from virt-aa-helper
Guido Günther
agx at sigxcpu.org
Thu Nov 3 17:15:21 UTC 2016
Hi,x
On Mon, Oct 31, 2016 at 11:32:44AM +0100, Christian Ehrhardt wrote:
> When parsing labels virt-aa-helper does no more pass
> VIR_DOMAIN_DEF_PARSE_INACTIVE due to dfbc9a83 that tried to mitigate the
> changes of a89f05ba. For those it had to switch from
I wouldn't call it mitigate. It was rather a fix.
> VIR_DOMAIN_DEF_PARSE_INACTIVE to active since we need the domain id
> (ctl->def->id) as it is part of the socket path now which is needed for the
> aa profile.
>
> But that turned out to break non apparmor seclabels as well as apparmor
> seclabels in xmls without labels.
While I understand the "non apparmor seclabel" part I don't understand
what you mean by "apparmor seclabels in xmls without labels". On Debian
based distros e.g. virtinst creates all VMs without seclabels and we let
the dac and apparmor security drivers do the work. I.e. this gets added
to the domains live xml upon start:
<seclabel type='dynamic' model='apparmor' relabel='yes'>
<label>libvirt-a9287b6e-ca06-42fe-b1a2-06830752843a</label>
<imagelabel>libvirt-a9287b6e-ca06-42fe-b1a2-06830752843a</imagelabel>
</seclabel>
<seclabel type='dynamic' model='dac' relabel='yes'>
<label>+117:+128</label>
<imagelabel>+117:+128</imagelabel>
</seclabel>
> In those cases due to VIR_DOMAIN_DEF_PARSE_INACTIVE now not set anymore
> virSecurityLabelDefParseXML insists on finding labels. Cases:
> - non-apparmor seclabel - virt-aa-helper breaks
> - apparmor seclabel without labels on a defined domain - virt-aa-helper breaks
> This was not spotted due to labels getting dynamically created on
> definition.
As far as I understand it dynamic labels get created on domain start not
domain definition (and are updated on e.g. device plug).
> So "define, start, stop" works. But "define, edit (add label), start"
> does not.
What do you add during the edit step. Can you provide an example?
> Now turning back on VIR_DOMAIN_DEF_PARSE_INACTIVE would cause the old bug, so
> we have to differ those more fine grained. This is done by the new flag
> VIR_DOMAIN_DEF_PARSE_SKIP_ACTIVE_LABEL which is like
> VIR_DOMAIN_DEF_PARSE_INACTIVE but only for the security labels.
> So far only set by virt-aa-helper.
>
> Testcase with virt-aa-helper on xml file:
> virt-aa-helper -d -r -p 0 -u libvirt-<uuid> < your-guest.xml
> virt-aa-helper: error: could not parse XML
> virt-aa-helper: error: could not get VM definition
> (That should have printed a valid apparmor profile)
/usr/lib/libvirt/virt-aa-helper -d -r -p 0 -u libvirt-a9287b6e-ca06-42fe-b1a2-06830752843a < /etc/libvirt/qemu/wheezy.xml
virt-aa-helper:
/etc/apparmor.d/libvirt/libvirt-a9287b6e-ca06-42fe-b1a2-06830752843a.files
virt-aa-helper:
"/var/log/libvirt/**/wheezy.log" w,
"/var/lib/libvirt/qemu/domain-wheezy/monitor.sock" rw,
"/var/lib/libvirt/qemu/domain--1-wheezy/*" rw,
"/var/lib/libvirt/qemu/channel/target/domain--1-wheezy/*" rw,
"/var/run/libvirt/**/wheezy.pid" rwk,
"/run/libvirt/**/wheezy.pid" rwk,
"/var/run/libvirt/**/*.tunnelmigrate.dest.wheezy" rw,
"/run/libvirt/**/*.tunnelmigrate.dest.wheezy" rw,
"/var/scratch/sdcard/vmimages/wheezy.qcow2" rw,
"/var/scratch/src/lts/rails/**" rw,
"/var/scratch/src/lts/rails/" r,
/dev/vhost-net rw,
I don't understand why VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE is not sufficient.
Cheers,
-- Guido
>
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
> ---
> src/conf/domain_conf.c | 6 ++++--
> src/conf/domain_conf.h | 3 +++
> src/security/virt-aa-helper.c | 1 +
> 3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 03506cb..9eb7883 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6626,7 +6626,8 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
> * if the 'live' VM XML is requested
> */
> if (seclabel->type == VIR_DOMAIN_SECLABEL_STATIC ||
> - (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
> + (!(flags & (VIR_DOMAIN_DEF_PARSE_SKIP_ACTIVE_LABEL |
> + VIR_DOMAIN_DEF_PARSE_INACTIVE)) &&
> seclabel->type != VIR_DOMAIN_SECLABEL_NONE)) {
> p = virXPathStringLimit("string(./label[1])",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> @@ -6642,7 +6643,8 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
>
> /* Only parse imagelabel, if requested live XML with relabeling */
> if (seclabel->relabel &&
> - (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
> + (!(flags & (VIR_DOMAIN_DEF_PARSE_SKIP_ACTIVE_LABEL |
> + VIR_DOMAIN_DEF_PARSE_INACTIVE)) &&
> seclabel->type != VIR_DOMAIN_SECLABEL_NONE)) {
> p = virXPathStringLimit("string(./imagelabel[1])",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 24aa79c..90693c6 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2657,6 +2657,9 @@ typedef enum {
> VIR_DOMAIN_DEF_PARSE_ABI_UPDATE = 1 << 9,
> /* skip definition validation checks meant to be executed on define time only */
> VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE = 1 << 10,
> + /* in regard to security labels, skip parts of the XML that would only be
> + * present in an active libvirt XML. */
> + VIR_DOMAIN_DEF_PARSE_SKIP_ACTIVE_LABEL = 1 << 11,
> } virDomainDefParseFlags;
>
> typedef enum {
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index 77eeaff..0ca4c83 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -705,6 +705,7 @@ get_definition(vahControl * ctl, const char *xmlStr)
>
> ctl->def = virDomainDefParseString(xmlStr,
> ctl->caps, ctl->xmlopt, NULL,
> + VIR_DOMAIN_DEF_PARSE_SKIP_ACTIVE_LABEL |
> VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE);
>
> if (ctl->def == NULL) {
> --
> 2.7.4
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
More information about the libvir-list
mailing list