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

Commitcfa928f

Browse files
committed
Plug RLS related information leak in pg_stats view.
The pg_stats view is supposed to be restricted to only show rowsabout tables the user can read. However, it sometimes can leakinformation which could not otherwise be seen when row level securityis enabled. Fix that by not showing pg_stats rows to users that wouldbe subject to RLS on the table the row is related to. This is doneby creating/using the newly introduced SQL visible function,row_security_active().Along the way, clean up three call sites of check_enable_rls(). The secondargument of that function should only be specified as other thanInvalidOid when we are checking as a different user than the current one,as in when querying through a view. These sites were passing GetUserId()instead of InvalidOid, which can cause the function to return incorrectresults if the current user has the BYPASSRLS privilege and row_securityhas been set to OFF.Additionally fix a bug causing RI Trigger error messages to unintentionallyleak information when RLS is enabled, and other minor cleanup andimprovements. Also add WITH (security_barrier) to the definition of pg_stats.Bumped CATVERSION due to new SQL functions and pg_stats view definition.Back-patch to 9.5 where RLS was introduced. Reported by Yaroslav.Patch by Joe Conway and Dean Rasheed with review and input byMichael Paquier and Stephen Frost.
1 parent6087d95 commitcfa928f

File tree

16 files changed

+159
-31
lines changed

16 files changed

+159
-31
lines changed

‎doc/src/sgml/func.sgml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15244,6 +15244,12 @@ SET search_path TO <replaceable>schema</> <optional>, <replaceable>schema</>, ..
1524415244
<entry><type>boolean</type></entry>
1524515245
<entry>does current user have privilege for role</entry>
1524615246
</row>
15247+
<row>
15248+
<entry><literal><function>row_security_active</function>(<parameter>table</parameter>)</literal>
15249+
</entry>
15250+
<entry><type>boolean</type></entry>
15251+
<entry>does current user have row level security active for table</entry>
15252+
</row>
1524715253
</tbody>
1524815254
</tgroup>
1524915255
</table>
@@ -15284,6 +15290,9 @@ SET search_path TO <replaceable>schema</> <optional>, <replaceable>schema</>, ..
1528415290
<indexterm>
1528515291
<primary>pg_has_role</primary>
1528615292
</indexterm>
15293+
<indexterm>
15294+
<primary>row_security_active</primary>
15295+
</indexterm>
1528715296

1528815297
<para>
1528915298
<function>has_table_privilege</function> checks whether a user
@@ -15447,6 +15456,13 @@ SELECT has_function_privilege('joeuser', 'myfunc(int, text)', 'execute');
1544715456
are immediately available without doing <command>SET ROLE</>.
1544815457
</para>
1544915458

15459+
<para>
15460+
<function>row_security_active</function> checks whether row level
15461+
security is active for the specified table in the context of the
15462+
<function>current_user</function> and environment. The table can
15463+
be specified by name or by OID.
15464+
</para>
15465+
1545015466
<para>
1545115467
<xref linkend="functions-info-schema-table"> shows functions that
1545215468
determine whether a certain object is <firstterm>visible</> in the

‎src/backend/access/index/genam.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ BuildIndexValueDescription(Relation indexRelation,
204204
Assert(indexrelid==idxrec->indexrelid);
205205

206206
/* RLS check- if RLS is enabled then we don't return anything. */
207-
if (check_enable_rls(indrelid,GetUserId(), true)==RLS_ENABLED)
207+
if (check_enable_rls(indrelid,InvalidOid, true)==RLS_ENABLED)
208208
{
209209
ReleaseSysCache(ht_idx);
210210
returnNULL;

‎src/backend/catalog/system_views.sql

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ CREATE VIEW pg_indexes AS
150150
LEFT JOIN pg_tablespace TON (T.oid=I.reltablespace)
151151
WHEREC.relkindIN ('r','m')ANDI.relkind='i';
152152

153-
CREATEVIEWpg_statsAS
153+
CREATEVIEWpg_statsWITH (security_barrier)AS
154154
SELECT
155155
nspnameAS schemaname,
156156
relnameAS tablename,
@@ -211,7 +211,9 @@ CREATE VIEW pg_stats AS
211211
FROM pg_statistic sJOIN pg_class cON (c.oid=s.starelid)
212212
JOIN pg_attribute aON (c.oid= attrelidAND attnum=s.staattnum)
213213
LEFT JOIN pg_namespace nON (n.oid=c.relnamespace)
214-
WHERE NOT attisdroppedAND has_column_privilege(c.oid,a.attnum,'select');
214+
WHERE NOT attisdropped
215+
AND has_column_privilege(c.oid,a.attnum,'select')
216+
AND (c.relrowsecurity= falseOR NOT row_security_active(c.oid));
215217

216218
REVOKE ALLon pg_statisticFROM public;
217219

‎src/backend/executor/execMain.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1874,7 +1874,7 @@ ExecBuildSlotValueDescription(Oid reloid,
18741874
* then don't return anything. Otherwise, go through normal permission
18751875
* checks.
18761876
*/
1877-
if (check_enable_rls(reloid,GetUserId(), true)==RLS_ENABLED)
1877+
if (check_enable_rls(reloid,InvalidOid, true)==RLS_ENABLED)
18781878
returnNULL;
18791879

18801880
initStringInfo(&buf);

‎src/backend/rewrite/rowsecurity.c

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ get_row_security_policies(Query *root, CmdType commandType, RangeTblEntry *rte,
107107

108108
Relationrel;
109109
Oiduser_id;
110-
intsec_context;
111110
intrls_status;
112111
booldefaultDeny= false;
113112

@@ -117,22 +116,13 @@ get_row_security_policies(Query *root, CmdType commandType, RangeTblEntry *rte,
117116
*hasRowSecurity= false;
118117
*hasSubLinks= false;
119118

120-
/* This is just to get the security context */
121-
GetUserIdAndSecContext(&user_id,&sec_context);
119+
/* If this is not a normal relation, just return immediately */
120+
if (rte->relkind!=RELKIND_RELATION)
121+
return;
122122

123123
/* Switch to checkAsUser if it's set */
124124
user_id=rte->checkAsUser ?rte->checkAsUser :GetUserId();
125125

126-
/*
127-
* If this is not a normal relation, or we have been told to explicitly
128-
* skip RLS (perhaps because this is an FK check) then just return
129-
* immediately.
130-
*/
131-
if (rte->relid<FirstNormalObjectId
132-
||rte->relkind!=RELKIND_RELATION
133-
|| (sec_context&SECURITY_ROW_LEVEL_DISABLED))
134-
return;
135-
136126
/* Determine the state of RLS for this, pass checkAsUser explicitly */
137127
rls_status=check_enable_rls(rte->relid,rte->checkAsUser, false);
138128

‎src/backend/utils/adt/ri_triggers.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3243,7 +3243,7 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo,
32433243
* privileges.
32443244
*/
32453245

3246-
if (check_enable_rls(rel_oid,GetUserId(), true)!=RLS_ENABLED)
3246+
if (check_enable_rls(rel_oid,InvalidOid, true)!=RLS_ENABLED)
32473247
{
32483248
aclresult=pg_class_aclcheck(rel_oid,GetUserId(),ACL_SELECT);
32493249
if (aclresult!=ACLCHECK_OK)
@@ -3264,6 +3264,8 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo,
32643264
}
32653265
}
32663266
}
3267+
else
3268+
has_perm= false;
32673269

32683270
if (has_perm)
32693271
{

‎src/backend/utils/cache/plancache.c

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,6 @@ CreateCachedPlan(Node *raw_parse_tree,
153153
CachedPlanSource*plansource;
154154
MemoryContextsource_context;
155155
MemoryContextoldcxt;
156-
Oiduser_id;
157-
intsecurity_context;
158156

159157
Assert(query_string!=NULL);/* required as of 8.4 */
160158

@@ -177,8 +175,6 @@ CreateCachedPlan(Node *raw_parse_tree,
177175
*/
178176
oldcxt=MemoryContextSwitchTo(source_context);
179177

180-
GetUserIdAndSecContext(&user_id,&security_context);
181-
182178
plansource= (CachedPlanSource*)palloc0(sizeof(CachedPlanSource));
183179
plansource->magic=CACHEDPLANSOURCE_MAGIC;
184180
plansource->raw_parse_tree=copyObject(raw_parse_tree);
@@ -208,8 +204,7 @@ CreateCachedPlan(Node *raw_parse_tree,
208204
plansource->total_custom_cost=0;
209205
plansource->num_custom_plans=0;
210206
plansource->hasRowSecurity= false;
211-
plansource->rowSecurityDisabled
212-
= (security_context&SECURITY_ROW_LEVEL_DISABLED)!=0;
207+
plansource->rowSecurityDisabled=InRowLevelSecurityDisabled();
213208
plansource->row_security_env=row_security;
214209
plansource->planUserId=InvalidOid;
215210

‎src/backend/utils/init/miscinit.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ GetAuthenticatedUserId(void)
341341
* GetUserIdAndSecContext/SetUserIdAndSecContext - get/set the current user ID
342342
* and the SecurityRestrictionContext flags.
343343
*
344-
* Currently there aretwo valid bits in SecurityRestrictionContext:
344+
* Currently there arethree valid bits in SecurityRestrictionContext:
345345
*
346346
* SECURITY_LOCAL_USERID_CHANGE indicates that we are inside an operation
347347
* that is temporarily changing CurrentUserId via these functions. This is
@@ -359,6 +359,9 @@ GetAuthenticatedUserId(void)
359359
* where the called functions are really supposed to be side-effect-free
360360
* anyway, such as VACUUM/ANALYZE/REINDEX.
361361
*
362+
* SECURITY_ROW_LEVEL_DISABLED indicates that we are inside an operation that
363+
* needs to bypass row level security checks, for example FK checks.
364+
*
362365
* Unlike GetUserId, GetUserIdAndSecContext does *not* Assert that the current
363366
* value of CurrentUserId is valid; nor does SetUserIdAndSecContext require
364367
* the new value to be valid. In fact, these routines had better not
@@ -401,6 +404,15 @@ InSecurityRestrictedOperation(void)
401404
return (SecurityRestrictionContext&SECURITY_RESTRICTED_OPERATION)!=0;
402405
}
403406

407+
/*
408+
* InRowLevelSecurityDisabled - are we inside a RLS-disabled operation?
409+
*/
410+
bool
411+
InRowLevelSecurityDisabled(void)
412+
{
413+
return (SecurityRestrictionContext&SECURITY_ROW_LEVEL_DISABLED)!=0;
414+
}
415+
404416

405417
/*
406418
* These are obsolete versions of Get/SetUserIdAndSecContext that are

‎src/backend/utils/misc/rls.c

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@
1616

1717
#include"access/htup.h"
1818
#include"access/htup_details.h"
19+
#include"access/transam.h"
1920
#include"catalog/pg_class.h"
21+
#include"catalog/namespace.h"
2022
#include"miscadmin.h"
2123
#include"utils/acl.h"
24+
#include"utils/builtins.h"
2225
#include"utils/elog.h"
2326
#include"utils/rls.h"
2427
#include"utils/syscache.h"
@@ -37,7 +40,10 @@ extern intcheck_enable_rls(Oid relid, Oid checkAsUser, bool noError);
3740
* for the table and the plan cache needs to be invalidated if the environment
3841
* changes.
3942
*
40-
* Handle checking as another role via checkAsUser (for views, etc).
43+
* Handle checking as another role via checkAsUser (for views, etc). Note that
44+
* if *not* checking as another role, the caller should pass InvalidOid rather
45+
* than GetUserId(). Otherwise the check for row_security = OFF is skipped, and
46+
* so we may falsely report that RLS is active when the user has bypassed it.
4147
*
4248
* If noError is set to 'true' then we just return RLS_ENABLED instead of doing
4349
* an ereport() if the user has attempted to bypass RLS and they are not
@@ -53,6 +59,17 @@ check_enable_rls(Oid relid, Oid checkAsUser, bool noError)
5359
boolrelrowsecurity;
5460
Oiduser_id=checkAsUser ?checkAsUser :GetUserId();
5561

62+
/* Nothing to do for built-in relations */
63+
if (relid<FirstNormalObjectId)
64+
returnRLS_NONE;
65+
66+
/*
67+
* Check if we have been told to explicitly skip RLS (perhaps because this
68+
* is a foreign key check)
69+
*/
70+
if (InRowLevelSecurityDisabled())
71+
returnRLS_NONE;
72+
5673
tuple=SearchSysCache1(RELOID,ObjectIdGetDatum(relid));
5774
if (!HeapTupleIsValid(tuple))
5875
returnRLS_NONE;
@@ -111,3 +128,37 @@ check_enable_rls(Oid relid, Oid checkAsUser, bool noError)
111128
/* RLS should be fully enabled for this relation. */
112129
returnRLS_ENABLED;
113130
}
131+
132+
/*
133+
* row_security_active
134+
*
135+
* check_enable_rls wrapped as a SQL callable function except
136+
* RLS_NONE_ENV and RLS_NONE are the same for this purpose.
137+
*/
138+
Datum
139+
row_security_active(PG_FUNCTION_ARGS)
140+
{
141+
/* By OID */
142+
Oidtableoid=PG_GETARG_OID(0);
143+
intrls_status;
144+
145+
rls_status=check_enable_rls(tableoid,InvalidOid, true);
146+
PG_RETURN_BOOL(rls_status==RLS_ENABLED);
147+
}
148+
149+
Datum
150+
row_security_active_name(PG_FUNCTION_ARGS)
151+
{
152+
/* By qualified name */
153+
text*tablename=PG_GETARG_TEXT_P(0);
154+
RangeVar*tablerel;
155+
Oidtableoid;
156+
intrls_status;
157+
158+
/* Look up table name. Can't lock it - we might not have privileges. */
159+
tablerel=makeRangeVarFromNameList(textToQualifiedNameList(tablename));
160+
tableoid=RangeVarGetRelid(tablerel,NoLock, false);
161+
162+
rls_status=check_enable_rls(tableoid,InvalidOid, true);
163+
PG_RETURN_BOOL(rls_status==RLS_ENABLED);
164+
}

‎src/include/catalog/catversion.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@
5353
*/
5454

5555
/*yyyymmddN */
56-
#defineCATALOG_VERSION_NO201507251
56+
#defineCATALOG_VERSION_NO201507281
5757

5858
#endif

‎src/include/catalog/pg_proc.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5337,6 +5337,12 @@ DESCR("get progress for all replication origins");
53375337
#define PROVOLATILE_STABLE's'/* does not change within a scan */
53385338
#define PROVOLATILE_VOLATILE'v'/* can change even within a scan */
53395339

5340+
/* rls */
5341+
DATA(insert OID = 3298 ( row_security_active PGNSP PGUID 12 1 0 0 0 f f f f t f s 1 0 16 "26" _null_ _null_ _null_ _null_ _null_row_security_active _null_ _null_ _null_ ));
5342+
DESCR("row security for current context active on table by table oid");
5343+
DATA(insert OID = 3299 ( row_security_active PGNSP PGUID 12 1 0 0 0 f f f f t f s 1 0 16 "25" _null_ _null_ _null_ _null_ _null_row_security_active_name _null_ _null_ _null_ ));
5344+
DESCR("row security for current context active on table by table name");
5345+
53405346
/*
53415347
* Symbolic values for proargmodes column. Note that these must agree with
53425348
* the FunctionParameterMode enum in parsenodes.h; we declare them here to

‎src/include/miscadmin.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,7 @@ extern void GetUserIdAndSecContext(Oid *userid, int *sec_context);
305305
externvoidSetUserIdAndSecContext(Oiduserid,intsec_context);
306306
externboolInLocalUserIdChange(void);
307307
externboolInSecurityRestrictedOperation(void);
308+
externboolInRowLevelSecurityDisabled(void);
308309
externvoidGetUserIdAndContext(Oid*userid,bool*sec_def_context);
309310
externvoidSetUserIdAndContext(Oiduserid,boolsec_def_context);
310311
externvoidInitializeSessionUserId(constchar*rolename,Oiduseroid);

‎src/include/utils/builtins.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,6 +1120,10 @@ extern Datum set_config_by_name(PG_FUNCTION_ARGS);
11201120
externDatumshow_all_settings(PG_FUNCTION_ARGS);
11211121
externDatumshow_all_file_settings(PG_FUNCTION_ARGS);
11221122

1123+
/* rls.c */
1124+
externDatumrow_security_active(PG_FUNCTION_ARGS);
1125+
externDatumrow_security_active_name(PG_FUNCTION_ARGS);
1126+
11231127
/* lockfuncs.c */
11241128
externDatumpg_lock_status(PG_FUNCTION_ARGS);
11251129
externDatumpg_advisory_lock_int8(PG_FUNCTION_ARGS);

‎src/test/regress/expected/rowsecurity.out

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ SELECT * FROM document d FULL OUTER JOIN category c on d.cid = c.cid;
307307

308308
DELETE FROM category WHERE cid = 33; -- fails with FK violation
309309
ERROR: update or delete on table "category" violates foreign key constraint "document_cid_fkey" on table "document"
310-
DETAIL: Key(cid)=(33)is still referenced from table "document".
310+
DETAIL: Key is still referenced from table "document".
311311
-- can insert FK referencing invisible PK
312312
SET SESSION AUTHORIZATION rls_regress_user2;
313313
SELECT * FROM document d FULL OUTER JOIN category c on d.cid = c.cid;
@@ -2886,11 +2886,45 @@ SELECT * FROM current_check;
28862886
(1 row)
28872887

28882888
COMMIT;
2889+
--
2890+
-- check pg_stats view filtering
2891+
--
2892+
SET row_security TO ON;
2893+
SET SESSION AUTHORIZATION rls_regress_user0;
2894+
ANALYZE current_check;
2895+
-- Stats visible
2896+
SELECT row_security_active('current_check');
2897+
row_security_active
2898+
---------------------
2899+
f
2900+
(1 row)
2901+
2902+
SELECT most_common_vals FROM pg_stats where tablename = 'current_check';
2903+
most_common_vals
2904+
---------------------
2905+
2906+
2907+
{rls_regress_user1}
2908+
(3 rows)
2909+
2910+
SET SESSION AUTHORIZATION rls_regress_user1;
2911+
-- Stats not visible
2912+
SELECT row_security_active('current_check');
2913+
row_security_active
2914+
---------------------
2915+
t
2916+
(1 row)
2917+
2918+
SELECT most_common_vals FROM pg_stats where tablename = 'current_check';
2919+
most_common_vals
2920+
------------------
2921+
(0 rows)
2922+
28892923
--
28902924
-- Collation support
28912925
--
28922926
BEGIN;
2893-
SET row_security= force;
2927+
SET row_securityTO FORCE;
28942928
CREATE TABLE coll_t (c) AS VALUES ('bar'::text);
28952929
CREATE POLICY coll_p ON coll_t USING (c < ('foo'::text COLLATE "C"));
28962930
ALTER TABLE coll_t ENABLE ROW LEVEL SECURITY;

‎src/test/regress/expected/rules.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2061,7 +2061,7 @@ pg_stats| SELECT n.nspname AS schemaname,
20612061
JOIN pg_class c ON ((c.oid = s.starelid)))
20622062
JOIN pg_attribute a ON (((c.oid = a.attrelid) AND (a.attnum = s.staattnum))))
20632063
LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
2064-
WHERE ((NOT a.attisdropped) AND has_column_privilege(c.oid, a.attnum, 'select'::text));
2064+
WHERE ((NOT a.attisdropped) AND has_column_privilege(c.oid, a.attnum, 'select'::text) AND ((c.relrowsecurity = false) OR (NOT row_security_active(c.oid))));
20652065
pg_tables| SELECT n.nspname AS schemaname,
20662066
c.relname AS tablename,
20672067
pg_get_userbyid(c.relowner) AS tableowner,

0 commit comments

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