Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit93731d5

Browse files
committed
CREATE INDEX: use the original userid for more ACL checks.
Commita117ceb used the original useridfor ACL checks located directly in DefineIndex(), but it still adoptedthe table owner userid for more ACL checks than intended. That brokedump/reload of indexes that refer to an operator class, collation, orexclusion operator in a schema other than "public" or "pg_catalog".Back-patch to v10 (all supported versions), like the earlier commit.Nathan Bossart and Noah MischDiscussion:https://postgr.es/m/f8a4105f076544c180a87ef0c4822352@stmuk.bayern.de
1 parentec26f44 commit93731d5

File tree

4 files changed

+239
-16
lines changed

4 files changed

+239
-16
lines changed

‎contrib/citext/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ DATA = citext--1.4.sql \
1111
citext--1.0--1.1.sql citext--unpackaged--1.0.sql
1212
PGFILEDESC = "citext - case-insensitive character string data type"
1313

14-
REGRESS = citext
14+
REGRESS =create_index_aclcitext
1515

1616
ifdefUSE_PGXS
1717
PG_CONFIG = pg_config
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
-- Each DefineIndex() ACL check uses either the original userid or the table
2+
-- owner userid; see its header comment. Here, confirm that DefineIndex()
3+
-- uses its original userid where necessary. The test works by creating
4+
-- indexes that refer to as many sorts of objects as possible, with the table
5+
-- owner having as few applicable privileges as possible. (The privileges.sql
6+
-- regress_sro_user tests look for the opposite defect; they confirm that
7+
-- DefineIndex() uses the table owner userid where necessary.)
8+
-- Don't override tablespaces; this version lacks allow_in_place_tablespaces.
9+
BEGIN;
10+
CREATE ROLE regress_minimal;
11+
CREATE SCHEMA s;
12+
CREATE EXTENSION citext SCHEMA s;
13+
-- Revoke all conceivably-relevant ACLs within the extension. The system
14+
-- doesn't check all these ACLs, but this will provide some coverage if that
15+
-- ever changes.
16+
REVOKE ALL ON TYPE s.citext FROM PUBLIC;
17+
REVOKE ALL ON FUNCTION s.citext_pattern_lt FROM PUBLIC;
18+
REVOKE ALL ON FUNCTION s.citext_pattern_le FROM PUBLIC;
19+
REVOKE ALL ON FUNCTION s.citext_eq FROM PUBLIC;
20+
REVOKE ALL ON FUNCTION s.citext_pattern_ge FROM PUBLIC;
21+
REVOKE ALL ON FUNCTION s.citext_pattern_gt FROM PUBLIC;
22+
REVOKE ALL ON FUNCTION s.citext_pattern_cmp FROM PUBLIC;
23+
-- Functions sufficient for making an index column that has the side effect of
24+
-- changing search_path at expression planning time.
25+
CREATE FUNCTION public.setter() RETURNS bool VOLATILE
26+
LANGUAGE SQL AS $$SET search_path = s; SELECT true$$;
27+
CREATE FUNCTION s.const() RETURNS bool IMMUTABLE
28+
LANGUAGE SQL AS $$SELECT public.setter()$$;
29+
CREATE FUNCTION s.index_this_expr(s.citext, bool) RETURNS s.citext IMMUTABLE
30+
LANGUAGE SQL AS $$SELECT $1$$;
31+
REVOKE ALL ON FUNCTION public.setter FROM PUBLIC;
32+
REVOKE ALL ON FUNCTION s.const FROM PUBLIC;
33+
REVOKE ALL ON FUNCTION s.index_this_expr FROM PUBLIC;
34+
-- Even for an empty table, expression planning calls s.const & public.setter.
35+
GRANT EXECUTE ON FUNCTION public.setter TO regress_minimal;
36+
GRANT EXECUTE ON FUNCTION s.const TO regress_minimal;
37+
-- Function for index predicate.
38+
CREATE FUNCTION s.index_row_if(s.citext) RETURNS bool IMMUTABLE
39+
LANGUAGE SQL AS $$SELECT $1 IS NOT NULL$$;
40+
REVOKE ALL ON FUNCTION s.index_row_if FROM PUBLIC;
41+
-- Even for an empty table, CREATE INDEX checks ii_Predicate permissions.
42+
GRANT EXECUTE ON FUNCTION s.index_row_if TO regress_minimal;
43+
-- Non-extension, non-function objects.
44+
CREATE COLLATION s.coll (LOCALE="C");
45+
CREATE TABLE s.x (y s.citext);
46+
ALTER TABLE s.x OWNER TO regress_minimal;
47+
-- Empty-table DefineIndex()
48+
CREATE UNIQUE INDEX u0rows ON s.x USING btree
49+
((s.index_this_expr(y, s.const())) COLLATE s.coll s.citext_pattern_ops)
50+
WHERE s.index_row_if(y);
51+
ALTER TABLE s.x ADD CONSTRAINT e0rows EXCLUDE USING btree
52+
((s.index_this_expr(y, s.const())) COLLATE s.coll WITH s.=)
53+
WHERE (s.index_row_if(y));
54+
-- Make the table nonempty.
55+
INSERT INTO s.x VALUES ('foo'), ('bar');
56+
-- If the INSERT runs the planner on index expressions, a search_path change
57+
-- survives. As of 2022-06, the INSERT reuses a cached plan. It does so even
58+
-- under debug_discard_caches, since each index is new-in-transaction. If
59+
-- future work changes a cache lifecycle, this RESET may become necessary.
60+
RESET search_path;
61+
-- For a nonempty table, owner needs permissions throughout ii_Expressions.
62+
GRANT EXECUTE ON FUNCTION s.index_this_expr TO regress_minimal;
63+
CREATE UNIQUE INDEX u2rows ON s.x USING btree
64+
((s.index_this_expr(y, s.const())) COLLATE s.coll s.citext_pattern_ops)
65+
WHERE s.index_row_if(y);
66+
ALTER TABLE s.x ADD CONSTRAINT e2rows EXCLUDE USING btree
67+
((s.index_this_expr(y, s.const())) COLLATE s.coll WITH s.=)
68+
WHERE (s.index_row_if(y));
69+
-- Shall not find s.coll via search_path, despite the s.const->public.setter
70+
-- call having set search_path=s during expression planning. Suppress the
71+
-- message itself, which depends on the database encoding.
72+
\set VERBOSITY sqlstate
73+
ALTER TABLE s.x ADD CONSTRAINT underqualified EXCLUDE USING btree
74+
((s.index_this_expr(y, s.const())) COLLATE coll WITH s.=)
75+
WHERE (s.index_row_if(y));
76+
ERROR: 42704
77+
\set VERBOSITY default
78+
ROLLBACK;
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
-- Each DefineIndex() ACL check uses either the original userid or the table
2+
-- owner userid; see its header comment. Here, confirm that DefineIndex()
3+
-- uses its original userid where necessary. The test works by creating
4+
-- indexes that refer to as many sorts of objects as possible, with the table
5+
-- owner having as few applicable privileges as possible. (The privileges.sql
6+
-- regress_sro_user tests look for the opposite defect; they confirm that
7+
-- DefineIndex() uses the table owner userid where necessary.)
8+
9+
-- Don't override tablespaces; this version lacks allow_in_place_tablespaces.
10+
11+
BEGIN;
12+
CREATE ROLE regress_minimal;
13+
CREATESCHEMAs;
14+
CREATE EXTENSION citext SCHEMA s;
15+
-- Revoke all conceivably-relevant ACLs within the extension. The system
16+
-- doesn't check all these ACLs, but this will provide some coverage if that
17+
-- ever changes.
18+
REVOKE ALLON TYPEs.citextFROM PUBLIC;
19+
REVOKE ALLON FUNCTIONs.citext_pattern_ltFROM PUBLIC;
20+
REVOKE ALLON FUNCTIONs.citext_pattern_leFROM PUBLIC;
21+
REVOKE ALLON FUNCTIONs.citext_eqFROM PUBLIC;
22+
REVOKE ALLON FUNCTIONs.citext_pattern_geFROM PUBLIC;
23+
REVOKE ALLON FUNCTIONs.citext_pattern_gtFROM PUBLIC;
24+
REVOKE ALLON FUNCTIONs.citext_pattern_cmpFROM PUBLIC;
25+
-- Functions sufficient for making an index column that has the side effect of
26+
-- changing search_path at expression planning time.
27+
CREATEFUNCTIONpublic.setter() RETURNS bool VOLATILE
28+
LANGUAGE SQLAS $$SET search_path= s;SELECT true$$;
29+
CREATEFUNCTIONs.const() RETURNS bool IMMUTABLE
30+
LANGUAGE SQLAS $$SELECTpublic.setter()$$;
31+
CREATEFUNCTIONs.index_this_expr(s.citext, bool) RETURNSs.citext IMMUTABLE
32+
LANGUAGE SQLAS $$SELECT $1$$;
33+
REVOKE ALLON FUNCTIONpublic.setterFROM PUBLIC;
34+
REVOKE ALLON FUNCTIONs.constFROM PUBLIC;
35+
REVOKE ALLON FUNCTIONs.index_this_exprFROM PUBLIC;
36+
-- Even for an empty table, expression planning calls s.const & public.setter.
37+
GRANT EXECUTEON FUNCTIONpublic.setter TO regress_minimal;
38+
GRANT EXECUTEON FUNCTIONs.const TO regress_minimal;
39+
-- Function for index predicate.
40+
CREATEFUNCTIONs.index_row_if(s.citext) RETURNS bool IMMUTABLE
41+
LANGUAGE SQLAS $$SELECT $1IS NOT NULL$$;
42+
REVOKE ALLON FUNCTIONs.index_row_ifFROM PUBLIC;
43+
-- Even for an empty table, CREATE INDEX checks ii_Predicate permissions.
44+
GRANT EXECUTEON FUNCTIONs.index_row_if TO regress_minimal;
45+
-- Non-extension, non-function objects.
46+
CREATE COLLATIONs.coll (LOCALE="C");
47+
CREATETABLEs.x (ys.citext);
48+
ALTERTABLEs.x OWNER TO regress_minimal;
49+
-- Empty-table DefineIndex()
50+
CREATEUNIQUE INDEXu0rowsONs.x USING btree
51+
((s.index_this_expr(y,s.const())) COLLATEs.colls.citext_pattern_ops)
52+
WHEREs.index_row_if(y);
53+
ALTERTABLEs.x ADDCONSTRAINT e0rows EXCLUDE USING btree
54+
((s.index_this_expr(y,s.const())) COLLATEs.coll WITH s.=)
55+
WHERE (s.index_row_if(y));
56+
-- Make the table nonempty.
57+
INSERT INTOs.xVALUES ('foo'), ('bar');
58+
-- If the INSERT runs the planner on index expressions, a search_path change
59+
-- survives. As of 2022-06, the INSERT reuses a cached plan. It does so even
60+
-- under debug_discard_caches, since each index is new-in-transaction. If
61+
-- future work changes a cache lifecycle, this RESET may become necessary.
62+
RESET search_path;
63+
-- For a nonempty table, owner needs permissions throughout ii_Expressions.
64+
GRANT EXECUTEON FUNCTIONs.index_this_expr TO regress_minimal;
65+
CREATEUNIQUE INDEXu2rowsONs.x USING btree
66+
((s.index_this_expr(y,s.const())) COLLATEs.colls.citext_pattern_ops)
67+
WHEREs.index_row_if(y);
68+
ALTERTABLEs.x ADDCONSTRAINT e2rows EXCLUDE USING btree
69+
((s.index_this_expr(y,s.const())) COLLATEs.coll WITH s.=)
70+
WHERE (s.index_row_if(y));
71+
-- Shall not find s.coll via search_path, despite the s.const->public.setter
72+
-- call having set search_path=s during expression planning. Suppress the
73+
-- message itself, which depends on the database encoding.
74+
\set VERBOSITY sqlstate
75+
ALTERTABLEs.x ADDCONSTRAINT underqualified EXCLUDE USING btree
76+
((s.index_this_expr(y,s.const())) COLLATE coll WITH s.=)
77+
WHERE (s.index_row_if(y));
78+
\set VERBOSITY default
79+
ROLLBACK;

‎src/backend/commands/indexcmds.c

Lines changed: 81 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,10 @@ static void ComputeIndexAttrs(IndexInfo *indexInfo,
7979
OidrelId,
8080
constchar*accessMethodName,OidaccessMethodId,
8181
boolamcanorder,
82-
boolisconstraint);
82+
boolisconstraint,
83+
Oidddl_userid,
84+
intddl_sec_context,
85+
int*ddl_save_nestlevel);
8386
staticchar*ChooseIndexName(constchar*tabname,OidnamespaceId,
8487
List*colnames,List*exclusionOpNames,
8588
boolprimary,boolisconstraint);
@@ -197,9 +200,8 @@ CheckIndexCompatible(Oid oldId,
197200
* Compute the operator classes, collations, and exclusion operators for
198201
* the new index, so we can test whether it's compatible with the existing
199202
* one. Note that ComputeIndexAttrs might fail here, but that's OK:
200-
* DefineIndex would have called this function with the same arguments
201-
* later on, and it would have failed then anyway. Our attributeList
202-
* contains only key attributes, thus we're filling ii_NumIndexAttrs and
203+
* DefineIndex would have failed later. Our attributeList contains only
204+
* key attributes, thus we're filling ii_NumIndexAttrs and
203205
* ii_NumIndexKeyAttrs with same value.
204206
*/
205207
indexInfo=makeIndexInfo(numberOfAttributes,numberOfAttributes,
@@ -213,7 +215,7 @@ CheckIndexCompatible(Oid oldId,
213215
coloptions,attributeList,
214216
exclusionOpNames,relationId,
215217
accessMethodName,accessMethodId,
216-
amcanorder,isconstraint);
218+
amcanorder,isconstraint,InvalidOid,0,NULL);
217219

218220

219221
/* Get the soon-obsolete pg_index tuple. */
@@ -406,6 +408,19 @@ WaitForOlderSnapshots(TransactionId limitXmin, bool progress)
406408
* DefineIndex
407409
*Creates a new index.
408410
*
411+
* This function manages the current userid according to the needs of pg_dump.
412+
* Recreating old-database catalog entries in new-database is fine, regardless
413+
* of which users would have permission to recreate those entries now. That's
414+
* just preservation of state. Running opaque expressions, like calling a
415+
* function named in a catalog entry or evaluating a pg_node_tree in a catalog
416+
* entry, as anyone other than the object owner, is not fine. To adhere to
417+
* those principles and to remain fail-safe, use the table owner userid for
418+
* most ACL checks. Use the original userid for ACL checks reached without
419+
* traversing opaque expressions. (pg_dump can predict such ACL checks from
420+
* catalogs.) Overall, this is a mess. Future DDL development should
421+
* consider offering one DDL command for catalog setup and a separate DDL
422+
* command for steps that run opaque expressions.
423+
*
409424
* 'relationId': the OID of the heap relation on which the index is to be
410425
*created
411426
* 'stmt': IndexStmt describing the properties of the new index.
@@ -822,7 +837,8 @@ DefineIndex(Oid relationId,
822837
coloptions,allIndexParams,
823838
stmt->excludeOpNames,relationId,
824839
accessMethodName,accessMethodId,
825-
amcanorder,stmt->isconstraint);
840+
amcanorder,stmt->isconstraint,root_save_userid,
841+
root_save_sec_context,&root_save_nestlevel);
826842

827843
/*
828844
* Extra checks when creating a PRIMARY KEY index.
@@ -1098,11 +1114,8 @@ DefineIndex(Oid relationId,
10981114

10991115
/*
11001116
* Roll back any GUC changes executed by index functions, and keep
1101-
* subsequent changes local to this command. It's barely possible that
1102-
* some index function changed a behavior-affecting GUC, e.g. xmloption,
1103-
* that affects subsequent steps. This improves bug-compatibility with
1104-
* older PostgreSQL versions. They did the AtEOXact_GUC() here for the
1105-
* purpose of clearing the above default_tablespace change.
1117+
* subsequent changes local to this command. This is essential if some
1118+
* index function changed a behavior-affecting GUC, e.g. search_path.
11061119
*/
11071120
AtEOXact_GUC(false,root_save_nestlevel);
11081121
root_save_nestlevel=NewGUCNestLevel();
@@ -1638,6 +1651,10 @@ CheckPredicate(Expr *predicate)
16381651
* Compute per-index-column information, including indexed column numbers
16391652
* or index expressions, opclasses, and indoptions. Note, all output vectors
16401653
* should be allocated for all columns, including "including" ones.
1654+
*
1655+
* If the caller switched to the table owner, ddl_userid is the role for ACL
1656+
* checks reached without traversing opaque expressions. Otherwise, it's
1657+
* InvalidOid, and other ddl_* arguments are undefined.
16411658
*/
16421659
staticvoid
16431660
ComputeIndexAttrs(IndexInfo*indexInfo,
@@ -1651,12 +1668,17 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
16511668
constchar*accessMethodName,
16521669
OidaccessMethodId,
16531670
boolamcanorder,
1654-
boolisconstraint)
1671+
boolisconstraint,
1672+
Oidddl_userid,
1673+
intddl_sec_context,
1674+
int*ddl_save_nestlevel)
16551675
{
16561676
ListCell*nextExclOp;
16571677
ListCell*lc;
16581678
intattn;
16591679
intnkeycols=indexInfo->ii_NumIndexKeyAttrs;
1680+
Oidsave_userid;
1681+
intsave_sec_context;
16601682

16611683
/* Allocate space for exclusion operator info, if needed */
16621684
if (exclusionOpNames)
@@ -1670,6 +1692,9 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
16701692
else
16711693
nextExclOp=NULL;
16721694

1695+
if (OidIsValid(ddl_userid))
1696+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
1697+
16731698
/*
16741699
* process attributeList
16751700
*/
@@ -1800,10 +1825,24 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
18001825
}
18011826

18021827
/*
1803-
* Apply collation override if any
1828+
* Apply collation override if any. Use of ddl_userid is necessary
1829+
* due to ACL checks therein, and it's safe because collations don't
1830+
* contain opaque expressions (or non-opaque expressions).
18041831
*/
18051832
if (attribute->collation)
1833+
{
1834+
if (OidIsValid(ddl_userid))
1835+
{
1836+
AtEOXact_GUC(false,*ddl_save_nestlevel);
1837+
SetUserIdAndSecContext(ddl_userid,ddl_sec_context);
1838+
}
18061839
attcollation=get_collation_oid(attribute->collation, false);
1840+
if (OidIsValid(ddl_userid))
1841+
{
1842+
SetUserIdAndSecContext(save_userid,save_sec_context);
1843+
*ddl_save_nestlevel=NewGUCNestLevel();
1844+
}
1845+
}
18071846

18081847
/*
18091848
* Check we have a collation iff it's a collatable type. The only
@@ -1831,12 +1870,25 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
18311870
collationOidP[attn]=attcollation;
18321871

18331872
/*
1834-
* Identify the opclass to use.
1873+
* Identify the opclass to use. Use of ddl_userid is necessary due to
1874+
* ACL checks therein. This is safe despite opclasses containing
1875+
* opaque expressions (specifically, functions), because only
1876+
* superusers can define opclasses.
18351877
*/
1878+
if (OidIsValid(ddl_userid))
1879+
{
1880+
AtEOXact_GUC(false,*ddl_save_nestlevel);
1881+
SetUserIdAndSecContext(ddl_userid,ddl_sec_context);
1882+
}
18361883
classOidP[attn]=ResolveOpClass(attribute->opclass,
18371884
atttype,
18381885
accessMethodName,
18391886
accessMethodId);
1887+
if (OidIsValid(ddl_userid))
1888+
{
1889+
SetUserIdAndSecContext(save_userid,save_sec_context);
1890+
*ddl_save_nestlevel=NewGUCNestLevel();
1891+
}
18401892

18411893
/*
18421894
* Identify the exclusion operator, if any.
@@ -1850,9 +1902,23 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
18501902

18511903
/*
18521904
* Find the operator --- it must accept the column datatype
1853-
* without runtime coercion (but binary compatibility is OK)
1905+
* without runtime coercion (but binary compatibility is OK).
1906+
* Operators contain opaque expressions (specifically, functions).
1907+
* compatible_oper_opid() boils down to oper() and
1908+
* IsBinaryCoercible(). PostgreSQL would have security problems
1909+
* elsewhere if oper() started calling opaque expressions.
18541910
*/
1911+
if (OidIsValid(ddl_userid))
1912+
{
1913+
AtEOXact_GUC(false,*ddl_save_nestlevel);
1914+
SetUserIdAndSecContext(ddl_userid,ddl_sec_context);
1915+
}
18551916
opid=compatible_oper_opid(opname,atttype,atttype, false);
1917+
if (OidIsValid(ddl_userid))
1918+
{
1919+
SetUserIdAndSecContext(save_userid,save_sec_context);
1920+
*ddl_save_nestlevel=NewGUCNestLevel();
1921+
}
18561922

18571923
/*
18581924
* Only allow commutative operators to be used in exclusion

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp