Skip to content

Commit 23a4b89

Browse files
sfrostsfrost
committed
RLS refactoring
This refactors rewrite/rowsecurity.c to simplify the handling of the default deny case (reducing the number of places where we check for and add the default deny policy from three to one) by splitting up the retrival of the policies from the application of them. This also allowed us to do away with the policy_id field. A policy_name field was added for WithCheckOption policies and is used in error reporting, when available. Patch by Dean Rasheed, with various mostly cosmetic changes by me. Back-patch to 9.5 where RLS was introduced to avoid unnecessary differences, since we're still in alpha, per discussion with Robert.
1 parent ed301d6 commit 23a4b89

File tree

13 files changed

+447
-457
lines changed

13 files changed

+447
-457
lines changed

‎src/backend/commands/policy.c

-41
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,6 @@ policy_role_list_to_array(List *roles, int *num_roles)
186186
/*
187187
* Load row security policy from the catalog, and store it in
188188
* the relation's relcache entry.
189-
*
190-
* We will always set up some kind of policy here. If no explicit policies
191-
* are found then an implicit default-deny policy is created.
192189
*/
193190
void
194191
RelationBuildRowSecurity(Relation relation)
@@ -246,7 +243,6 @@ RelationBuildRowSecurity(Relation relation)
246243
char *with_check_value;
247244
Expr *with_check_qual;
248245
char *policy_name_value;
249-
Oid policy_id;
250246
bool isnull;
251247
RowSecurityPolicy *policy;
252248

@@ -298,14 +294,11 @@ RelationBuildRowSecurity(Relation relation)
298294
else
299295
with_check_qual = NULL;
300296

301-
policy_id = HeapTupleGetOid(tuple);
302-
303297
/* Now copy everything into the cache context */
304298
MemoryContextSwitchTo(rscxt);
305299

306300
policy = palloc0(sizeof(RowSecurityPolicy));
307301
policy->policy_name = pstrdup(policy_name_value);
308-
policy->policy_id = policy_id;
309302
policy->polcmd = cmd_value;
310303
policy->roles = DatumGetArrayTypePCopy(roles_datum);
311304
policy->qual = copyObject(qual_expr);
@@ -326,40 +319,6 @@ RelationBuildRowSecurity(Relation relation)
326319

327320
systable_endscan(sscan);
328321
heap_close(catalog, AccessShareLock);
329-
330-
/*
331-
* Check if no policies were added
332-
*
333-
* If no policies exist in pg_policy for this relation, then we need
334-
* to create a single default-deny policy. We use InvalidOid for the
335-
* Oid to indicate that this is the default-deny policy (we may decide
336-
* to ignore the default policy if an extension adds policies).
337-
*/
338-
if (rsdesc->policies == NIL)
339-
{
340-
RowSecurityPolicy *policy;
341-
Datum role;
342-
343-
MemoryContextSwitchTo(rscxt);
344-
345-
role = ObjectIdGetDatum(ACL_ID_PUBLIC);
346-
347-
policy = palloc0(sizeof(RowSecurityPolicy));
348-
policy->policy_name = pstrdup("default-deny policy");
349-
policy->policy_id = InvalidOid;
350-
policy->polcmd = '*';
351-
policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true,
352-
'i');
353-
policy->qual = (Expr *) makeConst(BOOLOID, -1, InvalidOid,
354-
sizeof(bool), BoolGetDatum(false),
355-
false, true);
356-
policy->with_check_qual = copyObject(policy->qual);
357-
policy->hassublinks = false;
358-
359-
rsdesc->policies = lcons(policy, rsdesc->policies);
360-
361-
MemoryContextSwitchTo(oldcxt);
362-
}
363322
}
364323
PG_CATCH();
365324
{

‎src/backend/executor/execMain.c

+16-4
Original file line numberDiff line numberDiff line change
@@ -1815,14 +1815,26 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
18151815
break;
18161816
case WCO_RLS_INSERT_CHECK:
18171817
case WCO_RLS_UPDATE_CHECK:
1818-
ereport(ERROR,
1819-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1818+
if (wco->polname != NULL)
1819+
ereport(ERROR,
1820+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1821+
errmsg("new row violates row level security policy \"%s\" for \"%s\"",
1822+
wco->polname, wco->relname)));
1823+
else
1824+
ereport(ERROR,
1825+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
18201826
errmsg("new row violates row level security policy for \"%s\"",
18211827
wco->relname)));
18221828
break;
18231829
case WCO_RLS_CONFLICT_CHECK:
1824-
ereport(ERROR,
1825-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1830+
if (wco->polname != NULL)
1831+
ereport(ERROR,
1832+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1833+
errmsg("new row violates row level security policy \"%s\" (USING expression) for \"%s\"",
1834+
wco->polname, wco->relname)));
1835+
else
1836+
ereport(ERROR,
1837+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
18261838
errmsg("new row violates row level security policy (USING expression) for \"%s\"",
18271839
wco->relname)));
18281840
break;

‎src/backend/nodes/copyfuncs.c

+1
Original file line numberDiff line numberDiff line change
@@ -2168,6 +2168,7 @@ _copyWithCheckOption(const WithCheckOption *from)
21682168

21692169
COPY_SCALAR_FIELD(kind);
21702170
COPY_STRING_FIELD(relname);
2171+
COPY_STRING_FIELD(polname);
21712172
COPY_NODE_FIELD(qual);
21722173
COPY_SCALAR_FIELD(cascaded);
21732174

‎src/backend/nodes/equalfuncs.c

+1
Original file line numberDiff line numberDiff line change
@@ -2445,6 +2445,7 @@ _equalWithCheckOption(const WithCheckOption *a, const WithCheckOption *b)
24452445
{
24462446
COMPARE_SCALAR_FIELD(kind);
24472447
COMPARE_STRING_FIELD(relname);
2448+
COMPARE_STRING_FIELD(polname);
24482449
COMPARE_NODE_FIELD(qual);
24492450
COMPARE_SCALAR_FIELD(cascaded);
24502451

‎src/backend/nodes/outfuncs.c

+1
Original file line numberDiff line numberDiff line change
@@ -2402,6 +2402,7 @@ _outWithCheckOption(StringInfo str, const WithCheckOption *node)
24022402

24032403
WRITE_ENUM_FIELD(kind, WCOKind);
24042404
WRITE_STRING_FIELD(relname);
2405+
WRITE_STRING_FIELD(polname);
24052406
WRITE_NODE_FIELD(qual);
24062407
WRITE_BOOL_FIELD(cascaded);
24072408
}

‎src/backend/nodes/readfuncs.c

+1
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ _readWithCheckOption(void)
270270

271271
READ_ENUM_FIELD(kind, WCOKind);
272272
READ_STRING_FIELD(relname);
273+
READ_STRING_FIELD(polname);
273274
READ_NODE_FIELD(qual);
274275
READ_BOOL_FIELD(cascaded);
275276

‎src/backend/rewrite/rewriteHandler.c

+3-2
Original file line numberDiff line numberDiff line change
@@ -1779,8 +1779,8 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
17791779
/*
17801780
* Fetch any new security quals that must be applied to this RTE.
17811781
*/
1782-
get_row_security_policies(parsetree, parsetree->commandType, rte,
1783-
rt_index, &securityQuals, &withCheckOptions,
1782+
get_row_security_policies(parsetree, rte, rt_index,
1783+
&securityQuals, &withCheckOptions,
17841784
&hasRowSecurity, &hasSubLinks);
17851785

17861786
if (securityQuals != NIL || withCheckOptions != NIL)
@@ -3019,6 +3019,7 @@ rewriteTargetView(Query *parsetree, Relation view)
30193019
wco = makeNode(WithCheckOption);
30203020
wco->kind = WCO_VIEW_CHECK;
30213021
wco->relname = pstrdup(RelationGetRelationName(view));
3022+
wco->polname = NULL;
30223023
wco->qual = NULL;
30233024
wco->cascaded = cascaded;
30243025

0 commit comments

Comments
 (0)