From ca8818854c46277aee6d307692e7ca12febe9642 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Tue, 31 May 2022 07:59:05 -0700 Subject: [PATCH v2 1/1] Ensure privilege checks in CREATE INDEX run as calling user. a117ceb, which ensures that the user ID is set to the relation owner before running user code, inadvertently caused the call to ResolveOpClass() in DefineIndex() to run as the relation owner instead of the calling user. Since the relation owner may not have the same lookup privileges as the caller, CREATE INDEX commands that used to succeed might now fail. In particular, pg_dump may generate such failing commands. This change introduces optional arguments to ComputeIndexAttrs() that DefineIndex() will use to indicate the role to use for ResolveOpClass(). --- src/backend/commands/indexcmds.c | 32 +++++++++++++++++++++--- src/test/regress/expected/privileges.out | 10 ++++++++ src/test/regress/sql/privileges.sql | 10 ++++++++ 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index eac13ac0b7..d5a05b1d78 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -80,7 +80,9 @@ static void ComputeIndexAttrs(IndexInfo *indexInfo, Oid relId, const char *accessMethodName, Oid accessMethodId, bool amcanorder, - bool isconstraint); + bool isconstraint, + Oid opclass_lookup_userid, + int opclass_lookup_secctx); static char *ChooseIndexName(const char *tabname, Oid namespaceId, List *colnames, List *exclusionOpNames, bool primary, bool isconstraint); @@ -236,7 +238,7 @@ CheckIndexCompatible(Oid oldId, coloptions, attributeList, exclusionOpNames, relationId, accessMethodName, accessMethodId, - amcanorder, isconstraint); + amcanorder, isconstraint, InvalidOid, 0); /* Get the soon-obsolete pg_index tuple. */ @@ -890,7 +892,8 @@ DefineIndex(Oid relationId, coloptions, allIndexParams, stmt->excludeOpNames, relationId, accessMethodName, accessMethodId, - amcanorder, stmt->isconstraint); + amcanorder, stmt->isconstraint, root_save_userid, + root_save_sec_context); /* * Extra checks when creating a PRIMARY KEY index. @@ -1730,6 +1733,12 @@ CheckPredicate(Expr *predicate) * Compute per-index-column information, including indexed column numbers * or index expressions, opclasses and their options. Note, all output vectors * should be allocated for all columns, including "including" ones. + * + * opclass_lookup_userid and opclass_lookup_secctx indicate the role and + * security context to use when looking up the opclass to use. The caller may + * have switched to the table owner, who may not have privileges to look up the + * specified opclass. If opclass_lookup_userid is InvalidOid, the role and + * security context are not switched when looking up the opclass. */ static void ComputeIndexAttrs(IndexInfo *indexInfo, @@ -1743,12 +1752,16 @@ ComputeIndexAttrs(IndexInfo *indexInfo, const char *accessMethodName, Oid accessMethodId, bool amcanorder, - bool isconstraint) + bool isconstraint, + Oid opclass_lookup_userid, + int opclass_lookup_secctx) { ListCell *nextExclOp; ListCell *lc; int attn; int nkeycols = indexInfo->ii_NumIndexKeyAttrs; + Oid saved_userid; + int saved_secctx; /* Allocate space for exclusion operator info, if needed */ if (exclusionOpNames) @@ -1922,6 +1935,14 @@ ComputeIndexAttrs(IndexInfo *indexInfo, collationOidP[attn] = attcollation; + /* If a user ID is provided, switch to it for opclass lookup. */ + if (OidIsValid(opclass_lookup_userid)) + { + GetUserIdAndSecContext(&saved_userid, &saved_secctx); + SetUserIdAndSecContext(opclass_lookup_userid, + opclass_lookup_secctx); + } + /* * Identify the opclass to use. */ @@ -1930,6 +1951,9 @@ ComputeIndexAttrs(IndexInfo *indexInfo, accessMethodName, accessMethodId); + if (OidIsValid(opclass_lookup_userid)) + SetUserIdAndSecContext(saved_userid, saved_secctx); + /* * Identify the exclusion operator, if any. */ diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 03df567d50..a353fe221e 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -2652,3 +2652,13 @@ SELECT COUNT(*) >= 0 AS ok FROM pg_shmem_allocations; RESET ROLE; -- clean up DROP ROLE regress_readallstats; +-- privilege checks in CREATE INDEX run as calling user +BEGIN; +CREATE ROLE limitedrole; +CREATE SCHEMA a; +CREATE OPERATOR CLASS a.ops FOR TYPE int USING btree AS STORAGE int; +CREATE TABLE x (y INT); +ALTER TABLE x OWNER TO limitedrole; +CREATE INDEX ON x USING btree(y a.ops); -- fails, but not due to permissions +ERROR: missing support function 1 for attribute 1 of index "x_y_idx" +ROLLBACK; diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index 2a6ba38e52..38c94e0846 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -1706,3 +1706,13 @@ RESET ROLE; -- clean up DROP ROLE regress_readallstats; + +-- privilege checks in CREATE INDEX run as calling user +BEGIN; +CREATE ROLE limitedrole; +CREATE SCHEMA a; +CREATE OPERATOR CLASS a.ops FOR TYPE int USING btree AS STORAGE int; +CREATE TABLE x (y INT); +ALTER TABLE x OWNER TO limitedrole; +CREATE INDEX ON x USING btree(y a.ops); -- fails, but not due to permissions +ROLLBACK; -- 2.25.1