Re: role self-revocation - Mailing list pgsql-hackers

From David G. Johnston
Subject Re: role self-revocation
Date
Msg-id CAKFQuwarebaHvSw0+yWpEPs0fnZ9HE1ceCax-fHJEXa6hjDMTw@mail.gmail.com
Whole thread Raw
In response to Re: role self-revocation  (Stephen Frost <sfrost@snowman.net>)
Responses Re: role self-revocation
List pgsql-hackers
On Thu, Mar 10, 2022 at 11:05 AM Stephen Frost <sfrost@snowman.net> wrote:
Greetings,

* David G. Johnston (david.g.johnston@gmail.com) wrote:
> On Thu, Mar 10, 2022 at 9:19 AM Stephen Frost <sfrost@snowman.net> wrote:
> > * David G. Johnston (david.g.johnston@gmail.com) wrote:
> > > On Thu, Mar 10, 2022 at 7:46 AM Robert Haas <robertmhaas@gmail.com>
> > wrote:

> 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.

A non-superuser cannot grant a non-superuser membership in a superuser role.  A superuser granting a user membership in a superuser role makes that user a superuser.  This seems sane.

If a superuser grants a non-superuser membership in a superuser role then today a non-superuser can grant a user membership in that intermediate role, thus having a non-superuser make another user a superuser.  This is arguably a bug that needs to be fixed.

My desired fix is to just require the superuser to mark (or have it marked by default ideally) the role inheriting superuser and put the responsibility on the superuser.  I agree this is not ideal, but it is probably quick and low risk.

I'll let someone else describe the details of the alternative option.  I suspect it will end up being a better option in terms of design.  But depending on time and risk even knowing that we want the better design eventually doesn't preclude getting the easier fix in now.


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.

Yes, and superusers implicitly have that right, while CREATEROLE users implicitly have that right on the pg_* role but not on superuser roles.  I just want to plug that hole and include the pg_* roles (or any role for that matter) in being able to be denied implied ADMIN rights for non-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.

I agree.  And I've suggested a minimal fix, adding an attribute to the role that prohibits non-superusers from granting it to others, that removes the insane behavior.

I'm on board for a hard-coded fix as well - if a superuser is in the membership chain of a role then non-superusers cannot grant membership in that role to others.

Neither of those really solves the pg_* roles problem.  We still need to indicate that they are somehow special.  Whether it is a nice matrix or roles and permissions or a simple attribute that makes them behave like they are superuser roles.
 

I disagree entirely with the idea that we must have some roles who can
only ever be administered by a superuser.

I don't think this is a must have.  I think that since we do have it today that fixes that leverage the status quo in order to be done more easily are perfectly valid solutions.

 
  If anything, we should be
moving away (as we have, in fact, been doing), from anything being the
exclusive purview of the superuser.

I totally agree.
 

> 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?

Because CREATEUSER is a thing and people want to prevent roles with that attribute from assigning membership to the predefined superuser-aspect roles.  If I've misunderstood that desire and the scope of delegation given by the superuser to CREATEUSER roles is acceptable, then no change here is needed.

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.

I made the observation that being able to manage the membership of a group without having the ability to create new users seems like a half a loaf of a feature.  That's it.  I would presume that any redesign of the permissions system here would address this adequately.

 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.

Now you tell me.  Robert should have led with that goal upfront.

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


That is correct, I was trying to figure out minimally invasive fixes to what are arguably being called bugs.

David J.

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: role self-revocation
Next
From: Andres Freund
Date:
Subject: Re: Reducing power consumption on idle servers