Re: Granting SET and ALTER SYSTE privileges for GUCs

On Wed, Dec 15, 2021 at 1:18 PM Mark Dilger
<mark(dot)dilger(at)enterprisedb(dot)com> wrote:
>
> > On Dec 15, 2021, at 10:02 AM, Joshua Brindle <joshua(dot)brindle(at)crunchydata(dot)com> wrote:
> >
> > Ah, I was actually requesting a hook where the acl check was done for
> > setting a GUC, such that we could deny setting them in a hook,
> > something that would be useful for the set_user extension
> > (github.com/pgaudit/set_user)
>
> Hmm, this seems orthogonal to the patch under discussion. This patch only adds a pg_setting_acl_aclcheck in ExecSetVariableStmt() for settings which have been explicitly granted, otherwise it works the traditional way (checking whether the setting is suset/userset). I don't think you'd get MAC support without finding a way to fire the hook for all settings, regardless of their presence in the new pg_setting_acl table. That is hard, because InvokeObjectPostAlterHook expects the classId (SettingAclRelationId) and the objectId (pg_setting_acl.oid), but you don't have those for many (most?) settings. As discussed upthread, we *do not* want to force an entry into the table for all settings, only for ones that have been explicitly granted.
>
> Do you agree? I'm happy to support MAC in this patch if can explain a simple way of doing so.

Ah, I understand now. Would it be possible to pass the
SettingAclRelationId if it exists or InvalidOid if not? That way if a
MAC implementation cares about a particular GUC it'll ensure it's in
pg_setting_acl.

I don't know if others will object to that but it seems like an okay
compromise.

> > but having a hook for grant/revoke is
> > also helpful.
>
> Yes, I see no reason to rip this out.
>