From 854eca4a95993bb1bd77a18de39efe1ed1a44bbd Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 7 Oct 2023 20:08:21 +0800 Subject: [PATCH] core/execute: always set $USER and introduce SetLoginEnvironment= Before this commit, $USER, $HOME, $LOGNAME and $SHELL are only set when User= is set for the unit. For system service, this results in different behaviors depending on whether User=root is set. $USER always makes sense on its own, so let's set it unconditionally. Ideally $HOME should be set too, but it causes trouble when e.g. getty passes '-p' to login(1), which then doesn't override $HOME. $LOGNAME and $SHELL are more like "login environments", and are generally not suitable for system services. Therefore, a new option SetLoginEnvironment= is also added to control the latter three variables. Fixes #23438 Replaces #8227 --- man/org.freedesktop.systemd1.xml | 38 +++++++++++++++++++---- man/systemd.exec.xml | 21 ++++++++++--- src/core/dbus-execute.c | 4 +++ src/core/execute.c | 43 ++++++++++++++++++--------- src/core/execute.h | 2 ++ src/core/load-fragment-gperf.gperf.in | 1 + src/shared/bus-unit-util.c | 3 +- 7 files changed, 88 insertions(+), 24 deletions(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 01d8f659d50..7247f3d2fc6 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -3117,6 +3117,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly b DynamicUser = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") + readonly b SetLoginEnvironment = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly b RemoveIPC = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly a(say) SetCredential = [...]; @@ -3710,6 +3712,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + @@ -4368,6 +4372,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + @@ -5160,6 +5166,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2esocket { @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly b DynamicUser = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") + readonly b SetLoginEnvironment = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly b RemoveIPC = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly a(say) SetCredential = [...]; @@ -5763,6 +5771,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2esocket { + + @@ -6403,6 +6413,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2esocket { + + @@ -7069,6 +7081,8 @@ node /org/freedesktop/systemd1/unit/home_2emount { @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly b DynamicUser = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") + readonly b SetLoginEnvironment = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly b RemoveIPC = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly a(say) SetCredential = [...]; @@ -7600,6 +7614,8 @@ node /org/freedesktop/systemd1/unit/home_2emount { + + @@ -8154,6 +8170,8 @@ node /org/freedesktop/systemd1/unit/home_2emount { + + @@ -8943,6 +8961,8 @@ node /org/freedesktop/systemd1/unit/dev_2dsda3_2eswap { @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly b DynamicUser = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") + readonly b SetLoginEnvironment = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly b RemoveIPC = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly a(say) SetCredential = [...]; @@ -9460,6 +9480,8 @@ node /org/freedesktop/systemd1/unit/dev_2dsda3_2eswap { + + @@ -10000,6 +10022,8 @@ node /org/freedesktop/systemd1/unit/dev_2dsda3_2eswap { + + @@ -11650,7 +11674,8 @@ $ gdbus introspect --system --dest org.freedesktop.systemd1 \ RootImagePolicy, MountImagePolicy, and ExtensionImagePolicy were added in version 254. - NFTSet was added in version 255. + NFTSet and + SetLoginEnvironment were added in version 255. Socket Unit Objects @@ -11674,8 +11699,9 @@ $ gdbus introspect --system --dest org.freedesktop.systemd1 \ MountImagePolicy, and ExtensionImagePolicy were added in version 254. PollLimitIntervalUSec, - PollLimitBurst, and - NFTSet were added in version 255. + PollLimitBurst, + NFTSet, and + SetLoginEnvironment were added in version 255. Mount Unit Objects @@ -11698,7 +11724,8 @@ $ gdbus introspect --system --dest org.freedesktop.systemd1 \ RootImagePolicy, MountImagePolicy, and ExtensionImagePolicy were added in version 254. - NFTSet was added in version 255. + NFTSet and + SetLoginEnvironment were added in version 255. Swap Unit Objects @@ -11721,7 +11748,8 @@ $ gdbus introspect --system --dest org.freedesktop.systemd1 \ RootImagePolicy, MountImagePolicy, and ExtensionImagePolicy were added in version 254. - NFTSet was added in version 255. + NFTSet and + SetLoginEnvironment were added in version 255. Slice Unit Objects diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index 1988d624cf0..e648aeedaa0 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -696,6 +696,19 @@ commands prefixed with +. + + SetLoginEnvironment= + + Takes a boolean parameter. If true, $HOME, $LOGNAME, + and $SHELL environment variables will be set for system services even if + User= is not set, i.e. when the default user root is used. + If false, the mentioned variables are not set by systemd, no matter whether User= + is set or not. This option normally has no effect on user services, since these variables are typically + inherited from user manager's own environment anyway. + + + + PAMName= @@ -3661,10 +3674,10 @@ StandardInputData=V2XigLJyZSBubyBzdHJhbmdlcnMgdG8gbG92ZQpZb3Uga25vdyB0aGUgcnVsZX $HOME $SHELL - User name (twice), home directory, and the - login shell. The variables are set for the units that have - User= set, which includes user - systemd instances. See + User name (twice), home directory, and the login shell. $USER is + set unconditionally, while $HOME, $LOGNAME, and $SHELL + are only set for the units that have User= set and SetLoginEnvironment= + unset or set to true. For user services, these variables are typically inherited from the user manager itself. See passwd5. diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 571d8c83980..74322645dec 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1273,6 +1273,7 @@ const sd_bus_vtable bus_exec_vtable[] = { SD_BUS_PROPERTY("User", "s", NULL, offsetof(ExecContext, user), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Group", "s", NULL, offsetof(ExecContext, group), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("DynamicUser", "b", bus_property_get_bool, offsetof(ExecContext, dynamic_user), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("SetLoginEnvironment", "b", bus_property_get_tristate, offsetof(ExecContext, set_login_environment), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RemoveIPC", "b", bus_property_get_bool, offsetof(ExecContext, remove_ipc), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("SetCredential", "a(say)", property_get_set_credential, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("SetCredentialEncrypted", "a(say)", property_get_set_credential, 0, SD_BUS_VTABLE_PROPERTY_CONST), @@ -1730,6 +1731,9 @@ int bus_exec_context_set_transient_property( if (streq(name, "Group")) return bus_set_transient_user_relaxed(u, name, &c->group, message, flags, error); + if (streq(name, "SetLoginEnvironment")) + return bus_set_transient_tristate(u, name, &c->set_login_environment, message, flags, error); + if (streq(name, "TTYPath")) return bus_set_transient_path(u, name, &c->tty_path, message, flags, error); diff --git a/src/core/execute.c b/src/core/execute.c index 04f6bcd703e..e25552daa67 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1923,7 +1923,33 @@ static int build_environment( our_env[n_env++] = x; } - if (home) { + /* We query "root" if this is a system unit and User= is not specified. $USER is always set. $HOME + * could cause problem for e.g. getty, since login doesn't override $HOME, and $LOGNAME and $SHELL don't + * really make much sense since we're not logged in. Hence we conditionalize the three based on + * SetLoginEnvironment= switch. */ + if (!c->user && !c->dynamic_user && p->runtime_scope == RUNTIME_SCOPE_SYSTEM) { + r = get_fixed_user("root", &username, NULL, NULL, &home, &shell); + if (r < 0) + return log_unit_error_errno(u, r, "Failed to determine user credentials for root: %m"); + } + + bool set_user_login_env = c->set_login_environment >= 0 ? c->set_login_environment : (c->user || c->dynamic_user); + + if (username) { + x = strjoin("USER=", username); + if (!x) + return -ENOMEM; + our_env[n_env++] = x; + + if (set_user_login_env) { + x = strjoin("LOGNAME=", username); + if (!x) + return -ENOMEM; + our_env[n_env++] = x; + } + } + + if (home && set_user_login_env) { x = strjoin("HOME=", home); if (!x) return -ENOMEM; @@ -1932,19 +1958,7 @@ static int build_environment( our_env[n_env++] = x; } - if (username) { - x = strjoin("LOGNAME=", username); - if (!x) - return -ENOMEM; - our_env[n_env++] = x; - - x = strjoin("USER=", username); - if (!x) - return -ENOMEM; - our_env[n_env++] = x; - } - - if (shell) { + if (shell && set_user_login_env) { x = strjoin("SHELL=", shell); if (!x) return -ENOMEM; @@ -5262,6 +5276,7 @@ void exec_context_init(ExecContext *c) { .tty_cols = UINT_MAX, .private_mounts = -1, .memory_ksm = -1, + .set_login_environment = -1, }; FOREACH_ARRAY(d, c->directories, _EXEC_DIRECTORY_TYPE_MAX) diff --git a/src/core/execute.h b/src/core/execute.h index 33fe77bf7c4..a68ea37d830 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -254,6 +254,8 @@ struct ExecContext { char *group; char **supplementary_groups; + int set_login_environment; + char *pam_name; char *utmp_id; diff --git a/src/core/load-fragment-gperf.gperf.in b/src/core/load-fragment-gperf.gperf.in index 77a0dce529f..0ab468ba176 100644 --- a/src/core/load-fragment-gperf.gperf.in +++ b/src/core/load-fragment-gperf.gperf.in @@ -19,6 +19,7 @@ {{type}}.User, config_parse_user_group_compat, 0, offsetof({{type}}, exec_context.user) {{type}}.Group, config_parse_user_group_compat, 0, offsetof({{type}}, exec_context.group) {{type}}.SupplementaryGroups, config_parse_user_group_strv_compat, 0, offsetof({{type}}, exec_context.supplementary_groups) +{{type}}.SetLoginEnvironment, config_parse_tristate, 0, offsetof({{type}}, exec_context.set_login_environment) {{type}}.Nice, config_parse_exec_nice, 0, offsetof({{type}}, exec_context) {{type}}.OOMScoreAdjust, config_parse_exec_oom_score_adjust, 0, offsetof({{type}}, exec_context) {{type}}.CoredumpFilter, config_parse_exec_coredump_filter, 0, offsetof({{type}}, exec_context) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index eeeabc66b15..634a8f08c29 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -1078,7 +1078,8 @@ static int bus_append_execute_property(sd_bus_message *m, const char *field, con "ProtectHostname", "MemoryKSM", "RestrictSUIDSGID", - "RootEphemeral")) + "RootEphemeral", + "SetLoginEnvironment")) return bus_append_parse_boolean(m, field, eq); if (STR_IN_SET(field, "ReadWriteDirectories",