Re: replacing role-level NOINHERIT with a grant-level option

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Joe Conway <mail(at)joeconway(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Bossart, Nathan" <bossartn(at)amazon(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: replacing role-level NOINHERIT with a grant-level option
Date: 2022-06-08 16:32:29
Message-ID: CA+TgmoZRAQb7qBji2VHaS7pGETzV3-Ecue1uMmRDyCdJsSbm=w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jun 8, 2022 at 10:16 AM Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > That's why I proposed the name SET, not MEMBERSHIP. You would still
> > get a catalog entry in pg_auth_members, so you are still a member in
> > some loose sense even if your grant has INHERIT FALSE and SET FALSE,
> > but in such a case the fact that you are a member isn't really doing
> > anything for you in terms of getting you access to privileges because
> > you're neither allowed to exercise them implicitly nor SET ROLE to the
> > role.
>
> Naming things is hard. :) I'm still not a fan of calling that option
> 'SET' and 'membership' feels like how it's typically described today
> when someone has the rights of a group (implicitly or explicitly). As
> for what to call "has a pg_auth_members row but no actual access", maybe
> 'associated'?

What I want here is two Boolean flags, one of which controls whether
you implicitly have the privileges of the granted role and the other
of which controls whether you can access those privileges via SET
ROLE. In each case, I want TRUE to be the state where you have or can
get the privileges and FALSE the state where you can't. I'm not
especially stuck on the names INHERIT and SET for those things,
although in my opinion INHERIT is pretty good and SET is where there
is, perhaps, more likely to be room for improvement. However, I don't
think ASSOCIATED is an improvement because it reverses the sense: now,
TRUE means you don't have the privileges (because you're only
associated, not actually a member, I guess) and FALSE means you do
(because you are not merely associated, but are a member). Yick. That
seems super-unintuitive.

Other alternatives to SET:

- BECOME (can you become that user?)
- ASSUME (can you assume that roles's privileges?)
- EXPLICIT (perhaps also renaming the INHERIT permission to IMPLICIT)

Honestly the main thing I don't like about SET is that it sounds like
it ought to be the action verb in the command, rather than just an
option name. But we've already crossed that Rubicon by deciding that
you can GRANT SET on a GUC, and what's the difference between that and
granting a role WITH SET? I'm actually a bit afraid that if we get
creative here it will sound inconsistent.

> That does lead me down a bit of a rabbit hole because every role in the
> entire system could be considered 'associated' with every other one and
> if the case of "no pg_auth_members row" is identical to the case of
> "pg_auth_members row with everything off/default" then it feels a bit
> odd to have an entry for it- and is there any way to get rid of that
> entry?

"everything off" and "everything default" are two very different
things. If you just do "GRANT foo TO bar" without setting any options,
then you get all the options set to their default values, and that's
going to generate a pg_auth_members entry that we certainly can't
omit. However, let's say that you then alter that grant by removing
every single privilege bit, one by one:

GRANT foo TO bar; -- normal grant
GRANT foo TO bar WITH INHERIT OFF; -- implicit inheritance of privileges removed
GRANT foo TO bar WITH SET OFF; -- explicit SET ROLE privilege now also removed
GRANT foo TO bar WITH JOIE_DE_VIVRE OFF; -- now there's nothing left

One can certainly make an argument that the last GRANT ought to just
go ahead and nuke the pg_auth_members entry entirely, and I'm willing
to do that if that's what most people want. However, I think it might
be better to leave well enough alone. In my proposal, that last GRANT
command is just adjusting the options of the existing grant, not
removing it. True, it is a pretty worthless grant, lacking even joie
de vivre, but it is a grant all the same, and it can still be dumped
and restored just fine. With this change, that last GRANT command
becomes, in effect, a REVOKE, which is a little odd. Right now, the
pg_auth_members row has no "payload data" and perhaps it never will,
but if we were storing, I don't know, the number of times the grant
had ever been used, or when it got created, or whatever, that
information would be lost when that row is deleted, and that seems
like something that the user might want to have happen only when they
explicitly asked for it.

I don't think this is a big deal either way. I've designed systems for
other things both ways in the past, and my general experience has been
that the implicit-removal behavior tends to turn out to be more
annoying than you initially think that it will be, but if people want
that, it should be possible.

> > > In your proposal, does:
> > >
> > > GRANT foo TO bar WITH FRUIT STRAWBERRY;
> > >
> > > mean that 'foo' is grant'd to 'bar' too?
>
> That is, regardless of how we track these things in the catalog or such,
> we have to respect that:
>
> GRANT foo TO bar;
>
> is a SQL-defined thing that says that a 'role authorization descriptor'
> is created. SET ROLE then checks if a role authorization descriptor
> exists or not matching the current role to the new role and if it does
> then the current role is changed to the new role. What I was really
> trying to get at above is that:
>
> GRANT foo TO bar WITH $anything-other-than-SET-false;
>
> should probably also create a 'role authorization descriptor' that SET
> ROLE will pick up on. In other words, the 'SET' thing, or if we call
> that something else, should exist as a distinct column in
> pg_auth_members, but the default value of it should be 'true', with the
> ability for it to be turned to false either at GRANT time or with a
> REVOKE.

I think we're on the same page here. I have a strong desire not to get
caught up in a fruitless argument about SQL specification compliance,
but I believe everything after "in other words" matches my plan.

--
Robert Haas
EDB: http://www.enterprisedb.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jonathan S. Katz 2022-06-08 16:55:50 Re: How about a psql backslash command to show GUCs?
Previous Message Roberto C. Sánchez 2022-06-08 16:04:04 Request for assistance to backport CVE-2022-1552 fixes to 9.6 and 9.4