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:
 seclabel type='none' model='dac'/
 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: seclabel type='none' model='dac'/
but the only thing in seclabel XML parsing that depends on the security
drivers is the default model that's filled in for seclabel type='none'/

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

[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:
seclabel type='none' model='dac'/
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;