You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
147 lines
5.6 KiB
147 lines
5.6 KiB
8 months ago
|
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
|