From 046a82c1b20f765a8aa704ddea87e1b906d8ded0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 26 Oct 2017 18:45:54 +0200 Subject: [PATCH 01/23] fd-util: add new helper move_fd() and make use of it We are using the same pattern at various places: call dup2() on an fd, and close the old fd, usually in combination with some O_CLOEXEC fiddling. Let's add a little helper for this, and port a few obvious cases over. --- src/basic/fd-util.c | 44 ++++++++++++++++++++++++++++++++++++++ src/basic/fd-util.h | 2 ++ src/core/execute.c | 38 ++++++-------------------------- src/import/import-common.c | 32 +++++++++++---------------- src/import/pull-common.c | 16 ++++++-------- 5 files changed, 71 insertions(+), 61 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 9d61044c89..0fd271318f 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -377,3 +377,47 @@ int fd_get_path(int fd, char **ret) { return r; } + +int move_fd(int from, int to, int cloexec) { + int r; + + /* Move fd 'from' to 'to', make sure FD_CLOEXEC remains equal if requested, and release the old fd. If + * 'cloexec' is passed as -1, the original FD_CLOEXEC is inherited for the new fd. If it is 0, it is turned + * off, if it is > 0 it is turned on. */ + + if (from < 0) + return -EBADF; + if (to < 0) + return -EBADF; + + if (from == to) { + + if (cloexec >= 0) { + r = fd_cloexec(to, cloexec); + if (r < 0) + return r; + } + + return to; + } + + if (cloexec < 0) { + int fl; + + fl = fcntl(from, F_GETFD, 0); + if (fl < 0) + return -errno; + + cloexec = !!(fl & FD_CLOEXEC); + } + + r = dup3(from, to, cloexec ? O_CLOEXEC : 0); + if (r < 0) + return -errno; + + assert(r == to); + + safe_close(from); + + return to; +} diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index 34b98d4aec..7c527850ed 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -75,6 +75,8 @@ bool fdname_is_valid(const char *s); int fd_get_path(int fd, char **ret); +int move_fd(int from, int to, int cloexec); + /* Hint: ENETUNREACH happens if we try to connect to "non-existing" special IP addresses, such as ::5 */ #define ERRNO_IS_DISCONNECT(r) \ IN_SET(r, ENOTCONN, ECONNRESET, ECONNREFUSED, ECONNABORTED, EPIPE, ENETUNREACH) diff --git a/src/core/execute.c b/src/core/execute.c index 45df0318e0..8083cccab2 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -275,7 +275,7 @@ static bool exec_context_needs_term(const ExecContext *c) { } static int open_null_as(int flags, int nfd) { - int fd, r; + int fd; assert(nfd >= 0); @@ -283,13 +283,7 @@ static int open_null_as(int flags, int nfd) { if (fd < 0) return -errno; - if (fd != nfd) { - r = dup2(fd, nfd) < 0 ? -errno : nfd; - safe_close(fd); - } else - r = nfd; - - return r; + return move_fd(fd, nfd, false); } static int connect_journal_socket(int fd, uid_t uid, gid_t gid) { @@ -381,16 +375,10 @@ static int connect_logger_as( is_kmsg_output(output), is_terminal_output(output)); - if (fd == nfd) - return nfd; - - r = dup2(fd, nfd) < 0 ? -errno : nfd; - safe_close(fd); - - return r; + return move_fd(fd, nfd, false); } static int open_terminal_as(const char *path, mode_t mode, int nfd) { - int fd, r; + int fd; assert(path); assert(nfd >= 0); @@ -399,13 +387,7 @@ static int open_terminal_as(const char *path, mode_t mode, int nfd) { if (fd < 0) return fd; - if (fd != nfd) { - r = dup2(fd, nfd) < 0 ? -errno : nfd; - safe_close(fd); - } else - r = nfd; - - return r; + return move_fd(fd, nfd, false); } static int fixup_input(ExecInput std_input, int socket_fd, bool apply_tty_stdin) { @@ -459,7 +441,7 @@ static int setup_input( case EXEC_INPUT_TTY: case EXEC_INPUT_TTY_FORCE: case EXEC_INPUT_TTY_FAIL: { - int fd, r; + int fd; fd = acquire_terminal(exec_context_tty_path(context), i == EXEC_INPUT_TTY_FAIL, @@ -469,13 +451,7 @@ static int setup_input( if (fd < 0) return fd; - if (fd != STDIN_FILENO) { - r = dup2(fd, STDIN_FILENO) < 0 ? -errno : STDIN_FILENO; - safe_close(fd); - } else - r = STDIN_FILENO; - - return r; + return move_fd(fd, STDIN_FILENO, false); } case EXEC_INPUT_SOCKET: diff --git a/src/import/import-common.c b/src/import/import-common.c index ae71682988..900c0f9591 100644 --- a/src/import/import-common.c +++ b/src/import/import-common.c @@ -103,28 +103,24 @@ int import_fork_tar_x(const char *path, pid_t *ret) { pipefd[1] = safe_close(pipefd[1]); - if (dup2(pipefd[0], STDIN_FILENO) != STDIN_FILENO) { - log_error_errno(errno, "Failed to dup2() fd: %m"); + r = move_fd(pipefd[0], STDIN_FILENO, false); + if (r < 0) { + log_error_errno(r, "Failed to move fd: %m"); _exit(EXIT_FAILURE); } - if (pipefd[0] != STDIN_FILENO) - pipefd[0] = safe_close(pipefd[0]); - null_fd = open("/dev/null", O_WRONLY|O_NOCTTY); if (null_fd < 0) { log_error_errno(errno, "Failed to open /dev/null: %m"); _exit(EXIT_FAILURE); } - if (dup2(null_fd, STDOUT_FILENO) != STDOUT_FILENO) { - log_error_errno(errno, "Failed to dup2() fd: %m"); + r = move_fd(null_fd, STDOUT_FILENO, false); + if (r < 0) { + log_error_errno(r, "Failed to move fd: %m"); _exit(EXIT_FAILURE); } - if (null_fd != STDOUT_FILENO) - null_fd = safe_close(null_fd); - stdio_unset_cloexec(); if (unshare(CLONE_NEWNET) < 0) @@ -175,28 +171,24 @@ int import_fork_tar_c(const char *path, pid_t *ret) { pipefd[0] = safe_close(pipefd[0]); - if (dup2(pipefd[1], STDOUT_FILENO) != STDOUT_FILENO) { - log_error_errno(errno, "Failed to dup2() fd: %m"); + r = move_fd(pipefd[1], STDOUT_FILENO, false); + if (r < 0) { + log_error_errno(r, "Failed to move fd: %m"); _exit(EXIT_FAILURE); } - if (pipefd[1] != STDOUT_FILENO) - pipefd[1] = safe_close(pipefd[1]); - null_fd = open("/dev/null", O_RDONLY|O_NOCTTY); if (null_fd < 0) { log_error_errno(errno, "Failed to open /dev/null: %m"); _exit(EXIT_FAILURE); } - if (dup2(null_fd, STDIN_FILENO) != STDIN_FILENO) { - log_error_errno(errno, "Failed to dup2() fd: %m"); + r = move_fd(null_fd, STDIN_FILENO, false); + if (r < 0) { + log_error_errno(errno, "Failed to move fd: %m"); _exit(EXIT_FAILURE); } - if (null_fd != STDIN_FILENO) - null_fd = safe_close(null_fd); - stdio_unset_cloexec(); if (unshare(CLONE_NEWNET) < 0) diff --git a/src/import/pull-common.c b/src/import/pull-common.c index 78840dd882..d7f7a7e29b 100644 --- a/src/import/pull-common.c +++ b/src/import/pull-common.c @@ -492,28 +492,24 @@ int pull_verify(PullJob *main_job, gpg_pipe[1] = safe_close(gpg_pipe[1]); - if (dup2(gpg_pipe[0], STDIN_FILENO) != STDIN_FILENO) { - log_error_errno(errno, "Failed to dup2() fd: %m"); + r = move_fd(gpg_pipe[0], STDIN_FILENO, false); + if (r < 0) { + log_error_errno(errno, "Failed to move fd: %m"); _exit(EXIT_FAILURE); } - if (gpg_pipe[0] != STDIN_FILENO) - gpg_pipe[0] = safe_close(gpg_pipe[0]); - null_fd = open("/dev/null", O_WRONLY|O_NOCTTY); if (null_fd < 0) { log_error_errno(errno, "Failed to open /dev/null: %m"); _exit(EXIT_FAILURE); } - if (dup2(null_fd, STDOUT_FILENO) != STDOUT_FILENO) { - log_error_errno(errno, "Failed to dup2() fd: %m"); + r = move_fd(null_fd, STDOUT_FILENO, false); + if (r < 0) { + log_error_errno(errno, "Failed to move fd: %m"); _exit(EXIT_FAILURE); } - if (null_fd != STDOUT_FILENO) - null_fd = safe_close(null_fd); - cmd[k++] = strjoina("--homedir=", gpg_home); /* We add the user keyring only to the command line From 1fb0682e78d0869d85a0e9b1f46a3490257aac6e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 26 Oct 2017 18:47:34 +0200 Subject: [PATCH 02/23] execute: check whether we are actually on a TTY before doing TIOCSCTTY Given that Linux assigns the same ioctl numbers ot multiple subsystems, we should be careful when invoking ioctls, so that we don't end up calling something we wouldn't want to call. --- src/core/execute.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index 8083cccab2..547ab40657 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -425,8 +425,10 @@ static int setup_input( return -errno; /* Try to make this the controlling tty, if it is a tty, and reset it */ - (void) ioctl(STDIN_FILENO, TIOCSCTTY, context->std_input == EXEC_INPUT_TTY_FORCE); - (void) reset_terminal_fd(STDIN_FILENO, true); + if (isatty(STDIN_FILENO)) { + (void) ioctl(STDIN_FILENO, TIOCSCTTY, context->std_input == EXEC_INPUT_TTY_FORCE); + (void) reset_terminal_fd(STDIN_FILENO, true); + } return STDIN_FILENO; } From 7fa0dcb6e3b45932fbbe8e9f3d7c437f366e6f5c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 26 Oct 2017 20:02:57 +0200 Subject: [PATCH 03/23] core: drop config_parse_input() as it is unused --- src/core/load-fragment.c | 1 - src/core/load-fragment.h | 1 - 2 files changed, 2 deletions(-) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 52a10dd24b..e52f608db7 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -831,7 +831,6 @@ int config_parse_socket_bindtodevice( return 0; } -DEFINE_CONFIG_PARSE_ENUM(config_parse_input, exec_input, ExecInput, "Failed to parse input literal specifier"); DEFINE_CONFIG_PARSE_ENUM(config_parse_output, exec_output, ExecOutput, "Failed to parse output literal specifier"); int config_parse_exec_input(const char *unit, diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h index fbf2de23eb..58d335807b 100644 --- a/src/core/load-fragment.h +++ b/src/core/load-fragment.h @@ -48,7 +48,6 @@ int config_parse_socket_bindtodevice(const char *unit, const char *filename, uns int config_parse_exec_output(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_output(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_exec_input(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); -int config_parse_input(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_exec_io_class(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_exec_io_priority(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_exec_cpu_sched_policy(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); From 9bd6a50e90427dd120d0c9495b50fda2f72887b6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 26 Oct 2017 20:06:42 +0200 Subject: [PATCH 04/23] core: clean up config_parse_exec_input() a bit Mostly coding style fixes, but most importantly, initialize c->std_input only after we know the free_and_strdup() invocation succeeded, so that we don't leave half-initialized fields around on failure. --- src/core/load-fragment.c | 44 ++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index e52f608db7..a3950b8d72 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -833,16 +833,18 @@ int config_parse_socket_bindtodevice( DEFINE_CONFIG_PARSE_ENUM(config_parse_output, exec_output, ExecOutput, "Failed to parse output literal specifier"); -int config_parse_exec_input(const char *unit, - const char *filename, - unsigned line, - const char *section, - unsigned section_line, - const char *lvalue, - int ltype, - const char *rvalue, - void *data, - void *userdata) { +int config_parse_exec_input( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + ExecContext *c = data; const char *name; int r; @@ -859,19 +861,25 @@ int config_parse_exec_input(const char *unit, log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid file descriptor name, ignoring: %s", name); return 0; } - c->std_input = EXEC_INPUT_NAMED_FD; + r = free_and_strdup(&c->stdio_fdname[STDIN_FILENO], name); if (r < 0) - log_oom(); - return r; + return log_oom(); + + c->std_input = EXEC_INPUT_NAMED_FD; } else { - ExecInput ei = exec_input_from_string(rvalue); - if (ei == _EXEC_INPUT_INVALID) + ExecInput ei; + + ei = exec_input_from_string(rvalue); + if (ei < 0) { log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse input specifier, ignoring: %s", rvalue); - else - c->std_input = ei; - return 0; + return 0; + } + + c->std_input = ei; } + + return 0; } int config_parse_exec_output(const char *unit, From a548e14d690133dd8cca2d5ab8082bb23259fd5f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 10:56:42 +0200 Subject: [PATCH 05/23] fd-util: add new acquire_data_fd() API helper All this function does is place some data in an in-memory read-only fd, that may be read back to get the original data back. Doing this in a way that works everywhere, given the different kernels we support as well as different privilege levels is surprisingly complex. --- src/basic/fd-util.c | 157 ++++++++++++++++++++++++++++++++++++++++ src/basic/fd-util.h | 10 +++ src/test/test-fd-util.c | 52 +++++++++++++ 3 files changed, 219 insertions(+) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 0fd271318f..b93c3bd743 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -26,8 +26,10 @@ #include "dirent-util.h" #include "fd-util.h" +#include "fileio.h" #include "fs-util.h" #include "macro.h" +#include "memfd-util.h" #include "missing.h" #include "parse-util.h" #include "path-util.h" @@ -421,3 +423,158 @@ int move_fd(int from, int to, int cloexec) { return to; } + +int acquire_data_fd(const void *data, size_t size, unsigned flags) { + + char procfs_path[strlen("/proc/self/fd/") + DECIMAL_STR_MAX(int)]; + _cleanup_close_pair_ int pipefds[2] = { -1, -1 }; + char pattern[] = "/dev/shm/data-fd-XXXXXX"; + _cleanup_close_ int fd = -1; + int isz = 0, r; + ssize_t n; + off_t f; + + assert(data || size == 0); + + /* Acquire a read-only file descriptor that when read from returns the specified data. This is much more + * complex than I wish it was. But here's why: + * + * a) First we try to use memfds. They are the best option, as we can seal them nicely to make them + * read-only. Unfortunately they require kernel 3.17, and – at the time of writing – we still support 3.14. + * + * b) Then, we try classic pipes. They are the second best options, as we can close the writing side, retaining + * a nicely read-only fd in the reading side. However, they are by default quite small, and unprivileged + * clients can only bump their size to a system-wide limit, which might be quite low. + * + * c) Then, we try an O_TMPFILE file in /dev/shm (that dir is the only suitable one known to exist from + * earliest boot on). To make it read-only we open the fd a second time with O_RDONLY via + * /proc/self/. Unfortunately O_TMPFILE is not available on older kernels on tmpfs. + * + * d) Finally, we try creating a regular file in /dev/shm, which we then delete. + * + * It sucks a bit that depending on the situation we return very different objects here, but that's Linux I + * figure. */ + + if (size == 0 && ((flags & ACQUIRE_NO_DEV_NULL) == 0)) { + /* As a special case, return /dev/null if we have been called for an empty data block */ + r = open("/dev/null", O_RDONLY|O_CLOEXEC|O_NOCTTY); + if (r < 0) + return -errno; + + return r; + } + + if ((flags & ACQUIRE_NO_MEMFD) == 0) { + fd = memfd_new("data-fd"); + if (fd < 0) + goto try_pipe; + + n = write(fd, data, size); + if (n < 0) + return -errno; + if ((size_t) n != size) + return -EIO; + + f = lseek(fd, 0, SEEK_SET); + if (f != 0) + return -errno; + + r = memfd_set_sealed(fd); + if (r < 0) + return r; + + r = fd; + fd = -1; + + return r; + } + +try_pipe: + if ((flags & ACQUIRE_NO_PIPE) == 0) { + if (pipe2(pipefds, O_CLOEXEC|O_NONBLOCK) < 0) + return -errno; + + isz = fcntl(pipefds[1], F_GETPIPE_SZ, 0); + if (isz < 0) + return -errno; + + if ((size_t) isz < size) { + isz = (int) size; + if (isz < 0 || (size_t) isz != size) + return -E2BIG; + + /* Try to bump the pipe size */ + (void) fcntl(pipefds[1], F_SETPIPE_SZ, isz); + + /* See if that worked */ + isz = fcntl(pipefds[1], F_GETPIPE_SZ, 0); + if (isz < 0) + return -errno; + + if ((size_t) isz < size) + goto try_dev_shm; + } + + n = write(pipefds[1], data, size); + if (n < 0) + return -errno; + if ((size_t) n != size) + return -EIO; + + (void) fd_nonblock(pipefds[0], false); + + r = pipefds[0]; + pipefds[0] = -1; + + return r; + } + +try_dev_shm: + if ((flags & ACQUIRE_NO_TMPFILE) == 0) { + fd = open("/dev/shm", O_RDWR|O_TMPFILE|O_CLOEXEC, 0500); + if (fd < 0) + goto try_dev_shm_without_o_tmpfile; + + n = write(fd, data, size); + if (n < 0) + return -errno; + if ((size_t) n != size) + return -EIO; + + /* Let's reopen the thing, in order to get an O_RDONLY fd for the original O_RDWR one */ + xsprintf(procfs_path, "/proc/self/fd/%i", fd); + r = open(procfs_path, O_RDONLY|O_CLOEXEC); + if (r < 0) + return -errno; + + return r; + } + +try_dev_shm_without_o_tmpfile: + if ((flags & ACQUIRE_NO_REGULAR) == 0) { + fd = mkostemp_safe(pattern); + if (fd < 0) + return fd; + + n = write(fd, data, size); + if (n < 0) { + r = -errno; + goto unlink_and_return; + } + if ((size_t) n != size) { + r = -EIO; + goto unlink_and_return; + } + + /* Let's reopen the thing, in order to get an O_RDONLY fd for the original O_RDWR one */ + r = open(pattern, O_RDONLY|O_CLOEXEC); + if (r < 0) + r = -errno; + + unlink_and_return: + (void) unlink(pattern); + return r; + } + + return -EOPNOTSUPP; +} diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index 7c527850ed..df5a5fad98 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -77,6 +77,16 @@ int fd_get_path(int fd, char **ret); int move_fd(int from, int to, int cloexec); +enum { + ACQUIRE_NO_DEV_NULL = 1 << 0, + ACQUIRE_NO_MEMFD = 1 << 1, + ACQUIRE_NO_PIPE = 1 << 2, + ACQUIRE_NO_TMPFILE = 1 << 3, + ACQUIRE_NO_REGULAR = 1 << 4, +}; + +int acquire_data_fd(const void *data, size_t size, unsigned flags); + /* Hint: ENETUNREACH happens if we try to connect to "non-existing" special IP addresses, such as ::5 */ #define ERRNO_IS_DISCONNECT(r) \ IN_SET(r, ENOTCONN, ECONNRESET, ECONNREFUSED, ECONNABORTED, EPIPE, ENETUNREACH) diff --git a/src/test/test-fd-util.c b/src/test/test-fd-util.c index 4425b5fe5f..dc18ea3d3a 100644 --- a/src/test/test-fd-util.c +++ b/src/test/test-fd-util.c @@ -24,6 +24,9 @@ #include "fd-util.h" #include "fileio.h" #include "macro.h" +#include "random-util.h" +#include "string-util.h" +#include "util.h" static void test_close_many(void) { int fds[3]; @@ -103,11 +106,60 @@ static void test_open_serialization_fd(void) { write(fd, "test\n", 5); } +static void test_acquire_data_fd_one(unsigned flags) { + char wbuffer[196*1024 - 7]; + char rbuffer[sizeof(wbuffer)]; + int fd; + + fd = acquire_data_fd("foo", 3, flags); + assert_se(fd >= 0); + + zero(rbuffer); + assert_se(read(fd, rbuffer, sizeof(rbuffer)) == 3); + assert_se(streq(rbuffer, "foo")); + + fd = safe_close(fd); + + fd = acquire_data_fd("", 0, flags); + assert_se(fd >= 0); + + zero(rbuffer); + assert_se(read(fd, rbuffer, sizeof(rbuffer)) == 0); + assert_se(streq(rbuffer, "")); + + fd = safe_close(fd); + + random_bytes(wbuffer, sizeof(wbuffer)); + + fd = acquire_data_fd(wbuffer, sizeof(wbuffer), flags); + assert_se(fd >= 0); + + zero(rbuffer); + assert_se(read(fd, rbuffer, sizeof(rbuffer)) == sizeof(rbuffer)); + assert_se(memcmp(rbuffer, wbuffer, sizeof(rbuffer)) == 0); + + fd = safe_close(fd); +} + +static void test_acquire_data_fd(void) { + + test_acquire_data_fd_one(0); + test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL); + test_acquire_data_fd_one(ACQUIRE_NO_MEMFD); + test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL|ACQUIRE_NO_MEMFD); + test_acquire_data_fd_one(ACQUIRE_NO_PIPE); + test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL|ACQUIRE_NO_PIPE); + test_acquire_data_fd_one(ACQUIRE_NO_MEMFD|ACQUIRE_NO_PIPE); + test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL|ACQUIRE_NO_MEMFD|ACQUIRE_NO_PIPE); + test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL|ACQUIRE_NO_MEMFD|ACQUIRE_NO_PIPE|ACQUIRE_NO_TMPFILE); +} + int main(int argc, char *argv[]) { test_close_many(); test_close_nointr(); test_same_fd(); test_open_serialization_fd(); + test_acquire_data_fd(); return 0; } From 6bbfdc672bd69cbd712d6d532db76dbb3c227784 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 11:04:57 +0200 Subject: [PATCH 06/23] bus-unit-util: propagate errors where it makes sense, don't make up EINVAL This is not only more technically correct, but also shortens our code quite a bit. --- src/shared/bus-unit-util.c | 79 +++++++++++++------------------------- 1 file changed, 27 insertions(+), 52 deletions(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 2b2480c2e1..52750872bc 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -323,10 +323,8 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen uint64_t u; r = cg_weight_parse(eq, &u); - if (r < 0) { - log_error("Failed to parse %s value %s.", field, eq); - return -EINVAL; - } + if (r < 0) + return log_error_errno(r, "Failed to parse %s value %s: %m", field, eq); r = sd_bus_message_append(m, "v", "t", u); @@ -334,10 +332,8 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen uint64_t u; r = cg_cpu_shares_parse(eq, &u); - if (r < 0) { - log_error("Failed to parse %s value %s.", field, eq); - return -EINVAL; - } + if (r < 0) + return log_error_errno(r, "Failed to parse %s value %s: %m", field, eq); r = sd_bus_message_append(m, "v", "t", u); @@ -345,10 +341,8 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen uint64_t u; r = cg_weight_parse(eq, &u); - if (r < 0) { - log_error("Failed to parse %s value %s.", field, eq); - return -EINVAL; - } + if (r < 0) + return log_error_errno(r, "Failed to parse %s value %s: %m", field, eq); r = sd_bus_message_append(m, "v", "t", u); @@ -356,10 +350,8 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen uint64_t u; r = cg_blkio_weight_parse(eq, &u); - if (r < 0) { - log_error("Failed to parse %s value %s.", field, eq); - return -EINVAL; - } + if (r < 0) + return log_error_errno(r, "Failed to parse %s value %s: %m", field, eq); r = sd_bus_message_append(m, "v", "t", u); @@ -413,10 +405,8 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen } else if (streq(field, "SecureBits")) { r = secure_bits_from_string(eq); - if (r < 0) { - log_error("Failed to parse %s value %s.", field, eq); - return -EINVAL; - } + if (r < 0) + return log_error_errno(r, "Failed to parse %s value %s: %m", field, eq); r = sd_bus_message_append(m, "v", "i", r); @@ -432,10 +422,8 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen } r = capability_set_from_string(p, &sum); - if (r < 0) { - log_error("Failed to parse %s value %s.", field, eq); - return -EINVAL; - } + if (r < 0) + return log_error_errno(r, "Failed to parse %s value %s: %m", field, eq); sum = invert ? ~sum : sum; @@ -491,10 +479,8 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen bytes = CGROUP_LIMIT_MAX; } else { r = parse_size(bandwidth, 1000, &bytes); - if (r < 0) { - log_error("Failed to parse byte value %s.", bandwidth); - return -EINVAL; - } + if (r < 0) + return log_error_errno(r, "Failed to parse byte value %s: %m", bandwidth); } r = sd_bus_message_append(m, "v", "a(st)", 1, path, bytes); @@ -523,10 +509,9 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen } r = safe_atou64(weight, &u); - if (r < 0) { - log_error("Failed to parse %s value %s.", field, weight); - return -EINVAL; - } + if (r < 0) + return log_error_errno(r, "Failed to parse %s value %s: %m", field, weight); + r = sd_bus_message_append(m, "v", "a(st)", 1, path, u); } @@ -857,10 +842,8 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen _cleanup_free_ char *word = NULL; r = extract_first_word(&p, &word, NULL, EXTRACT_QUOTES|EXTRACT_CUNESCAPE); - if (r < 0) { - log_error("Failed to parse Environment value %s", eq); - return -EINVAL; - } + if (r < 0) + return log_error_errno(r, "Failed to parse Environment value %s: %m", eq); if (r == 0) break; @@ -907,20 +890,16 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen nsec_t n; r = parse_nsec(eq, &n); - if (r < 0) { - log_error("Failed to parse %s value %s", field, eq); - return -EINVAL; - } + if (r < 0) + return log_error_errno(r, "Failed to parse %s value %s: %m", field, eq); r = sd_bus_message_append(m, "v", "t", n); } else if (streq(field, "OOMScoreAdjust")) { int oa; r = safe_atoi(eq, &oa); - if (r < 0) { - log_error("Failed to parse %s value %s", field, eq); - return -EINVAL; - } + if (r < 0) + return log_error_errno(r, "Failed to parse %s value %s: %m", field, eq); if (!oom_score_adjust_is_valid(oa)) { log_error("OOM score adjust value out of range"); @@ -945,10 +924,8 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen size_t offset; r = extract_first_word(&p, &word, NULL, EXTRACT_QUOTES); - if (r < 0) { - log_error("Failed to parse %s value %s", field, eq); - return -EINVAL; - } + if (r < 0) + return log_error_errno(r, "Failed to parse %s value %s: %m", field, eq); if (r == 0) break; @@ -993,10 +970,8 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen _cleanup_free_ char *word = NULL; r = extract_first_word(&p, &word, NULL, EXTRACT_QUOTES); - if (r < 0) { - log_error("Failed to parse %s value %s", field, eq); - return -EINVAL; - } + if (r < 0) + return log_error_errno(r, "Failed to parse %s value %s: %m", field, eq); if (r == 0) break; From ae6e414a611cdb7c4dd6e0b810a9efac3e9d2318 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 11:06:19 +0200 Subject: [PATCH 07/23] hexdecoct: slightly extend the unbase64mem() API and related If the string length is specified as (size_t) -1, let's use that as indicator for determining the length on our own. This makes it slightlier shorter to invoke these APIs for a very common case. Also, do some minor other coding style updates, and add assert()s here and there. --- src/basic/hexdecoct.c | 45 +++++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/src/basic/hexdecoct.c b/src/basic/hexdecoct.c index 766770389c..dec5552a77 100644 --- a/src/basic/hexdecoct.c +++ b/src/basic/hexdecoct.c @@ -96,7 +96,10 @@ int unhexmem(const char *p, size_t l, void **mem, size_t *len) { assert(mem); assert(len); - assert(p); + assert(p || l == 0); + + if (l == (size_t) -1) + l = strlen(p); if (l % 2 != 0) return -EINVAL; @@ -160,6 +163,8 @@ char *base32hexmem(const void *p, size_t l, bool padding) { const uint8_t *x; size_t len; + assert(p || l == 0); + if (padding) /* five input bytes makes eight output bytes, padding is added so we must round up */ len = 8 * (l + 4) / 5; @@ -269,7 +274,12 @@ int unbase32hexmem(const char *p, size_t l, bool padding, void **mem, size_t *_l size_t len; unsigned pad = 0; - assert(p); + assert(p || l == 0); + assert(mem); + assert(_len); + + if (l == (size_t) -1) + l = strlen(p); /* padding ensures any base32hex input has input divisible by 8 */ if (padding && l % 8 != 0) @@ -519,6 +529,9 @@ ssize_t base64mem(const void *p, size_t l, char **out) { char *r, *z; const uint8_t *x; + assert(p || l == 0); + assert(out); + /* three input bytes makes four output bytes, padding is added so we must round up */ z = r = malloc(4 * (l + 2) / 3 + 1); if (!r) @@ -554,10 +567,11 @@ ssize_t base64mem(const void *p, size_t l, char **out) { return z - r; } -static int base64_append_width(char **prefix, int plen, - const char *sep, int indent, - const void *p, size_t l, - int width) { +static int base64_append_width( + char **prefix, int plen, + const char *sep, int indent, + const void *p, size_t l, + int width) { _cleanup_free_ char *x = NULL; char *t, *s; @@ -596,17 +610,18 @@ static int base64_append_width(char **prefix, int plen, return 0; } -int base64_append(char **prefix, int plen, - const void *p, size_t l, - int indent, int width) { +int base64_append( + char **prefix, int plen, + const void *p, size_t l, + int indent, int width) { + if (plen > width / 2 || plen + indent > width) /* leave indent on the left, keep last column free */ return base64_append_width(prefix, plen, "\n", indent, p, l, width - indent - 1); else /* leave plen on the left, keep last column free */ return base64_append_width(prefix, plen, NULL, plen, p, l, width - plen - 1); -}; - +} int unbase64mem(const char *p, size_t l, void **mem, size_t *_len) { _cleanup_free_ uint8_t *r = NULL; @@ -615,7 +630,12 @@ int unbase64mem(const char *p, size_t l, void **mem, size_t *_len) { const char *x; size_t len; - assert(p); + assert(p || l == 0); + assert(mem); + assert(_len); + + if (l == (size_t) -1) + l = strlen(p); /* padding ensures any base63 input has input divisible by 4 */ if (l % 4 != 0) @@ -659,6 +679,7 @@ int unbase64mem(const char *p, size_t l, void **mem, size_t *_len) { } switch (l % 4) { + case 3: a = unbase64char(x[0]); if (a < 0) From a3ab8f2a9923b0dc35a0a9f17fd1d69adc5f75d2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 11:14:27 +0200 Subject: [PATCH 08/23] hexdcoct: dump to stdout if FILE* is specified as NULL We do a logic like that at various other places, let's do it here too, to make this as little surprising as possible. --- src/basic/hexdecoct.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/basic/hexdecoct.c b/src/basic/hexdecoct.c index dec5552a77..b61b032d99 100644 --- a/src/basic/hexdecoct.c +++ b/src/basic/hexdecoct.c @@ -737,7 +737,10 @@ void hexdump(FILE *f, const void *p, size_t s) { const uint8_t *b = p; unsigned n = 0; - assert(s == 0 || b); + assert(b || s == 0); + + if (!f) + f = stdout; while (s > 0) { size_t i; From 11f5d82507494b39fcce27e102e5a8b186451acf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 11:15:20 +0200 Subject: [PATCH 09/23] proc-cmdline: minor runlevel_to_target() coding style fixes Let's not mix function calls and variable declarations, as well as assignments and comparison in one expression. --- src/basic/proc-cmdline.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index 8592a428d5..8590133f9f 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -270,17 +270,21 @@ static const char * const rlmap_initrd[] = { }; const char* runlevel_to_target(const char *word) { + const char * const *rlmap_ptr; size_t i; - const char * const *rlmap_ptr = in_initrd() ? rlmap_initrd - : rlmap; if (!word) return NULL; - if (in_initrd() && (word = startswith(word, "rd.")) == NULL) - return NULL; + if (in_initrd()) { + word = startswith(word, "rd."); + if (!word) + return NULL; + } - for (i = 0; rlmap_ptr[i] != NULL; i += 2) + rlmap_ptr = in_initrd() ? rlmap_initrd : rlmap; + + for (i = 0; rlmap_ptr[i]; i += 2) if (streq(word, rlmap_ptr[i])) return rlmap_ptr[i+1]; From 08f3be7a38d245b5fed9902f10d3129f339477fd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 11:33:05 +0200 Subject: [PATCH 10/23] core: add two new unit file settings: StandardInputData= + StandardInputText= Both permit configuring data to pass through STDIN to an invoked process. StandardInputText= accepts a line of text (possibly with embedded C-style escapes as well as unit specifiers), which is appended to the buffer to pass as stdin, followed by a single newline. StandardInputData= is similar, but accepts arbitrary base64 encoded data, and will not resolve specifiers or C-style escapes, nor append newlines. This may be used to pass input/configuration data to services, directly in-line from unit files, either in a cooked or in a more raw format. --- src/core/dbus-execute.c | 64 +++++++++++++ src/core/execute.c | 36 +++++++- src/core/execute.h | 6 ++ src/core/load-fragment-gperf.gperf.m4 | 2 + src/core/load-fragment.c | 126 ++++++++++++++++++++++++++ src/core/load-fragment.h | 2 + src/core/service.c | 10 +- src/shared/bus-unit-util.c | 56 +++++++++++- src/systemctl/systemctl.c | 19 ++++ 9 files changed, 311 insertions(+), 10 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index eeb4ac9a01..3931bf7e70 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -34,6 +34,7 @@ #include "execute.h" #include "fd-util.h" #include "fileio.h" +#include "hexdecoct.h" #include "io-util.h" #include "ioprio.h" #include "journal-util.h" @@ -730,6 +731,25 @@ static int property_get_output_fdname( return sd_bus_message_append(reply, "s", name); } +static int property_get_input_data( + sd_bus *bus, + const char *path, + const char *interface, + const char *property, + sd_bus_message *reply, + void *userdata, + sd_bus_error *error) { + + ExecContext *c = userdata; + + assert(bus); + assert(c); + assert(property); + assert(reply); + + return sd_bus_message_append_array(reply, 'y', c->stdin_data, c->stdin_data_size); +} + static int property_get_bind_paths( sd_bus *bus, const char *path, @@ -858,6 +878,7 @@ const sd_bus_vtable bus_exec_vtable[] = { SD_BUS_PROPERTY("NonBlocking", "b", bus_property_get_bool, offsetof(ExecContext, non_blocking), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("StandardInput", "s", property_get_exec_input, offsetof(ExecContext, std_input), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("StandardInputFileDescriptorName", "s", property_get_input_fdname, 0, SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("StandardInputData", "ay", property_get_input_data, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("StandardOutput", "s", bus_property_get_exec_output, offsetof(ExecContext, std_output), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("StandardOutputFileDescriptorName", "s", property_get_output_fdname, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("StandardError", "s", bus_property_get_exec_output, offsetof(ExecContext, std_error), SD_BUS_VTABLE_PROPERTY_CONST), @@ -1840,6 +1861,49 @@ int bus_exec_context_set_transient_property( return 1; + } else if (streq(name, "StandardInputData")) { + const void *p; + size_t sz; + + r = sd_bus_message_read_array(message, 'y', &p, &sz); + if (r < 0) + return r; + + if (mode != UNIT_CHECK) { + _cleanup_free_ char *encoded = NULL; + + if (sz == 0) { + c->stdin_data = mfree(c->stdin_data); + c->stdin_data_size = 0; + + unit_write_drop_in_private(u, mode, name, "StandardInputData="); + } else { + void *q; + ssize_t n; + + if (c->stdin_data_size + sz < c->stdin_data_size || /* check for overflow */ + c->stdin_data_size + sz > EXEC_STDIN_DATA_MAX) + return -E2BIG; + + n = base64mem(p, sz, &encoded); + if (n < 0) + return (int) n; + + q = realloc(c->stdin_data, c->stdin_data_size + sz); + if (!q) + return -ENOMEM; + + memcpy((uint8_t*) q + c->stdin_data_size, p, sz); + + c->stdin_data = q; + c->stdin_data_size += sz; + + unit_write_drop_in_private_format(u, mode, name, "StandardInputData=%s", encoded); + } + } + + return 1; + } else if (STR_IN_SET(name, "IgnoreSIGPIPE", "TTYVHangup", "TTYReset", "TTYVTDisallocate", "PrivateTmp", "PrivateDevices", "PrivateNetwork", "PrivateUsers", diff --git a/src/core/execute.c b/src/core/execute.c index 547ab40657..f7243f3c0f 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -390,7 +390,16 @@ static int open_terminal_as(const char *path, mode_t mode, int nfd) { return move_fd(fd, nfd, false); } -static int fixup_input(ExecInput std_input, int socket_fd, bool apply_tty_stdin) { +static int fixup_input( + const ExecContext *context, + int socket_fd, + bool apply_tty_stdin) { + + ExecInput std_input; + + assert(context); + + std_input = context->std_input; if (is_terminal_input(std_input) && !apply_tty_stdin) return EXEC_INPUT_NULL; @@ -398,6 +407,9 @@ static int fixup_input(ExecInput std_input, int socket_fd, bool apply_tty_stdin) if (std_input == EXEC_INPUT_SOCKET && socket_fd < 0) return EXEC_INPUT_NULL; + if (std_input == EXEC_INPUT_DATA && context->stdin_data_size == 0) + return EXEC_INPUT_NULL; + return std_input; } @@ -433,7 +445,7 @@ static int setup_input( return STDIN_FILENO; } - i = fixup_input(context->std_input, socket_fd, params->flags & EXEC_APPLY_TTY_STDIN); + i = fixup_input(context, socket_fd, params->flags & EXEC_APPLY_TTY_STDIN); switch (i) { @@ -463,6 +475,16 @@ static int setup_input( (void) fd_nonblock(named_iofds[STDIN_FILENO], false); return dup2(named_iofds[STDIN_FILENO], STDIN_FILENO) < 0 ? -errno : STDIN_FILENO; + case EXEC_INPUT_DATA: { + int fd; + + fd = acquire_data_fd(context->stdin_data, context->stdin_data_size, 0); + if (fd < 0) + return fd; + + return move_fd(fd, STDIN_FILENO, false); + } + default: assert_not_reached("Unknown input type"); } @@ -507,7 +529,7 @@ static int setup_output( return STDERR_FILENO; } - i = fixup_input(context->std_input, socket_fd, params->flags & EXEC_APPLY_TTY_STDIN); + i = fixup_input(context, socket_fd, params->flags & EXEC_APPLY_TTY_STDIN); o = fixup_output(context->std_output, socket_fd); if (fileno == STDERR_FILENO) { @@ -536,8 +558,8 @@ static int setup_output( if (i == EXEC_INPUT_NULL && is_terminal_input(context->std_input)) return open_terminal_as(exec_context_tty_path(context), O_WRONLY, fileno); - /* If the input is connected to anything that's not a /dev/null, inherit that... */ - if (i != EXEC_INPUT_NULL) + /* If the input is connected to anything that's not a /dev/null or a data fd, inherit that... */ + if (!IN_SET(i, EXEC_INPUT_NULL, EXEC_INPUT_DATA)) return dup2(STDIN_FILENO, fileno) < 0 ? -errno : fileno; /* If we are not started from PID 1 we just inherit STDOUT from our parent process. */ @@ -3517,6 +3539,9 @@ void exec_context_done(ExecContext *c) { c->log_level_max = -1; exec_context_free_log_extra_fields(c); + + c->stdin_data = mfree(c->stdin_data); + c->stdin_data_size = 0; } int exec_context_destroy_runtime_directory(ExecContext *c, const char *runtime_prefix) { @@ -4608,6 +4633,7 @@ static const char* const exec_input_table[_EXEC_INPUT_MAX] = { [EXEC_INPUT_TTY_FAIL] = "tty-fail", [EXEC_INPUT_SOCKET] = "socket", [EXEC_INPUT_NAMED_FD] = "fd", + [EXEC_INPUT_DATA] = "data", }; DEFINE_STRING_TABLE_LOOKUP(exec_input, ExecInput); diff --git a/src/core/execute.h b/src/core/execute.h index ab9d0dbe2d..d1cf7dca93 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -37,6 +37,8 @@ typedef struct ExecParameters ExecParameters; #include "namespace.h" #include "nsflags.h" +#define EXEC_STDIN_DATA_MAX (64U*1024U*1024U) + typedef enum ExecUtmpMode { EXEC_UTMP_INIT, EXEC_UTMP_LOGIN, @@ -52,6 +54,7 @@ typedef enum ExecInput { EXEC_INPUT_TTY_FAIL, EXEC_INPUT_SOCKET, EXEC_INPUT_NAMED_FD, + EXEC_INPUT_DATA, _EXEC_INPUT_MAX, _EXEC_INPUT_INVALID = -1 } ExecInput; @@ -163,6 +166,9 @@ struct ExecContext { ExecOutput std_error; char *stdio_fdname[3]; + void *stdin_data; + size_t stdin_data_size; + nsec_t timer_slack_nsec; bool stdio_as_fds; diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index ffc3e20359..73b13977ed 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -41,6 +41,8 @@ $1.RemoveIPC, config_parse_bool, 0, $1.StandardInput, config_parse_exec_input, 0, offsetof($1, exec_context) $1.StandardOutput, config_parse_exec_output, 0, offsetof($1, exec_context) $1.StandardError, config_parse_exec_output, 0, offsetof($1, exec_context) +$1.StandardInputText, config_parse_exec_input_text, 0, offsetof($1, exec_context) +$1.StandardInputData, config_parse_exec_input_data, 0, offsetof($1, exec_context) $1.TTYPath, config_parse_unit_path_printf, 0, offsetof($1, exec_context.tty_path) $1.TTYReset, config_parse_bool, 0, offsetof($1, exec_context.tty_reset) $1.TTYVHangup, config_parse_bool, 0, offsetof($1, exec_context.tty_vhangup) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index a3950b8d72..cff3779f4b 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -45,6 +45,7 @@ #include "escape.h" #include "fd-util.h" #include "fs-util.h" +#include "hexdecoct.h" #include "io-util.h" #include "ioprio.h" #include "journal-util.h" @@ -882,6 +883,131 @@ int config_parse_exec_input( return 0; } +int config_parse_exec_input_text( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + + _cleanup_free_ char *unescaped = NULL, *resolved = NULL; + ExecContext *c = data; + Unit *u = userdata; + size_t sz; + void *p; + int r; + + assert(data); + assert(filename); + assert(line); + assert(rvalue); + + if (isempty(rvalue)) { + /* Reset if the empty string is assigned */ + c->stdin_data = mfree(c->stdin_data); + c->stdin_data_size = 0; + return 0; + } + + r = cunescape(rvalue, 0, &unescaped); + if (r < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, "Failed to decode C escaped text, ignoring: %s", rvalue); + return 0; + } + + r = unit_full_printf(u, unescaped, &resolved); + if (r < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve specifiers, ignoring: %s", unescaped); + return 0; + } + + sz = strlen(resolved); + if (c->stdin_data_size + sz + 1 < c->stdin_data_size || /* check for overflow */ + c->stdin_data_size + sz + 1 > EXEC_STDIN_DATA_MAX) { + log_syntax(unit, LOG_ERR, filename, line, r, "Standard input data too large (%zu), maximum of %zu permitted, ignoring.", c->stdin_data_size + sz, (size_t) EXEC_STDIN_DATA_MAX); + return 0; + } + + p = realloc(c->stdin_data, c->stdin_data_size + sz + 1); + if (!p) + return log_oom(); + + *((char*) mempcpy((char*) p + c->stdin_data_size, resolved, sz)) = '\n'; + + c->stdin_data = p; + c->stdin_data_size += sz + 1; + + return 0; +} + +int config_parse_exec_input_data( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + + _cleanup_free_ char *cleaned = NULL; + _cleanup_free_ void *p = NULL; + ExecContext *c = data; + size_t sz; + void *q; + int r; + + assert(data); + assert(filename); + assert(line); + assert(rvalue); + + if (isempty(rvalue)) { + /* Reset if the empty string is assigned */ + c->stdin_data = mfree(c->stdin_data); + c->stdin_data_size = 0; + return 0; + } + + /* Be tolerant to whitespace. Remove it all before decoding */ + cleaned = strdup(rvalue); + if (!cleaned) + return log_oom(); + delete_chars(cleaned, WHITESPACE); + + r = unbase64mem(cleaned, (size_t) -1, &p, &sz); + if (r < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, "Failed to decode base64 data, ignoring: %s", cleaned); + return 0; + } + + assert(sz > 0); + + if (c->stdin_data_size + sz < c->stdin_data_size || /* check for overflow */ + c->stdin_data_size + sz > EXEC_STDIN_DATA_MAX) { + log_syntax(unit, LOG_ERR, filename, line, r, "Standard input data too large (%zu), maximum of %zu permitted, ignoring.", c->stdin_data_size + sz, (size_t) EXEC_STDIN_DATA_MAX); + return 0; + } + + q = realloc(c->stdin_data, c->stdin_data_size + sz); + if (!q) + return log_oom(); + + memcpy((uint8_t*) q + c->stdin_data_size, p, sz); + + c->stdin_data = q; + c->stdin_data_size += sz; + + return 0; +} + int config_parse_exec_output(const char *unit, const char *filename, unsigned line, diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h index 58d335807b..bba1259ceb 100644 --- a/src/core/load-fragment.h +++ b/src/core/load-fragment.h @@ -48,6 +48,8 @@ int config_parse_socket_bindtodevice(const char *unit, const char *filename, uns int config_parse_exec_output(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_output(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_exec_input(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); +int config_parse_exec_input_text(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); +int config_parse_exec_input_data(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_exec_io_class(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_exec_io_priority(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_exec_cpu_sched_policy(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); diff --git a/src/core/service.c b/src/core/service.c index 4b912fc3b9..823c4c4166 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -591,10 +591,8 @@ static int service_add_default_dependencies(Service *s) { static void service_fix_output(Service *s) { assert(s); - /* If nothing has been explicitly configured, patch default - * output in. If input is socket/tty we avoid this however, - * since in that case we want output to default to the same - * place as we read input from. */ + /* If nothing has been explicitly configured, patch default output in. If input is socket/tty we avoid this + * however, since in that case we want output to default to the same place as we read input from. */ if (s->exec_context.std_error == EXEC_OUTPUT_INHERIT && s->exec_context.std_output == EXEC_OUTPUT_INHERIT && @@ -604,6 +602,10 @@ static void service_fix_output(Service *s) { if (s->exec_context.std_output == EXEC_OUTPUT_INHERIT && s->exec_context.std_input == EXEC_INPUT_NULL) s->exec_context.std_output = UNIT(s)->manager->default_std_output; + + if (s->exec_context.std_input == EXEC_INPUT_NULL && + s->exec_context.stdin_data_size > 0) + s->exec_context.std_input = EXEC_INPUT_DATA; } static int service_setup_bus_name(Service *s) { diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 52750872bc..8b189322b1 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -28,6 +28,7 @@ #include "errno-list.h" #include "escape.h" #include "hashmap.h" +#include "hexdecoct.h" #include "hostname-util.h" #include "in-addr-util.h" #include "list.h" @@ -273,6 +274,34 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen r = sd_bus_message_append(m, "sv", "TasksMax", "t", t); goto finish; + + } else if (streq(field, "StandardInputText")) { + _cleanup_free_ char *unescaped = NULL; + + r = cunescape(eq, 0, &unescaped); + if (r < 0) + return log_error_errno(r, "Failed to unescape text '%s': %m", eq); + + if (!strextend(&unescaped, "\n", NULL)) + return log_oom(); + + /* Note that we don't expand specifiers here, but that should be OK, as this is a programmatic + * interface anyway */ + + r = sd_bus_message_append(m, "s", "StandardInputData"); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_open_container(m, 'v', "ay"); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_append_array(m, 'y', unescaped, strlen(unescaped)); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_close_container(m); + goto finish; } r = sd_bus_message_append_basic(m, SD_BUS_TYPE_STRING, field); @@ -366,7 +395,32 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen "KeyringMode", "CollectMode")) r = sd_bus_message_append(m, "v", "s", eq); - else if (STR_IN_SET(field, "AppArmorProfile", "SmackProcessLabel")) { + else if (streq(field, "StandardInputData")) { + _cleanup_free_ char *cleaned = NULL; + _cleanup_free_ void *decoded = NULL; + size_t sz; + + cleaned = strdup(eq); + if (!cleaned) + return log_oom(); + + delete_chars(cleaned, WHITESPACE); + + r = unbase64mem(cleaned, (size_t) -1, &decoded, &sz); + if (r < 0) + return log_error_errno(r, "Failed to decode base64 data '%s': %m", cleaned); + + r = sd_bus_message_open_container(m, 'v', "ay"); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_append_array(m, 'y', decoded, sz); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_close_container(m); + + } else if (STR_IN_SET(field, "AppArmorProfile", "SmackProcessLabel")) { bool ignore; const char *s; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 9d9b45f08a..8b701103dc 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -55,6 +55,7 @@ #include "fs-util.h" #include "glob-util.h" #include "hostname-util.h" +#include "hexdecoct.h" #include "initreq.h" #include "install.h" #include "io-util.h" @@ -4977,6 +4978,24 @@ static int print_property(const char *name, sd_bus_message *m, const char *conte return bus_log_parse_error(r); return 0; + + } else if (contents[1] == SD_BUS_TYPE_BYTE && streq(name, "StandardInputData")) { + _cleanup_free_ char *h = NULL; + const void *p; + size_t sz; + ssize_t n; + + r = sd_bus_message_read_array(m, 'y', &p, &sz); + if (r < 0) + return bus_log_parse_error(r); + + n = base64mem(p, sz, &h); + if (n < 0) + return log_oom(); + + print_prop(name, "%s", h); + + return 0; } break; From da543f6a77d615a24972f2491818e068cd4c6b42 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 11:38:59 +0200 Subject: [PATCH 11/23] test: add tests for StandardInputText= and StandardInputData= --- src/test/test-execute.c | 5 +++++ test/meson.build | 1 + test/test-execute/exec-stdin-data.service | 19 +++++++++++++++++++ 3 files changed, 25 insertions(+) create mode 100644 test/test-execute/exec-stdin-data.service diff --git a/src/test/test-execute.c b/src/test/test-execute.c index 998724189f..34a93fb3eb 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -478,6 +478,10 @@ static void test_exec_specifier(Manager *m) { test(m, "exec-specifier.service", 0, CLD_EXITED); } +static void test_exec_stdin_data(Manager *m) { + test(m, "exec-stdin-data.service", 0, CLD_EXITED); +} + static int run_tests(UnitFileScope scope, const test_function_t *tests) { const test_function_t *test = NULL; Manager *m = NULL; @@ -534,6 +538,7 @@ int main(int argc, char *argv[]) { test_exec_spec_interpolation, test_exec_read_only_path_suceed, test_exec_unset_environment, + test_exec_stdin_data, NULL, }; static const test_function_t system_tests[] = { diff --git a/test/meson.build b/test/meson.build index 69d6c758b0..c5e45f1958 100644 --- a/test/meson.build +++ b/test/meson.build @@ -94,6 +94,7 @@ test_data_files = ''' test-execute/exec-runtimedirectory.service test-execute/exec-spec-interpolation.service test-execute/exec-specifier.service + test-execute/exec-stdin-data.service test-execute/exec-supplementarygroups-multiple-groups-default-group-user.service test-execute/exec-supplementarygroups-multiple-groups-withgid.service test-execute/exec-supplementarygroups-multiple-groups-withuid.service diff --git a/test/test-execute/exec-stdin-data.service b/test/test-execute/exec-stdin-data.service new file mode 100644 index 0000000000..00693d5a43 --- /dev/null +++ b/test/test-execute/exec-stdin-data.service @@ -0,0 +1,19 @@ +[Unit] +Description=Test for StandardInputText= and StandardInputData= + +[Service] +ExecStart=/bin/sh -x -c 'd=$$(mktemp -d -p /tmp); echo -e "this is a test\nand this is more\nsomething encoded!\nsomething in multiple lines\nand some more\nand a more bas64 data\nsomething with strange\nembedded\tcharacters\nand something with a exec-stdin-data.service specifier" > $d/text ; cmp $d/text' +Type=oneshot +StandardInput=data +StandardInputText=this is a test +StandardInputText=and this is more +StandardInputData=c29tZXRoaW5nIGVuY29kZWQhCg== +StandardInputText=something \ + in multiple lines +StandardInputText=\ +and some more +StandardInputData=YW5kIGEgbW9y \ + ZSBiYXM2NCBk\ +YXRhCg== +StandardInputText=something with strange\nembedded\tcharacters +StandardInputText=and something with a %n specifier From 9f617cd09f3312aa6a63a54f06ce5a25414f86b9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 11:40:53 +0200 Subject: [PATCH 12/23] bus-unit-util: drop #ifdef HAVE_SECCOMP from bus client side Whether seccomp is supported or not is a server implementation detail, the client should not be altered by that, and clients should be able to talk to servers configured differently than the client, hence drop the HAVE_SECCOMP ifdeffery here. (This would be different if we'd need libseccomp or so to implement the client, but we don't) --- src/shared/bus-unit-util.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 8b189322b1..01beac0342 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -707,8 +707,6 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen r = sd_bus_message_append(m, "v", "i", (int32_t) n); -#if HAVE_SECCOMP - } else if (streq(field, "SystemCallFilter")) { int whitelist; _cleanup_strv_free_ char **l = NULL; @@ -853,7 +851,7 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen r = sd_bus_message_close_container(m); if (r < 0) return bus_log_create_error(r); -#endif + } else if (streq(field, "FileDescriptorStoreMax")) { unsigned u; From 3a274a218dfa7f5a6235af155755d9687a44143a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 14:32:22 +0200 Subject: [PATCH 13/23] execute: fix type of open_terminal_as() flags parameter It's the flags parameter we propagate here, not the mode parameter, hence let's name it properly, and use the right type. --- src/core/execute.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index f7243f3c0f..de6a9211d2 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -377,13 +377,13 @@ static int connect_logger_as( return move_fd(fd, nfd, false); } -static int open_terminal_as(const char *path, mode_t mode, int nfd) { +static int open_terminal_as(const char *path, int flags, int nfd) { int fd; assert(path); assert(nfd >= 0); - fd = open_terminal(path, mode | O_NOCTTY); + fd = open_terminal(path, flags | O_NOCTTY); if (fd < 0) return fd; From 5073ff6becefef20e3cf651a76d330b1e3f29bb8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 14:57:12 +0200 Subject: [PATCH 14/23] core: fold property_get_input_fdname() and property_get_output_fdname() into one property_get_output_fdname() already had two different control flows for stdout and stderr, it might as well handle stdin too, thus shortening our code a bit. --- src/core/dbus-execute.c | 52 ++++++++++++++++------------------------- src/core/execute.c | 7 ++++++ 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 3931bf7e70..61341e0451 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -684,7 +684,7 @@ static int property_get_syslog_facility( return sd_bus_message_append(reply, "i", LOG_FAC(c->syslog_priority)); } -static int property_get_input_fdname( +static int property_get_stdio_fdname( sd_bus *bus, const char *path, const char *interface, @@ -694,41 +694,23 @@ static int property_get_input_fdname( sd_bus_error *error) { ExecContext *c = userdata; - const char *name; + int fileno; assert(bus); assert(c); assert(property); assert(reply); - name = exec_context_fdname(c, STDIN_FILENO); + if (streq(property, "StandardInputFileDescriptorName")) + fileno = STDIN_FILENO; + else if (streq(property, "StandardOutputFileDescriptorName")) + fileno = STDOUT_FILENO; + else { + assert(streq(property, "StandardErrorFileDescriptorName")); + fileno = STDERR_FILENO; + } - return sd_bus_message_append(reply, "s", name); -} - -static int property_get_output_fdname( - sd_bus *bus, - const char *path, - const char *interface, - const char *property, - sd_bus_message *reply, - void *userdata, - sd_bus_error *error) { - - ExecContext *c = userdata; - const char *name = NULL; - - assert(bus); - assert(c); - assert(property); - assert(reply); - - if (c->std_output == EXEC_OUTPUT_NAMED_FD && streq(property, "StandardOutputFileDescriptorName")) - name = exec_context_fdname(c, STDOUT_FILENO); - else if (c->std_error == EXEC_OUTPUT_NAMED_FD && streq(property, "StandardErrorFileDescriptorName")) - name = exec_context_fdname(c, STDERR_FILENO); - - return sd_bus_message_append(reply, "s", name); + return sd_bus_message_append(reply, "s", exec_context_fdname(c, fileno)); } static int property_get_input_data( @@ -877,12 +859,12 @@ const sd_bus_vtable bus_exec_vtable[] = { SD_BUS_PROPERTY("CPUSchedulingResetOnFork", "b", bus_property_get_bool, offsetof(ExecContext, cpu_sched_reset_on_fork), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("NonBlocking", "b", bus_property_get_bool, offsetof(ExecContext, non_blocking), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("StandardInput", "s", property_get_exec_input, offsetof(ExecContext, std_input), SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("StandardInputFileDescriptorName", "s", property_get_input_fdname, 0, SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("StandardInputFileDescriptorName", "s", property_get_stdio_fdname, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("StandardInputData", "ay", property_get_input_data, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("StandardOutput", "s", bus_property_get_exec_output, offsetof(ExecContext, std_output), SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("StandardOutputFileDescriptorName", "s", property_get_output_fdname, 0, SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("StandardOutputFileDescriptorName", "s", property_get_stdio_fdname, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("StandardError", "s", bus_property_get_exec_output, offsetof(ExecContext, std_error), SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("StandardErrorFileDescriptorName", "s", property_get_output_fdname, 0, SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("StandardErrorFileDescriptorName", "s", property_get_stdio_fdname, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("TTYPath", "s", NULL, offsetof(ExecContext, tty_path), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("TTYReset", "b", bus_property_get_bool, offsetof(ExecContext, tty_reset), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("TTYVHangup", "b", bus_property_get_bool, offsetof(ExecContext, tty_vhangup), SD_BUS_VTABLE_PROPERTY_CONST), @@ -1838,23 +1820,29 @@ int bus_exec_context_set_transient_property( return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid file descriptor name"); if (mode != UNIT_CHECK) { + if (streq(name, "StandardInputFileDescriptorName")) { c->std_input = EXEC_INPUT_NAMED_FD; r = free_and_strdup(&c->stdio_fdname[STDIN_FILENO], s); if (r < 0) return r; + unit_write_drop_in_private_format(u, mode, name, "StandardInput=fd:%s", s); + } else if (streq(name, "StandardOutputFileDescriptorName")) { c->std_output = EXEC_OUTPUT_NAMED_FD; r = free_and_strdup(&c->stdio_fdname[STDOUT_FILENO], s); if (r < 0) return r; + unit_write_drop_in_private_format(u, mode, name, "StandardOutput=fd:%s", s); + } else if (streq(name, "StandardErrorFileDescriptorName")) { c->std_error = EXEC_OUTPUT_NAMED_FD; r = free_and_strdup(&c->stdio_fdname[STDERR_FILENO], s); if (r < 0) return r; + unit_write_drop_in_private_format(u, mode, name, "StandardError=fd:%s", s); } } diff --git a/src/core/execute.c b/src/core/execute.c index de6a9211d2..6f93f04715 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -3616,18 +3616,25 @@ const char* exec_context_fdname(const ExecContext *c, int fd_index) { assert(c); switch (fd_index) { + case STDIN_FILENO: if (c->std_input != EXEC_INPUT_NAMED_FD) return NULL; + return c->stdio_fdname[STDIN_FILENO] ?: "stdin"; + case STDOUT_FILENO: if (c->std_output != EXEC_OUTPUT_NAMED_FD) return NULL; + return c->stdio_fdname[STDOUT_FILENO] ?: "stdout"; + case STDERR_FILENO: if (c->std_error != EXEC_OUTPUT_NAMED_FD) return NULL; + return c->stdio_fdname[STDERR_FILENO] ?: "stderr"; + default: return NULL; } From e75a9ed17684f4f285b3774f83c89d9bf31cbb26 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 14:59:05 +0200 Subject: [PATCH 15/23] execute: some extra asserts In some cases we checked for fd validity already explicitly, let's do this for all our fds. --- src/core/execute.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/core/execute.c b/src/core/execute.c index 6f93f04715..7018322958 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -469,9 +469,13 @@ static int setup_input( } case EXEC_INPUT_SOCKET: + assert(socket_fd >= 0); + return dup2(socket_fd, STDIN_FILENO) < 0 ? -errno : STDIN_FILENO; case EXEC_INPUT_NAMED_FD: + assert(named_iofds[STDIN_FILENO] >= 0); + (void) fd_nonblock(named_iofds[STDIN_FILENO], false); return dup2(named_iofds[STDIN_FILENO], STDIN_FILENO) < 0 ? -errno : STDIN_FILENO; @@ -612,9 +616,12 @@ static int setup_output( case EXEC_OUTPUT_SOCKET: assert(socket_fd >= 0); + return dup2(socket_fd, fileno) < 0 ? -errno : fileno; case EXEC_OUTPUT_NAMED_FD: + assert(named_iofds[fileno] >= 0); + (void) fd_nonblock(named_iofds[fileno], false); return dup2(named_iofds[fileno], fileno) < 0 ? -errno : fileno; From 0664775c846f4ff1b53ee6e93d5a1aef68912dad Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 16:08:11 +0200 Subject: [PATCH 16/23] core: fix handling of transient StandardOutputFileDescriptorName= and friends Let's make sure to process the fdname first, before changing the actual input/output setting, since the fdname part can fail due to OOM. This way we don't leave half-initialized bits around. --- src/core/dbus-execute.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 61341e0451..7a9d027858 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1816,34 +1816,38 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (!fdname_is_valid(s)) + if (isempty(s)) + s = NULL; + else if (!fdname_is_valid(s)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid file descriptor name"); if (mode != UNIT_CHECK) { if (streq(name, "StandardInputFileDescriptorName")) { - c->std_input = EXEC_INPUT_NAMED_FD; - r = free_and_strdup(&c->stdio_fdname[STDIN_FILENO], s); + r = free_and_strdup(c->stdio_fdname + STDIN_FILENO, s); if (r < 0) return r; - unit_write_drop_in_private_format(u, mode, name, "StandardInput=fd:%s", s); + c->std_input = EXEC_INPUT_NAMED_FD; + unit_write_drop_in_private_format(u, mode, name, "StandardInput=fd:%s", exec_context_fdname(c, STDIN_FILENO)); } else if (streq(name, "StandardOutputFileDescriptorName")) { - c->std_output = EXEC_OUTPUT_NAMED_FD; - r = free_and_strdup(&c->stdio_fdname[STDOUT_FILENO], s); + r = free_and_strdup(c->stdio_fdname + STDOUT_FILENO, s); if (r < 0) return r; - unit_write_drop_in_private_format(u, mode, name, "StandardOutput=fd:%s", s); + c->std_output = EXEC_OUTPUT_NAMED_FD; + unit_write_drop_in_private_format(u, mode, name, "StandardOutput=fd:%s", exec_context_fdname(c, STDOUT_FILENO)); + + } else { + assert(streq(name, "StandardErrorFileDescriptorName")); - } else if (streq(name, "StandardErrorFileDescriptorName")) { - c->std_error = EXEC_OUTPUT_NAMED_FD; r = free_and_strdup(&c->stdio_fdname[STDERR_FILENO], s); if (r < 0) return r; - unit_write_drop_in_private_format(u, mode, name, "StandardError=fd:%s", s); + c->std_error = EXEC_OUTPUT_NAMED_FD; + unit_write_drop_in_private_format(u, mode, name, "StandardError=fd:%s", exec_context_fdname(c, STDERR_FILENO)); } } From 2038c3f58486ea5da1a2fbf2fdb898f9fd6f8cae Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 16:09:57 +0200 Subject: [PATCH 17/23] core: add support for StandardInputFile= and friends These new settings permit specifiying arbitrary paths as stdin/stdout/stderr locations. We try to open/create them as necessary. Some special magic is applied: 1) if the same path is specified for both input and output/stderr, we'll open it only once O_RDWR, and duplicate them fd instead. 2) If we an AF_UNIX socket path is specified, we'll connect() to it, rather than open() it. This allows invoking systemd services with stdin/stdout/stderr connected to arbitrary foreign service sockets. Fixes: #3991 --- src/core/dbus-execute.c | 44 ++++++++++ src/core/execute.c | 88 ++++++++++++++++++- src/core/execute.h | 3 + src/core/load-fragment.c | 173 ++++++++++++++++++++++++------------- src/shared/bus-unit-util.c | 17 +++- 5 files changed, 263 insertions(+), 62 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 7a9d027858..47446307b7 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1853,6 +1853,50 @@ int bus_exec_context_set_transient_property( return 1; + } else if (STR_IN_SET(name, "StandardInputFile", "StandardOutputFile", "StandardErrorFile")) { + const char *s; + + r = sd_bus_message_read(message, "s", &s); + if (r < 0) + return r; + + if (!path_is_absolute(s)) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Path %s is not absolute", s); + if (!path_is_safe(s)) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Path %s is not normalized", s); + + if (mode != UNIT_CHECK) { + + if (streq(name, "StandardInputFile")) { + r = free_and_strdup(&c->stdio_file[STDIN_FILENO], s); + if (r < 0) + return r; + + c->std_input = EXEC_INPUT_FILE; + unit_write_drop_in_private_format(u, mode, name, "StandardInput=file:%s", s); + + } else if (streq(name, "StandardOutputFile")) { + r = free_and_strdup(&c->stdio_file[STDOUT_FILENO], s); + if (r < 0) + return r; + + c->std_output = EXEC_OUTPUT_FILE; + unit_write_drop_in_private_format(u, mode, name, "StandardOutput=file:%s", s); + + } else { + assert(streq(name, "StandardErrorFile")); + + r = free_and_strdup(&c->stdio_file[STDERR_FILENO], s); + if (r < 0) + return r; + + c->std_error = EXEC_OUTPUT_FILE; + unit_write_drop_in_private_format(u, mode, name, "StandardError=file:%s", s); + } + } + + return 1; + } else if (streq(name, "StandardInputData")) { const void *p; size_t sz; diff --git a/src/core/execute.c b/src/core/execute.c index 7018322958..7d6919f02b 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -390,6 +390,53 @@ static int open_terminal_as(const char *path, int flags, int nfd) { return move_fd(fd, nfd, false); } +static int acquire_path(const char *path, int flags, mode_t mode) { + union sockaddr_union sa = { + .sa.sa_family = AF_UNIX, + }; + int fd, r; + + assert(path); + + if (IN_SET(flags & O_ACCMODE, O_WRONLY, O_RDWR)) + flags |= O_CREAT; + + fd = open(path, flags|O_NOCTTY, mode); + if (fd >= 0) + return fd; + + if (errno != ENXIO) /* ENXIO is returned when we try to open() an AF_UNIX file system socket on Linux */ + return -errno; + if (strlen(path) > sizeof(sa.un.sun_path)) /* Too long, can't be a UNIX socket */ + return -ENXIO; + + /* So, it appears the specified path could be an AF_UNIX socket. Let's see if we can connect to it. */ + + fd = socket(AF_UNIX, SOCK_STREAM, 0); + if (fd < 0) + return -errno; + + strncpy(sa.un.sun_path, path, sizeof(sa.un.sun_path)); + if (connect(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0) { + safe_close(fd); + return errno == EINVAL ? -ENXIO : -errno; /* Propagate initial error if we get EINVAL, i.e. we have + * indication that his wasn't an AF_UNIX socket after all */ + } + + if ((flags & O_ACCMODE) == O_RDONLY) + r = shutdown(fd, SHUT_WR); + else if ((flags & O_ACCMODE) == O_WRONLY) + r = shutdown(fd, SHUT_RD); + else + return fd; + if (r < 0) { + safe_close(fd); + return -errno; + } + + return fd; +} + static int fixup_input( const ExecContext *context, int socket_fd, @@ -489,6 +536,22 @@ static int setup_input( return move_fd(fd, STDIN_FILENO, false); } + case EXEC_INPUT_FILE: { + bool rw; + int fd; + + assert(context->stdio_file[STDIN_FILENO]); + + rw = (context->std_output == EXEC_OUTPUT_FILE && streq_ptr(context->stdio_file[STDIN_FILENO], context->stdio_file[STDOUT_FILENO])) || + (context->std_error == EXEC_OUTPUT_FILE && streq_ptr(context->stdio_file[STDIN_FILENO], context->stdio_file[STDERR_FILENO])); + + fd = acquire_path(context->stdio_file[STDIN_FILENO], rw ? O_RDWR : O_RDONLY, 0666 & ~context->umask); + if (fd < 0) + return fd; + + return move_fd(fd, STDIN_FILENO, false); + } + default: assert_not_reached("Unknown input type"); } @@ -625,6 +688,25 @@ static int setup_output( (void) fd_nonblock(named_iofds[fileno], false); return dup2(named_iofds[fileno], fileno) < 0 ? -errno : fileno; + case EXEC_OUTPUT_FILE: { + bool rw; + int fd; + + assert(context->stdio_file[fileno]); + + rw = context->std_input == EXEC_INPUT_FILE && + streq_ptr(context->stdio_file[fileno], context->stdio_file[STDIN_FILENO]); + + if (rw) + return dup2(STDIN_FILENO, fileno) < 0 ? -errno : fileno; + + fd = acquire_path(context->stdio_file[fileno], O_WRONLY, 0666 & ~context->umask); + if (fd < 0) + return fd; + + return move_fd(fd, fileno, false); + } + default: assert_not_reached("Unknown error type"); } @@ -3507,8 +3589,10 @@ void exec_context_done(ExecContext *c) { for (l = 0; l < ELEMENTSOF(c->rlimit); l++) c->rlimit[l] = mfree(c->rlimit[l]); - for (l = 0; l < 3; l++) + for (l = 0; l < 3; l++) { c->stdio_fdname[l] = mfree(c->stdio_fdname[l]); + c->stdio_file[l] = mfree(c->stdio_file[l]); + } c->working_directory = mfree(c->working_directory); c->root_directory = mfree(c->root_directory); @@ -4648,6 +4732,7 @@ static const char* const exec_input_table[_EXEC_INPUT_MAX] = { [EXEC_INPUT_SOCKET] = "socket", [EXEC_INPUT_NAMED_FD] = "fd", [EXEC_INPUT_DATA] = "data", + [EXEC_INPUT_FILE] = "file", }; DEFINE_STRING_TABLE_LOOKUP(exec_input, ExecInput); @@ -4664,6 +4749,7 @@ static const char* const exec_output_table[_EXEC_OUTPUT_MAX] = { [EXEC_OUTPUT_JOURNAL_AND_CONSOLE] = "journal+console", [EXEC_OUTPUT_SOCKET] = "socket", [EXEC_OUTPUT_NAMED_FD] = "fd", + [EXEC_OUTPUT_FILE] = "file", }; DEFINE_STRING_TABLE_LOOKUP(exec_output, ExecOutput); diff --git a/src/core/execute.h b/src/core/execute.h index d1cf7dca93..2ae38aa222 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -55,6 +55,7 @@ typedef enum ExecInput { EXEC_INPUT_SOCKET, EXEC_INPUT_NAMED_FD, EXEC_INPUT_DATA, + EXEC_INPUT_FILE, _EXEC_INPUT_MAX, _EXEC_INPUT_INVALID = -1 } ExecInput; @@ -71,6 +72,7 @@ typedef enum ExecOutput { EXEC_OUTPUT_JOURNAL_AND_CONSOLE, EXEC_OUTPUT_SOCKET, EXEC_OUTPUT_NAMED_FD, + EXEC_OUTPUT_FILE, _EXEC_OUTPUT_MAX, _EXEC_OUTPUT_INVALID = -1 } ExecOutput; @@ -165,6 +167,7 @@ struct ExecContext { ExecOutput std_output; ExecOutput std_error; char *stdio_fdname[3]; + char *stdio_file[3]; void *stdin_data; size_t stdin_data_size; diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index cff3779f4b..37f0229de3 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -847,7 +847,9 @@ int config_parse_exec_input( void *userdata) { ExecContext *c = data; - const char *name; + Unit *u = userdata; + const char *n; + ExecInput ei; int r; assert(data); @@ -855,31 +857,55 @@ int config_parse_exec_input( assert(line); assert(rvalue); - name = startswith(rvalue, "fd:"); - if (name) { - /* Strip prefix and validate fd name */ - if (!fdname_is_valid(name)) { - log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid file descriptor name, ignoring: %s", name); - return 0; + n = startswith(rvalue, "fd:"); + if (n) { + _cleanup_free_ char *resolved = NULL; + + r = unit_full_printf(u, n, &resolved); + if (r < 0) + return log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers on %s: %m", n); + + if (isempty(resolved)) + resolved = mfree(resolved); + else if (!fdname_is_valid(resolved)) { + log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid file descriptor name: %s", resolved); + return -EINVAL; } - r = free_and_strdup(&c->stdio_fdname[STDIN_FILENO], name); + free_and_replace(c->stdio_fdname[STDIN_FILENO], resolved); + + ei = EXEC_INPUT_NAMED_FD; + + } else if ((n = startswith(rvalue, "file:"))) { + _cleanup_free_ char *resolved = NULL; + + r = unit_full_printf(u, n, &resolved); if (r < 0) - return log_oom(); + return log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers on %s: %m", n); + + if (!path_is_absolute(resolved)) { + log_syntax(unit, LOG_ERR, filename, line, 0, "file: requires an absolute path name: %s", resolved); + return -EINVAL; + } + + if (!path_is_safe(resolved)) { + log_syntax(unit, LOG_ERR, filename, line, 0, "file: requires a normalized path name: %s", resolved); + return -EINVAL; + } + + free_and_replace(c->stdio_file[STDIN_FILENO], resolved); + + ei = EXEC_INPUT_FILE; - c->std_input = EXEC_INPUT_NAMED_FD; } else { - ExecInput ei; - ei = exec_input_from_string(rvalue); if (ei < 0) { log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse input specifier, ignoring: %s", rvalue); return 0; } - - c->std_input = ei; } + c->std_input = ei; return 0; } @@ -915,22 +941,18 @@ int config_parse_exec_input_text( } r = cunescape(rvalue, 0, &unescaped); - if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Failed to decode C escaped text, ignoring: %s", rvalue); - return 0; - } + if (r < 0) + return log_syntax(unit, LOG_ERR, filename, line, r, "Failed to decode C escaped text: %s", rvalue); r = unit_full_printf(u, unescaped, &resolved); - if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve specifiers, ignoring: %s", unescaped); - return 0; - } + if (r < 0) + return log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve specifiers: %s", unescaped); sz = strlen(resolved); if (c->stdin_data_size + sz + 1 < c->stdin_data_size || /* check for overflow */ c->stdin_data_size + sz + 1 > EXEC_STDIN_DATA_MAX) { - log_syntax(unit, LOG_ERR, filename, line, r, "Standard input data too large (%zu), maximum of %zu permitted, ignoring.", c->stdin_data_size + sz, (size_t) EXEC_STDIN_DATA_MAX); - return 0; + log_syntax(unit, LOG_ERR, filename, line, 0, "Standard input data too large (%zu), maximum of %zu permitted, ignoring.", c->stdin_data_size + sz, (size_t) EXEC_STDIN_DATA_MAX); + return -E2BIG; } p = realloc(c->stdin_data, c->stdin_data_size + sz + 1); @@ -983,17 +1005,15 @@ int config_parse_exec_input_data( delete_chars(cleaned, WHITESPACE); r = unbase64mem(cleaned, (size_t) -1, &p, &sz); - if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Failed to decode base64 data, ignoring: %s", cleaned); - return 0; - } + if (r < 0) + return log_syntax(unit, LOG_ERR, filename, line, r, "Failed to decode base64 data, ignoring: %s", cleaned); assert(sz > 0); if (c->stdin_data_size + sz < c->stdin_data_size || /* check for overflow */ c->stdin_data_size + sz > EXEC_STDIN_DATA_MAX) { - log_syntax(unit, LOG_ERR, filename, line, r, "Standard input data too large (%zu), maximum of %zu permitted, ignoring.", c->stdin_data_size + sz, (size_t) EXEC_STDIN_DATA_MAX); - return 0; + log_syntax(unit, LOG_ERR, filename, line, 0, "Standard input data too large (%zu), maximum of %zu permitted, ignoring.", c->stdin_data_size + sz, (size_t) EXEC_STDIN_DATA_MAX); + return -E2BIG; } q = realloc(c->stdin_data, c->stdin_data_size + sz); @@ -1008,19 +1028,23 @@ int config_parse_exec_input_data( return 0; } -int config_parse_exec_output(const char *unit, - const char *filename, - unsigned line, - const char *section, - unsigned section_line, - const char *lvalue, - int ltype, - const char *rvalue, - void *data, - void *userdata) { +int config_parse_exec_output( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + + _cleanup_free_ char *resolved = NULL; + const char *n; ExecContext *c = data; + Unit *u = userdata; ExecOutput eo; - const char *name; int r; assert(data); @@ -1029,38 +1053,67 @@ int config_parse_exec_output(const char *unit, assert(lvalue); assert(rvalue); - name = startswith(rvalue, "fd:"); - if (name) { - /* Strip prefix and validate fd name */ - if (!fdname_is_valid(name)) { - log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid file descriptor name, ignoring: %s", name); - return 0; + n = startswith(rvalue, "fd:"); + if (n) { + r = unit_full_printf(u, n, &resolved); + if (r < 0) + return log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers on %s: %m", n); + + if (isempty(resolved)) + resolved = mfree(resolved); + else if (!fdname_is_valid(resolved)) { + log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid file descriptor name: %s", resolved); + return -EINVAL; } + eo = EXEC_OUTPUT_NAMED_FD; + + } else if ((n = startswith(rvalue, "file:"))) { + + r = unit_full_printf(u, n, &resolved); + if (r < 0) + return log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers on %s: %m", n); + + if (!path_is_absolute(resolved)) { + log_syntax(unit, LOG_ERR, filename, line, 0, "file: requires an absolute path name: %s", resolved); + return -EINVAL; + } + + if (!path_is_safe(resolved)) { + log_syntax(unit, LOG_ERR, filename, line, 0, "file: requires a normalized path name, ignoring: %s", resolved); + return -EINVAL; + } + + eo = EXEC_OUTPUT_FILE; + } else { eo = exec_output_from_string(rvalue); - if (eo == _EXEC_OUTPUT_INVALID) { + if (eo < 0) { log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse output specifier, ignoring: %s", rvalue); return 0; } } if (streq(lvalue, "StandardOutput")) { + if (eo == EXEC_OUTPUT_NAMED_FD) + free_and_replace(c->stdio_fdname[STDOUT_FILENO], resolved); + else + free_and_replace(c->stdio_file[STDOUT_FILENO], resolved); + c->std_output = eo; - r = free_and_strdup(&c->stdio_fdname[STDOUT_FILENO], name); - if (r < 0) - log_oom(); - return r; - } else if (streq(lvalue, "StandardError")) { - c->std_error = eo; - r = free_and_strdup(&c->stdio_fdname[STDERR_FILENO], name); - if (r < 0) - log_oom(); - return r; + } else { - log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse output property, ignoring: %s", lvalue); - return 0; + assert(streq(lvalue, "StandardError")); + + if (eo == EXEC_OUTPUT_NAMED_FD) + free_and_replace(c->stdio_fdname[STDERR_FILENO], resolved); + else + free_and_replace(c->stdio_file[STDERR_FILENO], resolved); + + c->std_error = eo; } + + return 0; } int config_parse_exec_io_class(const char *unit, diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 01beac0342..beabb3731c 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -275,6 +275,22 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen r = sd_bus_message_append(m, "sv", "TasksMax", "t", t); goto finish; + } else if (STR_IN_SET(field, "StandardInput", "StandardOutput", "StandardError")) { + const char *n, *appended; + + n = startswith(eq, "fd:"); + if (n) { + appended = strjoina(field, "FileDescriptorName"); + r = sd_bus_message_append(m, "sv", appended, "s", n); + + } else if ((n = startswith(eq, "file:"))) { + appended = strjoina(field, "File"); + r = sd_bus_message_append(m, "sv", appended, "s", n); + } else + r = sd_bus_message_append(m, "sv", field, "s", eq); + + goto finish; + } else if (streq(field, "StandardInputText")) { _cleanup_free_ char *unescaped = NULL; @@ -387,7 +403,6 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen } else if (STR_IN_SET(field, "User", "Group", "DevicePolicy", "KillMode", "UtmpIdentifier", "UtmpMode", "PAMName", "TTYPath", - "StandardInput", "StandardOutput", "StandardError", "Description", "Slice", "Type", "WorkingDirectory", "RootDirectory", "SyslogIdentifier", "ProtectSystem", "ProtectHome", "SELinuxContext", "Restart", "RootImage", From befc4a800e9dc212c4f51bb9b20052b25d808590 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 16:13:59 +0200 Subject: [PATCH 18/23] core: add exec_context_dump() support for fd: and file: stdio settings This was missing for using fdnames as stdio, let's add support for fdnames as well as file paths in one go. --- src/core/execute.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/core/execute.c b/src/core/execute.c index 7d6919f02b..fa3be18565 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -4035,6 +4035,20 @@ void exec_context_dump(ExecContext *c, FILE* f, const char *prefix) { prefix, exec_output_to_string(c->std_output), prefix, exec_output_to_string(c->std_error)); + if (c->std_input == EXEC_INPUT_NAMED_FD) + fprintf(f, "%sStandardInputFileDescriptorName: %s\n", prefix, c->stdio_fdname[STDIN_FILENO]); + if (c->std_output == EXEC_OUTPUT_NAMED_FD) + fprintf(f, "%sStandardOutputFileDescriptorName: %s\n", prefix, c->stdio_fdname[STDOUT_FILENO]); + if (c->std_error == EXEC_OUTPUT_NAMED_FD) + fprintf(f, "%sStandardErrorFileDescriptorName: %s\n", prefix, c->stdio_fdname[STDERR_FILENO]); + + if (c->std_input == EXEC_INPUT_FILE) + fprintf(f, "%sStandardInputFile: %s\n", prefix, c->stdio_file[STDIN_FILENO]); + if (c->std_output == EXEC_OUTPUT_FILE) + fprintf(f, "%sStandardOutputFile: %s\n", prefix, c->stdio_file[STDOUT_FILENO]); + if (c->std_error == EXEC_OUTPUT_FILE) + fprintf(f, "%sStandardErrorFile: %s\n", prefix, c->stdio_file[STDERR_FILENO]); + if (c->tty_path) fprintf(f, "%sTTYPath: %s\n" From 666d787787b7f7168478b73865adb7928b50cb88 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 16:16:19 +0200 Subject: [PATCH 19/23] test: add basic test for StandardInput=file: --- src/test/test-execute.c | 5 +++++ test/meson.build | 1 + test/test-execute/exec-stdin-data.service | 2 +- test/test-execute/exec-stdio-file.service | 7 +++++++ 4 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 test/test-execute/exec-stdio-file.service diff --git a/src/test/test-execute.c b/src/test/test-execute.c index 34a93fb3eb..f831654b0a 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -482,6 +482,10 @@ static void test_exec_stdin_data(Manager *m) { test(m, "exec-stdin-data.service", 0, CLD_EXITED); } +static void test_exec_stdio_file(Manager *m) { + test(m, "exec-stdio-file.service", 0, CLD_EXITED); +} + static int run_tests(UnitFileScope scope, const test_function_t *tests) { const test_function_t *test = NULL; Manager *m = NULL; @@ -539,6 +543,7 @@ int main(int argc, char *argv[]) { test_exec_read_only_path_suceed, test_exec_unset_environment, test_exec_stdin_data, + test_exec_stdio_file, NULL, }; static const test_function_t system_tests[] = { diff --git a/test/meson.build b/test/meson.build index c5e45f1958..3292c778ac 100644 --- a/test/meson.build +++ b/test/meson.build @@ -95,6 +95,7 @@ test_data_files = ''' test-execute/exec-spec-interpolation.service test-execute/exec-specifier.service test-execute/exec-stdin-data.service + test-execute/exec-stdio-file.service test-execute/exec-supplementarygroups-multiple-groups-default-group-user.service test-execute/exec-supplementarygroups-multiple-groups-withgid.service test-execute/exec-supplementarygroups-multiple-groups-withuid.service diff --git a/test/test-execute/exec-stdin-data.service b/test/test-execute/exec-stdin-data.service index 00693d5a43..1ca536ffc5 100644 --- a/test/test-execute/exec-stdin-data.service +++ b/test/test-execute/exec-stdin-data.service @@ -2,7 +2,7 @@ Description=Test for StandardInputText= and StandardInputData= [Service] -ExecStart=/bin/sh -x -c 'd=$$(mktemp -d -p /tmp); echo -e "this is a test\nand this is more\nsomething encoded!\nsomething in multiple lines\nand some more\nand a more bas64 data\nsomething with strange\nembedded\tcharacters\nand something with a exec-stdin-data.service specifier" > $d/text ; cmp $d/text' +ExecStart=/bin/sh -x -c 'd=$$(mktemp -d -p /tmp); echo -e "this is a test\nand this is more\nsomething encoded!\nsomething in multiple lines\nand some more\nand a more bas64 data\nsomething with strange\nembedded\tcharacters\nand something with a exec-stdin-data.service specifier" > $d/text ; cmp $d/text ; rm -rf $d' Type=oneshot StandardInput=data StandardInputText=this is a test diff --git a/test/test-execute/exec-stdio-file.service b/test/test-execute/exec-stdio-file.service new file mode 100644 index 0000000000..8fd11caf8e --- /dev/null +++ b/test/test-execute/exec-stdio-file.service @@ -0,0 +1,7 @@ +[Unit] +Description=Test for StandardInput=file: + +[Service] +ExecStart=/usr/bin/cmp /usr/bin/cmp +Type=oneshot +StandardInput=file:/usr/bin/cmp From 5db9818772757bd6ad431a17cecc38320211c72b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 16:22:38 +0200 Subject: [PATCH 20/23] core: don't allow DefaultStandardOutput= be set to socket/fd:/file: These three settings only make sense within the context of actual unit files, hence filter this out when applied to the per-manager default, and generate a log message about it. --- src/core/load-fragment.c | 2 -- src/core/load-fragment.h | 1 - src/core/main.c | 38 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 37f0229de3..b4405b5e21 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -832,8 +832,6 @@ int config_parse_socket_bindtodevice( return 0; } -DEFINE_CONFIG_PARSE_ENUM(config_parse_output, exec_output, ExecOutput, "Failed to parse output literal specifier"); - int config_parse_exec_input( const char *unit, const char *filename, diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h index bba1259ceb..481bab7cbd 100644 --- a/src/core/load-fragment.h +++ b/src/core/load-fragment.h @@ -46,7 +46,6 @@ int config_parse_service_type(const char *unit, const char *filename, unsigned l int config_parse_service_restart(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_socket_bindtodevice(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_exec_output(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); -int config_parse_output(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_exec_input(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_exec_input_text(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_exec_input_data(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); diff --git a/src/core/main.c b/src/core/main.c index 96cac1cd9e..1dd417749d 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -571,6 +571,40 @@ static int config_parse_show_status( return 0; } +static int config_parse_output_restricted( + const char* unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + + ExecOutput t, *eo = data; + + assert(filename); + assert(lvalue); + assert(rvalue); + assert(data); + + t = exec_output_from_string(rvalue); + if (t < 0) { + log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse output type, ignoring: %s", rvalue); + return 0; + } + + if (IN_SET(t, EXEC_OUTPUT_SOCKET, EXEC_OUTPUT_NAMED_FD, EXEC_OUTPUT_FILE)) { + log_syntax(unit, LOG_ERR, filename, line, 0, "Standard output types socket, fd:, file: are not supported as defaults, ignoring: %s", rvalue); + return 0; + } + + *eo = t; + return 0; +} + static int config_parse_crash_chvt( const char* unit, const char *filename, @@ -722,8 +756,8 @@ static int parse_config_file(void) { #endif { "Manager", "TimerSlackNSec", config_parse_nsec, 0, &arg_timer_slack_nsec }, { "Manager", "DefaultTimerAccuracySec", config_parse_sec, 0, &arg_default_timer_accuracy_usec }, - { "Manager", "DefaultStandardOutput", config_parse_output, 0, &arg_default_std_output }, - { "Manager", "DefaultStandardError", config_parse_output, 0, &arg_default_std_error }, + { "Manager", "DefaultStandardOutput", config_parse_output_restricted,0, &arg_default_std_output }, + { "Manager", "DefaultStandardError", config_parse_output_restricted,0, &arg_default_std_error }, { "Manager", "DefaultTimeoutStartSec", config_parse_sec, 0, &arg_default_timeout_start_usec }, { "Manager", "DefaultTimeoutStopSec", config_parse_sec, 0, &arg_default_timeout_stop_usec }, { "Manager", "DefaultRestartSec", config_parse_sec, 0, &arg_default_restart_usec }, From 99be45a46fc9b45046a0f7b83a9f3e321f29d2da Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 16:28:15 +0200 Subject: [PATCH 21/23] =?UTF-8?q?fs-util:=20rename=20path=5Fis=5Fsafe()=20?= =?UTF-8?q?=E2=86=92=20path=5Fis=5Fnormalized()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Already, path_is_safe() refused paths container the "." dir. Doing that isn't strictly necessary to be "safe" by most definitions of the word. But it is necessary in order to consider a path "normalized". Hence, "path_is_safe()" is slightly misleading a name, but "path_is_normalize()" is more descriptive, hence let's rename things accordingly. No functional changes. --- src/basic/cgroup-util.c | 4 ++-- src/basic/fs-util.c | 4 ++-- src/basic/path-util.c | 3 +-- src/basic/path-util.h | 2 +- src/basic/unit-name.c | 4 ++-- src/basic/user-util.c | 2 +- src/core/dbus-execute.c | 4 ++-- src/core/load-fragment.c | 6 +++--- src/core/unit.c | 2 +- src/machine/machine-dbus.c | 4 ++-- src/mount/mount-tool.c | 12 ++++++------ 11 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index b52104768f..63aee015d0 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -1278,7 +1278,7 @@ int cg_split_spec(const char *spec, char **controller, char **path) { assert(spec); if (*spec == '/') { - if (!path_is_safe(spec)) + if (!path_is_normalized(spec)) return -EINVAL; if (path) { @@ -1331,7 +1331,7 @@ int cg_split_spec(const char *spec, char **controller, char **path) { return -ENOMEM; } - if (!path_is_safe(u) || + if (!path_is_normalized(u) || !path_is_absolute(u)) { free(t); free(u); diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 946f779a32..f3cb5d207a 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -509,7 +509,7 @@ static int getenv_tmp_dir(const char **ret_path) { r = -ENOTDIR; goto next; } - if (!path_is_safe(e)) { + if (!path_is_normalized(e)) { r = -EPERM; goto next; } @@ -707,7 +707,7 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, if (errno == ENOENT && (flags & CHASE_NONEXISTENT) && - (isempty(todo) || path_is_safe(todo))) { + (isempty(todo) || path_is_normalized(todo))) { /* If CHASE_NONEXISTENT is set, and the path does not exist, then that's OK, return * what we got so far. But don't allow this if the remaining path contains "../ or "./" diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 6c06bd2acb..9fac7ff6ec 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -721,7 +721,7 @@ bool filename_is_valid(const char *p) { return true; } -bool path_is_safe(const char *p) { +bool path_is_normalized(const char *p) { if (isempty(p)) return false; @@ -735,7 +735,6 @@ bool path_is_safe(const char *p) { if (strlen(p)+1 > PATH_MAX) return false; - /* The following two checks are not really dangerous, but hey, they still are confusing */ if (startswith(p, "./") || endswith(p, "/.") || strstr(p, "/./")) return false; diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 546246595c..4b1410a21b 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -131,7 +131,7 @@ int parse_path_argument_and_warn(const char *path, bool suppress_root, char **ar char* dirname_malloc(const char *path); bool filename_is_valid(const char *p) _pure_; -bool path_is_safe(const char *p) _pure_; +bool path_is_normalized(const char *p) _pure_; char *file_in_same_dir(const char *path, const char *filename); diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index 27ce432197..d37b75b72f 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -382,7 +382,7 @@ int unit_name_path_escape(const char *f, char **ret) { if (STR_IN_SET(p, "/", "")) s = strdup("-"); else { - if (!path_is_safe(p)) + if (!path_is_normalized(p)) return -EINVAL; /* Truncate trailing slashes */ @@ -432,7 +432,7 @@ int unit_name_path_unescape(const char *f, char **ret) { if (!s) return -ENOMEM; - if (!path_is_safe(s)) { + if (!path_is_normalized(s)) { free(s); return -EINVAL; } diff --git a/src/basic/user-util.c b/src/basic/user-util.c index a691a0d3fc..f1a0958617 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -605,7 +605,7 @@ bool valid_home(const char *p) { if (!path_is_absolute(p)) return false; - if (!path_is_safe(p)) + if (!path_is_normalized(p)) return false; /* Colons are used as field separators, and hence not OK */ diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 47446307b7..2889bf96ba 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1862,7 +1862,7 @@ int bus_exec_context_set_transient_property( if (!path_is_absolute(s)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Path %s is not absolute", s); - if (!path_is_safe(s)) + if (!path_is_normalized(s)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Path %s is not normalized", s); if (mode != UNIT_CHECK) { @@ -2462,7 +2462,7 @@ int bus_exec_context_set_transient_property( return r; STRV_FOREACH(p, l) { - if (!path_is_safe(*p) || path_is_absolute(*p)) + if (!path_is_normalized(*p) || path_is_absolute(*p)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "%s= path is not valid: %s", name, *p); } diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index b4405b5e21..2784f6ba80 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -886,7 +886,7 @@ int config_parse_exec_input( return -EINVAL; } - if (!path_is_safe(resolved)) { + if (!path_is_normalized(resolved)) { log_syntax(unit, LOG_ERR, filename, line, 0, "file: requires a normalized path name: %s", resolved); return -EINVAL; } @@ -1077,7 +1077,7 @@ int config_parse_exec_output( return -EINVAL; } - if (!path_is_safe(resolved)) { + if (!path_is_normalized(resolved)) { log_syntax(unit, LOG_ERR, filename, line, 0, "file: requires a normalized path name, ignoring: %s", resolved); return -EINVAL; } @@ -4050,7 +4050,7 @@ int config_parse_exec_directories( continue; } - if (!path_is_safe(k) || path_is_absolute(k)) { + if (!path_is_normalized(k) || path_is_absolute(k)) { log_syntax(unit, LOG_ERR, filename, line, 0, "%s= path is not valid, ignoring assignment: %s", lvalue, rvalue); continue; diff --git a/src/core/unit.c b/src/core/unit.c index 25cdc04506..e6f2e23dee 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4460,7 +4460,7 @@ int unit_require_mounts_for(Unit *u, const char *path, UnitDependencyMask mask) path_kill_slashes(p); - if (!path_is_safe(p)) { + if (!path_is_normalized(p)) { free(p); return -EPERM; } diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c index 3cb90be67d..34a3778ad7 100644 --- a/src/machine/machine-dbus.c +++ b/src/machine/machine-dbus.c @@ -854,12 +854,12 @@ int bus_machine_method_bind_mount(sd_bus_message *message, void *userdata, sd_bu if (r < 0) return r; - if (!path_is_absolute(src) || !path_is_safe(src)) + if (!path_is_absolute(src) || !path_is_normalized(src)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Source path must be absolute and not contain ../."); if (isempty(dest)) dest = src; - else if (!path_is_absolute(dest) || !path_is_safe(dest)) + else if (!path_is_absolute(dest) || !path_is_normalized(dest)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Destination path must be absolute and not contain ../."); r = bus_verify_polkit_async( diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index 7bd40613c3..f6e845e04a 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -886,8 +886,8 @@ static int stop_mounts( return -EINVAL; } - if (!path_is_safe(where)) { - log_error("Path contains unsafe components: %s", where); + if (!path_is_normalized(where)) { + log_error("Path contains non-normalized components: %s", where); return -EINVAL; } @@ -1568,8 +1568,8 @@ int main(int argc, char* argv[]) { goto finish; } - if (!path_is_safe(arg_mount_what)) { - log_error("Path contains unsafe components: %s", arg_mount_what); + if (!path_is_normalized(arg_mount_what)) { + log_error("Path contains non-normalized components: %s", arg_mount_what); r = -EINVAL; goto finish; } @@ -1592,8 +1592,8 @@ int main(int argc, char* argv[]) { goto finish; } - if (!path_is_safe(arg_mount_where)) { - log_error("Path contains unsafe components: %s", arg_mount_where); + if (!path_is_normalized(arg_mount_where)) { + log_error("Path contains non-normalized components: %s", arg_mount_where); r = -EINVAL; goto finish; } From fc8d038130cedf0f1488f906a3a82930b9673a4d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 18:10:34 +0200 Subject: [PATCH 22/23] man: document all the new options we acquired --- man/systemd.exec.xml | 277 ++++++++++++++++++++++--------------------- 1 file changed, 141 insertions(+), 136 deletions(-) diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index d043555860..190d3af317 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -482,145 +482,116 @@ StandardInput= - Controls where file descriptor 0 (STDIN) of - the executed processes is connected to. Takes one of - , - , - , - , - or - . + Controls where file descriptor 0 (STDIN) of the executed processes is connected to. Takes one + of , , , , + , , or + . - If is selected, standard input - will be connected to /dev/null, i.e. all - read attempts by the process will result in immediate - EOF. + If is selected, standard input will be connected to /dev/null, + i.e. all read attempts by the process will result in immediate EOF. - If is selected, standard input is - connected to a TTY (as configured by - TTYPath=, see below) and the executed - process becomes the controlling process of the terminal. If - the terminal is already being controlled by another process, - the executed process waits until the current controlling - process releases the terminal. + If is selected, standard input is connected to a TTY (as configured by + TTYPath=, see below) and the executed process becomes the controlling process of the + terminal. If the terminal is already being controlled by another process, the executed process waits until the + current controlling process releases the terminal. - is similar to - , but the executed process is forcefully - and immediately made the controlling process of the terminal, - potentially removing previous controlling processes from the - terminal. + is similar to , but the executed process is forcefully and + immediately made the controlling process of the terminal, potentially removing previous controlling processes + from the terminal. - is similar to - but if the terminal already has a - controlling process start-up of the executed process - fails. + is similar to , but if the terminal already has a + controlling process start-up of the executed process fails. - The option is only valid in - socket-activated services, and only when the socket - configuration file (see - systemd.socket5 - for details) specifies a single socket only. If this option is - set, standard input will be connected to the socket the - service was activated from, which is primarily useful for - compatibility with daemons designed for use with the - traditional - inetd8 + The option may be used to configure arbitrary textual or binary data to pass via + standard input to the executed process. The data to pass is configured via + StandardInputText=/StandardInputData= (see below). Note that the actual + file descriptor type passed (memory file, regular file, UNIX pipe, …) might depend on the kernel and available + privileges. In any case, the file descriptor is read-only, and when read returns the specified data + followed by EOF. + + The option may be used to connect a specific file + system object to standard input. An absolute path following the : character is expected, + which may refer to a regular file, a FIFO or special file. If an AF_UNIX socket in the + file system is specified, a stream socket is connected to it. The latter is useful for connecting standard + input of processes to arbitrary system services. + + The option is valid in socket-activated services only, and requires the relevant + socket unit file (see + systemd.socket5 for details) + to have Accept=yes set, or to specify a single socket only. If this option is set, standard + input will be connected to the socket the service was activated from, which is primarily useful for + compatibility with daemons designed for use with the traditional inetd8 socket activation daemon. - The option connects - the input stream to a single file descriptor provided by a socket unit. - A custom named file descriptor can be specified as part of this option, - after a : (e.g. fd:foobar). - If no name is specified, stdin is assumed - (i.e. fd is equivalent to fd:stdin). - At least one socket unit defining such name must be explicitly provided via the - Sockets= option, and file descriptor name may differ - from the name of its containing socket unit. - If multiple matches are found, the first one will be used. - See FileDescriptorName= in - systemd.socket5 - for more details about named descriptors and ordering. + The option connects standard input to a specific, + named file descriptor provided by a socket unit. The name may be specified as part of this option, following a + : character (e.g. fd:foobar). If no name is specified, the name + stdin is implied (i.e. fd is equivalent to fd:stdin). + At least one socket unit defining the specified name must be provided via the Sockets= + option, and the file descriptor name may differ from the name of its containing socket unit. If multiple + matches are found, the first one will be used. See FileDescriptorName= in + systemd.socket5 for more + details about named file descriptors and their ordering. - This setting defaults to - . + This setting defaults to . StandardOutput= - Controls where file descriptor 1 (STDOUT) of - the executed processes is connected to. Takes one of - , - , - , - , - , - , - , - , - , - or - . + Controls where file descriptor 1 (STDOUT) of the executed processes is connected to. Takes one + of , , , , + , , , + , , + , or + . - duplicates the file descriptor - of standard input for standard output. + duplicates the file descriptor of standard input for standard output. - connects standard output to - /dev/null, i.e. everything written to it - will be lost. + connects standard output to /dev/null, i.e. everything written + to it will be lost. - connects standard output to a tty - (as configured via TTYPath=, see below). If - the TTY is used for output only, the executed process will not - become the controlling process of the terminal, and will not - fail or wait for other processes to release the - terminal. + connects standard output to a tty (as configured via TTYPath=, + see below). If the TTY is used for output only, the executed process will not become the controlling process of + the terminal, and will not fail or wait for other processes to release the terminal. - connects standard output with - the journal which is accessible via - journalctl1. - Note that everything that is written to syslog or kmsg (see - below) is implicitly stored in the journal as well, the - specific two options listed below are hence supersets of this - one. + connects standard output with the journal which is accessible via + journalctl1. Note that + everything that is written to syslog or kmsg (see below) is implicitly stored in the journal as well, the + specific two options listed below are hence supersets of this one. - connects standard output to the - syslog3 - system syslog service, in addition to the journal. Note that - the journal daemon is usually configured to forward everything - it receives to syslog anyway, in which case this option is no - different from . + connects standard output to the syslog3 system syslog + service, in addition to the journal. Note that the journal daemon is usually configured to forward everything + it receives to syslog anyway, in which case this option is no different from . - connects standard output with the - kernel log buffer which is accessible via + connects standard output with the kernel log buffer which is accessible via dmesg1, - in addition to the journal. The journal daemon might be - configured to send all logs to kmsg anyway, in which case this - option is no different from . + in addition to the journal. The journal daemon might be configured to send all logs to kmsg anyway, in which + case this option is no different from . - , - and - work in a similar way as the - three options above but copy the output to the system console - as well. + , and work + in a similar way as the three options above but copy the output to the system console as well. - connects standard output to a - socket acquired via socket activation. The semantics are - similar to the same option of - StandardInput=. + The option may be used to connect a specific file + system object to standard output. The semantics are similar to the same option of + StandardInputText=, see above. If standard input and output are directed to the same file + path, it is opened only once, for reading as well as writing and duplicated. This is particular useful when the + specified path refers to an AF_UNIX socket in the file system, as in that case only a + single stream connection is created for both input and output. - The option connects - the output stream to a single file descriptor provided by a socket unit. - A custom named file descriptor can be specified as part of this option, - after a : (e.g. fd:foobar). - If no name is specified, stdout is assumed - (i.e. fd is equivalent to fd:stdout). - At least one socket unit defining such name must be explicitly provided via the - Sockets= option, and file descriptor name may differ - from the name of its containing socket unit. - If multiple matches are found, the first one will be used. - See FileDescriptorName= in - systemd.socket5 - for more details about named descriptors and ordering. + connects standard output to a socket acquired via socket activation. The + semantics are similar to the same option of StandardInput=, see above. + + The option connects standard output to a specific, + named file descriptor provided by a socket unit. A name may be specified as part of this option, following a + : character (e.g. fd:foobar). If no name is + specified, the name stdout is implied (i.e. fd is equivalent to + fd:stdout). At least one socket unit defining the specified name must be provided via the + Sockets= option, and the file descriptor name may differ from the name of its containing socket + unit. If multiple matches are found, the first one will be used. See FileDescriptorName= + in systemd.socket5 for more + details about named descriptors and their ordering. If the standard output (or error output, see below) of a unit is connected to the journal, syslog or the kernel log buffer, the unit will implicitly gain a dependency of type After= on @@ -630,32 +601,66 @@ "hello" > /dev/stderr for writing text to stderr will not work. To mitigate this use the construct echo "hello" >&2 instead, which is mostly equivalent and avoids this pitfall. - This setting defaults to the value set with - DefaultStandardOutput= in - systemd-system.conf5, - which defaults to . Note that setting - this parameter might result in additional dependencies to be - added to the unit (see above). + This setting defaults to the value set with DefaultStandardOutput= in + systemd-system.conf5, which + defaults to . Note that setting this parameter might result in additional dependencies + to be added to the unit (see above). StandardError= - Controls where file descriptor 2 (STDERR) of - the executed processes is connected to. The available options - are identical to those of StandardOutput=, - with some exceptions: if set to the - file descriptor used for standard output is duplicated for - standard error, while operates on the error - stream and will look by default for a descriptor named + Controls where file descriptor 2 (STDERR) of the executed processes is connected to. The + available options are identical to those of StandardOutput=, with some exceptions: if set to + the file descriptor used for standard output is duplicated for standard error, while + will use a default file descriptor name of stderr. - This setting defaults to the value set with - DefaultStandardError= in - systemd-system.conf5, - which defaults to . Note that setting - this parameter might result in additional dependencies to be - added to the unit (see above). + This setting defaults to the value set with DefaultStandardError= in + systemd-system.conf5, which + defaults to . Note that setting this parameter might result in additional dependencies + to be added to the unit (see above). + + + + StandardInputText= + StandardInputData= + + Configures arbitrary textual or binary data to pass via file descriptor 0 (STDIN) to the + executed processes. These settings have no effect unless StandardInput= is set to + . Use this option to embed process input data directly in the unit file. + + StandardInputText= accepts arbitrary textual data. C-style escapes for special + characters as well as the usual %-specifiers are resolved. Each time this setting is used + the the specified text is appended to the per-unit data buffer, followed by a newline character (thus every use + appends a new line to the end of the buffer). Note that leading and trailing whitespace of lines configured + with this option is removed. If an empty line is specified the buffer is cleared (hence, in order to insert an + empty line, add an additional \n to the end or beginning of a line). + + StandardInputData= accepts arbitrary binary data, encoded in Base64. No escape sequences or specifiers are + resolved. Any whitespace in the encoded version is ignored during decoding. + + Note that StandardInputText= and StandardInputData= operate on the + same data buffer, and may be mixed in order to configure both binary and textual data for the same input + stream. The textual or binary data is joined strictly in the order the settings appear in the unit + file. Assigning an empty string to either will reset the data buffer. + + Please keep in mind that in order to maintain readability long unit file settings may be split into + multiple lines, by suffixing each line (except for the last) with a \ character (see + systemd.unit5 for + details). This is particularly useful for large data configured with these two options. Example: + + … +StandardInput=data +StandardInputData=SWNrIHNpdHplIGRhIHVuJyBlc3NlIEtsb3BzLAp1ZmYgZWVtYWwga2xvcHAncy4KSWNrIGtpZWtl \ + LCBzdGF1bmUsIHd1bmRyZSBtaXIsCnVmZiBlZW1hbCBqZWh0IHNlIHVmZiBkaWUgVMO8ci4KTmFu \ + dSwgZGVuayBpY2ssIGljayBkZW5rIG5hbnUhCkpldHogaXNzZSB1ZmYsIGVyc2NodCB3YXIgc2Ug \ + enUhCkljayBqZWhlIHJhdXMgdW5kIGJsaWNrZSDigJQKdW5kIHdlciBzdGVodCBkcmF1w59lbj8g \ + SWNrZSEK +… + + From 370f9c21b9560883ccbbf1cea4c94c9a6b81d8bd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 19:01:21 +0200 Subject: [PATCH 23/23] update TODO --- TODO | 37 ++++++++++++------------------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/TODO b/TODO index 553575d087..9bd06dca47 100644 --- a/TODO +++ b/TODO @@ -28,13 +28,6 @@ Features: * Add NetworkNamespacePath= to specify a path to a network namespace -* Add StandardInputData= and StandardInputText= for putting together data to - pass to a service through stdin - -* Add StandardInputPath=, StandardOutputPath=, StandardErrorPath= to connect a - service to a specific file. Be smart, and if the specified path refers to an - AF_UNIX socket, connect() to it. - * maybe use SOURCE_DATE_EPOCH (i.e. the env var the reproducible builds folks introduced) as the RTC epoch, instead of the mtime of NEWS. @@ -47,6 +40,9 @@ Features: * document Environment=SYSTEMD_LOG_LEVEL=debug drop-in in debugging document +* rework ExecOutput and ExecInput enums so that EXEC_OUTPUT_NULL loses its + magic meaning and is no longer upgraded to something else if set explicitly. + * add a way to remove fds from the fdstore by name, and make logind use it * in the long run: permit a system with /etc/machine-id linked to /dev/null, to @@ -95,9 +91,18 @@ Features: taken if multiple dirs are configured. Maybe avoid setting the env vars in that case? +* introduce SuccessAction= that permits shutting down the system when a service + succeeds. This is useful to replace "ExecPost=/usr/bin/systemctl poweroff" and + similar constructs, which are frequently used. This is particularly nice for + implementation of a systemd.run= kernel command line option that runs some + command and immediately shuts down. + * 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 @@ -106,12 +111,6 @@ Features: * replace all uses of fgets() + LINE_MAX by read_line() -* set IPAddressDeny=any on all services that shouldn't do networking (possibly - combined with IPAddressAllow=localhost). - -* dissect: when we discover squashfs, don't claim we had a "writable" partition - in systemd-dissect - * Add AddUser= setting to unit files, similar to DynamicUser=1 which however creates a static, persistent user rather than a dynamic, transient user. We can leverage code from sysusers.d for this. @@ -146,15 +145,6 @@ Features: --as-pid2 switch, and sanely proxy sd_notify() messages dropping stuff such as MAINPID. -* change the dependency Set* objects in Unit structures to become Hashmap*, and - then store a bit mask who created a specific dependency: the source unit via - fragment configuration, the destination unit via fragment configuration, or - the source unit via udev rules (in case of .device units), or any combination - thereof. This information can then be used to flush out old udev-created - dependencies when the udev properties change, and eventually to implement a - "systemctl refresh" operation for reloading the configuration of individual - units without reloading the whole set. - * Add ExecMonitor= setting. May be used multiple times. Forks off a process in the service cgroup, which is supposed to monitor the service, and when it exits the service is considered failed by its monitor. @@ -330,8 +320,6 @@ Features: * Rework systemctl's GetAll property parsing to use the generic bus_map_all_properties() API -* implement a per-service firewall based on net_cls - * Port various tools to make use of verbs.[ch], where applicable: busctl, coredumpctl, hostnamectl, localectl, systemd-analyze, timedatectl @@ -685,7 +673,6 @@ Features: - document that deps in [Unit] sections ignore Alias= fields in [Install] units of other units, unless those units are disabled - man: clarify that time-sync.target is not only sysv compat but also useful otherwise. Same for similar targets - - document the exit codes when services fail before they are exec()ed - document that service reload may be implemented as service reexec - document in wiki how to map ical recurrence events to systemd timer unit calendar specifications - add a man page containing packaging guidelines and recommending usage of things like Documentation=, PrivateTmp=, PrivateNetwork= and ReadOnlyDirectories=/etc /usr.