From af6b0ecc4c15ba2dbb492581e9e1870bd0af8a59 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 7 Dec 2017 11:27:07 +0100 Subject: [PATCH 1/5] core: make "taint" string logic a bit more generic and output it at boot The tainting logic existed for a long time, but was hidden inside the bus interfaces. Let's give it a small bit more coverage, by logging its value early at boot during initialization. --- src/core/dbus-manager.c | 26 +++++------------------- src/core/main.c | 8 ++++++++ src/core/manager.c | 45 +++++++++++++++++++++++++++++++++++++++++ src/core/manager.h | 2 ++ 4 files changed, 60 insertions(+), 21 deletions(-) diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index e5b899fc01..ec9e65879f 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -27,7 +27,6 @@ #include "architecture.h" #include "build.h" #include "bus-common-errors.h" -#include "clock-util.h" #include "dbus-execute.h" #include "dbus-job.h" #include "dbus-manager.h" @@ -140,33 +139,18 @@ static int property_get_tainted( void *userdata, sd_bus_error *error) { - char buf[sizeof("split-usr:cgroups-missing:local-hwclock:var-run-bad:")] = "", *e = buf; - _cleanup_free_ char *destination = NULL; + _cleanup_free_ char *s = NULL; Manager *m = userdata; - int r; assert(bus); assert(reply); assert(m); - if (m->taint_usr) - e = stpcpy(e, "split-usr:"); + s = manager_taint_string(m); + if (!s) + return log_oom(); - if (access("/proc/cgroups", F_OK) < 0) - e = stpcpy(e, "cgroups-missing:"); - - if (clock_is_localtime(NULL) > 0) - e = stpcpy(e, "local-hwclock:"); - - r = readlink_malloc("/var/run", &destination); - if (r < 0 || !PATH_IN_SET(destination, "../run", "/run")) - e = stpcpy(e, "var-run-bad:"); - - /* remove the last ':' */ - if (e != buf) - e[-1] = 0; - - return sd_bus_message_append(reply, "s", buf); + return sd_bus_message_append(reply, "s", s); } static int property_get_log_target( diff --git a/src/core/main.c b/src/core/main.c index 36a29d9ac6..87b116dd03 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2394,6 +2394,14 @@ int main(int argc, char *argv[]) { "Loaded units and determined initial transaction in %s.", format_timespan(timespan, sizeof(timespan), after_startup - before_startup, 100 * USEC_PER_MSEC)); + if (arg_system) { + _cleanup_free_ char *taint; + + taint = manager_taint_string(m); + if (!isempty(taint)) + log_notice("System is tainted: %s", taint); + } + if (arg_action == ACTION_TEST) { printf("-> By units:\n"); manager_dump_units(m, stdout, "\t"); diff --git a/src/core/manager.c b/src/core/manager.c index be44ab3778..4e0f73f4f8 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -48,6 +48,7 @@ #include "bus-kernel.h" #include "bus-util.h" #include "clean-ipc.h" +#include "clock-util.h" #include "dbus-job.h" #include "dbus-manager.h" #include "dbus-unit.h" @@ -3851,6 +3852,50 @@ int manager_dispatch_user_lookup_fd(sd_event_source *source, int fd, uint32_t re return 0; } +char *manager_taint_string(Manager *m) { + _cleanup_free_ char *destination = NULL; + char *buf, *e; + int r; + + assert(m); + + buf = new(char, sizeof("split-usr:" + "cgroups-missing:" + "local-hwclock:" + "var-run-bad:" + "weird-nobody-user:" + "weird-nobody-group:")); + if (!buf) + return NULL; + + e = buf; + + if (m->taint_usr) + e = stpcpy(e, "split-usr:"); + + if (access("/proc/cgroups", F_OK) < 0) + e = stpcpy(e, "cgroups-missing:"); + + if (clock_is_localtime(NULL) > 0) + e = stpcpy(e, "local-hwclock:"); + + r = readlink_malloc("/var/run", &destination); + if (r < 0 || !PATH_IN_SET(destination, "../run", "/run")) + e = stpcpy(e, "var-run-bad:"); + + if (!streq(NOBODY_USER_NAME, "nobody")) + e = stpcpy(e, "weird-nobody-user:"); + + if (!streq(NOBODY_GROUP_NAME, "nobody")) + e = stpcpy(e, "weird-nobody-group:"); + + /* remove the last ':' */ + if (e != buf) + e[-1] = 0; + + return buf; +} + static const char *const manager_state_table[_MANAGER_STATE_MAX] = { [MANAGER_INITIALIZING] = "initializing", [MANAGER_STARTING] = "starting", diff --git a/src/core/manager.h b/src/core/manager.h index 779e09bc97..902af2609d 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -435,6 +435,8 @@ void manager_deserialize_uid_refs_one(Manager *m, const char *value); void manager_serialize_gid_refs(Manager *m, FILE *f); void manager_deserialize_gid_refs_one(Manager *m, const char *value); +char *manager_taint_string(Manager *m); + const char *manager_state_to_string(ManagerState m) _const_; ManagerState manager_state_from_string(const char *s) _pure_; From cefb3eda603eafb3546694a15ff59edc556e09e3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 7 Dec 2017 11:28:00 +0100 Subject: [PATCH 2/5] core: add console error message if manager_startup() fails --- src/core/main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/main.c b/src/core/main.c index 87b116dd03..08d9188385 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2375,6 +2375,7 @@ int main(int argc, char *argv[]) { r = manager_startup(m, arg_serialization, fds); if (r < 0) { log_error_errno(r, "Failed to fully start up daemon: %m"); + error_message = "Failed to start up manager"; goto finish; } From 90d7464d8300aec62799805e21eda7546b5843aa Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 7 Dec 2017 11:35:02 +0100 Subject: [PATCH 3/5] manager: taint the manager if the overflowuid/overflowgid aren't set to 65534 --- src/core/manager.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 4e0f73f4f8..a773c1e94b 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3853,7 +3853,7 @@ int manager_dispatch_user_lookup_fd(sd_event_source *source, int fd, uint32_t re } char *manager_taint_string(Manager *m) { - _cleanup_free_ char *destination = NULL; + _cleanup_free_ char *destination = NULL, *overflowuid = NULL, *overflowgid = NULL; char *buf, *e; int r; @@ -3864,7 +3864,9 @@ char *manager_taint_string(Manager *m) { "local-hwclock:" "var-run-bad:" "weird-nobody-user:" - "weird-nobody-group:")); + "weird-nobody-group:" + "overflowuid-not-65534:" + "overflowgid-not-65534:")); if (!buf) return NULL; @@ -3889,6 +3891,14 @@ char *manager_taint_string(Manager *m) { if (!streq(NOBODY_GROUP_NAME, "nobody")) e = stpcpy(e, "weird-nobody-group:"); + r = read_one_line_file("/proc/sys/kernel/overflowuid", &overflowuid); + if (r >= 0 && !streq(overflowuid, "65534")) + e = stpcpy(e, "overflowuid-not-65534:"); + + r = read_one_line_file("/proc/sys/kernel/overflowgid", &overflowgid); + if (r >= 0 && !streq(overflowgid, "65534")) + e = stpcpy(e, "overflowgid-not-65534:"); + /* remove the last ':' */ if (e != buf) e[-1] = 0; From 8fd010bb1b232f8d7afd9ae6b96d844d9f65ea88 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 7 Dec 2017 11:58:25 +0100 Subject: [PATCH 4/5] nspawn: turn on watchdog logic for nspawn too It's a long-running daemon, and it's easy to enable, hence do it. --- src/nspawn/nspawn.c | 2 ++ units/systemd-nspawn@.service.in | 1 + 2 files changed, 3 insertions(+) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 4b82345d08..f217def92d 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3642,6 +3642,8 @@ static int run(int master, if (r < 0) return log_error_errno(r, "Failed to get default event source: %m"); + (void) sd_event_set_watchdog(event, true); + if (bus) { r = sd_bus_attach_event(bus, event, 0); if (r < 0) diff --git a/units/systemd-nspawn@.service.in b/units/systemd-nspawn@.service.in index 3d3590751f..c3194d4f21 100644 --- a/units/systemd-nspawn@.service.in +++ b/units/systemd-nspawn@.service.in @@ -21,6 +21,7 @@ KillMode=mixed Type=notify RestartForceExitStatus=133 SuccessExitStatus=133 +WatchdogSec=3min Slice=machine.slice Delegate=yes TasksMax=16384 From 38edd43d48259b4a5c1197270e84ee955c764b0f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 7 Dec 2017 11:58:58 +0100 Subject: [PATCH 5/5] update TODO --- TODO | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/TODO b/TODO index 0f9b28ab49..9f8327ca28 100644 --- a/TODO +++ b/TODO @@ -33,8 +33,7 @@ Features: systemd-journald writes to /var/log/journal, which could be useful when we doing disk usage calculations and so on. -* taint systemd if the overflowuid/overflowgid is not 65534, and if there are - fewer than 65536 users assigned to the system. +* taint systemd if there are fewer than 65536 users assigned to the system. * deprecate PermissionsStartOnly= and RootDirectoryStartOnly= in favour of the ExecStart= prefix chars @@ -49,8 +48,6 @@ Features: * support projid-based quota in machinectl for containers, and then drop implicit btrfs loopback magic in machined -* let's log the "tainted" string at boot - * Add NetworkNamespacePath= to specify a path to a network namespace * maybe use SOURCE_DATE_EPOCH (i.e. the env var the reproducible builds folks @@ -117,9 +114,6 @@ Features: * expose IO accounting data on the bus, show it in systemd-run --wait and log about it in the resource log message -* rework unbase64 code to drop whitespace automatically, so that we don't have - to drop it first. - * add "systemctl purge" for flushing out configuration, state, logs, ... of a unit when it is stopped @@ -196,8 +190,6 @@ Features: partition, that is mounted to / and is writable, and where the actual root's /usr is mounted into. -* .mount and .swap units: add Format=yes|no option that formats the partition before mounting/enabling it, implicitly - * gpt-auto logic: support encrypted swap, add kernel cmdline option to force it, and honour a gpt bit about it, plus maybe a configuration file * drop nss-myhostname in favour of nss-resolve? @@ -413,8 +405,6 @@ Features: * figure out a nice way how we can let the admin know what child/sibling unit causes cgroup membership for a specific unit -* mount_cgroup_controllers(): symlinks need to get the label applied - * For timer units: add some mechanisms so that timer units that trigger immediately on boot do not have the services they run added to the initial transaction and thus confuse Type=idle. @@ -728,7 +718,6 @@ Features: https://github.com/systemd/systemd/pull/272#issuecomment-113153176 - should optionally support receiving WATCHDOG=1 messages from its payload PID 1... - - should send out sd_notify("WATCHDOG=1") messages - optionally automatically add FORWARD rules to iptables whenever nspawn is running, remove them when shut down. - maybe make copying of /etc/resolv.conf optional, and skip it if --read-only