From: Zbigniew Jędrzejewski-Szmek Date: Wed, 24 Apr 2024 07:33:25 +0000 (+0200) Subject: core: drop unused param, move taint calculation to separate file X-Git-Tag: v256-rc1~24^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d851637ca627ed8d312a3957a5e109f6ef7e2a3b;p=thirdparty%2Fsystemd.git core: drop unused param, move taint calculation to separate file Follow-up for 2b28dfe6e632f47a9058d9378fb88a0c99b34a91. I also considered moving the function to src/basic, but since it's only used by the manager, it doesn't seem useful. --- diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 397919dd300..7a969ce6c15 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -40,6 +40,7 @@ #include "string-util.h" #include "strv.h" #include "syslog-util.h" +#include "taint.h" #include "user-util.h" #include "version.h" #include "virt.h" @@ -126,13 +127,10 @@ static int property_get_tainted( void *userdata, sd_bus_error *error) { - _cleanup_free_ char *s = NULL; - Manager *m = ASSERT_PTR(userdata); - assert(bus); assert(reply); - s = manager_taint_string(m); + _cleanup_free_ char *s = taint_string(); if (!s) return log_oom(); diff --git a/src/core/manager.c b/src/core/manager.c index ebaf33bc5f6..d429ae7d00e 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -9,7 +9,6 @@ #include #include #include -#include #include #include @@ -90,6 +89,7 @@ #include "strxcpyx.h" #include "sysctl-util.h" #include "syslog-util.h" +#include "taint.h" #include "terminal-util.h" #include "time-util.h" #include "transaction.h" @@ -3683,8 +3683,6 @@ bool manager_unit_inactive_or_pending(Manager *m, const char *name) { } static void log_taint_string(Manager *m) { - _cleanup_free_ char *taint = NULL; - assert(m); if (MANAGER_IS_USER(m) || m->taint_logged) @@ -3692,7 +3690,7 @@ static void log_taint_string(Manager *m) { m->taint_logged = true; /* only check for taint once */ - taint = manager_taint_string(m); + _cleanup_free_ char *taint = taint_string(); if (isempty(taint)) return; @@ -4817,79 +4815,6 @@ int manager_dispatch_user_lookup_fd(sd_event_source *source, int fd, uint32_t re return 0; } -static int short_uid_range(const char *path) { - _cleanup_(uid_range_freep) UIDRange *p = NULL; - int r; - - assert(path); - - /* Taint systemd if we the UID range assigned to this environment doesn't at least cover 0…65534, - * i.e. from root to nobody. */ - - r = uid_range_load_userns(path, UID_RANGE_USERNS_INSIDE, &p); - if (ERRNO_IS_NEG_NOT_SUPPORTED(r)) - return false; - if (r < 0) - return log_debug_errno(r, "Failed to load %s: %m", path); - - return !uid_range_covers(p, 0, 65535); -} - -char* manager_taint_string(const Manager *m) { - const char *stage[12] = {}; - size_t n = 0; - - /* Returns a "taint string", e.g. "local-hwclock:var-run-bad". Only things that are detected at - * runtime should be tagged here. For stuff that is known during compilation, emit a warning in the - * configuration phase. */ - - assert(m); - - _cleanup_free_ char *bin = NULL, *usr_sbin = NULL, *var_run = NULL; - - if (readlink_malloc("/bin", &bin) < 0 || !PATH_IN_SET(bin, "usr/bin", "/usr/bin")) - stage[n++] = "unmerged-usr"; - - /* Note that the check is different from default_PATH(), as we want to taint on uncanonical symlinks - * too. */ - if (readlink_malloc("/usr/sbin", &usr_sbin) < 0 || !PATH_IN_SET(usr_sbin, "bin", "/usr/bin")) - stage[n++] = "unmerged-bin"; - - if (readlink_malloc("/var/run", &var_run) < 0 || !PATH_IN_SET(var_run, "../run", "/run")) - stage[n++] = "var-run-bad"; - - if (cg_all_unified() == 0) - stage[n++] = "cgroupsv1"; - - if (clock_is_localtime(NULL) > 0) - stage[n++] = "local-hwclock"; - - if (os_release_support_ended(NULL, /* quiet= */ true, NULL) > 0) - stage[n++] = "support-ended"; - - struct utsname uts; - assert_se(uname(&uts) >= 0); - if (strverscmp_improved(uts.release, KERNEL_BASELINE_VERSION) < 0) - stage[n++] = "old-kernel"; - - _cleanup_free_ char *overflowuid = NULL, *overflowgid = NULL; - if (read_one_line_file("/proc/sys/kernel/overflowuid", &overflowuid) >= 0 && - !streq(overflowuid, "65534")) - stage[n++] = "overflowuid-not-65534"; - if (read_one_line_file("/proc/sys/kernel/overflowgid", &overflowgid) >= 0 && - !streq(overflowgid, "65534")) - stage[n++] = "overflowgid-not-65534"; - - if (short_uid_range("/proc/self/uid_map") > 0) - stage[n++] = "short-uid-range"; - if (short_uid_range("/proc/self/gid_map") > 0) - stage[n++] = "short-gid-range"; - - assert(n < ELEMENTSOF(stage) - 1); /* One extra for NULL terminator */ - - return strv_join((char**) stage, ":"); -} - void manager_ref_console(Manager *m) { assert(m); diff --git a/src/core/manager.h b/src/core/manager.h index c4ad42293e5..f0f814f940c 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -616,8 +616,6 @@ int manager_ref_uid(Manager *m, uid_t uid, bool clean_ipc); void manager_unref_gid(Manager *m, gid_t gid, bool destroy_now); int manager_ref_gid(Manager *m, gid_t gid, bool clean_ipc); -char* manager_taint_string(const Manager *m); - void manager_ref_console(Manager *m); void manager_unref_console(Manager *m); diff --git a/src/core/meson.build b/src/core/meson.build index 06ca1449549..7a2012a372d 100644 --- a/src/core/meson.build +++ b/src/core/meson.build @@ -61,6 +61,7 @@ libcore_sources = files( 'smack-setup.c', 'socket.c', 'swap.c', + 'taint.c', 'target.c', 'timer.c', 'transaction.c', diff --git a/src/core/taint.c b/src/core/taint.c new file mode 100644 index 00000000000..c9f154b0b55 --- /dev/null +++ b/src/core/taint.c @@ -0,0 +1,87 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include + +#include "alloc-util.h" +#include "cgroup-util.h" +#include "clock-util.h" +#include "errno-util.h" +#include "fileio.h" +#include "fs-util.h" +#include "log.h" +#include "os-util.h" +#include "path-util.h" +#include "strv.h" +#include "taint.h" +#include "uid-range.h" + +static int short_uid_range(const char *path) { + _cleanup_(uid_range_freep) UIDRange *p = NULL; + int r; + + assert(path); + + /* Taint systemd if we the UID range assigned to this environment doesn't at least cover 0…65534, + * i.e. from root to nobody. */ + + r = uid_range_load_userns(path, UID_RANGE_USERNS_INSIDE, &p); + if (ERRNO_IS_NEG_NOT_SUPPORTED(r)) + return false; + if (r < 0) + return log_debug_errno(r, "Failed to load %s: %m", path); + + return !uid_range_covers(p, 0, 65535); +} + +char* taint_string(void) { + const char *stage[12] = {}; + size_t n = 0; + + /* Returns a "taint string", e.g. "local-hwclock:var-run-bad". Only things that are detected at + * runtime should be tagged here. For stuff that is known during compilation, emit a warning in the + * configuration phase. */ + + _cleanup_free_ char *bin = NULL, *usr_sbin = NULL, *var_run = NULL; + + if (readlink_malloc("/bin", &bin) < 0 || !PATH_IN_SET(bin, "usr/bin", "/usr/bin")) + stage[n++] = "unmerged-usr"; + + /* Note that the check is different from default_PATH(), as we want to taint on uncanonical symlinks + * too. */ + if (readlink_malloc("/usr/sbin", &usr_sbin) < 0 || !PATH_IN_SET(usr_sbin, "bin", "/usr/bin")) + stage[n++] = "unmerged-bin"; + + if (readlink_malloc("/var/run", &var_run) < 0 || !PATH_IN_SET(var_run, "../run", "/run")) + stage[n++] = "var-run-bad"; + + if (cg_all_unified() == 0) + stage[n++] = "cgroupsv1"; + + if (clock_is_localtime(NULL) > 0) + stage[n++] = "local-hwclock"; + + if (os_release_support_ended(NULL, /* quiet= */ true, NULL) > 0) + stage[n++] = "support-ended"; + + struct utsname uts; + assert_se(uname(&uts) >= 0); + if (strverscmp_improved(uts.release, KERNEL_BASELINE_VERSION) < 0) + stage[n++] = "old-kernel"; + + _cleanup_free_ char *overflowuid = NULL, *overflowgid = NULL; + if (read_one_line_file("/proc/sys/kernel/overflowuid", &overflowuid) >= 0 && + !streq(overflowuid, "65534")) + stage[n++] = "overflowuid-not-65534"; + if (read_one_line_file("/proc/sys/kernel/overflowgid", &overflowgid) >= 0 && + !streq(overflowgid, "65534")) + stage[n++] = "overflowgid-not-65534"; + + if (short_uid_range("/proc/self/uid_map") > 0) + stage[n++] = "short-uid-range"; + if (short_uid_range("/proc/self/gid_map") > 0) + stage[n++] = "short-gid-range"; + + assert(n < ELEMENTSOF(stage) - 1); /* One extra for NULL terminator */ + + return strv_join((char**) stage, ":"); +} diff --git a/src/core/taint.h b/src/core/taint.h new file mode 100644 index 00000000000..2e514e33352 --- /dev/null +++ b/src/core/taint.h @@ -0,0 +1,4 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +char* taint_string(void); diff --git a/src/test/meson.build b/src/test/meson.build index 6419edda490..69b2b1e5878 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -535,7 +535,7 @@ executables += [ 'parallel' : false, }, core_test_template + { - 'sources' : files('test-manager.c'), + 'sources' : files('test-taint.c'), }, core_test_template + { 'sources' : files('test-namespace.c'), diff --git a/src/test/test-manager.c b/src/test/test-manager.c deleted file mode 100644 index 76e094bf012..00000000000 --- a/src/test/test-manager.c +++ /dev/null @@ -1,19 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ - -#include "manager.h" -#include "tests.h" - -TEST(manager_taint_string) { - Manager m = {}; - - _cleanup_free_ char *a = manager_taint_string(&m); - assert_se(a); - log_debug("taint string: '%s'", a); - - if (cg_all_unified() == 0) - assert_se(strstr(a, "cgroupsv1")); - else - assert_se(!strstr(a, "cgroupsv1")); -} - -DEFINE_TEST_MAIN(LOG_DEBUG); diff --git a/src/test/test-taint.c b/src/test/test-taint.c new file mode 100644 index 00000000000..d9aa79dca57 --- /dev/null +++ b/src/test/test-taint.c @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "taint.h" +#include "tests.h" + +TEST(taint_string) { + _cleanup_free_ char *a = taint_string(); + assert_se(a); + log_debug("taint string: '%s'", a); + + assert_se(!!strstr(a, "cgroupsv1") == (cg_all_unified() == 0)); +} + +DEFINE_TEST_MAIN(LOG_DEBUG);