From af614e45c3efb49e37c4b1507ccc4e450119c089 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 7 Dec 2022 15:20:15 +0100 Subject: [PATCH 1/2] selinux: accept the fact that getxyzcon() can return success and NULL Inspired by #25664: let's check explicitly for NULL everywhere we do one of those getXYZcon() calls. We usually turn this into EOPNOTSUPP, as when selinux is off (which is supposed to be the only case this can happen according to selinux docs) we otherwise return EOPNOTSUPP in that case. Note that in most cases we have an explicit mac_selinux_use() call beforehand, hence this should mostly not be triggerable codepaths. --- src/core/selinux-access.c | 10 +++++++--- src/shared/selinux-util.c | 30 ++++++++++++++++++++++++------ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c index e05ebdc631..62181a6309 100644 --- a/src/core/selinux-access.c +++ b/src/core/selinux-access.c @@ -229,9 +229,7 @@ int mac_selinux_access_check_internal( } else { /* If no unit context is known, use our own */ if (getcon_raw(&fcon) < 0) { - r = -errno; - - log_warning_errno(r, "SELinux getcon_raw() failed%s (perm=%s): %m", + log_warning_errno(errno, "SELinux getcon_raw() failed%s (perm=%s): %m", enforce ? "" : ", ignoring", permission); if (!enforce) @@ -239,6 +237,12 @@ int mac_selinux_access_check_internal( return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to get current context: %m"); } + if (!fcon) { + if (!enforce) + return 0; + + return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "We appear not to have any SELinux context: %m"); + } acon = fcon; tclass = "system"; diff --git a/src/shared/selinux-util.c b/src/shared/selinux-util.c index f7d8353b49..e240cdc2c3 100644 --- a/src/shared/selinux-util.c +++ b/src/shared/selinux-util.c @@ -277,7 +277,7 @@ static int selinux_fix_fd( return 0; /* If the old label is identical to the new one, suppress any kind of error */ - if (getfilecon_raw(FORMAT_PROC_FD_PATH(fd), &oldcon) >= 0 && streq(fcon, oldcon)) + if (getfilecon_raw(FORMAT_PROC_FD_PATH(fd), &oldcon) >= 0 && streq_ptr(fcon, oldcon)) return 0; return log_enforcing_errno(r, "Unable to fix SELinux security context of %s: %m", label_path); @@ -381,9 +381,13 @@ int mac_selinux_get_create_label_from_exe(const char *exe, char **label) { if (getcon_raw(&mycon) < 0) return -errno; + if (!mycon) + return -EOPNOTSUPP; if (getfilecon_raw(exe, &fcon) < 0) return -errno; + if (!fcon) + return -EOPNOTSUPP; sclass = string_to_security_class("process"); if (sclass == 0) @@ -395,14 +399,21 @@ int mac_selinux_get_create_label_from_exe(const char *exe, char **label) { #endif } -int mac_selinux_get_our_label(char **label) { -#if HAVE_SELINUX - assert(label); +int mac_selinux_get_our_label(char **ret) { + assert(ret); +#if HAVE_SELINUX if (!mac_selinux_use()) return -EOPNOTSUPP; - return RET_NERRNO(getcon_raw(label)); + _cleanup_freecon_ char *con = NULL; + if (getcon_raw(&con) < 0) + return -errno; + if (!con) + return -EOPNOTSUPP; + + *ret = TAKE_PTR(con); + return 0; #else return -EOPNOTSUPP; #endif @@ -424,13 +435,20 @@ int mac_selinux_get_child_mls_label(int socket_fd, const char *exe, const char * if (getcon_raw(&mycon) < 0) return -errno; + if (!mycon) + return -EOPNOTSUPP; if (getpeercon_raw(socket_fd, &peercon) < 0) return -errno; + if (!peercon) + return -EOPNOTSUPP; - if (!exec_label) /* If there is no context set for next exec let's use context of target executable */ + if (!exec_label) { /* If there is no context set for next exec let's use context of target executable */ if (getfilecon_raw(exe, &fcon) < 0) return -errno; + if (!fcon) + return -EOPNOTSUPP; + } bcon = context_new(mycon); if (!bcon) From 4a69c2c748bcadb3c6d323b1cc6c7163ca6980b6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 7 Dec 2022 15:23:02 +0100 Subject: [PATCH 2/2] selinux-setup: minor modernizations --- src/core/selinux-setup.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/core/selinux-setup.c b/src/core/selinux-setup.c index 153322442c..17fa2c0142 100644 --- a/src/core/selinux-setup.c +++ b/src/core/selinux-setup.c @@ -30,30 +30,28 @@ int mac_selinux_setup(bool *loaded_policy) { usec_t before_load, after_load; char *con; int r; - bool initialized = false; + bool initialized; assert(loaded_policy); /* Turn off all of SELinux' own logging, we want to do that */ - selinux_set_callback(SELINUX_CB_LOG, (union selinux_callback) { .func_log = null_log }); + selinux_set_callback(SELINUX_CB_LOG, (const union selinux_callback) { .func_log = null_log }); - /* Don't load policy in the initrd if we don't appear to have - * it. For the real root, we check below if we've already - * loaded policy, and return gracefully. - */ + /* Don't load policy in the initrd if we don't appear to have it. For the real root, we check below + * if we've already loaded policy, and return gracefully. */ if (in_initrd() && access(selinux_path(), F_OK) < 0) return 0; /* Already initialized by somebody else? */ r = getcon_raw(&con); - /* getcon_raw can return 0, and still give us a NULL pointer if - * /proc/self/attr/current is empty. SELinux guarantees this won't - * happen, but that file isn't specific to SELinux, and may be provided - * by some other arbitrary LSM with different semantics. */ + /* getcon_raw can return 0, and still give us a NULL pointer if /proc/self/attr/current is + * empty. SELinux guarantees this won't happen, but that file isn't specific to SELinux, and may be + * provided by some other arbitrary LSM with different semantics. */ if (r == 0 && con) { initialized = !streq(con, "kernel"); freecon(con); - } + } else + initialized = false; /* Make sure we have no fds open while loading the policy and * transitioning */