Fix improper interactions between session_authorization and role.

The SQL spec mandates that SET SESSION AUTHORIZATION implies
SET ROLE NONE.  We tried to implement that within the lowest-level
functions that manipulate these settings, but that was a bad idea.
In particular, guc.c assumes that it doesn't matter in what order
it applies GUC variable updates, but that was not the case for these
two variables.  This problem, compounded by some hackish attempts to
work around it, led to some security-grade issues:

* Rolling back a transaction that had done SET SESSION AUTHORIZATION
would revert to SET ROLE NONE, even if that had not been the previous
state, so that the effective user ID might now be different from what
it had been.

* The same for SET SESSION AUTHORIZATION in a function SET clause.

* If a parallel worker inspected current_setting('role'), it saw
"none" even when it should see something else.

Also, although the parallel worker startup code intended to cope
with the current role's pg_authid row having disappeared, its
implementation of that was incomplete so it would still fail.

Fix by fully separating the miscinit.c functions that assign
session_authorization from those that assign role.  To implement the
spec's requirement, teach set_config_option itself to perform "SET
ROLE NONE" when it sets session_authorization.  (This is undoubtedly
ugly, but the alternatives seem worse.  In particular, there's no way
to do it within assign_session_authorization without incompatible
changes in the API for GUC assign hooks.)  Also, improve
ParallelWorkerMain to directly set all the relevant user-ID variables
instead of relying on some of them to get set indirectly.  That
allows us to survive not finding the pg_authid row during worker
startup.

In v16 and earlier, this includes back-patching 9987a7bf3 which
fixed a violation of GUC coding rules: SetSessionAuthorization
is not an appropriate place to be throwing errors from.

Security: CVE-2024-10978

1 files changed, 93 insertions, 64 deletions

diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 8997193ca1c..3fc27d7255d 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -29,6 +29,7 @@
 #include <utime.h>
 
 #include "access/htup_details.h"
+#include "access/parallel.h"
 #include "catalog/pg_authid.h"
 #include "common/file_perm.h"
 #include "libpq/libpq.h"
@@ -508,7 +509,7 @@ GetOuterUserId(void)
 
 
 static void
-SetOuterUserId(Oid userid)
+SetOuterUserId(Oid userid, bool is_superuser)
 {
 	AssertState(SecurityRestrictionContext == 0);
 	AssertArg(OidIsValid(userid));
@@ -516,6 +517,11 @@ SetOuterUserId(Oid userid)
 
 	/* We force the effective user ID to match, too */
 	CurrentUserId = userid;
+
+	/* Also update the is_superuser GUC to match OuterUserId's property */
+	SetConfigOption("is_superuser",
+					is_superuser ? "on" : "off",
+					PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
 }
 
 
@@ -529,6 +535,12 @@ GetSessionUserId(void)
 	return SessionUserId;
 }
 
+bool
+GetSessionUserIsSuperuser(void)
+{
+	Assert(OidIsValid(SessionUserId));
+	return SessionUserIsSuperuser;
+}
 
 static void
 SetSessionUserId(Oid userid, bool is_superuser)
@@ -537,15 +549,11 @@ SetSessionUserId(Oid userid, bool is_superuser)
 	AssertArg(OidIsValid(userid));
 	SessionUserId = userid;
 	SessionUserIsSuperuser = is_superuser;
-	SetRoleIsActive = false;
-
-	/* We force the effective user IDs to match, too */
-	OuterUserId = userid;
-	CurrentUserId = userid;
 }
 
 /*
- * GetAuthenticatedUserId - get the authenticated user ID
+ * GetAuthenticatedUserId/SetAuthenticatedUserId - get/set the authenticated
+ * user ID
  */
 Oid
 GetAuthenticatedUserId(void)
@@ -554,6 +562,32 @@ GetAuthenticatedUserId(void)
 	return AuthenticatedUserId;
 }
 
+/*
+ * Return whether the authenticated user was superuser at connection start.
+ */
+bool
+GetAuthenticatedUserIsSuperuser(void)
+{
+	Assert(OidIsValid(AuthenticatedUserId));
+	return AuthenticatedUserIsSuperuser;
+}
+
+void
+SetAuthenticatedUserId(Oid userid, bool is_superuser)
+{
+	Assert(OidIsValid(userid));
+
+	/* call only once */
+	Assert(!OidIsValid(AuthenticatedUserId));
+
+	AuthenticatedUserId = userid;
+	AuthenticatedUserIsSuperuser = is_superuser;
+
+	/* Also mark our PGPROC entry with the authenticated user id */
+	/* (We assume this is an atomic store so no lock is needed) */
+	MyProc->roleId = userid;
+}
+
 
 /*
  * GetUserIdAndSecContext/SetUserIdAndSecContext - get/set the current user ID
@@ -699,6 +733,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
@@ -706,9 +741,6 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
 	 */
 	AssertState(!IsBootstrapProcessingMode());
 
-	/* call only once */
-	AssertState(!OidIsValid(AuthenticatedUserId));
-
 	/*
 	 * Make sure syscache entries are flushed for recent catalog changes. This
 	 * allows us to find roles that were created on-the-fly during
@@ -716,36 +748,52 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
 	 */
 	AcceptInvalidationMessages();
 
+	/*
+	 * Look up the role, either by name if that's given or by OID if not.
+	 * Normally we have to fail if we don't find it, but in parallel workers
+	 * just return without doing anything: all the critical work has been done
+	 * already.  The upshot of that is that if the role has been deleted, we
+	 * will not enforce its rolconnlimit against parallel workers anymore.
+	 */
 	if (rolename != NULL)
 	{
 		roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(rolename));
 		if (!HeapTupleIsValid(roleTup))
+		{
+			if (InitializingParallelWorker)
+				return;
 			ereport(FATAL,
 					(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
 					 errmsg("role \"%s\" does not exist", rolename)));
+		}
 	}
 	else
 	{
 		roleTup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
 		if (!HeapTupleIsValid(roleTup))
+		{
+			if (InitializingParallelWorker)
+				return;
 			ereport(FATAL,
 					(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
 					 errmsg("role with OID %u does not exist", roleid)));
+		}
 	}
 
 	rform = (Form_pg_authid) GETSTRUCT(roleTup);
 	roleid = rform->oid;
 	rname = NameStr(rform->rolname);
+	is_superuser = rform->rolsuper;
 
-	AuthenticatedUserId = roleid;
-	AuthenticatedUserIsSuperuser = rform->rolsuper;
-
-	/* This sets OuterUserId/CurrentUserId too */
-	SetSessionUserId(roleid, AuthenticatedUserIsSuperuser);
+	/* In a parallel worker, ParallelWorkerMain already set these variables */
+	if (!InitializingParallelWorker)
+	{
+		SetAuthenticatedUserId(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) */
-	MyProc->roleId = roleid;
+		/* Set SessionUserId and related variables via the GUC mechanisms */
+		SetConfigOption("session_authorization", rname,
+						PGC_BACKEND, PGC_S_OVERRIDE);
+	}
 
 	/*
 	 * These next checks are not enforced when in standalone mode, so that
@@ -774,7 +822,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),
@@ -782,13 +830,6 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
 							rname)));
 	}
 
-	/* Record username and superuser status as GUC settings too */
-	SetConfigOption("session_authorization", rname,
-					PGC_BACKEND, PGC_S_OVERRIDE);
-	SetConfigOption("is_superuser",
-					AuthenticatedUserIsSuperuser ? "on" : "off",
-					PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
-
 	ReleaseSysCache(roleTup);
 }
 
@@ -811,48 +852,39 @@ InitializeSessionUserIdStandalone(void)
 	AuthenticatedUserId = BOOTSTRAP_SUPERUSERID;
 	AuthenticatedUserIsSuperuser = true;
 
-	SetSessionUserId(BOOTSTRAP_SUPERUSERID, true);
-
 	/*
-	 * XXX This should set SetConfigOption("session_authorization"), too.
-	 * Since we don't, C code will get NULL, and current_setting() will get an
-	 * empty string.
+	 * XXX Ideally we'd do this via SetConfigOption("session_authorization"),
+	 * but we lack the role name needed to do that, and we can't fetch it
+	 * because one reason for this special case is to be able to start up even
+	 * if something's happened to the BOOTSTRAP_SUPERUSERID's pg_authid row.
+	 * Since we don't set the GUC itself, C code will see the value as NULL,
+	 * and current_setting() will report an empty string within this session.
 	 */
-	SetConfigOption("is_superuser", "on",
-					PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+	SetSessionAuthorization(BOOTSTRAP_SUPERUSERID, true);
+
+	/* We could do SetConfigOption("role"), but let's be consistent */
+	SetCurrentRoleId(InvalidOid, false);
 }
 
 
 /*
  * Change session auth ID while running
  *
- * Only a superuser may set auth ID to something other than himself.  Note
- * that in case of multiple SETs in a single session, the original userid's
- * superuserness is what matters.  But we set the GUC variable is_superuser
- * to indicate whether the *current* session userid is a superuser.
- *
- * Note: this is not an especially clean place to do the permission check.
- * It's OK because the check does not require catalog access and can't
- * fail during an end-of-transaction GUC reversion, but we may someday
- * have to push it up into assign_session_authorization.
+ * The SQL standard says that SET SESSION AUTHORIZATION implies SET ROLE NONE.
+ * We mechanize that at higher levels not here, because this is the GUC
+ * assign hook for "session_authorization", and it must be commutative with
+ * SetCurrentRoleId (the hook for "role") because guc.c provides no guarantees
+ * which will run first during cases such as transaction rollback.  Therefore,
+ * we update derived state (OuterUserId/CurrentUserId/is_superuser) only if
+ * !SetRoleIsActive.
  */
 void
 SetSessionAuthorization(Oid userid, bool is_superuser)
 {
-	/* Must have authenticated already, else can't make permission check */
-	AssertState(OidIsValid(AuthenticatedUserId));
-
-	if (userid != AuthenticatedUserId &&
-		!AuthenticatedUserIsSuperuser)
-		ereport(ERROR,
-				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("permission denied to set session authorization")));
-
 	SetSessionUserId(userid, is_superuser);
 
-	SetConfigOption("is_superuser",
-					is_superuser ? "on" : "off",
-					PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+	if (!SetRoleIsActive)
+		SetOuterUserId(userid, is_superuser);
 }
 
 /*
@@ -888,28 +920,25 @@ SetCurrentRoleId(Oid roleid, bool is_superuser)
 	/*
 	 * Get correct info if it's SET ROLE NONE
 	 *
-	 * If SessionUserId hasn't been set yet, just do nothing --- the eventual
-	 * SetSessionUserId call will fix everything.  This is needed since we
-	 * will get called during GUC initialization.
+	 * If SessionUserId hasn't been set yet, do nothing beyond updating
+	 * SetRoleIsActive --- the eventual SetSessionAuthorization call will
+	 * update the derived state.  This is needed since we will get called
+	 * during GUC initialization.
 	 */
 	if (!OidIsValid(roleid))
 	{
+		SetRoleIsActive = false;
+
 		if (!OidIsValid(SessionUserId))
 			return;
 
 		roleid = SessionUserId;
 		is_superuser = SessionUserIsSuperuser;
-
-		SetRoleIsActive = false;
 	}
 	else
 		SetRoleIsActive = true;
 
-	SetOuterUserId(roleid);
-
-	SetConfigOption("is_superuser",
-					is_superuser ? "on" : "off",
-					PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+	SetOuterUserId(roleid, is_superuser);
 }