From 85b64c6d4f599b2988343c4e7121acab505c9006 Mon Sep 17 00:00:00 2001 From: Artur Zakirov Date: Thu, 7 Nov 2024 14:21:14 +0100 Subject: [PATCH] Allow to run pg_repack by non-superuser (#431) The option --no-superuser-check allows to by-pass the check if the user is a superuser. That was done for users which run pg_repack on Amazon, where users cannot run it as a superuser. The problem is that the option --no-superuser-check works only on the CLI level, skipping the check only by the pg_repack client. But there are also checks done by the extension functions exported to SQL. The PR removes the check that the user is a superuser from functions repack_trigger() and repack_apply(). That check is redundant since queries executed by that functions can be executed by a user manually. Moreover repack_trigger() is a SECURITY DEFINER function, which means that it is executed with superuser privileges (pg_repack extension can be created only by superuser). The PR changes privilege check in functions repack_swap(), repack_drop() and repack_index_swap(). Now that functions can be run by an owner of a table. That check is necessary since _swap functions swap relfilenodes on pg_class system catalog table. repack_drop() acquires ACCESS EXCLUSIVE lock and therefore it also requires privilege check. --- META.json | 4 ++-- bin/pgut/pgut.c | 5 ++++- doc/pg_repack.rst | 3 ++- doc/pg_repack_jp.rst | 3 ++- lib/repack.c | 28 ++++++++++++++++------------ regress/expected/nosuper.out | 13 +++++++++++++ regress/expected/nosuper_1.out | 15 +++++++++++++++ regress/sql/nosuper.sql | 9 +++++++++ 8 files changed, 63 insertions(+), 17 deletions(-) diff --git a/META.json b/META.json index 1e272aeb..94aae8a1 100644 --- a/META.json +++ b/META.json @@ -2,7 +2,7 @@ "name": "pg_repack", "abstract": "PostgreSQL module for data reorganization", "description": "Reorganize tables in PostgreSQL databases with minimal locks", - "version": "1.5.1", + "version": "1.5.2", "maintainer": [ "Beena Emerson ", "Josh Kupershmidt ", @@ -17,7 +17,7 @@ "provides": { "pg_repack": { "file": "lib/pg_repack.sql", - "version": "1.5.1", + "version": "1.5.2", "abstract": "Reorganize tables in PostgreSQL databases with minimal locks" } }, diff --git a/bin/pgut/pgut.c b/bin/pgut/pgut.c index db72a978..106d8dda 100644 --- a/bin/pgut/pgut.c +++ b/bin/pgut/pgut.c @@ -1235,9 +1235,12 @@ call_atexit_callbacks(bool fatal) { pgut_atexit_item *item; - for (item = pgut_atexit_stack; item; item = item->next) + item = pgut_atexit_stack; + while (item != NULL) { + pgut_atexit_stack = pgut_atexit_stack->next; item->callback(fatal, item->userdata); + item = pgut_atexit_stack; } } diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index 1a92f5e1..63b9d395 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -24,7 +24,8 @@ You can choose one of the following methods to reorganize: NOTICE: -* Only superusers can use the utility. +* Only superusers or owners of tables and indexes can use the utility. To run + pg_repack as an owner you need to use the option `--no-superuser-check`. * Target table must have a PRIMARY KEY, or at least a UNIQUE total index on a NOT NULL column. diff --git a/doc/pg_repack_jp.rst b/doc/pg_repack_jp.rst index 0a8d81a9..e8695290 100644 --- a/doc/pg_repack_jp.rst +++ b/doc/pg_repack_jp.rst @@ -41,7 +41,8 @@ pg_repackでは再編成する方法として次のものが選択できます .. NOTICE: - * Only superusers can use the utility. + * Only superusers or owners of tables and indexes can use the utility. To run + pg_repack as an owner you need to use the option `--no-superuser-check`. * Target table must have a PRIMARY KEY, or at least a UNIQUE total index on a NOT NULL column. diff --git a/lib/repack.c b/lib/repack.c index 32809b42..a36a0b2c 100644 --- a/lib/repack.c +++ b/lib/repack.c @@ -102,10 +102,20 @@ static void swap_heap_or_index_files(Oid r1, Oid r2); /* check access authority */ static void -must_be_superuser(const char *func) +must_be_owner(Oid relId) { - if (!superuser()) - elog(ERROR, "must be superuser to use %s function", func); +#if PG_VERSION_NUM >= 160000 + if (!object_ownercheck(RelationRelationId, relId, GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relId)), + get_rel_name(relId)); +#elif PG_VERSION_NUM >= 110000 + if (!pg_class_ownercheck(relId, GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relId)), + get_rel_name(relId)); +#else + if (!pg_class_ownercheck(relId, GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, get_rel_name(relId)); +#endif } @@ -163,9 +173,6 @@ repack_trigger(PG_FUNCTION_ARGS) Oid relid; StringInfo sql; - /* authority check */ - must_be_superuser("repack_trigger"); - /* make sure it's called as a trigger at all */ if (!CALLED_AS_TRIGGER(fcinfo) || !TRIGGER_FIRED_AFTER(trigdata->tg_event) || @@ -259,9 +266,6 @@ repack_apply(PG_FUNCTION_ARGS) initStringInfo(&sql_pop); - /* authority check */ - must_be_superuser("repack_apply"); - /* connect to SPI manager */ repack_init(); @@ -859,7 +863,7 @@ repack_swap(PG_FUNCTION_ARGS) Oid owner2; /* authority check */ - must_be_superuser("repack_swap"); + must_be_owner(oid); /* connect to SPI manager */ repack_init(); @@ -1060,7 +1064,7 @@ repack_drop(PG_FUNCTION_ARGS) } /* authority check */ - must_be_superuser("repack_drop"); + must_be_owner(oid); /* connect to SPI manager */ repack_init(); @@ -1413,7 +1417,7 @@ repack_index_swap(PG_FUNCTION_ARGS) HeapTuple tuple; /* authority check */ - must_be_superuser("repack_index_swap"); + must_be_owner(orig_idx_oid); /* connect to SPI manager */ repack_init(); diff --git a/regress/expected/nosuper.out b/regress/expected/nosuper.out index 1d2fad2d..8d0a94ec 100644 --- a/regress/expected/nosuper.out +++ b/regress/expected/nosuper.out @@ -16,4 +16,17 @@ ERROR: pg_repack failed with error: You must be a superuser to use pg_repack ERROR: pg_repack failed with error: ERROR: permission denied for schema repack LINE 1: select repack.version(), repack.version_sql() ^ +GRANT ALL ON ALL TABLES IN SCHEMA repack TO nosuper; +GRANT USAGE ON SCHEMA repack TO nosuper; +-- => ERROR +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check +INFO: repacking table "public.tbl_cluster" +ERROR: query failed: ERROR: current transaction is aborted, commands ignored until end of transaction block +DETAIL: query was: RESET lock_timeout +ERROR: query failed: ERROR: current transaction is aborted, commands ignored until end of transaction block +DETAIL: query was: RESET lock_timeout +ERROR: permission denied for table tbl_cluster +ERROR: permission denied for table tbl_cluster +REVOKE ALL ON ALL TABLES IN SCHEMA repack FROM nosuper; +REVOKE USAGE ON SCHEMA repack FROM nosuper; DROP ROLE IF EXISTS nosuper; diff --git a/regress/expected/nosuper_1.out b/regress/expected/nosuper_1.out index 973ab293..3eda71f2 100644 --- a/regress/expected/nosuper_1.out +++ b/regress/expected/nosuper_1.out @@ -14,4 +14,19 @@ ERROR: pg_repack failed with error: You must be a superuser to use pg_repack -- => ERROR \! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check ERROR: pg_repack failed with error: ERROR: permission denied for schema repack +LINE 1: select repack.version(), repack.version_sql() + ^ +GRANT ALL ON ALL TABLES IN SCHEMA repack TO nosuper; +GRANT USAGE ON SCHEMA repack TO nosuper; +-- => ERROR +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check +INFO: repacking table "public.tbl_cluster" +ERROR: query failed: ERROR: current transaction is aborted, commands ignored until end of transaction block +DETAIL: query was: RESET lock_timeout +ERROR: query failed: ERROR: current transaction is aborted, commands ignored until end of transaction block +DETAIL: query was: RESET lock_timeout +ERROR: permission denied for relation tbl_cluster +ERROR: permission denied for relation tbl_cluster +REVOKE ALL ON ALL TABLES IN SCHEMA repack FROM nosuper; +REVOKE USAGE ON SCHEMA repack FROM nosuper; DROP ROLE IF EXISTS nosuper; diff --git a/regress/sql/nosuper.sql b/regress/sql/nosuper.sql index 198f3458..072f0fac 100644 --- a/regress/sql/nosuper.sql +++ b/regress/sql/nosuper.sql @@ -11,4 +11,13 @@ CREATE ROLE nosuper WITH LOGIN; \! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper -- => ERROR \! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check + +GRANT ALL ON ALL TABLES IN SCHEMA repack TO nosuper; +GRANT USAGE ON SCHEMA repack TO nosuper; + +-- => ERROR +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check + +REVOKE ALL ON ALL TABLES IN SCHEMA repack FROM nosuper; +REVOKE USAGE ON SCHEMA repack FROM nosuper; DROP ROLE IF EXISTS nosuper;