audit-util: return -ENODATA from audit_{session|loginuid}_from_pid() if invoked in...

The auditing subsystem is still not virtualized for containers, hence the two
values don't really make sense inside them, they will just leak
information from outside into the container. Hence don't make use of the
data if we detect we are run inside of a container.

This has visible effects: logind will no longer try to reuse the
auditing session ids as its own session ids when run inside a container.

While are at it, modernize the calls in more ways:

1. switch to pidref behaviour, all but one of our uses are using pidref
   anyway already.
2. use read_virtual_file() + proc_mounted()
3. reasonable distinguish ENOENT errors when reading the process proc
   files: distinguish the case where /proc is not mounted, from the case
   where the process is already gone, from where auditing is not enabled
   in the kernel build.

diff --git a/src/basic/audit-util.c b/src/basic/audit-util.c
index 7f86f84fa353fb86e70126ea3704ff91724878d6..1f59bba744be9c251d37c63bbdc1b45e7ca6aa50 100644 (file)
--- a/src/basic/audit-util.c
+++ b/src/basic/audit-util.c
@@ -15,60 +15,84 @@
 #include "parse-util.h"
 #include "process-util.h"
 #include "socket-util.h"
+#include "stat-util.h"
 #include "user-util.h"
+#include "virt.h"
 
-int audit_session_from_pid(pid_t pid, uint32_t *id) {
-        _cleanup_free_ char *s = NULL;
-        const char *p;
-        uint32_t u;
+static int audit_read_field(const PidRef *pid, const char *field, char **ret) {
         int r;
 
-        assert(id);
+        assert(field);
+        assert(ret);
 
-        /* We don't convert ENOENT to ESRCH here, since we can't
-         * really distinguish between "audit is not available in the
-         * kernel" and "the process does not exist", both which will
-         * result in ENOENT. */
+        if (!pidref_is_set(pid))
+                return -ESRCH;
 
-        p = procfs_file_alloca(pid, "sessionid");
+        /* Auditing is currently not virtualized for containers. Let's hence not use the audit session ID or
+         * login UID for now, it will be leaked in from the host */
+        if (detect_container() > 0)
+                return -ENODATA;
 
-        r = read_one_line_file(p, &s);
-        if (r < 0)
+        const char *p = procfs_file_alloca(pid->pid, field);
+
+        _cleanup_free_ char *s = NULL;
+        bool enoent = false;
+        r = read_virtual_file(p, SIZE_MAX, &s, /* ret_size= */ NULL);
+        if (r == -ENOENT) {
+                if (proc_mounted() == 0)
+                        return -ENOSYS;
+                enoent = true;
+        } else if (r < 0)
                 return r;
 
-        r = safe_atou32(s, &u);
+        r = pidref_verify(pid);
         if (r < 0)
                 return r;
 
-        if (!audit_session_is_valid(u))
+        if (enoent) /* We got ENOENT, but /proc/ was mounted and the PID still valid? In that case it appears
+                     * auditing is not supported by the kernel. */
                 return -ENODATA;
 
-        *id = u;
+        delete_trailing_chars(s, NEWLINE);
+
+        *ret = TAKE_PTR(s);
         return 0;
 }
 
-int audit_loginuid_from_pid(pid_t pid, uid_t *uid) {
+int audit_session_from_pid(const PidRef *pid, uint32_t *ret_id) {
         _cleanup_free_ char *s = NULL;
-        const char *p;
-        uid_t u;
         int r;
 
-        assert(uid);
-
-        p = procfs_file_alloca(pid, "loginuid");
+        r = audit_read_field(pid, "sessionid", &s);
+        if (r < 0)
+                return r;
 
-        r = read_one_line_file(p, &s);
+        uint32_t u;
+        r = safe_atou32(s, &u);
         if (r < 0)
                 return r;
 
-        r = parse_uid(s, &u);
-        if (r == -ENXIO) /* the UID was -1 */
+        if (!audit_session_is_valid(u))
                 return -ENODATA;
+
+        if (ret_id)
+                *ret_id = u;
+
+        return 0;
+}
+
+int audit_loginuid_from_pid(const PidRef *pid, uid_t *ret_uid) {
+        _cleanup_free_ char *s = NULL;
+        int r;
+
+        r = audit_read_field(pid, "loginuid", &s);
         if (r < 0)
                 return r;
 
-        *uid = u;
-        return 0;
+        if (streq(s, "4294967295")) /* loginuid as 4294967295 means not part of any session. */
+                return -ENODATA;
+
+        return parse_uid(s, ret_uid);
 }
 
 static int try_audit_request(int fd) {

diff --git a/src/basic/audit-util.h b/src/basic/audit-util.h
index 964082bac6640014981059316c47dae1bfc9323c..9a74e4f1023a96b95255b852d71f41dd0bcc4e55 100644 (file)
--- a/src/basic/audit-util.h
+++ b/src/basic/audit-util.h
@@ -5,10 +5,12 @@
 #include <stdint.h>
 #include <sys/types.h>
 
+#include "pidref.h"
+
 #define AUDIT_SESSION_INVALID UINT32_MAX
 
-int audit_session_from_pid(pid_t pid, uint32_t *id);
-int audit_loginuid_from_pid(pid_t pid, uid_t *uid);
+int audit_session_from_pid(const PidRef *pid, uint32_t *id);
+int audit_loginuid_from_pid(const PidRef *pid, uid_t *uid);
 
 bool use_audit(void);
 

diff --git a/src/journal/journald-context.c b/src/journal/journald-context.c
index b44f70006b5e22b86c6f4c787ae24e601cfca7c2..c48ad81f37349846f7da33864ef4f5be9f14d878 100644 (file)
--- a/src/journal/journald-context.c
+++ b/src/journal/journald-context.c
@@ -526,8 +526,8 @@ static void client_context_really_refresh(
         client_context_read_basic(c);
         (void) client_context_read_label(c, label, label_size);
 
-        (void) audit_session_from_pid(c->pid, &c->auditid);
-        (void) audit_loginuid_from_pid(c->pid, &c->loginuid);
+        (void) audit_session_from_pid(&PIDREF_MAKE_FROM_PID(c->pid), &c->auditid);
+        (void) audit_loginuid_from_pid(&PIDREF_MAKE_FROM_PID(c->pid), &c->loginuid);
 
         (void) client_context_read_cgroup(s, c, unit_id);
         (void) client_context_read_invocation_id(s, c);

diff --git a/src/libsystemd/sd-bus/bus-creds.c b/src/libsystemd/sd-bus/bus-creds.c
index adbdeaa210e6e1e31bf9fcfc6842078f3d9b2c00..4a4f4f882f531e4203163365363196c97297d83a 100644 (file)
--- a/src/libsystemd/sd-bus/bus-creds.c
+++ b/src/libsystemd/sd-bus/bus-creds.c
@@ -1118,7 +1118,7 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, PidRef *pidref, pid_t tid
         }
 
         if (missing & SD_BUS_CREDS_AUDIT_SESSION_ID) {
-                r = audit_session_from_pid(pidref->pid, &c->audit_session_id);
+                r = audit_session_from_pid(pidref, &c->audit_session_id);
                 if (r == -ENODATA) {
                         /* ENODATA means: no audit session id assigned */
                         c->audit_session_id = AUDIT_SESSION_INVALID;
@@ -1131,7 +1131,7 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, PidRef *pidref, pid_t tid
         }
 
         if (missing & SD_BUS_CREDS_AUDIT_LOGIN_UID) {
-                r = audit_loginuid_from_pid(pidref->pid, &c->audit_login_uid);
+                r = audit_loginuid_from_pid(pidref, &c->audit_login_uid);
                 if (r == -ENODATA) {
                         /* ENODATA means: no audit login uid assigned */
                         c->audit_login_uid = UID_INVALID;

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 1b6e358b7419caf37945f817dd73570ca562ac79..80a2470a71f747d8c24d7a19dc7e9d4ef0ce9533 100644 (file)
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -1006,7 +1006,7 @@ static int create_session(
                                          "Maximum number of sessions (%" PRIu64 ") reached, refusing further sessions.",
                                          m->sessions_max);
 
-        (void) audit_session_from_pid(leader.pid, &audit_id);
+        (void) audit_session_from_pid(&leader, &audit_id);
         if (audit_session_is_valid(audit_id)) {
                 /* Keep our session IDs and the audit session IDs in sync */
 

diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 7f359864dc808f3860e92fe12f3444159f147b6b..26ed2d3e491c3900689ccc97bc21e838eae8965c 100644 (file)
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -254,7 +254,7 @@ int session_set_leader_consume(Session *s, PidRef _leader) {
                         s->leader_fd_saved = true;
         }
 
-        (void) audit_session_from_pid(s->leader.pid, &s->audit_id);
+        (void) audit_session_from_pid(&s->leader, &s->audit_id);
 
         return 1;
 }

diff --git a/src/test/meson.build b/src/test/meson.build
index 7d73e860c1f8b66c56bb04e901ce09cdd40ae4db..9dae4996f48ea56dfac281999202cb79737f7c49 100644 (file)
--- a/src/test/meson.build
+++ b/src/test/meson.build
@@ -46,6 +46,7 @@ simple_tests += files(
         'test-alloc-util.c',
         'test-architecture.c',
         'test-argv-util.c',
+        'test-audit-util.c',
         'test-barrier.c',
         'test-bitfield.c',
         'test-bitmap.c',

diff --git a/src/test/test-audit-util.c b/src/test/test-audit-util.c
new file mode 100644 (file)
index 0000000..242a3f1
--- /dev/null
+++ b/src/test/test-audit-util.c
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "audit-util.h"
+#include "tests.h"
+
+TEST(audit_loginuid_from_pid) {
+        _cleanup_(pidref_done) PidRef self = PIDREF_NULL, pid1 = PIDREF_NULL;
+        int r;
+
+        assert_se(pidref_set_self(&self) >= 0);
+        assert_se(pidref_set_pid(&pid1, 1) >= 0);
+
+        uid_t uid;
+        r = audit_loginuid_from_pid(&self, &uid);
+        assert_se(r >= 0 || r == -ENODATA);
+        if (r >= 0)
+                log_info("self audit login uid: " UID_FMT, uid);
+
+        assert_se(audit_loginuid_from_pid(&pid1, &uid) == -ENODATA);
+
+        uint32_t sessionid;
+        r = audit_session_from_pid(&self, &sessionid);
+        assert_se(r >= 0 || r == -ENODATA);
+        if (r >= 0)
+                log_info("self audit session id: %" PRIu32, sessionid);
+
+        assert_se(audit_session_from_pid(&pid1, &sessionid) == -ENODATA);
+}
+
+static int intro(void) {
+        log_show_color(true);
+        return EXIT_SUCCESS;
+}
+
+DEFINE_TEST_MAIN_WITH_INTRO(LOG_INFO, intro);