Re: pg_parameter_aclcheck() and trusted extensions

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Nathan Bossart <nathandbossart(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pg_parameter_aclcheck() and trusted extensions
Date: 2022-07-14 20:02:30
Message-ID: 3126292.1657828950@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> Michael Paquier <michael(at)paquier(dot)xyz> writes:
>> Looks like a bug to me, so I have added an open item assigned to Tom.

> Yeah. So the fix here seems pretty obvious: rather than applying the
> permissions check using bare GetUserId(), we need to remember the role
> OID that originally applied the setting, and use that.

Here's a draft patch for that. I initially ran around and changed all
the set_config_option callers as I threatened before, but as I did it
I could not help observing that they were all changing in exactly the
same way: basically, they were passing GetUserId() if the GucContext
is PGC_S_SESSION and BOOTSTRAP_SUPERUSERID otherwise. Not counting
guc.c internal call sites, there is a grand total of one caller that
fails to fit the pattern. So that brought me around to liking the idea
of keeping set_config_option's API stable by making it a thin wrapper
around another function with an explicit role argument. The result,
attached, poses far less of an API/ABI hazard than I was anticipating.
If you're not poking into the GUC tables you have little to fear.

Most of the bulk of this is mechanical changes to pass the source
role around properly in guc.c's data structures. That's all basically
copy-and-paste from the code to track the source context (scontext).

I noted something that ought to be looked at separately:
validate_option_array_item() seems like it needs to be taught about
grantable permissions on GUCs. I think that right now it may report
permissions failures in some cases where it should succeed.

regards, tom lane

Attachment Content-Type Size
fix-guc-permissions-checking-1.patch text/x-diff 25.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2022-07-14 20:04:23 Re: [RFC] building postgres with meson -v9
Previous Message Bruce Momjian 2022-07-14 19:44:48 Re: doc: Clarify Savepoint Behavior