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

2 files changed, 63 insertions, 2 deletions

diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c
index afebadeccc..22a70ce5a6 100644
--- a/src/journal/journald-stream.c
+++ b/src/journal/journald-stream.c
@@ -487,11 +487,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)) {
@@ -511,20 +522,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)
@@ -562,6 +603,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 4e539aa151..de27eb0064 100755
--- a/test/TEST-04-JOURNAL/test-journal.sh
+++ b/test/TEST-04-JOURNAL/test-journal.sh
@@ -74,6 +74,22 @@ cmp /expected /output
 { journalctl -ball -b  -m 2>&1 || :; } | head -1 > /output
 cmp /expected /output
 
+# https://github.com/systemd/systemd/issues/13708
+ID=$(systemd-id128 new)
+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
+
+# Add new tests before here, the journald restarts below
+# may make tests flappy.
+
 # Don't lose streams on restart
 systemctl start forever-print-hola
 sleep 3