Make standard maintenance operations (including VACUUM, ANALYZE, REINDEX,
authorTom Lane <[email protected]>
Thu, 3 Jan 2008 21:24:26 +0000 (21:24 +0000)
committerTom Lane <[email protected]>
Thu, 3 Jan 2008 21:24:26 +0000 (21:24 +0000)
and CLUSTER) execute as the table owner rather than the calling user, using
the same privilege-switching mechanism already used for SECURITY DEFINER
functions.  The purpose of this change is to ensure that user-defined
functions used in index definitions cannot acquire the privileges of a
superuser account that is performing routine maintenance.  While a function
used in an index is supposed to be IMMUTABLE and thus not able to do anything
very interesting, there are several easy ways around that restriction; and
even if we could plug them all, there would remain a risk of reading sensitive
information and broadcasting it through a covert channel such as CPU usage.

To prevent bypassing this security measure, execution of SET SESSION
AUTHORIZATION and SET ROLE is now forbidden within a SECURITY DEFINER context.

Thanks to Itagaki Takahiro for reporting this vulnerability.

Security: CVE-2007-6600

13 files changed:
doc/src/sgml/ref/set_role.sgml [new file with mode: 0644]
doc/src/sgml/ref/set_session_auth.sgml
doc/src/sgml/ref/show.sgml
src/backend/access/transam/xact.c
src/backend/catalog/index.c
src/backend/commands/analyze.c
src/backend/commands/schemacmds.c
src/backend/commands/vacuum.c
src/backend/commands/variable.c
src/backend/utils/adt/ri_triggers.c
src/backend/utils/fmgr/fmgr.c
src/backend/utils/init/miscinit.c
src/include/miscadmin.h

diff --git a/doc/src/sgml/ref/set_role.sgml b/doc/src/sgml/ref/set_role.sgml
new file mode 100644 (file)
index 0000000..8a8b82e
--- /dev/null
@@ -0,0 +1,159 @@
+<!--
+$PostgreSQL$
+PostgreSQL documentation
+-->
+
+<refentry id="SQL-SET-ROLE">
+ <refmeta>
+  <refentrytitle id="sql-set-role-title">SET ROLE</refentrytitle>
+  <refmiscinfo>SQL - Language Statements</refmiscinfo>
+ </refmeta>
+
+ <refnamediv>
+  <refname>SET ROLE</refname>
+  <refpurpose>set the current user identifier of the current session</refpurpose>
+ </refnamediv>
+
+ <indexterm zone="sql-set-role">
+  <primary>SET ROLE</primary>
+ </indexterm>
+
+ <refsynopsisdiv>
+<synopsis>
+SET [ SESSION | LOCAL ] ROLE <replaceable class="parameter">rolename</replaceable>
+SET [ SESSION | LOCAL ] ROLE NONE
+RESET ROLE
+</synopsis>
+ </refsynopsisdiv>
+
+ <refsect1>
+  <title>Description</title>
+
+  <para>
+   This command sets the current user
+   identifier of the current SQL session to be <replaceable
+   class="parameter">rolename</replaceable>.  The role name may be
+   written as either an identifier or a string literal.
+   After <command>SET ROLE</>, permissions checking for SQL commands
+   is carried out as though the named role were the one that had logged
+   in originally.
+  </para>
+
+  <para>
+   The specified <replaceable class="parameter">rolename</replaceable>
+   must be a role that the current session user is a member of.
+   (If the session user is a superuser, any role can be selected.)
+  </para>
+
+  <para>
+   The <literal>SESSION</> and <literal>LOCAL</> modifiers act the same
+   as for the regular <xref linkend="SQL-SET" endterm="SQL-SET-title">
+   command.
+  </para>
+
+  <para>
+   The <literal>NONE</> and <literal>RESET</> forms reset the current
+   user identifier to be the current session user identifier.
+   These forms may be executed by any user.
+  </para>
+ </refsect1>
+
+ <refsect1>
+  <title>Notes</title>
+
+  <para>
+   Using this command, it is possible to either add privileges or restrict
+   one's privileges.  If the session user role has the <literal>INHERITS</>
+   attribute, then it automatically has all the privileges of every role that
+   it could <command>SET ROLE</> to; in this case <command>SET ROLE</>
+   effectively drops all the privileges assigned directly to the session user
+   and to the other roles it is a member of, leaving only the privileges
+   available to the named role.  On the other hand, if the session user role
+   has the <literal>NOINHERITS</> attribute, <command>SET ROLE</> drops the
+   privileges assigned directly to the session user and instead acquires the
+   privileges available to the named role.
+  </para>
+
+  <para>
+   In particular, when a superuser chooses to <command>SET ROLE</> to a
+   non-superuser role, she loses her superuser privileges.
+  </para>
+
+  <para>
+   <command>SET ROLE</> has effects comparable to
+   <xref linkend="sql-set-session-authorization"
+   endterm="sql-set-session-authorization-title">, but the privilege
+   checks involved are quite different.  Also,
+   <command>SET SESSION AUTHORIZATION</> determines which roles are
+   allowable for later <command>SET ROLE</> commands, whereas changing
+   roles with <command>SET ROLE</> does not change the set of roles
+   allowed to a later <command>SET ROLE</>.
+  </para>
+
+  <para>
+   <command>SET ROLE</> cannot be used within a
+   <literal>SECURITY DEFINER</> function.
+  </para>
+ </refsect1>
+
+ <refsect1>
+  <title>Examples</title>
+
+<programlisting>
+SELECT SESSION_USER, CURRENT_USER;
+
+ session_user | current_user 
+--------------+--------------
+ peter        | peter
+
+SET ROLE 'paul';
+
+SELECT SESSION_USER, CURRENT_USER;
+
+ session_user | current_user 
+--------------+--------------
+ peter        | paul
+</programlisting>
+ </refsect1>
+
+ <refsect1>
+  <title>Compatibility</title>
+
+  <para>
+   <productname>PostgreSQL</productname>
+   allows identifier syntax (<literal>"rolename"</literal>), while
+   the SQL standard requires the role name to be written as a string
+   literal.  SQL does not allow this command during a transaction;
+   <productname>PostgreSQL</productname> does not make this
+   restriction because there is no reason to.
+   The <literal>SESSION</> and <literal>LOCAL</> modifiers are a
+   <productname>PostgreSQL</productname> extension, as is the
+   <literal>RESET</> syntax.
+  </para>
+ </refsect1>
+
+ <refsect1>
+  <title>See Also</title>
+
+  <simplelist type="inline">
+   <member><xref linkend="sql-set-session-authorization" endterm="sql-set-session-authorization-title"></member>
+  </simplelist>
+ </refsect1>
+</refentry>
+
+<!-- Keep this comment at the end of the file
+Local variables:
+mode:sgml
+sgml-omittag:nil
+sgml-shorttag:t
+sgml-minimize-attributes:nil
+sgml-always-quote-attributes:t
+sgml-indent-step:1
+sgml-indent-data:t
+sgml-parent-document:nil
+sgml-default-dtd-file:"../reference.ced"
+sgml-exposed-tags:nil
+sgml-local-catalogs:("/usr/lib/sgml/catalog")
+sgml-local-ecat-files:nil
+End:
+-->
index 4f8ebeb0f86191df17428a5c3f8cfefbbf213714..b7a16aa37c57b8e32dced50468286232855bd590 100644 (file)
@@ -27,7 +27,7 @@ RESET SESSION AUTHORIZATION
 
   <para>
    This command sets the session user identifier and the current user
-   identifier of the current SQL-session context to be <replaceable
+   identifier of the current SQL session to be <replaceable
    class="parameter">username</replaceable>.  The user name may be
    written as either an identifier or a string literal.  Using this
    command, it is possible, for example, to temporarily become an
@@ -38,7 +38,7 @@ RESET SESSION AUTHORIZATION
    The session user identifier is initially set to be the (possibly
    authenticated) user name provided by the client.  The current user
    identifier is normally equal to the session user identifier, but
-   may change temporarily in the context of <quote>setuid</quote>
+   might change temporarily in the context of <literal>SECURITY DEFINER</>
    functions and similar mechanisms; it can also be changed by
    <xref linkend="sql-set-role" endterm="sql-set-role-title">.
    The current user identifier is relevant for permission checking.
@@ -64,6 +64,15 @@ RESET SESSION AUTHORIZATION
   </para>
  </refsect1>
 
+ <refsect1>
+  <title>Notes</title>
+
+  <para>
+   <command>SET SESSION AUTHORIZATION</> cannot be used within a
+   <literal>SECURITY DEFINER</> function.
+  </para>
+ </refsect1>
+
  <refsect1>
   <title>Examples</title>
 
index 3101b62a58c77157b07314407c6ccb2e575d60ec..4564fbcb847f37bc236815257e87232f574d5b08 100644 (file)
@@ -104,8 +104,7 @@ SHOW ALL
         <term><literal>IS_SUPERUSER</literal></term>
         <listitem>
          <para>
-          True if the current session authorization identifier has
-          superuser privileges.
+          True if the current role has superuser privileges.
          </para>
         </listitem>
        </varlistentry>
index cc8d70e978f1a8ab6b51adca6b8a73a4b8cb2f31..104f313e31886bc3d371a93c2c6bb3df57eeaf4b 100644 (file)
@@ -120,7 +120,8 @@ typedef struct TransactionStateData
        MemoryContext curTransactionContext;            /* my xact-lifetime context */
        ResourceOwner curTransactionOwner;      /* my query resources */
        List       *childXids;          /* subcommitted child XIDs */
-       Oid                     currentUser;    /* subxact start current_user */
+       Oid                     prevUser;               /* previous CurrentUserId setting */
+       bool            prevSecDefCxt;  /* previous SecurityDefinerContext setting */
        bool            prevXactReadOnly;               /* entry-time xact r/o state */
        struct TransactionStateData *parent;            /* back link to parent */
 } TransactionStateData;
@@ -152,7 +153,8 @@ static TransactionStateData TopTransactionStateData = {
        NULL,                                           /* cur transaction context */
        NULL,                                           /* cur transaction resource owner */
        NIL,                                            /* subcommitted child Xids */
-       0,                                                      /* entry-time current userid */
+       InvalidOid,                                     /* previous CurrentUserId setting */
+       false,                                          /* previous SecurityDefinerContext setting */
        false,                                          /* entry-time xact r/o state */
        NULL                                            /* link to parent state block */
 };
@@ -1373,18 +1375,14 @@ StartTransaction(void)
 
        /*
         * initialize current transaction state fields
+        *
+        * note: prevXactReadOnly is not used at the outermost level
         */
        s->nestingLevel = 1;
        s->childXids = NIL;
-
-       /*
-        * You might expect to see "s->currentUser = GetUserId();" here, but you
-        * won't because it doesn't work during startup; the userid isn't set yet
-        * during a backend's first transaction start.  We only use the
-        * currentUser field in sub-transaction state structs.
-        *
-        * prevXactReadOnly is also valid only in sub-transactions.
-        */
+       GetUserIdAndContext(&s->prevUser, &s->prevSecDefCxt);
+       /* SecurityDefinerContext should never be set outside a transaction */
+       Assert(!s->prevSecDefCxt);
 
        /*
         * initialize other subsystems for new transaction
@@ -1866,17 +1864,16 @@ AbortTransaction(void)
        AtAbort_ResourceOwner();
 
        /*
-        * Reset user id which might have been changed transiently.  We cannot use
-        * s->currentUser, since it may not be set yet; instead rely on internal
-        * state of miscinit.c.
+        * Reset user ID which might have been changed transiently.  We need this
+        * to clean up in case control escaped out of a SECURITY DEFINER function
+        * or other local change of CurrentUserId; therefore, the prior value
+        * of SecurityDefinerContext also needs to be restored.
         *
-        * (Note: it is not necessary to restore session authorization here
-        * because that can only be changed via GUC, and GUC will take care of
-        * rolling it back if need be.  However, an error within a SECURITY
-        * DEFINER function could send control here with the wrong current
-        * userid.)
+        * (Note: it is not necessary to restore session authorization or role
+        * settings here because those can only be changed via GUC, and GUC will
+        * take care of rolling them back if need be.)
         */
-       AtAbort_UserId();
+       SetUserIdAndContext(s->prevUser, s->prevSecDefCxt);
 
        /*
         * do abort processing
@@ -3716,6 +3713,12 @@ AbortSubTransaction(void)
        AtSubAbort_Memory();
        AtSubAbort_ResourceOwner();
 
+       /*
+        * Reset user ID which might have been changed transiently.  (See notes
+        * in AbortTransaction.)
+        */
+       SetUserIdAndContext(s->prevUser, s->prevSecDefCxt);
+
        /*
         * We can skip all this stuff if the subxact failed before creating a
         * ResourceOwner...
@@ -3768,20 +3771,6 @@ AbortSubTransaction(void)
                AtEOSubXact_HashTables(false, s->nestingLevel);
        }
 
-       /*
-        * Reset user id which might have been changed transiently.  Here we want
-        * to restore to the userid that was current at subxact entry. (As in
-        * AbortTransaction, we need not worry about the session userid.)
-        *
-        * Must do this after AtEOXact_GUC to handle the case where we entered the
-        * subxact inside a SECURITY DEFINER function (hence current and session
-        * userids were different) and then session auth was changed inside the
-        * subxact.  GUC will reset both current and session userids to the
-        * entry-time session userid.  This is right in every other scenario so it
-        * seems simplest to let GUC do that and fix it here.
-        */
-       SetUserId(s->currentUser);
-
        /*
         * Restore the upper transaction's read-only state, too.  This should be
         * redundant with GUC's cleanup but we may as well do it for consistency
@@ -3836,13 +3825,6 @@ PushTransaction(void)
 {
        TransactionState p = CurrentTransactionState;
        TransactionState s;
-       Oid                     currentUser;
-
-       /*
-        * At present, GetUserId cannot fail, but let's not assume that.  Get the
-        * ID before entering the critical code sequence.
-        */
-       currentUser = GetUserId();
 
        /*
         * We keep subtransaction state nodes in TopTransactionContext.
@@ -3875,7 +3857,7 @@ PushTransaction(void)
        s->savepointLevel = p->savepointLevel;
        s->state = TRANS_DEFAULT;
        s->blockState = TBLOCK_SUBBEGIN;
-       s->currentUser = currentUser;
+       GetUserIdAndContext(&s->prevUser, &s->prevSecDefCxt);
        s->prevXactReadOnly = XactReadOnly;
 
        CurrentTransactionState = s;
index f74bb41a42f048769956b030b3359bb27ff52da0..e0e73bfd8330c8932b44ab8317009c6fe0ad75bb 100644 (file)
@@ -1361,6 +1361,8 @@ index_build(Relation heapRelation,
                        IndexInfo *indexInfo)
 {
        RegProcedure procedure;
+       Oid                     save_userid;
+       bool            save_secdefcxt;
 
        /*
         * sanity checks
@@ -1371,6 +1373,13 @@ index_build(Relation heapRelation,
        procedure = indexRelation->rd_am->ambuild;
        Assert(RegProcedureIsValid(procedure));
 
+       /*
+        * Switch to the table owner's userid, so that any index functions are
+        * run as that user.
+        */
+       GetUserIdAndContext(&save_userid, &save_secdefcxt);
+       SetUserIdAndContext(heapRelation->rd_rel->relowner, true);
+
        /*
         * Call the access method's build procedure
         */
@@ -1378,6 +1387,9 @@ index_build(Relation heapRelation,
                                         PointerGetDatum(heapRelation),
                                         PointerGetDatum(indexRelation),
                                         PointerGetDatum(indexInfo));
+
+       /* Restore userid */
+       SetUserIdAndContext(save_userid, save_secdefcxt);
 }
 
 
index a8b06e177d48da73b2af3878e12ee6eafd73ae92..f5ad8f97a6b4af3ab122e0a16c5fec15954541aa 100644 (file)
@@ -112,6 +112,8 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)
        double          totalrows,
                                totaldeadrows;
        HeapTuple  *rows;
+       Oid                     save_userid;
+       bool            save_secdefcxt;
 
        if (vacstmt->verbose)
                elevel = INFO;
@@ -199,6 +201,13 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)
                                        get_namespace_name(RelationGetNamespace(onerel)),
                                        RelationGetRelationName(onerel))));
 
+       /*
+        * Switch to the table owner's userid, so that any index functions are
+        * run as that user.
+        */
+       GetUserIdAndContext(&save_userid, &save_secdefcxt);
+       SetUserIdAndContext(onerel->rd_rel->relowner, true);
+
        /*
         * Determine which columns to analyze
         *
@@ -318,9 +327,7 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)
                                                                  onerel->rd_rel->relisshared,
                                                                  0, 0);
 
-               vac_close_indexes(nindexes, Irel, AccessShareLock);
-               relation_close(onerel, AccessShareLock);
-               return;
+               goto cleanup;
        }
 
        /*
@@ -439,6 +446,9 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)
                                                          totalrows, totaldeadrows);
        }
 
+       /* We skip to here if there were no analyzable columns */
+cleanup:
+
        /* Done with indexes */
        vac_close_indexes(nindexes, Irel, NoLock);
 
@@ -448,6 +458,9 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)
         * we made in pg_statistic.)
         */
        relation_close(onerel, NoLock);
+
+       /* Restore userid */
+       SetUserIdAndContext(save_userid, save_secdefcxt);
 }
 
 /*
index 25e914e83878e7f4684c91b4923d3c0f8a4f6a49..2a336c41c6987dd4c899870d8f116d62bc1079f4 100644 (file)
@@ -44,9 +44,10 @@ CreateSchemaCommand(CreateSchemaStmt *stmt)
        ListCell   *parsetree_item;
        Oid                     owner_uid;
        Oid                     saved_uid;
+       bool            saved_secdefcxt;
        AclResult       aclresult;
 
-       saved_uid = GetUserId();
+       GetUserIdAndContext(&saved_uid, &saved_secdefcxt);
 
        /*
         * Who is supposed to own the new schema?
@@ -82,11 +83,11 @@ CreateSchemaCommand(CreateSchemaStmt *stmt)
         * temporarily set the current user so that the object(s) will be created
         * with the correct ownership.
         *
-        * (The setting will revert to session user on error or at the end of this
-        * routine.)
+        * (The setting will be restored at the end of this routine, or in case
+        * of error, transaction abort will clean things up.)
         */
        if (saved_uid != owner_uid)
-               SetUserId(owner_uid);
+               SetUserIdAndContext(owner_uid, true);
 
        /* Create the schema's namespace */
        namespaceId = NamespaceCreate(schemaName, owner_uid);
@@ -138,7 +139,7 @@ CreateSchemaCommand(CreateSchemaStmt *stmt)
        PopSpecialNamespace(namespaceId);
 
        /* Reset current user */
-       SetUserId(saved_uid);
+       SetUserIdAndContext(saved_uid, saved_secdefcxt);
 }
 
 
index 653dbb36f6f5110fe9fc8f2292da32ea4d8618fe..136f33e38326d0add9847098df37e6c27b4fa537 100644 (file)
@@ -961,6 +961,8 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
        LockRelId       onerelid;
        Oid                     toast_relid;
        bool            result;
+       Oid                     save_userid;
+       bool            save_secdefcxt;
 
        /* Begin a transaction for vacuuming this relation */
        StartTransactionCommand();
@@ -1071,6 +1073,14 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
         */
        toast_relid = onerel->rd_rel->reltoastrelid;
 
+       /*
+        * Switch to the table owner's userid, so that any index functions are
+        * run as that user.  (This is unnecessary, but harmless, for lazy
+        * VACUUM.)
+        */
+       GetUserIdAndContext(&save_userid, &save_secdefcxt);
+       SetUserIdAndContext(onerel->rd_rel->relowner, true);
+
        /*
         * Do the actual work --- either FULL or "lazy" vacuum
         */
@@ -1081,6 +1091,9 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
 
        result = true;                          /* did the vacuum */
 
+       /* Restore userid */
+       SetUserIdAndContext(save_userid, save_secdefcxt);
+
        /* all done with this class, but hold lock until commit */
        relation_close(onerel, NoLock);
 
index 36fcd0c7d36c5836813af10cef4a236c43b2a38c..0c16b8bca4cf6783bed25b15413ef4968add45cb 100644 (file)
@@ -623,6 +623,22 @@ assign_session_authorization(const char *value, bool doit, GucSource source)
                /* not a saved ID, so look it up */
                HeapTuple       roleTup;
 
+               if (InSecurityDefinerContext())
+               {
+                       /*
+                        * Disallow SET SESSION AUTHORIZATION inside a security definer
+                        * context.  We need to do this because when we exit the context,
+                        * GUC won't be notified, leaving things out of sync.  Note that
+                        * this test is positioned so that restoring a previously saved
+                        * setting isn't prevented.
+                        */
+                       if (source >= PGC_S_INTERACTIVE)
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                                errmsg("cannot set session authorization within security-definer function")));
+                       return NULL;
+               }
+
                if (!IsTransactionState())
                {
                        /*
@@ -730,6 +746,25 @@ assign_role(const char *value, bool doit, GucSource source)
                }
        }
 
+       if (roleid == InvalidOid && InSecurityDefinerContext())
+       {
+               /*
+                * Disallow SET ROLE inside a security definer context.  We need to do
+                * this because when we exit the context, GUC won't be notified,
+                * leaving things out of sync.  Note that this test is arranged so
+                * that restoring a previously saved setting isn't prevented.
+                *
+                * XXX it would be nice to allow this case in future, with the
+                * behavior being that the SET ROLE's effects end when the security
+                * definer context is exited.
+                */
+               if (source >= PGC_S_INTERACTIVE)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                        errmsg("cannot set role within security-definer function")));
+               return NULL;
+       }
+
        if (roleid == InvalidOid &&
                strcmp(actual_rolename, "none") != 0)
        {
index fad14aa97e810fad32735de4a5fe28ef112ebbab..2cef43bf1cf44198ba7dc0e8837a923735d06353 100644 (file)
@@ -3003,7 +3003,8 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes,
 {
        void       *qplan;
        Relation        query_rel;
-       Oid                     save_uid;
+       Oid                     save_userid;
+       bool            save_secdefcxt;
 
        /*
         * The query is always run against the FK table except when this is an
@@ -3017,8 +3018,8 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes,
                query_rel = fk_rel;
 
        /* Switch to proper UID to perform check as */
-       save_uid = GetUserId();
-       SetUserId(RelationGetForm(query_rel)->relowner);
+       GetUserIdAndContext(&save_userid, &save_secdefcxt);
+       SetUserIdAndContext(RelationGetForm(query_rel)->relowner, true);
 
        /* Create the plan */
        qplan = SPI_prepare(querystr, nargs, argtypes);
@@ -3027,7 +3028,7 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes,
                elog(ERROR, "SPI_prepare returned %d for %s", SPI_result, querystr);
 
        /* Restore UID */
-       SetUserId(save_uid);
+       SetUserIdAndContext(save_userid, save_secdefcxt);
 
        /* Save the plan if requested */
        if (cache_plan)
@@ -3056,7 +3057,8 @@ ri_PerformCheck(RI_QueryKey *qkey, void *qplan,
        Snapshot        crosscheck_snapshot;
        int                     limit;
        int                     spi_result;
-       Oid                     save_uid;
+       Oid                     save_userid;
+       bool            save_secdefcxt;
        Datum           vals[RI_MAX_NUMKEYS * 2];
        char            nulls[RI_MAX_NUMKEYS * 2];
 
@@ -3134,8 +3136,8 @@ ri_PerformCheck(RI_QueryKey *qkey, void *qplan,
        limit = (expect_OK == SPI_OK_SELECT) ? 1 : 0;
 
        /* Switch to proper UID to perform check as */
-       save_uid = GetUserId();
-       SetUserId(RelationGetForm(query_rel)->relowner);
+       GetUserIdAndContext(&save_userid, &save_secdefcxt);
+       SetUserIdAndContext(RelationGetForm(query_rel)->relowner, true);
 
        /* Finally we can run the query. */
        spi_result = SPI_execute_snapshot(qplan,
@@ -3144,7 +3146,7 @@ ri_PerformCheck(RI_QueryKey *qkey, void *qplan,
                                                                          false, false, limit);
 
        /* Restore UID */
-       SetUserId(save_uid);
+       SetUserIdAndContext(save_userid, save_secdefcxt);
 
        /* Check result */
        if (spi_result < 0)
index 9b91c86b1918ba9e54d060db09dedca6d4f6988e..ebf7c105151b26f7d7b86ad17cb1fdc5bc4aa6b5 100644 (file)
@@ -784,6 +784,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
        FmgrInfo   *save_flinfo;
        struct fmgr_security_definer_cache *volatile fcache;
        Oid                     save_userid;
+       bool            save_secdefcxt;
        HeapTuple       tuple;
 
        if (!fcinfo->flinfo->fn_extra)
@@ -809,26 +810,32 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
        else
                fcache = fcinfo->flinfo->fn_extra;
 
+       GetUserIdAndContext(&save_userid, &save_secdefcxt);
+       SetUserIdAndContext(fcache->userid, true);
+
+       /*
+        * We don't need to restore the userid settings on error, because the
+        * ensuing xact or subxact abort will do that.  The PG_TRY block is only
+        * needed to clean up the flinfo link.
+        */
        save_flinfo = fcinfo->flinfo;
-       save_userid = GetUserId();
 
        PG_TRY();
        {
                fcinfo->flinfo = &fcache->flinfo;
-               SetUserId(fcache->userid);
 
                result = FunctionCallInvoke(fcinfo);
        }
        PG_CATCH();
        {
                fcinfo->flinfo = save_flinfo;
-               SetUserId(save_userid);
                PG_RE_THROW();
        }
        PG_END_TRY();
 
        fcinfo->flinfo = save_flinfo;
-       SetUserId(save_userid);
+
+       SetUserIdAndContext(save_userid, save_secdefcxt);
 
        return result;
 }
index caf915384e4e0b85a51d77452ed7dbd4ff30e46e..924f47621ec24bd18462c1cb077db22e2f0ebbf8 100644 (file)
@@ -286,13 +286,15 @@ make_absolute_path(const char *path)
  * OuterUserId is the current user ID in effect at the "outer level" (outside
  * any transaction or function).  This is initially the same as SessionUserId,
  * but can be changed by SET ROLE to any role that SessionUserId is a
- * member of.  We store this mainly so that AtAbort_UserId knows what to
- * reset CurrentUserId to.
+ * member of.  (XXX rename to something like CurrentRoleId?)
  *
  * CurrentUserId is the current effective user ID; this is the one to use
  * for all normal permissions-checking purposes.  At outer level this will
  * be the same as OuterUserId, but it changes during calls to SECURITY
  * DEFINER functions, as well as locally in some specialized commands.
+ *
+ * SecurityDefinerContext is TRUE if we are within a SECURITY DEFINER function
+ * or another context that temporarily changes CurrentUserId.
  * ----------------------------------------------------------------
  */
 static Oid     AuthenticatedUserId = InvalidOid;
@@ -304,12 +306,16 @@ static Oid        CurrentUserId = InvalidOid;
 static bool AuthenticatedUserIsSuperuser = false;
 static bool SessionUserIsSuperuser = false;
 
+static bool SecurityDefinerContext = false;
+
 /* We also remember if a SET ROLE is currently active */
 static bool SetRoleIsActive = false;
 
 
 /*
- * GetUserId/SetUserId - get/set the current effective user ID.
+ * GetUserId - get the current effective user ID.
+ *
+ * Note: there's no SetUserId() anymore; use SetUserIdAndContext().
  */
 Oid
 GetUserId(void)
@@ -319,14 +325,6 @@ GetUserId(void)
 }
 
 
-void
-SetUserId(Oid userid)
-{
-       AssertArg(OidIsValid(userid));
-       CurrentUserId = userid;
-}
-
-
 /*
  * GetOuterUserId/SetOuterUserId - get/set the outer-level user ID.
  */
@@ -341,6 +339,7 @@ GetOuterUserId(void)
 static void
 SetOuterUserId(Oid userid)
 {
+       AssertState(!SecurityDefinerContext);
        AssertArg(OidIsValid(userid));
        OuterUserId = userid;
 
@@ -363,6 +362,7 @@ GetSessionUserId(void)
 static void
 SetSessionUserId(Oid userid, bool is_superuser)
 {
+       AssertState(!SecurityDefinerContext);
        AssertArg(OidIsValid(userid));
        SessionUserId = userid;
        SessionUserIsSuperuser = is_superuser;
@@ -374,6 +374,44 @@ SetSessionUserId(Oid userid, bool is_superuser)
 }
 
 
+/*
+ * GetUserIdAndContext/SetUserIdAndContext - get/set the current user ID
+ * and the SecurityDefinerContext flag.
+ *
+ * Unlike GetUserId, GetUserIdAndContext does *not* Assert that the current
+ * value of CurrentUserId is valid; nor does SetUserIdAndContext require
+ * the new value to be valid.  In fact, these routines had better not
+ * ever throw any kind of error.  This is because they are used by
+ * StartTransaction and AbortTransaction to save/restore the settings,
+ * and during the first transaction within a backend, the value to be saved
+ * and perhaps restored is indeed invalid.  We have to be able to get
+ * through AbortTransaction without asserting in case InitPostgres fails.
+ */
+void
+GetUserIdAndContext(Oid *userid, bool *sec_def_context)
+{
+       *userid = CurrentUserId;
+       *sec_def_context = SecurityDefinerContext;
+}
+
+void
+SetUserIdAndContext(Oid userid, bool sec_def_context)
+{
+       CurrentUserId = userid;
+       SecurityDefinerContext = sec_def_context;
+}
+
+
+/*
+ * InSecurityDefinerContext - are we inside a SECURITY DEFINER context?
+ */
+bool
+InSecurityDefinerContext(void)
+{
+       return SecurityDefinerContext;
+}
+
+
 /*
  * Initialize user identity during normal backend startup
  */
@@ -496,21 +534,6 @@ InitializeSessionUserIdStandalone(void)
 }
 
 
-/*
- * Reset effective userid during AbortTransaction
- *
- * This is essentially SetUserId(GetOuterUserId()), but without the Asserts.
- * The reason is that if a backend's InitPostgres transaction fails (eg,
- * because an invalid user name was given), we have to be able to get through
- * AbortTransaction without asserting.
- */
-void
-AtAbort_UserId(void)
-{
-       CurrentUserId = OuterUserId;
-}
-
-
 /*
  * Change session auth ID while running
  *
index 63a329ec3e6b0b727da47296731bb4f31a1b0be0..d512e68f049bff03dbde42bbed1f7ad80974d615 100644 (file)
@@ -229,12 +229,13 @@ extern void SetDatabasePath(const char *path);
 
 extern char *GetUserNameFromId(Oid roleid);
 extern Oid     GetUserId(void);
-extern void SetUserId(Oid userid);
 extern Oid     GetOuterUserId(void);
 extern Oid     GetSessionUserId(void);
+extern void GetUserIdAndContext(Oid *userid, bool *sec_def_context);
+extern void SetUserIdAndContext(Oid userid, bool sec_def_context);
+extern bool InSecurityDefinerContext(void);
 extern void InitializeSessionUserId(const char *rolename);
 extern void InitializeSessionUserIdStandalone(void);
-extern void AtAbort_UserId(void);
 extern void SetSessionAuthorization(Oid userid, bool is_superuser);
 extern Oid     GetCurrentRoleId(void);
 extern void SetCurrentRoleId(Oid roleid, bool is_superuser);