diff -up ./src/polkitbackend/polkitbackendinteractiveauthority.c.ori ./src/polkitbackend/polkitbackendinteractiveauthority.c --- ./src/polkitbackend/polkitbackendinteractiveauthority.c.ori 2019-10-23 14:05:44.252434959 +0200 +++ ./src/polkitbackend/polkitbackendinteractiveauthority.c 2019-10-23 14:07:08.329436414 +0200 @@ -556,7 +556,7 @@ log_result (PolkitBackendInteractiveAuth if (polkit_authorization_result_get_is_authorized (result)) log_result_str = "ALLOWING"; - user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, subject, NULL); + user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, subject, NULL, NULL); subject_str = polkit_subject_to_string (subject); user_of_subject_str = polkit_identity_to_string (user_of_subject); @@ -824,6 +824,7 @@ polkit_backend_interactive_authority_che gchar *subject_str; PolkitIdentity *user_of_caller; PolkitIdentity *user_of_subject; + gboolean user_of_subject_matches; gchar *user_of_caller_str; gchar *user_of_subject_str; PolkitAuthorizationResult *result; @@ -869,7 +870,7 @@ polkit_backend_interactive_authority_che action_id); user_of_caller = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, - caller, + caller, NULL, &error); if (error != NULL) { @@ -884,7 +885,7 @@ polkit_backend_interactive_authority_che g_debug (" user of caller is %s", user_of_caller_str); user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, - subject, + subject, &user_of_subject_matches, &error); if (error != NULL) { @@ -914,7 +915,10 @@ polkit_backend_interactive_authority_che * We only allow this if, and only if, * * - processes may check for another process owned by the *same* user but not - * if details are passed (otherwise you'd be able to spoof the dialog) + * if details are passed (otherwise you'd be able to spoof the dialog); + * the caller supplies the user_of_subject value, so we additionally + * require it to match at least at one point in time (via + * user_of_subject_matches). * * - processes running as uid 0 may check anything and pass any details * @@ -922,7 +926,9 @@ polkit_backend_interactive_authority_che * then any uid referenced by that annotation is also allowed to check * to check anything and pass any details */ - if (!polkit_identity_equal (user_of_caller, user_of_subject) || has_details) + if (!user_of_subject_matches + || !polkit_identity_equal (user_of_caller, user_of_subject) + || has_details) { if (!may_identity_check_authorization (interactive_authority, action_id, user_of_caller)) { @@ -1087,9 +1093,10 @@ check_authorization_sync (PolkitBackendA goto out; } - /* every subject has a user */ + /* every subject has a user; this is supplied by the client, so we rely + * on the caller to validate its acceptability. */ user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, - subject, + subject, NULL, error); if (user_of_subject == NULL) goto out; @@ -2369,6 +2376,7 @@ polkit_backend_interactive_authority_reg PolkitSubject *session_for_caller; PolkitIdentity *user_of_caller; PolkitIdentity *user_of_subject; + gboolean user_of_subject_matches; AuthenticationAgent *agent; gboolean ret; gchar *caller_cmdline; @@ -2423,7 +2431,7 @@ polkit_backend_interactive_authority_reg goto out; } - user_of_caller = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, caller, NULL); + user_of_caller = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, caller, NULL, NULL); if (user_of_caller == NULL) { g_set_error (error, @@ -2432,7 +2440,7 @@ polkit_backend_interactive_authority_reg "Cannot determine user of caller"); goto out; } - user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, subject, NULL); + user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, subject, &user_of_subject_matches, NULL); if (user_of_subject == NULL) { g_set_error (error, @@ -2441,7 +2449,8 @@ polkit_backend_interactive_authority_reg "Cannot determine user of subject"); goto out; } - if (!polkit_identity_equal (user_of_caller, user_of_subject)) + if (!user_of_subject_matches + || !polkit_identity_equal (user_of_caller, user_of_subject)) { if (POLKIT_IS_UNIX_USER (user_of_caller) && polkit_unix_user_get_uid (POLKIT_UNIX_USER (user_of_caller)) == 0) { @@ -2528,6 +2537,7 @@ polkit_backend_interactive_authority_unr PolkitSubject *session_for_caller; PolkitIdentity *user_of_caller; PolkitIdentity *user_of_subject; + gboolean user_of_subject_matches; AuthenticationAgent *agent; gboolean ret; gchar *scope_str; @@ -2576,7 +2586,7 @@ polkit_backend_interactive_authority_unr goto out; } - user_of_caller = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, caller, NULL); + user_of_caller = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, caller, NULL, NULL); if (user_of_caller == NULL) { g_set_error (error, @@ -2585,7 +2595,7 @@ polkit_backend_interactive_authority_unr "Cannot determine user of caller"); goto out; } - user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, subject, NULL); + user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, subject, &user_of_subject_matches, NULL); if (user_of_subject == NULL) { g_set_error (error, @@ -2594,7 +2604,8 @@ polkit_backend_interactive_authority_unr "Cannot determine user of subject"); goto out; } - if (!polkit_identity_equal (user_of_caller, user_of_subject)) + if (!user_of_subject_matches + || !polkit_identity_equal (user_of_caller, user_of_subject)) { if (POLKIT_IS_UNIX_USER (user_of_caller) && polkit_unix_user_get_uid (POLKIT_UNIX_USER (user_of_caller)) == 0) { @@ -2703,7 +2714,7 @@ polkit_backend_interactive_authority_aut identity_str); user_of_caller = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, - caller, + caller, NULL, error); if (user_of_caller == NULL) goto out; diff -up ./src/polkitbackend/polkitbackendsessionmonitor.c.ori ./src/polkitbackend/polkitbackendsessionmonitor.c --- ./src/polkitbackend/polkitbackendsessionmonitor.c.ori 2013-02-02 04:22:52.000000000 +0100 +++ ./src/polkitbackend/polkitbackendsessionmonitor.c 2019-10-23 15:05:06.313135051 +0200 @@ -27,6 +27,7 @@ #include #include +#include #include "polkitbackendsessionmonitor.h" #define CKDB_PATH "/var/run/ConsoleKit/database" @@ -273,28 +274,40 @@ polkit_backend_session_monitor_get_sessi * polkit_backend_session_monitor_get_user: * @monitor: A #PolkitBackendSessionMonitor. * @subject: A #PolkitSubject. + * @result_matches: If not %NULL, set to indicate whether the return value matches current (RACY) state. * @error: Return location for error. * * Gets the user corresponding to @subject or %NULL if no user exists. * + * NOTE: For a #PolkitUnixProcess, the UID is read from @subject (which may + * come from e.g. a D-Bus client), so it may not correspond to the actual UID + * of the referenced process (at any point in time). This is indicated by + * setting @result_matches to %FALSE; the caller may reject such subjects or + * require additional privileges. @result_matches == %TRUE only indicates that + * the UID matched the underlying process at ONE point in time, it may not match + * later. + * * Returns: %NULL if @error is set otherwise a #PolkitUnixUser that should be freed with g_object_unref(). */ PolkitIdentity * polkit_backend_session_monitor_get_user_for_subject (PolkitBackendSessionMonitor *monitor, PolkitSubject *subject, + gboolean *result_matches, GError **error) { PolkitIdentity *ret; + gboolean matches; GError *local_error; - gchar *group; - guint32 uid; ret = NULL; + matches = FALSE; if (POLKIT_IS_UNIX_PROCESS (subject)) { - uid = polkit_unix_process_get_uid (POLKIT_UNIX_PROCESS (subject)); - if ((gint) uid == -1) + gint subject_uid, current_uid; + + subject_uid = polkit_unix_process_get_uid (POLKIT_UNIX_PROCESS (subject)); + if (subject_uid == -1) { g_set_error (error, POLKIT_ERROR, @@ -302,11 +315,21 @@ polkit_backend_session_monitor_get_user_ "Unix process subject does not have uid set"); goto out; } - ret = polkit_unix_user_new (uid); + local_error = NULL; + current_uid = polkit_unix_process_get_owner (POLKIT_UNIX_PROCESS (subject), &local_error); + if (local_error != NULL) + { + g_propagate_error (error, local_error); + goto out; + } + ret = polkit_unix_user_new (subject_uid); + matches = (subject_uid == current_uid); + } else if (POLKIT_IS_SYSTEM_BUS_NAME (subject)) { GVariant *result; + gint uid; result = g_dbus_connection_call_sync (monitor->system_bus, "org.freedesktop.DBus", @@ -325,9 +348,13 @@ polkit_backend_session_monitor_get_user_ g_variant_unref (result); ret = polkit_unix_user_new (uid); + matches = TRUE; } else if (POLKIT_IS_UNIX_SESSION (subject)) { + gint uid; + gchar *group; + if (!ensure_database (monitor, error)) { g_prefix_error (error, "Error getting user for session: Error ensuring CK database at " CKDB_PATH ": "); @@ -346,9 +373,14 @@ polkit_backend_session_monitor_get_user_ g_free (group); ret = polkit_unix_user_new (uid); + matches = TRUE; } out: + if (result_matches != NULL) + { + *result_matches = matches; + } return ret; } diff -up ./src/polkitbackend/polkitbackendsessionmonitor.h.ori ./src/polkitbackend/polkitbackendsessionmonitor.h --- ./src/polkitbackend/polkitbackendsessionmonitor.h.ori 2013-02-02 04:22:52.000000000 +0100 +++ ./src/polkitbackend/polkitbackendsessionmonitor.h 2019-10-23 14:07:08.330436425 +0200 @@ -47,6 +47,7 @@ GList *polkit_back PolkitIdentity *polkit_backend_session_monitor_get_user_for_subject (PolkitBackendSessionMonitor *monitor, PolkitSubject *subject, + gboolean *result_matches, GError **error); PolkitSubject *polkit_backend_session_monitor_get_session_for_subject (PolkitBackendSessionMonitor *monitor, diff -up ./src/polkitbackend/polkitbackendsessionmonitor-systemd.c.ori ./src/polkitbackend/polkitbackendsessionmonitor-systemd.c --- ./src/polkitbackend/polkitbackendsessionmonitor-systemd.c.ori 2013-02-02 04:22:52.000000000 +0100 +++ ./src/polkitbackend/polkitbackendsessionmonitor-systemd.c 2019-10-23 15:04:36.304774015 +0200 @@ -29,6 +29,7 @@ #include #include +#include #include "polkitbackendsessionmonitor.h" /* @@ -246,26 +247,40 @@ polkit_backend_session_monitor_get_sessi * polkit_backend_session_monitor_get_user: * @monitor: A #PolkitBackendSessionMonitor. * @subject: A #PolkitSubject. + * @result_matches: If not %NULL, set to indicate whether the return value matches current (RACY) state. * @error: Return location for error. * * Gets the user corresponding to @subject or %NULL if no user exists. * + * NOTE: For a #PolkitUnixProcess, the UID is read from @subject (which may + * come from e.g. a D-Bus client), so it may not correspond to the actual UID + * of the referenced process (at any point in time). This is indicated by + * setting @result_matches to %FALSE; the caller may reject such subjects or + * require additional privileges. @result_matches == %TRUE only indicates that + * the UID matched the underlying process at ONE point in time, it may not match + * later. + * * Returns: %NULL if @error is set otherwise a #PolkitUnixUser that should be freed with g_object_unref(). */ PolkitIdentity * polkit_backend_session_monitor_get_user_for_subject (PolkitBackendSessionMonitor *monitor, PolkitSubject *subject, + gboolean *result_matches, GError **error) { PolkitIdentity *ret; - guint32 uid; + gboolean matches; ret = NULL; + matches = FALSE; if (POLKIT_IS_UNIX_PROCESS (subject)) { - uid = polkit_unix_process_get_uid (POLKIT_UNIX_PROCESS (subject)); - if ((gint) uid == -1) + gint subject_uid, current_uid; + GError *local_error; + + subject_uid = polkit_unix_process_get_uid (POLKIT_UNIX_PROCESS (subject)); + if (subject_uid == -1) { g_set_error (error, POLKIT_ERROR, @@ -273,11 +288,20 @@ polkit_backend_session_monitor_get_user_ "Unix process subject does not have uid set"); goto out; } - ret = polkit_unix_user_new (uid); + local_error = NULL; + current_uid = polkit_unix_process_get_owner (POLKIT_UNIX_PROCESS (subject), &local_error); + if (local_error != NULL) + { + g_propagate_error (error, local_error); + goto out; + } + ret = polkit_unix_user_new (subject_uid); + matches = (subject_uid == current_uid); } else if (POLKIT_IS_SYSTEM_BUS_NAME (subject)) { GVariant *result; + gint uid; result = g_dbus_connection_call_sync (monitor->system_bus, "org.freedesktop.DBus", @@ -296,9 +320,11 @@ polkit_backend_session_monitor_get_user_ g_variant_unref (result); ret = polkit_unix_user_new (uid); + matches = TRUE; } else if (POLKIT_IS_UNIX_SESSION (subject)) { + uid_t uid; if (sd_session_get_uid (polkit_unix_session_get_session_id (POLKIT_UNIX_SESSION (subject)), &uid) < 0) { @@ -310,9 +336,14 @@ polkit_backend_session_monitor_get_user_ } ret = polkit_unix_user_new (uid); + matches = TRUE; } out: + if (result_matches != NULL) + { + *result_matches = matches; + } return ret; } diff -up ./src/polkit/polkitprivate.h.ori ./src/polkit/polkitprivate.h diff -up ./src/polkit/polkitunixprocess.c.ori ./src/polkit/polkitunixprocess.c --- ./src/polkit/polkitunixprocess.c.ori 2019-10-23 14:05:44.254434983 +0200 +++ ./src/polkit/polkitunixprocess.c 2019-10-23 15:03:01.643635120 +0200 @@ -52,6 +52,15 @@ * To uniquely identify processes, both the process id and the start * time of the process (a monotonic increasing value representing the * time since the kernel was started) is used. + * + * NOTE: This object stores, and provides access to, the real UID of the + * process. That value can change over time (with set*uid*(2) and exec*(2)). + * Checks whether an operation is allowed need to take care to use the UID + * value as of the time when the operation was made (or, following the open() + * privilege check model, when the connection making the operation possible + * was initiated). That is usually done by initializing this with + * polkit_unix_process_new_for_owner() with trusted data. + */ /* See https://gitlab.freedesktop.org/polkit/polkit/issues/75 @@ -148,13 +157,12 @@ enum }; static void subject_iface_init (PolkitSubjectIface *subject_iface); +static gint polkit_unix_process_get_racy_uid__ (PolkitUnixProcess *process, + GError **error); static guint64 get_start_time_for_pid (gint pid, GError **error); -static gint _polkit_unix_process_get_owner (PolkitUnixProcess *process, - GError **error); - #ifdef HAVE_FREEBSD static gboolean get_kinfo_proc (gint pid, struct kinfo_proc *p); #endif @@ -239,7 +247,7 @@ polkit_unix_process_constructed (GObject { GError *error; error = NULL; - process->uid = _polkit_unix_process_get_owner (process, &error); + process->uid = polkit_unix_process_get_racy_uid__ (process, &error); if (error != NULL) { process->uid = -1; @@ -328,6 +336,12 @@ polkit_unix_process_class_init (PolkitUn * Gets the user id for @process. Note that this is the real user-id, * not the effective user-id. * + * NOTE: The UID may change over time, so the returned value may not match the + * current state of the underlying process; or the UID may have been set by + * polkit_unix_process_new_for_owner() or polkit_unix_process_set_uid(), + * in which case it may not correspond to the actual UID of the referenced + * process at all (at any point in time). + * * Returns: The user id for @process or -1 if unknown. */ gint @@ -723,18 +737,27 @@ out: return start_time; } +/* + * Private: Return the "current" UID. Note that this is inherently racy, + * and the value may already be obsolete by the time this function returns; + * this function only guarantees that the UID was valid at some point during + * its execution. + */ static gint -_polkit_unix_process_get_owner (PolkitUnixProcess *process, - GError **error) +polkit_unix_process_get_racy_uid__ (PolkitUnixProcess *process, + GError **error) { gint result; gchar *contents; gchar **lines; + guint64 start_time; + #ifdef HAVE_FREEBSD struct kinfo_proc p; #else gchar filename[64]; guint n; + GError *local_error; #endif g_return_val_if_fail (POLKIT_IS_UNIX_PROCESS (process), 0); @@ -757,6 +780,8 @@ _polkit_unix_process_get_owner (PolkitUn } result = p.ki_uid; + start_time = (guint64) p.ki_start.tv_sec; + #else /* see 'man proc' for layout of the status file @@ -790,17 +815,37 @@ _polkit_unix_process_get_owner (PolkitUn else { result = real_uid; - goto out; + goto found; } } - g_set_error (error, POLKIT_ERROR, POLKIT_ERROR_FAILED, "Didn't find any line starting with `Uid:' in file %s", filename); + goto out; + +found: + /* The UID and start time are, sadly, not available in a single file. So, + * read the UID first, and then the start time; if the start time is the same + * before and after reading the UID, it couldn't have changed. + */ + local_error = NULL; + start_time = get_start_time_for_pid (process->pid, &local_error); + if (local_error != NULL) + { + g_propagate_error (error, local_error); + goto out; + } #endif + if (process->start_time != start_time) + { + g_set_error (error, POLKIT_ERROR, POLKIT_ERROR_FAILED, + "process with PID %d has been replaced", process->pid); + goto out; + } + out: g_strfreev (lines); g_free (contents); @@ -819,5 +864,5 @@ gint polkit_unix_process_get_owner (PolkitUnixProcess *process, GError **error) { - return _polkit_unix_process_get_owner (process, error); + return polkit_unix_process_get_racy_uid__ (process, error); }