On Wed, Oct 11, 2023 at 08:26:42AM +0900, Michael Paquier wrote: > On Tue, Oct 10, 2023 at 09:12:49AM +0200, Drouvot, Bertrand wrote: > > On 10/10/23 7:58 AM, Michael Paquier wrote: > >> I was looking at v8 just before you sent this v9, and still got > >> annoyed by the extra boolean argument added to InitPostgres(). > > > > Arf, I did not look at it as I had in mind to look at it once > > this one is in. > > No problem. I'm OK to do it.
Applied 0001 for now. > I am not sure that this is necessary in the code paths of > BackgroundWorkerInitializeConnectionByOid() and > BackgroundWorkerInitializeConnection() as datallowconn is handled a > few lines down. /* flags for InitPostgres() */ #define INIT_PG_LOAD_SESSION_LIBS 0x0001 #define INIT_PG_OVERRIDE_ALLOW_CONNS 0x0002 +#define INIT_PG_BYPASS_ROLE_LOGIN 0x0004 In 0002, I am not sure that this is the best name for this new flag. There is consistency with the bgworker part, for sure, but shouldn't we name that OVERRIDE_ROLE_LOGIN instead in miscadmin.h? -- Michael
From 550019a0ec84fcde8588bb9c2ae4ab08ad4d5fa5 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Tue, 10 Oct 2023 14:56:36 +0900
Subject: [PATCH v11] 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().
---
src/include/miscadmin.h | 4 ++-
src/include/postmaster/bgworker.h | 6 ++--
src/backend/postmaster/postmaster.c | 6 ++++
src/backend/utils/init/miscinit.c | 4 +--
src/backend/utils/init/postinit.c | 6 ++--
.../modules/worker_spi/t/001_worker_spi.pl | 31 +++++++++++++++++++
src/test/modules/worker_spi/worker_spi.c | 2 ++
doc/src/sgml/bgworker.sgml | 4 ++-
8 files changed, 55 insertions(+), 8 deletions(-)
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 1cc3712c0f..e280b8192d 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);
@@ -466,6 +467,7 @@ extern PGDLLIMPORT AuxProcType MyAuxProcType;
/* flags for InitPostgres() */
#define INIT_PG_LOAD_SESSION_LIBS 0x0001
#define INIT_PG_OVERRIDE_ALLOW_CONNS 0x0002
+#define INIT_PG_BYPASS_ROLE_LOGIN 0x0004
extern void pg_split_opts(char **argv, int *argcp, const char *optstr);
extern void InitializeMaxBackends(void);
extern void InitPostgres(const char *in_dbname, Oid dboid,
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/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 3d7fec995a..b1cabbd0d6 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -5567,6 +5567,9 @@ BackgroundWorkerInitializeConnection(const char *dbname, const char *username, u
/* ignore datallowconn? */
if (flags & BGWORKER_BYPASS_ALLOWCONN)
init_flags |= INIT_PG_OVERRIDE_ALLOW_CONNS;
+ /* bypass login check? */
+ if (flags & BGWORKER_BYPASS_ROLELOGINCHECK)
+ init_flags |= INIT_PG_BYPASS_ROLE_LOGIN;
/* XXX is this the right errcode? */
if (!(worker->bgw_flags & BGWORKER_BACKEND_DATABASE_CONNECTION))
@@ -5598,6 +5601,9 @@ BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, uint32 flags)
/* ignore datallowconn? */
if (flags & BGWORKER_BYPASS_ALLOWCONN)
init_flags |= INIT_PG_OVERRIDE_ALLOW_CONNS;
+ /* bypass login check? */
+ if (flags & BGWORKER_BYPASS_ROLELOGINCHECK)
+ init_flags |= INIT_PG_BYPASS_ROLE_LOGIN;
/* XXX is this the right errcode? */
if (!(worker->bgw_flags & BGWORKER_BACKEND_DATABASE_CONNECTION))
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 449541e942..c5b91bd4af 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -684,6 +684,7 @@ BaseInit(void)
* flags:
* - INIT_PG_LOAD_SESSION_LIBS to honor [session|local]_preload_libraries.
* - INIT_PG_OVERRIDE_ALLOW_CONNS to connect despite !datallowconn.
+ * - INIT_PG_BYPASS_ROLE_LOGIN to bypass role login check.
* out_dbname: optional output parameter, see below; pass NULL if not used
*
* The database can be specified by name, using the in_dbname parameter, or by
@@ -901,7 +902,8 @@ InitPostgres(const char *in_dbname, Oid dboid,
}
else
{
- InitializeSessionUserId(username, useroid);
+ InitializeSessionUserId(username, useroid,
+ (flags & INIT_PG_BYPASS_ROLE_LOGIN) != 0);
am_superuser = superuser();
}
}
@@ -910,7 +912,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/test/modules/worker_spi/t/001_worker_spi.pl b/src/test/modules/worker_spi/t/001_worker_spi.pl
index 178962eddb..b5ac4bcaa2 100644
--- a/src/test/modules/worker_spi/t/001_worker_spi.pl
+++ b/src/test/modules/worker_spi/t/001_worker_spi.pl
@@ -131,4 +131,35 @@ ok( $node->poll_query_until(
qq[noconndb|myrole|WorkerSpiMain]),
'dynamic bgworker with BYPASS_ALLOWCONN started');
+# Check BGWORKER_BYPASS_ROLELOGINCHECK.
+# First create a role without login access.
+$node->safe_psql(
+ 'postgres', qq[
+ CREATE ROLE nologrole WITH NOLOGIN;
+ GRANT CREATE ON DATABASE mydb TO nologrole;
+]);
+my $nologrole_id = $node->safe_psql('mydb',
+ "SELECT oid FROM pg_roles where rolname = 'nologrole';");
+$log_offset = -s $node->logfile;
+
+# bgworker cannot be launched with login restriction.
+$node->psql('postgres',
+ qq[SELECT worker_spi_launch(13, $mydb_id, $nologrole_id);]);
+$node->wait_for_log(qr/role "nologrole" is not permitted to log in/,
+ $log_offset);
+
+# bgworker bypasses the login restriction, and can be launched.
+$log_offset = -s $node->logfile;
+my $worker5_pid = $node->safe_psql('mydb',
+ qq(SELECT worker_spi_launch(13, $mydb_id, $nologrole_id, '{"ROLELOGINCHECK"}');)
+);
+ok( $node->poll_query_until(
+ 'mydb',
+ qq[SELECT datname, usename, wait_event FROM pg_stat_activity
+ WHERE backend_type = 'worker_spi dynamic' AND
+ pid = $worker5_pid;],
+ 'mydb|nologrole|WorkerSpiMain'),
+ 'dynamic bgworker with BYPASS_ROLELOGINCHECK launched'
+) or die "Timed out while waiting for dynamic bgworkers to be launched";
+
done_testing();
diff --git a/src/test/modules/worker_spi/worker_spi.c b/src/test/modules/worker_spi/worker_spi.c
index 1c619d4b18..c26ffe1766 100644
--- a/src/test/modules/worker_spi/worker_spi.c
+++ b/src/test/modules/worker_spi/worker_spi.c
@@ -452,6 +452,8 @@ worker_spi_launch(PG_FUNCTION_ARGS)
if (strcmp(optname, "ALLOWCONN") == 0)
flags |= BGWORKER_BYPASS_ALLOWCONN;
+ else if (strcmp(optname, "ROLELOGINCHECK") == 0)
+ flags |= BGWORKER_BYPASS_ROLELOGINCHECK;
else
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
diff --git a/doc/src/sgml/bgworker.sgml b/doc/src/sgml/bgworker.sgml
index 9ad1146ba0..8b61cf1c55 100644
--- a/doc/src/sgml/bgworker.sgml
+++ b/doc/src/sgml/bgworker.sgml
@@ -200,7 +200,9 @@ 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 for the
+ role used 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>
--
2.42.0
signature.asc
Description: PGP signature
