Re: role self-revocation

On Thu, Mar 10, 2022 at 11:05 AM Stephen Frost <sfrost(at)snowman(dot)net> wrote:

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