From 5ec76812a438322d39ff10b84b949ae79361878a Mon Sep 17 00:00:00 2001 From: Christoph Heiss Date: Thu, 16 Dec 2021 19:19:48 +0100 Subject: [PATCH 1/3] Add new reloption enum 'security' to views Only a single value is supported for now: 'relation_permissions'. When this relopt is set on a view, all rules table references will be checked against the invoking user rather than the view owner, as is currently implemented. --- src/backend/access/common/reloptions.c | 20 ++++++++ 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 | 62 +++++++++++++---------- src/include/nodes/parsenodes.h | 3 ++ src/include/utils/rel.h | 21 +++++++- 11 files changed, 84 insertions(+), 29 deletions(-) diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c index b5602f5323..2882a383bd 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -489,6 +489,13 @@ relopt_enum_elt_def gistBufferingOptValues[] = {(const char *) NULL} /* list terminator */ }; +/* values from ViewOptSecurity */ +relopt_enum_elt_def viewSecurityOptValues[] = +{ + {"relation_permissions", VIEW_OPTION_SECURITY_RELATION_PERMISSIONS}, + {(const char *) NULL} /* list terminator */ +}; + /* values from ViewOptCheckOption */ relopt_enum_elt_def viewCheckOptValues[] = { @@ -522,6 +529,17 @@ static relopt_enum enumRelOpts[] = GIST_OPTION_BUFFERING_AUTO, gettext_noop("Valid values are \"on\", \"off\", and \"auto\".") }, + { + { + "security", + "View has security option defined (only relation_permissions).", + RELOPT_KIND_VIEW, + AccessExclusiveLock + }, + viewSecurityOptValues, + VIEW_OPTION_SECURITY_NOT_SET, + gettext_noop("Only valid value is \"relation_permissions\".") + }, { { "check_option", @@ -1996,6 +2014,8 @@ view_reloptions(Datum reloptions, bool validate) static const relopt_parse_elt tab[] = { {"security_barrier", RELOPT_TYPE_BOOL, offsetof(ViewOptions, security_barrier)}, + {"security", RELOPT_TYPE_ENUM, + offsetof(ViewOptions, security_option)}, {"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..9741d63d87 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_relation_permissions); 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..d7052a96b8 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_relation_permissions); 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..d9884a6100 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_relation_permissions); 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..6ffec8a940 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_relation_permissions); 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..cbdd4cfee1 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_relation_permissions = 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..59cdc84338 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_relation_permissions = 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..ba47cabf1f 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_relation_permissions = RelationHasSecurityRelationPermissions(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..d962ff2b53 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 and that view has the security + * relopt set to relation_permissions, 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,11 @@ 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 + && RelationHasSecurityRelationPermissions(relation))) { + setRuleCheckAsUser((Node *) rule->actions, relation->rd_rel->relowner); + setRuleCheckAsUser(rule->qual, relation->rd_rel->relowner); + } if (numlocks >= maxlocks) { @@ -1163,27 +1169,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 +1200,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..921c80b0f2 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1042,6 +1042,9 @@ typedef struct RangeTblEntry Query *subquery; /* the sub-query */ bool security_barrier; /* is from security_barrier view? */ + /* Is from a view defined with the security option set? */ + bool security_relation_permissions; + /* * 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..04fda5822c 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -390,6 +390,13 @@ typedef enum ViewOptCheckOption VIEW_OPTION_CHECK_OPTION_CASCADED } ViewOptCheckOption; +/* ViewOptions->security values */ +typedef enum ViewOptSecurityOption +{ + VIEW_OPTION_SECURITY_NOT_SET, + VIEW_OPTION_SECURITY_RELATION_PERMISSIONS +} ViewOptSecurityOption; + /* * ViewOptions * Contents of rd_options for views @@ -398,7 +405,8 @@ typedef struct ViewOptions { int32 vl_len_; /* varlena header (do not touch directly!) */ bool security_barrier; - ViewOptCheckOption check_option; + ViewOptSecurityOption security_option; + ViewOptCheckOption check_option; } ViewOptions; /* @@ -411,6 +419,17 @@ typedef struct ViewOptions (relation)->rd_options ? \ ((ViewOptions *) (relation)->rd_options)->security_barrier : false) +/* + * RelationHasSecurityRelationPermissions + * Returns true if the relation is a view defined with the security option + * set to relation_permissions. Note multiple eval of argument! + */ +#define RelationHasSecurityRelationPermissions(relation) \ + (AssertMacro(relation->rd_rel->relkind == RELKIND_VIEW), \ + (relation)->rd_options && \ + ((ViewOptions *) (relation)->rd_options)->security_option == \ + VIEW_OPTION_SECURITY_RELATION_PERMISSIONS) + /* * RelationHasCheckOption * Returns true if the relation is a view defined with either the local -- 2.34.1