From 3a29a8ee0dde7703e2caae3e1cd64981264c4aed Mon Sep 17 00:00:00 2001 From: Jan Rybar Date: Tue, 28 May 2024 21:21:36 +0200 Subject: [PATCH] - session-monitor: watch sessions only - PolkitPermission: react on really changed sessions - allow polkit-pkla-compat to be uninstalled if no .pkla rules - Resolves: RHEL-39063 --- .polkit.metadata | 2 + pkpermission-watch-changed-ssn-only.patch | 391 ++++++++++++++++++++++ polkit.spec | 15 +- session-monitor-watch-sessions-only.patch | 13 + 4 files changed, 419 insertions(+), 2 deletions(-) create mode 100644 .polkit.metadata create mode 100644 pkpermission-watch-changed-ssn-only.patch create mode 100644 session-monitor-watch-sessions-only.patch diff --git a/.polkit.metadata b/.polkit.metadata new file mode 100644 index 0000000..74ce9a8 --- /dev/null +++ b/.polkit.metadata @@ -0,0 +1,2 @@ +0c375fa621bc9f74f2972e00fb517a408f419adf polkit-0.117.tar.gz +d6a24b62d6027c8a68ae5a2743ed3b3881ed15f8 polkit-0.117.tar.gz.sign diff --git a/pkpermission-watch-changed-ssn-only.patch b/pkpermission-watch-changed-ssn-only.patch new file mode 100644 index 0000000..ab0855c --- /dev/null +++ b/pkpermission-watch-changed-ssn-only.patch @@ -0,0 +1,391 @@ +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 + diff --git a/polkit.spec b/polkit.spec index 1fe995f..df34f57 100644 --- a/polkit.spec +++ b/polkit.spec @@ -22,7 +22,7 @@ Summary: An authorization framework Name: polkit Version: 0.117 -Release: 12%{?dist} +Release: 13%{?dist} License: LGPLv2+ URL: http://www.freedesktop.org/wiki/Software/polkit Source0: http://www.freedesktop.org/software/polkit/releases/%{name}-%{version}.tar.gz @@ -34,6 +34,8 @@ Patch1003: CVE-2021-4034.patch Patch1004: CVE-2021-4115.patch Patch1005: tty-restore-flags-if-changed.patch Patch1006: pkttyagent-coredump-after-eof.patch +Patch1007: session-monitor-watch-sessions-only.patch +Patch1008: pkpermission-watch-changed-ssn-only.patch %if 0%{?bundled_mozjs} Source2: https://ftp.mozilla.org/pub/firefox/releases/%{mozjs_version}esr/source/firefox-%{mozjs_version}esr.source.tar.xz @@ -112,7 +114,8 @@ BuildRequires: automake BuildRequires: libtool %endif -Requires: dbus, polkit-pkla-compat +Requires: dbus +Recommends: polkit-pkla-compat Requires: %{name}-libs%{?_isa} = %{version}-%{release} Requires(pre): shadow-utils @@ -183,6 +186,8 @@ Libraries files for polkit. %patch1004 -p1 %patch1005 -p1 %patch1006 -p1 +%patch1007 -p1 +%patch1008 -p1 %if 0%{?bundled_mozjs} # Extract mozjs archive @@ -389,6 +394,12 @@ exit 0 %endif %changelog +* Tue May 28 2024 Jan Rybar - 0.117-13 +- session-monitor: watch sessions only +- PolkitPermission: react on really changed sessions +- allow polkit-pkla-compat to be uninstalled if no .pkla rules +- Resolves: RHEL-39063 + * Mon Mar 18 2024 Jan Rybar - 0.117-12 - pkttyagent: EOF in passwd results in coredump - Resolves: RHEL-5772 diff --git a/session-monitor-watch-sessions-only.patch b/session-monitor-watch-sessions-only.patch new file mode 100644 index 0000000..1000322 --- /dev/null +++ b/session-monitor-watch-sessions-only.patch @@ -0,0 +1,13 @@ +diff --git a/src/polkitbackend/polkitbackendsessionmonitor-systemd.c b/src/polkitbackend/polkitbackendsessionmonitor-systemd.c +index 1a6107a..3abd7c5 100644 +--- a/src/polkitbackend/polkitbackendsessionmonitor-systemd.c ++++ b/src/polkitbackend/polkitbackendsessionmonitor-systemd.c +@@ -106,7 +106,7 @@ sd_source_new (void) + source = g_source_new (&sd_source_funcs, sizeof (SdSource)); + sd_source = (SdSource *)source; + +- if ((ret = sd_login_monitor_new (NULL, &sd_source->monitor)) < 0) ++ if ((ret = sd_login_monitor_new ("session", &sd_source->monitor)) < 0) + { + g_printerr ("Error getting login monitor: %d", ret); + }