label: rework label_fix() implementations (#8583)

This reworks the SELinux and SMACK label fixing calls in a number of
ways:

1. The two separate boolean arguments of these functions are converted
   into a flags type LabelFixFlags.

2. The operations are now implemented based on O_PATH. This should
   resolve TTOCTTOU races between determining the label for the file
   system object and applying it, as it it allows to pin the object
   while we are operating on it.

3. When changing a label fails we'll query the label previously set, and
   if matches what we want to set anyway we'll suppress the error.

Also, all calls to label_fix() are now (void)ified, when we ignore the
return values.

Fixes: #8566

diff --git a/src/basic/label.c b/src/basic/label.c
index 50cdd8c7319315a214add2b6d9f261dec3d25e60..d988dae0459555839ddbfe7a9771c79ea414ac86 100644 (file)
--- a/src/basic/label.c
+++ b/src/basic/label.c
@@ -28,11 +28,11 @@
 #include "selinux-util.h"
 #include "smack-util.h"
 
-int label_fix(const char *path, bool ignore_enoent, bool ignore_erofs) {
+int label_fix(const char *path, LabelFixFlags flags) {
         int r, q;
 
-        r = mac_selinux_fix(path, ignore_enoent, ignore_erofs);
-        q = mac_smack_fix(path, ignore_enoent, ignore_erofs);
+        r = mac_selinux_fix(path, flags);
+        q = mac_smack_fix(path, flags);
 
         if (r < 0)
                 return r;
@@ -62,7 +62,7 @@ int symlink_label(const char *old_path, const char *new_path) {
         if (r < 0)
                 return r;
 
-        return mac_smack_fix(new_path, false, false);
+        return mac_smack_fix(new_path, 0);
 }
 
 int btrfs_subvol_make_label(const char *path) {
@@ -80,6 +80,6 @@ int btrfs_subvol_make_label(const char *path) {
         if (r < 0)
                 return r;
 
-        return mac_smack_fix(path, false, false);
+        return mac_smack_fix(path, 0);
 }
 #endif // 0

diff --git a/src/basic/label.h b/src/basic/label.h
index 4bfcfe39f631e32dd819b5372b734b6e0a4e71de..cad224ce73c468f4e513222384789f3a37fdf3a4 100644 (file)
--- a/src/basic/label.h
+++ b/src/basic/label.h
@@ -23,7 +23,12 @@
 #include <stdbool.h>
 #include <sys/types.h>
 
-int label_fix(const char *path, bool ignore_enoent, bool ignore_erofs);
+typedef enum LabelFixFlags {
+        LABEL_IGNORE_ENOENT = 1U << 0,
+        LABEL_IGNORE_EROFS  = 1U << 1,
+} LabelFixFlags;
+
+int label_fix(const char *path, LabelFixFlags flags);
 
 int mkdir_label(const char *path, mode_t mode);
 #if 0 /// UNNEEDED by elogind

diff --git a/src/basic/mkdir-label.c b/src/basic/mkdir-label.c
index 4e5b7c96195adb522461eedbab6b06f8c6d55188..6b46566699175fc4b4222d373340762e83a3750e 100644 (file)
--- a/src/basic/mkdir-label.c
+++ b/src/basic/mkdir-label.c
@@ -44,7 +44,7 @@ int mkdir_label(const char *path, mode_t mode) {
         if (r < 0)
                 return r;
 
-        return mac_smack_fix(path, false, false);
+        return mac_smack_fix(path, 0);
 }
 
 int mkdir_safe_label(const char *path, mode_t mode, uid_t uid, gid_t gid, MkdirFlags flags) {

diff --git a/src/basic/selinux-util.c b/src/basic/selinux-util.c
index 5d782ab5d4e593fd3a372ed916f2ecbf5370a33e..955b8def7e6fd0e381ca2cabb9a7b0ad2d636f02 100644 (file)
--- a/src/basic/selinux-util.c
+++ b/src/basic/selinux-util.c
@@ -34,10 +34,12 @@
 #endif
 
 #include "alloc-util.h"
+//#include "fd-util.h"
 #include "log.h"
 #include "macro.h"
 #include "path-util.h"
 #include "selinux-util.h"
+//#include "stdio-util.h"
 #include "time-util.h"
 #include "util.h"
 
@@ -51,7 +53,8 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(context_t, context_free);
 static int cached_use = -1;
 static struct selabel_handle *label_hnd = NULL;
 
-#define log_enforcing(...) log_full_errno(security_getenforce() == 1 ? LOG_ERR : LOG_DEBUG, errno, __VA_ARGS__)
+#define log_enforcing(...) log_full(security_getenforce() == 1 ? LOG_ERR : LOG_DEBUG, __VA_ARGS__)
+#define log_enforcing_errno(r, ...) log_full_errno(security_getenforce() == 1 ? LOG_ERR : LOG_DEBUG, r, __VA_ARGS__)
 #endif
 
 bool mac_selinux_use(void) {
@@ -120,9 +123,12 @@ void mac_selinux_finish(void) {
 #endif
 }
 
-int mac_selinux_fix(const char *path, bool ignore_enoent, bool ignore_erofs) {
+int mac_selinux_fix(const char *path, LabelFixFlags flags) {
 
 #if HAVE_SELINUX
+        char procfs_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int)];
+        _cleanup_freecon_ char* fcon = NULL;
+        _cleanup_close_ int fd = -1;
         struct stat st;
         int r;
 
@@ -132,37 +138,55 @@ int mac_selinux_fix(const char *path, bool ignore_enoent, bool ignore_erofs) {
         if (!label_hnd)
                 return 0;
 
-        r = lstat(path, &st);
-        if (r >= 0) {
-                _cleanup_freecon_ char* fcon = NULL;
+        /* Open the file as O_PATH, to pin it while we determine and adjust the label */
+        fd = open(path, O_NOFOLLOW|O_CLOEXEC|O_PATH);
+        if (fd < 0) {
+                if ((flags & LABEL_IGNORE_ENOENT) && errno == ENOENT)
+                        return 0;
 
-                r = selabel_lookup_raw(label_hnd, &fcon, path, st.st_mode);
+                return -errno;
+        }
+
+        if (fstat(fd, &st) < 0)
+                return -errno;
+
+        if (selabel_lookup_raw(label_hnd, &fcon, path, st.st_mode) < 0) {
+                r = -errno;
 
                 /* If there's no label to set, then exit without warning */
-                if (r < 0 && errno == ENOENT)
+                if (r == -ENOENT)
                         return 0;
 
-                if (r >= 0) {
-                        r = lsetfilecon_raw(path, fcon);
-
-                        /* If the FS doesn't support labels, then exit without warning */
-                        if (r < 0 && errno == EOPNOTSUPP)
-                                return 0;
-                }
+                goto fail;
         }
 
-        if (r < 0) {
-                /* Ignore ENOENT in some cases */
-                if (ignore_enoent && errno == ENOENT)
+        xsprintf(procfs_path, "/proc/self/fd/%i", fd);
+        if (setfilecon_raw(procfs_path, fcon) < 0) {
+                _cleanup_freecon_ char *oldcon = NULL;
+
+                r = -errno;
+
+                /* If the FS doesn't support labels, then exit without warning */
+                if (r == -EOPNOTSUPP)
                         return 0;
 
-                if (ignore_erofs && errno == EROFS)
+                /* It the FS is read-only and we were told to ignore failures caused by that, suppress error */
+                if (r == -EROFS && (flags & LABEL_IGNORE_EROFS))
                         return 0;
 
-                log_enforcing("Unable to fix SELinux security context of %s: %m", path);
-                if (security_getenforce() == 1)
-                        return -errno;
+                /* If the old label is identical to the new one, suppress any kind of error */
+                if (getfilecon_raw(procfs_path, &oldcon) >= 0 && streq(fcon, oldcon))
+                        return 0;
+
+                goto fail;
         }
+
+        return 0;
+
+fail:
+        log_enforcing_errno(r, "Unable to fix SELinux security context of %s: %m", path);
+        if (security_getenforce() == 1)
+                return r;
 #endif
 
         return 0;

diff --git a/src/basic/selinux-util.h b/src/basic/selinux-util.h
index 4dfef4f06b20e5d25d55c22355b6a9586733b926..ea7386cdf3b00b0a9c43298f9e0504f4f60648bb 100644 (file)
--- a/src/basic/selinux-util.h
+++ b/src/basic/selinux-util.h
@@ -25,6 +25,7 @@
 #include <sys/types.h>
 
 #include "macro.h"
+//#include "label.h"
 
 bool mac_selinux_use(void);
 void mac_selinux_retest(void);
@@ -32,7 +33,7 @@ void mac_selinux_retest(void);
 int mac_selinux_init(void);
 void mac_selinux_finish(void);
 
-int mac_selinux_fix(const char *path, bool ignore_enoent, bool ignore_erofs);
+int mac_selinux_fix(const char *path, LabelFixFlags flags);
 #if 0 /// UNNEEDED by elogind
 int mac_selinux_apply(const char *path, const char *label);
 

diff --git a/src/basic/smack-util.c b/src/basic/smack-util.c
index fc09226a505fefef7b465dfd0ef06ed8b5789655..682a612b10ef068182085aefb29c7f6d0dd68784 100644 (file)
--- a/src/basic/smack-util.c
+++ b/src/basic/smack-util.c
@@ -21,18 +21,21 @@
 ***/
 
 #include <errno.h>
+//#include <fcntl.h>
 #include <string.h>
 #include <sys/stat.h>
 #include <sys/xattr.h>
 #include <unistd.h>
 
 #include "alloc-util.h"
+//#include "fd-util.h"
 #include "fileio.h"
 #include "log.h"
 #include "macro.h"
 #include "path-util.h"
 #include "process-util.h"
 #include "smack-util.h"
+//#include "stdio-util.h"
 #include "string-table.h"
 #include "xattr-util.h"
 
@@ -136,7 +139,10 @@ int mac_smack_apply_pid(pid_t pid, const char *label) {
 }
 #endif // 0
 
-int mac_smack_fix(const char *path, bool ignore_enoent, bool ignore_erofs) {
+int mac_smack_fix(const char *path, LabelFixFlags flags) {
+        char procfs_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int)];
+        _cleanup_close_ int fd = -1;
+        const char *label;
         struct stat st;
         int r;
 
@@ -145,50 +151,59 @@ int mac_smack_fix(const char *path, bool ignore_enoent, bool ignore_erofs) {
         if (!mac_smack_use())
                 return 0;
 
-        /*
-         * Path must be in /dev and must exist
-         */
+        /* Path must be in /dev */
         if (!path_startswith(path, "/dev"))
                 return 0;
 
-        r = lstat(path, &st);
-        if (r >= 0) {
-                const char *label;
-
-                /*
-                 * Label directories and character devices "*".
-                 * Label symlinks "_".
-                 * Don't change anything else.
-                 */
-
-                if (S_ISDIR(st.st_mode))
-                        label = SMACK_STAR_LABEL;
-                else if (S_ISLNK(st.st_mode))
-                        label = SMACK_FLOOR_LABEL;
-                else if (S_ISCHR(st.st_mode))
-                        label = SMACK_STAR_LABEL;
-                else
+        fd = open(path, O_NOFOLLOW|O_CLOEXEC|O_PATH);
+        if (fd < 0) {
+                if ((flags & LABEL_IGNORE_ENOENT) && errno == ENOENT)
                         return 0;
 
-                r = lsetxattr(path, "security.SMACK64", label, strlen(label), 0);
+                return -errno;
+        }
+
+        if (fstat(fd, &st) < 0)
+                return -errno;
+
+        /*
+         * Label directories and character devices "*".
+         * Label symlinks "_".
+         * Don't change anything else.
+         */
+
+        if (S_ISDIR(st.st_mode))
+                label = SMACK_STAR_LABEL;
+        else if (S_ISLNK(st.st_mode))
+                label = SMACK_FLOOR_LABEL;
+        else if (S_ISCHR(st.st_mode))
+                label = SMACK_STAR_LABEL;
+        else
+                return 0;
+
+        xsprintf(procfs_path, "/proc/self/fd/%i", fd);
+        if (setxattr(procfs_path, "security.SMACK64", label, strlen(label), 0) < 0) {
+                _cleanup_free_ char *old_label = NULL;
+
+                r = -errno;
 
                 /* If the FS doesn't support labels, then exit without warning */
-                if (r < 0 && errno == EOPNOTSUPP)
+                if (r == -EOPNOTSUPP)
                         return 0;
-        }
 
-        if (r < 0) {
-                /* Ignore ENOENT in some cases */
-                if (ignore_enoent && errno == ENOENT)
+                /* It the FS is read-only and we were told to ignore failures caused by that, suppress error */
+                if (r == -EROFS && (flags & LABEL_IGNORE_EROFS))
                         return 0;
 
-                if (ignore_erofs && errno == EROFS)
+                /* If the old label is identical to the new one, suppress any kind of error */
+                if (getxattr_malloc(procfs_path, "security.SMACK64", &old_label, false) >= 0 &&
+                    streq(old_label, label))
                         return 0;
 
-                r = log_debug_errno(errno, "Unable to fix SMACK label of %s: %m", path);
+                return log_debug_errno(r, "Unable to fix SMACK label of %s: %m", path);
         }
 
-        return r;
+        return 0;
 }
 
 #if 0 /// UNNEEDED by elogind

diff --git a/src/basic/smack-util.h b/src/basic/smack-util.h
index c691fdc1214570c38897fa4bea1b9f63cb4210c1..6f8e3ea0c4ab5acf89017c459851730c035ee636 100644 (file)
--- a/src/basic/smack-util.h
+++ b/src/basic/smack-util.h
@@ -25,6 +25,7 @@
 #include <stdbool.h>
 #include <sys/types.h>
 
+//#include "label.h"
 #include "macro.h"
 
 #define SMACK_FLOOR_LABEL "_"
@@ -32,12 +33,12 @@
 
 #if 0 /// UNNEEDED by elogind
 typedef enum SmackAttr {
-        SMACK_ATTR_ACCESS = 0,
-        SMACK_ATTR_EXEC = 1,
-        SMACK_ATTR_MMAP = 2,
-        SMACK_ATTR_TRANSMUTE = 3,
-        SMACK_ATTR_IPIN = 4,
-        SMACK_ATTR_IPOUT = 5,
+        SMACK_ATTR_ACCESS,
+        SMACK_ATTR_EXEC,
+        SMACK_ATTR_MMAP,
+        SMACK_ATTR_TRANSMUTE,
+        SMACK_ATTR_IPIN,
+        SMACK_ATTR_IPOUT,
         _SMACK_ATTR_MAX,
         _SMACK_ATTR_INVALID = -1,
 } SmackAttr;
@@ -45,7 +46,7 @@ typedef enum SmackAttr {
 
 bool mac_smack_use(void);
 
-int mac_smack_fix(const char *path, bool ignore_enoent, bool ignore_erofs);
+int mac_smack_fix(const char *path, LabelFixFlags flags);
 
 #if 0 /// UNNEEDED by elogind
 const char* smack_attr_to_string(SmackAttr i) _const_;

diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c
index 7073175e17a1f3c9d25e821bf7ecdcfc8262f8d3..f53d2fca0dc0c069fb2c70fdfddab8c6888b1163 100644 (file)
--- a/src/core/mount-setup.c
+++ b/src/core/mount-setup.c
@@ -182,7 +182,7 @@ static int mount_one(const MountPoint *p, bool relabel) {
 
         /* Relabel first, just in case */
         if (relabel)
-                (void) label_fix(p->where, true, true);
+                (void) label_fix(p->where, LABEL_IGNORE_ENOENT|LABEL_IGNORE_EROFS);
 
         r = path_is_mount_point(p->where, NULL, AT_SYMLINK_FOLLOW);
         if (r < 0 && r != -ENOENT) {
@@ -220,7 +220,7 @@ static int mount_one(const MountPoint *p, bool relabel) {
 
         /* Relabel again, since we now mounted something fresh here */
         if (relabel)
-                (void) label_fix(p->where, false, false);
+                (void) label_fix(p->where, 0);
 
         if (p->mode & MNT_CHECK_WRITABLE) {
                 if (access(p->where, W_OK) < 0) {
@@ -389,7 +389,7 @@ static int nftw_cb(
         if (_unlikely_(ftwbuf->level == 0))
                 return FTW_CONTINUE;
 
-        label_fix(fpath, false, false);
+        (void) label_fix(fpath, 0);
 
         /* /run/initramfs is static data and big, no need to
          * dynamically relabel its contents at boot... */
@@ -430,7 +430,7 @@ int mount_setup(bool loaded_policy) {
                 r = cg_all_unified();
                 if (r == 0) {
                         (void) mount(NULL, "/sys/fs/cgroup", NULL, MS_REMOUNT, NULL);
-                        label_fix("/sys/fs/cgroup", false, false);
+                        (void) label_fix("/sys/fs/cgroup", 0);
                         nftw("/sys/fs/cgroup", nftw_cb, 64, FTW_MOUNT|FTW_PHYS|FTW_ACTIONRETVAL);
                         (void) mount(NULL, "/sys/fs/cgroup", NULL, MS_REMOUNT|MS_RDONLY, NULL);
                 } else if (r < 0)

diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index 155ec6a5ae0392e508d1efe299bde5b03578711f..20b8601c1f6486a92deb338e7ecda09623152aa8 100644 (file)
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -374,7 +374,7 @@ static int user_mkdir_runtime_path(User *u) {
                         }
                 }
 
-                r = label_fix(u->runtime_path, false, false);
+                r = label_fix(u->runtime_path, 0);
                 if (r < 0)
                         log_warning_errno(r, "Failed to fix label of '%s', ignoring: %m", u->runtime_path);
         }