fedora-selinux / selinux

Fedora SELinux Userspace
Other
21 stars 17 forks source link

Keep recursing if matchpathcon returns ENOENT #35

Closed rhatdan closed 7 years ago

rhatdan commented 7 years ago

We currently throw an exception in restorecon when matchpathcon returns ENOENT, which could indicate a directory with <> label. We should ignore this and just return.

rhatdan commented 7 years ago

@stephensmalley PTAL

stephensmalley commented 7 years ago

Actually, we want this rewritten to use the new selinux_restorecon() function in libselinux, see https://github.com/SELinuxProject/selinux/issues/29. We moved all of the setfiles/restorecon logic into a libselinux function, originally used in Android and then ported to upstream.

rhatdan commented 7 years ago

Does it ignore ENOENT from matchpathcon? We have an issue with cloud-init trying to relabel /var/lib and blowing up on a <> directory.

bachradsusi commented 7 years ago

Using the following patch, selinux_restorecon() can be exported via swig as python binding and used for directories with <<None>>:

--- a/libselinux/src/selinuxswig.i
+++ b/libselinux/src/selinuxswig.i
@@ -9,6 +9,7 @@
        #include "../include/selinux/get_context_list.h"
        #include "../include/selinux/get_default_type.h"
        #include "../include/selinux/label.h"
+       #include "../include/selinux/restorecon.h"
        #include "../include/selinux/selinux.h"
 %}
 %apply int *OUTPUT { int *enforce };
@@ -61,4 +62,5 @@
 %include "../include/selinux/get_context_list.h"
 %include "../include/selinux/get_default_type.h"
 %include "../include/selinux/label.h"
+%include "../include/selinux/restorecon.h"
 %include "../include/selinux/selinux.h"
# python3
>>> import selinux
>>> selinux.selinux_restorecon('/var/lib/nfs/rpc_pipefs', selinux.SELINUX_RESTORECON_VERBOSE)            
0
>>> selinux.selinux_restorecon('/var/lib/', selinux.SELINUX_RESTORECON_VERBOSE | selinux.SELINUX_RESTORECON_RECURSE)     
Updated digest for: /var/lib/
0
rhatdan commented 7 years ago

I would rather you add this to restorecon, also, so that recursive would work properly.

rhatdan commented 7 years ago

selinux.restorecon(PATH, Recurse=false, Verbose=false)

bachradsusi commented 7 years ago
--- a/libselinux/src/selinuxswig_python.i
+++ b/libselinux/src/selinuxswig_python.i
@@ -17,28 +17,10 @@ ENFORCING = 1
 def restorecon(path, recursive=False):
     """ Restore SELinux context on a given path """

-    try:
-        mode = os.lstat(path)[stat.ST_MODE]
-        status, context = matchpathcon(path, mode)
-    except OSError:
-        path = os.path.realpath(os.path.expanduser(path))
-        mode = os.lstat(path)[stat.ST_MODE]
-        status, context = matchpathcon(path, mode)
-
-    if status == 0:
-        try:
-            status, oldcontext = lgetfilecon(path)
-        except OSError as e:
-            if e.errno != errno.ENODATA:
-                raise
-            oldcontext = None
-        if context != oldcontext:
-            lsetfilecon(path, context)
-
-        if recursive:
-            for root, dirs, files in os.walk(path):
-                for name in files + dirs:
-                   restorecon(os.path.join(root, name))
+    restorecon_flags = SELINUX_RESTORECON_IGNORE_DIGEST
+    if recursive:
+        restorecon_flags |= SELINUX_RESTORECON_RECURSE
+    selinux_restorecon(os.path.expanduser(path), restorecon_flags)

 def chcon(path, context, recursive=False):
     """ Set the SELinux context on a given path """
bachradsusi commented 7 years ago

The patch above should follow the original behavior but using selinux_restorecon() internally. But there are probably some differences which needs to be handled.

Due to significant changes inside the restorecon(), I'd rather stay with the original patch for RHEL and use the selinux_restorecon() version only for Fedora.

rhatdan commented 7 years ago

The fix we are looking for is to be able to relabel /var/ or /var/lib. /var/lib/nfs/ fails, and the restorecon never completes.

stephensmalley commented 7 years ago

Yes, let's take the second patch upstream. The original patch seems fine for RHEL.

stephensmalley commented 7 years ago

Looks like you might want SELINUX_RESTORECON_REALPATH set in flags too to preserve current behavior of restorecon() method.

bachradsusi commented 7 years ago

I've got a patch set. I'm testing it on vanilla sources now and when I'm finished I'll send them upstream.

bachradsusi commented 7 years ago

Upstream patches merged - https://github.com/fedora-selinux/selinux/commit/b65c2317592945a4cd3ee1c67491ca668525f101 https://github.com/fedora-selinux/selinux/commit/3dcc89405fc1efdcd41b96c50b030174fcaf4514