public inbox for [email protected]
help / color / mirror / Atom feedFrom: Drouvot, Bertrand <[email protected]>
To: Michael Paquier <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Add a new BGWORKER_BYPASS_ROLELOGINCHECK flag
Date: Mon, 2 Oct 2023 10:01:04 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
Hi,
On 9/29/23 8:19 AM, Michael Paquier wrote:
> On Thu, Sep 28, 2023 at 02:37:02PM +0200, Drouvot, Bertrand wrote:
>> This patch allows the role provided in BackgroundWorkerInitializeConnection()
>> and BackgroundWorkerInitializeConnectionByOid() to lack login authorization.
>
> Interesting. Yes, there would be use cases for that, I suppose.
Thanks for looking at it!
>
>> + uint32 flags,
>> char *out_dbname)
>> {
>
> This may be more adapted with a bits32 for the flags.
Done that way in v2 attached.
>
>> +# 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();
>
> It would be cheaper to use a dynamic background worker for such tests.
> Something that I've been tempted to do in this module is to extend the
> amount of data that's given to bgw_main_arg when launching a worker
> with worker_spi_launch(). How about extending the SQL function so as
> it is possible to give in input a role name (or a regrole), a database
> name (or a database OID) and a text[] for the flags? This would
> require a bit more refactoring, but this would be benefitial to show
> or one can pass down a full structure from the registration to the
> main() routine. On top of that, it would make the addition of the new
> GUCs worker_spi.bypass_login_check and worker_spi.role unnecessary.
>
I think that would make sense to have more flexibility in the worker_spi
module. I think that could be done in a dedicated patch though. I think it makes
more sense to have the current patch "focusing" on this new flag (while adding a test
about it without too much refactoring). What about doing the worker_spi module
re-factoring as a follow up of this one?
>> +# return the size of logfile of $node in bytes
>> +sub get_log_size
>> +{
>> + my ($node) = @_;
>> +
>> + return (stat $node->logfile)[7];
>> +}
>
> Just use -s here.
Done in v2 attached.
> See other tests that want to check the contents of
> the logs from an offset.
>
Oh right, worth to modify 019_replslot_limit.pl, 002_corrupted.pl and
001_pg_controldata.pl in a separate patch for consistency? (they are using
"(stat $node->logfile)[7]" or "(stat($pg_control))[7]").
>> - * 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
>
> The structure of the patch is inconsistent. These flags are in
> bgworker.h, but they are used also by InitPostgres(). Perhaps a
> second boolean flag would be OK rather than a second set of flags for
> InitPostgres() mapping with the bgworker set.
I did not want initially to add an extra parameter to InitPostgres() but
I agree it would make more sense. So done that way in v2.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From 59de41f3c305b009babfa6d05c054d8ae9e4d730 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Wed, 20 Sep 2023 08:28:59 +0000
Subject: [PATCH v2] 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 | 5 +--
src/backend/postmaster/postmaster.c | 6 ++-
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 | 10 +++--
.../modules/worker_spi/t/001_worker_spi.pl | 40 +++++++++++++++++++
src/test/modules/worker_spi/worker_spi.c | 25 +++++++++++-
11 files changed, 88 insertions(+), 17 deletions(-)
7.6% doc/src/sgml/
11.4% src/backend/postmaster/
12.6% src/backend/utils/init/
11.8% src/include/postmaster/
29.2% src/test/modules/worker_spi/t/
23.2% 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..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 54e9bfb8c4..37f5b94469 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -5575,7 +5575,7 @@ MaxLivePostmasterChildren(void)
* Connect background worker to a database.
*/
void
-BackgroundWorkerInitializeConnection(const char *dbname, const char *username, uint32 flags)
+BackgroundWorkerInitializeConnection(const char *dbname, const char *username, bits32 flags)
{
BackgroundWorker *worker = MyBgworkerEntry;
@@ -5589,6 +5589,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 */
@@ -5602,7 +5603,7 @@ BackgroundWorkerInitializeConnection(const char *dbname, const char *username, u
* Connect background worker to a database using OIDs.
*/
void
-BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, uint32 flags)
+BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, bits32 flags)
{
BackgroundWorker *worker = MyBgworkerEntry;
@@ -5616,6 +5617,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..0fdbfaf431 100644
--- a/src/include/postmaster/bgworker.h
+++ b/src/include/postmaster/bgworker.h
@@ -141,18 +141,20 @@ extern PGDLLIMPORT BackgroundWorker *MyBgworkerEntry;
* If dbname is NULL, connection is made to no specific database;
* only shared catalogs can be accessed.
*/
-extern void BackgroundWorkerInitializeConnection(const char *dbname, const char *username, uint32 flags);
+extern void BackgroundWorkerInitializeConnection(const char *dbname, const char *username, bits32 flags);
/* Just like the above, but specifying database and user by OID. */
-extern void BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, uint32 flags);
+extern void BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, bits32 flags);
/*
* 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..dcf37606a4 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,44 @@ 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 = -s $node->logfile;
+
+# 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 = -s $node->logfile;
+
+# 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();
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
Attachments:
[text/plain] v2-0001-Allow-background-workers-to-bypass-login-check.patch (13.6K, ../[email protected]/2-v2-0001-Allow-background-workers-to-bypass-login-check.patch)
download | inline diff:
From 59de41f3c305b009babfa6d05c054d8ae9e4d730 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Wed, 20 Sep 2023 08:28:59 +0000
Subject: [PATCH v2] 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 | 5 +--
src/backend/postmaster/postmaster.c | 6 ++-
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 | 10 +++--
.../modules/worker_spi/t/001_worker_spi.pl | 40 +++++++++++++++++++
src/test/modules/worker_spi/worker_spi.c | 25 +++++++++++-
11 files changed, 88 insertions(+), 17 deletions(-)
7.6% doc/src/sgml/
11.4% src/backend/postmaster/
12.6% src/backend/utils/init/
11.8% src/include/postmaster/
29.2% src/test/modules/worker_spi/t/
23.2% 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..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 54e9bfb8c4..37f5b94469 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -5575,7 +5575,7 @@ MaxLivePostmasterChildren(void)
* Connect background worker to a database.
*/
void
-BackgroundWorkerInitializeConnection(const char *dbname, const char *username, uint32 flags)
+BackgroundWorkerInitializeConnection(const char *dbname, const char *username, bits32 flags)
{
BackgroundWorker *worker = MyBgworkerEntry;
@@ -5589,6 +5589,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 */
@@ -5602,7 +5603,7 @@ BackgroundWorkerInitializeConnection(const char *dbname, const char *username, u
* Connect background worker to a database using OIDs.
*/
void
-BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, uint32 flags)
+BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, bits32 flags)
{
BackgroundWorker *worker = MyBgworkerEntry;
@@ -5616,6 +5617,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..0fdbfaf431 100644
--- a/src/include/postmaster/bgworker.h
+++ b/src/include/postmaster/bgworker.h
@@ -141,18 +141,20 @@ extern PGDLLIMPORT BackgroundWorker *MyBgworkerEntry;
* If dbname is NULL, connection is made to no specific database;
* only shared catalogs can be accessed.
*/
-extern void BackgroundWorkerInitializeConnection(const char *dbname, const char *username, uint32 flags);
+extern void BackgroundWorkerInitializeConnection(const char *dbname, const char *username, bits32 flags);
/* Just like the above, but specifying database and user by OID. */
-extern void BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, uint32 flags);
+extern void BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, bits32 flags);
/*
* 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..dcf37606a4 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,44 @@ 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 = -s $node->logfile;
+
+# 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 = -s $node->logfile;
+
+# 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();
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
view thread (8+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: Add a new BGWORKER_BYPASS_ROLELOGINCHECK flag
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox