From 937b244237dd560acbd508bb5b3a7dcc3083ea2c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 31 Aug 2026 09:14:27 +0200 Subject: [PATCH] core: extend ExecStart= varlink support for StartTransientUnit() to accept execution flags --- src/core/dbus-execute.c | 37 ++------ src/core/execute.c | 50 +++++++++++ src/core/execute.h | 1 + src/core/varlink-unit.c | 85 ++++++++++++------- src/shared/exec-util.c | 10 +++ src/shared/exec-util.h | 1 + .../TEST-74-AUX-UTILS.varlinkctl-unit.sh | 60 +++++++++++++ 7 files changed, 182 insertions(+), 62 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 4d86c07a41..ad9c665529 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1634,14 +1634,6 @@ int bus_property_get_exec_ex_command_list( return sd_bus_message_close_container(reply); } -static char* exec_command_flags_to_exec_chars(ExecCommandFlags flags) { - return strjoin(FLAGS_SET(flags, EXEC_COMMAND_IGNORE_FAILURE) ? "-" : "", - FLAGS_SET(flags, EXEC_COMMAND_NO_ENV_EXPAND) ? ":" : "", - FLAGS_SET(flags, EXEC_COMMAND_FULLY_PRIVILEGED) ? "+" : "", - FLAGS_SET(flags, EXEC_COMMAND_NO_SETUID) ? "!" : "", - FLAGS_SET(flags, EXEC_COMMAND_VIA_SHELL) ? "|" : ""); -} - int bus_set_transient_exec_command( Unit *u, const char *name, @@ -1767,32 +1759,13 @@ int bus_set_transient_exec_command( fprintf(f, "%s=\n", written_name); LIST_FOREACH(command, c, *exec_command) { - _cleanup_free_ char *a = NULL, *exec_chars = NULL; - UnitWriteFlags esc_flags = UNIT_ESCAPE_SPECIFIERS | - (FLAGS_SET(c->flags, EXEC_COMMAND_NO_ENV_EXPAND) ? UNIT_ESCAPE_EXEC_SYNTAX : UNIT_ESCAPE_EXEC_SYNTAX_ENV); - bool via_shell = FLAGS_SET(c->flags, EXEC_COMMAND_VIA_SHELL); + _cleanup_free_ char *a = NULL; - exec_chars = exec_command_flags_to_exec_chars(c->flags); - if (!exec_chars) - return -ENOMEM; + r = exec_command_to_setting(c, &a); + if (r < 0) + return r; - a = unit_concat_strv(via_shell ? strv_skip(c->argv, 1) : c->argv, esc_flags); - if (!a) - return -ENOMEM; - - if (via_shell || streq(c->path, c->argv[0])) - fprintf(f, "%s=%s%s%s\n", - written_name, exec_chars, via_shell && c->argv[0][0] == '-' ? "@" : "", a); - else { - _cleanup_free_ char *t = NULL; - const char *p; - - p = unit_escape_setting(c->path, esc_flags, &t); - if (!p) - return -ENOMEM; - - fprintf(f, "%s=%s@%s %s\n", written_name, exec_chars, p, a); - } + fprintf(f, "%s=%s\n", written_name, a); } r = memstream_finalize(&m, &buf, NULL); diff --git a/src/core/execute.c b/src/core/execute.c index 295ae13039..a57d1040a2 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -2317,6 +2317,56 @@ void exec_command_append_list(ExecCommand **l, ExecCommand *e) { *l = e; } +int exec_command_to_setting(const ExecCommand *c, char **ret) { + _cleanup_free_ char *exec_chars = NULL, *a = NULL, *s = NULL; + UnitWriteFlags esc_flags; + bool via_shell; + + assert(c); + assert(c->path); + assert(!strv_isempty(c->argv)); + assert(ret); + + /* Formats the command the way it appears on the right-hand side of an ExecStart= (or similar) + * assignment in a unit file, i.e. the prefix characters encoding the flags, followed by the + * (escaped) command line. This is the inverse of the ExecStart= parser in load-fragment.c, and used + * when writing transient unit files. */ + + esc_flags = UNIT_ESCAPE_SPECIFIERS | + (FLAGS_SET(c->flags, EXEC_COMMAND_NO_ENV_EXPAND) ? UNIT_ESCAPE_EXEC_SYNTAX : UNIT_ESCAPE_EXEC_SYNTAX_ENV); + via_shell = FLAGS_SET(c->flags, EXEC_COMMAND_VIA_SHELL); + + exec_chars = exec_command_flags_to_exec_chars(c->flags); + if (!exec_chars) + return -ENOMEM; + + /* For via-shell commands argv[0] is always "sh" or "-sh", which is implied by the "|" prefix + * (plus "@" for the login shell case), hence skip it. */ + a = unit_concat_strv(via_shell ? strv_skip(c->argv, 1) : c->argv, esc_flags); + if (!a) + return -ENOMEM; + + /* If argv[0] matches the path the path is implied, too. Note that we use streq() instead of + * path_equal() here as argv[0] can be arbitrary and may not be a path. */ + if (via_shell || streq(c->path, c->argv[0])) + s = strjoin(exec_chars, via_shell && c->argv[0][0] == '-' ? "@" : "", a); + else { + _cleanup_free_ char *t = NULL; + const char *p; + + p = unit_escape_setting(c->path, esc_flags, &t); + if (!p) + return -ENOMEM; + + s = strjoin(exec_chars, "@", p, " ", a); + } + if (!s) + return -ENOMEM; + + *ret = TAKE_PTR(s); + return 0; +} + int exec_command_set(ExecCommand *c, const char *path, ...) { va_list ap; char **l, *p; diff --git a/src/core/execute.h b/src/core/execute.h index e553a72b85..e66b9bcd5b 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -540,6 +540,7 @@ void exec_command_reset_status_list_array(ExecCommand **c, size_t n); void exec_command_dump(ExecCommand *c, FILE *f, const char *prefix); void exec_command_dump_list(ExecCommand *c, FILE *f, const char *prefix); void exec_command_append_list(ExecCommand **l, ExecCommand *e); +int exec_command_to_setting(const ExecCommand *c, char **ret); int exec_command_set(ExecCommand *c, const char *path, ...) _sentinel_; int exec_command_append(ExecCommand *c, const char *path, ...) _sentinel_; diff --git a/src/core/varlink-unit.c b/src/core/varlink-unit.c index e72cb660ba..374c4ef6ab 100644 --- a/src/core/varlink-unit.c +++ b/src/core/varlink-unit.c @@ -1,5 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include + #include "sd-bus.h" #include "sd-json.h" @@ -654,6 +656,11 @@ void varlink_job_send_removed_signal(Job *j) { typedef struct TransientExecCommandItem { const char *path; char **arguments; + bool ignore_failure; + bool privileged; + bool no_setuid; + bool no_env_expand; + bool via_shell; } TransientExecCommandItem; static void transient_exec_command_item_done(TransientExecCommandItem *i) { @@ -820,8 +827,13 @@ static void transient_service_parameters_init(TransientServiceParameters *p) { static int dispatch_transient_exec_command(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata) { static const sd_json_dispatch_field exec_command_dispatch[] = { - { "path", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, offsetof(TransientExecCommandItem, path), SD_JSON_MANDATORY }, - { "arguments", SD_JSON_VARIANT_ARRAY, sd_json_dispatch_strv, offsetof(TransientExecCommandItem, arguments), 0 }, + { "path", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, offsetof(TransientExecCommandItem, path), SD_JSON_MANDATORY }, + { "arguments", SD_JSON_VARIANT_ARRAY, sd_json_dispatch_strv, offsetof(TransientExecCommandItem, arguments), 0 }, + { "ignoreFailure", SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(TransientExecCommandItem, ignore_failure), 0 }, + { "privileged", SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(TransientExecCommandItem, privileged), 0 }, + { "noSetuid", SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(TransientExecCommandItem, no_setuid), 0 }, + { "noEnvExpand", SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(TransientExecCommandItem, no_env_expand), 0 }, + { "viaShell", SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(TransientExecCommandItem, via_shell), 0 }, {} }; @@ -1695,11 +1707,24 @@ static int transient_service_apply_properties(Service *s, TransientServiceParame FOREACH_ARRAY(item, sp->exec_start, sp->n_exec_start) { _cleanup_(exec_command_freep) ExecCommand *c = NULL; _cleanup_strv_free_ char **argv = NULL; + const char *path = item->path; - if (!filename_or_absolute_path_is_valid(item->path)) { + if (item->privileged && item->no_setuid) { if (reterr_field) *reterr_field = "Service.ExecStart"; - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid ExecStart path: %s", item->path); + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "ExecStart 'privileged' and 'noSetuid' flags may not be combined."); + } + + if (item->via_shell) + /* Always normalize path (and argv[0] below) to "sh", matching the D-Bus and unit + * file counterparts. The real shell of the target user is determined at execution + * time. */ + path = _PATH_BSHELL; + else if (!filename_or_absolute_path_is_valid(path)) { + if (reterr_field) + *reterr_field = "Service.ExecStart"; + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid ExecStart path: %s", path); } if (!strv_isempty(item->arguments)) { @@ -1712,13 +1737,29 @@ static int transient_service_apply_properties(Service *s, TransientServiceParame if (!c) return -ENOMEM; - r = path_simplify_alloc(item->path, &c->path); + c->flags = + (item->ignore_failure ? EXEC_COMMAND_IGNORE_FAILURE : 0) | + (item->privileged ? EXEC_COMMAND_FULLY_PRIVILEGED : 0) | + (item->no_setuid ? EXEC_COMMAND_NO_SETUID : 0) | + (item->no_env_expand ? EXEC_COMMAND_NO_ENV_EXPAND : 0) | + (item->via_shell ? EXEC_COMMAND_VIA_SHELL : 0); + + r = path_simplify_alloc(path, &c->path); if (r < 0) return r; - /* If no arguments were provided, default argv[0] to the executable path. - * Otherwise the caller is expected to include argv[0] in the arguments array. */ - if (strv_isempty(argv)) { + if (item->via_shell) { + /* Normalize argv[0] to "sh", preserving login shell semantics requested via a + * leading dash. */ + if (strv_isempty(argv)) + r = strv_extend(&argv, "sh"); + else + r = free_and_strdup(&argv[0], argv[0][0] == '-' ? "-sh" : "sh"); + if (r < 0) + return r; + } else if (strv_isempty(argv)) { + /* If no arguments were provided, default argv[0] to the executable path. + * Otherwise the caller is expected to include argv[0] in the arguments array. */ r = strv_extend(&argv, c->path); if (r < 0) return r; @@ -1730,30 +1771,14 @@ static int transient_service_apply_properties(Service *s, TransientServiceParame } /* Write ExecStart= lines to the transient file */ - if (sp->n_exec_start > 0) { - UnitWriteFlags esc_flags = UNIT_ESCAPE_SPECIFIERS|UNIT_ESCAPE_EXEC_SYNTAX_ENV; + LIST_FOREACH(command, c, s->exec_command[SERVICE_EXEC_START]) { + _cleanup_free_ char *a = NULL; - LIST_FOREACH(command, c, s->exec_command[SERVICE_EXEC_START]) { - _cleanup_free_ char *a = NULL; + r = exec_command_to_setting(c, &a); + if (r < 0) + return r; - a = unit_concat_strv(c->argv, esc_flags); - if (!a) - return -ENOMEM; - - /* streq() instead path_equal() as argv[0] can be arbitrary and may not be a path */ - if (streq(c->path, c->argv[0])) - unit_write_settingf(u, UNIT_RUNTIME|UNIT_PRIVATE, "ExecStart", "ExecStart=%s", a); - else { - _cleanup_free_ char *t = NULL; - const char *p; - - p = unit_escape_setting(c->path, esc_flags, &t); - if (!p) - return -ENOMEM; - - unit_write_settingf(u, UNIT_RUNTIME|UNIT_PRIVATE, "ExecStart", "ExecStart=@%s %s", p, a); - } - } + unit_write_settingf(u, UNIT_RUNTIME|UNIT_PRIVATE, "ExecStart", "ExecStart=%s", a); } return 0; diff --git a/src/shared/exec-util.c b/src/shared/exec-util.c index 420803c77d..3e6818f56a 100644 --- a/src/shared/exec-util.c +++ b/src/shared/exec-util.c @@ -519,6 +519,16 @@ ExecCommandFlags exec_command_flags_from_string(const char *s) { return 1U << idx; } +char* exec_command_flags_to_exec_chars(ExecCommandFlags flags) { + /* Returns the prefix characters that encode the flags in unit file ExecStart= lines, i.e. the + * inverse of the prefix parsing in load-fragment.c */ + return strjoin(FLAGS_SET(flags, EXEC_COMMAND_IGNORE_FAILURE) ? "-" : "", + FLAGS_SET(flags, EXEC_COMMAND_NO_ENV_EXPAND) ? ":" : "", + FLAGS_SET(flags, EXEC_COMMAND_FULLY_PRIVILEGED) ? "+" : "", + FLAGS_SET(flags, EXEC_COMMAND_NO_SETUID) ? "!" : "", + FLAGS_SET(flags, EXEC_COMMAND_VIA_SHELL) ? "|" : ""); +} + int fexecve_or_execve(int executable_fd, const char *executable, char *const argv[], char *const envp[]) { /* Refuse invalid fds, regardless if fexecve() use is enabled or not */ if (executable_fd < 0) diff --git a/src/shared/exec-util.h b/src/shared/exec-util.h index 11b01f321f..24f880005a 100644 --- a/src/shared/exec-util.h +++ b/src/shared/exec-util.h @@ -55,6 +55,7 @@ typedef enum ExecCommandFlags { int exec_command_flags_from_strv(char * const *ex_opts, ExecCommandFlags *ret); int exec_command_flags_to_strv(ExecCommandFlags flags, char ***ret); +char* exec_command_flags_to_exec_chars(ExecCommandFlags flags); DECLARE_STRING_TABLE_LOOKUP(exec_command_flags, ExecCommandFlags); diff --git a/test/units/TEST-74-AUX-UTILS.varlinkctl-unit.sh b/test/units/TEST-74-AUX-UTILS.varlinkctl-unit.sh index f4a4cb6fff..83bd6dc6a6 100755 --- a/test/units/TEST-74-AUX-UTILS.varlinkctl-unit.sh +++ b/test/units/TEST-74-AUX-UTILS.varlinkctl-unit.sh @@ -249,6 +249,61 @@ timeout 30 bash -c 'until systemctl is-active varlink-transient-kill.service; do systemctl show -P KillMode varlink-transient-kill.service | grep '^process$' >/dev/null systemctl show -P SendSIGHUP varlink-transient-kill.service | grep '^yes$' >/dev/null +# ExecStart execution flags, i.e. the Varlink counterparts of the "-", ":", "|", "+" and "!" prefixes of +# unit file ExecStart= lines. First the ones observable from an unprivileged unit: +# - ignoreFailure: a failing command does not fail the unit +# - noEnvExpand: "${VAR}" in the arguments is passed as-is rather than expanded +# - viaShell: the command line is handed to the user's shell via "-c" (argv[0] is normalized to "sh") +rm -f /tmp/varlink-transient-execflags-* +defer_transient_cleanup varlink-transient-execflags.service +# shellcheck disable=SC2016 +result=$(varlinkctl call "$MANAGER_SOCKET" io.systemd.Unit.StartTransient \ + '{"context":{"ID":"varlink-transient-execflags.service","Exec":{"Environment":["FLAGTEST=expanded"]},"Service":{"Type":"oneshot","RemainAfterExit":true,"ExecStart":[ + {"path":"/bin/false","ignoreFailure":true}, + {"path":"/usr/bin/touch","arguments":["touch","/tmp/varlink-transient-execflags-${FLAGTEST}"]}, + {"path":"/usr/bin/touch","arguments":["touch","/tmp/varlink-transient-execflags-${FLAGTEST}"],"noEnvExpand":true}, + {"path":"/bin/sh","arguments":["sh","touch","/tmp/varlink-transient-execflags-shell"],"viaShell":true}]}}}') +echo "$result" | jq -e '.context.Service.ExecStart[0].ignoreFailure == true' +echo "$result" | jq -e '.context.Service.ExecStart[1].noEnvExpand == false' +echo "$result" | jq -e '.context.Service.ExecStart[2].noEnvExpand == true' +echo "$result" | jq -e '.context.Service.ExecStart[3].viaShell == true' +echo "$result" | jq -e '.context.Service.ExecStart[3].path == "/bin/sh"' +echo "$result" | jq -e '.context.Service.ExecStart[3].arguments[0] == "sh"' +# The unit only becomes active once all ExecStart= commands succeeded (or were allowed to fail) +timeout 30 bash -c 'until systemctl is-active varlink-transient-execflags.service; do sleep 0.5; done' +test -e /tmp/varlink-transient-execflags-expanded +# shellcheck disable=SC2016 +test -e '/tmp/varlink-transient-execflags-${FLAGTEST}' +test -e /tmp/varlink-transient-execflags-shell +# The flags must show up as the matching prefix characters in the transient unit file +fragment=$(systemctl show -P FragmentPath varlink-transient-execflags.service) +grep -E '^ExecStart=-"/bin/false"$' "$fragment" >/dev/null +grep -E '^ExecStart=:' "$fragment" >/dev/null +grep -E '^ExecStart=\|' "$fragment" >/dev/null +rm -f /tmp/varlink-transient-execflags-* + +# Now the privilege related ones, in a unit running as an unprivileged user: +# - privileged/noSetuid: the command runs with full privileges, i.e. can write where the user cannot +# - (neither): the command runs as the user, and fails to write there (tolerated via ignoreFailure) +rm -f /run/varlink-transient-execflags-* +defer_transient_cleanup varlink-transient-execflags-priv.service +result=$(varlinkctl call "$MANAGER_SOCKET" io.systemd.Unit.StartTransient \ + '{"context":{"ID":"varlink-transient-execflags-priv.service","Exec":{"User":"nobody"},"Service":{"Type":"oneshot","RemainAfterExit":true,"ExecStart":[ + {"path":"/usr/bin/touch","arguments":["touch","/run/varlink-transient-execflags-plain"],"ignoreFailure":true}, + {"path":"/usr/bin/touch","arguments":["touch","/run/varlink-transient-execflags-privileged"],"privileged":true}, + {"path":"/usr/bin/touch","arguments":["touch","/run/varlink-transient-execflags-nosetuid"],"noSetuid":true}]}}}') +echo "$result" | jq -e '.context.Service.ExecStart[0].privileged == false' +echo "$result" | jq -e '.context.Service.ExecStart[1].privileged == true' +echo "$result" | jq -e '.context.Service.ExecStart[2].noSetuid == true' +timeout 30 bash -c 'until systemctl is-active varlink-transient-execflags-priv.service; do sleep 0.5; done' +test ! -e /run/varlink-transient-execflags-plain +test -e /run/varlink-transient-execflags-privileged +test -e /run/varlink-transient-execflags-nosetuid +fragment=$(systemctl show -P FragmentPath varlink-transient-execflags-priv.service) +grep -E '^ExecStart=\+' "$fragment" >/dev/null +grep -E '^ExecStart=!' "$fragment" >/dev/null +rm -f /run/varlink-transient-execflags-* + # Exec.SetCredential: pass a credential and verify the running process can read it defer_transient_cleanup varlink-transient-cred.service CRED_VALUE_B64=$(printf 'secret-value' | base64 -w0) @@ -348,6 +403,11 @@ defer_transient_cleanup varlink-transient-badpath.service expect_invalid_parameter \ '{"context":{"ID":"varlink-transient-badpath.service","Service":{"Type":"simple","ExecStart":[{"path":""}]}}}' \ "Service.ExecStart" +# The 'privileged' and 'noSetuid' execution flags are mutually exclusive +defer_transient_cleanup varlink-transient-badflags.service +expect_invalid_parameter \ + '{"context":{"ID":"varlink-transient-badflags.service","Service":{"Type":"oneshot","ExecStart":[{"path":"/bin/true","privileged":true,"noSetuid":true}]}}}' \ + "Service.ExecStart" # Relative WorkingDirectory path is rejected defer_transient_cleanup varlink-transient-bad-wd.service expect_invalid_parameter \