Hi,
On 10/6/23 8:48 AM, Drouvot, Bertrand wrote:
Hi,
On 10/5/23 6:23 PM, Bharath Rupireddy wrote:
On Thu, Oct 5, 2023 at 9:32 PM Drouvot, Bertrand
<bertranddrouvot...@gmail.com> wrote:
+ CREATE ROLE nologrole with nologin;
+ GRANT CREATE ON DATABASE mydb TO nologrole;
A few nit-picks:
1. s/with/WITH
2. s/nologin/NOLOGIN
done in v8 attached.
3. + is specified as <varname>flags</varname> it is possible to
bypass the login check to connect to databases.
How about "it is possible to bypass the login check for the role used
to connect to databases."?
"for the role used" sounds implicit to me but I don't have a strong opinion
about it so re-worded as per your proposal in v8.
Please find attached v9 (v8 rebase due to f483b2090).
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From e3a75359b2f3a57d7312ff4ab3f77ae78f44f221 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 5 Oct 2023 14:06:37 +0900
Subject: [PATCH v9] 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 | 4 ++-
src/backend/bootstrap/bootstrap.c | 2 +-
src/backend/postmaster/autovacuum.c | 5 ++-
src/backend/postmaster/postmaster.c | 2 ++
src/backend/tcop/postgres.c | 1 +
src/backend/utils/init/miscinit.c | 4 +--
src/backend/utils/init/postinit.c | 5 +--
src/include/miscadmin.h | 4 ++-
src/include/postmaster/bgworker.h | 6 ++--
.../modules/worker_spi/t/001_worker_spi.pl | 36 +++++++++++++++++++
src/test/modules/worker_spi/worker_spi.c | 2 ++
11 files changed, 59 insertions(+), 12 deletions(-)
10.0% doc/src/sgml/
10.9% src/backend/postmaster/
15.4% src/backend/utils/init/
9.6% src/include/postmaster/
45.8% src/test/modules/worker_spi/t/
3.1% src/test/modules/worker_spi/
4.8% src/
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>
diff --git a/src/backend/bootstrap/bootstrap.c
b/src/backend/bootstrap/bootstrap.c
index 5810f8825e..bdd005b66a 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, false, false,
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..4686b81f68 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, false, false,
NULL);
SetProcessingMode(NormalProcessing);
@@ -1706,8 +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,
- dbname);
+ InitPostgres(NULL, dbid, NULL, InvalidOid, false, false, false,
dbname);
SetProcessingMode(NormalProcessing);
set_ps_display(dbname);
ereport(DEBUG1,
diff --git a/src/backend/postmaster/postmaster.c
b/src/backend/postmaster/postmaster.c
index 0761b38bf8..0502588013 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -5573,6 +5573,7 @@ BackgroundWorkerInitializeConnection(const char *dbname,
const char *username, u
username, InvalidOid, /* role to connect as */
false, /* never honor
session_preload_libraries */
(flags & BGWORKER_BYPASS_ALLOWCONN) != 0,
/* ignore datallowconn? */
+ (flags & BGWORKER_BYPASS_ROLELOGINCHECK) != 0,
/* bypass login check? */
NULL); /* no out_dbname */
/* it had better not gotten out of "init" mode yet */
@@ -5600,6 +5601,7 @@ BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid
useroid, uint32 flags)
NULL, useroid, /* role to connect as */
false, /* never honor
session_preload_libraries */
(flags & BGWORKER_BYPASS_ALLOWCONN) != 0,
/* ignore datallowconn? */
+ (flags & BGWORKER_BYPASS_ROLELOGINCHECK) != 0,
/* bypass login check? */
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..5c1e21b99d 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4211,6 +4211,7 @@ PostgresMain(const char *dbname, const char *username)
username, InvalidOid, /* role to connect as */
!am_walsender, /* honor
session_preload_libraries? */
false, /* don't ignore
datallowconn */
+ false, /* don't bypass login
check */
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..dcfeb30832 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -719,6 +719,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
const char *username, Oid useroid,
bool load_session_libraries,
bool override_allow_connections,
+ bool bypass_login_check,
char *out_dbname)
{
bool bootstrap = IsBootstrapProcessingMode();
@@ -901,7 +902,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
}
else
{
- InitializeSessionUserId(username, useroid);
+ InitializeSessionUserId(username, useroid,
bypass_login_check);
am_superuser = superuser();
}
}
@@ -910,7 +911,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..f1d03f2df8 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);
@@ -469,6 +470,7 @@ extern void InitPostgres(const char *in_dbname, Oid dboid,
const char *username, Oid
useroid,
bool load_session_libraries,
bool
override_allow_connections,
+ bool bypass_login_check,
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 178962eddb..02c57ca54e 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,40 @@ 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.
+my $worker5_pid = $node->safe_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);
+
+$result = $node->safe_psql('postgres',
+ "SELECT count(*) FROM pg_stat_activity WHERE pid = $worker5_pid;");
+is($result, '0',
+ 'dynamic bgworker without BYPASS_ROLELOGINCHECK not started');
+
+# bgworker bypasses the login restriction, and can be launched.
+$log_offset = -s $node->logfile;
+my $worker6_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 = $worker6_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),
--
2.34.1
