Skip to content

Commit 842faa7

Browse files
sfrostsfrost
committed
Make security barrier views automatically updatable
Views which are marked as security_barrier must have their quals applied before any user-defined quals are called, to prevent user-defined functions from being able to see rows which the security barrier view is intended to prevent them from seeing. Remove the restriction on security barrier views being automatically updatable by adding a new securityQuals list to the RTE structure which keeps track of the quals from security barrier views at each level, independently of the user-supplied quals. When RTEs are later discovered which have securityQuals populated, they are turned into subquery RTEs which are marked as security_barrier to prevent any user-supplied quals being pushed down (modulo LEAKPROOF quals). Dean Rasheed, reviewed by Craig Ringer, Simon Riggs, KaiGai Kohei
1 parent 9d229f3 commit 842faa7

File tree

19 files changed

+1372
-102
lines changed

19 files changed

+1372
-102
lines changed

‎doc/src/sgml/ref/create_view.sgml

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -323,12 +323,6 @@ CREATE VIEW vista AS SELECT text 'Hello World' AS hello;
323323
or set-returning functions.
324324
</para>
325325
</listitem>
326-
327-
<listitem>
328-
<para>
329-
The view must not have the <literal>security_barrier</> property.
330-
</para>
331-
</listitem>
332326
</itemizedlist>
333327
</para>
334328

@@ -361,6 +355,19 @@ CREATE VIEW vista AS SELECT text 'Hello World' AS hello;
361355
such rows that are not visible through the view.
362356
</para>
363357

358+
<para>
359+
If an automatically updatable view is marked with the
360+
<literal>security_barrier</> property then all the view's <literal>WHERE</>
361+
conditions (and any conditions using operators which are marked as LEAKPROOF)
362+
will always be evaluated before any conditions that a user of the view has
363+
added. See <xref linkend="rules-privileges"> for full details. Note that,
364+
due to this, rows which are not ultimately returned (because they do not
365+
pass the user's <literal>WHERE</> conditions) may still end up being locked.
366+
<command>EXPLAIN</command> can be used to see which conditions are
367+
applied at the relation level (and therefore do not lock rows) and which are
368+
not.
369+
</para>
370+
364371
<para>
365372
A more complex view that does not satisfy all these conditions is
366373
read-only by default: the system will not allow an insert, update, or

‎src/backend/commands/tablecmds.c

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8910,16 +8910,13 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
89108910
List *view_options = untransformRelOptions(newOptions);
89118911
ListCell *cell;
89128912
bool check_option = false;
8913-
bool security_barrier = false;
89148913

89158914
foreach(cell, view_options)
89168915
{
89178916
DefElem *defel = (DefElem *) lfirst(cell);
89188917

89198918
if (pg_strcasecmp(defel->defname, "check_option") == 0)
89208919
check_option = true;
8921-
if (pg_strcasecmp(defel->defname, "security_barrier") == 0)
8922-
security_barrier = defGetBoolean(defel);
89238920
}
89248921

89258922
/*
@@ -8929,8 +8926,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
89298926
if (check_option)
89308927
{
89318928
const char *view_updatable_error =
8932-
view_query_is_auto_updatable(view_query,
8933-
security_barrier, true);
8929+
view_query_is_auto_updatable(view_query, true);
89348930

89358931
if (view_updatable_error)
89368932
ereport(ERROR,

‎src/backend/commands/view.c

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,6 @@ DefineView(ViewStmt *stmt, const char *queryString)
396396
RangeVar *view;
397397
ListCell *cell;
398398
bool check_option;
399-
bool security_barrier;
400399

401400
/*
402401
* Run parse analysis to convert the raw parse tree to a Query. Note this
@@ -451,16 +450,13 @@ DefineView(ViewStmt *stmt, const char *queryString)
451450
* specified.
452451
*/
453452
check_option = false;
454-
security_barrier = false;
455453

456454
foreach(cell, stmt->options)
457455
{
458456
DefElem *defel = (DefElem *) lfirst(cell);
459457

460458
if (pg_strcasecmp(defel->defname, "check_option") == 0)
461459
check_option = true;
462-
if (pg_strcasecmp(defel->defname, "security_barrier") == 0)
463-
security_barrier = defGetBoolean(defel);
464460
}
465461

466462
/*
@@ -470,7 +466,7 @@ DefineView(ViewStmt *stmt, const char *queryString)
470466
if (check_option)
471467
{
472468
const char *view_updatable_error =
473-
view_query_is_auto_updatable(viewParse, security_barrier, true);
469+
view_query_is_auto_updatable(viewParse, true);
474470

475471
if (view_updatable_error)
476472
ereport(ERROR,

‎src/backend/nodes/copyfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1998,6 +1998,7 @@ _copyRangeTblEntry(const RangeTblEntry *from)
19981998
COPY_SCALAR_FIELD(checkAsUser);
19991999
COPY_BITMAPSET_FIELD(selectedCols);
20002000
COPY_BITMAPSET_FIELD(modifiedCols);
2001+
COPY_NODE_FIELD(securityQuals);
20012002

20022003
return newnode;
20032004
}

‎src/backend/nodes/equalfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2296,6 +2296,7 @@ _equalRangeTblEntry(const RangeTblEntry *a, const RangeTblEntry *b)
22962296
COMPARE_SCALAR_FIELD(checkAsUser);
22972297
COMPARE_BITMAPSET_FIELD(selectedCols);
22982298
COMPARE_BITMAPSET_FIELD(modifiedCols);
2299+
COMPARE_NODE_FIELD(securityQuals);
22992300

23002301
return true;
23012302
}

‎src/backend/nodes/nodeFuncs.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2020,6 +2020,9 @@ range_table_walker(List *rtable,
20202020
return true;
20212021
break;
20222022
}
2023+
2024+
if (walker(rte->securityQuals, context))
2025+
return true;
20232026
}
20242027
return false;
20252028
}
@@ -2755,6 +2758,7 @@ range_table_mutator(List *rtable,
27552758
MUTATE(newrte->values_lists, rte->values_lists, List *);
27562759
break;
27572760
}
2761+
MUTATE(newrte->securityQuals, rte->securityQuals, List *);
27582762
newrt = lappend(newrt, newrte);
27592763
}
27602764
return newrt;

‎src/backend/nodes/outfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2409,6 +2409,7 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
24092409
WRITE_OID_FIELD(checkAsUser);
24102410
WRITE_BITMAPSET_FIELD(selectedCols);
24112411
WRITE_BITMAPSET_FIELD(modifiedCols);
2412+
WRITE_NODE_FIELD(securityQuals);
24122413
}
24132414

24142415
static void

‎src/backend/nodes/readfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,6 +1252,7 @@ _readRangeTblEntry(void)
12521252
READ_OID_FIELD(checkAsUser);
12531253
READ_BITMAPSET_FIELD(selectedCols);
12541254
READ_BITMAPSET_FIELD(modifiedCols);
1255+
READ_NODE_FIELD(securityQuals);
12551256

12561257
READ_DONE();
12571258
}

‎src/backend/optimizer/plan/planner.c

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -915,6 +915,12 @@ inheritance_planner(PlannerInfo *root)
915915
/* Generate plan */
916916
subplan = grouping_planner(&subroot, 0.0 /* retrieve all tuples */ );
917917

918+
/*
919+
* Planning may have modified the query result relation (if there
920+
* were security barrier quals on the result RTE).
921+
*/
922+
appinfo->child_relid = subroot.parse->resultRelation;
923+
918924
/*
919925
* If this child rel was excluded by constraint exclusion, exclude it
920926
* from the result plan.
@@ -932,9 +938,40 @@ inheritance_planner(PlannerInfo *root)
932938
if (final_rtable == NIL)
933939
final_rtable = subroot.parse->rtable;
934940
else
935-
final_rtable = list_concat(final_rtable,
941+
{
942+
List *tmp_rtable = NIL;
943+
ListCell *cell1, *cell2;
944+
945+
/*
946+
* Check to see if any of the original RTEs were turned into
947+
* subqueries during planning. Currently, this should only ever
948+
* happen due to securityQuals being involved which push a
949+
* relation down under a subquery, to ensure that the security
950+
* barrier quals are evaluated first.
951+
*
952+
* When this happens, we want to use the new subqueries in the
953+
* final rtable.
954+
*/
955+
forboth(cell1, final_rtable, cell2, subroot.parse->rtable)
956+
{
957+
RangeTblEntry *rte1 = (RangeTblEntry *) lfirst(cell1);
958+
RangeTblEntry *rte2 = (RangeTblEntry *) lfirst(cell2);
959+
960+
if (rte1->rtekind == RTE_RELATION &&
961+
rte2->rtekind == RTE_SUBQUERY)
962+
{
963+
/* Should only be when there are securityQuals today */
964+
Assert(rte1->securityQuals != NIL);
965+
tmp_rtable = lappend(tmp_rtable, rte2);
966+
}
967+
else
968+
tmp_rtable = lappend(tmp_rtable, rte1);
969+
}
970+
971+
final_rtable = list_concat(tmp_rtable,
936972
list_copy_tail(subroot.parse->rtable,
937973
list_length(final_rtable)));
974+
}
938975

939976
/*
940977
* We need to collect all the RelOptInfos from all child plans into
@@ -1162,6 +1199,12 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
11621199
/* Preprocess targetlist */
11631200
tlist = preprocess_targetlist(root, tlist);
11641201

1202+
/*
1203+
* Expand any rangetable entries that have security barrier quals.
1204+
* This may add new security barrier subquery RTEs to the rangetable.
1205+
*/
1206+
expand_security_quals(root, tlist);
1207+
11651208
/*
11661209
* Locate any window functions in the tlist. (We don't need to look
11671210
* anywhere else, since expressions used in ORDER BY will be in there

‎src/backend/optimizer/prep/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,6 @@ subdir = src/backend/optimizer/prep
1212
top_builddir = ../../../..
1313
include $(top_builddir)/src/Makefile.global
1414

15-
OBJS = prepjointree.o prepqual.o preptlist.o prepunion.o
15+
OBJS = prepjointree.o prepqual.o prepsecurity.o preptlist.o prepunion.o
1616

1717
include $(top_srcdir)/src/backend/common.mk

0 commit comments

Comments
 (0)