Re: pg_auth_members.grantor is bunk

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_auth_members.grantor is bunk
Date: 2022-06-06 23:41:02
Message-ID: 20220606234102.GB9030@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Greetings,

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Thu, Jun 2, 2022 at 3:51 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > > I sort of thought http://postgr.es/m/3981966.1646429663@sss.pgh.pa.us
> > > constituted a completed investigation of this sort. No?
> >
> > I didn't think so. It's clear that the spec expects us to track the
> > grantor, but I didn't chase down what it expects us to *do* with that
> > information, nor what it thinks the rules are for merging multiple
> > authorizations.
>
> Hmm, OK. Well, one problem is that I've never had any luck
> interpreting what the spec says about anything, and I've sort of given
> up. But even if that were not so, I'm a little unclear what other
> conclusion is possible here. The spec either wants the same behavior
> that we already have for other object types, which is what I am here
> proposing that we do, or it wants something different. If it wants
> something different, it probably wants that for all object types, not
> just roles. Since I doubt we would want the behavior for roles to be
> inconsistent with what we do for all other object types, in that case
> we would probably either change the behavior for all other object
> types to something new, and then clean up the role stuff afterwards,
> or else first do what I proposed here and then later change it all at
> once. In which case the proposal that I've made is as good a way to
> start as any.
>
> Now, if it happens to be the case that the spec proposes a different
> behavior for roles than for non-role objects, and if the behavior for
> roles is something other than the only we currently have for non-role
> objects, then I'd agree that the plan I propose here needs revision. I
> suspect that's unlikely but I can't make anything of the spec so ....
> maybe?

Thankfully, at least from my reading, the spec isn't all that
complicated on this particular point. The spec talks about "role
authorization descriptor"s and those are "created with role name,
grantee, and grantor" and then further says "redundant duplicate role
authorization descriptors are destroyed", presumably meaning that the
entire thing has to be identical. In other words, yeah, the PK should
include the grantor. There's a further comment that the 'set of
involved grantees' is the union of all the 'grantees', clearly
indicating that you can have multiple GRANT 'foo' to 'bar's with
distinct grantees.

In terms of how that's then used, yeah, it's during REVOKE because a
REVOKE is only able to 'find' role authorization descriptors which match
the triple of role revoked, grantee, grantor (though there's a caveat in
that the 'grantor' role could be the current role, or the current user).

Interestingly, at least in my looking it over today, it doesn't seem
that the 'grantor' could be 'any applicable role' (which is what's
usually used to indicate that it could be any role that the current role
inherits), meaning you have to include the GRANTED BY in the REVOKE
statement or do a SET ROLE first when doing a REVOKE if it's for a role
that you aren't currently running as (but which you are a member of).

Anyhow, in other words, I do think Robert's got it right here. Happy to
discuss further though if there are doubts.

Thanks,

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Nasby 2022-06-07 00:10:37 Re: Collation version tracking for macOS
Previous Message David Rowley 2022-06-06 23:36:55 Re: Reducing Memory Consumption (aset and generation)