From 7d54a90606a35571151db433df94976e2024c205 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Sun, 9 Jul 2023 15:01:53 -0700
Subject: [PATCH v6 3/3] Prevent non-superusers from altering session auth

Previously, if a user connected with as a role that had the superuser
attribute, then they could always execute a SET SESSION AUTHORIZATION
statement for the duration of their session. Even if the role was
altered to set superuser to false, the user was still allowed to
execute SET SESSION AUTHORIZATION. This allowed them to set their
session role to some other superuser and effectively regain the
superuser privileges. They could even reset their own superuser
attribute to true.

This commit alters the privilege checks for SET SESSION AUTHORIZATION
such that a user can only execute it if the role they connected with is
currently a superuser. This prevents users from regaining superuser
privileges after it has been revoked.

Author: Joseph Koshakow
Discussion: https://postgr.es/m/CAAvxfHc-HHzONQ2oXdvhFF9ayRnidPwK%2BfVBhRzaBWYYLVQL-g%40mail.gmail.com
---
 doc/src/sgml/ref/set_session_auth.sgml |  2 +-
 src/backend/commands/variable.c        |  2 +-
 src/backend/utils/init/miscinit.c      | 28 ++++++++------------------
 src/include/miscadmin.h                |  1 -
 4 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/doc/src/sgml/ref/set_session_auth.sgml b/doc/src/sgml/ref/set_session_auth.sgml
index f8fcafc194..94adab2468 100644
--- 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 b8a75012a5..8404818d09 100644
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -852,7 +852,7 @@ check_session_authorization(char **newval, void **extra, GucSource source)
 	 * original 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 64545bc373..1e671c560c 100644
--- 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 11d6e6869d..14bd574fc2 100644
--- 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);
-- 
2.25.1

