[Drouvot, Bertrand <bertranddrouvot.pg@gmail.com>]

Hi hackers,

Please find attached a patch proposal to $SUBJECT.

This patch allows the role provided in BackgroundWorkerInitializeConnection()
and BackgroundWorkerInitializeConnectionByOid() to lack login authorization.

In InitPostgres(), in case of a background worker, authentication is not performed
(PerformAuthentication() is not called), so having the role used to connect to the database
lacking login authorization seems to make sense.

With this new flag in place, one could give "high" privileges to the role used to initialize
the background workers connections without any risk of seeing this role being used by a
"normal user" to login.

The attached patch:

- adds the new flag
- adds documentation
- adds testing

Looking forward to your feedback,

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachments:

v1-0001-Allow-background-workers-to-bypass-login-check.patch (text/plain, 12.9K) download
From 82ea53a81a5e9f5e8b680ec6de849a48a7222463 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Wed, 20 Sep 2023 08:28:59 +0000
Subject: [PATCH v1] Allow background workers to bypass login check

This adds a new flag value (BGWORKER_BYPASS_ROLELOGINCHECK) to the flags being
used in BackgroundWorkerInitializeConnection() and BackgroundWorkerInitializeConnectionByOid().

This flag allows the background workers to bypass the login check done in InitializeSessionUserId().
---
 doc/src/sgml/bgworker.sgml                    |  3 +-
 src/backend/bootstrap/bootstrap.c             |  2 +-
 src/backend/postmaster/autovacuum.c           |  4 +-
 src/backend/postmaster/postmaster.c           |  4 +-
 src/backend/tcop/postgres.c                   |  2 +-
 src/backend/utils/init/miscinit.c             |  4 +-
 src/backend/utils/init/postinit.c             |  9 ++--
 src/include/miscadmin.h                       |  5 +-
 src/include/postmaster/bgworker.h             |  6 ++-
 .../modules/worker_spi/t/001_worker_spi.pl    | 48 +++++++++++++++++++
 src/test/modules/worker_spi/worker_spi.c      | 25 +++++++++-
 11 files changed, 95 insertions(+), 17 deletions(-)
   6.9% doc/src/sgml/
   3.4% src/backend/bootstrap/
  10.6% src/backend/postmaster/
  16.8% src/backend/utils/init/
   7.2% src/include/postmaster/
  29.7% src/test/modules/worker_spi/t/
  21.1% src/test/modules/worker_spi/
   4.0% src/

diff --git a/doc/src/sgml/bgworker.sgml b/doc/src/sgml/bgworker.sgml
index 9ad1146ba0..7335c02ed8 100644
--- a/doc/src/sgml/bgworker.sgml
+++ b/doc/src/sgml/bgworker.sgml
@@ -200,7 +200,8 @@ typedef struct BackgroundWorker
    <literal>InvalidOid</literal>, the process will run as the superuser created
    during <command>initdb</command>. If <literal>BGWORKER_BYPASS_ALLOWCONN</literal>
    is specified as <varname>flags</varname> it is possible to bypass the restriction
-   to connect to databases not allowing user connections.
+   to connect to databases not allowing user connections. If <literal>BGWORKER_BYPASS_ROLELOGINCHECK</literal>
+   is specified as <varname>flags</varname> it is possible to bypass the login check to connect to databases.
    A background worker can only call one of these two functions, and only
    once.  It is not possible to switch databases.
   </para>
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 5810f8825e..d4b6425585 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -345,7 +345,7 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
 	if (pg_link_canary_is_frontend())
 		elog(ERROR, "backend is incorrectly linked to frontend functions");
 
-	InitPostgres(NULL, InvalidOid, NULL, InvalidOid, false, false, NULL);
+	InitPostgres(NULL, InvalidOid, NULL, InvalidOid, false, 0, NULL);
 
 	/* Initialize stuff for bootstrap-file processing */
 	for (i = 0; i < MAXATTR; i++)
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index ae9be9b911..dde83e76fb 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -488,7 +488,7 @@ AutoVacLauncherMain(int argc, char *argv[])
 	/* Early initialization */
 	BaseInit();
 
-	InitPostgres(NULL, InvalidOid, NULL, InvalidOid, false, false, NULL);
+	InitPostgres(NULL, InvalidOid, NULL, InvalidOid, false, 0, NULL);
 
 	SetProcessingMode(NormalProcessing);
 
@@ -1706,7 +1706,7 @@ AutoVacWorkerMain(int argc, char *argv[])
 		 * Note: if we have selected a just-deleted database (due to using
 		 * stale stats info), we'll fail and exit here.
 		 */
-		InitPostgres(NULL, dbid, NULL, InvalidOid, false, false,
+		InitPostgres(NULL, dbid, NULL, InvalidOid, false, 0,
 					 dbname);
 		SetProcessingMode(NormalProcessing);
 		set_ps_display(dbname);
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 54e9bfb8c4..443b1c4725 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -5588,7 +5588,7 @@ BackgroundWorkerInitializeConnection(const char *dbname, const char *username, u
 	InitPostgres(dbname, InvalidOid,	/* database to connect to */
 				 username, InvalidOid,	/* role to connect as */
 				 false,			/* never honor session_preload_libraries */
-				 (flags & BGWORKER_BYPASS_ALLOWCONN) != 0,	/* ignore datallowconn? */
+				 flags,
 				 NULL);			/* no out_dbname */
 
 	/* it had better not gotten out of "init" mode yet */
@@ -5615,7 +5615,7 @@ BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, uint32 flags)
 	InitPostgres(NULL, dboid,	/* database to connect to */
 				 NULL, useroid, /* role to connect as */
 				 false,			/* never honor session_preload_libraries */
-				 (flags & BGWORKER_BYPASS_ALLOWCONN) != 0,	/* ignore datallowconn? */
+				 flags,
 				 NULL);			/* no out_dbname */
 
 	/* it had better not gotten out of "init" mode yet */
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 21b9763183..f5ef8f52a8 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4210,7 +4210,7 @@ PostgresMain(const char *dbname, const char *username)
 	InitPostgres(dbname, InvalidOid,	/* database to connect to */
 				 username, InvalidOid,	/* role to connect as */
 				 !am_walsender, /* honor session_preload_libraries? */
-				 false,			/* don't ignore datallowconn */
+				 0,
 				 NULL);			/* no out_dbname */
 
 	/*
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 1e671c560c..182d666852 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -725,7 +725,7 @@ has_rolreplication(Oid roleid)
  * Initialize user identity during normal backend startup
  */
 void
-InitializeSessionUserId(const char *rolename, Oid roleid)
+InitializeSessionUserId(const char *rolename, Oid roleid, bool bypass_login_check)
 {
 	HeapTuple	roleTup;
 	Form_pg_authid rform;
@@ -789,7 +789,7 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
 		/*
 		 * Is role allowed to login at all?
 		 */
-		if (!rform->rolcanlogin)
+		if (!bypass_login_check && !rform->rolcanlogin)
 			ereport(FATAL,
 					(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
 					 errmsg("role \"%s\" is not permitted to log in",
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index df4d15a50f..6ccedffd62 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -41,6 +41,7 @@
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "postmaster/autovacuum.h"
+#include "postmaster/bgworker.h"
 #include "postmaster/postmaster.h"
 #include "replication/slot.h"
 #include "replication/walsender.h"
@@ -718,10 +719,12 @@ void
 InitPostgres(const char *in_dbname, Oid dboid,
 			 const char *username, Oid useroid,
 			 bool load_session_libraries,
-			 bool override_allow_connections,
+			 uint32 flags,
 			 char *out_dbname)
 {
 	bool		bootstrap = IsBootstrapProcessingMode();
+	bool		override_allow_connections = (flags & BGWORKER_BYPASS_ALLOWCONN) != 0;
+	bool		bypass_login_check = (flags & BGWORKER_BYPASS_ROLELOGINCHECK) != 0;
 	bool		am_superuser;
 	char	   *fullpath;
 	char		dbname[NAMEDATALEN];
@@ -901,7 +904,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
 		}
 		else
 		{
-			InitializeSessionUserId(username, useroid);
+			InitializeSessionUserId(username, useroid, bypass_login_check);
 			am_superuser = superuser();
 		}
 	}
@@ -910,7 +913,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
 		/* normal multiuser case */
 		Assert(MyProcPort != NULL);
 		PerformAuthentication(MyProcPort);
-		InitializeSessionUserId(username, useroid);
+		InitializeSessionUserId(username, useroid, false);
 		/* ensure that auth_method is actually valid, aka authn_id is not NULL */
 		if (MyClientConnectionInfo.authn_id)
 			InitializeSystemUser(MyClientConnectionInfo.authn_id,
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 14bd574fc2..7e6ee415cb 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -364,7 +364,8 @@ extern bool InSecurityRestrictedOperation(void);
 extern bool InNoForceRLSOperation(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 roleid);
+extern void InitializeSessionUserId(const char *rolename, Oid roleid,
+									bool bypass_login_check);
 extern void InitializeSessionUserIdStandalone(void);
 extern void SetSessionAuthorization(Oid userid, bool is_superuser);
 extern Oid	GetCurrentRoleId(void);
@@ -468,7 +469,7 @@ extern void InitializeMaxBackends(void);
 extern void InitPostgres(const char *in_dbname, Oid dboid,
 						 const char *username, Oid useroid,
 						 bool load_session_libraries,
-						 bool override_allow_connections,
+						 uint32 flags,
 						 char *out_dbname);
 extern void BaseInit(void);
 
diff --git a/src/include/postmaster/bgworker.h b/src/include/postmaster/bgworker.h
index 7815507e3d..d7a5c1a946 100644
--- a/src/include/postmaster/bgworker.h
+++ b/src/include/postmaster/bgworker.h
@@ -150,9 +150,11 @@ extern void BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, ui
  * Flags to BackgroundWorkerInitializeConnection et al
  *
  *
- * Allow bypassing datallowconn restrictions when connecting to database
+ * Allow bypassing datallowconn restrictions and login check when connecting
+ * to database
  */
-#define BGWORKER_BYPASS_ALLOWCONN 1
+#define BGWORKER_BYPASS_ALLOWCONN 0x0001
+#define BGWORKER_BYPASS_ROLELOGINCHECK 0x0002
 
 
 /* Block/unblock signals in a background worker process */
diff --git a/src/test/modules/worker_spi/t/001_worker_spi.pl b/src/test/modules/worker_spi/t/001_worker_spi.pl
index 2965acd789..ff8b72f5b5 100644
--- a/src/test/modules/worker_spi/t/001_worker_spi.pl
+++ b/src/test/modules/worker_spi/t/001_worker_spi.pl
@@ -93,4 +93,52 @@ ok( $node->poll_query_until(
 	'dynamic bgworkers all launched'
 ) or die "Timed out while waiting for dynamic bgworkers to be launched";
 
+# Check BGWORKER_BYPASS_ROLELOGINCHECK behaves as expected.
+
+# First create a role without login.
+$node->safe_psql(
+	'postgres', qq[
+  CREATE ROLE nologrole with nologin;
+  ALTER ROLE nologrole with superuser;
+]);
+
+my $log_start = get_log_size($node);
+
+# Ask the background workers to connect with this role.
+$node->append_conf(
+	'postgresql.conf', q{
+worker_spi.role = 'nologrole'
+});
+$node->restart;
+
+# An error message should be issued.
+ok( $node->log_contains(
+		"role \"nologrole\" is not permitted to log in", $log_start),
+	"nologrole not allowed to connect if BGWORKER_BYPASS_ROLELOGINCHECK is not set"
+);
+
+
+$log_start = get_log_size($node);
+
+# Ask the background workers to connect with this role with the flag in place.
+$node->append_conf(
+	'postgresql.conf', q{
+worker_spi.role = 'nologrole'
+worker_spi.bypass_login_check = true
+});
+$node->restart;
+
+# An error message should not be issued.
+ok( !$node->log_contains(
+		"role \"nologrole\" is not permitted to log in", $log_start),
+	"nologrole allowed to connect if BGWORKER_BYPASS_ROLELOGINCHECK is set");
+
 done_testing();
+
+# return the size of logfile of $node in bytes
+sub get_log_size
+{
+	my ($node) = @_;
+
+	return (stat $node->logfile)[7];
+}
diff --git a/src/test/modules/worker_spi/worker_spi.c b/src/test/modules/worker_spi/worker_spi.c
index 98f8d4194b..d2ca99f2c5 100644
--- a/src/test/modules/worker_spi/worker_spi.c
+++ b/src/test/modules/worker_spi/worker_spi.c
@@ -52,6 +52,8 @@ PGDLLEXPORT void worker_spi_main(Datum main_arg) pg_attribute_noreturn();
 static int	worker_spi_naptime = 10;
 static int	worker_spi_total_workers = 2;
 static char *worker_spi_database = NULL;
+static char *worker_spi_role = NULL;
+static bool worker_spi_bypass_login_check = false;
 
 /* value cached, fetched from shared memory */
 static uint32 worker_spi_wait_event_main = 0;
@@ -152,7 +154,10 @@ worker_spi_main(Datum main_arg)
 	BackgroundWorkerUnblockSignals();
 
 	/* Connect to our database */
-	BackgroundWorkerInitializeConnection(worker_spi_database, NULL, 0);
+	if (worker_spi_bypass_login_check)
+		BackgroundWorkerInitializeConnection(worker_spi_database, worker_spi_role, BGWORKER_BYPASS_ROLELOGINCHECK);
+	else
+		BackgroundWorkerInitializeConnection(worker_spi_database, worker_spi_role, 0);
 
 	elog(LOG, "%s initialized with %s.%s",
 		 MyBgworkerEntry->bgw_name, table->schema, table->name);
@@ -316,6 +321,24 @@ _PG_init(void)
 							   0,
 							   NULL, NULL, NULL);
 
+	DefineCustomStringVariable("worker_spi.role",
+							   "Role to connect with.",
+							   NULL,
+							   &worker_spi_role,
+							   NULL,
+							   PGC_SIGHUP,
+							   0,
+							   NULL, NULL, NULL);
+
+	DefineCustomBoolVariable("worker_spi.bypass_login_check",
+							 "Should BGW allows to connect with non login role.",
+							 NULL,
+							 &worker_spi_bypass_login_check,
+							 false,
+							 PGC_SIGHUP,
+							 0,
+							 NULL, NULL, NULL);
+
 	if (!process_shared_preload_libraries_in_progress)
 		return;
 
-- 
2.34.1