From 7e02ee98d86702b8e7055f0a6d8e6ea48267ad23 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 7 Nov 2024 14:48:57 +0100 Subject: [PATCH 1/3] audit-util: return -ENODATA from audit_{session|loginuid}_from_pid() if invoked in a container 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. --- src/basic/audit-util.c | 76 ++++++++++++++++++++----------- src/basic/audit-util.h | 6 ++- src/journal/journald-context.c | 4 +- src/libsystemd/sd-bus/bus-creds.c | 4 +- src/login/logind-dbus.c | 2 +- src/login/logind-session.c | 2 +- src/test/meson.build | 1 + src/test/test-audit-util.c | 35 ++++++++++++++ 8 files changed, 96 insertions(+), 34 deletions(-) create mode 100644 src/test/test-audit-util.c diff --git a/src/basic/audit-util.c b/src/basic/audit-util.c index 7f86f84fa3..1f59bba744 100644 --- a/src/basic/audit-util.c +++ b/src/basic/audit-util.c @@ -15,27 +15,59 @@ #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); + 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 = pidref_verify(pid); if (r < 0) return r; + 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; + + delete_trailing_chars(s, NEWLINE); + + *ret = TAKE_PTR(s); + return 0; +} + +int audit_session_from_pid(const PidRef *pid, uint32_t *ret_id) { + _cleanup_free_ char *s = NULL; + int r; + + r = audit_read_field(pid, "sessionid", &s); + if (r < 0) + return r; + + uint32_t u; r = safe_atou32(s, &u); if (r < 0) return r; @@ -43,32 +75,24 @@ int audit_session_from_pid(pid_t pid, uint32_t *id) { if (!audit_session_is_valid(u)) return -ENODATA; - *id = u; + if (ret_id) + *ret_id = u; + return 0; } -int audit_loginuid_from_pid(pid_t pid, uid_t *uid) { +int audit_loginuid_from_pid(const PidRef *pid, uid_t *ret_uid) { _cleanup_free_ char *s = NULL; - const char *p; - uid_t u; int r; - assert(uid); - - p = procfs_file_alloca(pid, "loginuid"); - - r = read_one_line_file(p, &s); + r = audit_read_field(pid, "loginuid", &s); if (r < 0) return r; - r = parse_uid(s, &u); - if (r == -ENXIO) /* the UID was -1 */ + if (streq(s, "4294967295")) /* loginuid as 4294967295 means not part of any session. */ return -ENODATA; - if (r < 0) - return r; - *uid = u; - return 0; + 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 964082bac6..9a74e4f102 100644 --- a/src/basic/audit-util.h +++ b/src/basic/audit-util.h @@ -5,10 +5,12 @@ #include #include +#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 b44f70006b..c48ad81f37 100644 --- 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 adbdeaa210..4a4f4f882f 100644 --- 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 1b6e358b74..80a2470a71 100644 --- 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 7f359864dc..26ed2d3e49 100644 --- 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 7d73e860c1..9dae4996f4 100644 --- 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 index 0000000000..242a3f16de --- /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); From 68c554f23af37ae086e98e5f41218cfbd962d8c0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 7 Nov 2024 17:49:34 +0100 Subject: [PATCH 2/3] audit-util: modernize use_audit() a bit Use ERRNO_IS_xyz() macros where appropriate. Also, reduce indentation a bit by inverted early check. And log in more error codepaths. --- src/basic/audit-util.c | 51 +++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/src/basic/audit-util.c b/src/basic/audit-util.c index 1f59bba744..4f50339d6d 100644 --- a/src/basic/audit-util.c +++ b/src/basic/audit-util.c @@ -137,33 +137,32 @@ bool use_audit(void) { static int cached_use = -1; int r; - if (cached_use < 0) { - int fd; + if (cached_use >= 0) + return cached_use; - fd = socket(AF_NETLINK, SOCK_RAW|SOCK_CLOEXEC|SOCK_NONBLOCK, NETLINK_AUDIT); - if (fd < 0) { - cached_use = !IN_SET(errno, EAFNOSUPPORT, EPROTONOSUPPORT, EPERM); - if (!cached_use) - log_debug_errno(errno, "Won't talk to audit: %m"); - } else { - /* If we try and use the audit fd but get -ECONNREFUSED, it is because - * we are not in the initial user namespace, and the kernel does not - * have support for audit outside of the initial user namespace - * (see https://elixir.bootlin.com/linux/latest/C/ident/audit_netlink_ok). - * - * If we receive any other error, do not disable audit because we are not - * sure that the error indicates that audit will not work in general. */ - r = try_audit_request(fd); - if (r < 0) { - cached_use = r != -ECONNREFUSED; - log_debug_errno(r, cached_use ? - "Failed to make request on audit fd, ignoring: %m" : - "Won't talk to audit: %m"); - } else - cached_use = true; - - safe_close(fd); - } + _cleanup_close_ int fd = socket(AF_NETLINK, SOCK_RAW|SOCK_CLOEXEC|SOCK_NONBLOCK, NETLINK_AUDIT); + if (fd < 0) { + cached_use = !ERRNO_IS_PRIVILEGE(errno) && !ERRNO_IS_NOT_SUPPORTED(errno); + if (cached_use) + log_debug_errno(errno, "Unexpected error while creating audit socket, proceeding with its use: %m"); + else + log_debug_errno(errno, "Won't talk to audit, because feature or privilege absent: %m"); + } else { + /* If we try and use the audit fd but get -ECONNREFUSED, it is because we are not in the + * initial user namespace, and the kernel does not have support for audit outside of the + * initial user namespace (see + * https://elixir.bootlin.com/linux/latest/C/ident/audit_netlink_ok). + * + * If we receive any other error, do not disable audit because we are not sure that the error + * indicates that audit will not work in general. */ + r = try_audit_request(fd); + if (r < 0) { + cached_use = r != -ECONNREFUSED; + log_debug_errno(r, cached_use ? + "Failed to make request on audit fd, ignoring: %m" : + "Won't talk to audit: %m"); + } else + cached_use = true; } return cached_use; From 7bf0149e9b46d614c3431e39ca59805aebf71bc4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 7 Nov 2024 14:54:21 +0100 Subject: [PATCH 3/3] process-util: more gracefully handle oom adjust parsing/setting Who knows what kind of mount shenanigans people employ, let's gracefully handle parse failures of proc files, like we alway do otherwsie. --- src/basic/process-util.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index ee0edfaf94..9c159b4605 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1815,6 +1815,9 @@ int namespace_fork( int set_oom_score_adjust(int value) { char t[DECIMAL_STR_MAX(int)]; + if (!oom_score_adjust_is_valid(value)) + return -EINVAL; + xsprintf(t, "%i", value); return write_string_file("/proc/self/oom_score_adj", t, @@ -1831,11 +1834,16 @@ int get_oom_score_adjust(int *ret) { delete_trailing_chars(t, WHITESPACE); - assert_se(safe_atoi(t, &a) >= 0); - assert_se(oom_score_adjust_is_valid(a)); + r = safe_atoi(t, &a); + if (r < 0) + return r; + + if (!oom_score_adjust_is_valid(a)) + return -ENODATA; if (ret) *ret = a; + return 0; }