From 25267e6b8a2ffd81f14acbee95ef08d9edf3d31c Mon Sep 17 00:00:00 2001 From: Christoph Heiss Date: Tue, 18 Jan 2022 15:42:58 +0100 Subject: [PATCH 1/3] [PATCH v2 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 b5602f5323..3c84982fda 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 df0b747883..6efaa07523 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -2464,6 +2464,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 cb7ddd463c..7f0401fa84 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 91a89b6d51..7dee550856 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -3260,6 +3260,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 dcec3b728f..95f006cf04 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -1446,6 +1446,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 c9f7a09d10..c2b1a201ba 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 387a35e112..18b209ef7d 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 9521e81100..68830412c5 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 105d8d4601..e9515ee896 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 4c5a8a39bf..31d8a951ea 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1041,6 +1041,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 31281279cf..d84491baa7 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