From 7e8aa5c2eebd86efe9bbf36d8db1e98964611aab Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 29 Jan 2024 10:18:30 +0100 Subject: [PATCH 1/8] socket-util: start SO_PEERGROUP loop with sysconf(_SC_NGROUPS_MAX), too We do this for getgroups_malloc() hence we should do this here too, after all whether we do it for a socket peer or for ourselves doesn't make too much of a difference. --- src/basic/socket-util.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index 0a6c87b023..a9ad1cc993 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -930,12 +930,14 @@ int getpeersec(int fd, char **ret) { } int getpeergroups(int fd, gid_t **ret) { - socklen_t n = sizeof(gid_t) * 64; _cleanup_free_ gid_t *d = NULL; assert(fd >= 0); assert(ret); + long ngroups_max = sysconf(_SC_NGROUPS_MAX); + socklen_t n = sizeof(gid_t) * MAX((socklen_t) ngroups_max, 64U); + for (;;) { d = malloc(n); if (!d) @@ -953,7 +955,7 @@ int getpeergroups(int fd, gid_t **ret) { assert_se(n % sizeof(gid_t) == 0); n /= sizeof(gid_t); - if ((socklen_t) (int) n != n) + if (n > INT_MAX) return -E2BIG; *ret = TAKE_PTR(d); From 71be64064c9e22c9edcb0c25d7ccc9e3c9ed1978 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Jan 2024 16:46:09 +0100 Subject: [PATCH 2/8] sd-bus: add pidfd to the sd_bus_creds structure Let's continue with the pidfd'ification, and include pidfd in our sd_bus_creds structure tha tracks a peers credentials. --- src/libsystemd/libsystemd.sym | 6 + src/libsystemd/sd-bus/bus-control.c | 95 ++++++++--- src/libsystemd/sd-bus/bus-creds.c | 157 +++++++++++++----- src/libsystemd/sd-bus/bus-creds.h | 18 +- src/libsystemd/sd-bus/bus-dump.c | 2 + src/libsystemd/sd-bus/bus-internal.h | 1 + src/libsystemd/sd-bus/bus-message.c | 2 + src/libsystemd/sd-bus/bus-socket.c | 10 +- src/libsystemd/sd-bus/sd-bus.c | 1 + src/libsystemd/sd-bus/test-bus-creds.c | 22 ++- src/libsystemd/sd-bus/test-bus-peersockaddr.c | 16 +- src/systemd/sd-bus.h | 5 +- 12 files changed, 261 insertions(+), 74 deletions(-) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 4113920ee0..22cf48c5f8 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -834,3 +834,9 @@ global: sd_id128_get_app_specific; sd_device_enumerator_add_match_property_required; } LIBSYSTEMD_254; + +LIBSYSTEMD_256 { +global: + sd_bus_creds_get_pidfd_dup; + sd_bus_creds_new_from_pidfd; +} LIBSYSTEMD_255; diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c index 1355e41ed0..3b11d754a2 100644 --- a/src/libsystemd/sd-bus/bus-control.c +++ b/src/libsystemd/sd-bus/bus-control.c @@ -14,6 +14,7 @@ #include "bus-internal.h" #include "bus-message.h" #include "capability-util.h" +#include "fd-util.h" #include "process-util.h" #include "stdio-util.h" #include "string-util.h" @@ -430,7 +431,6 @@ _public_ int sd_bus_get_name_creds( _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply_unique = NULL, *reply = NULL; _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *c = NULL; const char *unique; - pid_t pid = 0; int r; assert_return(bus, -EINVAL); @@ -484,7 +484,8 @@ _public_ int sd_bus_get_name_creds( if (mask != 0) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - bool need_pid, need_uid, need_selinux, need_separate_calls; + bool need_pid, need_uid, need_selinux, need_separate_calls, need_pidfd, need_augment; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; c = bus_creds_new(); if (!c) @@ -498,20 +499,24 @@ _public_ int sd_bus_get_name_creds( c->mask |= SD_BUS_CREDS_UNIQUE_NAME; } - need_pid = (mask & SD_BUS_CREDS_PID) || - ((mask & SD_BUS_CREDS_AUGMENT) && - (mask & (SD_BUS_CREDS_UID|SD_BUS_CREDS_SUID|SD_BUS_CREDS_FSUID| - SD_BUS_CREDS_GID|SD_BUS_CREDS_EGID|SD_BUS_CREDS_SGID|SD_BUS_CREDS_FSGID| - SD_BUS_CREDS_SUPPLEMENTARY_GIDS| - SD_BUS_CREDS_COMM|SD_BUS_CREDS_EXE|SD_BUS_CREDS_CMDLINE| - SD_BUS_CREDS_CGROUP|SD_BUS_CREDS_UNIT|SD_BUS_CREDS_USER_UNIT|SD_BUS_CREDS_SLICE|SD_BUS_CREDS_SESSION|SD_BUS_CREDS_OWNER_UID| - SD_BUS_CREDS_EFFECTIVE_CAPS|SD_BUS_CREDS_PERMITTED_CAPS|SD_BUS_CREDS_INHERITABLE_CAPS|SD_BUS_CREDS_BOUNDING_CAPS| - SD_BUS_CREDS_SELINUX_CONTEXT| - SD_BUS_CREDS_AUDIT_SESSION_ID|SD_BUS_CREDS_AUDIT_LOGIN_UID))); + need_augment = + (mask & SD_BUS_CREDS_AUGMENT) && + (mask & (SD_BUS_CREDS_UID|SD_BUS_CREDS_SUID|SD_BUS_CREDS_FSUID| + SD_BUS_CREDS_GID|SD_BUS_CREDS_EGID|SD_BUS_CREDS_SGID|SD_BUS_CREDS_FSGID| + SD_BUS_CREDS_SUPPLEMENTARY_GIDS| + SD_BUS_CREDS_COMM|SD_BUS_CREDS_EXE|SD_BUS_CREDS_CMDLINE| + SD_BUS_CREDS_CGROUP|SD_BUS_CREDS_UNIT|SD_BUS_CREDS_USER_UNIT|SD_BUS_CREDS_SLICE|SD_BUS_CREDS_SESSION|SD_BUS_CREDS_OWNER_UID| + SD_BUS_CREDS_EFFECTIVE_CAPS|SD_BUS_CREDS_PERMITTED_CAPS|SD_BUS_CREDS_INHERITABLE_CAPS|SD_BUS_CREDS_BOUNDING_CAPS| + SD_BUS_CREDS_SELINUX_CONTEXT| + SD_BUS_CREDS_AUDIT_SESSION_ID|SD_BUS_CREDS_AUDIT_LOGIN_UID| + SD_BUS_CREDS_PIDFD)); + + need_pid = (mask & SD_BUS_CREDS_PID) || need_augment; need_uid = mask & SD_BUS_CREDS_EUID; need_selinux = mask & SD_BUS_CREDS_SELINUX_CONTEXT; + need_pidfd = (mask & SD_BUS_CREDS_PIDFD) || need_augment; - if (need_pid + need_uid + need_selinux > 1) { + if (need_pid + need_uid + need_selinux + need_pidfd > 1) { /* If we need more than one of the credentials, then use GetConnectionCredentials() */ @@ -572,7 +577,9 @@ _public_ int sd_bus_get_name_creds( if (r < 0) return r; - pid = p; + if (!pidref_is_set(&pidref)) + pidref = PIDREF_MAKE_FROM_PID(p); + if (mask & SD_BUS_CREDS_PID) { c->pid = p; c->mask |= SD_BUS_CREDS_PID; @@ -599,6 +606,26 @@ _public_ int sd_bus_get_name_creds( r = sd_bus_message_exit_container(reply); if (r < 0) return r; + } else if (need_pidfd && streq(m, "ProcessFD")) { + int fd; + + r = sd_bus_message_read(reply, "v", "h", &fd); + if (r < 0) + return r; + + pidref_done(&pidref); + r = pidref_set_pidfd(&pidref, fd); + if (r < 0) + return r; + + if (mask & SD_BUS_CREDS_PIDFD) { + fd = fcntl(fd, F_DUPFD_CLOEXEC, 3); + if (fd < 0) + return -errno; + + close_and_replace(c->pidfd, fd); + c->mask |= SD_BUS_CREDS_PIDFD; + } } else { r = sd_bus_message_skip(reply, "v"); if (r < 0) @@ -614,7 +641,7 @@ _public_ int sd_bus_get_name_creds( if (r < 0) return r; - if (need_pid && pid == 0) + if (need_pid && !pidref_is_set(&pidref)) return -EPROTO; } @@ -642,7 +669,9 @@ _public_ int sd_bus_get_name_creds( if (r < 0) return r; - pid = u; + if (!pidref_is_set(&pidref)) + pidref = PIDREF_MAKE_FROM_PID(u); + if (mask & SD_BUS_CREDS_PID) { c->pid = u; c->mask |= SD_BUS_CREDS_PID; @@ -710,9 +739,11 @@ _public_ int sd_bus_get_name_creds( } } - r = bus_creds_add_more(c, mask, pid, 0); - if (r < 0 && r != -ESRCH) /* Return the error, but ignore ESRCH which just means the process is already gone */ - return r; + if (pidref_is_set(&pidref)) { + r = bus_creds_add_more(c, mask, &pidref, 0); + if (r < 0 && r != -ESRCH) /* Return the error, but ignore ESRCH which just means the process is already gone */ + return r; + } } if (creds) @@ -765,8 +796,8 @@ not_found: _public_ int sd_bus_get_owner_creds(sd_bus *bus, uint64_t mask, sd_bus_creds **ret) { _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *c = NULL; - bool do_label, do_groups, do_sockaddr_peer; - pid_t pid = 0; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + bool do_label, do_groups, do_sockaddr_peer, do_pidfd; int r; assert_return(bus, -EINVAL); @@ -786,9 +817,10 @@ _public_ int sd_bus_get_owner_creds(sd_bus *bus, uint64_t mask, sd_bus_creds **r do_sockaddr_peer = bus->sockaddr_size_peer >= offsetof(struct sockaddr_un, sun_path) + 1 && bus->sockaddr_peer.sa.sa_family == AF_UNIX && bus->sockaddr_peer.un.sun_path[0] == 0; + do_pidfd = bus->pidfd >= 0 && (mask & SD_BUS_CREDS_PIDFD); /* Avoid allocating anything if we have no chance of returning useful data */ - if (!bus->ucred_valid && !do_label && !do_groups && !do_sockaddr_peer) + if (!bus->ucred_valid && !do_label && !do_groups && !do_sockaddr_peer && !do_pidfd) return -ENODATA; c = bus_creds_new(); @@ -797,8 +829,10 @@ _public_ int sd_bus_get_owner_creds(sd_bus *bus, uint64_t mask, sd_bus_creds **r if (bus->ucred_valid) { if (pid_is_valid(bus->ucred.pid)) { - pid = c->pid = bus->ucred.pid; + c->pid = bus->ucred.pid; c->mask |= SD_BUS_CREDS_PID & mask; + + pidref = PIDREF_MAKE_FROM_PID(c->pid); } if (uid_is_valid(bus->ucred.uid)) { @@ -859,7 +893,20 @@ _public_ int sd_bus_get_owner_creds(sd_bus *bus, uint64_t mask, sd_bus_creds **r } } - r = bus_creds_add_more(c, mask, pid, 0); + if (do_pidfd) { + c->pidfd = fcntl(bus->pidfd, F_DUPFD_CLOEXEC, 3); + if (c->pidfd < 0) + return -errno; + + pidref_done(&pidref); + r = pidref_set_pidfd(&pidref, bus->pidfd); + if (r < 0) + return r; + + c->mask |= SD_BUS_CREDS_PIDFD; + } + + r = bus_creds_add_more(c, mask, &pidref, 0); if (r < 0 && r != -ESRCH) /* If the process vanished, then don't complain, just return what we got */ return r; diff --git a/src/libsystemd/sd-bus/bus-creds.c b/src/libsystemd/sd-bus/bus-creds.c index 33ae2a00f6..adbdeaa210 100644 --- a/src/libsystemd/sd-bus/bus-creds.c +++ b/src/libsystemd/sd-bus/bus-creds.c @@ -53,6 +53,8 @@ void bus_creds_done(sd_bus_creds *c) { * below. */ strv_free(c->cmdline_array); + + safe_close(c->pidfd); } _public_ sd_bus_creds *sd_bus_creds_ref(sd_bus_creds *c) { @@ -129,44 +131,70 @@ _public_ uint64_t sd_bus_creds_get_augmented_mask(const sd_bus_creds *c) { sd_bus_creds* bus_creds_new(void) { sd_bus_creds *c; - c = new0(sd_bus_creds, 1); + c = new(sd_bus_creds, 1); if (!c) return NULL; - c->allocated = true; - c->n_ref = 1; + *c = (sd_bus_creds) { + .allocated = true, + .n_ref = 1, + SD_BUS_CREDS_INIT_FIELDS, + }; + return c; } -_public_ int sd_bus_creds_new_from_pid(sd_bus_creds **ret, pid_t pid, uint64_t mask) { +static int bus_creds_new_from_pidref(sd_bus_creds **ret, PidRef *pidref, uint64_t mask) { _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *c = NULL; int r; + assert_return(mask <= _SD_BUS_CREDS_ALL, -EOPNOTSUPP); + assert_return(ret, -EINVAL); + + c = bus_creds_new(); + if (!c) + return -ENOMEM; + + r = bus_creds_add_more(c, mask | SD_BUS_CREDS_AUGMENT, pidref, 0); + if (r < 0) + return r; + + r = pidref_verify(pidref); + if (r < 0) + return r; + + *ret = TAKE_PTR(c); + return 0; +} + +_public_ int sd_bus_creds_new_from_pid(sd_bus_creds **ret, pid_t pid, uint64_t mask) { + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + int r; + assert_return(pid >= 0, -EINVAL); assert_return(mask <= _SD_BUS_CREDS_ALL, -EOPNOTSUPP); assert_return(ret, -EINVAL); - if (pid == 0) - pid = getpid_cached(); - - c = bus_creds_new(); - if (!c) - return -ENOMEM; - - r = bus_creds_add_more(c, mask | SD_BUS_CREDS_AUGMENT, pid, 0); + r = pidref_set_pid(&pidref, pid); if (r < 0) return r; - /* Check if the process existed at all, in case we haven't - * figured that out already */ - r = pid_is_alive(pid); + return bus_creds_new_from_pidref(ret, &pidref, mask); +} + +_public_ int sd_bus_creds_new_from_pidfd(sd_bus_creds **ret, int pidfd, uint64_t mask) { + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + int r; + + assert_return(mask <= _SD_BUS_CREDS_ALL, -EOPNOTSUPP); + assert_return(ret, -EINVAL); + assert_return(pidfd >= 0, -EBADF); + + r = pidref_set_pidfd(&pidref, pidfd); if (r < 0) return r; - if (r == 0) - return -ESRCH; - *ret = TAKE_PTR(c); - return 0; + return bus_creds_new_from_pidref(ret, &pidref, mask); } _public_ int sd_bus_creds_get_uid(sd_bus_creds *c, uid_t *uid) { @@ -280,6 +308,23 @@ _public_ int sd_bus_creds_get_pid(sd_bus_creds *c, pid_t *pid) { return 0; } +_public_ int sd_bus_creds_get_pidfd_dup(sd_bus_creds *c, int *ret_fd) { + _cleanup_close_ int copy = -EBADF; + + assert_return(c, -EINVAL); + assert_return(ret_fd, -EINVAL); + + if (!(c->mask & SD_BUS_CREDS_PIDFD)) + return -ENODATA; + + copy = fcntl(c->pidfd, F_DUPFD_CLOEXEC, 3); + if (copy < 0) + return -errno; + + *ret_fd = TAKE_FD(copy); + return 0; +} + _public_ int sd_bus_creds_get_ppid(sd_bus_creds *c, pid_t *ppid) { assert_return(c, -EINVAL); assert_return(ppid, -EINVAL); @@ -750,7 +795,8 @@ static int parse_caps(sd_bus_creds *c, unsigned offset, const char *p) { return 0; } -int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { +int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, PidRef *pidref, pid_t tid) { + _cleanup_(pidref_done) PidRef pidref_buf = PIDREF_NULL; uint64_t missing; int r; @@ -761,12 +807,26 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { return 0; /* Try to retrieve PID from creds if it wasn't passed to us */ - if (pid > 0) { - c->pid = pid; + if (pidref_is_set(pidref)) { + if ((c->mask & SD_BUS_CREDS_PID) && c->pid != pidref->pid) /* Insist that things match if already set */ + return -EBUSY; + + c->pid = pidref->pid; c->mask |= SD_BUS_CREDS_PID; - } else if (c->mask & SD_BUS_CREDS_PID) - pid = c->pid; - else + } else if (c->mask & SD_BUS_CREDS_PIDFD) { + r = pidref_set_pidfd(&pidref_buf, c->pidfd); + if (r < 0) + return r; + + pidref = &pidref_buf; + + } else if (c->mask & SD_BUS_CREDS_PID) { + r = pidref_set_pid(&pidref_buf, c->pid); + if (r < 0) + return r; + + pidref = &pidref_buf; + } else /* Without pid we cannot do much... */ return 0; @@ -784,6 +844,14 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { c->mask |= SD_BUS_CREDS_TID; } + if ((missing & SD_BUS_CREDS_PIDFD) && pidref->fd >= 0) { + c->pidfd = fcntl(pidref->fd, F_DUPFD_CLOEXEC, 3); + if (c->pidfd < 0) + return -errno; + + c->mask |= SD_BUS_CREDS_PIDFD; + } + if (missing & (SD_BUS_CREDS_PPID | SD_BUS_CREDS_UID | SD_BUS_CREDS_EUID | SD_BUS_CREDS_SUID | SD_BUS_CREDS_FSUID | SD_BUS_CREDS_GID | SD_BUS_CREDS_EGID | SD_BUS_CREDS_SGID | SD_BUS_CREDS_FSGID | @@ -794,13 +862,13 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { _cleanup_fclose_ FILE *f = NULL; const char *p; - p = procfs_file_alloca(pid, "status"); + p = procfs_file_alloca(pidref->pid, "status"); f = fopen(p, "re"); if (!f) { if (errno == ENOENT) return -ESRCH; - else if (!ERRNO_IS_PRIVILEGE(errno)) + if (!ERRNO_IS_PRIVILEGE(errno)) return -errno; } else { @@ -958,7 +1026,7 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { if (missing & SD_BUS_CREDS_SELINUX_CONTEXT) { const char *p; - p = procfs_file_alloca(pid, "attr/current"); + p = procfs_file_alloca(pidref->pid, "attr/current"); r = read_one_line_file(p, &c->label); if (r < 0) { if (!IN_SET(r, -ENOENT, -EINVAL, -EPERM, -EACCES)) @@ -968,7 +1036,7 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { } if (missing & SD_BUS_CREDS_COMM) { - r = pid_get_comm(pid, &c->comm); + r = pid_get_comm(pidref->pid, &c->comm); if (r < 0) { if (!ERRNO_IS_PRIVILEGE(r)) return r; @@ -977,7 +1045,7 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { } if (missing & SD_BUS_CREDS_EXE) { - r = get_process_exe(pid, &c->exe); + r = get_process_exe(pidref->pid, &c->exe); if (r == -ESRCH) { /* Unfortunately we cannot really distinguish * the case here where the process does not @@ -998,7 +1066,7 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { if (missing & SD_BUS_CREDS_CMDLINE) { const char *p; - p = procfs_file_alloca(pid, "cmdline"); + p = procfs_file_alloca(pidref->pid, "cmdline"); r = read_full_virtual_file(p, &c->cmdline, &c->cmdline_size); if (r == -ENOENT) return -ESRCH; @@ -1016,7 +1084,7 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { if (tid > 0 && (missing & SD_BUS_CREDS_TID_COMM)) { _cleanup_free_ char *p = NULL; - if (asprintf(&p, "/proc/"PID_FMT"/task/"PID_FMT"/comm", pid, tid) < 0) + if (asprintf(&p, "/proc/"PID_FMT"/task/"PID_FMT"/comm", pidref->pid, tid) < 0) return -ENOMEM; r = read_one_line_file(p, &c->tid_comm); @@ -1032,7 +1100,7 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { if (missing & (SD_BUS_CREDS_CGROUP|SD_BUS_CREDS_UNIT|SD_BUS_CREDS_USER_UNIT|SD_BUS_CREDS_SLICE|SD_BUS_CREDS_USER_SLICE|SD_BUS_CREDS_SESSION|SD_BUS_CREDS_OWNER_UID)) { if (!c->cgroup) { - r = cg_pid_get_path(NULL, pid, &c->cgroup); + r = cg_pid_get_path(NULL, pidref->pid, &c->cgroup); if (r < 0) { if (!ERRNO_IS_PRIVILEGE(r)) return r; @@ -1050,7 +1118,7 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { } if (missing & SD_BUS_CREDS_AUDIT_SESSION_ID) { - r = audit_session_from_pid(pid, &c->audit_session_id); + r = audit_session_from_pid(pidref->pid, &c->audit_session_id); if (r == -ENODATA) { /* ENODATA means: no audit session id assigned */ c->audit_session_id = AUDIT_SESSION_INVALID; @@ -1063,7 +1131,7 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { } if (missing & SD_BUS_CREDS_AUDIT_LOGIN_UID) { - r = audit_loginuid_from_pid(pid, &c->audit_login_uid); + r = audit_loginuid_from_pid(pidref->pid, &c->audit_login_uid); if (r == -ENODATA) { /* ENODATA means: no audit login uid assigned */ c->audit_login_uid = UID_INVALID; @@ -1076,7 +1144,7 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { } if (missing & SD_BUS_CREDS_TTY) { - r = get_ctty(pid, NULL, &c->tty); + r = get_ctty(pidref->pid, NULL, &c->tty); if (r == -ENXIO) { /* ENXIO means: process has no controlling TTY */ c->tty = NULL; @@ -1088,16 +1156,12 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) { c->mask |= SD_BUS_CREDS_TTY; } - /* In case only the exe path was to be read we cannot distinguish the case where the exe path was - * unreadable because the process was a kernel thread, or when the process didn't exist at - * all. Hence, let's do a final check, to be sure. */ - r = pid_is_alive(pid); + r = pidref_verify(pidref); if (r < 0) return r; - if (r == 0) - return -ESRCH; - if (tid > 0 && tid != pid && pid_is_unwaited(tid) == 0) + /* Validate tid is still valid, too */ + if (tid > 0 && tid != pidref->pid && pid_is_unwaited(tid) == 0) return -ESRCH; c->augmented = missing & c->mask; @@ -1131,6 +1195,13 @@ int bus_creds_extend_by_pid(sd_bus_creds *c, uint64_t mask, sd_bus_creds **ret) n->mask |= SD_BUS_CREDS_PID; } + if (c->mask & mask & SD_BUS_CREDS_PIDFD) { + n->pidfd = fcntl(c->pidfd, F_DUPFD_CLOEXEC, 3); + if (n->pidfd < 0) + return -errno; + n->mask |= SD_BUS_CREDS_PIDFD; + } + if (c->mask & mask & SD_BUS_CREDS_TID) { n->tid = c->tid; n->mask |= SD_BUS_CREDS_TID; diff --git a/src/libsystemd/sd-bus/bus-creds.h b/src/libsystemd/sd-bus/bus-creds.h index 7806d9e362..f45de1cb9c 100644 --- a/src/libsystemd/sd-bus/bus-creds.h +++ b/src/libsystemd/sd-bus/bus-creds.h @@ -5,6 +5,9 @@ #include "sd-bus.h" +#include "pidref.h" +#include "user-util.h" + struct sd_bus_creds { bool allocated; unsigned n_ref; @@ -27,6 +30,7 @@ struct sd_bus_creds { pid_t ppid; pid_t pid; pid_t tid; + int pidfd; char *comm; char *tid_comm; @@ -63,10 +67,22 @@ struct sd_bus_creds { char *description, *unescaped_description; }; +#define SD_BUS_CREDS_INIT_FIELDS \ + .uid = UID_INVALID, \ + .euid = UID_INVALID, \ + .suid = UID_INVALID, \ + .fsuid = UID_INVALID, \ + .gid = GID_INVALID, \ + .egid = GID_INVALID, \ + .sgid = GID_INVALID, \ + .fsgid = GID_INVALID, \ + .pidfd = -EBADF, \ + .audit_login_uid = UID_INVALID + sd_bus_creds* bus_creds_new(void); void bus_creds_done(sd_bus_creds *c); -int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid); +int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, PidRef *pidref, pid_t tid); int bus_creds_extend_by_pid(sd_bus_creds *c, uint64_t mask, sd_bus_creds **ret); diff --git a/src/libsystemd/sd-bus/bus-dump.c b/src/libsystemd/sd-bus/bus-dump.c index 6d24f3b1c9..aa46fec91b 100644 --- a/src/libsystemd/sd-bus/bus-dump.c +++ b/src/libsystemd/sd-bus/bus-dump.c @@ -355,6 +355,8 @@ int bus_creds_dump(sd_bus_creds *c, FILE *f, bool terse) { if (c->mask & SD_BUS_CREDS_PID) fprintf(f, "%sPID=%s"PID_FMT"%s", prefix, color, c->pid, suffix); + if (c->mask & SD_BUS_CREDS_PIDFD) + fprintf(f, "%sPIDFD=%syes%s", prefix, color, suffix); if (c->mask & SD_BUS_CREDS_TID) fprintf(f, "%sTID=%s"PID_FMT"%s", prefix, color, c->tid, suffix); if (c->mask & SD_BUS_CREDS_PPID) { diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index 098a518605..8229c432c2 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -269,6 +269,7 @@ struct sd_bus { size_t n_groups; union sockaddr_union sockaddr_peer; socklen_t sockaddr_size_peer; + int pidfd; uint64_t creds_mask; diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index e3d1eb8f33..296f450826 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -373,6 +373,7 @@ static int message_from_header( if (!m) return -ENOMEM; + m->creds = (sd_bus_creds) { SD_BUS_CREDS_INIT_FIELDS }; m->sealed = true; m->header = buffer; @@ -469,6 +470,7 @@ _public_ int sd_bus_message_new( return -ENOMEM; t->n_ref = 1; + t->creds = (sd_bus_creds) { SD_BUS_CREDS_INIT_FIELDS }; t->bus = sd_bus_ref(bus); t->header = (struct bus_header*) ((uint8_t*) t + ALIGN(sizeof(struct sd_bus_message))); t->header->endian = BUS_NATIVE_ENDIAN; diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index 8fe8854350..a99cdcc451 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -643,14 +643,20 @@ static void bus_get_peercred(sd_bus *b) { /* Get the SELinux context of the peer */ r = getpeersec(b->input_fd, &b->label); if (r < 0 && !IN_SET(r, -EOPNOTSUPP, -ENOPROTOOPT)) - log_debug_errno(r, "Failed to determine peer security context: %m"); + log_debug_errno(r, "Failed to determine peer security context, ignoring: %m"); /* Get the list of auxiliary groups of the peer */ r = getpeergroups(b->input_fd, &b->groups); if (r >= 0) b->n_groups = (size_t) r; else if (!IN_SET(r, -EOPNOTSUPP, -ENOPROTOOPT)) - log_debug_errno(r, "Failed to determine peer's group list: %m"); + log_debug_errno(r, "Failed to determine peer's group list, ignoring: %m"); + + r = getpeerpidfd(b->input_fd); + if (r < 0) + log_debug_errno(r, "Failed to determin peer pidfd, ignoring: %m"); + else + close_and_replace(b->pidfd, r); /* Let's query the peers socket address, it might carry information such as the peer's comm or * description string */ diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 4fdfc9a581..2ad29af105 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -256,6 +256,7 @@ _public_ int sd_bus_new(sd_bus **ret) { .n_groups = SIZE_MAX, .close_on_exit = true, .ucred = UCRED_INVALID, + .pidfd = -EBADF, .runtime_scope = _RUNTIME_SCOPE_INVALID, }; diff --git a/src/libsystemd/sd-bus/test-bus-creds.c b/src/libsystemd/sd-bus/test-bus-creds.c index d18ce88a25..7eb7a38c39 100644 --- a/src/libsystemd/sd-bus/test-bus-creds.c +++ b/src/libsystemd/sd-bus/test-bus-creds.c @@ -4,6 +4,7 @@ #include "bus-dump.h" #include "cgroup-util.h" +#include "errno-util.h" #include "tests.h" int main(int argc, char *argv[]) { @@ -24,11 +25,30 @@ int main(int argc, char *argv[]) { creds = sd_bus_creds_unref(creds); r = sd_bus_creds_new_from_pid(&creds, 1, _SD_BUS_CREDS_ALL); - if (r != -EACCES) { + if (!ERRNO_IS_NEG_PRIVILEGE(r)) { assert_se(r >= 0); putchar('\n'); bus_creds_dump(creds, NULL, true); } + creds = sd_bus_creds_unref(creds); + + _cleanup_(sd_bus_unrefp) sd_bus *bus = NULL; + r = sd_bus_default_system(&bus); + if (r < 0) + log_warning_errno(r, "Unable to connect to system bus, skipping rest of test."); + else { + const char *unique; + + assert_se(sd_bus_get_unique_name(bus, &unique) >= 0); + + r = sd_bus_get_name_creds(bus, unique, _SD_BUS_CREDS_ALL, &creds); + log_full_errno(r < 0 ? LOG_ERR : LOG_DEBUG, r, "sd_bus_get_name_creds: %m"); + assert_se(r >= 0); + + putchar('\n'); + bus_creds_dump(creds, NULL, true); + } + return 0; } diff --git a/src/libsystemd/sd-bus/test-bus-peersockaddr.c b/src/libsystemd/sd-bus/test-bus-peersockaddr.c index 79556e8a85..0c57d38218 100644 --- a/src/libsystemd/sd-bus/test-bus-peersockaddr.c +++ b/src/libsystemd/sd-bus/test-bus-peersockaddr.c @@ -5,6 +5,8 @@ #include "sd-bus.h" +#include "bus-dump.h" +#include "bus-util.h" #include "fd-util.h" #include "process-util.h" #include "socket-util.h" @@ -27,11 +29,13 @@ static void *server(void *p) { assert_se(sd_bus_set_fd(bus, fd, fd) >= 0); TAKE_FD(fd); assert_se(sd_bus_set_server(bus, true, id) >= 0); - assert_se(sd_bus_negotiate_creds(bus, 1, SD_BUS_CREDS_EUID|SD_BUS_CREDS_EGID|SD_BUS_CREDS_PID|SD_BUS_CREDS_COMM|SD_BUS_CREDS_DESCRIPTION) >= 0); + assert_se(sd_bus_negotiate_creds(bus, 1, SD_BUS_CREDS_EUID|SD_BUS_CREDS_EGID|SD_BUS_CREDS_PID|SD_BUS_CREDS_COMM|SD_BUS_CREDS_DESCRIPTION|SD_BUS_CREDS_PIDFD) >= 0); assert_se(sd_bus_start(bus) >= 0); - assert_se(sd_bus_get_owner_creds(bus, SD_BUS_CREDS_EUID|SD_BUS_CREDS_EGID|SD_BUS_CREDS_PID|SD_BUS_CREDS_COMM|SD_BUS_CREDS_DESCRIPTION, &c) >= 0); + assert_se(sd_bus_get_owner_creds(bus, SD_BUS_CREDS_EUID|SD_BUS_CREDS_EGID|SD_BUS_CREDS_PID|SD_BUS_CREDS_COMM|SD_BUS_CREDS_DESCRIPTION|SD_BUS_CREDS_PIDFD, &c) >= 0); + + bus_creds_dump(c, /* f= */ NULL, /* terse= */ false); uid_t u; assert_se(sd_bus_creds_get_euid(c, &u) >= 0); @@ -45,6 +49,14 @@ static void *server(void *p) { assert_se(sd_bus_creds_get_pid(c, &pid) >= 0); assert_se(pid == getpid_cached()); + int pidfd = -EBADF; + if (sd_bus_creds_get_pidfd_dup(c, &pidfd) >= 0) { + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + + assert_se(pidref_set_pidfd_take(&pidref, pidfd) >= 0); + assert_se(pidref.pid == getpid_cached()); + } + const char *comm; assert_se(sd_bus_creds_get_comm(c, &comm) >= 0); assert_se(pid_get_comm(0, &our_comm) >= 0); diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index bd3da36c36..f1a3311992 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -90,8 +90,9 @@ __extension__ enum { SD_BUS_CREDS_UNIQUE_NAME = 1ULL << 31, SD_BUS_CREDS_WELL_KNOWN_NAMES = 1ULL << 32, SD_BUS_CREDS_DESCRIPTION = 1ULL << 33, + SD_BUS_CREDS_PIDFD = 1ULL << 34, SD_BUS_CREDS_AUGMENT = 1ULL << 63, /* special flag, if on sd-bus will augment creds struct, in a potentially race-full way. */ - _SD_BUS_CREDS_ALL = (1ULL << 34) -1 + _SD_BUS_CREDS_ALL = (1ULL << 35) -1 }; __extension__ enum { @@ -402,12 +403,14 @@ int sd_bus_match_signal_async(sd_bus *bus, sd_bus_slot **ret, const char *sender /* Credential handling */ int sd_bus_creds_new_from_pid(sd_bus_creds **ret, pid_t pid, uint64_t creds_mask); +int sd_bus_creds_new_from_pidfd(sd_bus_creds **ret, int pidfd, uint64_t creds_mask); sd_bus_creds* sd_bus_creds_ref(sd_bus_creds *c); sd_bus_creds* sd_bus_creds_unref(sd_bus_creds *c); uint64_t sd_bus_creds_get_mask(const sd_bus_creds *c); uint64_t sd_bus_creds_get_augmented_mask(const sd_bus_creds *c); int sd_bus_creds_get_pid(sd_bus_creds *c, pid_t *pid); +int sd_bus_creds_get_pidfd_dup(sd_bus_creds *c, int *ret_fd); int sd_bus_creds_get_ppid(sd_bus_creds *c, pid_t *ppid); int sd_bus_creds_get_tid(sd_bus_creds *c, pid_t *tid); int sd_bus_creds_get_uid(sd_bus_creds *c, uid_t *uid); From 25fd5343ca3a3481d2e0d3f3aebab2fcf4e42bc7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Jan 2024 17:09:02 +0100 Subject: [PATCH 3/8] sd-bus: also read supplementary gids from bus driver --- src/libsystemd/sd-bus/bus-control.c | 48 ++++++++++++++++++- src/libsystemd/sd-bus/test-bus-peersockaddr.c | 43 ++++++++++++++++- 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c index 3b11d754a2..0953b6ab26 100644 --- a/src/libsystemd/sd-bus/bus-control.c +++ b/src/libsystemd/sd-bus/bus-control.c @@ -483,8 +483,8 @@ _public_ int sd_bus_get_name_creds( } if (mask != 0) { + bool need_pid, need_uid, need_gids, need_selinux, need_separate_calls, need_pidfd, need_augment; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - bool need_pid, need_uid, need_selinux, need_separate_calls, need_pidfd, need_augment; _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; c = bus_creds_new(); @@ -513,10 +513,11 @@ _public_ int sd_bus_get_name_creds( need_pid = (mask & SD_BUS_CREDS_PID) || need_augment; need_uid = mask & SD_BUS_CREDS_EUID; + need_gids = mask & SD_BUS_CREDS_SUPPLEMENTARY_GIDS; need_selinux = mask & SD_BUS_CREDS_SELINUX_CONTEXT; need_pidfd = (mask & SD_BUS_CREDS_PIDFD) || need_augment; - if (need_pid + need_uid + need_selinux + need_pidfd > 1) { + if (need_pid + need_uid + need_selinux + need_pidfd + need_gids > 1) { /* If we need more than one of the credentials, then use GetConnectionCredentials() */ @@ -626,6 +627,49 @@ _public_ int sd_bus_get_name_creds( close_and_replace(c->pidfd, fd); c->mask |= SD_BUS_CREDS_PIDFD; } + } else if (need_gids && streq(m, "UnixGroupIDs")) { + + /* Note that D-Bus actualy only gives us a combined list of + * primary gid and supplementary gids. And we don't know + * which one the primary one is. We'll take the whole shebang + * hence and use it as the supplementary group list, and not + * initialize the primary gid field. This is slightly + * incorrect of course, but only slightly, as in effect if + * the primary gid is also listed in the supplementary gid + * it has zero effect. */ + + r = sd_bus_message_enter_container(reply, 'v', "au"); + if (r < 0) + return r; + + r = sd_bus_message_enter_container(reply, 'a', "u"); + if (r < 0) + return r; + + for (;;) { + uint32_t u; + + r = sd_bus_message_read(reply, "u", &u); + if (r < 0) + return r; + if (r == 0) + break; + + if (!GREEDY_REALLOC(c->supplementary_gids, c->n_supplementary_gids+1)) + return -ENOMEM; + + c->supplementary_gids[c->n_supplementary_gids++] = (gid_t) u; + } + + r = sd_bus_message_exit_container(reply); + if (r < 0) + return r; + + r = sd_bus_message_exit_container(reply); + if (r < 0) + return r; + + c->mask |= SD_BUS_CREDS_SUPPLEMENTARY_GIDS; } else { r = sd_bus_message_skip(reply, "v"); if (r < 0) diff --git a/src/libsystemd/sd-bus/test-bus-peersockaddr.c b/src/libsystemd/sd-bus/test-bus-peersockaddr.c index 0c57d38218..a7bba17a98 100644 --- a/src/libsystemd/sd-bus/test-bus-peersockaddr.c +++ b/src/libsystemd/sd-bus/test-bus-peersockaddr.c @@ -10,7 +10,34 @@ #include "fd-util.h" #include "process-util.h" #include "socket-util.h" +#include "sort-util.h" #include "tests.h" +#include "user-util.h" + +static bool gid_list_contained(const gid_t *a, size_t n, const gid_t *b, size_t m) { + assert_se(a || n == 0); + assert_se(b || m == 0); + + /* Checks if every entry in a[] is also in b[] */ + + for (size_t i = 0; i < n; i++) { + size_t j; + + for (j = 0; j < m; j++) + if (a[i] == b[j]) + break; + + if (j >= m) + return false; + } + + return true; +} + +static bool gid_list_same(const gid_t *a, size_t n, const gid_t *b, size_t m) { + return gid_list_contained(a, n, b, m) && + gid_list_contained(b, m, a, n); +} static void *server(void *p) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; @@ -29,11 +56,11 @@ static void *server(void *p) { assert_se(sd_bus_set_fd(bus, fd, fd) >= 0); TAKE_FD(fd); assert_se(sd_bus_set_server(bus, true, id) >= 0); - assert_se(sd_bus_negotiate_creds(bus, 1, SD_BUS_CREDS_EUID|SD_BUS_CREDS_EGID|SD_BUS_CREDS_PID|SD_BUS_CREDS_COMM|SD_BUS_CREDS_DESCRIPTION|SD_BUS_CREDS_PIDFD) >= 0); + assert_se(sd_bus_negotiate_creds(bus, 1, SD_BUS_CREDS_EUID|SD_BUS_CREDS_EGID|SD_BUS_CREDS_PID|SD_BUS_CREDS_COMM|SD_BUS_CREDS_DESCRIPTION|SD_BUS_CREDS_PIDFD|SD_BUS_CREDS_SUPPLEMENTARY_GIDS) >= 0); assert_se(sd_bus_start(bus) >= 0); - assert_se(sd_bus_get_owner_creds(bus, SD_BUS_CREDS_EUID|SD_BUS_CREDS_EGID|SD_BUS_CREDS_PID|SD_BUS_CREDS_COMM|SD_BUS_CREDS_DESCRIPTION|SD_BUS_CREDS_PIDFD, &c) >= 0); + assert_se(sd_bus_get_owner_creds(bus, SD_BUS_CREDS_EUID|SD_BUS_CREDS_EGID|SD_BUS_CREDS_PID|SD_BUS_CREDS_COMM|SD_BUS_CREDS_DESCRIPTION|SD_BUS_CREDS_PIDFD|SD_BUS_CREDS_SUPPLEMENTARY_GIDS, &c) >= 0); bus_creds_dump(c, /* f= */ NULL, /* terse= */ false); @@ -57,6 +84,18 @@ static void *server(void *p) { assert_se(pidref.pid == getpid_cached()); } + const gid_t *gl = NULL; + int n; + n = sd_bus_creds_get_supplementary_gids(c, &gl); + + if (n >= 0) { + _cleanup_free_ gid_t *gg = NULL; + r = getgroups_alloc(&gg); + assert_se(r >= 0); + + assert_se(gid_list_same(gl, n, gg, r)); + } + const char *comm; assert_se(sd_bus_creds_get_comm(c, &comm) >= 0); assert_se(pid_get_comm(0, &our_comm) >= 0); From 75e00d5e32fa517b25289cbac806db7d88c50c8c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Jan 2024 18:01:25 +0100 Subject: [PATCH 4/8] sd-bus: tighten rules on sd_bus_query_sender_creds() a bit Let's always derive credentials from a bus name or a conneciton fd if we can, because they pin things. Let's not go via PID really, because it's always racy to do so. Note that this doesn't change much, since we wouldn't use such augmented data for auth anyway (because it will be masked in the sd_bus_creds.augmented mask as untrusted). But still, let's prefer trusted data over untrusted data. --- src/libsystemd/sd-bus/bus-convenience.c | 38 ++++++++----------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-convenience.c b/src/libsystemd/sd-bus/bus-convenience.c index 989e577de5..14d8073bc9 100644 --- a/src/libsystemd/sd-bus/bus-convenience.c +++ b/src/libsystemd/sd-bus/bus-convenience.c @@ -640,8 +640,8 @@ _public_ int sd_bus_set_property( } _public_ int sd_bus_query_sender_creds(sd_bus_message *call, uint64_t mask, sd_bus_creds **ret) { + uint64_t missing; sd_bus_creds *c; - int r; assert_return(call, -EINVAL); assert_return(call->sealed, -EPERM); @@ -653,36 +653,22 @@ _public_ int sd_bus_query_sender_creds(sd_bus_message *call, uint64_t mask, sd_b return -ENOTCONN; c = sd_bus_message_get_creds(call); - - /* All data we need? */ - if (c && (mask & ~SD_BUS_CREDS_AUGMENT & ~c->mask) == 0) { + if (c) + missing = mask & ~SD_BUS_CREDS_AUGMENT & ~c->mask; + else + missing = mask & ~SD_BUS_CREDS_AUGMENT; + if (missing == 0) { /* All data we need? */ *ret = sd_bus_creds_ref(c); return 0; } - /* No data passed? Or not enough data passed to retrieve the missing bits? */ - if (!c || !(c->mask & SD_BUS_CREDS_PID)) { - /* We couldn't read anything from the call, let's try - * to get it from the sender or peer. */ + /* There's a sender, use that */ + if (call->sender && call->bus->bus_client) + return sd_bus_get_name_creds(call->bus, call->sender, mask, ret); - if (call->sender) - /* There's a sender, but the creds are missing. */ - return sd_bus_get_name_creds(call->bus, call->sender, mask, ret); - else - /* There's no sender. For direct connections - * the credentials of the AF_UNIX peer matter, - * which may be queried via sd_bus_get_owner_creds(). */ - return sd_bus_get_owner_creds(call->bus, mask, ret); - } - - r = bus_creds_extend_by_pid(c, mask, ret); - if (r == -ESRCH) { - /* Process doesn't exist anymore? propagate the few things we have */ - *ret = sd_bus_creds_ref(c); - return 0; - } - - return r; + /* There's no sender. For direct connections the credentials of the AF_UNIX peer matter, which may be + * queried via sd_bus_get_owner_creds(). */ + return sd_bus_get_owner_creds(call->bus, mask, ret); } _public_ int sd_bus_query_sender_privilege(sd_bus_message *call, int capability) { From 1b78be0bb567d07f00922809411c16887a084033 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Jan 2024 18:40:09 +0100 Subject: [PATCH 5/8] bus-util: add helper for getting PidRef structs from bus This adds two helpers: one for extracting a PidRef from an sd_bus_creds object, and one from doing this from and sd_bus_message object. --- src/shared/bus-util.c | 44 +++++++++++++++++++++++++++++++++++++++++++ src/shared/bus-util.h | 4 ++++ 2 files changed, 48 insertions(+) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 09a3734aeb..eea6c2321a 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -710,3 +710,47 @@ int bus_property_get_string_set( return bus_message_append_string_set(reply, *s); } + +int bus_creds_get_pidref( + sd_bus_creds *c, + PidRef *ret) { + + int pidfd = -EBADF; + pid_t pid; + int r; + + assert(c); + assert(ret); + + r = sd_bus_creds_get_pid(c, &pid); + if (r < 0) + return r; + + r = sd_bus_creds_get_pidfd_dup(c, &pidfd); + if (r < 0 && r != -ENODATA) + return r; + + *ret = (PidRef) { + .pid = pid, + .fd = pidfd, + }; + + return 0; +} + +int bus_query_sender_pidref( + sd_bus_message *m, + PidRef *ret) { + + _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; + int r; + + assert(m); + assert(ret); + + r = sd_bus_query_sender_creds(m, SD_BUS_CREDS_PID|SD_BUS_CREDS_PIDFD, &creds); + if (r < 0) + return r; + + return bus_creds_get_pidref(creds, ret); +} diff --git a/src/shared/bus-util.h b/src/shared/bus-util.h index 869c639aeb..1004c79406 100644 --- a/src/shared/bus-util.h +++ b/src/shared/bus-util.h @@ -11,6 +11,7 @@ #include "errno-util.h" #include "macro.h" +#include "pidref.h" #include "runtime-scope.h" #include "set.h" #include "string-util.h" @@ -73,3 +74,6 @@ extern const struct hash_ops bus_message_hash_ops; int bus_message_append_string_set(sd_bus_message *m, Set *s); int bus_property_get_string_set(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error); + +int bus_creds_get_pidref(sd_bus_creds *c, PidRef *ret); +int bus_query_sender_pidref(sd_bus_message *m, PidRef *ret); From 4ac08d8ad6fb905ef8cccf7545304ac60a6c2d0d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Jan 2024 18:41:24 +0100 Subject: [PATCH 6/8] tree-wide: port various things over to new pidref helpers THis not only mkaes a lot of code shorter, but also safer, as we pin the clients via a pidfd. --- src/core/dbus-manager.c | 59 ++++++----------- src/core/dbus-scope.c | 24 +++---- src/core/dbus-unit.c | 17 ++--- src/core/dbus.c | 30 ++++----- src/core/selinux-access.c | 2 +- src/login/logind-dbus.c | 132 ++++++++++++++++---------------------- 6 files changed, 108 insertions(+), 156 deletions(-) diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 6552491261..31b2f1daef 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -11,6 +11,7 @@ #include "bus-common-errors.h" #include "bus-get-properties.h" #include "bus-log-control-api.h" +#include "bus-util.h" #include "chase.h" #include "confidential-virt.h" #include "data-fd-util.h" @@ -464,18 +465,13 @@ static int bus_get_unit_by_name(Manager *m, sd_bus_message *message, const char * its sleeve: if the name is specified empty we use the client's unit. */ if (isempty(name)) { - _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; - pid_t pid; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; - r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_PID, &creds); + r = bus_query_sender_pidref(message, &pidref); if (r < 0) return r; - r = sd_bus_creds_get_pid(creds, &pid); - if (r < 0) - return r; - - u = manager_get_unit_by_pid(m, pid); + u = manager_get_unit_by_pidref(m, &pidref); if (!u) return sd_bus_error_set(error, BUS_ERROR_NO_SUCH_UNIT, "Client not member of any unit."); } else { @@ -542,7 +538,7 @@ static int method_get_unit(sd_bus_message *message, void *userdata, sd_bus_error static int method_get_unit_by_pid(sd_bus_message *message, void *userdata, sd_bus_error *error) { Manager *m = ASSERT_PTR(userdata); - pid_t pid; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; Unit *u; int r; @@ -552,27 +548,20 @@ static int method_get_unit_by_pid(sd_bus_message *message, void *userdata, sd_bu /* Anyone can call this method */ - r = sd_bus_message_read(message, "u", &pid); + r = sd_bus_message_read(message, "u", &pidref.pid); if (r < 0) return r; - if (pid < 0) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid PID " PID_FMT, pid); - - if (pid == 0) { - _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; - - r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_PID, &creds); - if (r < 0) - return r; - - r = sd_bus_creds_get_pid(creds, &pid); + if (pidref.pid < 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid PID " PID_FMT, pidref.pid); + if (pidref.pid == 0) { + r = bus_query_sender_pidref(message, &pidref); if (r < 0) return r; } - u = manager_get_unit_by_pid(m, pid); + u = manager_get_unit_by_pidref(m, &pidref); if (!u) - return sd_bus_error_setf(error, BUS_ERROR_NO_UNIT_FOR_PID, "PID "PID_FMT" does not belong to any loaded unit.", pid); + return sd_bus_error_setf(error, BUS_ERROR_NO_UNIT_FOR_PID, "PID "PID_FMT" does not belong to any loaded unit.", pidref.pid); return reply_unit_path(u, message, error); } @@ -601,21 +590,16 @@ static int method_get_unit_by_invocation_id(sd_bus_message *message, void *userd return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid invocation ID"); if (sd_id128_is_null(id)) { - _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; - pid_t pid; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; - r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_PID, &creds); + r = bus_query_sender_pidref(message, &pidref); if (r < 0) return r; - r = sd_bus_creds_get_pid(creds, &pid); - if (r < 0) - return r; - - u = manager_get_unit_by_pid(m, pid); + u = manager_get_unit_by_pidref(m, &pidref); if (!u) return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, - "Client " PID_FMT " not member of any unit.", pid); + "Client " PID_FMT " not member of any unit.", pidref.pid); } else { u = hashmap_get(m->units_by_invocation_id, &id); if (!u) @@ -2934,7 +2918,7 @@ static int method_dump_unit_descriptor_store(sd_bus_message *message, void *user } static int aux_scope_from_message(Manager *m, sd_bus_message *message, Unit **ret_scope, sd_bus_error *error) { - _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; + _cleanup_(pidref_done) PidRef sender_pidref = PIDREF_NULL; _cleanup_free_ PidRef *pidrefs = NULL; const char *name; Unit *from, *scope; @@ -2942,20 +2926,15 @@ static int aux_scope_from_message(Manager *m, sd_bus_message *message, Unit **re CGroupContext *cc; size_t n_pids = 0; uint64_t flags; - pid_t pid; int r; assert(ret_scope); - r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_PID, &creds); + r = bus_query_sender_pidref(message, &sender_pidref); if (r < 0) return r; - r = sd_bus_creds_get_pid(creds, &pid); - if (r < 0) - return r; - - from = manager_get_unit_by_pid(m, pid); + from = manager_get_unit_by_pidref(m, &sender_pidref); if (!from) return sd_bus_error_set(error, BUS_ERROR_NO_SUCH_UNIT, "Client not member of any unit."); diff --git a/src/core/dbus-scope.c b/src/core/dbus-scope.c index 78196a1a08..165aa65a7b 100644 --- a/src/core/dbus-scope.c +++ b/src/core/dbus-scope.c @@ -3,6 +3,7 @@ #include "alloc-util.h" #include "bus-common-errors.h" #include "bus-get-properties.h" +#include "bus-util.h" #include "dbus-cgroup.h" #include "dbus-kill.h" #include "dbus-manager.h" @@ -84,7 +85,7 @@ static int bus_scope_set_transient_property( return bus_set_transient_oom_policy(u, name, &s->oom_policy, message, flags, error); if (streq(name, "PIDs")) { - _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; + _cleanup_(pidref_done) PidRef sender_pidref = PIDREF_NULL; unsigned n = 0; r = sd_bus_message_enter_container(message, 'a', "u"); @@ -94,7 +95,7 @@ static int bus_scope_set_transient_property( for (;;) { _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; uint32_t upid; - pid_t pid; + PidRef *p; r = sd_bus_message_read(message, "u", &upid); if (r < 0) @@ -103,28 +104,27 @@ static int bus_scope_set_transient_property( break; if (upid == 0) { - if (!creds) { - r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_PID, &creds); + if (!pidref_is_set(&sender_pidref)) { + r = bus_query_sender_pidref(message, &sender_pidref); if (r < 0) return r; } - r = sd_bus_creds_get_pid(creds, &pid); + p = &sender_pidref; + } else { + r = pidref_set_pid(&pidref, upid); if (r < 0) return r; - } else - pid = (uid_t) upid; - r = pidref_set_pid(&pidref, pid); - if (r < 0) - return r; + p = &pidref; + } - r = unit_pid_attachable(u, &pidref, error); + r = unit_pid_attachable(u, p, error); if (r < 0) return r; if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - r = unit_watch_pidref(u, &pidref, /* exclusive= */ false); + r = unit_watch_pidref(u, p, /* exclusive= */ false); if (r < 0 && r != -EEXIST) return r; } diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 83d2302faa..4b017bff48 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -7,6 +7,7 @@ #include "bus-common-errors.h" #include "bus-get-properties.h" #include "bus-polkit.h" +#include "bus-util.h" #include "cgroup-util.h" #include "condition.h" #include "dbus-job.h" @@ -1485,7 +1486,7 @@ int bus_unit_method_attach_processes(sd_bus_message *message, void *userdata, sd if (UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(u))) return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Unit is not active, refusing."); - r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_EUID|SD_BUS_CREDS_PID, &creds); + r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_EUID|SD_BUS_CREDS_PID|SD_BUS_CREDS_PIDFD, &creds); if (r < 0) return r; @@ -1496,7 +1497,6 @@ int bus_unit_method_attach_processes(sd_bus_message *message, void *userdata, sd _cleanup_(pidref_freep) PidRef *pidref = NULL; uid_t process_uid, sender_uid; uint32_t upid; - pid_t pid; r = sd_bus_message_read(message, "u", &upid); if (r < 0) @@ -1505,13 +1505,14 @@ int bus_unit_method_attach_processes(sd_bus_message *message, void *userdata, sd break; if (upid == 0) { - r = sd_bus_creds_get_pid(creds, &pid); + _cleanup_(pidref_done) PidRef p = PIDREF_NULL; + r = bus_creds_get_pidref(creds, &p); if (r < 0) return r; - } else - pid = (uid_t) upid; - r = pidref_new_from_pid(pid, &pidref); + r = pidref_dup(&p, &pidref); + } else + r = pidref_new_from_pid(upid, &pidref); if (r < 0) return r; @@ -1537,9 +1538,9 @@ int bus_unit_method_attach_processes(sd_bus_message *message, void *userdata, sd return sd_bus_error_set_errnof(error, r, "Failed to retrieve process UID: %m"); if (process_uid != sender_uid) - return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Process " PID_FMT " not owned by client's UID. Refusing.", pid); + return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Process " PID_FMT " not owned by client's UID. Refusing.", pidref->pid); if (process_uid != u->ref_uid) - return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Process " PID_FMT " not owned by target unit's UID. Refusing.", pid); + return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Process " PID_FMT " not owned by target unit's UID. Refusing.", pidref->pid); } r = set_ensure_consume(&pids, &pidref_hash_ops_free, TAKE_PTR(pidref)); diff --git a/src/core/dbus.c b/src/core/dbus.c index 49a80bbcfa..a336b04daf 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -232,6 +232,8 @@ static int mac_selinux_filter(sd_bus_message *message, void *userdata, sd_bus_er return 0; path = sd_bus_message_get_path(message); + if (!path) + return 0; if (object_path_startswith("/org/freedesktop/systemd1", path)) { r = mac_selinux_access_check(message, verb, error); @@ -241,25 +243,20 @@ static int mac_selinux_filter(sd_bus_message *message, void *userdata, sd_bus_er return 0; } - if (streq_ptr(path, "/org/freedesktop/systemd1/unit/self")) { - _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; - pid_t pid; + if (streq(path, "/org/freedesktop/systemd1/unit/self")) { + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; - r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_PID, &creds); + r = bus_query_sender_pidref(message, &pidref); if (r < 0) return 0; - r = sd_bus_creds_get_pid(creds, &pid); - if (r < 0) - return 0; - - u = manager_get_unit_by_pid(m, pid); + u = manager_get_unit_by_pidref(m, &pidref); } else { r = manager_get_job_from_dbus_path(m, path, &j); if (r >= 0) u = j->unit; else - manager_load_unit_from_dbus_path(m, path, NULL, &u); + (void) manager_load_unit_from_dbus_path(m, path, NULL, &u); } if (!u) return 0; @@ -280,24 +277,19 @@ static int find_unit(Manager *m, sd_bus *bus, const char *path, Unit **unit, sd_ assert(bus); assert(path); - if (streq_ptr(path, "/org/freedesktop/systemd1/unit/self")) { - _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; + if (streq(path, "/org/freedesktop/systemd1/unit/self")) { + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; sd_bus_message *message; - pid_t pid; message = sd_bus_get_current_message(bus); if (!message) return 0; - r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_PID, &creds); + r = bus_query_sender_pidref(message, &pidref); if (r < 0) return r; - r = sd_bus_creds_get_pid(creds, &pid); - if (r < 0) - return r; - - u = manager_get_unit_by_pid(m, pid); + u = manager_get_unit_by_pidref(m, &pidref); if (!u) return 0; } else { diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c index 62181a6309..1214bd8f22 100644 --- a/src/core/selinux-access.c +++ b/src/core/selinux-access.c @@ -248,7 +248,7 @@ int mac_selinux_access_check_internal( tclass = "system"; } - sd_bus_creds_get_cmdline(creds, &cmdline); + (void) sd_bus_creds_get_cmdline(creds, &cmdline); cl = strv_join(cmdline, " "); struct audit_info audit_info = { diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 097b63246b..199c7f9e9b 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -759,8 +759,8 @@ static int create_session( void *userdata, sd_bus_error *error, uid_t uid, - pid_t pid, - int pidfd, + pid_t leader_pid, + int leader_pidfd, const char *service, const char *type, const char *class, @@ -793,32 +793,18 @@ static int create_session( if (flags != 0) return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Flags must be zero."); - if (pidfd >= 0) { - r = pidref_set_pidfd(&leader, pidfd); - if (r < 0) - return r; - } else if (pid == 0) { - _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; - pid_t p; + if (leader_pidfd >= 0) + r = pidref_set_pidfd(&leader, leader_pidfd); + else if (leader_pid == 0) + r = bus_query_sender_pidref(message, &leader); + else { + if (leader_pid < 0) + return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Leader PID is not valid"); - r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_PID, &creds); - if (r < 0) - return r; - - r = sd_bus_creds_get_pid(creds, &p); - if (r < 0) - return r; - - r = pidref_set_pid(&leader, p); - if (r < 0) - return r; - } else { - assert(pid > 0); - - r = pidref_set_pid(&leader, pid); - if (r < 0) - return r; + r = pidref_set_pid(&leader, leader_pid); } + if (r < 0) + return r; if (leader.pid == 1 || leader.pid == getpid_cached()) return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid leader PID"); @@ -1096,32 +1082,32 @@ fail: static int method_create_session(sd_bus_message *message, void *userdata, sd_bus_error *error) { const char *service, *type, *class, *cseat, *tty, *display, *remote_user, *remote_host, *desktop; - pid_t leader; + pid_t leader_pid; + uint32_t vtnr; uid_t uid; - int remote; - uint32_t vtnr = 0; - int r; + int remote, r; assert(message); assert_cc(sizeof(pid_t) == sizeof(uint32_t)); assert_cc(sizeof(uid_t) == sizeof(uint32_t)); - r = sd_bus_message_read(message, - "uusssssussbss", - &uid, - &leader, - &service, - &type, - &class, - &desktop, - &cseat, - &vtnr, - &tty, - &display, - &remote, - &remote_user, - &remote_host); + r = sd_bus_message_read( + message, + "uusssssussbss", + &uid, + &leader_pid, + &service, + &type, + &class, + &desktop, + &cseat, + &vtnr, + &tty, + &display, + &remote, + &remote_user, + &remote_host); if (r < 0) return r; @@ -1130,7 +1116,7 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus userdata, error, uid, - leader, + leader_pid, /* pidfd = */ -EBADF, service, type, @@ -1148,29 +1134,28 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus static int method_create_session_pidfd(sd_bus_message *message, void *userdata, sd_bus_error *error) { const char *service, *type, *class, *cseat, *tty, *display, *remote_user, *remote_host, *desktop; - int leaderfd = -EBADF; - uid_t uid; - int remote; - uint32_t vtnr = 0; uint64_t flags; - int r; + uint32_t vtnr; + uid_t uid; + int leader_fd = -EBADF, remote, r; - r = sd_bus_message_read(message, - "uhsssssussbsst", - &uid, - &leaderfd, - &service, - &type, - &class, - &desktop, - &cseat, - &vtnr, - &tty, - &display, - &remote, - &remote_user, - &remote_host, - &flags); + r = sd_bus_message_read( + message, + "uhsssssussbsst", + &uid, + &leader_fd, + &service, + &type, + &class, + &desktop, + &cseat, + &vtnr, + &tty, + &display, + &remote, + &remote_user, + &remote_host, + &flags); if (r < 0) return r; @@ -1179,8 +1164,8 @@ static int method_create_session_pidfd(sd_bus_message *message, void *userdata, userdata, error, uid, - /* pid = */ 0, - leaderfd, + /* leader_pid = */ 0, + leader_fd, service, type, class, @@ -3506,7 +3491,6 @@ static int method_inhibit(sd_bus_message *message, void *userdata, sd_bus_error Manager *m = ASSERT_PTR(userdata); InhibitMode mm; InhibitWhat w; - pid_t pid; uid_t uid; int r; @@ -3557,7 +3541,7 @@ static int method_inhibit(sd_bus_message *message, void *userdata, sd_bus_error if (r == 0) return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ - r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_EUID|SD_BUS_CREDS_PID, &creds); + r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_EUID|SD_BUS_CREDS_PID|SD_BUS_CREDS_PIDFD, &creds); if (r < 0) return r; @@ -3565,14 +3549,10 @@ static int method_inhibit(sd_bus_message *message, void *userdata, sd_bus_error if (r < 0) return r; - r = sd_bus_creds_get_pid(creds, &pid); + r = bus_creds_get_pidref(creds, &pidref); if (r < 0) return r; - r = pidref_set_pid(&pidref, pid); - if (r < 0) - return sd_bus_error_set_errnof(error, r, "Failed pin source process "PID_FMT": %m", pid); - if (hashmap_size(m->inhibitors) >= m->inhibitors_max) return sd_bus_error_setf(error, SD_BUS_ERROR_LIMITS_EXCEEDED, "Maximum number of inhibitors (%" PRIu64 ") reached, refusing further inhibitors.", From a6671075941254a2c79f0b193512101ba15c68b5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Jan 2024 19:00:43 +0100 Subject: [PATCH 7/8] man: document the new APIs --- man/rules/meson.build | 2 + man/sd_bus_creds_get_pid.xml | 21 ++++- man/sd_bus_creds_new_from_pid.xml | 150 +++++++++++++----------------- 3 files changed, 85 insertions(+), 88 deletions(-) diff --git a/man/rules/meson.build b/man/rules/meson.build index 1e8747e4ef..b0b56af1f0 100644 --- a/man/rules/meson.build +++ b/man/rules/meson.build @@ -202,6 +202,7 @@ manpages = [ 'sd_bus_creds_get_fsuid', 'sd_bus_creds_get_gid', 'sd_bus_creds_get_owner_uid', + 'sd_bus_creds_get_pidfd_dup', 'sd_bus_creds_get_ppid', 'sd_bus_creds_get_selinux_context', 'sd_bus_creds_get_session', @@ -227,6 +228,7 @@ manpages = [ '3', ['sd_bus_creds_get_augmented_mask', 'sd_bus_creds_get_mask', + 'sd_bus_creds_new_from_pidfd', 'sd_bus_creds_ref', 'sd_bus_creds_unref', 'sd_bus_creds_unrefp'], diff --git a/man/sd_bus_creds_get_pid.xml b/man/sd_bus_creds_get_pid.xml index 62deb229ac..6f3e1ee07b 100644 --- a/man/sd_bus_creds_get_pid.xml +++ b/man/sd_bus_creds_get_pid.xml @@ -17,6 +17,7 @@ sd_bus_creds_get_pid + sd_bus_creds_get_pidfd_dup sd_bus_creds_get_ppid sd_bus_creds_get_tid sd_bus_creds_get_uid @@ -64,6 +65,12 @@ pid_t *pid + + int sd_bus_creds_get_pidfd_dup + sd_bus_creds *c + int *ret_fd + + int sd_bus_creds_get_ppid sd_bus_creds *c @@ -289,11 +296,14 @@ sd_bus_creds_get_mask3 to determine the mask of fields available. - sd_bus_creds_get_pid() will retrieve - the PID (process identifier). Similarly, - sd_bus_creds_get_ppid() will retrieve the - parent PID. Note that PID 1 has no parent process, in which case - -ENXIO is returned. + sd_bus_creds_get_pid() will retrieve the PID (process identifier). Similarly, + sd_bus_creds_get_ppid() will retrieve the parent PID. Note that PID 1 has no parent + process, in which case -ENXIO is returned. + + sd_bus_creds_get_pidfd_dup() will retrieve the PID file descriptor (pidfd), + see pidfd_open2 for + details. The file descriptor is duplicated and thus must be closed by the caller. sd_bus_creds_get_tid() will retrieve the TID (thread identifier). @@ -543,6 +553,7 @@ sd_bus_creds_get_tty(), and sd_bus_creds_get_description() were added in version 220. sd_bus_creds_get_user_slice() was added in version 223. + sd_bus_creds_get_pidfd_dup() was added in version 256. diff --git a/man/sd_bus_creds_new_from_pid.xml b/man/sd_bus_creds_new_from_pid.xml index ca9cab7292..4b39f4a003 100644 --- a/man/sd_bus_creds_new_from_pid.xml +++ b/man/sd_bus_creds_new_from_pid.xml @@ -17,6 +17,7 @@ sd_bus_creds_new_from_pid + sd_bus_creds_new_from_pidfd sd_bus_creds_get_mask sd_bus_creds_get_augmented_mask sd_bus_creds_ref @@ -37,6 +38,13 @@ sd_bus_creds **ret + + int sd_bus_creds_new_from_pidfd + int pidfd + uint64_t creds_mask + sd_bus_creds **ret + + uint64_t sd_bus_creds_get_mask sd_bus_creds *c @@ -98,6 +106,7 @@ SD_BUS_CREDS_UNIQUE_NAME, SD_BUS_CREDS_WELL_KNOWN_NAMES, SD_BUS_CREDS_DESCRIPTION, + SD_BUS_CREDS_PIDFD, SD_BUS_CREDS_AUGMENT, _SD_BUS_CREDS_ALL @@ -116,91 +125,65 @@ and sd_bus_message_get_creds3. - The information that will be stored is determined by - creds_mask. It may contain a subset of ORed - constants SD_BUS_CREDS_PID, - SD_BUS_CREDS_PPID, - SD_BUS_CREDS_TID, - SD_BUS_CREDS_UID, - SD_BUS_CREDS_EUID, - SD_BUS_CREDS_SUID, - SD_BUS_CREDS_FSUID, - SD_BUS_CREDS_GID, - SD_BUS_CREDS_EGID, - SD_BUS_CREDS_SGID, - SD_BUS_CREDS_FSGID, - SD_BUS_CREDS_SUPPLEMENTARY_GIDS, - SD_BUS_CREDS_COMM, - SD_BUS_CREDS_TID_COMM, - SD_BUS_CREDS_EXE, - SD_BUS_CREDS_CMDLINE, - SD_BUS_CREDS_CGROUP, - SD_BUS_CREDS_UNIT, - SD_BUS_CREDS_SLICE, - SD_BUS_CREDS_USER_UNIT, - SD_BUS_CREDS_USER_SLICE, - SD_BUS_CREDS_SESSION, - SD_BUS_CREDS_OWNER_UID, - SD_BUS_CREDS_EFFECTIVE_CAPS, - SD_BUS_CREDS_PERMITTED_CAPS, - SD_BUS_CREDS_INHERITABLE_CAPS, - SD_BUS_CREDS_BOUNDING_CAPS, - SD_BUS_CREDS_SELINUX_CONTEXT, - SD_BUS_CREDS_AUDIT_SESSION_ID, - SD_BUS_CREDS_AUDIT_LOGIN_UID, - SD_BUS_CREDS_TTY, - SD_BUS_CREDS_UNIQUE_NAME, - SD_BUS_CREDS_WELL_KNOWN_NAMES, and - SD_BUS_CREDS_DESCRIPTION. Use the special - value _SD_BUS_CREDS_ALL to request all - supported fields. The SD_BUS_CREDS_AUGMENT - constant may not be ORed into the mask for invocations of - sd_bus_creds_new_from_pid(). + sd_bus_creds_new_from_pidfd() is identical to + sd_bus_creds_new_from_pid(), but takes a PID file descriptor rather than a numeric + PID as reference to the process. See pidfd_open2. + + The information that will be stored is determined by creds_mask. It may + contain a subset of ORed constants SD_BUS_CREDS_PID, + SD_BUS_CREDS_PPID, SD_BUS_CREDS_TID, + SD_BUS_CREDS_UID, SD_BUS_CREDS_EUID, + SD_BUS_CREDS_SUID, SD_BUS_CREDS_FSUID, + SD_BUS_CREDS_GID, SD_BUS_CREDS_EGID, + SD_BUS_CREDS_SGID, SD_BUS_CREDS_FSGID, + SD_BUS_CREDS_SUPPLEMENTARY_GIDS, SD_BUS_CREDS_COMM, + SD_BUS_CREDS_TID_COMM, SD_BUS_CREDS_EXE, + SD_BUS_CREDS_CMDLINE, SD_BUS_CREDS_CGROUP, + SD_BUS_CREDS_UNIT, SD_BUS_CREDS_SLICE, + SD_BUS_CREDS_USER_UNIT, SD_BUS_CREDS_USER_SLICE, + SD_BUS_CREDS_SESSION, SD_BUS_CREDS_OWNER_UID, + SD_BUS_CREDS_EFFECTIVE_CAPS, SD_BUS_CREDS_PERMITTED_CAPS, + SD_BUS_CREDS_INHERITABLE_CAPS, SD_BUS_CREDS_BOUNDING_CAPS, + SD_BUS_CREDS_SELINUX_CONTEXT, SD_BUS_CREDS_AUDIT_SESSION_ID, + SD_BUS_CREDS_AUDIT_LOGIN_UID, SD_BUS_CREDS_TTY, + SD_BUS_CREDS_UNIQUE_NAME, SD_BUS_CREDS_WELL_KNOWN_NAMES, + SD_BUS_CREDS_DESCRIPTION, and SD_BUS_CREDS_PIDFD. Use the + special value _SD_BUS_CREDS_ALL to request all supported fields. The + SD_BUS_CREDS_AUGMENT constant may not be ORed into the mask for invocations of + sd_bus_creds_new_from_pid() or + sd_bus_creds_new_from_pidfd(). Fields can be retrieved from the credentials object using sd_bus_creds_get_pid3 and other functions which correspond directly to the constants listed above. - A mask of fields which were actually successfully retrieved - can be retrieved with - sd_bus_creds_get_mask(). If the credentials - object was created with - sd_bus_creds_new_from_pid(), this will be a - subset of fields requested in creds_mask. + A mask of fields which were actually successfully retrieved can be retrieved with + sd_bus_creds_get_mask(). If the credentials object was created with + sd_bus_creds_new_from_pid() or sd_bus_creds_new_from_pidfd(), + this will be a subset of fields requested in creds_mask. - Similar to sd_bus_creds_get_mask(), the - function sd_bus_creds_get_augmented_mask() - returns a bitmask of field constants. The mask indicates which - credential fields have been retrieved in a non-atomic fashion. For - credential objects created via - sd_bus_creds_new_from_pid(), this mask will be - identical to the mask returned by - sd_bus_creds_get_mask(). However, for - credential objects retrieved via - sd_bus_get_name_creds(), this mask will be set - for the credential fields that could not be determined atomically - at peer connection time, and which were later added by reading - augmenting credential data from - /proc/. Similarly, for credential objects - retrieved via sd_bus_get_owner_creds(), the - mask is set for the fields that could not be determined atomically - at bus creation time, but have been augmented. Similarly, for - credential objects retrieved via - sd_bus_message_get_creds(), the mask is set - for the fields that could not be determined atomically at message - sending time, but have been augmented. The mask returned by - sd_bus_creds_get_augmented_mask() is always a - subset of (or identical to) the mask returned by - sd_bus_creds_get_mask() for the same - object. The latter call hence returns all credential fields - available in the credential object, the former then marks the - subset of those that have been augmented. Note that augmented - fields are unsuitable for authorization decisions, as they may be - retrieved at different times, thus being subject to races. Hence, - augmented fields should be used exclusively for informational - purposes. + Similar to sd_bus_creds_get_mask(), the function + sd_bus_creds_get_augmented_mask() returns a bitmask of field constants. The mask + indicates which credential fields have been retrieved in a non-atomic fashion. For credential objects + created via sd_bus_creds_new_from_pid() or + sd_bus_creds_new_from_pidfd(), this mask will be identical to the mask returned by + sd_bus_creds_get_mask(). However, for credential objects retrieved via + sd_bus_get_name_creds(), this mask will be set for the credential fields that could + not be determined atomically at peer connection time, and which were later added by reading augmenting + credential data from /proc/. Similarly, for credential objects retrieved via + sd_bus_get_owner_creds(), the mask is set for the fields that could not be + determined atomically at bus creation time, but have been augmented. Similarly, for credential objects + retrieved via sd_bus_message_get_creds(), the mask is set for the fields that could + not be determined atomically at message sending time, but have been augmented. The mask returned by + sd_bus_creds_get_augmented_mask() is always a subset of (or identical to) the mask + returned by sd_bus_creds_get_mask() for the same object. The latter call hence + returns all credential fields available in the credential object, the former then marks the subset of + those that have been augmented. Note that augmented fields are unsuitable for authorization decisions, as + they may be retrieved at different times, thus being subject to races. Hence, augmented fields should be + used exclusively for informational purposes. sd_bus_creds_ref() creates a new @@ -234,9 +217,9 @@ Return Value - On success, sd_bus_creds_new_from_pid() - returns 0 or a positive integer. On failure, it returns a negative - errno-style error code. + On success, sd_bus_creds_new_from_pid() and + sd_bus_creds_new_from_pidfd() return 0 or a positive integer. On failure, they return + a negative errno-style error code. sd_bus_creds_get_mask() returns the mask of successfully acquired fields. @@ -256,9 +239,9 @@ Reference ownership - Function sd_bus_creds_new_from_pid() - creates a new object and the caller owns the sole reference. When - not needed anymore, this reference should be destroyed with + The functions sd_bus_creds_new_from_pid() and + sd_bus_creds_new_from_pidfd() create a new object and the caller owns the sole + reference. When not needed anymore, this reference should be destroyed with sd_bus_creds_unref3. @@ -307,6 +290,7 @@ sd_bus_creds_unref() were added in version 209. sd_bus_creds_get_augmented_mask() was added in version 223. sd_bus_creds_unrefp() was added in version 229. + sd_bus_creds_new_from_pidfd() was added in version 256. From c496e5f67f4518cfa8f7953b9ad0c0159d0a9429 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Jan 2024 18:42:33 +0100 Subject: [PATCH 8/8] update TODO --- TODO | 1 - 1 file changed, 1 deletion(-) diff --git a/TODO b/TODO index 74316fe3c9..e2e6162ce8 100644 --- a/TODO +++ b/TODO @@ -379,7 +379,6 @@ Features: - actually wait for POLLIN on pidref's pidfd in service logic - exec_spawn() + safe_fork() - openpt_allocate_in_namespace() - - sd_bus_creds - unit_attach_pid_to_cgroup_via_bus() - cg_attach() – requires new kernel feature