Re: [libvirt] [PATCH] SELinux: don't silently fail when no label is present

2014-06-10 Thread Ján Tomko 
On 06/10/2014 12:33 AM, Eric Blake wrote:
> On 06/09/2014 08:30 AM, Ján Tomko wrote:
>> This fixes startup of a domain with:
>> 
>> on a host with selinux and dac drivers and
>> security_default_confined = 0
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1105939
>> ---
>>  src/security/security_selinux.c | 98 
>> -
>>  1 file changed, 29 insertions(+), 69 deletions(-)
> 
> Shouldn't there also be a change somewhere under tests/ to ensure that
> we parse XML in that situation?

I can add a test for parsing: 
but the only thing in seclabel XML parsing that depends on the security
drivers is the default model that's filled in for 

I'll send it separately as it wouldn't be directly related to the bug.

> 
> Otherwise, this looks like a fairly mechanical conversion of
> virDomainDefGetSecurityLabelDef() returning NULL from being a silent
> error into being successful; and that makes sense since domain_conf.c
> does not report any errors if a label cannot be looked up.  ACK.
> 

Thanks, pushed.

Jan



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] SELinux: don't silently fail when no label is present

2014-06-09 Thread Eric Blake 
On 06/09/2014 08:30 AM, Ján Tomko wrote:
> This fixes startup of a domain with:
> 
> on a host with selinux and dac drivers and
> security_default_confined = 0
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1105939
> ---
>  src/security/security_selinux.c | 98 
> -
>  1 file changed, 29 insertions(+), 69 deletions(-)

Shouldn't there also be a change somewhere under tests/ to ensure that
we parse XML in that situation?

Otherwise, this looks like a fairly mechanical conversion of
virDomainDefGetSecurityLabelDef() returning NULL from being a silent
error into being successful; and that makes sense since domain_conf.c
does not report any errors if a label cannot be looked up.  ACK.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] SELinux: don't silently fail when no label is present

2014-06-09 Thread Ján Tomko 
This fixes startup of a domain with:

on a host with selinux and dac drivers and
security_default_confined = 0

https://bugzilla.redhat.com/show_bug.cgi?id=1105939
---
 src/security/security_selinux.c | 98 -
 1 file changed, 29 insertions(+), 69 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 8380bba..008c58c 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -585,7 +585,7 @@ virSecuritySELinuxGenSecurityLabel(virSecurityManagerPtr 
mgr,
 
 seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
 if (seclabel == NULL)
-return rc;
+return 0;
 
 data = virSecurityManagerGetPrivateData(mgr);
 
@@ -739,11 +739,7 @@ 
virSecuritySELinuxReserveSecurityLabel(virSecurityManagerPtr mgr,
 virSecurityLabelDefPtr seclabel;
 
 seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
-if (seclabel == NULL) {
-return -1;
-}
-
-if (seclabel->type == VIR_DOMAIN_SECLABEL_STATIC)
+if (!seclabel || seclabel->type == VIR_DOMAIN_SECLABEL_STATIC)
 return 0;
 
 if (getpidcon_raw(pid, &pctx) == -1) {
@@ -1060,7 +1056,7 @@ 
virSecuritySELinuxSetSecurityTPMFileLabel(virSecurityManagerPtr mgr,
 
 seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
 if (seclabel == NULL)
-return -1;
+return 0;
 
 switch (tpm->type) {
 case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
@@ -1102,7 +1098,7 @@ 
virSecuritySELinuxRestoreSecurityTPMFileLabelInt(virSecurityManagerPtr mgr,
 
 seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
 if (seclabel == NULL)
-return -1;
+return 0;
 
 switch (tpm->type) {
 case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
@@ -1136,7 +1132,7 @@ 
virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
 
 seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
 if (seclabel == NULL)
-return -1;
+return 0;
 
 disk_seclabel = virDomainDiskDefGetSecurityLabelDef(disk,
 SECURITY_SELINUX_NAME);
@@ -1256,10 +1252,7 @@ 
virSecuritySELinuxSetSecurityImageLabel(virSecurityManagerPtr mgr,
 cbdata.manager = mgr;
 cbdata.secdef = virDomainDefGetSecurityLabelDef(def, 
SECURITY_SELINUX_NAME);
 
-if (cbdata.secdef == NULL)
-return -1;
-
-if (cbdata.secdef->norelabel)
+if (!cbdata.secdef || cbdata.secdef->norelabel)
 return 0;
 
 if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NETWORK)
@@ -1279,7 +1272,7 @@ virSecuritySELinuxSetSecurityHostdevLabelHelper(const 
char *file, void *opaque)
 
 secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
 if (secdef == NULL)
-return -1;
+return 0;
 return virSecuritySELinuxSetFilecon(file, secdef->imagelabel);
 }
 
@@ -1397,7 +1390,7 @@ 
virSecuritySELinuxSetSecurityHostdevCapsLabel(virDomainDefPtr def,
 
 secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
 if (secdef == NULL)
-return -1;
+return 0;
 
 switch (dev->source.caps.type) {
 case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_STORAGE: {
@@ -1447,10 +1440,7 @@ 
virSecuritySELinuxSetSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UN
 virSecurityLabelDefPtr secdef;
 
 secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
-if (secdef == NULL)
-return -1;
-
-if (secdef->norelabel)
+if (!secdef || secdef->norelabel)
 return 0;
 
 switch (dev->mode) {
@@ -1635,10 +1625,7 @@ 
virSecuritySELinuxRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr,
 virSecurityLabelDefPtr secdef;
 
 secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
-if (secdef == NULL)
-return -1;
-
-if (secdef->norelabel)
+if (!secdef || secdef->norelabel)
 return 0;
 
 switch (dev->mode) {
@@ -1667,14 +1654,14 @@ 
virSecuritySELinuxSetSecurityChardevLabel(virDomainDefPtr def,
 int ret = -1;
 
 seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
-if (seclabel == NULL)
-return -1;
+if (!seclabel || seclabel->norelabel)
+return 0;
 
 if (dev)
 chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
   
SECURITY_SELINUX_NAME);
 
-if (seclabel->norelabel || (chr_seclabel && chr_seclabel->norelabel))
+if (chr_seclabel && chr_seclabel->norelabel)
 return 0;
 
 if (chr_seclabel)
@@ -1738,13 +1725,13 @@ 
virSecuritySELinuxRestoreSecurityChardevLabel(virSecurityManagerPtr mgr,
 int ret = -1;
 
 seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
-if (seclabel == NULL)
-return -1;
+if (!seclabel || seclabel->norelabel)
+return 0;
 
 if (dev)