From b1b5a6c6bd63c509b3bcdef6d1e7a548413c2a9d Mon Sep 17 00:00:00 2001 From: Christoph Heiss Date: Tue, 18 Jan 2022 15:42:58 +0100 Subject: [PATCH 1/3] [PATCH v3 1/3] Add new boolean reloption security_invoker to views When this reloption is set to true, all references to the underlying tables will be checked against the invoking user rather than the view owner, as is currently implemented. Row level security must be enabled on the tables for this to take effect. Signed-off-by: Christoph Heiss --- src/backend/access/common/reloptions.c | 11 ++++ src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/nodes/outfuncs.c | 1 + src/backend/nodes/readfuncs.c | 1 + src/backend/optimizer/plan/subselect.c | 1 + src/backend/optimizer/prep/prepjointree.c | 1 + src/backend/rewrite/rewriteHandler.c | 1 + src/backend/utils/cache/relcache.c | 63 +++++++++++++---------- src/include/nodes/parsenodes.h | 1 + src/include/utils/rel.h | 11 ++++ 11 files changed, 65 insertions(+), 28 deletions(-) diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c index d592655258..beb170b5e6 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -140,6 +140,15 @@ static relopt_bool boolRelOpts[] = }, false }, + { + { + "security_invoker", + "View subquery in invoked within the current security context.", + RELOPT_KIND_VIEW, + AccessExclusiveLock + }, + false + }, { { "vacuum_truncate", @@ -1996,6 +2005,8 @@ view_reloptions(Datum reloptions, bool validate) static const relopt_parse_elt tab[] = { {"security_barrier", RELOPT_TYPE_BOOL, offsetof(ViewOptions, security_barrier)}, + {"security_invoker", RELOPT_TYPE_BOOL, + offsetof(ViewOptions, security_invoker)}, {"check_option", RELOPT_TYPE_ENUM, offsetof(ViewOptions, check_option)} }; diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 90b5da51c9..b171992ba8 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -2465,6 +2465,7 @@ _copyRangeTblEntry(const RangeTblEntry *from) COPY_NODE_FIELD(tablesample); COPY_NODE_FIELD(subquery); COPY_SCALAR_FIELD(security_barrier); + COPY_SCALAR_FIELD(security_invoker); COPY_SCALAR_FIELD(jointype); COPY_SCALAR_FIELD(joinmergedcols); COPY_NODE_FIELD(joinaliasvars); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 06345da3ba..a832c5fefe 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2766,6 +2766,7 @@ _equalRangeTblEntry(const RangeTblEntry *a, const RangeTblEntry *b) COMPARE_NODE_FIELD(tablesample); COMPARE_NODE_FIELD(subquery); COMPARE_SCALAR_FIELD(security_barrier); + COMPARE_SCALAR_FIELD(security_invoker); COMPARE_SCALAR_FIELD(jointype); COMPARE_SCALAR_FIELD(joinmergedcols); COMPARE_NODE_FIELD(joinaliasvars); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 2b0236937a..883284ad0d 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -3261,6 +3261,7 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node) case RTE_SUBQUERY: WRITE_NODE_FIELD(subquery); WRITE_BOOL_FIELD(security_barrier); + WRITE_BOOL_FIELD(security_invoker); break; case RTE_JOIN: WRITE_ENUM_FIELD(jointype, JoinType); diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 3f68f7c18d..ad825bb27f 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -1444,6 +1444,7 @@ _readRangeTblEntry(void) case RTE_SUBQUERY: READ_NODE_FIELD(subquery); READ_BOOL_FIELD(security_barrier); + READ_BOOL_FIELD(security_invoker); break; case RTE_JOIN: READ_ENUM_FIELD(jointype, JoinType); diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c index 41bd1ae7d4..30c66b9c6d 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -1216,6 +1216,7 @@ inline_cte_walker(Node *node, inline_cte_walker_context *context) rte->rtekind = RTE_SUBQUERY; rte->subquery = newquery; rte->security_barrier = false; + rte->security_invoker = false; /* Zero out CTE-specific fields */ rte->ctename = NULL; diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 282589dec8..bd7dc1c348 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -660,6 +660,7 @@ preprocess_function_rtes(PlannerInfo *root) rte->rtekind = RTE_SUBQUERY; rte->subquery = funcquery; rte->security_barrier = false; + rte->security_invoker = false; /* Clear fields that should not be set in a subquery RTE */ rte->functions = NIL; rte->funcordinality = false; diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 3d82138cb3..0dcdbe2968 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -1838,6 +1838,7 @@ ApplyRetrieveRule(Query *parsetree, rte->rtekind = RTE_SUBQUERY; rte->subquery = rule_action; rte->security_barrier = RelationIsSecurityView(relation); + rte->security_invoker = RelationHasSecurityInvoker(relation); /* Clear fields that should not be set in a subquery RTE */ rte->relid = InvalidOid; rte->relkind = 0; diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 2e760e8a3b..9ae03e3e8d 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -825,11 +825,14 @@ RelationBuildRuleLock(Relation relation) pfree(rule_str); /* - * We want the rule's table references to be checked as though by the - * table owner, not the user referencing the rule. Therefore, scan - * through the rule's actions and set the checkAsUser field on all - * rtable entries. We have to look at the qual as well, in case it - * contains sublinks. + * If we're dealing with a view that has the security_invoker relopt + * set to true, we want the rule's table references to be checked as + * the user referencing the rule. + * + * In all other cases, we want the rule's table references to be checked + * as though by the table owner. Therefore, scan through the rule's + * actions and set the checkAsUser field on all rtable entries. We + * have to look at the qual as well, in case it contains sublinks. * * The reason for doing this when the rule is loaded, rather than when * it is stored, is that otherwise ALTER TABLE OWNER would have to @@ -837,8 +840,12 @@ RelationBuildRuleLock(Relation relation) * the rule tree during load is relatively cheap (compared to * constructing it in the first place), so we do it here. */ - setRuleCheckAsUser((Node *) rule->actions, relation->rd_rel->relowner); - setRuleCheckAsUser(rule->qual, relation->rd_rel->relowner); + if (!(relation->rd_rel->relkind == RELKIND_VIEW + && RelationHasSecurityInvoker(relation))) + { + setRuleCheckAsUser((Node *) rule->actions, relation->rd_rel->relowner); + setRuleCheckAsUser(rule->qual, relation->rd_rel->relowner); + } if (numlocks >= maxlocks) { @@ -1163,27 +1170,6 @@ retry: */ RelationBuildTupleDesc(relation); - /* - * Fetch rules and triggers that affect this relation - */ - if (relation->rd_rel->relhasrules) - RelationBuildRuleLock(relation); - else - { - relation->rd_rules = NULL; - relation->rd_rulescxt = NULL; - } - - if (relation->rd_rel->relhastriggers) - RelationBuildTriggers(relation); - else - relation->trigdesc = NULL; - - if (relation->rd_rel->relrowsecurity) - RelationBuildRowSecurity(relation); - else - relation->rd_rsdesc = NULL; - /* foreign key data is not loaded till asked for */ relation->rd_fkeylist = NIL; relation->rd_fkeyvalid = false; @@ -1215,6 +1201,27 @@ retry: /* extract reloptions if any */ RelationParseRelOptions(relation, pg_class_tuple); + /* + * Fetch rules and triggers that affect this relation + */ + if (relation->rd_rel->relhasrules) + RelationBuildRuleLock(relation); + else + { + relation->rd_rules = NULL; + relation->rd_rulescxt = NULL; + } + + if (relation->rd_rel->relhastriggers) + RelationBuildTriggers(relation); + else + relation->trigdesc = NULL; + + if (relation->rd_rel->relrowsecurity) + RelationBuildRowSecurity(relation); + else + relation->rd_rsdesc = NULL; + /* * initialize the relation lock manager information */ diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 3e9bdc781f..1362f5d111 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1042,6 +1042,7 @@ typedef struct RangeTblEntry */ Query *subquery; /* the sub-query */ bool security_barrier; /* is from security_barrier view? */ + bool security_invoker; /* from a view with security_invoker set? */ /* * Fields valid for a join RTE (else NULL/zero): diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index 6da1b220cd..9a8866c91d 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -398,6 +398,7 @@ typedef struct ViewOptions { int32 vl_len_; /* varlena header (do not touch directly!) */ bool security_barrier; + bool security_invoker; ViewOptCheckOption check_option; } ViewOptions; @@ -411,6 +412,16 @@ typedef struct ViewOptions (relation)->rd_options ? \ ((ViewOptions *) (relation)->rd_options)->security_barrier : false) +/* + * RelationHasSecurityRelationPermissions + * Returns true if the relation has the security invoker property set, or + * not. Note multiple eval of argument! + */ +#define RelationHasSecurityInvoker(relation) \ + (AssertMacro(relation->rd_rel->relkind == RELKIND_VIEW), \ + (relation)->rd_options ? \ + ((ViewOptions *) (relation)->rd_options)->security_invoker : false) + /* * RelationHasCheckOption * Returns true if the relation is a view defined with either the local -- 2.34.1