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(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list