mirror of
https://github.com/postgres/postgres.git
synced 2025-07-09 22:41:56 +03:00
Revert MAINTAIN privilege and pg_maintain predefined role.
This reverts the following commits:4dbdb82513
,c2122aae63
,5b1a879943
,9e1e9d6560
,ff9618e82a
,60684dd834
,4441fc704d
, andb5d6382496
. A role with the MAINTAIN privilege may be able to use search_path tricks to escalate privileges to the table owner. Unfortunately, it is too late in the v16 development cycle to apply the proposed fix, i.e., restricting search_path when running maintenance commands. Bumps catversion. Reviewed-by: Jeff Davis Discussion: https://postgr.es/m/E1q7j7Y-000z1H-Hr%40gemulon.postgresql.org Backpatch-through: 16
This commit is contained in:
@ -159,15 +159,16 @@ analyze_rel(Oid relid, RangeVar *relation,
|
||||
return;
|
||||
|
||||
/*
|
||||
* Check if relation needs to be skipped based on privileges. This check
|
||||
* Check if relation needs to be skipped based on ownership. This check
|
||||
* happens also when building the relation list to analyze for a manual
|
||||
* operation, and needs to be done additionally here as ANALYZE could
|
||||
* happen across multiple transactions where privileges could have changed
|
||||
* in-between. Make sure to generate only logs for ANALYZE in this case.
|
||||
* happen across multiple transactions where relation ownership could have
|
||||
* changed in-between. Make sure to generate only logs for ANALYZE in
|
||||
* this case.
|
||||
*/
|
||||
if (!vacuum_is_permitted_for_relation(RelationGetRelid(onerel),
|
||||
onerel->rd_rel,
|
||||
params->options & ~VACOPT_VACUUM))
|
||||
if (!vacuum_is_relation_owner(RelationGetRelid(onerel),
|
||||
onerel->rd_rel,
|
||||
params->options & VACOPT_ANALYZE))
|
||||
{
|
||||
relation_close(onerel, ShareUpdateExclusiveLock);
|
||||
return;
|
||||
|
@ -80,7 +80,6 @@ static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
|
||||
static List *get_tables_to_cluster(MemoryContext cluster_context);
|
||||
static List *get_tables_to_cluster_partitioned(MemoryContext cluster_context,
|
||||
Oid indexOid);
|
||||
static bool cluster_is_permitted_for_relation(Oid relid, Oid userid);
|
||||
|
||||
|
||||
/*---------------------------------------------------------------------------
|
||||
@ -148,8 +147,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
|
||||
tableOid = RangeVarGetRelidExtended(stmt->relation,
|
||||
AccessExclusiveLock,
|
||||
0,
|
||||
RangeVarCallbackMaintainsTable,
|
||||
NULL);
|
||||
RangeVarCallbackOwnsTable, NULL);
|
||||
rel = table_open(tableOid, NoLock);
|
||||
|
||||
/*
|
||||
@ -366,8 +364,8 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
|
||||
*/
|
||||
if (recheck)
|
||||
{
|
||||
/* Check that the user still has privileges for the relation */
|
||||
if (!cluster_is_permitted_for_relation(tableOid, save_userid))
|
||||
/* Check that the user still owns the relation */
|
||||
if (!object_ownercheck(RelationRelationId, tableOid, save_userid))
|
||||
{
|
||||
relation_close(OldHeap, AccessExclusiveLock);
|
||||
goto out;
|
||||
@ -1642,7 +1640,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
|
||||
|
||||
|
||||
/*
|
||||
* Get a list of tables that the current user has privileges on and
|
||||
* Get a list of tables that the current user owns and
|
||||
* have indisclustered set. Return the list in a List * of RelToCluster
|
||||
* (stored in the specified memory context), each one giving the tableOid
|
||||
* and the indexOid on which the table is already clustered.
|
||||
@ -1659,8 +1657,8 @@ get_tables_to_cluster(MemoryContext cluster_context)
|
||||
List *rtcs = NIL;
|
||||
|
||||
/*
|
||||
* Get all indexes that have indisclustered set and that the current user
|
||||
* has the appropriate privileges for.
|
||||
* Get all indexes that have indisclustered set and are owned by
|
||||
* appropriate user.
|
||||
*/
|
||||
indRelation = table_open(IndexRelationId, AccessShareLock);
|
||||
ScanKeyInit(&entry,
|
||||
@ -1674,7 +1672,7 @@ get_tables_to_cluster(MemoryContext cluster_context)
|
||||
|
||||
index = (Form_pg_index) GETSTRUCT(indexTuple);
|
||||
|
||||
if (!cluster_is_permitted_for_relation(index->indrelid, GetUserId()))
|
||||
if (!object_ownercheck(RelationRelationId, index->indrelid, GetUserId()))
|
||||
continue;
|
||||
|
||||
/* Use a permanent memory context for the result list */
|
||||
@ -1722,13 +1720,10 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
|
||||
if (get_rel_relkind(indexrelid) != RELKIND_INDEX)
|
||||
continue;
|
||||
|
||||
/*
|
||||
* It's possible that the user does not have privileges to CLUSTER the
|
||||
* leaf partition despite having such privileges on the partitioned
|
||||
* table. We skip any partitions which the user is not permitted to
|
||||
* CLUSTER.
|
||||
*/
|
||||
if (!cluster_is_permitted_for_relation(relid, GetUserId()))
|
||||
/* Silently skip partitions which the user has no access to. */
|
||||
if (!object_ownercheck(RelationRelationId, relid, GetUserId()) &&
|
||||
(!object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) ||
|
||||
IsSharedRelation(relid)))
|
||||
continue;
|
||||
|
||||
/* Use a permanent memory context for the result list */
|
||||
@ -1744,19 +1739,3 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
|
||||
|
||||
return rtcs;
|
||||
}
|
||||
|
||||
/*
|
||||
* Return whether userid has privileges to CLUSTER relid. If not, this
|
||||
* function emits a WARNING.
|
||||
*/
|
||||
static bool
|
||||
cluster_is_permitted_for_relation(Oid relid, Oid userid)
|
||||
{
|
||||
if (pg_class_aclcheck(relid, userid, ACL_MAINTAIN) == ACLCHECK_OK)
|
||||
return true;
|
||||
|
||||
ereport(WARNING,
|
||||
(errmsg("permission denied to cluster \"%s\", skipping it",
|
||||
get_rel_name(relid))));
|
||||
return false;
|
||||
}
|
||||
|
@ -26,7 +26,6 @@
|
||||
#include "catalog/index.h"
|
||||
#include "catalog/indexing.h"
|
||||
#include "catalog/pg_am.h"
|
||||
#include "catalog/pg_authid.h"
|
||||
#include "catalog/pg_constraint.h"
|
||||
#include "catalog/pg_database.h"
|
||||
#include "catalog/pg_inherits.h"
|
||||
@ -2829,7 +2828,6 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
|
||||
char relkind;
|
||||
struct ReindexIndexCallbackState *state = arg;
|
||||
LOCKMODE table_lockmode;
|
||||
Oid table_oid;
|
||||
|
||||
/*
|
||||
* Lock level here should match table lock in reindex_index() for
|
||||
@ -2869,19 +2867,14 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
|
||||
errmsg("\"%s\" is not an index", relation->relname)));
|
||||
|
||||
/* Check permissions */
|
||||
table_oid = IndexGetRelation(relId, true);
|
||||
if (OidIsValid(table_oid))
|
||||
{
|
||||
AclResult aclresult;
|
||||
|
||||
aclresult = pg_class_aclcheck(table_oid, GetUserId(), ACL_MAINTAIN);
|
||||
if (aclresult != ACLCHECK_OK)
|
||||
aclcheck_error(aclresult, OBJECT_INDEX, relation->relname);
|
||||
}
|
||||
if (!object_ownercheck(RelationRelationId, relId, GetUserId()))
|
||||
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_INDEX, relation->relname);
|
||||
|
||||
/* Lock heap before index to avoid deadlock. */
|
||||
if (relId != oldRelId)
|
||||
{
|
||||
Oid table_oid = IndexGetRelation(relId, true);
|
||||
|
||||
/*
|
||||
* If the OID isn't valid, it means the index was concurrently
|
||||
* dropped, which is not a problem for us; just return normally.
|
||||
@ -2916,7 +2909,7 @@ ReindexTable(RangeVar *relation, ReindexParams *params, bool isTopLevel)
|
||||
(params->options & REINDEXOPT_CONCURRENTLY) != 0 ?
|
||||
ShareUpdateExclusiveLock : ShareLock,
|
||||
0,
|
||||
RangeVarCallbackMaintainsTable, NULL);
|
||||
RangeVarCallbackOwnsTable, NULL);
|
||||
|
||||
if (get_rel_relkind(heapOid) == RELKIND_PARTITIONED_TABLE)
|
||||
ReindexPartitions(heapOid, params, isTopLevel);
|
||||
@ -2998,8 +2991,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
|
||||
{
|
||||
objectOid = get_namespace_oid(objectName, false);
|
||||
|
||||
if (!object_ownercheck(NamespaceRelationId, objectOid, GetUserId()) &&
|
||||
!has_privs_of_role(GetUserId(), ROLE_PG_MAINTAIN))
|
||||
if (!object_ownercheck(NamespaceRelationId, objectOid, GetUserId()))
|
||||
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SCHEMA,
|
||||
objectName);
|
||||
}
|
||||
@ -3011,8 +3003,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
|
||||
ereport(ERROR,
|
||||
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
|
||||
errmsg("can only reindex the currently open database")));
|
||||
if (!object_ownercheck(DatabaseRelationId, objectOid, GetUserId()) &&
|
||||
!has_privs_of_role(GetUserId(), ROLE_PG_MAINTAIN))
|
||||
if (!object_ownercheck(DatabaseRelationId, objectOid, GetUserId()))
|
||||
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_DATABASE,
|
||||
get_database_name(objectOid));
|
||||
}
|
||||
@ -3084,12 +3075,15 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
|
||||
continue;
|
||||
|
||||
/*
|
||||
* We already checked privileges on the database or schema, but we
|
||||
* further restrict reindexing shared catalogs to roles with the
|
||||
* MAINTAIN privilege on the relation.
|
||||
* The table can be reindexed if the user is superuser, the table
|
||||
* owner, or the database/schema owner (but in the latter case, only
|
||||
* if it's not a shared relation). object_ownercheck includes the
|
||||
* superuser case, and depending on objectKind we already know that
|
||||
* the user has permission to run REINDEX on this database or schema
|
||||
* per the permission checks at the beginning of this routine.
|
||||
*/
|
||||
if (classtuple->relisshared &&
|
||||
pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK)
|
||||
!object_ownercheck(RelationRelationId, relid, GetUserId()))
|
||||
continue;
|
||||
|
||||
/*
|
||||
|
@ -284,7 +284,7 @@ LockTableAclCheck(Oid reloid, LOCKMODE lockmode, Oid userid)
|
||||
AclMode aclmask;
|
||||
|
||||
/* any of these privileges permit any lock mode */
|
||||
aclmask = ACL_MAINTAIN | ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE;
|
||||
aclmask = ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE;
|
||||
|
||||
/* SELECT privileges also permit ACCESS SHARE and below */
|
||||
if (lockmode <= AccessShareLock)
|
||||
|
@ -165,8 +165,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
|
||||
*/
|
||||
matviewOid = RangeVarGetRelidExtended(stmt->relation,
|
||||
lockmode, 0,
|
||||
RangeVarCallbackMaintainsTable,
|
||||
NULL);
|
||||
RangeVarCallbackOwnsTable, NULL);
|
||||
matviewRel = table_open(matviewOid, NoLock);
|
||||
relowner = matviewRel->rd_rel->relowner;
|
||||
|
||||
|
@ -16978,16 +16978,15 @@ AtEOSubXact_on_commit_actions(bool isCommit, SubTransactionId mySubid,
|
||||
* This is intended as a callback for RangeVarGetRelidExtended(). It allows
|
||||
* the relation to be locked only if (1) it's a plain or partitioned table,
|
||||
* materialized view, or TOAST table and (2) the current user is the owner (or
|
||||
* the superuser) or has been granted MAINTAIN. This meets the
|
||||
* permission-checking needs of CLUSTER, REINDEX TABLE, and REFRESH
|
||||
* MATERIALIZED VIEW; we expose it here so that it can be used by all.
|
||||
* the superuser). This meets the permission-checking needs of CLUSTER,
|
||||
* REINDEX TABLE, and REFRESH MATERIALIZED VIEW; we expose it here so that it
|
||||
* can be used by all.
|
||||
*/
|
||||
void
|
||||
RangeVarCallbackMaintainsTable(const RangeVar *relation,
|
||||
Oid relId, Oid oldRelId, void *arg)
|
||||
RangeVarCallbackOwnsTable(const RangeVar *relation,
|
||||
Oid relId, Oid oldRelId, void *arg)
|
||||
{
|
||||
char relkind;
|
||||
AclResult aclresult;
|
||||
|
||||
/* Nothing to do if the relation was not found. */
|
||||
if (!OidIsValid(relId))
|
||||
@ -17008,9 +17007,8 @@ RangeVarCallbackMaintainsTable(const RangeVar *relation,
|
||||
errmsg("\"%s\" is not a table or materialized view", relation->relname)));
|
||||
|
||||
/* Check permissions */
|
||||
aclresult = pg_class_aclcheck(relId, GetUserId(), ACL_MAINTAIN);
|
||||
if (aclresult != ACLCHECK_OK)
|
||||
aclcheck_error(aclresult, OBJECT_TABLE, relation->relname);
|
||||
if (!object_ownercheck(RelationRelationId, relId, GetUserId()))
|
||||
aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relId)), relation->relname);
|
||||
}
|
||||
|
||||
/*
|
||||
|
@ -697,35 +697,32 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,
|
||||
}
|
||||
|
||||
/*
|
||||
* Check if the current user has privileges to vacuum or analyze the relation.
|
||||
* If not, issue a WARNING log message and return false to let the caller
|
||||
* decide what to do with this relation. This routine is used to decide if a
|
||||
* relation can be processed for VACUUM or ANALYZE.
|
||||
* Check if a given relation can be safely vacuumed or analyzed. If the
|
||||
* user is not the relation owner, issue a WARNING log message and return
|
||||
* false to let the caller decide what to do with this relation. This
|
||||
* routine is used to decide if a relation can be processed for VACUUM or
|
||||
* ANALYZE.
|
||||
*/
|
||||
bool
|
||||
vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
|
||||
bits32 options)
|
||||
vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple, bits32 options)
|
||||
{
|
||||
char *relname;
|
||||
|
||||
Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0);
|
||||
|
||||
/*
|
||||
* Privilege checks are bypassed in some cases (e.g., when recursing to a
|
||||
* relation's TOAST table).
|
||||
* Check permissions.
|
||||
*
|
||||
* We allow the user to vacuum or analyze a table if he is superuser, the
|
||||
* table owner, or the database owner (but in the latter case, only if
|
||||
* it's not a shared relation). object_ownercheck includes the superuser
|
||||
* case.
|
||||
*
|
||||
* Note we choose to treat permissions failure as a WARNING and keep
|
||||
* trying to vacuum or analyze the rest of the DB --- is this appropriate?
|
||||
*/
|
||||
if (options & VACOPT_SKIP_PRIVS)
|
||||
return true;
|
||||
|
||||
/*----------
|
||||
* A role has privileges to vacuum or analyze the relation if any of the
|
||||
* following are true:
|
||||
* - the role owns the current database and the relation is not shared
|
||||
* - the role has the MAINTAIN privilege on the relation
|
||||
*----------
|
||||
*/
|
||||
if ((object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) && !reltuple->relisshared) ||
|
||||
pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK)
|
||||
if (object_ownercheck(RelationRelationId, relid, GetUserId()) ||
|
||||
(object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) && !reltuple->relisshared))
|
||||
return true;
|
||||
|
||||
relname = NameStr(reltuple->relname);
|
||||
@ -941,10 +938,10 @@ expand_vacuum_rel(VacuumRelation *vrel, MemoryContext vac_context,
|
||||
classForm = (Form_pg_class) GETSTRUCT(tuple);
|
||||
|
||||
/*
|
||||
* Make a returnable VacuumRelation for this rel if the user has the
|
||||
* required privileges.
|
||||
* Make a returnable VacuumRelation for this rel if user is a proper
|
||||
* owner.
|
||||
*/
|
||||
if (vacuum_is_permitted_for_relation(relid, classForm, options))
|
||||
if (vacuum_is_relation_owner(relid, classForm, options))
|
||||
{
|
||||
oldcontext = MemoryContextSwitchTo(vac_context);
|
||||
vacrels = lappend(vacrels, makeVacuumRelation(vrel->relation,
|
||||
@ -1041,7 +1038,7 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
|
||||
continue;
|
||||
|
||||
/* check permissions of relation */
|
||||
if (!vacuum_is_permitted_for_relation(relid, classForm, options))
|
||||
if (!vacuum_is_relation_owner(relid, classForm, options))
|
||||
continue;
|
||||
|
||||
/*
|
||||
@ -2034,15 +2031,16 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
|
||||
}
|
||||
|
||||
/*
|
||||
* Check if relation needs to be skipped based on privileges. This check
|
||||
* Check if relation needs to be skipped based on ownership. This check
|
||||
* happens also when building the relation list to vacuum for a manual
|
||||
* operation, and needs to be done additionally here as VACUUM could
|
||||
* happen across multiple transactions where privileges could have changed
|
||||
* in-between. Make sure to only generate logs for VACUUM in this case.
|
||||
* happen across multiple transactions where relation ownership could have
|
||||
* changed in-between. Make sure to only generate logs for VACUUM in this
|
||||
* case.
|
||||
*/
|
||||
if (!vacuum_is_permitted_for_relation(RelationGetRelid(rel),
|
||||
rel->rd_rel,
|
||||
params->options & ~VACOPT_ANALYZE))
|
||||
if (!vacuum_is_relation_owner(RelationGetRelid(rel),
|
||||
rel->rd_rel,
|
||||
params->options & VACOPT_VACUUM))
|
||||
{
|
||||
relation_close(rel, lmode);
|
||||
PopActiveSnapshot();
|
||||
@ -2228,14 +2226,9 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
|
||||
{
|
||||
VacuumParams toast_vacuum_params;
|
||||
|
||||
/*
|
||||
* Force VACOPT_PROCESS_MAIN so vacuum_rel() processes it. Likewise,
|
||||
* set VACOPT_SKIP_PRIVS since privileges on the main relation are
|
||||
* sufficient to process it.
|
||||
*/
|
||||
/* force VACOPT_PROCESS_MAIN so vacuum_rel() processes it */
|
||||
memcpy(&toast_vacuum_params, params, sizeof(VacuumParams));
|
||||
toast_vacuum_params.options |= VACOPT_PROCESS_MAIN;
|
||||
toast_vacuum_params.options |= VACOPT_SKIP_PRIVS;
|
||||
|
||||
vacuum_rel(toast_relid, NULL, &toast_vacuum_params, bstrategy);
|
||||
}
|
||||
|
Reference in New Issue
Block a user