From a42cf9af339f48f633fa0b17a960e1e407b7450f Mon Sep 17 00:00:00 2001

From: Lorenz Bauer <lmb@cloudflare.com>

Date: Mon, 4 Nov 2019 16:35:46 +0000

Subject: [PATCH] journal: refresh cached credentials of stdout streams

journald assumes that getsockopt(SO_PEERCRED) correctly identifies the

process on the remote end of the socket. However, this is incorrect

according to man 7 socket:

    The returned  credentials  are  those that were in effect at the

    time of the call to connect(2) or socketpair(2).

This becomes a problem when a new process inherits the stdout stream

from a parent. First, log messages from the child process will

be attributed to the parent. Second, the struct ucred used by journald

becomes invalid as soon as the parent exits. Further sendmsg calls then

fail with ENOENT. Logs for the child process then vanish from the journal.

Fix this by using recvmsg on the stdout stream, and refreshing the cached

struct ucred if SCM_CREDENTIALS indicate a new process.

Fixes #13708

(cherry picked from commit 09d0b46ab61bebafe5bdc1be95ee153dfb13d6bc)

Resolves: #1931806

---

 src/journal/journald-stream.c        | 49 ++++++++++++++++++++++++++--

 test/TEST-04-JOURNAL/test-journal.sh | 13 ++++++++

 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c

index 6f8a4011ff..302a82d3d7 100644

--- a/src/journal/journald-stream.c

+++ b/src/journal/journald-stream.c

@@ -484,11 +484,22 @@ static int stdout_stream_scan(StdoutStream *s, bool force_flush) {

 }

 static int stdout_stream_process(sd_event_source *es, int fd, uint32_t revents, void *userdata) {

+        uint8_t buf[CMSG_SPACE(sizeof(struct ucred))];

         StdoutStream *s = userdata;

+        struct ucred *ucred = NULL;

+        struct cmsghdr *cmsg;

+        struct iovec iovec;

         size_t limit;

         ssize_t l;

         int r;

+        struct msghdr msghdr = {

+                .msg_iov = &iovec,

+                .msg_iovlen = 1,

+                .msg_control = buf,

+                .msg_controllen = sizeof(buf),

+        };

+

         assert(s);

         if ((revents|EPOLLIN|EPOLLHUP) != (EPOLLIN|EPOLLHUP)) {

@@ -508,20 +519,50 @@ static int stdout_stream_process(sd_event_source *es, int fd, uint32_t revents,

          * always leave room for a terminating NUL we might need to add. */

         limit = MIN(s->allocated - 1, s->server->line_max);

-        l = read(s->fd, s->buffer + s->length, limit - s->length);

+        iovec = IOVEC_MAKE(s->buffer + s->length, limit - s->length);

+

+        l = recvmsg(s->fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);

         if (l < 0) {

-                if (errno == EAGAIN)

+                if (IN_SET(errno, EINTR, EAGAIN))

                         return 0;

                 log_warning_errno(errno, "Failed to read from stream: %m");

                 goto terminate;

         }

+        cmsg_close_all(&msghdr);

         if (l == 0) {

                 stdout_stream_scan(s, true);

                 goto terminate;

         }

+        CMSG_FOREACH(cmsg, &msghdr)

+                if (cmsg->cmsg_level == SOL_SOCKET &&

+                    cmsg->cmsg_type == SCM_CREDENTIALS &&

+                    cmsg->cmsg_len == CMSG_LEN(sizeof(struct ucred))) {

+                        ucred = (struct ucred *)CMSG_DATA(cmsg);

+                        break;

+                }

+

+        /* Invalidate the context if the pid of the sender changed.

+         * This happens when a forked process inherits stdout / stderr

+         * from a parent. In this case getpeercred returns the ucred

+         * of the parent, which can be invalid if the parent has exited

+         * in the meantime.

+         */

+        if (ucred && ucred->pid != s->ucred.pid) {

+                /* force out any previously half-written lines from a

+                 * different process, before we switch to the new ucred

+                 * structure for everything we just added */

+                r = stdout_stream_scan(s, true);

+                if (r < 0)

+                        goto terminate;

+

+                s->ucred = *ucred;

+                client_context_release(s->server, s->context);

+                s->context = NULL;

+        }

+

         s->length += l;

         r = stdout_stream_scan(s, false);

         if (r < 0)

@@ -559,6 +600,10 @@ int stdout_stream_install(Server *s, int fd, StdoutStream **ret) {

         if (r < 0)

                 return log_error_errno(r, "Failed to determine peer credentials: %m");

+        r = setsockopt_int(fd, SOL_SOCKET, SO_PASSCRED, true);

+        if (r < 0)

+                return log_error_errno(r, "SO_PASSCRED failed: %m");

+

         if (mac_selinux_use()) {

                 r = getpeersec(fd, &stream->label);

                 if (r < 0 && r != -EOPNOTSUPP)

diff --git a/test/TEST-04-JOURNAL/test-journal.sh b/test/TEST-04-JOURNAL/test-journal.sh

index 260cae09ab..52a6ee84d1 100755

--- a/test/TEST-04-JOURNAL/test-journal.sh

+++ b/test/TEST-04-JOURNAL/test-journal.sh

@@ -63,6 +63,19 @@ grep -q '^PRIORITY=6$' /output

 ! grep -q '^FOO=' /output

 ! grep -q '^SYSLOG_FACILITY=' /output

+# https://github.com/systemd/systemd/issues/13708

+ID=$(journalctl --new-id128 | sed -n 2p)

+systemd-cat -t "$ID" bash -c 'echo parent; (echo child) & wait' &

+PID=$!

+wait %%

+journalctl --sync

+# We can drop this grep when https://github.com/systemd/systemd/issues/13937

+# has a fix.

+journalctl -b -o export -t "$ID" --output-fields=_PID | grep '^_PID=' >/output

+[[ `grep -c . /output` -eq 2 ]]

+grep -q "^_PID=$PID" /output

+grep -vq "^_PID=$PID" /output

+

 # Don't lose streams on restart

 systemctl start forever-print-hola

 sleep 3