From 80d523dcc82268096d3a50643650413ef749e94b Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 20 Mar 2023 02:00:51 +0000 Subject: [PATCH 1/3] DBusCredentials: add support for PID FDs via SO_PEERPIDFD The new socket option SO_PEERPIDFD allows to pin the process on the other side of the socket by file descriptor, which closes a race condition where a PID can be reused before we can pin it manually. Available since Linux v6.5. When storing credentials, pin the process by FD from the PID. When querying the PID, if the PID FD is available, resolve it from there first if possible. Ensure the DBusCredentials object only returns the PID FD if it was obtained by this call, so that we know for sure we can rely on it being safe against PID reuse attacks. Signed-off-by: Luca Boccassi --- cmake/ConfigureChecks.cmake | 1 + cmake/config.h.cmake | 1 + configure.ac | 2 + dbus/dbus-auth.c | 18 +++- dbus/dbus-credentials.c | 118 ++++++++++++++++++++++++- dbus/dbus-credentials.h | 8 +- dbus/dbus-sysdeps-unix.c | 109 ++++++++++++++++++++++- dbus/dbus-sysdeps.h | 12 +++ meson.build | 8 ++ test/internals/dbus-credentials-util.c | 33 ++++--- 10 files changed, 289 insertions(+), 21 deletions(-) diff --git a/cmake/ConfigureChecks.cmake b/cmake/ConfigureChecks.cmake index 4e0c9ce6..737913ba 100644 --- a/cmake/ConfigureChecks.cmake +++ b/cmake/ConfigureChecks.cmake @@ -42,6 +42,7 @@ set(HAVE_BACKTRACE ${Backtrace_FOUND}) check_symbol_exists(LOG_PERROR "syslog.h" HAVE_DECL_LOG_PERROR) check_symbol_exists(MSG_NOSIGNAL "sys/socket.h" HAVE_DECL_MSG_NOSIGNAL) check_symbol_exists(SCM_RIGHTS "sys/types.h;sys/socket.h;sys/un.h" HAVE_UNIX_FD_PASSING) +check_symbol_exists(SYS_pidfd_open "sys/syscall.h" HAVE_DECL_SYS_PIDFD_OPEN) # dbus-sysdeps-unix.c check_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4) check_symbol_exists(clearenv "stdlib.h" HAVE_CLEARENV) # dbus-sysdeps.c check_symbol_exists(close_range "unistd.h" HAVE_CLOSE_RANGE) # dbus-sysdeps-unix.c diff --git a/cmake/config.h.cmake b/cmake/config.h.cmake index a4811b54..e16d2247 100644 --- a/cmake/config.h.cmake +++ b/cmake/config.h.cmake @@ -262,5 +262,6 @@ #cmakedefine01 HAVE_DECL_ENVIRON #cmakedefine01 HAVE_DECL_LOG_PERROR #cmakedefine01 HAVE_DECL_MSG_NOSIGNAL +#cmakedefine01 HAVE_DECL_SYS_PIDFD_OPEN #endif // _DBUS_CONFIG_H diff --git a/configure.ac b/configure.ac index f6ce9e6c..b1809e75 100644 --- a/configure.ac +++ b/configure.ac @@ -1029,6 +1029,8 @@ fi AC_CHECK_HEADERS(sys/vfs.h, [AC_CHECK_FUNCS(fstatfs)]) AC_CHECK_HEADERS([linux/magic.h]) +AC_CHECK_DECLS([SYS_pidfd_open], [], [], [[ #include ]]) + #### Set up final flags LIBDBUS_LIBS="$THREAD_LIBS $NETWORK_libs $SYSTEMD_LIBS" AC_SUBST([LIBDBUS_LIBS]) diff --git a/dbus/dbus-auth.c b/dbus/dbus-auth.c index bcb612a8..09942f80 100644 --- a/dbus/dbus-auth.c +++ b/dbus/dbus-auth.c @@ -803,8 +803,12 @@ sha1_handle_second_client_response (DBusAuth *auth, auth->desired_identity)) goto out_3; - /* Copy process ID from the socket credentials if it's there + /* Copy process ID (and PID FD) from the socket credentials if it's there */ + if (!_dbus_credentials_add_credential (auth->authorized_identity, + DBUS_CREDENTIAL_UNIX_PROCESS_FD, + auth->credentials)) + goto out_3; if (!_dbus_credentials_add_credential (auth->authorized_identity, DBUS_CREDENTIAL_UNIX_PROCESS_ID, auth->credentials)) @@ -1187,6 +1191,11 @@ handle_server_data_external_mech (DBusAuth *auth, /* also copy misc process info from the socket credentials */ + if (!_dbus_credentials_add_credential (auth->authorized_identity, + DBUS_CREDENTIAL_UNIX_PROCESS_FD, + auth->credentials)) + return FALSE; + if (!_dbus_credentials_add_credential (auth->authorized_identity, DBUS_CREDENTIAL_UNIX_PROCESS_ID, auth->credentials)) @@ -1304,8 +1313,13 @@ handle_server_data_anonymous_mech (DBusAuth *auth, /* We want to be anonymous (clear in case some other protocol got midway through I guess) */ _dbus_credentials_clear (auth->desired_identity); - /* Copy process ID from the socket credentials + /* Copy process ID (and PID FD) from the socket credentials */ + if (!_dbus_credentials_add_credential (auth->authorized_identity, + DBUS_CREDENTIAL_UNIX_PROCESS_FD, + auth->credentials)) + return FALSE; + if (!_dbus_credentials_add_credential (auth->authorized_identity, DBUS_CREDENTIAL_UNIX_PROCESS_ID, auth->credentials)) diff --git a/dbus/dbus-credentials.c b/dbus/dbus-credentials.c index d932865e..44cd40bf 100644 --- a/dbus/dbus-credentials.c +++ b/dbus/dbus-credentials.c @@ -25,8 +25,17 @@ #include #include #include +#ifdef HAVE_UNISTD_H +#include +#endif +#ifdef HAVE_SYS_SYSCALL_H +#include +#endif #include "dbus-credentials.h" #include "dbus-internals.h" +#ifdef DBUS_UNIX +#include "dbus-sysdeps-unix.h" +#endif /** * @defgroup DBusCredentials Credentials provable through authentication @@ -54,6 +63,7 @@ struct DBusCredentials { dbus_gid_t *unix_gids; size_t n_unix_gids; dbus_pid_t pid; + int pid_fd; char *windows_sid; char *linux_security_label; void *adt_audit_data; @@ -86,6 +96,7 @@ _dbus_credentials_new (void) creds->unix_gids = NULL; creds->n_unix_gids = 0; creds->pid = DBUS_PID_UNSET; + creds->pid_fd = -1; creds->windows_sid = NULL; creds->linux_security_label = NULL; creds->adt_audit_data = NULL; @@ -145,12 +156,22 @@ _dbus_credentials_unref (DBusCredentials *credentials) dbus_free (credentials->windows_sid); dbus_free (credentials->linux_security_label); dbus_free (credentials->adt_audit_data); +#ifdef DBUS_UNIX + if (credentials->pid_fd >= 0) + { + close (credentials->pid_fd); + credentials->pid_fd = -1; + } +#endif dbus_free (credentials); } } /** - * Add a UNIX process ID to the credentials. + * Add a UNIX process ID to the credentials. If the + * process ID FD is set, it will always take + * precendence when querying the PID of this + * credential. * * @param credentials the object * @param pid the process ID @@ -164,6 +185,30 @@ _dbus_credentials_add_pid (DBusCredentials *credentials, return TRUE; } +/** + * Add a UNIX process ID FD to the credentials. The + * FD is now owned by the credentials object. + * + * @param credentials the object + * @param pid_fd the process ID FD + * @returns #FALSE if no memory + */ +#ifndef DBUS_UNIX +_DBUS_GNUC_NORETURN +#endif +void +_dbus_credentials_take_pid_fd (DBusCredentials *credentials, + int pid_fd) +{ +#ifdef DBUS_UNIX + if (credentials->pid_fd >= 0) + close (credentials->pid_fd); + credentials->pid_fd = pid_fd; +#else + _dbus_assert_not_reached ("pidfd never set on non-Unix"); +#endif +} + /** * Add a UNIX user ID to the credentials. * @@ -323,7 +368,10 @@ _dbus_credentials_include (DBusCredentials *credentials, switch (type) { case DBUS_CREDENTIAL_UNIX_PROCESS_ID: - return credentials->pid != DBUS_PID_UNSET; + return credentials->pid != DBUS_PID_UNSET || + credentials->pid_fd >= 0; + case DBUS_CREDENTIAL_UNIX_PROCESS_FD: + return credentials->pid_fd >= 0; case DBUS_CREDENTIAL_UNIX_USER_ID: return credentials->unix_uid != DBUS_UID_UNSET; case DBUS_CREDENTIAL_UNIX_GROUP_IDS: @@ -343,6 +391,8 @@ _dbus_credentials_include (DBusCredentials *credentials, /** * Gets the UNIX process ID in the credentials, or #DBUS_PID_UNSET if * the credentials object doesn't contain a process ID. + * If the PID FD is set, it will first try to resolve from it, and + * only return the stored PID if that fails. * * @param credentials the object * @returns UNIX process ID @@ -350,9 +400,35 @@ _dbus_credentials_include (DBusCredentials *credentials, dbus_pid_t _dbus_credentials_get_pid (DBusCredentials *credentials) { +#ifdef DBUS_UNIX + dbus_pid_t pid; + + if (credentials->pid_fd >= 0) + { + pid = _dbus_resolve_pid_fd (credentials->pid_fd); + if (pid > 0) + return pid; + } +#endif + return credentials->pid; } +/** + * Gets the UNIX process ID FD in the credentials as obtained by 'safe' + * means (e.g.: Linux's SO_PEERPIDFD), or -1 if the credentials object + * doesn't contain a process ID FD. The file FD is owned by the credentials + * object and must not be closed by the caller. + * + * @param credentials the object + * @returns UNIX process ID FD + */ +int +_dbus_credentials_get_pid_fd (DBusCredentials *credentials) +{ + return credentials->pid_fd; +} + /** * Gets the UNIX user ID in the credentials, or #DBUS_UID_UNSET if * the credentials object doesn't contain a user ID. @@ -463,6 +539,7 @@ _dbus_credentials_are_empty (DBusCredentials *credentials) { return credentials->pid == DBUS_PID_UNSET && + credentials->pid_fd == -1 && credentials->unix_uid == DBUS_UID_UNSET && credentials->unix_gids == NULL && credentials->n_unix_gids == 0 && @@ -498,6 +575,9 @@ _dbus_credentials_add_credentials (DBusCredentials *credentials, DBusCredentials *other_credentials) { return + _dbus_credentials_add_credential (credentials, + DBUS_CREDENTIAL_UNIX_PROCESS_FD, + other_credentials) && _dbus_credentials_add_credential (credentials, DBUS_CREDENTIAL_UNIX_PROCESS_ID, other_credentials) && @@ -582,6 +662,19 @@ _dbus_credentials_add_credential (DBusCredentials *credentials, if (!_dbus_credentials_add_adt_audit_data (credentials, other_credentials->adt_audit_data, other_credentials->adt_audit_data_size)) return FALSE; } + /* _dbus_dup() is only available on UNIX platforms. */ +#ifdef DBUS_UNIX + else if (which == DBUS_CREDENTIAL_UNIX_PROCESS_FD && + other_credentials->pid_fd >= 0) + { + int pid_fd = _dbus_dup (other_credentials->pid_fd, NULL); + + if (pid_fd < 0) + return FALSE; + + _dbus_credentials_take_pid_fd (credentials, pid_fd); + } +#endif return TRUE; } @@ -595,6 +688,13 @@ void _dbus_credentials_clear (DBusCredentials *credentials) { credentials->pid = DBUS_PID_UNSET; +#ifdef DBUS_UNIX + if (credentials->pid_fd >= 0) + { + close (credentials->pid_fd); + credentials->pid_fd = -1; + } +#endif credentials->unix_uid = DBUS_UID_UNSET; dbus_free (credentials->unix_gids); credentials->unix_gids = NULL; @@ -677,9 +777,12 @@ _dbus_credentials_to_string_append (DBusCredentials *credentials, goto oom; join = TRUE; } - if (credentials->pid != DBUS_PID_UNSET) + if (credentials->pid != DBUS_PID_UNSET || credentials->pid_fd >= 0) { - if (!_dbus_string_append_printf (string, "%spid=" DBUS_PID_FORMAT, join ? " " : "", credentials->pid)) + if (!_dbus_string_append_printf (string, + "%spid=" DBUS_PID_FORMAT, + join ? " " : "", + _dbus_credentials_get_pid (credentials))) goto oom; join = TRUE; } @@ -715,6 +818,13 @@ _dbus_credentials_to_string_append (DBusCredentials *credentials, join = TRUE; } + if (credentials->pid_fd >= 0) + { + if (!_dbus_string_append_printf (string, "%spidfd=%d", join ? " " : "", credentials->pid_fd)) + goto oom; + join = TRUE; + } + return TRUE; oom: return FALSE; diff --git a/dbus/dbus-credentials.h b/dbus/dbus-credentials.h index 6da0d1ae..407d5cb5 100644 --- a/dbus/dbus-credentials.h +++ b/dbus/dbus-credentials.h @@ -38,7 +38,8 @@ typedef enum { DBUS_CREDENTIAL_UNIX_GROUP_IDS, DBUS_CREDENTIAL_ADT_AUDIT_DATA_ID, DBUS_CREDENTIAL_LINUX_SECURITY_LABEL, - DBUS_CREDENTIAL_WINDOWS_SID + DBUS_CREDENTIAL_WINDOWS_SID, + DBUS_CREDENTIAL_UNIX_PROCESS_FD, } DBusCredentialType; DBUS_PRIVATE_EXPORT @@ -53,6 +54,9 @@ DBUS_PRIVATE_EXPORT dbus_bool_t _dbus_credentials_add_pid (DBusCredentials *credentials, dbus_pid_t pid); DBUS_PRIVATE_EXPORT +void _dbus_credentials_take_pid_fd (DBusCredentials *credentials, + int pid_fd); +DBUS_PRIVATE_EXPORT dbus_bool_t _dbus_credentials_add_unix_uid (DBusCredentials *credentials, dbus_uid_t uid); DBUS_PRIVATE_EXPORT @@ -73,6 +77,8 @@ dbus_bool_t _dbus_credentials_include (DBusCredentials DBUS_PRIVATE_EXPORT dbus_pid_t _dbus_credentials_get_pid (DBusCredentials *credentials); DBUS_PRIVATE_EXPORT +int _dbus_credentials_get_pid_fd (DBusCredentials *credentials); +DBUS_PRIVATE_EXPORT dbus_uid_t _dbus_credentials_get_unix_uid (DBusCredentials *credentials); DBUS_PRIVATE_EXPORT dbus_bool_t _dbus_credentials_get_unix_gids (DBusCredentials *credentials, diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c index 6962b077..c66096e0 100644 --- a/dbus/dbus-sysdeps-unix.c +++ b/dbus/dbus-sysdeps-unix.c @@ -2217,6 +2217,7 @@ _dbus_read_credentials_socket (DBusSocket client_fd, dbus_gid_t primary_gid_read; dbus_pid_t pid_read; int bytes_read; + int pid_fd_read; #ifdef HAVE_CMSGCRED union { @@ -2236,6 +2237,7 @@ _dbus_read_credentials_socket (DBusSocket client_fd, uid_read = DBUS_UID_UNSET; primary_gid_read = DBUS_GID_UNSET; pid_read = DBUS_PID_UNSET; + pid_fd_read = -1; _DBUS_ASSERT_ERROR_IS_CLEAR (error); @@ -2328,6 +2330,24 @@ _dbus_read_credentials_socket (DBusSocket client_fd, primary_gid_read = cr.gid; #endif } + +#ifdef SO_PEERPIDFD + /* If we have SO_PEERCRED we might also have SO_PEERPIDFD, which + * allows to pin the process ID, and is available on Linux since v6.5. */ + cr_len = sizeof (int); + + if (getsockopt (client_fd.fd, SOL_SOCKET, SO_PEERPIDFD, &pid_fd_read, &cr_len) != 0) + { + _dbus_verbose ("Failed to getsockopt(SO_PEERPIDFD): %s\n", + _dbus_strerror (errno)); + } + else if (cr_len != sizeof (int)) + { + _dbus_verbose ("Failed to getsockopt(SO_PEERPIDFD), returned %d bytes, expected %d\n", + cr_len, (int) sizeof (int)); + } +#endif + #elif defined(HAVE_UNPCBID) && defined(LOCAL_PEEREID) /* Another variant of the above - used on NetBSD */ @@ -2483,6 +2503,11 @@ _dbus_read_credentials_socket (DBusSocket client_fd, pid_read, uid_read); + /* Assign this first, so we don't have to close it manually in case one of + * the next steps fails. */ + if (pid_fd_read >= 0) + _dbus_credentials_take_pid_fd (credentials, pid_fd_read); + if (pid_read != DBUS_PID_UNSET) { if (!_dbus_credentials_add_pid (credentials, pid_read)) @@ -2960,6 +2985,8 @@ _dbus_user_info_fill_uid (DBusUserInfo *info, dbus_bool_t _dbus_credentials_add_from_current_process (DBusCredentials *credentials) { + dbus_pid_t pid = _dbus_getpid (); + /* The POSIX spec certainly doesn't promise this, but * we need these assertions to fail as soon as we're wrong about * it so we can do the porting fixups @@ -2968,7 +2995,14 @@ _dbus_credentials_add_from_current_process (DBusCredentials *credentials) _DBUS_STATIC_ASSERT (sizeof (uid_t) <= sizeof (dbus_uid_t)); _DBUS_STATIC_ASSERT (sizeof (gid_t) <= sizeof (dbus_gid_t)); - if (!_dbus_credentials_add_pid(credentials, _dbus_getpid())) +#if HAVE_DECL_SYS_PIDFD_OPEN + /* Normally this syscall would have a race condition, but we can trust + * that our own process isn't going to exit, so the pid won't get reused. */ + int pid_fd = (int) syscall (SYS_pidfd_open, pid, 0); + if (pid_fd >= 0) + _dbus_credentials_take_pid_fd (credentials, pid_fd); +#endif + if (!_dbus_credentials_add_pid (credentials, pid)) return FALSE; if (!_dbus_credentials_add_unix_uid(credentials, _dbus_geteuid())) return FALSE; @@ -2976,6 +3010,79 @@ _dbus_credentials_add_from_current_process (DBusCredentials *credentials) return TRUE; } +/** + * Resolve the PID from the PID FD, if any. This allows us to avoid + * PID reuse attacks. Returns DBUS_PID_UNSET if the PID could not be resolved. + * Note that this requires being able to read /proc/self/fdinfo/, + * which is created as 600 and owned by the original UID that the + * process started as. So it cannot work when the start as root and + * drop privileges mechanism is in use (the systemd unit no longer + * does this, but third-party init-scripts might). + * + * @param pid_fd the PID FD + * @returns the resolved PID if found, DBUS_PID_UNSET otherwise + */ +dbus_pid_t +_dbus_resolve_pid_fd (int pid_fd) +{ +#ifdef __linux__ + DBusError error = DBUS_ERROR_INIT; + DBusString content = _DBUS_STRING_INIT_INVALID; + DBusString filename = _DBUS_STRING_INIT_INVALID; + dbus_pid_t result = DBUS_PID_UNSET; + int pid_index; + + if (pid_fd < 0) + goto out; + + if (!_dbus_string_init (&content)) + goto out; + + if (!_dbus_string_init (&filename)) + goto out; + + if (!_dbus_string_append_printf (&filename, "/proc/self/fdinfo/%d", pid_fd)) + goto out; + + if (!_dbus_file_get_contents (&content, &filename, &error)) + { + _dbus_verbose ("Cannot read '/proc/self/fdinfo/%d', unable to resolve PID, %s: %s\n", + pid_fd, error.name, error.message); + goto out; + } + + /* Ensure we are not reading PPid, either it's the first line of the file or + * there's a newline before it. */ + if (!_dbus_string_find (&content, 0, "Pid:", &pid_index) || + (pid_index > 0 && _dbus_string_get_byte (&content, pid_index - 1) != '\n')) + { + _dbus_verbose ("Cannot find 'Pid:' in '/proc/self/fdinfo/%d', unable to resolve PID\n", + pid_fd); + goto out; + } + + if (!_dbus_string_parse_uint (&content, pid_index + strlen ("Pid:"), &result, NULL)) + { + _dbus_verbose ("Cannot parse 'Pid:' from '/proc/self/fdinfo/%d', unable to resolve PID\n", + pid_fd); + goto out; + } + +out: + _dbus_string_free (&content); + _dbus_string_free (&filename); + dbus_error_free (&error); + + if (result <= 0) + return DBUS_PID_UNSET; + + return result; +#else + return DBUS_PID_UNSET; +#endif + +} + /** * Append to the string the identity we would like to have when we * authenticate, on UNIX this is the current process UID and on diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index 36ababdb..e9737179 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -311,6 +311,18 @@ dbus_bool_t _dbus_daemon_unpublish_session_bus_address (void); dbus_bool_t _dbus_socket_can_pass_unix_fd(DBusSocket fd); +/* PID FDs are Linux-specific. */ +#ifdef DBUS_WIN +static inline dbus_pid_t _dbus_resolve_pid_fd (int pid_fd) +{ + return DBUS_PID_UNSET; +} + +#else +DBUS_PRIVATE_EXPORT +dbus_pid_t _dbus_resolve_pid_fd (int pid_fd); +#endif + /** Opaque type representing an atomically-modifiable integer * that can be used from multiple threads. */ diff --git a/meson.build b/meson.build index 5438b184..9bfcb4a8 100644 --- a/meson.build +++ b/meson.build @@ -769,6 +769,14 @@ config.set('HAVE_FSTATFS', ) ) +config.set10('HAVE_DECL_SYS_PIDFD_OPEN', + cc.has_header_symbol( + 'sys/syscall.h', + 'SYS_pidfd_open', + args: compile_args_c, + ) +) + ############################################################################### # Project options diff --git a/test/internals/dbus-credentials-util.c b/test/internals/dbus-credentials-util.c index 06400925..6fada302 100644 --- a/test/internals/dbus-credentials-util.c +++ b/test/internals/dbus-credentials-util.c @@ -129,13 +129,15 @@ _dbus_credentials_test (const char *test_data_dir) DBusCredentials *creds; DBusCredentials *creds2; DBusString str; + DBusString str2; const dbus_gid_t *gids; size_t n; + dbus_pid_t pid = _dbus_getpid(); if (test_data_dir == NULL) return TRUE; - creds = make_credentials (12, 511, 1, SAMPLE_SID); + creds = make_credentials (12, pid, 1, SAMPLE_SID); if (creds == NULL) _dbus_test_fatal ("oom"); @@ -149,7 +151,7 @@ _dbus_credentials_test (const char *test_data_dir) _dbus_assert (_dbus_credentials_include (creds, DBUS_CREDENTIAL_WINDOWS_SID)); _dbus_assert (_dbus_credentials_get_unix_uid (creds) == 12); - _dbus_assert (_dbus_credentials_get_pid (creds) == 511); + _dbus_assert (_dbus_credentials_get_pid (creds) == pid); _dbus_assert (strcmp (_dbus_credentials_get_windows_sid (creds), SAMPLE_SID) == 0); _dbus_assert (_dbus_credentials_get_unix_gids (creds, &gids, &n)); _dbus_assert (n == 4); @@ -172,7 +174,7 @@ _dbus_credentials_test (const char *test_data_dir) _dbus_assert (_dbus_credentials_include (creds2, DBUS_CREDENTIAL_WINDOWS_SID)); _dbus_assert (_dbus_credentials_get_unix_uid (creds2) == 12); - _dbus_assert (_dbus_credentials_get_pid (creds2) == 511); + _dbus_assert (_dbus_credentials_get_pid (creds2) == pid); _dbus_assert (strcmp (_dbus_credentials_get_windows_sid (creds2), SAMPLE_SID) == 0); _dbus_assert (_dbus_credentials_get_unix_gids (creds2, &gids, &n)); _dbus_assert (n == 4); @@ -245,7 +247,7 @@ _dbus_credentials_test (const char *test_data_dir) _dbus_credentials_unref (creds2); /* Same user, but not a superset, if groups are different */ - creds2 = make_credentials (12, 511, 2, SAMPLE_SID); + creds2 = make_credentials (12, pid, 2, SAMPLE_SID); if (creds2 == NULL) _dbus_test_fatal ("oom"); @@ -255,7 +257,7 @@ _dbus_credentials_test (const char *test_data_dir) _dbus_credentials_unref (creds2); /* Groups being in the same order make no difference */ - creds2 = make_credentials (12, 511, 3, SAMPLE_SID); + creds2 = make_credentials (12, pid, 3, SAMPLE_SID); if (creds2 == NULL) _dbus_test_fatal ("oom"); @@ -282,7 +284,7 @@ _dbus_credentials_test (const char *test_data_dir) _dbus_credentials_unref (creds); /* Make some more realistic credentials blobs to test stringification */ - if (!_dbus_string_init (&str)) + if (!_dbus_string_init (&str) || !_dbus_string_init (&str2)) _dbus_test_fatal ("oom"); creds = make_credentials (12, DBUS_PID_UNSET, 0, NULL); @@ -298,7 +300,7 @@ _dbus_credentials_test (const char *test_data_dir) _dbus_credentials_unref (creds); - creds = make_credentials (12, 511, 1, NULL); + creds = make_credentials (12, pid, 1, NULL); if (creds == NULL) _dbus_test_fatal ("oom"); @@ -308,9 +310,11 @@ _dbus_credentials_test (const char *test_data_dir) if (!_dbus_credentials_to_string_append (creds, &str)) _dbus_test_fatal ("oom"); + if (!_dbus_string_append_printf(&str2, "uid=12 pid=" DBUS_PID_FORMAT " gid=42 gid=123 gid=1000 gid=5678", pid)) + _dbus_test_fatal ("oom"); + _dbus_test_diag ("Unix complete set: %s", _dbus_string_get_const_data (&str)); - _dbus_assert (strcmp (_dbus_string_get_const_data (&str), - "uid=12 pid=511 gid=42 gid=123 gid=1000 gid=5678") == 0); + _dbus_assert (strcmp (_dbus_string_get_const_data (&str), _dbus_string_get_const_data (&str2)) == 0); _dbus_credentials_unref (creds); @@ -330,23 +334,26 @@ _dbus_credentials_test (const char *test_data_dir) _dbus_credentials_unref (creds); - creds = make_credentials (DBUS_UID_UNSET, 511, 0, SAMPLE_SID); + creds = make_credentials (DBUS_UID_UNSET, pid, 0, SAMPLE_SID); if (creds == NULL) _dbus_test_fatal ("oom"); - if (!_dbus_string_set_length (&str, 0)) + if (!_dbus_string_set_length (&str, 0) || !_dbus_string_set_length (&str2, 0)) _dbus_test_fatal ("oom"); if (!_dbus_credentials_to_string_append (creds, &str)) _dbus_test_fatal ("oom"); + if (!_dbus_string_append_printf(&str2, "pid=" DBUS_PID_FORMAT " sid=" SAMPLE_SID, pid)) + _dbus_test_fatal ("oom"); + _dbus_test_diag ("Windows complete set: %s", _dbus_string_get_const_data (&str)); - _dbus_assert (strcmp (_dbus_string_get_const_data (&str), - "pid=511 sid=" SAMPLE_SID) == 0); + _dbus_assert (strcmp (_dbus_string_get_const_data (&str), _dbus_string_get_const_data (&str2)) == 0); _dbus_credentials_unref (creds); _dbus_string_free (&str); + _dbus_string_free (&str2); return TRUE; } From 8883f0dd010170df6832be97030519abd4e6e753 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 20 Mar 2023 02:35:10 +0000 Subject: [PATCH 2/3] _dbus_asv_add_unix_fd: add Add a new helper to add unix FDs to arrays. Will be used for GetConnectionCredentials(). Signed-off-by: Luca Boccassi --- dbus/dbus-asv-util.c | 39 +++++++++++++++++++++++++++++++++++++++ dbus/dbus-asv-util.h | 3 +++ 2 files changed, 42 insertions(+) diff --git a/dbus/dbus-asv-util.c b/dbus/dbus-asv-util.c index ace8f503..5ac0ef2b 100644 --- a/dbus/dbus-asv-util.c +++ b/dbus/dbus-asv-util.c @@ -376,3 +376,42 @@ _dbus_asv_add_byte_array (DBusMessageIter *arr_iter, return _dbus_asv_add_fixed_array (arr_iter, key, DBUS_TYPE_BYTE, value, n_elements); } + +/** + * Create a new entry in an a{sv} (map from string to variant) + * with a Unix file descriptor as value. + * + * If this function fails, the a{sv} must be abandoned, for instance + * with _dbus_asv_abandon(). + * + * The FD remains owned by the caller regardless of the result returned + * by this function. + * + * @param arr_iter the iterator which is appending to the array + * @param key a UTF-8 key for the map + * @param value the value + * @returns #TRUE on success, or #FALSE if not enough memory + */ +dbus_bool_t +_dbus_asv_add_unix_fd (DBusMessageIter *arr_iter, + const char *key, + int value) +{ + DBusMessageIter entry_iter, var_iter; + + if (!_dbus_asv_open_entry (arr_iter, &entry_iter, key, + DBUS_TYPE_UNIX_FD_AS_STRING, &var_iter)) + return FALSE; + + if (!dbus_message_iter_append_basic (&var_iter, DBUS_TYPE_UNIX_FD, + &value)) + { + _dbus_asv_abandon_entry (arr_iter, &entry_iter, &var_iter); + return FALSE; + } + + if (!_dbus_asv_close_entry (arr_iter, &entry_iter, &var_iter)) + return FALSE; + + return TRUE; +} diff --git a/dbus/dbus-asv-util.h b/dbus/dbus-asv-util.h index da1ef25b..466885ab 100644 --- a/dbus/dbus-asv-util.h +++ b/dbus/dbus-asv-util.h @@ -67,5 +67,8 @@ dbus_bool_t _dbus_asv_close_entry (DBusMessageIter *arr_iter, void _dbus_asv_abandon_entry (DBusMessageIter *arr_iter, DBusMessageIter *entry_iter, DBusMessageIter *var_iter); +dbus_bool_t _dbus_asv_add_unix_fd (DBusMessageIter *arr_iter, + const char *key, + int value); #endif From 7a4c47a929f1ae2b0e725f329488ea14c2e230db Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 20 Mar 2023 01:55:18 +0000 Subject: [PATCH 3/3] bus: return ProcessFD in GetConnectionCredentials() Allows to track a process by pinning to a file descriptor, which unlike a PID cannot be reused. root@image:~# busctl call org.freedesktop.DBus /org/freedesktop/DBus org.freedesktop.DBus GetConnectionCredentials "s" org.freedesktop.systemd1 a{sv} 3 "ProcessID" u 1 "UnixUserID" u 0 "ProcessFD" h 4 Signed-off-by: Luca Boccassi --- bus/containers.c | 2 ++ bus/driver.c | 26 ++++++++++++++++++++------ bus/driver.h | 3 ++- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/bus/containers.c b/bus/containers.c index 470177c0..816f3e1b 100644 --- a/bus/containers.c +++ b/bus/containers.c @@ -1206,6 +1206,7 @@ bus_containers_handle_get_connection_instance (DBusConnection *caller, goto oom; if (!bus_driver_fill_connection_credentials (NULL, instance->creator, + caller, &arr_writer)) { dbus_message_iter_abandon_container (&writer, &arr_writer); @@ -1289,6 +1290,7 @@ bus_containers_handle_get_instance_info (DBusConnection *connection, goto oom; if (!bus_driver_fill_connection_credentials (NULL, instance->creator, + connection, &arr_writer)) { dbus_message_iter_abandon_container (&writer, &arr_writer); diff --git a/bus/driver.c b/bus/driver.c index b1d34f68..7bc12f42 100644 --- a/bus/driver.c +++ b/bus/driver.c @@ -1970,7 +1970,8 @@ bus_driver_credentials_fill_unix_gids (DBusCredentials *credentials, */ dbus_bool_t bus_driver_fill_connection_credentials (DBusCredentials *credentials, - DBusConnection *conn, + DBusConnection *peer_conn, + DBusConnection *caller_conn, DBusMessageIter *asv_iter) { dbus_uid_t uid = DBUS_UID_UNSET; @@ -1980,13 +1981,19 @@ bus_driver_fill_connection_credentials (DBusCredentials *credentials, #ifdef DBUS_ENABLE_CONTAINERS const char *path; #endif +#ifdef HAVE_UNIX_FD_PASSING + int pid_fd = -1; /* owned by credentials */ +#endif - if (credentials == NULL && conn != NULL) - credentials = _dbus_connection_get_credentials (conn); + if (credentials == NULL && peer_conn != NULL) + credentials = _dbus_connection_get_credentials (peer_conn); if (credentials != NULL) { pid = _dbus_credentials_get_pid (credentials); +#ifdef HAVE_UNIX_FD_PASSING + pid_fd = _dbus_credentials_get_pid_fd (credentials); +#endif uid = _dbus_credentials_get_unix_uid (credentials); windows_sid = _dbus_credentials_get_windows_sid (credentials); linux_security_label = @@ -2036,8 +2043,8 @@ bus_driver_fill_connection_credentials (DBusCredentials *credentials, #ifdef DBUS_ENABLE_CONTAINERS /* This has to come from the connection, not the credentials */ - if (conn != NULL && - bus_containers_connection_is_contained (conn, &path, NULL, NULL)) + if (peer_conn != NULL && + bus_containers_connection_is_contained (peer_conn, &path, NULL, NULL)) { if (!_dbus_asv_add_object_path (asv_iter, DBUS_INTERFACE_CONTAINERS1 ".Instance", @@ -2046,6 +2053,13 @@ bus_driver_fill_connection_credentials (DBusCredentials *credentials, } #endif +#ifdef HAVE_UNIX_FD_PASSING + if (caller_conn != NULL && pid_fd >= 0 && + dbus_connection_can_send_type (caller_conn, DBUS_TYPE_UNIX_FD) && + !_dbus_asv_add_unix_fd (asv_iter, "ProcessFD", pid_fd)) + return FALSE; +#endif + return TRUE; } @@ -2094,7 +2108,7 @@ bus_driver_handle_get_connection_credentials (DBusConnection *connection, reply = _dbus_asv_new_method_return (message, &reply_iter, &array_iter); if (reply == NULL || - !bus_driver_fill_connection_credentials (credentials, conn, &array_iter) || + !bus_driver_fill_connection_credentials (credentials, conn, connection, &array_iter) || !_dbus_asv_close (&reply_iter, &array_iter)) goto oom; diff --git a/bus/driver.h b/bus/driver.h index 2c03a4ae..4365e016 100644 --- a/bus/driver.h +++ b/bus/driver.h @@ -58,7 +58,8 @@ dbus_bool_t bus_driver_generate_introspect_string (DBusString *xml, dbus_bool_t canonical_path, DBusMessage *message); dbus_bool_t bus_driver_fill_connection_credentials (DBusCredentials *credentials, - DBusConnection *conn, + DBusConnection *peer_conn, + DBusConnection *caller_conn, DBusMessageIter *asv_iter); BusDriverFound bus_driver_get_conn_helper (DBusConnection *connection,