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 <glib/gstdio.h>
#include <polkit/polkit.h>
+#include <polkit/polkitunixprocess.h>
#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 <stdlib.h>
#include <polkit/polkit.h>
+#include <polkit/polkitunixprocess.h>
#include "polkitbackendsessionmonitor.h"
/* <internal>
@@ -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);
}