Re: Granting SET and ALTER SYSTE privileges for GUCs

On Wed, Mar 16, 2022 at 3:06 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com> writes:
> > On Mar 16, 2022, at 11:47 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> ... I therefore judge the
> >> hook calls added to ExecSetVariableStmt and AlterSystemSetConfigFile
> >> to be 100% useless, in fact probably counterproductive because they
> >> introduce a boatload of worries about whether the right things happen
> >> if the hook errors out or does something guc.c isn't expecting.
>
> > I think Joshua was planning to use these hooks for security purposes. The hooks are supposed to check whether the Oid is valid, and if not, still be able to make choices based on the other information. Joshua, any comment on this?
>
> It's going to be hard to do anything useful in a hook that (a) does
> not know which GUC is being assigned to and (b) cannot do catalog
> accesses for fear that we're not inside a transaction. (b), in
> particular, seems like a rather thorough API break; up to now
> ObjectPostAlter hooks could assume that catalog accesses are OK.
>

Can you elaborate on this point? This is perhaps an area where I don't
know the rules of the road, to test this hook I modified the set_user
extension, which normally uses the parse tree to figure out if someone
is trying to SET log_statement or ALTER SYSTEM log_statement and
replaced it with:

case OAT_POST_ALTER:
if (classId == SettingAclRelationId)
{
ObjectAddress object;
object.classId = SettingAclRelationId;
object.objectId = objectId;
object.objectSubId = 0;
if (strcmp(getObjectIdentity(&object),
"log_statement") == 0)
{
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("Setting %s blocked",
getObjectIdentity(&object))));
}
}

Is that inherently unsafe?