On Wed, Dec 15, 2021 at 1:18 PM Mark Dilger <mark.dil...@enterprisedb.com> wrote: > > > On Dec 15, 2021, at 10:02 AM, Joshua Brindle > > <joshua.brin...@crunchydata.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. >
