[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCHv2 6/6] seclabel: honor device override in selinux



This wires up the XML changes in the previous patch to let SELinux
labeling honor user overrides, as well as affecting the live XML
configuration in one case where the user didn't specify anything
in the offline XML.

I noticed that the logs contained messages like this:

2011-12-05 23:32:40.382+0000: 26569: warning : SELinuxRestoreSecurityFileLabel:533 : cannot lookup default selinux label for /nfs/libvirt/images/dom.img

for all my domain images living on NFS.  But if we would just remember
that on domain creation that we were unable to set a SELinux label (due to
NFSv3 lacking labels, or NFSv4 not being configured to expose attributes),
then we could avoid wasting the time trying to clear the label on
domain shutdown.  This in turn is one less point of NFS failure,
especially since there have been documented cases of virDomainDestroy
hanging during an attempted operation on a failed NFS connection.

* src/security/security_selinux.c (SELinuxSetFilecon): Move guts...
(SELinuxSetFileconHelper): ...to new function.
(SELinuxSetFileconOptional): New function.
(SELinuxSetSecurityFileLabel): Honor override label, and remember
if labeling failed.
(SELinuxRestoreSecurityImageLabelInt): Skip relabeling based on
override.
---

Tested on a system with NFS, and /etc/libvirt/qemu.conf having
dynamic_ownership=0; without this patch, killing NFS via
'iptables -I INPUT -s $server -j DROP' while a guest was running,
then trying 'virsh destroy guest', would hang, and lock out
attempts to use virsh from other terminals.  After this patch,
'virsh destroy guest' succeeded without hanging.

Also tested that custom labels are honored.

 src/security/security_selinux.c |   53 ++++++++++++++++++++++++++++++---------
 1 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 6ef61c7..cdc28ad 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -394,8 +394,11 @@ SELinuxGetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
     return 0;
 }

+/* Attempt to change the label of PATH to TCON.  If OPTIONAL is true,
+ * return 1 if labelling was not possible.  Otherwise, require a label
+ * change, and return 0 for success, -1 for failure.  */
 static int
-SELinuxSetFilecon(const char *path, char *tcon)
+SELinuxSetFileconHelper(const char *path, char *tcon, bool optional)
 {
     security_context_t econ;

@@ -408,7 +411,7 @@ SELinuxSetFilecon(const char *path, char *tcon)
             if (STREQ(tcon, econ)) {
                 freecon(econ);
                 /* It's alright, there's nothing to change anyway. */
-                return 0;
+                return optional ? 1 : 0;
             }
             freecon(econ);
         }
@@ -440,12 +443,26 @@ SELinuxSetFilecon(const char *path, char *tcon)
                 VIR_INFO("Setting security context '%s' on '%s' not supported",
                          tcon, path);
             }
+            if (optional)
+                return 1;
         }
     }
     return 0;
 }

 static int
+SELinuxSetFileconOptional(const char *path, char *tcon)
+{
+    return SELinuxSetFileconHelper(path, tcon, true);
+}
+
+static int
+SELinuxSetFilecon(const char *path, char *tcon)
+{
+    return SELinuxSetFileconHelper(path, tcon, false);
+}
+
+static int
 SELinuxFSetFilecon(int fd, char *tcon)
 {
     security_context_t econ;
@@ -549,7 +566,7 @@ SELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
 {
     const virSecurityLabelDefPtr secdef = &vm->def->seclabel;

-    if (secdef->norelabel)
+    if (secdef->norelabel || (disk->seclabel && disk->seclabel->norelabel))
         return 0;

     /* Don't restore labels on readoly/shared disks, because
@@ -604,23 +621,35 @@ SELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk,
     const virSecurityLabelDefPtr secdef = opaque;
     int ret;

-    if (depth == 0) {
+    if (disk->seclabel && disk->seclabel->norelabel)
+        return 0;
+
+    if (disk->seclabel && !disk->seclabel->norelabel &&
+        disk->seclabel->label) {
+        ret = SELinuxSetFilecon(path, disk->seclabel->label);
+    } else if (depth == 0) {
         if (disk->shared) {
-            ret = SELinuxSetFilecon(path, default_image_context);
+            ret = SELinuxSetFileconOptional(path, default_image_context);
         } else if (disk->readonly) {
-            ret = SELinuxSetFilecon(path, default_content_context);
+            ret = SELinuxSetFileconOptional(path, default_content_context);
         } else if (secdef->imagelabel) {
-            ret = SELinuxSetFilecon(path, secdef->imagelabel);
+            ret = SELinuxSetFileconOptional(path, secdef->imagelabel);
         } else {
             ret = 0;
         }
     } else {
-        ret = SELinuxSetFilecon(path, default_content_context);
+        ret = SELinuxSetFileconOptional(path, default_content_context);
+    }
+    if (ret == 1 && !disk->seclabel) {
+        /* If we failed to set a label, but virt_use_nfs let us
+         * proceed anyway, then we don't need to relabel later.  */
+        if (VIR_ALLOC(disk->seclabel) < 0) {
+            virReportOOMError();
+            return -1;
+        }
+        disk->seclabel->norelabel = true;
+        ret = 0;
     }
-    if (ret < 0 &&
-        virStorageFileIsSharedFSType(path,
-                                     VIR_STORAGE_FILE_SHFS_NFS) == 1)
-       ret = 0;
     return ret;
 }

-- 
1.7.7.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]