Fix privilege check for SET SESSION AUTHORIZATION.

Presently, the privilege check for SET SESSION AUTHORIZATION checks
whether the original authenticated role was a superuser at
connection start time.  Even if the role loses the superuser
attribute, its existing sessions are permitted to change session
authorization to any role.

This commit modifies this privilege check to verify the original
authenticated role currently has superuser.  In the event that the
authenticated role loses superuser within a session authorization
change, the authorization change will remain in effect, which means
the user can still take advantage of the target role's privileges.
However, [RE]SET SESSION AUTHORIZATION will only permit switching
to the original authenticated role.

Author: Joseph Koshakow
Discussion: https://postgr.es/m/CAAvxfHc-HHzONQ2oXdvhFF9ayRnidPwK%2BfVBhRzaBWYYLVQL-g%40mail.gmail.com

diff --git a/doc/src/sgml/ref/set_session_auth.sgml b/doc/src/sgml/ref/set_session_auth.sgml
index f8fcafc1946e8ca7b54f6da9baeb2d59726d066e..94adab2468d90dd251e1ae5398835d716e81713d 100644 (file)
--- a/doc/src/sgml/ref/set_session_auth.sgml
+++ b/doc/src/sgml/ref/set_session_auth.sgml
@@ -51,7 +51,7 @@ RESET SESSION AUTHORIZATION
 
   <para>
    The session user identifier can be changed only if the initial session
-   user (the <firstterm>authenticated user</firstterm>) had the
+   user (the <firstterm>authenticated user</firstterm>) has the
    superuser privilege.  Otherwise, the command is accepted only if it
    specifies the authenticated user name.
   </para>

diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index 071bef6375401bb5f0a8268d1c4600cd889f2dde..a88cf5f118f0bfad83ee4d4d872a27adaba51bf6 100644 (file)
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -854,7 +854,7 @@ check_session_authorization(char **newval, void **extra, GucSource source)
     * authenticated user's superuserness is what matters.
     */
    if (roleid != GetAuthenticatedUserId() &&
-       !GetAuthenticatedUserIsSuperuser())
+       !superuser_arg(GetAuthenticatedUserId()))
    {
        if (source == PGC_S_TEST)
        {

diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 64545bc373891d7149c14963d04f3d81887ee609..1e671c560c83099f31cd11d562fed5eb23f5393d 100644 (file)
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -467,8 +467,8 @@ ChangeToDataDir(void)
  * AuthenticatedUserId is determined at connection start and never changes.
  *
  * SessionUserId is initially the same as AuthenticatedUserId, but can be
- * changed by SET SESSION AUTHORIZATION (if AuthenticatedUserIsSuperuser).
- * This is the ID reported by the SESSION_USER SQL function.
+ * changed by SET SESSION AUTHORIZATION (if AuthenticatedUserId is a
+ * superuser).  This is the ID reported by the SESSION_USER SQL function.
  *
  * OuterUserId is the current user ID in effect at the "outer level" (outside
  * any transaction or function).  This is initially the same as SessionUserId,
@@ -492,8 +492,7 @@ static Oid  OuterUserId = InvalidOid;
 static Oid CurrentUserId = InvalidOid;
 static const char *SystemUser = NULL;
 
-/* We also have to remember the superuser state of some of these levels */
-static bool AuthenticatedUserIsSuperuser = false;
+/* We also have to remember the superuser state of the session user */
 static bool SessionUserIsSuperuser = false;
 
 static int SecurityRestrictionContext = 0;
@@ -582,16 +581,6 @@ GetAuthenticatedUserId(void)
    return AuthenticatedUserId;
 }
 
-/*
- * Return whether the authenticated user was superuser at connection start.
- */
-bool
-GetAuthenticatedUserIsSuperuser(void)
-{
-   Assert(OidIsValid(AuthenticatedUserId));
-   return AuthenticatedUserIsSuperuser;
-}
-
 
 /*
  * GetUserIdAndSecContext/SetUserIdAndSecContext - get/set the current user ID
@@ -741,6 +730,7 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
    HeapTuple   roleTup;
    Form_pg_authid rform;
    char       *rname;
+   bool        is_superuser;
 
    /*
     * Don't do scans if we're bootstrapping, none of the system catalogs
@@ -780,10 +770,10 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
    rname = NameStr(rform->rolname);
 
    AuthenticatedUserId = roleid;
-   AuthenticatedUserIsSuperuser = rform->rolsuper;
+   is_superuser = rform->rolsuper;
 
    /* This sets OuterUserId/CurrentUserId too */
-   SetSessionUserId(roleid, AuthenticatedUserIsSuperuser);
+   SetSessionUserId(roleid, is_superuser);
 
    /* Also mark our PGPROC entry with the authenticated user id */
    /* (We assume this is an atomic store so no lock is needed) */
@@ -816,7 +806,7 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
         * just document that the connection limit is approximate.
         */
        if (rform->rolconnlimit >= 0 &&
-           !AuthenticatedUserIsSuperuser &&
+           !is_superuser &&
            CountUserBackends(roleid) > rform->rolconnlimit)
            ereport(FATAL,
                    (errcode(ERRCODE_TOO_MANY_CONNECTIONS),
@@ -828,7 +818,7 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
    SetConfigOption("session_authorization", rname,
                    PGC_BACKEND, PGC_S_OVERRIDE);
    SetConfigOption("is_superuser",
-                   AuthenticatedUserIsSuperuser ? "on" : "off",
+                   is_superuser ? "on" : "off",
                    PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
 
    ReleaseSysCache(roleTup);
@@ -851,8 +841,6 @@ InitializeSessionUserIdStandalone(void)
    Assert(!OidIsValid(AuthenticatedUserId));
 
    AuthenticatedUserId = BOOTSTRAP_SUPERUSERID;
-   AuthenticatedUserIsSuperuser = true;
-
    SetSessionUserId(BOOTSTRAP_SUPERUSERID, true);
 }
 

diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 11d6e6869deee3900a18ba7f2f9ceb99f14f0a01..14bd574fc24ef2e64ce75ed8432942d9108dce12 100644 (file)
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -357,7 +357,6 @@ extern Oid  GetUserId(void);
 extern Oid GetOuterUserId(void);
 extern Oid GetSessionUserId(void);
 extern Oid GetAuthenticatedUserId(void);
-extern bool GetAuthenticatedUserIsSuperuser(void);
 extern void GetUserIdAndSecContext(Oid *userid, int *sec_context);
 extern void SetUserIdAndSecContext(Oid userid, int sec_context);
 extern bool InLocalUserIdChange(void);