diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index bc3ed93d106..73aacc9d182 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -102,6 +102,7 @@ #include "parse-util.h" #include "path-lookup.h" #include "path-util.h" +#include "pidfd-util.h" #include "pidref.h" #include "polkit-agent.h" #include "pretty-print.h" @@ -3303,6 +3304,7 @@ static int inner_child( Barrier *barrier, int fd_inner_socket, FDSet *fds, + char **fdnames, char **os_release_pairs) { _cleanup_free_ char *home = NULL; @@ -3318,7 +3320,7 @@ static int inner_child( NULL, /* LOGNAME */ NULL, /* container_uuid */ NULL, /* LISTEN_FDS */ - NULL, /* LISTEN_PID */ + NULL, /* LISTEN_FDNAMES */ NULL, /* NOTIFY_SOCKET */ NULL, /* CREDENTIALS_DIRECTORY */ NULL, /* LANG */ @@ -3592,9 +3594,17 @@ static int inner_child( if (r < 0) return log_error_errno(r, "Failed to unset O_CLOEXEC for file descriptors."); - if ((asprintf(envp + n_env++, "LISTEN_FDS=%u", fdset_size(fds)) < 0) || - (asprintf(envp + n_env++, "LISTEN_PID=1") < 0)) + if (asprintf(envp + n_env++, "LISTEN_FDS=%u", fdset_size(fds)) < 0) return log_oom(); + + if (fdnames) { + _cleanup_free_ char *joined = strv_join(fdnames, ":"); + if (!joined) + return log_oom(); + + if (asprintf(envp + n_env++, "LISTEN_FDNAMES=%s", joined) < 0) + return log_oom(); + } } if (asprintf(envp + n_env++, "NOTIFY_SOCKET=%s", NSPAWN_NOTIFY_SOCKET_PATH) < 0) return log_oom(); @@ -3634,6 +3644,23 @@ static int inner_child( return r; } + /* Now that we know which process is going to execute the payload, set $LISTEN_PID and + * $LISTEN_PIDFDID. If $LISTEN_PID is overridden via --setenv=, leave $LISTEN_PIDFDID alone too, as + * it would refer to a different process than $LISTEN_PID then. */ + if (!fdset_isempty(fds) && !strv_env_get(arg_setenv, "LISTEN_PID")) { + r = strv_env_assignf(&env_use, "LISTEN_PID", PID_FMT, getpid_cached()); + if (r < 0) + return log_error_errno(r, "Failed to set $LISTEN_PID: %m"); + + uint64_t pidfdid; + if (!strv_env_get(arg_setenv, "LISTEN_PIDFDID") && + pidfd_get_inode_id_self_cached(&pidfdid) >= 0) { + r = strv_env_assignf(&env_use, "LISTEN_PIDFDID", "%" PRIu64, pidfdid); + if (r < 0) + return log_error_errno(r, "Failed to set $LISTEN_PIDFDID: %m"); + } + } + if (arg_console_mode != CONSOLE_PIPE) { /* So far our pty wasn't controlled by any process. Finally, it's time to change that, if we * are configured for that. Acquire it as controlling tty. */ @@ -3891,6 +3918,7 @@ static int outer_child( int fd_outer_socket, int fd_inner_socket, FDSet *fds, + char **fdnames, int netns_fd, const char *unix_export_path) { @@ -4459,7 +4487,7 @@ static int outer_child( return log_error_errno(r, "Failed to move root directory: %m"); } - r = inner_child(barrier, fd_inner_socket, fds, os_release_pairs); + r = inner_child(barrier, fd_inner_socket, fds, fdnames, os_release_pairs); if (r < 0) _exit(EXIT_FAILURE); @@ -4588,18 +4616,23 @@ static int setup_uid_map( return 0; } +static bool fd_store_forwarding_enabled(void) { + /* We can only forward fd-store related messages if we have an upstream supervisor (NOTIFY_SOCKET) + * with an fd store available (FDSTORE). */ + return getenv("NOTIFY_SOCKET") && fdstore_detected(); +} + static int forward_fd_store(char **tags, FDSet *fds) { int r; /* Forward fd-store related messages to our own service manager, so that file descriptors stored - * by the inner payload propagate up the chain and are preserved across restarts. Skip entirely - * if we have no upstream supervisor (no NOTIFY_SOCKET) or no fd store available (no FDSTORE). + * by the inner payload propagate up the chain and are preserved across restarts. * * Forwarded entries are namespaced with a "payload-" prefix on their FDNAME so that they * cannot collide with fd-store entries that nspawn itself might want to push to its own * upstream supervisor (the container payload and nspawn share a single upstream fdstore * namespace, since there's only one init system per container). */ - if (!getenv("NOTIFY_SOCKET") || !fdstore_detected()) + if (!fd_store_forwarding_enabled()) return 0; const char *pattern = strv_find_startswith(tags, "FDSTOREWIPE="); @@ -5330,6 +5363,7 @@ static int run_container( MStack *mstack, int userns_fd, FDSet *fds, + char **fdnames, char veth_name[IFNAMSIZ], bool *veth_created, struct ExposeArgs *expose_args, @@ -5491,6 +5525,7 @@ static int run_container( fd_outer_socket_pair[1], fd_inner_socket_pair[1], fds, + fdnames, child_netns_fd, unix_export_host_dir); if (r < 0) @@ -6200,10 +6235,70 @@ static int do_cleanup(void) { return 0; } +static int acquire_listen_fdnames(FDSet *fds, char ***ret) { + _cleanup_strv_free_ char **l = NULL; + const char *e; + int r; + + assert(ret); + + e = getenv("LISTEN_FDNAMES"); + if (!e || fdset_isempty(fds)) { + *ret = NULL; + return 0; + } + + /* Split the same way sd_listen_fds_with_names() does, so that we end up with the same names. */ + r = strv_split_full(&l, e, ":", EXTRACT_DONT_COALESCE_SEPARATORS); + if (r == -ENOMEM) + return log_oom(); + if (r < 0) { + log_warning_errno(r, "Failed to parse $LISTEN_FDNAMES, ignoring: %m"); + *ret = NULL; + return 0; + } + + if ((size_t) r != fdset_size(fds)) { + log_warning("Number of entries in $LISTEN_FDNAMES (%i) doesn't match number of passed file descriptors (%u), ignoring.", + r, fdset_size(fds)); + *ret = NULL; + return 0; + } + + /* forward_fd_store() prefixes the names of the payload's fd store entries with "payload-" when + * pushing them upstream. Strip it again when they are passed back to us, so that the payload gets + * them back under the name it stored them with. Leave the names alone if we wouldn't forward fd + * store entries, as they can't have come from the payload then. */ + bool strip_prefix = fd_store_forwarding_enabled(); + + STRV_FOREACH(n, l) { + if (!fdname_is_valid(*n)) { + _cleanup_free_ char *esc = cescape(*n); + log_warning("$LISTEN_FDNAMES contains invalid file descriptor name '%s', ignoring.", strna(esc)); + *ret = NULL; + return 0; + } + + const char *name = (strip_prefix ? startswith(*n, "payload-") : NULL) ?: *n; + + /* The payload's sd_listen_fds_with_names() will unescape the names again, hence escape + * them here. */ + char *esc = shell_escape(name, ":"); + if (!esc) + return log_oom(); + + free_and_replace(*n, esc); + } + + *ret = TAKE_PTR(l); + return 0; +} + static int run(int argc, char *argv[]) { bool remove_image = false, veth_created = false; _cleanup_close_ int master = -EBADF, userns_fd = -EBADF, mount_fd = -EBADF; _cleanup_fdset_free_ FDSet *fds = NULL; + _cleanup_strv_free_ char **fdnames = NULL; int r, ret = EXIT_SUCCESS; char veth_name[IFNAMSIZ] = ""; struct ExposeArgs expose_args = {}; @@ -6316,6 +6411,10 @@ static int run(int argc, char *argv[]) { goto finish; } + r = acquire_listen_fdnames(fds, &fdnames); + if (r < 0) + goto finish; + /* The "default" umask. This is appropriate for most file and directory * operations performed by nspawn, and is the umask that will be used for * the child. Functions like copy_devnodes() change the umask temporarily. */ @@ -6841,6 +6940,7 @@ static int run(int argc, char *argv[]) { mstack, userns_fd, fds, + fdnames, veth_name, &veth_created, &expose_args, &master, diff --git a/src/test/test-fdstore.c b/src/test/test-fdstore.c index 92ceea4596b..7bdbcc473c2 100644 --- a/src/test/test-fdstore.c +++ b/src/test/test-fdstore.c @@ -2,8 +2,10 @@ /* In 'store' mode pushes a couple of memfds with known content into the supervisor's fd store via FDSTORE=1 * sd_notify() messages. In 'check' mode reads back the fds passed via LISTEN_FDS and verifies the content - * matches what was pushed. In 'wipe[=GLOB]' mode removes all or glob-matched stored fds. - * Multiple modes may be run in sequence. + * matches what was pushed, and that they are passed back under the names they were pushed with. In + * 'wipe[=GLOB]' mode removes all or glob-matched stored fds. In 'listen' mode verifies that sd_listen_fds() + * accepts all fds passed via LISTEN_FDS, i.e. that LISTEN_PID and LISTEN_PIDFDID refer to us. In 'exit' mode + * exits immediately instead of staying alive. Multiple modes may be run in sequence. * * This binary is intentionally linked against libsystemd only so that it can go in the minimal image. */ @@ -17,6 +19,8 @@ #include "sd-daemon.h" +#define NAME_A "test-fd-a" +#define NAME_B "test-fd-b" #define DATA_A "fdstore-data-a" #define DATA_B "fdstore-data-b" @@ -30,6 +34,15 @@ static void closep(int *fd) { *fd = -EBADF; } +static void strv_freep(char ***l) { + if (!l || !*l) + return; + + for (char **i = *l; *i; i++) + free(*i); + free(*l); +} + static _Noreturn void exit_handler(int sig) { _exit(EXIT_SUCCESS); } @@ -79,10 +92,10 @@ static int push_one(const char *fdname, const char *content) { static int do_store(void) { int r; - if (push_one("test-fd-a", DATA_A) < 0) + if (push_one(NAME_A, DATA_A) < 0) return EXIT_FAILURE; - if (push_one("test-fd-b", DATA_B) < 0) + if (push_one(NAME_B, DATA_B) < 0) return EXIT_FAILURE; /* Wait for our supervisor to actually process the FDSTORE messages before we exit, otherwise @@ -98,13 +111,14 @@ static int do_store(void) { } static int do_check(void) { + _cleanup_(strv_freep) char **names = NULL; bool seen_a = false, seen_b = false; int n; - n = sd_listen_fds(/* unset_environment= */ 0); + n = sd_listen_fds_with_names(/* unset_environment= */ 0, &names); if (n < 0) { errno = -n; - fprintf(stderr, "sd_listen_fds failed: %m\n"); + fprintf(stderr, "sd_listen_fds_with_names failed: %m\n"); return EXIT_FAILURE; } if (n < 2) { @@ -128,12 +142,23 @@ static int do_check(void) { } buf[k] = 0; - if (strcmp(buf, DATA_A) == 0) + const char *expected_name; + if (strcmp(buf, DATA_A) == 0) { seen_a = true; - else if (strcmp(buf, DATA_B) == 0) + expected_name = NAME_A; + } else if (strcmp(buf, DATA_B) == 0) { seen_b = true; - else + expected_name = NAME_B; + } else { fprintf(stderr, "Unexpected fd content: '%s'\n", buf); + continue; + } + + if (strcmp(names[i], expected_name) != 0) { + fprintf(stderr, "Unexpected name '%s' for fd with content '%s', expected '%s'\n", + names[i], buf, expected_name); + return EXIT_FAILURE; + } } if (!seen_a || !seen_b) { @@ -141,7 +166,42 @@ static int do_check(void) { return EXIT_FAILURE; } - printf("Payload received both preserved fds with matching content.\n"); + printf("Payload received both preserved fds with matching content and names.\n"); + return EXIT_SUCCESS; +} + +static int do_listen(void) { + const char *e; + char *end; + long m; + int n; + + e = getenv("LISTEN_FDS"); + if (!e) { + fprintf(stderr, "$LISTEN_FDS not set\n"); + return EXIT_FAILURE; + } + + errno = 0; + m = strtol(e, &end, 10); + if (errno != 0 || end == e || *end != 0 || m <= 0) { + fprintf(stderr, "Invalid $LISTEN_FDS: '%s'\n", e); + return EXIT_FAILURE; + } + + /* sd_listen_fds() ignores $LISTEN_FDS if $LISTEN_PID or $LISTEN_PIDFDID refer to another process */ + n = sd_listen_fds(/* unset_environment= */ 0); + if (n < 0) { + errno = -n; + fprintf(stderr, "sd_listen_fds failed: %m\n"); + return EXIT_FAILURE; + } + if (n != m) { + fprintf(stderr, "sd_listen_fds returned %d, but $LISTEN_FDS=%s\n", n, e); + return EXIT_FAILURE; + } + + printf("sd_listen_fds accepted all %d fds.\n", n); return EXIT_SUCCESS; } @@ -166,7 +226,7 @@ int main(int argc, char *argv[]) { int r; if (argc < 2) { - fprintf(stderr, "Usage: %s {store|check|wipe[=GLOB]}...\n", argv[0]); + fprintf(stderr, "Usage: %s {store|check|listen|wipe[=GLOB]|exit}...\n", argv[0]); return EXIT_FAILURE; } @@ -175,6 +235,10 @@ int main(int argc, char *argv[]) { r = do_store(); else if (strcmp(argv[i], "check") == 0) r = do_check(); + else if (strcmp(argv[i], "listen") == 0) + r = do_listen(); + else if (strcmp(argv[i], "exit") == 0) + return EXIT_SUCCESS; else if (strcmp(argv[i], "wipe") == 0) r = notify("FDSTOREWIPE", "*"); else if (strncmp(argv[i], "wipe=", strlen("wipe=")) == 0) diff --git a/test/units/TEST-13-NSPAWN.nspawn.sh b/test/units/TEST-13-NSPAWN.nspawn.sh index 375ab2d08e9..e0981cd806f 100755 --- a/test/units/TEST-13-NSPAWN.nspawn.sh +++ b/test/units/TEST-13-NSPAWN.nspawn.sh @@ -1178,6 +1178,115 @@ EOF rm -fr "$root" } +testcase_socket_activation() { + # Verify that the socket activation environment variables are properly propagated into the + # container, i.e. that $LISTEN_FDNAMES survives, and that $LISTEN_PID refers to the process that + # actually ends up running the payload (PID 2 when --as-pid2 is used). + # https://github.com/systemd/systemd/issues/43764 + local root entrypoint socket1 socket2 nspawn + + root="$(mktemp -d /var/lib/machines/TEST-13-NSPAWN.socket-activation.XXX)" + create_dummy_container "$root" + entrypoint="$root/entrypoint.sh" + socket1="/run/TEST-13-NSPAWN-socket-activation-1.sock" + socket2="/run/TEST-13-NSPAWN-socket-activation-2.sock" + rm -f "$socket1" "$socket2" + + cat >"$entrypoint" <<\EOF +#!/usr/bin/env bash +set -ex + +env | grep '^LISTEN_' +test "$LISTEN_FDS" = "${EXPECTED_FDS:-1}" +test "${LISTEN_FDNAMES-unset}" = "$EXPECTED_FDNAMES" +if [[ -v EXPECTED_PID ]]; then + test "$LISTEN_PID" = "$EXPECTED_PID" + # $LISTEN_PIDFDID must not refer to a different process than $LISTEN_PID + test "${LISTEN_PIDFDID-unset}" = "unset" +else + test "$LISTEN_PID" = "$$" +fi +for ((fd = 3; fd < 3 + LISTEN_FDS; fd++)); do + test -S "/proc/self/fd/$fd" +done +if [[ ! -v EXPECTED_PID ]]; then + # sd_listen_fds() only accepts the fds if both $LISTEN_PID and $LISTEN_PIDFDID refer to the calling + # process. exec keeps both our PID and our pidfd inode ID. + exec /usr/bin/test-fdstore listen exit +fi +EOF + chmod +x "$entrypoint" + + nspawn=(systemd-nspawn --register=no --directory="$root") + + # Without --as-pid2 the payload is PID 1 in the container... + systemd-socket-activate --listen="$socket1" --fdname=hoge --now -- \ + "${nspawn[@]}" --setenv=EXPECTED_FDNAMES=hoge "${entrypoint##"$root"}" + rm -f "$socket1" + + # ...and with --as-pid2 it's PID 2, as the stub PID 1 forks it off. + systemd-socket-activate --listen="$socket1" --fdname=hoge --now -- \ + "${nspawn[@]}" --as-pid2 --setenv=EXPECTED_FDNAMES=hoge "${entrypoint##"$root"}" + rm -f "$socket1" + + # Multiple file descriptors are passed on in order + systemd-socket-activate --listen="$socket1" --listen="$socket2" --fdname=hoge --fdname=fuga --now -- \ + "${nspawn[@]}" --as-pid2 --setenv=EXPECTED_FDS=2 --setenv=EXPECTED_FDNAMES=hoge:fuga \ + "${entrypoint##"$root"}" + rm -f "$socket1" "$socket2" + + # Escape sequences in $LISTEN_FDNAMES are resolved the same way sd_listen_fds_with_names() does, and the + # names are escaped again when passed on, so that the payload ends up with the same names + systemd-socket-activate --listen="$socket1" --listen="$socket2" \ + --fdname='ho\ge' --fdname='fu\\ga' --now -- \ + "${nspawn[@]}" --as-pid2 --setenv=EXPECTED_FDS=2 --setenv='EXPECTED_FDNAMES=hoge:fu\\ga' \ + "${entrypoint##"$root"}" + rm -f "$socket1" "$socket2" + + # The prefix nspawn adds to the names of fd store entries it forwards upstream is stripped again, but + # only if nspawn actually forwards fd store entries, i.e. it has an upstream fd store + systemd-socket-activate --listen="$socket1" --fdname=payload-hoge --now -- \ + "${nspawn[@]}" --as-pid2 --setenv=EXPECTED_FDNAMES=payload-hoge "${entrypoint##"$root"}" + rm -f "$socket1" + systemd-run --wait --pipe -p Type=notify -p NotifyAccess=all -p FileDescriptorStoreMax=1 -- \ + bash -c 'exec systemd-socket-activate -E NOTIFY_SOCKET -E FDSTORE "$@"' _ \ + --listen="$socket1" --fdname=payload-hoge --now -- \ + "${nspawn[@]}" --keep-unit --as-pid2 --setenv=EXPECTED_FDNAMES=hoge "${entrypoint##"$root"}" + rm -f "$socket1" + + # $LISTEN_FDNAMES is not passed on if it is not set, ... + systemd-socket-activate --listen="$socket1" --now -- \ + "${nspawn[@]}" --as-pid2 --setenv=EXPECTED_FDNAMES=unset "${entrypoint##"$root"}" + rm -f "$socket1" + + # ...doesn't match the number of file descriptors, ... + systemd-socket-activate --listen="$socket1" --fdname=hoge --fdname=fuga --now -- \ + "${nspawn[@]}" --as-pid2 --setenv=EXPECTED_FDNAMES=unset "${entrypoint##"$root"}" + rm -f "$socket1" + + # ...cannot be parsed, ... + # shellcheck disable=SC1003 + systemd-socket-activate --listen="$socket1" --fdname='hoge\' --now -- \ + "${nspawn[@]}" --as-pid2 --setenv=EXPECTED_FDNAMES=unset "${entrypoint##"$root"}" + rm -f "$socket1" + + # ...or contains invalid names + systemd-socket-activate --listen="$socket1" --fdname=$'ho\tge' --now -- \ + "${nspawn[@]}" --as-pid2 --setenv=EXPECTED_FDNAMES=unset "${entrypoint##"$root"}" + rm -f "$socket1" + + # An explicit --setenv= takes precedence over the socket activation variables, and $LISTEN_PIDFDID is + # not set if $LISTEN_PID is overridden + systemd-socket-activate --listen="$socket1" --fdname=hoge --now -- \ + "${nspawn[@]}" --as-pid2 \ + --setenv=LISTEN_FDNAMES=override --setenv=EXPECTED_FDNAMES=override \ + --setenv=LISTEN_PID=4711 --setenv=EXPECTED_PID=4711 \ + "${entrypoint##"$root"}" + rm -f "$socket1" + + rm -fr "$root" +} + testcase_os_release() { local root entrypoint os_release_source