From d096d391630186fa89a937f1bd209d35754181c2 Mon Sep 17 00:00:00 2001 From: reshke kirill Date: Mon, 26 Feb 2024 20:10:21 +0000 Subject: [PATCH v1] Allow non-superuser to cancel superuser tasks. Patch adds new pre-defined role `pg_signal_autovacuum` and tap tests. Role, granted with `pg_signal_autovacuum` is able to cancel running autovacuum worker. Note that to cancel all other type of queries role need to be granted with `pg_singal_backend`. --- src/backend/storage/ipc/signalfuncs.c | 28 ++++++--- src/include/catalog/pg_authid.dat | 5 ++ src/test/signals/.gitignore | 2 + src/test/signals/Makefile | 23 ++++++++ src/test/signals/t/001_signal_autovacuum.pl | 63 +++++++++++++++++++++ 5 files changed, 113 insertions(+), 8 deletions(-) create mode 100644 src/test/signals/.gitignore create mode 100644 src/test/signals/Makefile create mode 100644 src/test/signals/t/001_signal_autovacuum.pl diff --git a/src/backend/storage/ipc/signalfuncs.c b/src/backend/storage/ipc/signalfuncs.c index 81d1a59659..c006f4101e 100644 --- a/src/backend/storage/ipc/signalfuncs.c +++ b/src/backend/storage/ipc/signalfuncs.c @@ -78,15 +78,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. + * In later case, check if pid is actually autovacuum_worker, and allow + * to signal if role has proper priviledge. */ - if ((!OidIsValid(proc->roleId) || superuser_arg(proc->roleId)) && - !superuser()) - return SIGNAL_BACKEND_NOSUPERUSER; - - /* 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)) - return SIGNAL_BACKEND_NOPERMISSION; + if (!superuser()) { + if (!OidIsValid(proc->roleId)) { + LocalPgBackendStatus *local_beentry; + local_beentry = pgstat_get_local_beentry_by_backend_id(proc->backendId); + + if (!(local_beentry && local_beentry->backendStatus.st_backendType == B_AUTOVAC_WORKER && + has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_AUTOVACUUM))) + return SIGNAL_BACKEND_NOSUPERUSER; + } else { + if (superuser_arg(proc->roleId)) + return SIGNAL_BACKEND_NOSUPERUSER; + + /* 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)) + return SIGNAL_BACKEND_NOPERMISSION; + } + } /* * Can the process we just validated above end, followed by the pid being diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat index 82a2ec2862..aa0d9845ef 100644 --- a/src/include/catalog/pg_authid.dat +++ b/src/include/catalog/pg_authid.dat @@ -94,5 +94,10 @@ rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', rolpassword => '_null_', rolvaliduntil => '_null_' }, +{ oid => '6312', oid_symbol => 'ROLE_PG_SIGNAL_AUTOVACUUM', + rolname => 'pg_signal_autovacuum', rolsuper => 'f', rolinherit => 't', + rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', + rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', + rolpassword => '_null_', rolvaliduntil => '_null_' }, ] diff --git a/src/test/signals/.gitignore b/src/test/signals/.gitignore new file mode 100644 index 0000000000..871e943d50 --- /dev/null +++ b/src/test/signals/.gitignore @@ -0,0 +1,2 @@ +# Generated by test suite +/tmp_check/ diff --git a/src/test/signals/Makefile b/src/test/signals/Makefile new file mode 100644 index 0000000000..688f6a7e9e --- /dev/null +++ b/src/test/signals/Makefile @@ -0,0 +1,23 @@ +#------------------------------------------------------------------------- +# +# Makefile for src/test/recovery +# +# Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group +# Portions Copyright (c) 1994, Regents of the University of California +# +# src/test/recovery/Makefile +# +#------------------------------------------------------------------------- + +subdir = src/test/signals +top_builddir = ../../.. +include $(top_builddir)/src/Makefile.global + +check: + $(prove_check) + +installcheck: + $(prove_installcheck) + +clean distclean maintainer-clean: + rm -rf tmp_check diff --git a/src/test/signals/t/001_signal_autovacuum.pl b/src/test/signals/t/001_signal_autovacuum.pl new file mode 100644 index 0000000000..ec4b3e78de --- /dev/null +++ b/src/test/signals/t/001_signal_autovacuum.pl @@ -0,0 +1,63 @@ +# Copyright (c) 2024-2024, PostgreSQL Global Development Group + +# Minimal test testing pg_signal_autovacuum role. +use strict; +use warnings; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# Initialize primary node +my $node_primary = PostgreSQL::Test::Cluster->new('primary'); +$node_primary->init(); +$node_primary->start; + +$node_primary->safe_psql('postgres', + " + CREATE DATABASE regress; + CREATE ROLE psa_reg_role_1; + CREATE ROLE psa_reg_role_2; + GRANT pg_signal_backend TO psa_reg_role_1; + GRANT pg_signal_autovacuum TO psa_reg_role_2; +"); + +# Create some content on primary and set autovacuum setting such that +# it would be triggered. +$node_primary->safe_psql('regress', + " + CREATE TABLE tab_int(i int); + INSERT INTO tab_int SELECT * FROM generate_series(1, 1000000); + ALTER SYSTEM SET autovacuum_vacuum_cost_limit TO 1; + ALTER SYSTEM SET autovacuum_vacuum_cost_delay TO 100; + ALTER SYSTEM SET autovacuum_naptime TO 1; +"); + +$node_primary->restart; + +#wait for autovac to start. + +sleep 1; + +my $res_pid = $node_primary->safe_psql('regress', + " + SELECT pid FROM pg_stat_activity WHERE backend_type = 'autovacuum worker' and datname = 'regress';; +"); + + +my ($res_reg_psa_1, $stdout_reg_psa_1, $stderr_reg_psa_1) = $node_primary->psql('regress', + " + SET ROLE psa_reg_role_1; + SELECT pg_terminate_backend($res_pid); +"); + +ok($res_reg_psa_1 != 0, "should fail for non pg_signal_autovacuum"); +like($stderr_reg_psa_1, qr/Only roles with the SUPERUSER attribute may terminate processes of roles with the SUPERUSER attribute./, "matches"); + +my ($res_reg_psa_2, $stdout_reg_psa_2, $stderr_reg_psa_2) = $node_primary->psql('regress', " + SET ROLE psa_reg_role_2; + SELECT pg_terminate_backend($res_pid); +"); + +ok($res_reg_psa_2 == 0, "should success for pg_signal_autovacuum"); + +done_testing(); -- 2.34.1