Re: Add a new BGWORKER_BYPASS_ROLELOGINCHECK flag

On Mon, Oct 2, 2023 at 4:58 PM Drouvot, Bertrand
<bertranddrouvot.pg@gmail.com> wrote:
>
> 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.

Correct. It allows the roles that don't have LOGIN capabilities to
start and use bg workers.

> > This may be more adapted with a bits32 for the flags.
>
> Done that way in v2 attached.

While I like the idea of the flag to skip login checks for bg workers,
I don't quite like the APIs being changes InitializeSessionUserId and
InitPostgres (adding a new input parameter),
BackgroundWorkerInitializeConnection and
BackgroundWorkerInitializeConnectionByOid (changing of input parameter
type) given that all of these functions are available for external
modules and will break things for sure.

What if BGWORKER_BYPASS_ROLELOGINCHECK be part of bgw_flags? With
this, none of the API needs to be changed, so no compatibility
problems as such for external modules and the InitializeSessionUserId
can just do something like [1]. We might be tempted to add
BGWORKER_BYPASS_ALLOWCONN also to bgw_flags, but I'd prefer not to do
it for the same compatibility reasons.

Thoughts?

[1]
diff --git a/src/backend/utils/init/miscinit.c
b/src/backend/utils/init/miscinit.c
index 1e671c560c..27dcf052ab 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -786,10 +786,17 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
         */
        if (IsUnderPostmaster)
        {
+               bool    skip_check = false;
+
+               /* If asked, skip the role login check for background
workers. */
+               if (IsBackgroundWorker &&
+                       (MyBgworkerEntry->bgw_flags &
BGWORKER_BYPASS_ROLELOGINCHECK) != 0)
+               skip_check = true;
+
                /*
                 * Is role allowed to login at all?
                 */
-               if (!rform->rolcanlogin)
+               if (!skip_check && !rform->rolcanlogin)
                        ereport(FATAL,

(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
                                         errmsg("role \"%s\" is not
permitted to log in",

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