[systemd-devel] [RFC PATCH] journal: pass uid.gid in the stream header
Lennart Poettering
lennart at poettering.net
Mon Jan 5 05:12:45 PST 2015
On Thu, 01.01.15 04:40, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:
Sounds generally OK.
> A disadvantage of the solution implemented here, otoh, is that both
> systemd and journald must be restarted for it to take effect.
This is something I am concerned about. This will break updates, as
restarting journald is something we cannot really do without losing
stdout/stderr of most running services. This means restarting journald
doesn't really work, but then we couldn't reexec PID1 either on
updates... Grrr...
SO_PEERCRED apparently returns the euid/egid of the original
process. The UNIX "saved" uid was invented precisely to allow
temporarily lowering the euid and later on returning to it. Maybe
that's what we should use here: if the uid to run something is is not
root: drop to the final euid temporarily, making use of the save uid
to return to root, then connecting to journald, and then returning
back to the root euid for the rest of the way. ugly, but unix.
> ---
>
> Not sure if this is the right approach. Commentes and alternative/better
> ideas welcome.
>
> src/core/execute.c | 42 ++++++++++++++++++---------------
> src/journal/journal-send.c | 12 ++++++----
> src/journal/journald-stream.c | 55 +++++++++++++++++++++++++++++++------------
> 3 files changed, 71 insertions(+), 38 deletions(-)
>
> diff --git a/src/core/execute.c b/src/core/execute.c
> index a806d42827..767a86a0e1 100644
> --- a/src/core/execute.c
> +++ b/src/core/execute.c
> @@ -219,7 +219,7 @@ static int open_null_as(int flags, int nfd) {
> return r;
> }
>
> -static int connect_logger_as(const ExecContext *context, ExecOutput output, const char *ident, const char *unit_id, int nfd) {
> +static int connect_logger_as(const ExecContext *context, ExecOutput output, const char *ident, const char *unit_id, int nfd, uid_t uid, gid_t gid) {
> int fd, r;
> union sockaddr_union sa = {
> .un.sun_family = AF_UNIX,
> @@ -255,14 +255,18 @@ static int connect_logger_as(const ExecContext *context, ExecOutput output, cons
> "%i\n"
> "%i\n"
> "%i\n"
> - "%i\n",
> + "%i\n"
> + UID_FMT"\n"
> + GID_FMT"\n",
> context->syslog_identifier ? context->syslog_identifier : ident,
> unit_id,
> context->syslog_priority,
> !!context->syslog_level_prefix,
> output == EXEC_OUTPUT_SYSLOG || output == EXEC_OUTPUT_SYSLOG_AND_CONSOLE,
> output == EXEC_OUTPUT_KMSG || output == EXEC_OUTPUT_KMSG_AND_CONSOLE,
> - is_terminal_output(output));
> + is_terminal_output(output),
> + uid == UID_INVALID ? 0 : uid,
> + gid == GID_INVALID ? 0 : gid);
>
> if (fd != nfd) {
> r = dup2(fd, nfd) < 0 ? -errno : nfd;
> @@ -358,7 +362,7 @@ static int setup_input(const ExecContext *context, int socket_fd, bool apply_tty
> }
> }
>
> -static int setup_output(const ExecContext *context, int fileno, int socket_fd, const char *ident, const char *unit_id, bool apply_tty_stdin) {
> +static int setup_output(const ExecContext *context, int fileno, int socket_fd, const char *ident, const char *unit_id, bool apply_tty_stdin, uid_t uid, gid_t gid) {
> ExecOutput o;
> ExecInput i;
> int r;
> @@ -425,7 +429,7 @@ static int setup_output(const ExecContext *context, int fileno, int socket_fd, c
> case EXEC_OUTPUT_KMSG_AND_CONSOLE:
> case EXEC_OUTPUT_JOURNAL:
> case EXEC_OUTPUT_JOURNAL_AND_CONSOLE:
> - r = connect_logger_as(context, o, ident, unit_id, fileno);
> + r = connect_logger_as(context, o, ident, unit_id, fileno, uid, gid);
> if (r < 0) {
> log_unit_struct(unit_id,
> LOG_CRIT,
> @@ -1327,6 +1331,15 @@ static int exec_child(ExecCommand *command,
> }
> }
>
> + if (context->user) {
> + username = context->user;
> + err = get_user_creds(&username, &uid, &gid, &home, &shell);
> + if (err < 0) {
> + *error = EXIT_USER;
> + return err;
> + }
> + }
> +
> /* If a socket is connected to STDIN/STDOUT/STDERR, we
> * must sure to drop O_NONBLOCK */
> if (socket_fd >= 0)
> @@ -1338,13 +1351,13 @@ static int exec_child(ExecCommand *command,
> return err;
> }
>
> - err = setup_output(context, STDOUT_FILENO, socket_fd, basename(command->path), params->unit_id, params->apply_tty_stdin);
> + err = setup_output(context, STDOUT_FILENO, socket_fd, basename(command->path), params->unit_id, params->apply_tty_stdin, uid, gid);
> if (err < 0) {
> *error = EXIT_STDOUT;
> return err;
> }
>
> - err = setup_output(context, STDERR_FILENO, socket_fd, basename(command->path), params->unit_id, params->apply_tty_stdin);
> + err = setup_output(context, STDERR_FILENO, socket_fd, basename(command->path), params->unit_id, params->apply_tty_stdin, uid, gid);
> if (err < 0) {
> *error = EXIT_STDERR;
> return err;
> @@ -1419,21 +1432,12 @@ static int exec_child(ExecCommand *command,
> if (context->utmp_id)
> utmp_put_init_process(context->utmp_id, getpid(), getsid(0), context->tty_path);
>
> - if (context->user) {
> - username = context->user;
> - err = get_user_creds(&username, &uid, &gid, &home, &shell);
> + if (context->user && is_terminal_input(context->std_input)) {
> + err = chown_terminal(STDIN_FILENO, uid);
> if (err < 0) {
> - *error = EXIT_USER;
> + *error = EXIT_STDIN;
> return err;
> }
> -
> - if (is_terminal_input(context->std_input)) {
> - err = chown_terminal(STDIN_FILENO, uid);
> - if (err < 0) {
> - *error = EXIT_STDIN;
> - return err;
> - }
> - }
> }
>
> #ifdef ENABLE_KDBUS
> diff --git a/src/journal/journal-send.c b/src/journal/journal-send.c
> index 56a96c55dd..e6e5fcd3fc 100644
> --- a/src/journal/journal-send.c
> +++ b/src/journal/journal-send.c
> @@ -437,7 +437,7 @@ _public_ int sd_journal_stream_fd(const char *identifier, int priority, int leve
> identifier = "";
>
> l = strlen(identifier);
> - header = alloca(l + 1 + 1 + 2 + 2 + 2 + 2 + 2);
> + header = alloca(l + 1 + 1 + 2 + 2 + 2 + 2 + 2 + 2 + 2);
>
> memcpy(header, identifier, l);
> header[l++] = '\n';
> @@ -446,11 +446,15 @@ _public_ int sd_journal_stream_fd(const char *identifier, int priority, int leve
> header[l++] = '\n';
> header[l++] = '0' + !!level_prefix;
> header[l++] = '\n';
> - header[l++] = '0';
> + header[l++] = '0'; /* forward to syslog */
> header[l++] = '\n';
> - header[l++] = '0';
> + header[l++] = '0'; /* forward to kmsg */
> header[l++] = '\n';
> - header[l++] = '0';
> + header[l++] = '0'; /* forward to console */
> + header[l++] = '\n';
> + header[l++] = '0'; /* UID */
> + header[l++] = '\n';
> + header[l++] = '0'; /* GID */
> header[l++] = '\n';
>
> r = loop_write(fd, header, l, false);
> diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c
> index be498d4919..0d368f6aa1 100644
> --- a/src/journal/journald-stream.c
> +++ b/src/journal/journald-stream.c
> @@ -47,6 +47,8 @@ typedef enum StdoutStreamState {
> STDOUT_STREAM_FORWARD_TO_SYSLOG,
> STDOUT_STREAM_FORWARD_TO_KMSG,
> STDOUT_STREAM_FORWARD_TO_CONSOLE,
> + STDOUT_STREAM_UID,
> + STDOUT_STREAM_GID,
> STDOUT_STREAM_RUNNING
> } StdoutStreamState;
>
> @@ -68,6 +70,7 @@ struct StdoutStream {
> bool forward_to_syslog:1;
> bool forward_to_kmsg:1;
> bool forward_to_console:1;
> + bool privileged:1;
>
> char buffer[LINE_MAX+1];
> size_t length;
> @@ -160,21 +163,17 @@ static int stdout_stream_line(StdoutStream *s, char *p) {
> return log_oom();
> }
>
> - s->state = STDOUT_STREAM_UNIT_ID;
> + s->state ++;
> return 0;
>
> case STDOUT_STREAM_UNIT_ID:
> - if (s->ucred.uid == 0) {
> - if (isempty(p))
> - s->unit_id = NULL;
> - else {
> - s->unit_id = strdup(p);
> - if (!s->unit_id)
> - return log_oom();
> - }
> + if (s->privileged && !isempty(p)) {
> + s->unit_id = strdup(p);
> + if (!s->unit_id)
> + return log_oom();
> }
>
> - s->state = STDOUT_STREAM_PRIORITY;
> + s->state ++;
> return 0;
>
> case STDOUT_STREAM_PRIORITY:
> @@ -184,7 +183,7 @@ static int stdout_stream_line(StdoutStream *s, char *p) {
> return -EINVAL;
> }
>
> - s->state = STDOUT_STREAM_LEVEL_PREFIX;
> + s->state ++;
> return 0;
>
> case STDOUT_STREAM_LEVEL_PREFIX:
> @@ -195,7 +194,7 @@ static int stdout_stream_line(StdoutStream *s, char *p) {
> }
>
> s->level_prefix = !!r;
> - s->state = STDOUT_STREAM_FORWARD_TO_SYSLOG;
> + s->state ++;
> return 0;
>
> case STDOUT_STREAM_FORWARD_TO_SYSLOG:
> @@ -206,7 +205,7 @@ static int stdout_stream_line(StdoutStream *s, char *p) {
> }
>
> s->forward_to_syslog = !!r;
> - s->state = STDOUT_STREAM_FORWARD_TO_KMSG;
> + s->state ++;
> return 0;
>
> case STDOUT_STREAM_FORWARD_TO_KMSG:
> @@ -217,7 +216,7 @@ static int stdout_stream_line(StdoutStream *s, char *p) {
> }
>
> s->forward_to_kmsg = !!r;
> - s->state = STDOUT_STREAM_FORWARD_TO_CONSOLE;
> + s->state ++;
> return 0;
>
> case STDOUT_STREAM_FORWARD_TO_CONSOLE:
> @@ -228,7 +227,31 @@ static int stdout_stream_line(StdoutStream *s, char *p) {
> }
>
> s->forward_to_console = !!r;
> - s->state = STDOUT_STREAM_RUNNING;
> + s->state ++;
> + return 0;
> +
> + case STDOUT_STREAM_UID:
> + if (s->privileged) {
> + r = parse_uid(p, &s->ucred.uid);
> + if (r < 0) {
> + log_warning("Failed to parse sender UID.");
> + return -EINVAL;
> + }
> + }
> +
> + s->state ++;
> + return 0;
> +
> + case STDOUT_STREAM_GID:
> + if (s->privileged) {
> + r = parse_gid(p, &s->ucred.gid);
> + if (r < 0) {
> + log_warning("Failed to parse sender GID.");
> + return -EINVAL;
> + }
> + }
> +
> + s->state ++;
> return 0;
>
> case STDOUT_STREAM_RUNNING:
> @@ -394,6 +417,8 @@ static int stdout_stream_new(sd_event_source *es, int listen_fd, uint32_t revent
> goto fail;
> }
>
> + stream->privileged = stream->ucred.uid == 0;
> +
> #ifdef HAVE_SELINUX
> if (mac_selinux_use()) {
> if (getpeercon(fd, &stream->security_context) < 0 && errno != ENOPROTOOPT)
> --
> 2.1.1
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Lennart
--
Lennart Poettering, Red Hat
More information about the systemd-devel
mailing list