Re: role self-revocation

Greetings,

* David G. Johnston (david(dot)g(dot)johnston(at)gmail(dot)com) wrote:
> On Thu, Mar 10, 2022 at 9:19 AM Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > * David G. Johnston (david(dot)g(dot)johnston(at)gmail(dot)com) wrote:
> > > On Thu, Mar 10, 2022 at 7:46 AM Robert Haas <robertmhaas(at)gmail(dot)com>
> > wrote:
> > > > On Wed, Mar 9, 2022 at 4:31 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > > > > I don't think we need syntax to describe it. As I just said in my
> > > > > other reply, we have a perfectly good precedent for this already
> > > > > in ordinary object permissions. That is: an object owner always,
> > > > > implicitly, has GRANT OPTION for all the object's privileges, even
> > > > > if she revoked the corresponding plain privilege from herself.
> > > > >
> > > > > Yeah, this does mean that we're effectively deciding that the creator
> > > > > of a role is its owner. What's the problem with that?
> > > >
> > > > I don't think that's entirely the wrong concept, but it doesn't make a
> > > > lot of sense in a world where the creator has to be a superuser. If
> > > > alice, bob, and charlie are superusers who take turns creating new
> > > > users, and then we let charlie go due to budget cuts, forcing alice
> > > > and bob to change the owner of all the users he created to some other
> > > > superuser as a condition of dropping his account is a waste of
> > > > everyone's time. They can do exactly the same things to every account
> > > > on the system after we change the role owner as before.
> > >
> > > Then maybe we should just implement the idea that if a superuser would
> > > become the owner we instead substitute in the bootstrap user. Or give
> > the
> > > DBA the choice whether they want to retain knowledge of specific roles -
> > > and thus are willing to accept the "waste of time".
> >
> > This doesn't strike me as going in the right direction. Falling back to
> > the bootstrap superuser is generally a hack and not a great one. I'll
> > also point out that the SQL spec hasn't got a concept of role ownership
> > either.
> >
> > > > But wait, I hear you cry, what about CREATEROLE? Well, CREATEROLE is
> > > > generally agreed to be broken right now, and if you don't agree with
> > > > that, consider that it can grant pg_execute_server_programs to a
> > > > newly-created account and then explain to me how it's functionally
> > > > different from superuser.
> > >
> > > CREATEROLE has long been defined as basically having "with admin option"
> > on
> > > every role in the system. The failure to special-case the roles that
> > grant
> > > different aspects of superuser-ness to its members doesn't make
> > CREATEROLE
> > > itself broken, it makes the implementation of pg_execute_server_programs
> > > broken. Only superusers should be considered to have with admin option
> > on
> > > these roles. They can delegate through the usual membership+admin
> > mechanism
> > > to a CREATEROLE role if they desire.
> >
> > No, CREATEROLE having admin option on every role in the system is broken
> > and always has been. It's not just an issue for predefined roles like
> > pg_execute_server_program,
>
>
>
> > it's an issue for any role that could become
> > a superuser either directly or indirectly and that extends beyond the
> > predefined ones.
>
>
> The only indirect way for a role to become superuser is to have been
> granted membership in a superuser group, then SET ROLE. Non-superusers
> cannot do this. If a superuser does this I consider the outcome to be no
> different than if they go and do:

A non-superuser absolutely can be GRANT'd membership in a superuser role
and then SET ROLE to that user thus becoming a superuser. Giving users
a regular role to log in as and then membership in a role that can
become a superuser is akin to having a sudoers group in Unix and is good
practice, not something that everyone should have to be super-dooper
careful to not do, lest a CREATEROLE user be able to leverage that.

> SET allow_system_table_mods TO true;
> DROP pg_catalog.pg_class;

I don't equate these in the least.

> In short, having a CREATEROLE user issuing:
> GRANT pg_read_all_stats TO davidj;
> should result in the same outcome as them issuing:
> GRANT postgres TO davidj;
> -- ERROR: must be superuser to alter superusers

No, what should matter is if the role doing the GRANT has admin rights
on pg_read_all_stats, or on the postgres role. That also happens to be
what the spec says.

> Superusers can break their system and we don't go to great effort to stop
> them. I see no difference here, so arguments of this nature aren't all
> that compelling to me.

That you don't feel they're compelling don't make them somehow not real,
nor even particularly uncommon, nor do I view ignoring that possibility
as somehow creating a strong authentication system.

> CREATEROLE shouldn't be
> > the determining factor in the question of if a role can GRANT a
> > predefined (or any other role) to some other role- that should be
> > governed by the admin option on that role, and that should work exactly
> > the same for predefined roles as it does for any other.
> >
>
> Never granting the CREATEROLE attribute to anyone will give you this
> outcome today.

... which is why CREATEROLE is broken.

> > ADMIN option
> > without membership isn't something the catalogs today can understand
>
> Today, they don't need to in order for the system to function within its
> existing design specs.

Eh? Your argument here is "don't use CREATEROLE"? While I agree with
that being a generally good idea today, it hardly makes sense to suggest
it in a thread where we're talking about how to make CREATEROLE, or
something like it, be useful.

> > > ALTER ROLE pg_superuser WITH [NO] ADMIN;
> > >
> > > Then adding a role membership including the WITH ADMIN OPTION can be
> > > rejected, as can the non-superuser situation. Setting WITH NO ADMIN
> > should
> > > fail if any existing members have admin. You must be a superuser to
> > > execute WITH ADMIN (maybe WITH NO ADMIN as well...). And possibly even a
> > > new pg_* role that grants this ability (and maybe some others) for use
> > by a
> > > backup/restore user.
> >
> > I'm not following this in general or how it helps. Surely we don't want
> > to limit WITH ADMIN to superusers.
>
> Today a non-superuser cannot "grant postgres to someuser;"

No, but a role can be created like 'admin', which a superuser GRANT's
'postgres' to and then that role can be GRANT'd to anyone by anyone who
has CREATEROLE rights. That's not sane.

> The point of this attribute is to allow the superuser to apply that rule to
> other roles that aren't superuser. In particular, the predefined pg_*
> roles. But it could extend to any other role the superuser would like to
> limit. It means, for that for named role, ADMIN privileges cannot be
> delegated to other roles - thus all administration of that role's
> membership roster must happen by a superuser.

The whole "X can't modify a superuser role without being a superuser"
concept is just broken and was a poor choice when it was originally
done specifically because it only looks at individual roles and their
specific rolsuper bit, completely ignoring the fact that role membership
exists as a thing that we should handle sanely, including a
non-superuser role being grant'd a superuser role. Predefined roles
haven't got anything to do with any of this, they only make it more
obvious to people who didn't understand how the system worked before
they came along.

I disagree entirely with the idea that we must have some roles who can
only ever be administered by a superuser. If anything, we should be
moving away (as we have, in fact, been doing), from anything being the
exclusive purview of the superuser.

> In particular, this means CREATEROLE roles cannot assign membership in the
> marked roles; just like they cannot assign membership in superuser roles
> today.

I disagree with the idea that we need to mark some roles as only being
able to be modified by the superuser- why invent this? We have the
ADMIN option already and that can be applied to allow any role X to have
the ability to modify the members of role Y. That's a whole lot better
than some explicit flag that says "only superusers can modify this
role". If an admin wants that, they can set things up that way already
today, as long as they don't use the current CREATEROLE attribute.
Ideally, we'd modify CREATEROLE, or remove it and replace it with
something better, which still maintains that same flexibility. What you
seem to be arguing for here is to rip out the ADMIN functionality, which
is defined by spec and not even exclusively by PG, and replace it with a
single per-role flag that says if that role can only be modified by
superusers. That seems entirely backwards to me.

> For me, because the SUPERUSER cannot have its role become a group without a
> superuser making that choice, and by default the default pg_* roles will
> all have this property as well, and any newly superuser created roles that
> may be members of either superuser or pg_* can have the property defined as
> well, gives full control to the superuser as to how superuser abilities are
> doled out and so the design itself allows for what many of you are
> considering to be "safe usage". That "unsafe configurations" are possible
> is due to the policy that superusers are unrestricted in what they can do,
> including making unsafe and destructive choices.

I disagree that it's an 'unsafe configuration' for there to ever exist a
non-superuser role that has been granted a superuser role. The only
thing that makes this unsafe is the existance of CREATEROLE.

Why are we making this all about superusers though? In what you're
proposing, you're suggesting that it's perfectly fine for any role which
has CREATEROLE to be able to take over any other role in the entire
system, excluding predefined roles and superusers. How is that sane, or
truely much less than what the superuser has in terms of ability? The
short answer is that it's not- which is why we have documented
CREATEROLE as being 'superuser light'. The goal here is to get rid of
that.

> In short, removing the self-administration rule solves the "login roles
> should not be automatically considered groups administered by themselves"
> problem - or at least a feature we really don't need.
> And defining a "superuser administration only" attribute to a role solves
> the indirect superuser privileges and assignment thereof by non-superusers
> problem.

But it doesn't *actually* make CREATEROLE something that you can give
out to folks on a general basis because anyone with CREATEROLE would
still be able to take over every single non-superuser and non-predefined
role in the system. We do *not* want that.

> I can see value in adding a feature whereby we allow the DBA to define a
> group as a schema-like container and then assign roles to that group with a
> fine-grained permissions model. My take is this proposal is a new feature
> while the two problems noted above can be solved more readily and with less
> risk with the two suggested changes.

Yes, we're talking about a new feature- one intended to replace the
broken way that CREATEROLE works, which your proposal doesn't.

Thanks,

Stephen