Remove the SECURITY_ROW_LEVEL_DISABLED security context bit.

This commit's parent made superfluous the bit's sole usage.  Referential
integrity checks have long run as the subject table's owner, and that
now implies RLS bypass.  Safe use of the bit was tricky, requiring
strict control over the SQL expressions evaluating therein.  Back-patch
to 9.5, where the bit was introduced.

Based on a patch by Stephen Frost.

diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 61edde9c5d35a4ccdae954d7e25329d208ab15e8..fc1c77f072f2f6f4d0222da36f8bab3c4b23ac88 100644 (file)
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -2970,7 +2970,6 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes,
        Relation        query_rel;
        Oid                     save_userid;
        int                     save_sec_context;
-       int                     temp_sec_context;
 
        /*
         * Use the query type code to determine whether the query is run against
@@ -2983,22 +2982,8 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes,
 
        /* Switch to proper UID to perform check as */
        GetUserIdAndSecContext(&save_userid, &save_sec_context);
-
-       /*
-        * Row-level security should be disabled in the case where a foreign-key
-        * relation is queried to check existence of tuples that references the
-        * primary-key being modified.
-        */
-       temp_sec_context = save_sec_context | SECURITY_LOCAL_USERID_CHANGE;
-       if (qkey->constr_queryno == RI_PLAN_CHECK_LOOKUPPK
-               || qkey->constr_queryno == RI_PLAN_CHECK_LOOKUPPK_FROM_PK
-               || qkey->constr_queryno == RI_PLAN_RESTRICT_DEL_CHECKREF
-               || qkey->constr_queryno == RI_PLAN_RESTRICT_UPD_CHECKREF)
-               temp_sec_context |= SECURITY_ROW_LEVEL_DISABLED;
-
-
        SetUserIdAndSecContext(RelationGetForm(query_rel)->relowner,
-                                                  temp_sec_context);
+                                                  save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
 
        /* Create the plan */
        qplan = SPI_prepare(querystr, nargs, argtypes);

diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 525794fb64450407504bab67e2da2da655a44eb8..1be20fcd9aa72df2639e6ea3b958d9503094f31c 100644 (file)
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -204,7 +204,6 @@ CreateCachedPlan(Node *raw_parse_tree,
        plansource->total_custom_cost = 0;
        plansource->num_custom_plans = 0;
        plansource->hasRowSecurity = false;
-       plansource->rowSecurityDisabled = InRowLevelSecurityDisabled();
        plansource->row_security_env = row_security;
        plansource->planUserId = InvalidOid;
 
@@ -601,17 +600,10 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
        }
 
        /*
-        * Check if row security is enabled for this query and things have changed
-        * such that we need to invalidate this plan and rebuild it.  Note that if
-        * row security was explicitly disabled (eg: this is a FK check plan) then
-        * we don't invalidate due to RLS.
-        *
-        * Otherwise, if the plan has a possible RLS dependency, force a replan if
-        * either the role under which the plan was planned or the row_security
-        * setting has been changed.
+        * If the plan has a possible RLS dependency, force a replan if either the
+        * role or the row_security setting has changed.
         */
        if (plansource->is_valid
-               && !plansource->rowSecurityDisabled
                && plansource->hasRowSecurity
                && (plansource->planUserId != GetUserId()
                        || plansource->row_security_env != row_security))

diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 5bf595c9e5fe4249454f6a574574ea9647828a6e..f0099d31fad0b482a9df7632173fef94b928db91 100644 (file)
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -341,7 +341,7 @@ GetAuthenticatedUserId(void)
  * GetUserIdAndSecContext/SetUserIdAndSecContext - get/set the current user ID
  * and the SecurityRestrictionContext flags.
  *
- * Currently there are three valid bits in SecurityRestrictionContext:
+ * Currently there are two valid bits in SecurityRestrictionContext:
  *
  * SECURITY_LOCAL_USERID_CHANGE indicates that we are inside an operation
  * that is temporarily changing CurrentUserId via these functions.  This is
@@ -359,9 +359,6 @@ GetAuthenticatedUserId(void)
  * where the called functions are really supposed to be side-effect-free
  * anyway, such as VACUUM/ANALYZE/REINDEX.
  *
- * SECURITY_ROW_LEVEL_DISABLED indicates that we are inside an operation that
- * needs to bypass row level security checks, for example FK checks.
- *
  * Unlike GetUserId, GetUserIdAndSecContext does *not* Assert that the current
  * value of CurrentUserId is valid; nor does SetUserIdAndSecContext require
  * the new value to be valid.  In fact, these routines had better not
@@ -404,15 +401,6 @@ InSecurityRestrictedOperation(void)
        return (SecurityRestrictionContext & SECURITY_RESTRICTED_OPERATION) != 0;
 }
 
-/*
- * InRowLevelSecurityDisabled - are we inside a RLS-disabled operation?
- */
-bool
-InRowLevelSecurityDisabled(void)
-{
-       return (SecurityRestrictionContext & SECURITY_ROW_LEVEL_DISABLED) != 0;
-}
-
 
 /*
  * These are obsolete versions of Get/SetUserIdAndSecContext that are

diff --git a/src/backend/utils/misc/rls.c b/src/backend/utils/misc/rls.c
index abaf344506801cbdde3c74608d7558fb77f2f8ef..c900c98848b98b199b8cde50c5b0a7467072b59d 100644 (file)
--- a/src/backend/utils/misc/rls.c
+++ b/src/backend/utils/misc/rls.c
@@ -63,13 +63,6 @@ check_enable_rls(Oid relid, Oid checkAsUser, bool noError)
        if (relid < FirstNormalObjectId)
                return RLS_NONE;
 
-       /*
-        * Check if we have been told to explicitly skip RLS (perhaps because this
-        * is a foreign key check)
-        */
-       if (InRowLevelSecurityDisabled())
-               return RLS_NONE;
-
        tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
        if (!HeapTupleIsValid(tuple))
                return RLS_NONE;

diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index e0cc69f27ef1e39f182abb0796490262940c02eb..80ac7329dcea18744fb67b0bbcfed3d0158c2a33 100644 (file)
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -286,7 +286,6 @@ extern int  trace_recovery(int trace_level);
 /* flags to be OR'd to form sec_context */
 #define SECURITY_LOCAL_USERID_CHANGE   0x0001
 #define SECURITY_RESTRICTED_OPERATION  0x0002
-#define SECURITY_ROW_LEVEL_DISABLED            0x0004
 
 extern char *DatabasePath;
 
@@ -305,7 +304,6 @@ extern void GetUserIdAndSecContext(Oid *userid, int *sec_context);
 extern void SetUserIdAndSecContext(Oid userid, int sec_context);
 extern bool InLocalUserIdChange(void);
 extern bool InSecurityRestrictedOperation(void);
-extern bool InRowLevelSecurityDisabled(void);
 extern void GetUserIdAndContext(Oid *userid, bool *sec_def_context);
 extern void SetUserIdAndContext(Oid userid, bool sec_def_context);
 extern void InitializeSessionUserId(const char *rolename, Oid useroid);

diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index b683b070e3517c51d0b53157fc0121b3561d3044..4b9a0c68cee4bc8a147a65c0bb21cea881115fbb 100644 (file)
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -111,7 +111,6 @@ typedef struct CachedPlanSource
        int                     num_custom_plans;               /* number of plans included in total */
        bool            hasRowSecurity; /* planned with row security? */
        bool            row_security_env;               /* row security setting when planned */
-       bool            rowSecurityDisabled;    /* is row security disabled? */
 } CachedPlanSource;
 
 /*