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 18c9a23fea79abadd870e3042ce861908d6fd9b9..134dd8d80aff465cbcc70f61c3ef8b1f20aa1d31 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;
@@ -60,7 +60,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) {
@@ -78,5 +78,5 @@ 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);
 }

diff --git a/src/basic/label.h b/src/basic/label.h
index d73dacec4fe3af47e96a831ae2347591be105ac8..269591c979e93b3f12aaaa0967ed307543c38c55 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);
 int symlink_label(const char *old_path, const char *new_path);

diff --git a/src/basic/mkdir-label.c b/src/basic/mkdir-label.c
index 2a44d276cb14d906d42495760996874d04700188..9d4ef60e77840ba8aba7b5c2ae5c9a2e0d38f91e 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 0c6e99b1d7df6bbe111e76af4f694196f29e87e2..c60057f4f7bf4b49dda0aaa52fce3ba82b691e59 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 9780dca81e8d5981d388852a0d7be50f8fe0335c..1cb4ea4fe0cf41005399877efbdb8d6ecbfa9801 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);
 int mac_selinux_apply(const char *path, const char *label);
 
 int mac_selinux_get_create_label_from_exe(const char *exe, char **label);

diff --git a/src/basic/smack-util.c b/src/basic/smack-util.c
index f0018f013f793581ebd05e63285f005c310e8bfe..977de014f7a7be5ec52bf7655b71307b97c0dfc9 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"
 
@@ -134,7 +137,10 @@ int mac_smack_apply_pid(pid_t pid, const char *label) {
         return r;
 }
 
-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;
 
@@ -143,50 +149,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;
 }
 
 int mac_smack_copy(const char *dest, const char *src) {

diff --git a/src/basic/smack-util.h b/src/basic/smack-util.h
index e4d46d7736ca40bb7435b3ee37c73953a7d45f9b..58e6a548b620aed9b2b4e6cec2da6b2e56c5c1c8 100644 (file)
--- a/src/basic/smack-util.h
+++ b/src/basic/smack-util.h
@@ -25,25 +25,26 @@
 #include <stdbool.h>
 #include <sys/types.h>
 
+#include "label.h"
 #include "macro.h"
 
 #define SMACK_FLOOR_LABEL "_"
 #define SMACK_STAR_LABEL  "*"
 
 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;
 
 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);
 
 const char* smack_attr_to_string(SmackAttr i) _const_;
 SmackAttr smack_attr_from_string(const char *s) _pure_;

diff --git a/src/core/automount.c b/src/core/automount.c
index a8d773686b02572819ddedb6d6ab03e76447916b..bcc6b0b04e7e33ab5cee598a142bc96ed108d11b 100644 (file)
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -346,7 +346,7 @@ static int open_dev_autofs(Manager *m) {
         if (m->dev_autofs_fd >= 0)
                 return m->dev_autofs_fd;
 
-        label_fix("/dev/autofs", false, false);
+        (void) label_fix("/dev/autofs", 0);
 
         m->dev_autofs_fd = open("/dev/autofs", O_CLOEXEC|O_RDONLY);
         if (m->dev_autofs_fd < 0)

diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c
index b7d654619cc2d002719e657cfd31ff1984b39c23..0fe794d2afd90f6d253eaaef1d2e28110d5ee195 100644 (file)
--- a/src/core/mount-setup.c
+++ b/src/core/mount-setup.c
@@ -168,7 +168,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) {
@@ -206,7 +206,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) {
@@ -374,7 +374,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... */
@@ -413,7 +413,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/hwdb/hwdb.c b/src/hwdb/hwdb.c
index f579ef737efda3829f51e0ee998328350f975e8c..a60809ca654512aee043f910aaeec07ef9e6b8da 100644 (file)
--- a/src/hwdb/hwdb.c
+++ b/src/hwdb/hwdb.c
@@ -688,7 +688,7 @@ static int hwdb_update(int argc, char *argv[], void *userdata) {
         if (r < 0)
                 return log_error_errno(r, "Failure writing database %s: %m", hwdb_bin);
 
-        return label_fix(hwdb_bin, false, false);
+        return label_fix(hwdb_bin, 0);
 }
 
 static void help(void) {

diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index dc8feacbf979aa318c58429fe347e81afcc61925..28574f49a208d57c7e086b08907d0b3915da2670 100644 (file)
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -368,7 +368,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);
         }

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index b56b7ac963c48aff0d9241012d6d2a0d838445ca..61e76570b1524d125308869755ad3a2c5e7d8541 100644 (file)
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -850,7 +850,7 @@ static int fd_set_perms(Item *i, int fd, const struct stat *st) {
         }
 
 shortcut:
-        return label_fix(path, false, false);
+        return label_fix(path, 0);
 }
 
 static int path_set_perms(Item *i, const char *path) {

diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c
index 6a3ee93ca2b4c813c083e8e5749c97c7577da0d5..755bf665abfa662728fb86052494332b00441435 100644 (file)
--- a/src/udev/udev-node.c
+++ b/src/udev/udev-node.c
@@ -79,7 +79,7 @@ static int node_symlink(struct udev_device *dev, const char *node, const char *s
                                 buf[len] = '\0';
                                 if (streq(target, buf)) {
                                         log_debug("preserve already existing symlink '%s' to '%s'", slink, target);
-                                        label_fix(slink, true, false);
+                                        (void) label_fix(slink, LABEL_IGNORE_ENOENT);
                                         utimensat(AT_FDCWD, slink, NULL, AT_SYMLINK_NOFOLLOW);
                                         goto exit;
                                 }
@@ -324,7 +324,7 @@ static int node_permissions_apply(struct udev_device *dev, bool apply,
 
                 /* set the defaults */
                 if (!selinux)
-                        mac_selinux_fix(devnode, true, false);
+                        (void) mac_selinux_fix(devnode, LABEL_IGNORE_ENOENT);
                 if (!smack)
                         mac_smack_apply(devnode, SMACK_ATTR_ACCESS, NULL);
         }

diff --git a/src/udev/udevadm-hwdb.c b/src/udev/udevadm-hwdb.c
index dc3ae7484df1c876b2181d492c7decf63d87b08f..3a5b138b31a66ca73c603404eaa5b894ec7396db 100644 (file)
--- a/src/udev/udevadm-hwdb.c
+++ b/src/udev/udevadm-hwdb.c
@@ -680,7 +680,7 @@ static int adm_hwdb(struct udev *udev, int argc, char *argv[]) {
                         rc = EXIT_FAILURE;
                 }
 
-                label_fix(hwdb_bin, false, false);
+                (void) label_fix(hwdb_bin, 0);
         }
 
         if (test) {