I adjusted 0001 based on my upthread feedback. -- nathan
>From 118f95346fcf8099ab28d2f9186185171e3b88af Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Wed, 12 Jun 2024 15:38:14 -0500 Subject: [PATCH v5 1/1] Introduce pg_signal_autovacuum_worker.
Since commit 3a9b18b309, roles with privileges of pg_signal_backend cannot signal autovacuum workers. Many users treated the ability to signal autovacuum workers as a feature instead of a bug, so we are reintroducing it via a new predefined role. Having privileges of this new role, named pg_signal_autovacuum_worker, only permits signaling autovacuum workers. It does not permit signaling other types of superuser backends. Author: Kirill Reshke Reviewed-by: Anthony Leung, Michael Paquier Discussion: https://postgr.es/m/CALdSSPhC4GGmbnugHfB9G0%3DfAxjCSug_-rmL9oUh0LTxsyBfsg%40mail.gmail.com --- doc/src/sgml/func.sgml | 8 +++-- doc/src/sgml/user-manag.sgml | 5 +++ src/backend/storage/ipc/signalfuncs.c | 46 +++++++++++++++++++++------ src/include/catalog/pg_authid.dat | 5 +++ 4 files changed, 53 insertions(+), 11 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 17c44bc338..7d5b7acb68 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -28037,7 +28037,9 @@ SELECT currval(pg_get_serial_sequence('sometable', 'id')); <para> Cancels the current query of the session whose backend process has the specified process ID. This is also allowed if the - calling role is a member of the role whose backend is being canceled or + calling role has privileges of the role whose backend is being canceled, + the backend process is an autovacuum worker and the calling role has + privileges of <literal>pg_signal_autovacuum_worker</literal>, or the calling role has privileges of <literal>pg_signal_backend</literal>, however only superusers can cancel superuser backends. </para></entry> @@ -28111,7 +28113,9 @@ SELECT currval(pg_get_serial_sequence('sometable', 'id')); <para> Terminates the session whose backend process has the specified process ID. This is also allowed if the calling role - is a member of the role whose backend is being terminated or the + has privileges of the role whose backend is being terminated, + the backend process is an autovacuum worker and the calling role has + privileges of <literal>pg_signal_autovacuum_worker</literal>, or the calling role has privileges of <literal>pg_signal_backend</literal>, however only superusers can terminate superuser backends. </para> diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml index 07a16247d7..340cefff70 100644 --- a/doc/src/sgml/user-manag.sgml +++ b/doc/src/sgml/user-manag.sgml @@ -661,6 +661,11 @@ DROP ROLE doomed_role; <entry>pg_signal_backend</entry> <entry>Signal another backend to cancel a query or terminate its session.</entry> </row> + <row> + <entry>pg_signal_autovacuum_worker</entry> + <entry>Signal an autovacuum worker to cancel the current table's vacuum + or terminate its session.</entry> + </row> <row> <entry>pg_read_server_files</entry> <entry>Allow reading files from any location the database can access on the server with COPY and diff --git a/src/backend/storage/ipc/signalfuncs.c b/src/backend/storage/ipc/signalfuncs.c index 88e9bf8125..780a1c682a 100644 --- a/src/backend/storage/ipc/signalfuncs.c +++ b/src/backend/storage/ipc/signalfuncs.c @@ -34,8 +34,9 @@ * role as the backend being signaled. For "dangerous" signals, an explicit * check for superuser needs to be done prior to calling this function. * - * Returns 0 on success, 1 on general failure, 2 on normal permission error - * and 3 if the caller needs to be a superuser. + * Returns 0 on success, 1 on general failure, 2 on normal permission error, + * 3 if the caller needs to be a superuser, and 4 if the caller needs to have + * privileges of pg_signal_autovacuum_worker. * * In the event of a general failure (return code 1), a warning message will * be emitted. For permission errors, doing that is the responsibility of @@ -45,6 +46,7 @@ #define SIGNAL_BACKEND_ERROR 1 #define SIGNAL_BACKEND_NOPERMISSION 2 #define SIGNAL_BACKEND_NOSUPERUSER 3 +#define SIGNAL_BACKEND_NOAUTOVAC 4 static int pg_signal_backend(int pid, int sig) { @@ -77,15 +79,27 @@ pg_signal_backend(int pid, int sig) /* * Only allow superusers to signal superuser-owned backends. Any process * not advertising a role might have the importance of a superuser-owned - * backend, so treat it that way. + * backend, so treat it that way. As an exception, we allow roles with + * privileges of pg_signal_autovacuum_worker to signal autovacuum workers + * (which do not advertise a role). + * + * Otherwise, users can signal backends for roles they have privileges of. */ - if ((!OidIsValid(proc->roleId) || superuser_arg(proc->roleId)) && - !superuser()) - return SIGNAL_BACKEND_NOSUPERUSER; + if (!OidIsValid(proc->roleId) || superuser_arg(proc->roleId)) + { + ProcNumber procNumber = GetNumberFromPGProc(proc); + PgBackendStatus *procStatus = pgstat_get_beentry_by_proc_number(procNumber); - /* Users can signal backends they have role membership in. */ - if (!has_privs_of_role(GetUserId(), proc->roleId) && - !has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_BACKEND)) + if (procStatus && procStatus->st_backendType == B_AUTOVAC_WORKER) + { + if (!has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_AUTOVACUUM_WORKER)) + return SIGNAL_BACKEND_NOAUTOVAC; + } + else if (!superuser()) + return SIGNAL_BACKEND_NOSUPERUSER; + } + else if (!has_privs_of_role(GetUserId(), proc->roleId) && + !has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_BACKEND)) return SIGNAL_BACKEND_NOPERMISSION; /* @@ -130,6 +144,13 @@ pg_cancel_backend(PG_FUNCTION_ARGS) errdetail("Only roles with the %s attribute may cancel queries of roles with the %s attribute.", "SUPERUSER", "SUPERUSER"))); + if (r == SIGNAL_BACKEND_NOAUTOVAC) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied to cancel query"), + errdetail("Only roles with privileges of the \"%s\" role may cancel queries of autovacuum workers.", + "pg_signal_autovacuum_worker"))); + if (r == SIGNAL_BACKEND_NOPERMISSION) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), @@ -236,6 +257,13 @@ pg_terminate_backend(PG_FUNCTION_ARGS) errdetail("Only roles with the %s attribute may terminate processes of roles with the %s attribute.", "SUPERUSER", "SUPERUSER"))); + if (r == SIGNAL_BACKEND_NOAUTOVAC) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied to terminate process"), + errdetail("Only roles with privileges of the \"%s\" role may terminate autovacuum worker processes.", + "pg_signal_autovacuum_worker"))); + if (r == SIGNAL_BACKEND_NOPERMISSION) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat index bf00815c14..0129f67eaa 100644 --- a/src/include/catalog/pg_authid.dat +++ b/src/include/catalog/pg_authid.dat @@ -99,5 +99,10 @@ rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', rolpassword => '_null_', rolvaliduntil => '_null_' }, +{ oid => '8916', oid_symbol => 'ROLE_PG_SIGNAL_AUTOVACUUM_WORKER', + rolname => 'pg_signal_autovacuum_worker', rolsuper => 'f', rolinherit => 't', + rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', + rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', + rolpassword => '_null_', rolvaliduntil => '_null_' }, ] -- 2.39.3 (Apple Git-146)
