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

Greetings,

* David G. Johnston (david.g.johnston@gmail.com) wrote:
> On Thu, Mar 10, 2022 at 7:46 AM Robert Haas <robertmhaas@gmail.com> wrote:
> > On Wed, Mar 9, 2022 at 4:31 PM Tom Lane <tgl@sss.pgh.pa.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.  As this issue with CREATEROLE existed way before
predefined roles were added to PG, claiming that it's an issue with
predefined roles doesn't make a bit of sense.

> > The whole area needs a rethink. I believe
> > everyone involved in the discussion on the other threads agrees that
> > some reform of CREATEROLE is necessary, and more generally with the
> > idea that it's useful for non-superusers to be able to create roles.
>
> As the documentation says, using SUPERUSER for day-to-day administration is
> contrary to good security practices.  Role management is considered to be a
> day-to-day administration activity.  I agree with this principle.  It was
> designed to neither be a superuser nor grant superuser, so removing the
> ability to grant the pg_* role memberships remains consistent with its
> original intent.

That would not be sufficient to make CREATEROLE safe.  Far, far from it.

> > I want that because I want mini-superusers, where alice can administer
> > the users that alice creates just as if she were a superuser,
> > including having their permissions implicitly and dropping them when
> > she wants them gone, but where alice cannot break out to the operating
> > system as a true superuser could do.
>
> CREATEROLE (once the pg_* with admin rules are fixed) + Ownership and rules
> restricting interfering with another role's objects (unless superuser)
> seems to handle this.

This is not sufficient- roles can be not-superuser themselves but have
the ability to become superuser if GRANT'd a superuser role and
therefore we can't have a system where CREATEROLE allows arbitrary
GRANT'ing of roles to each other.  I'm a bit confused too as anything
where we are curtailing what CREATEROLE roles are able to do in a manner
that means they're only able to modify some subset of roles should
equally apply to predefined roles too- that is, 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.

I disagree that ownership is needed that's not what the spec calls for
either.  What we need is more flexibility when it comes to the
relationships which are allowed to be created between roles and what
privileges come with them.  To that end, I'd argue that we should be
extending pg_auth_members, first by separating out membership itself
into an explicitly tracked attribute (instead of being implicit in the
existance of a row in the table) and then adding on what other
privileges we see fit to add, such as the ability to DROP a role.  We
do need to remove the ability for a role who hasn't been explicitly
given the admin right on another role to modify that role's membership
too, as was originally proposed here.  This also seems to more closely
follow the spec's expectation, something that role ownership doesn't.

> > the bot can stand up
> > accounts, can grant them membership in a defined set of groups, but
> > cannot exercise the privileges of those accounts (or hack superuser
> > either).
>
> The bot should be provided a security definer procedure that encapsulates
> all of this rather than us trying to hack the permission system.  This
> isn't a user permission concern, it is an unauthorized privilege escalation
> concern.  Anyone with the bot's credentials can trivially overcome the
> third restriction by creating a role with the desired membership and then
> logging in as that role - and there is nothing the system can do to prevent
> that while also allowing the other two permissions.

Falling back to security definer functions may be one approach but it's
not a great one and it only works if it's possible to end up with the
catalogs having what is actually desired- for example, ADMIN option
without membership isn't something the catalogs today can understand
because existance in pg_auth_members implies membership and you can't
have ADMIN without having that row.  The same issue would exist with
ownership if ownership implied the same- that's not improving things.

> > And that's why I'm not sure it's really the right idea to say that we
> > don't need syntax for this admin-without-member concept.
>
> We already have this syntax in the form of CREATEROLE.  But we do need a
> fix, just on the group side.  We need a way to define a group as having no
> ADMINS.

We don't have this syntax today nor do we have a way to store such a
concept in the catalogs either, so I'm pretty baffled by this.  Defining
a group without admins is, in fact, what we actually have support for
today in the catalogs- it's just a case where there aren't any rows in
pg_auth_members which have 'admin_option' as true.  The opposite is what
we're talking about here- rows which have 'admin_option' as true but
don't have membership, and that can't be the case today because
existance in the table itself implies membership.

> 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.  As for if we should migrate
CREATEROLE to a new predefined role, maybe, but that seems like a
different question.

> Or just special-case pg_* roles.

As I hopefully made clear above, this isn't actually a solution, nor do
pg_* roles need to be treated somehow differently in this aspect.

> The advantage of exposing this to the DBA is that they can then package
> pg_* roles into a custom group and still have the benefit of superuser only
> administration.  In the special-case implementation the presence of a pg_*
> role in a group hierarchy would then preclude a non-superuser from having
> admin on the entire tree (the pg_* roles are all roots, or in the case of
> pg_monitor, directly emanate from a root role).

We are very much trying to move away from 'superuser only
administration'.

Thanks,

Stephen