From 7db50de3c71c269a31bbee0650705f41b59fb0df Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot Date: Sat, 30 May 2026 14:35:40 +0000 Subject: [PATCH v27 1/3] Recheck permissions after lock acquisition in dependency recording After dependencyLockAndCheckObject() acquires a lock on a referenced object (which may have blocked on a concurrent DROP), re-verify any permission check that was done earlier in the statement. This closes the TOCTOU window where a REVOKE could land between the original permission check and the dependency recording. The approach: record SharedInvalidMessageCounter at the time of the original aclcheck. After acquiring the lock and verifying the object still exists, compare the saved counter with the current value. If it changed (meaning catalog invalidations arrived since the original check), re-verify the permission. Since the OID is fixed (no name re-resolution), a failure is definitive and no retry loop is needed. Each ProcessUtility() call allocates a TrackAclTable in CurrentMemoryContext and saves the previous one in a local variable. A PG_TRY/PG_FINALLY block ensures that the table is freed and the previous pointer restored on both normal return and error. This forms an implicit stack: each nesting level tracks only its own ACL checks, and the outer level's entries remain undisturbed. No dedicated memory context or AbortTransaction cleanup is needed. Recording is gated by checking CurrentTrackAclTable != NULL, so DML and queries pay only a single pointer test. Alternatives considered: - To avoid allocating memory for each statement, keep the array in TopMemoryContext and never free it (only resetting the count). But that left the high-water mark allocated for the lifetime of the backend. - Passing privilege info (roleId, mode) as extra arguments through the dependency recording APIs (recordDependencyOn, recordMultipleDependencies, etc.) was discarded because expression-based dependencies (recordDependencyOnExpr, find_expr_references_walker) discover objects by walking expression trees: the caller never sees individual objects and cannot attach privilege info to them. Author: Bertrand Drouvot Reviewed-by: Jeff Davis Discussion: https://postgr.es/m/ZiYjn0eVc7pxVY45@ip-10-97-1-34.eu-west-3.compute.internal --- src/backend/catalog/aclchk.c | 32 ++++ src/backend/catalog/pg_depend.c | 104 ++++++++++++- src/backend/tcop/utility.c | 41 +++-- src/include/catalog/aclcheck_track.h | 80 ++++++++++ .../expected/ddl-dependency-locking.out | 64 +++++++- .../specs/ddl-dependency-locking.spec | 140 ++++++++++++++++++ src/tools/pgindent/typedefs.list | 2 + 7 files changed, 441 insertions(+), 22 deletions(-) 20.0% src/backend/catalog/ 6.7% src/backend/tcop/ 11.7% src/include/catalog/ 20.5% src/test/isolation/expected/ 40.7% src/test/isolation/specs/ diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index 007ede997c5..ab19db9d789 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -44,6 +44,7 @@ #include "access/sysattr.h" #include "access/tableam.h" #include "access/xact.h" +#include "catalog/aclcheck_track.h" #include "catalog/binary_upgrade.h" #include "catalog/catalog.h" #include "catalog/dependency.h" @@ -84,6 +85,10 @@ #include "utils/rel.h" #include "utils/syscache.h" +#define ACLCHECK_TRACK_INITIAL_SIZE 64 + +TrackAclTable *CurrentTrackAclTable = NULL; + /* * Internal format used by ALTER DEFAULT PRIVILEGES. */ @@ -3891,6 +3896,7 @@ object_aclcheck_ext(Oid classid, Oid objectid, Oid roleid, AclMode mode, bool *is_missing) { + aclcheck_track_record(classid, objectid, roleid, mode); if (object_aclmask_ext(classid, objectid, roleid, mode, ACLMASK_ANY, is_missing) != 0) return ACLCHECK_OK; @@ -4093,6 +4099,7 @@ AclResult pg_class_aclcheck_ext(Oid table_oid, Oid roleid, AclMode mode, bool *is_missing) { + aclcheck_track_record(RelationRelationId, table_oid, roleid, mode); if (pg_class_aclmask_ext(table_oid, roleid, mode, ACLMASK_ANY, is_missing) != 0) return ACLCHECK_OK; @@ -5036,3 +5043,28 @@ RemoveRoleFromInitPriv(Oid roleid, Oid classid, Oid objid, int32 objsubid) table_close(rel, RowExclusiveLock); } + +/* + * Allocate a new tracking table in CurrentMemoryContext. + */ +TrackAclTable * +CreateTrackAclTable(void) +{ + TrackAclTable *acltable = (TrackAclTable *) palloc(sizeof(TrackAclTable)); + + acltable->entries = (AclCheckEntry *) + palloc(ACLCHECK_TRACK_INITIAL_SIZE * sizeof(AclCheckEntry)); + acltable->max = ACLCHECK_TRACK_INITIAL_SIZE; + acltable->count = 0; + return acltable; +} + +/* + * Free a tracking table. + */ +void +FreeTrackAclTable(TrackAclTable *acltable) +{ + pfree(acltable->entries); + pfree(acltable); +} diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c index 9a7a401aced..912c92e8fef 100644 --- a/src/backend/catalog/pg_depend.c +++ b/src/backend/catalog/pg_depend.c @@ -17,6 +17,7 @@ #include "access/genam.h" #include "access/htup_details.h" #include "access/table.h" +#include "catalog/aclcheck_track.h" #include "catalog/catalog.h" #include "catalog/dependency.h" #include "catalog/indexing.h" @@ -29,6 +30,8 @@ #include "miscadmin.h" #include "storage/lmgr.h" #include "storage/lock.h" +#include "storage/sinval.h" +#include "utils/acl.h" #include "utils/fmgroids.h" #include "utils/lsyscache.h" #include "utils/rel.h" @@ -38,6 +41,7 @@ static bool isObjectPinned(const ObjectAddress *object); static void dependencyLockAndCheckObject(Oid classId, Oid objectId); +static void recheckAcl(Oid classId, Oid objectId); /* @@ -732,12 +736,76 @@ isObjectPinned(const ObjectAddress *object) } +/* + * recheckAcl() + * + * Re-verify all tracked permission checks on the given object. + * + * Called after acquiring the lock and verifying the object still exists. + * If permission checks were tracked for this object and catalog + * invalidations arrived since the original checks, re-verify them all. + * This closes the TOCTOU window where a REVOKE could land between the + * original permission check and the dependency recording. + * + * Multiple checks may have been done on the same object with different + * modes or roles, so we iterate through all matching entries. + * + * Since we don't re-resolve names (the OID is fixed), there is no need for + * a retry loop here. If a recheck fails, it's a definitive failure. + * + * The linear scan over the tracking array is O(n) in the number of tracked + * entries, but that's fine since a typical DDL statement tracks only a few + * objects. + */ +static void +recheckAcl(Oid classId, Oid objectId) +{ + TrackAclTable *acltable = CurrentTrackAclTable; + + if (acltable == NULL) + return; + + for (int i = acltable->count - 1; i >= 0; i--) + { + if (acltable->entries[i].classId == classId && + acltable->entries[i].objectId == objectId && + acltable->entries[i].inval_count != SharedInvalidMessageCounter) + { + AclResult aclresult; + + if (classId == RelationRelationId) + aclresult = pg_class_aclcheck(objectId, + acltable->entries[i].roleId, + acltable->entries[i].mode); + else + aclresult = object_aclcheck(classId, objectId, + acltable->entries[i].roleId, + acltable->entries[i].mode); + + if (aclresult != ACLCHECK_OK) + { + ObjectAddress object; + + object.classId = classId; + object.objectId = objectId; + object.objectSubId = 0; + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied for %s", + getObjectDescription(&object, false)))); + } + } + } +} + + /* * dependencyLockAndCheckObject * - * Lock the object that we are about to record a dependency on. After it's - * locked, verify that it hasn't been dropped while we weren't looking. If it - * has been dropped, throw an an error. + * Lock the object that we are about to record a dependency on. After it's + * locked, verify that it hasn't been dropped while we weren't looking. If it + * has been dropped, throw an error. Then re-verify any tracked permission + * check to close the TOCTOU window. * * If the caller already holds a lock that conflicts with DROP * (AccessShareLock or stronger), this does nothing. Callers should acquire @@ -773,7 +841,11 @@ dependencyLockAndCheckObject(Oid classId, Oid objectId) 0); if (LockHeldByMe(&tag, AccessShareLock, true)) + { + /* Recheck ACL */ + recheckAcl(classId, objectId); return; + } /* Assume we should lock the whole object not a sub-object */ LockDatabaseObject(classId, objectId, 0, AccessShareLock); @@ -786,7 +858,11 @@ dependencyLockAndCheckObject(Oid classId, Oid objectId) if (cache != SYSCACHEID_INVALID) { if (SearchSysCacheExists1(cache, ObjectIdGetDatum(objectId))) + { + /* Recheck ACL */ + recheckAcl(classId, objectId); return; + } } /* @@ -814,6 +890,9 @@ dependencyLockAndCheckObject(Oid classId, Oid objectId) systable_endscan(scan); table_close(rel, AccessShareLock); + + /* Recheck ACL */ + recheckAcl(classId, objectId); } else { @@ -834,14 +913,23 @@ dependencyLockAndCheckObject(Oid classId, Oid objectId) Assert(!IsSharedRelation(objectId)); if (CheckRelationOidLockedByMe(objectId, AccessShareLock, true)) + { + /* Recheck ACL */ + recheckAcl(classId, objectId); return; + } + + /* Acquire lock */ LockRelationOid(objectId, AccessShareLock); - if (SearchSysCacheExists1(RELOID, ObjectIdGetDatum(objectId))) - return; - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("referenced relation was concurrently dropped"))); + /* Check that the object still exists */ + if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(objectId))) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("referenced relation was concurrently dropped"))); + + /* Recheck ACL */ + recheckAcl(classId, objectId); } } diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 73a56f1df1d..1fad5d9a78d 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -20,6 +20,7 @@ #include "access/twophase.h" #include "access/xact.h" #include "access/xlog.h" +#include "catalog/aclcheck_track.h" #include "catalog/namespace.h" #include "catalog/pg_authid.h" #include "catalog/pg_inherits.h" @@ -510,24 +511,38 @@ ProcessUtility(PlannedStmt *pstmt, DestReceiver *dest, QueryCompletion *qc) { + TrackAclTable *prevTrackAclTable = CurrentTrackAclTable; + Assert(IsA(pstmt, PlannedStmt)); Assert(pstmt->commandType == CMD_UTILITY); Assert(queryString != NULL); /* required as of 8.4 */ Assert(qc == NULL || qc->commandTag == CMDTAG_UNKNOWN); - /* - * We provide a function hook variable that lets loadable plugins get - * control when ProcessUtility is called. Such a plugin would normally - * call standard_ProcessUtility(). - */ - if (ProcessUtility_hook) - (*ProcessUtility_hook) (pstmt, queryString, readOnlyTree, - context, params, queryEnv, - dest, qc); - else - standard_ProcessUtility(pstmt, queryString, readOnlyTree, - context, params, queryEnv, - dest, qc); + /* Allocate a fresh acl tracking table for this utility statement. */ + CurrentTrackAclTable = CreateTrackAclTable(); + + PG_TRY(); + { + /* + * We provide a function hook variable that lets loadable plugins get + * control when ProcessUtility is called. Such a plugin would + * normally call standard_ProcessUtility(). + */ + if (ProcessUtility_hook) + (*ProcessUtility_hook) (pstmt, queryString, readOnlyTree, + context, params, queryEnv, + dest, qc); + else + standard_ProcessUtility(pstmt, queryString, readOnlyTree, + context, params, queryEnv, + dest, qc); + } + PG_FINALLY(); + { + FreeTrackAclTable(CurrentTrackAclTable); + CurrentTrackAclTable = prevTrackAclTable; + } + PG_END_TRY(); } /* diff --git a/src/include/catalog/aclcheck_track.h b/src/include/catalog/aclcheck_track.h new file mode 100644 index 00000000000..d994cf7889b --- /dev/null +++ b/src/include/catalog/aclcheck_track.h @@ -0,0 +1,80 @@ +/*------------------------------------------------------------------------- + * + * aclcheck_track.h + * Track permission checks for revalidation after lock acquisition + * in dependencyLockAndCheckObject(). + * + * DDL may perform ACL checks on referenced objects without first holding a lock + * on them. In that case, the lock is acquired much later, when recording + * dependencies. + * Track the ACL checks, so that we can re-check them after acquiring the + * lock while recording dependencies. + * + * XXX: consider refactoring so that we perform the name lookup, acquire the + * lock, and check ACLs all in unison, like RangeVarGetRelidExtended(). + * + * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/catalog/aclcheck_track.h + * + *------------------------------------------------------------------------- + */ +#ifndef ACLCHECK_TRACK_H +#define ACLCHECK_TRACK_H + +#include "storage/sinval.h" +#include "utils/acl.h" + +typedef struct AclCheckEntry +{ + Oid classId; + Oid objectId; + Oid roleId; + AclMode mode; + uint64 inval_count; +} AclCheckEntry; + +typedef struct TrackAclTable +{ + AclCheckEntry *entries; + int count; + int max; +} TrackAclTable; + +extern TrackAclTable *CurrentTrackAclTable; + +extern TrackAclTable *CreateTrackAclTable(void); +extern void FreeTrackAclTable(TrackAclTable *acltable); + +/* + * Record an aclcheck for later revalidation. + * + * Called from object_aclcheck_ext() and pg_class_aclcheck_ext(). + * Only records when inside an utility statement. + */ +static inline void +aclcheck_track_record(Oid classId, Oid objectId, Oid roleId, AclMode mode) +{ + TrackAclTable *acltable = CurrentTrackAclTable; + AclCheckEntry *entry; + + if (acltable == NULL) + return; + + if (acltable->count >= acltable->max) + { + acltable->max *= 2; + acltable->entries = (AclCheckEntry *) + repalloc(acltable->entries, acltable->max * sizeof(AclCheckEntry)); + } + + entry = &acltable->entries[acltable->count++]; + entry->classId = classId; + entry->objectId = objectId; + entry->roleId = roleId; + entry->mode = mode; + entry->inval_count = SharedInvalidMessageCounter; +} + +#endif /* ACLCHECK_TRACK_H */ diff --git a/src/test/isolation/expected/ddl-dependency-locking.out b/src/test/isolation/expected/ddl-dependency-locking.out index 636de281022..77cc7489170 100644 --- a/src/test/isolation/expected/ddl-dependency-locking.out +++ b/src/test/isolation/expected/ddl-dependency-locking.out @@ -1,4 +1,4 @@ -Parsed test spec with 2 sessions +Parsed test spec with 3 sessions starting permutation: s1_begin s1_create_function_in_schema s2_drop_schema s1_commit step s1_begin: BEGIN; @@ -135,3 +135,65 @@ step s2_drop_role: DROP ROLE regress_dependency; step s1_commit: COMMIT; step s2_drop_role: <... completed> ERROR: role "regress_dependency" cannot be dropped because some objects depend on it + +starting permutation: s2_begin s2_drop_fdw_wrapper s1_create_server_as_role_user s3_revoke_role s2_rollback +step s2_begin: BEGIN; +step s2_drop_fdw_wrapper: DROP FOREIGN DATA WRAPPER fdw_wrapper RESTRICT; +step s1_create_server_as_role_user: SET ROLE role_user; CREATE SERVER srv_role_revoked FOREIGN DATA WRAPPER fdw_wrapper; RESET ROLE; +step s3_revoke_role: REVOKE role_fdw FROM role_user; +step s2_rollback: ROLLBACK; +step s1_create_server_as_role_user: <... completed> +ERROR: permission denied for foreign-data wrapper fdw_wrapper + +starting permutation: s2_begin s2_drop_fdw_spi_func s1_create_server_spi_func s3_revoke_role s2_rollback +step s2_begin: BEGIN; +step s2_drop_fdw_spi_func: DROP FOREIGN DATA WRAPPER fdw_spi_func RESTRICT; +step s1_create_server_spi_func: SET ROLE role_user; CREATE SERVER srv_spi_func FOREIGN DATA WRAPPER fdw_spi_func; RESET ROLE; +step s3_revoke_role: REVOKE role_fdw FROM role_user; +step s2_rollback: ROLLBACK; +step s1_create_server_spi_func: <... completed> +ERROR: permission denied for foreign-data wrapper fdw_spi_func + +starting permutation: s2_begin s2_drop_fdw_spi_rollback s1_create_server_spi_rollback s3_revoke_role s2_rollback +step s2_begin: BEGIN; +step s2_drop_fdw_spi_rollback: DROP FOREIGN DATA WRAPPER fdw_spi_rollback RESTRICT; +step s1_create_server_spi_rollback: SET ROLE role_user; CREATE SERVER srv_spi_rollback FOREIGN DATA WRAPPER fdw_spi_rollback; RESET ROLE; +step s3_revoke_role: REVOKE role_fdw FROM role_user; +step s2_rollback: ROLLBACK; +step s1_create_server_spi_rollback: <... completed> +ERROR: permission denied for foreign-data wrapper fdw_spi_rollback + +starting permutation: s2_begin s2_drop_fdw_trigger s1_insert_trigger_ddl s3_revoke_role s2_rollback +step s2_begin: BEGIN; +step s2_drop_fdw_trigger: DROP FOREIGN DATA WRAPPER fdw_trigger RESTRICT; +step s1_insert_trigger_ddl: SET ROLE role_user; INSERT INTO trigger_tbl VALUES (1); RESET ROLE; +step s3_revoke_role: REVOKE role_fdw FROM role_user; +step s2_rollback: ROLLBACK; +step s1_insert_trigger_ddl: <... completed> +ERROR: permission denied for foreign-data wrapper fdw_trigger + +starting permutation: s2_begin s2_drop_fdw_trigger_spi s1_insert_trigger_spi_ddl s3_revoke_role s2_rollback +step s2_begin: BEGIN; +step s2_drop_fdw_trigger_spi: DROP FOREIGN DATA WRAPPER fdw_trigger_spi_func RESTRICT; +step s1_insert_trigger_spi_ddl: SET ROLE role_user; INSERT INTO trigger_spi_tbl VALUES (1); RESET ROLE; +step s3_revoke_role: REVOKE role_fdw FROM role_user; +step s2_rollback: ROLLBACK; +step s1_insert_trigger_spi_ddl: <... completed> +ERROR: permission denied for foreign-data wrapper fdw_trigger_spi_func + +starting permutation: s2_begin s2_drop_fdw_inner s1_create_server_nested_toctou s3_revoke_both_roles s2_rollback +step s2_begin: BEGIN; +step s2_drop_fdw_inner: DROP FOREIGN DATA WRAPPER fdw_inner RESTRICT; +step s1_create_server_nested_toctou: SET ROLE role_user; CREATE SERVER srv_outer FOREIGN DATA WRAPPER fdw_outer; RESET ROLE; +step s3_revoke_both_roles: REVOKE role_fdw, role_fdw_inner FROM role_user; +step s2_rollback: ROLLBACK; +step s1_create_server_nested_toctou: <... completed> +ERROR: permission denied for foreign-data wrapper fdw_inner + +starting permutation: s2_begin s2_drop_fdw_inner s1_create_server_nested_rollback s3_revoke_role_inner s2_rollback +step s2_begin: BEGIN; +step s2_drop_fdw_inner: DROP FOREIGN DATA WRAPPER fdw_inner RESTRICT; +step s1_create_server_nested_rollback: SET ROLE role_user; CREATE SERVER srv_outer_rollback FOREIGN DATA WRAPPER fdw_outer_rollback; RESET ROLE; +step s3_revoke_role_inner: REVOKE role_fdw_inner FROM role_user; +step s2_rollback: ROLLBACK; +step s1_create_server_nested_rollback: <... completed> diff --git a/src/test/isolation/specs/ddl-dependency-locking.spec b/src/test/isolation/specs/ddl-dependency-locking.spec index de5bd88d35e..72aa97c6dfd 100644 --- a/src/test/isolation/specs/ddl-dependency-locking.spec +++ b/src/test/isolation/specs/ddl-dependency-locking.spec @@ -11,7 +11,71 @@ setup CREATE FUNCTION f() RETURNS int LANGUAGE SQL RETURN 1; CREATE FUNCTION public.falter() RETURNS int LANGUAGE SQL RETURN 1; CREATE FOREIGN DATA WRAPPER fdw_wrapper; + CREATE ROLE role_fdw; + CREATE ROLE role_user LOGIN; + GRANT USAGE ON FOREIGN DATA WRAPPER fdw_wrapper TO role_fdw; + GRANT role_fdw TO role_user; CREATE ROLE regress_dependency; + CREATE FUNCTION spi_func(text[], oid) RETURNS void + LANGUAGE plpgsql AS $$ BEGIN EXECUTE 'CREATE TEMP TABLE IF NOT EXISTS validator_side_effect(x int)'; END; $$; + CREATE FUNCTION spi_func_rollback(text[], oid) RETURNS void + LANGUAGE plpgsql AS $$ + BEGIN + BEGIN + EXECUTE 'CREATE TEMP TABLE validator_will_rollback(x int)'; + RAISE EXCEPTION 'force rollback'; + EXCEPTION WHEN OTHERS THEN + NULL; + END; + END; $$; + CREATE FOREIGN DATA WRAPPER fdw_spi_func VALIDATOR spi_func; + CREATE FOREIGN DATA WRAPPER fdw_spi_rollback VALIDATOR spi_func_rollback; + GRANT USAGE ON FOREIGN DATA WRAPPER fdw_spi_func TO role_fdw; + GRANT USAGE ON FOREIGN DATA WRAPPER fdw_spi_rollback TO role_fdw; + CREATE FOREIGN DATA WRAPPER fdw_trigger; + GRANT USAGE ON FOREIGN DATA WRAPPER fdw_trigger TO role_fdw; + CREATE FUNCTION trg_create_server() RETURNS trigger + LANGUAGE plpgsql AS $$ BEGIN EXECUTE 'CREATE SERVER srv_from_trigger FOREIGN DATA WRAPPER fdw_trigger'; RETURN NEW; END; $$; + CREATE TABLE trigger_tbl(a int); + GRANT INSERT ON trigger_tbl TO role_user; + CREATE TRIGGER trg BEFORE INSERT ON trigger_tbl + FOR EACH ROW EXECUTE FUNCTION trg_create_server(); + CREATE FOREIGN DATA WRAPPER fdw_trigger_spi_func VALIDATOR spi_func; + GRANT USAGE ON FOREIGN DATA WRAPPER fdw_trigger_spi_func TO role_fdw; + CREATE FUNCTION trg_create_server_spi_func() RETURNS trigger + LANGUAGE plpgsql AS $$ BEGIN EXECUTE 'CREATE SERVER srv_from_trigger_spi FOREIGN DATA WRAPPER fdw_trigger_spi_func'; RETURN NEW; END; $$; + CREATE TABLE trigger_spi_tbl(a int); + GRANT INSERT ON trigger_spi_tbl TO role_user; + CREATE TRIGGER trg BEFORE INSERT ON trigger_spi_tbl + FOR EACH ROW EXECUTE FUNCTION trg_create_server_spi_func(); + CREATE FOREIGN DATA WRAPPER fdw_inner; + GRANT USAGE ON FOREIGN DATA WRAPPER fdw_inner TO role_fdw; + CREATE FUNCTION spi_func_create_server(text[], oid) RETURNS void + LANGUAGE plpgsql AS $$ + BEGIN + IF $2 = 'pg_catalog.pg_foreign_server'::regclass::oid THEN + EXECUTE 'CREATE SERVER srv_nested_inner FOREIGN DATA WRAPPER fdw_inner'; + END IF; + END; $$; + CREATE FOREIGN DATA WRAPPER fdw_outer VALIDATOR spi_func_create_server; + GRANT USAGE ON FOREIGN DATA WRAPPER fdw_outer TO role_fdw; + CREATE ROLE role_fdw_inner; + GRANT USAGE ON FOREIGN DATA WRAPPER fdw_inner TO role_fdw_inner; + GRANT role_fdw_inner TO role_user; + CREATE FUNCTION spi_func_create_server_rollback(text[], oid) RETURNS void + LANGUAGE plpgsql AS $$ + BEGIN + IF $2 = 'pg_catalog.pg_foreign_server'::regclass::oid THEN + BEGIN + EXECUTE 'CREATE SERVER srv_nested_rollback FOREIGN DATA WRAPPER fdw_inner'; + RAISE EXCEPTION 'force rollback'; + EXCEPTION WHEN OTHERS THEN + NULL; + END; + END IF; + END; $$; + CREATE FOREIGN DATA WRAPPER fdw_outer_rollback VALIDATOR spi_func_create_server_rollback; + GRANT USAGE ON FOREIGN DATA WRAPPER fdw_outer_rollback TO role_fdw; } teardown @@ -24,7 +88,19 @@ teardown DROP FUNCTION IF EXISTS alterschema.falter(); DROP DOMAIN IF EXISTS idid; DROP SERVER IF EXISTS srv_fdw_wrapper; + DROP SERVER IF EXISTS srv_role_revoked; + DROP SERVER IF EXISTS srv_spi_func; + DROP SERVER IF EXISTS srv_spi_rollback; + DROP SERVER IF EXISTS srv_from_trigger; + DROP SERVER IF EXISTS srv_from_trigger_spi; + DROP SERVER IF EXISTS srv_from_spi; + DROP SERVER IF EXISTS srv_nested_inner; + DROP SERVER IF EXISTS srv_nested_rollback; + DROP SERVER IF EXISTS srv_outer; + DROP SERVER IF EXISTS srv_outer_rollback; DROP TABLE IF EXISTS tabtype; + DROP TABLE IF EXISTS trigger_tbl; + DROP TABLE IF EXISTS trigger_spi_tbl; DROP SCHEMA IF EXISTS testschema; DROP SCHEMA IF EXISTS alterschema; DROP TYPE IF EXISTS public.foo; @@ -32,6 +108,22 @@ teardown DROP DOMAIN IF EXISTS id; DROP FUNCTION IF EXISTS f(); DROP FOREIGN DATA WRAPPER IF EXISTS fdw_wrapper; + DROP FOREIGN DATA WRAPPER IF EXISTS fdw_spi_func; + DROP FOREIGN DATA WRAPPER IF EXISTS fdw_spi_rollback; + DROP FOREIGN DATA WRAPPER IF EXISTS fdw_trigger; + DROP FOREIGN DATA WRAPPER IF EXISTS fdw_trigger_spi_func; + DROP FOREIGN DATA WRAPPER IF EXISTS fdw_inner; + DROP FOREIGN DATA WRAPPER IF EXISTS fdw_outer; + DROP FOREIGN DATA WRAPPER IF EXISTS fdw_outer_rollback; + DROP FUNCTION IF EXISTS spi_func(text[], oid); + DROP FUNCTION IF EXISTS spi_func_rollback(text[], oid); + DROP FUNCTION IF EXISTS spi_func_create_server(text[], oid); + DROP FUNCTION IF EXISTS spi_func_create_server_rollback(text[], oid); + DROP FUNCTION IF EXISTS trg_create_server(); + DROP FUNCTION IF EXISTS trg_create_server_spi_func(); + DROP ROLE IF EXISTS role_user; + DROP ROLE IF EXISTS role_fdw; + DROP ROLE IF EXISTS role_fdw_inner; DROP ROLE regress_dependency; } @@ -47,6 +139,13 @@ step "s1_alter_function_schema" { ALTER FUNCTION public.falter() SET SCHEMA alte step "s1_create_domain_with_domain" { CREATE DOMAIN idid as id; } step "s1_create_table_with_type" { CREATE TABLE tabtype(a footab); } step "s1_create_server_with_fdw_wrapper" { CREATE SERVER srv_fdw_wrapper FOREIGN DATA WRAPPER fdw_wrapper; } +step "s1_create_server_as_role_user" { SET ROLE role_user; CREATE SERVER srv_role_revoked FOREIGN DATA WRAPPER fdw_wrapper; RESET ROLE; } +step "s1_create_server_spi_func" { SET ROLE role_user; CREATE SERVER srv_spi_func FOREIGN DATA WRAPPER fdw_spi_func; RESET ROLE; } +step "s1_create_server_spi_rollback" { SET ROLE role_user; CREATE SERVER srv_spi_rollback FOREIGN DATA WRAPPER fdw_spi_rollback; RESET ROLE; } +step "s1_insert_trigger_ddl" { SET ROLE role_user; INSERT INTO trigger_tbl VALUES (1); RESET ROLE; } +step "s1_insert_trigger_spi_ddl" { SET ROLE role_user; INSERT INTO trigger_spi_tbl VALUES (1); RESET ROLE; } +step "s1_create_server_nested_toctou" { SET ROLE role_user; CREATE SERVER srv_outer FOREIGN DATA WRAPPER fdw_outer; RESET ROLE; } +step "s1_create_server_nested_rollback" { SET ROLE role_user; CREATE SERVER srv_outer_rollback FOREIGN DATA WRAPPER fdw_outer_rollback; RESET ROLE; } step "s1_commit" { COMMIT; } session "s2" @@ -60,8 +159,20 @@ step "s2_drop_footab_type" { DROP TYPE public.footab; } step "s2_drop_function_f" { DROP FUNCTION f(); } step "s2_drop_domain_id" { DROP DOMAIN id; } step "s2_drop_fdw_wrapper" { DROP FOREIGN DATA WRAPPER fdw_wrapper RESTRICT; } +step "s2_drop_fdw_spi_func" { DROP FOREIGN DATA WRAPPER fdw_spi_func RESTRICT; } +step "s2_drop_fdw_spi_rollback" { DROP FOREIGN DATA WRAPPER fdw_spi_rollback RESTRICT; } +step "s2_drop_fdw_trigger" { DROP FOREIGN DATA WRAPPER fdw_trigger RESTRICT; } +step "s2_drop_fdw_trigger_spi" { DROP FOREIGN DATA WRAPPER fdw_trigger_spi_func RESTRICT; } +step "s2_drop_fdw_inner" { DROP FOREIGN DATA WRAPPER fdw_inner RESTRICT; } step "s2_drop_role" { DROP ROLE regress_dependency; } step "s2_commit" { COMMIT; } +step "s2_rollback" { ROLLBACK; } + +session "s3" + +step "s3_revoke_role" { REVOKE role_fdw FROM role_user; } +step "s3_revoke_role_inner" { REVOKE role_fdw_inner FROM role_user; } +step "s3_revoke_both_roles" { REVOKE role_fdw, role_fdw_inner FROM role_user; } # create function - drop schema permutation "s1_begin" "s1_create_function_in_schema" "s2_drop_schema" "s1_commit" @@ -102,3 +213,32 @@ permutation "s1_begin" "s1_alter_function_owner" "s2_drop_role" "s1_commit" # was concurrently dropped", contains an OID that is not stable. # # permutation "s2_begin" "s2_drop_role" "s1_alter_function_owner" "s2_commit" + +# Role membership TOCTOU: permission via role revoked during lock wait. +permutation "s2_begin" "s2_drop_fdw_wrapper" "s1_create_server_as_role_user" "s3_revoke_role" "s2_rollback" + +# Role membership TOCTOU with SPI DDL in FDW validator. +permutation "s2_begin" "s2_drop_fdw_spi_func" "s1_create_server_spi_func" "s3_revoke_role" "s2_rollback" + +# Same as above but the validator's SPI DDL fails and rolls back its +# subtransaction. +permutation "s2_begin" "s2_drop_fdw_spi_rollback" "s1_create_server_spi_rollback" "s3_revoke_role" "s2_rollback" + +# Role membership TOCTOU with DDL triggered from DML: a trigger function does +# DDL via SPI during INSERT. +permutation "s2_begin" "s2_drop_fdw_trigger" "s1_insert_trigger_ddl" "s3_revoke_role" "s2_rollback" + +# Same as above but the DDL inside the trigger uses an FDW with a SPI validator, +# combining trigger-initiated DDL with nested SPI. +permutation "s2_begin" "s2_drop_fdw_trigger_spi" "s1_insert_trigger_spi_ddl" "s3_revoke_role" "s2_rollback" + +# TOCTOU on the nested SPI DDL itself: the validator creates a server using +# fdw_inner, session 2 blocks that by dropping fdw_inner, and the REVOKE +# removes access to fdw_inner. +permutation "s2_begin" "s2_drop_fdw_inner" "s1_create_server_nested_toctou" "s3_revoke_both_roles" "s2_rollback" + +# Rolled-back nested DDL should not cause the outer to fail: the validator +# tries to create a server using fdw_inner (via role_fdw_inner) but rolls back. +# The REVOKE removes role_fdw_inner, but the outer CREATE SERVER only needs +# role_fdw (for fdw_outer_rollback), so it should succeed. +permutation "s2_begin" "s2_drop_fdw_inner" "s1_create_server_nested_rollback" "s3_revoke_role_inner" "s2_rollback" diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index f9eb23e52c9..637e15b38b3 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -19,6 +19,7 @@ AbsoluteTime AccessMethodInfo AccessPriv Acl +AclCheckEntry AclItem AclMaskHow AclMode @@ -3226,6 +3227,7 @@ ToastedAttribute TocEntry TokenAuxData TokenizedAuthLine +TrackAclTable TrackItem TransApplyAction TransInvalidationInfo -- 2.34.1