From 31ebedebf1d9850a4c699af5cfe57b81e908f642 Mon Sep 17 00:00:00 2001 From: Jan Rybar Date: Thu, 23 May 2024 08:59:11 +0200 Subject: [PATCH 1/3] Only instances affected by sessions change should call for CheckAuthorization (#453) * Only instances affected by sessions change should call for CheckAuthorization Currently, every time the systemd-logind monitor sends a notification about change in sessions, all instances of PolkitPermission (and probably other classes using PolkitAuthority) send CheckAuthorization to the daemon even though their session is not affected. This hogs the cpu needlessly, because ALL programs/applets in ALL instances for ALL users send CheckAuthorization, making each such request even repeated. This PR adds recognition of a change in sessions, adds it to the "Changed" dbus signal as a parameter, and on the client side of polkit (i.e. PolkitAuthority) enables to react accordingly. This enables PolkitPermission to assess whether the session change affects just the objects in affected sessions. --- data/org.freedesktop.PolicyKit1.Authority.xml | 2 +- src/polkit/polkitauthority.c | 32 ++++++- src/polkit/polkitpermission.c | 87 +++++++++++++++++++ src/polkitbackend/polkitbackendauthority.c | 54 +++++++++++- .../polkitbackendinteractiveauthority.c | 2 +- 5 files changed, 170 insertions(+), 7 deletions(-) diff --git a/data/org.freedesktop.PolicyKit1.Authority.xml b/data/org.freedesktop.PolicyKit1.Authority.xml index 453ffc8..214b8c2 100644 --- a/data/org.freedesktop.PolicyKit1.Authority.xml +++ b/data/org.freedesktop.PolicyKit1.Authority.xml @@ -431,7 +431,7 @@ Must match the effective UID of the caller of org.freedesktop.PolicyKit1.Authori - + diff --git a/src/polkit/polkitauthority.c b/src/polkit/polkitauthority.c index 71d527c..08cb511 100644 --- a/src/polkit/polkitauthority.c +++ b/src/polkit/polkitauthority.c @@ -84,6 +84,7 @@ static PolkitAuthority *the_authority = NULL; enum { CHANGED_SIGNAL, + SESSIONS_CHANGED_SIGNAL, LAST_SIGNAL, }; @@ -113,9 +114,23 @@ on_proxy_signal (GDBusProxy *proxy, gpointer user_data) { PolkitAuthority *authority = POLKIT_AUTHORITY (user_data); + guint16 msg_mask; + if (g_strcmp0 (signal_name, "Changed") == 0) { - g_signal_emit_by_name (authority, "changed"); + if ((parameters != NULL) && g_variant_check_format_string(parameters, "(q)", FALSE)) + { + g_variant_get(parameters, "(q)", &msg_mask); + if (msg_mask >= LAST_SIGNAL) + { + msg_mask = CHANGED_SIGNAL; /* If signal not valid, we send generic "changed". */ + } + g_signal_emit (authority, signals[msg_mask], 0); + } + else + { + g_signal_emit_by_name (authority, "changed"); + } } } @@ -287,6 +302,21 @@ polkit_authority_class_init (PolkitAuthorityClass *klass) g_cclosure_marshal_VOID__VOID, G_TYPE_NONE, 0); + /** + * PolkitAuthority::sessions-changed: + * @authority: A #PolkitAuthority. + * + * Emitted when sessions change + */ + signals[SESSIONS_CHANGED_SIGNAL] = g_signal_new ("sessions-changed", + POLKIT_TYPE_AUTHORITY, + G_SIGNAL_RUN_LAST, + 0, /* class offset */ + NULL, /* accumulator */ + NULL, /* accumulator data */ + g_cclosure_marshal_VOID__VOID, + G_TYPE_NONE, + 0); } /* ---------------------------------------------------------------------------------------------------- */ diff --git a/src/polkit/polkitpermission.c b/src/polkit/polkitpermission.c index d4b2459..c53f2cb 100644 --- a/src/polkit/polkitpermission.c +++ b/src/polkit/polkitpermission.c @@ -24,6 +24,10 @@ # include "config.h" #endif +#ifdef HAVE_LIBSYSTEMD +# include +#endif + #include #include @@ -60,6 +64,8 @@ struct _PolkitPermission gchar *action_id; + gchar *session_state; + /* non-NULL exactly when authorized with a temporary authorization */ gchar *tmp_authz_id; }; @@ -74,9 +80,14 @@ enum static void process_result (PolkitPermission *permission, PolkitAuthorizationResult *result); +static char *get_session_state(); + static void on_authority_changed (PolkitAuthority *authority, gpointer user_data); +static void on_sessions_changed (PolkitAuthority *authority, + gpointer user_data); + static gboolean acquire (GPermission *permission, GCancellable *cancellable, GError **error); @@ -126,6 +137,8 @@ polkit_permission_constructed (GObject *object) if (G_OBJECT_CLASS (polkit_permission_parent_class)->constructed != NULL) G_OBJECT_CLASS (polkit_permission_parent_class)->constructed (object); + + permission->session_state = get_session_state(); } static void @@ -135,6 +148,7 @@ polkit_permission_finalize (GObject *object) g_free (permission->action_id); g_free (permission->tmp_authz_id); + g_free (permission->session_state); g_object_unref (permission->subject); if (permission->authority != NULL) @@ -142,6 +156,9 @@ polkit_permission_finalize (GObject *object) g_signal_handlers_disconnect_by_func (permission->authority, on_authority_changed, permission); + g_signal_handlers_disconnect_by_func (permission->authority, + on_sessions_changed, + permission); g_object_unref (permission->authority); } @@ -420,6 +437,11 @@ polkit_permission_initable_init (GInitable *initable, G_CALLBACK (on_authority_changed), permission); + g_signal_connect (permission->authority, + "sessions-changed", + G_CALLBACK (on_sessions_changed), + permission); + result = polkit_authority_check_authorization_sync (permission->authority, permission->subject, permission->action_id, @@ -472,6 +494,37 @@ changed_check_cb (GObject *source_object, g_object_unref (permission); } +static char *get_session_state() +{ +#ifdef HAVE_LIBSYSTEMD + char *session = NULL; + char *state = NULL; + uid_t uid; + + if ( sd_pid_get_session(getpid(), &session) < 0 ) + { + if ( sd_pid_get_owner_uid(getpid(), &uid) < 0) + { + goto out; + } + if (sd_uid_get_display(uid, &session) < 0) + { + goto out; + } + } + + if (session != NULL) + { + sd_session_get_state(session, &state); + } +out: + g_free(session); + return state; +#else + return NULL; +#endif +} + static void on_authority_changed (PolkitAuthority *authority, gpointer user_data) @@ -488,6 +541,40 @@ on_authority_changed (PolkitAuthority *authority, g_object_ref (permission)); } + +static void on_sessions_changed (PolkitAuthority *authority, + gpointer user_data) +{ +#ifdef HAVE_LIBSYSTEMD + char *new_session_state = NULL; + char *last_state = NULL; + + PolkitPermission *permission = POLKIT_PERMISSION (user_data); + + new_session_state = get_session_state(); + + /* if we cannot tell the session state, we should do CheckAuthorization anyway */ + if ((new_session_state == NULL) || ( g_strcmp0(new_session_state, permission->session_state) != 0 )) + { + last_state = permission->session_state; + permission->session_state = new_session_state; + g_free(last_state); + + polkit_authority_check_authorization (permission->authority, + permission->subject, + permission->action_id, + NULL, /* PolkitDetails */ + POLKIT_CHECK_AUTHORIZATION_FLAGS_NONE, + NULL /* cancellable */, + changed_check_cb, + g_object_ref (permission)); + } +#else + on_authority_changed(authority, user_data); /* TODO: resolve the "too many session signals" issue for non-systemd systems later */ +#endif +} + + static void process_result (PolkitPermission *permission, PolkitAuthorizationResult *result) diff --git a/src/polkitbackend/polkitbackendauthority.c b/src/polkitbackend/polkitbackendauthority.c index d4c6f7d..c74216e 100644 --- a/src/polkitbackend/polkitbackendauthority.c +++ b/src/polkitbackend/polkitbackendauthority.c @@ -48,6 +48,7 @@ enum { CHANGED_SIGNAL, + SESSIONS_CHANGED_SIGNAL, LAST_SIGNAL, }; @@ -78,6 +79,15 @@ polkit_backend_authority_class_init (PolkitBackendAuthorityClass *klass) g_cclosure_marshal_VOID__VOID, G_TYPE_NONE, 0); + signals[SESSIONS_CHANGED_SIGNAL] = g_signal_new ("sessions-changed", + POLKIT_BACKEND_TYPE_AUTHORITY, + G_SIGNAL_RUN_LAST, + G_STRUCT_OFFSET (PolkitBackendAuthorityClass, changed), + NULL, /* accumulator */ + NULL, /* accumulator data */ + g_cclosure_marshal_VOID__VOID, + G_TYPE_NONE, + 0); } /** @@ -501,6 +511,8 @@ typedef struct gulong authority_changed_id; + gulong authority_session_monitor_signaller; + gchar *object_path; GHashTable *cancellation_id_to_check_auth_data; @@ -523,6 +535,9 @@ server_free (Server *server) if (server->authority != NULL && server->authority_changed_id > 0) g_signal_handler_disconnect (server->authority, server->authority_changed_id); + if (server->authority != NULL && server->authority_session_monitor_signaller > 0) + g_signal_handler_disconnect (server->authority, server->authority_session_monitor_signaller); + if (server->cancellation_id_to_check_auth_data != NULL) g_hash_table_unref (server->cancellation_id_to_check_auth_data); @@ -531,20 +546,23 @@ server_free (Server *server) g_free (server); } -static void -on_authority_changed (PolkitBackendAuthority *authority, - gpointer user_data) +static void changed_dbus_call_handler(PolkitBackendAuthority *authority, + gpointer user_data, + guint16 msg_mask) { Server *server = user_data; GError *error; + GVariant *parameters; error = NULL; + + parameters = g_variant_new("(q)", msg_mask); if (!g_dbus_connection_emit_signal (server->connection, NULL, /* destination bus name */ server->object_path, "org.freedesktop.PolicyKit1.Authority", "Changed", - NULL, + parameters, &error)) { g_warning ("Error emitting Changed() signal: %s", error->message); @@ -552,6 +570,29 @@ on_authority_changed (PolkitBackendAuthority *authority, } } + +static void +on_authority_changed (PolkitBackendAuthority *authority, + gpointer user_data) +{ + guint16 msg_mask; + + msg_mask = (guint16) CHANGED_SIGNAL; + changed_dbus_call_handler(authority, user_data, msg_mask); +} + + +static void +on_sessions_changed (PolkitBackendAuthority *authority, + gpointer user_data) +{ + guint16 msg_mask; + + msg_mask = (guint16) SESSIONS_CHANGED_SIGNAL; + changed_dbus_call_handler(authority, user_data, msg_mask); +} + + static const gchar *server_introspection_data = "" " " @@ -1397,6 +1438,11 @@ polkit_backend_authority_register (PolkitBackendAuthority *authority, G_CALLBACK (on_authority_changed), server); + server->authority_session_monitor_signaller = g_signal_connect (server->authority, + "sessions-changed", + G_CALLBACK (on_sessions_changed), + server); + return server; error: diff --git a/src/polkitbackend/polkitbackendinteractiveauthority.c b/src/polkitbackend/polkitbackendinteractiveauthority.c index 9dab476..517e715 100644 --- a/src/polkitbackend/polkitbackendinteractiveauthority.c +++ b/src/polkitbackend/polkitbackendinteractiveauthority.c @@ -284,7 +284,7 @@ on_session_monitor_changed (PolkitBackendSessionMonitor *monitor, gpointer user_data) { PolkitBackendInteractiveAuthority *authority = POLKIT_BACKEND_INTERACTIVE_AUTHORITY (user_data); - g_signal_emit_by_name (authority, "changed"); + g_signal_emit_by_name (authority, "sessions-changed"); } static void -- 2.40.1