Subject: Re: role self-revocation

From: Mark Dilger <[email-masked]>
Date: Mon, 7 Mar 2022 12:03:55 -0800

> On Mar 7, 2022, at 12:01 PM, Robert Haas <[email-masked]> wrote:
> 
> It's been pointed out upthread that this would have undesirable
> security implications, because the admin option would be inherited,
> and the implicit permission isn't.

Right, but with a reflexive self-admin-option, we could document that it works in a non-inherited way.  We'd just be saying the current hard-coded behavior is an option which can be revoked rather than something you're stuck with.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

---

From: Mark Dilger <[email-masked]>
Date: Mon, 7 Mar 2022 12:09:48 -0800

> On Mar 7, 2022, at 12:03 PM, Mark Dilger <[email-masked]> wrote:
> 

We could also say that the default is to not have admin option on yourself, with that being something grantable, but that is a larger change from the historical behavior and might have more consequences for dump/restore, etc.

My concern about just nuking self-admin is that there may be sites which use self-admin and we'd be leaving them without a simple work-around after upgrade, because they couldn't restore the behavior by executing a grant.  They'd have to more fundamentally restructure their role relationships to not depend on self-admin, something which might be harder for them to do.  Perhaps nobody is using self-admin, or very few people are using it, and I'm being overly concerned.

—
Mark Dilger

---

From: Tom Lane <[email-masked]>
Date: Mon, 07 Mar 2022 15:16:09 -0500

> Right, but with a reflexive self-admin-option, we could document that it works in a non-inherited way.  We'd just be saying the current hard-coded behavior is an option which can be revoked rather…

After reflection, I think that role self-admin is probably a bad idea that
we should stay away from.  It could perhaps be reasonable given some other
system design and/or syntax than what SQL gives us, but we're dealing in
SQL.  It doesn't make sense to GRANT a role to itself, and therefore it
likewise doesn't make sense to GRANT WITH ADMIN OPTION.

Based on Robert's archaeological dig, it now seems that the fact that
we have any such behavior at all was just a mistake.  What would be
lost if we drop it?

Having said that, one thing that I find fishy is that it's not clear
where the admin privilege for a role originates.  After "CREATE ROLE
alice", alice has no members, therefore none that have admin privilege,
therefore the only way that the first member could be added is via
superuser deus ex machina.  This does not seem clean.  If we recorded
which user created the role, we could act as though that user has
admin privilege (whether or not it's a member).  Perhaps I'm
reinventing something that was already discussed upthread.  I wonder
what the SQL spec has to say on this point, too.

			regards, tom lane

---

From: "David G. Johnston" <[email-masked]>
Date: Mon, 7 Mar 2022 14:34:20 -0700

> we have any such behavior at all was just a mistake.  What would be

Probably nothing that couldn't be replaced, and with a better model, but I
do have a concern that there are setups in the wild inadvertently using
this behavior.  Enough so that I would vote to change it but include a
migration GUC to restore the current behavior, probably with a deprecation
warning.  Kinda depends on the post-change dump/restore mechanics.  But
just tearing it out wouldn't seem extraordinary for us.

> where the admin privilege for a role originates.

I do not see a problem with there being no inherent admin privilege for a
role.  A superuser or CREATEROLE user holds admin privilege on all roles in
the cluster.  They can delegate the privilege to administer a role to yet
another role in the system.  The necessitates creating two roles - the one
being administered and the one being delegated to.  I don't see a benefit
to saving which specific superuser or CREATEROLE user "owns" the role that
is to be administered.  Not unless non-owner CREATEROLE users are prevented
from exercising admin privileges on the role.  That all said, I'd accept
the choice to include such ownership information as a requirement for
meeting the auditing needs of DBAs.  But I would argue that such auditing
probably needs to be external to the working system - the fact that
ownership can be changed reduces the benefit of an in-database value.

> If we recorded

> admin privilege (whether or not it's a member).

I suppose we could record the current owner of a role but that seems
unnecessary.  I dislike using the "created" concept by virtue of the fact
that, for routines, "security definer" implies creator but it actually
means "security owner".

David J.

---

From: Mark Dilger <[email-masked]>
Date: Mon, 7 Mar 2022 20:14:31 -0800

> On Mar 7, 2022, at 12:16 PM, Tom Lane <[email-masked]> wrote:
> 
> What would be
> lost if we drop it?

I looked into this a bit.  Removing that bit of code, the only regression test changes for "check-world" are the expected ones, with nothing else breaking.  Running installcheck+pg_upgrade to the patched version of HEAD from each of versions 11, 12, 13 and 14 doesn't turn up anything untoward.  The change I used (for reference) is attached:

—
Mark Dilger

---

From: Robert Haas <[email-masked]>
Date: Wed, 9 Mar 2022 15:51:10 -0500

On Mon, Mar 7, 2022 at 11:14 PM Mark Dilger
<[email-masked]> wrote:
> I looked into this a bit.  Removing that bit of code, the only regression test changes for "check-world" are the expected ones, with nothing else breaking.  Running installcheck+pg_upgrade to the patched version of HEAD from each of versions 11, 12, 13 and 14 doesn't turn up anything untoward.

I looked into this a bit, too. I attach a draft patch for removing the
self-admin exception.

I found that having is_admin_of_role() return true matters in three
ways: (1) It lets you grant membership in the role to some other role.
(2) It lets you revoke membership in the role from some other role.
(3) It changes the return value of pg_role_aclcheck(), which is used
in the implementation of various SQL-callable functions all invoked
via the name pg_has_role(). We've mostly been discussing (2) as an
issue, but (1) and (3) are pretty interesting too. Regarding (3),
there is a comment in the code indicating that Noah considered the
self-admin exception something of a wart as far as pg_has_role() is
concerned. As to (1), I discovered that today you can do this:

rhaas=# create user foo;
CREATE ROLE
rhaas=# create user bar;
CREATE ROLE
rhaas=# \q
[rhaas ~]$ psql -U foo rhaas
psql (15devel)
Type "help" for help.

rhaas=> grant foo to bar with admin option;
GRANT ROLE

I don't know why I didn't realize that before. It's a natural result
of treating the logged-in user as if they had admin option. But it's
weird that you can't even be granted WITH ADMIN OPTION on your own
login role, but at the same time without having it you can grant it to
someone else!

I believe there are three other points worth some consideration here.

First, in the course of my investigation I re-discovered what Tom
already did a good job articulating:

tgl> Having said that, one thing that I find fishy is that it's not clear
tgl> where the admin privilege for a role originates.  After "CREATE ROLE
tgl> alice", alice has no members, therefore none that have admin privilege,
tgl> therefore the only way that the first member could be added is via
tgl> superuser deus ex machina.  This does not seem clean.

I agree with that, but I don't think it's a sufficient reason for
keeping the self-admin exception, because the same problem exists for
non-login roles. I don't even think it's the right idea conceptually
to suppose that the power to administer a role originates from the
role itself. If that were so, then it would be inherited by all
members of the role along with all the rest of the role's privileges,
which is so clearly not right that we've already prohibited a role
from having WITH ADMIN OPTION on itself. In my opinion, the right to
administer a role - regardless of whether or not it is a login role -
most naturally vests in the role that created it, or something in that
direction at least, if not that exact thing. Today, that means the
superuser or a CREATEROLE user who could hack superuser if they
wished. In the future, I hope for other alternatives, as recently
argued on other threads. But we need not resolve the question of how
that should work exactly in order to agree (as I hope we do) that
doubling down on the self-administration exception is not the answer.

Second, it occured to me to wonder what implications a change like
this might have for dump and restore. If privilege restoration somehow
relied on this behavior, then we'd have a problem. But I don't think
it does, because (a) pg_dump can SET ROLE but can't change the session
user without reconnecting, so it's unclear how we could be relying on
it; (b) it wouldn't work for non-login roles, and it's unlikely that
we would treat login and non-login roles different in terms of
restoring privileges, and (c) when I execute the example shown above
and then run pg_dump, there's no attempt to change the current user,
it just dumps "GRANT foo TO bar WITH ADMIN OPTION GRANTED BY foo".

Third, it occurred to me to wonder whether some users might be using
and relying upon this behavior. It's certainly possible, and it does
suck that we'd be removing it without providing a workable substitute.
But it's probably not a LOT of users because most people who have
commented on this topic on this mailing list seem to find granting
membership in a login role a super-weird thing to do, because a lot of
people really seem to want every role to be a user or a group, and a
login role with members feels like it's blurring that line. I'm
inclined to think that the small number of people who may be unhappy
is an acceptable price to pay for removing this wart, but it's a
judgement call and if someone has information to suggest that I'm
wrong, it'd be good to hear about that.

Thanks,

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

---

From: Tom Lane <[email-masked]>
Date: Wed, 09 Mar 2022 16:01:40 -0500

> role itself.

Actually, that's the same thing I was trying to say.  But if it doesn't
originate from the role itself, where does it originate from?

> In my opinion, the right to
> direction at least, if not that exact thing.

This seems like a reasonable answer to me too: the creating role has admin
option implicitly, and can then choose to grant that to other roles.
Obviously some work needs to be done to make that happen (and we should
see whether the SQL spec has some different idea).

			regards, tom lane

---

From: Robert Haas <[email-masked]>
Date: Wed, 9 Mar 2022 16:15:51 -0500

> see whether the SQL spec has some different idea).

Well, the problem is that as far as I can see, the admin option is an
optional feature of membership. You can grant someone membership
without admin option, or with admin option, but you can't grant them
the admin option without membership, just like you can't purchase an
upgrade to first class without the underlying plane ticket. What would
the syntax look even like for this? GRANT foo TO bar WITH ADMIN OPTION
BUT WITHOUT MEMBERSHIP? Yikes.

But do we really have to solve this problem before we can clean up
this session exception? I hope not, because I think that's a much
bigger can of worms than this is.

---

From: Stephen Frost <[email-masked]>
Date: Wed, 9 Mar 2022 16:20:05 -0500

Greetings,

> option implicitly, and can then choose to grant that to other roles.

I agree that this has some appeal, but it's not desirable in all cases
and so I wouldn't want it to be fully baked into the system ala the role
'owner' concept.

> see whether the SQL spec has some different idea).

Agreed on this, though I don't recall it having much to say on it.

Thanks,

Stephen

---

From: Stephen Frost <[email-masked]>
Date: Wed, 9 Mar 2022 16:23:48 -0500

Greetings,

> BUT WITHOUT MEMBERSHIP? Yikes.

I've been meaning to reply to your other email regarding this, but I
don't really agree that the syntax ends up being so terrible or
difficult to deal with, considering we have these same general things
for ALTER ROLE already and there hasn't been all that much complaining.
That is, we have LOGIN and NOLOGIN, CREATEROLE and NOCREATEROLE, and we
could have MEMBERSHIP and NOMEMBERSHIP pretty easily here if we wanted
to.

> bigger can of worms than this is.

I do believe we can deal with the above independently and at a later
time and go ahead and clean up the session excepton bit without dealing
with the above at the same time.

Thanks,

Stephen

---

From: Tom Lane <[email-masked]>
Date: Wed, 09 Mar 2022 16:24:04 -0500

> see whether the SQL spec has some different idea).

Ah, here we go: it's buried under CREATE ROLE.  SQL:2021 12.4 <role
definition> saith that when role A executes CREATE ROLE <role name>,
then

1) A grantable role authorization descriptor is created whose role name
is <role name>, whose grantor is "_SYSTEM", and whose grantee is A.

Since nobody is _SYSTEM, this grant can't be deleted except by dropping
the new role (or, maybe, dropping A?).  So that has nearly the same
end result as "the creating role has admin option implicitly".  The main
difference I can see is that it also means the creating role is a *member*
implicitly, which is something I'd argue we don't want to enforce.  This
is analogous to the way we let an object owner revoke her own ordinary
permissions, which the SQL model doesn't allow since those permissions
were granted to her by _SYSTEM.

			regards, tom lane

---

From: Tom Lane <[email-masked]>
Date: Wed, 09 Mar 2022 16:31:00 -0500

> BUT WITHOUT MEMBERSHIP? Yikes.

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?

> this session exception?

I think we need a plan for where we're going.  I don't see "clean up
the session exception" as an end in itself; it's part of re-examining
how all of this ought to work.  I don't say that we have to have a
complete patch right away, only that we need a coherent end goal.

			regards, tom lane

---

From: "David G. Johnston" <[email-masked]>
Date: Wed, 9 Mar 2022 15:00:51 -0700

> if she revoked the corresponding plain privilege from herself.

So CREATE ROLE will assign ownership of AND membership in the newly created
role to the session_user UNLESS the OWNER clause is present in which case
the named role, so long as the session_user can SET ROLE to the named role,
becomes the owner & member.  Subsequent to that the owner can issue: REVOKE
new_role FROM role_name where role_name is again the session_user role or
one that can be SET ROLE to.

> of a role is its owner.  What's the problem with that?

I'm fine with this.  It does introduce an OWNER concept to roles and so at
minimum we need to add:

ALTER ROLE foo OWNER TO { new_owner | CURRENT_ROLE | CURRENT_USER |
SESSION_USER }

And similar for CREATE ROLE
And keep the USER alias commands in sync.
GROUP commands are only present for backward compatibility and so don't get
updated with new features by design.

Obviously a superuser can change ownership.

Playing with table ownership I find this behavior:
-- superuser
CREATE ROLE tblowner;
CREATE TABLE tblowner_test (id serial primary key);
ALTER TABLE tblowner_test OWNER TO tblowner;

CREATE ROLE boss;
GRANT boss TO tblowner;

SET SESSION AUTHORIZATION tblowner;
ALTER TABLE tblowner_test OWNER TO boss; --works

So tblowner can push their ownership attribute to any group they are a
member of.  Is that the behavior we want for roles as well?

David J.

---

From: Tom Lane <[email-masked]>
Date: Wed, 09 Mar 2022 17:35:18 -0500

"David G. Johnston" <[email-masked]> writes:
> role to the session_user

I would NOT have it automatically assign membership in the new role,
even though the SQL spec says so.  We've not done that historically
and it doesn't seem desirable.  In particular, it's *really* not
desirable for a user (role with LOGIN).

> ALTER ROLE foo OWNER TO { new_owner | CURRENT_ROLE | CURRENT_USER |

Agreed.

			regards, tom lane

---

From: Robert Haas <[email-masked]>
Date: Thu, 10 Mar 2022 09:46:46 -0500

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

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. 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.
But the reasons why people want that vary.

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. I want this because the lack of
meaningful privilege separation that led to CVE-2019-9193 being filed
spuriously is a very real problem. It's a thing a lot of people want,
and I want to give it to them. David Steele, on the other hand, wants
to build a user-creating bot that can create accounts but otherwise
conforms to the principle of least privilege: 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). Other people may well want other 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. If we just
want to bolt role ownership onto the existing framework without really
changing anything else, we can do that without extra syntax and, as
you say here, make it an implicit property of role ownership. But I
don't see that as has having much value; we just end up with a bunch
of superuser owners. Whatever. Now Stephen made the argument that we
ought to actually have admin-without-member as a first class concept,
something that could be assigned to arbitrary users. Actually, I think
he wanted it even more fine grained with that. And I think that could
make the concept a lot more useful, but then it needs some kind of
understandable syntax.

There's a lot of moving parts here. It's not just about coming up with
something that sounds generally logical, but about creating a system
that has some real-world utility.

> complete patch right away, only that we need a coherent end goal.

I'd like to have a plan, too, but if this behavior is accidental, I
still think we can remove it without making big decisions about future
direction. The perfect is the enemy of the good.

---

From: "David G. Johnston" <[email-masked]>
Date: Thu, 10 Mar 2022 08:56:36 -0700

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

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

> The whole area needs a rethink. I believe

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.

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

> the bot can stand up
> 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.

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

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.

Or just special-case pg_* roles.

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

David J.

David J.

---

From: Stephen Frost <[email-masked]>
Date: Thu, 10 Mar 2022 11:19:10 -0500

Greetings,

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

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

> ability to grant the pg_* role memberships remains consistent with its

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

> restricting interfering with another role's objects (unless superuser)

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.

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

> fix, just on the group side.  We need a way to define a group as having no

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.

> new pg_* role that grants this ability (and maybe some others) for use by a

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.

> pg_monitor, directly emanate from a root role).

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

Thanks,

Stephen

---

From: Mark Dilger <[email-masked]>
Date: Thu, 10 Mar 2022 08:26:42 -0800

> On Mar 10, 2022, at 7:56 AM, David G. Johnston <[email-masked]> wrote:
> 
> 
> 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.

What if one of alice's subordinates also owns roles?  Can alice interfere with *that* role's objects?  I don't see that a simple rule restricting roles from interfering with another role's objects is quite enough.  That raises the question of whether role ownership is transitive, and whether we need a concept similar to inherit/noinherit for ownership.

There is also the problem that CREATEROLE currently allows a set of privileges to be granted to created roles, and that set of privileges is hard-coded.  You've suggested changing the hard-coded rules to remove pg_* roles from the list of grantable privileges, but that's still an inflexible set of hardcoded privileges.  Wouldn't it make more sense for the grantor to need GRANT OPTION on any privilege they give to roles they create?

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

Doesn't this assume password authentication?  If the server uses ldap authentication, for example, wouldn't the bot need valid ldap credentials for at least one user for this attack to work?  And if CREATEROLE has been made more configurable, wouldn't the bot only be able to grant that ldap user the limited set of privileges that the bot's database user has been granted ADMIN OPTION for?

—
Mark Dilger

---

From: Robert Haas <[email-masked]>
Date: Thu, 10 Mar 2022 12:11:36 -0500

> follow the spec's expectation, something that role ownership doesn't.

I do not have a problem with more fine-grained kinds of authorization
even though I think there are syntactic issues to work out, but I
strongly disagree with the idea that we can't or shouldn't also have
role ownership. Marc invented it. Now Tom has invented it
independently. All sorts of other objects have it already. Trying to
make it out like this is some kind of kooky idea is not believable.
Yeah, it's not the most sophisticated or elegant model and that's why
it's good for us to also have other things, but for simple cases it is
easy to understand and works great.

---

From: "David G. Johnston" <[email-masked]>
Date: Thu, 10 Mar 2022 10:26:08 -0700

On Thu, Mar 10, 2022 at 9:19 AM Stephen Frost <[email-masked]> wrote:

> pg_execute_server_program,

> it's an issue for any role that could become
> 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:

SET allow_system_table_mods TO true;
DROP pg_catalog.pg_class;

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

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.

CREATEROLE shouldn't be
> the same for predefined roles as it does for any other.

Never granting the CREATEROLE attribute to anyone will give you this
outcome today.

> ADMIN option

Today, they don't need to in order for the system to function within its
existing design specs.

> to limit WITH ADMIN to superusers.

Today a non-superuser cannot "grant postgres to someuser;"

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.

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

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.

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.

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.

David J.

---

From: Joshua Brindle <[email-masked]>
Date: Thu, 10 Mar 2022 12:26:42 -0500

> easy to understand and works great.

Ownership implies DAC, the ability to grant others rights to an
object. It's not "kooky" to see roles as owned objects, but it isn't
required either. For example most objects on a UNIX system are owned
and subject to DAC but users aren't.

Stephen's, and now my, issue with ownership is that, since it implies
DAC, most checks will be bypassed for the owner. We would both prefer
for everyone to be subject to the grants, including whoever created
the role.

Rather, we'd like to see a "creators of roles get this set of grants
against the role by default" and "as a superuser I can revoke grants
from creators against roles they created"

---

From: Stephen Frost <[email-masked]>
Date: Thu, 10 Mar 2022 13:05:54 -0500

Greetings,

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

> DROP pg_catalog.pg_class;

I don't equate these in the least.

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

> them.  I see no difference here, so arguments of this nature aren't all

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.

> Never granting the CREATEROLE attribute to anyone will give you this

... which is why CREATEROLE is broken.

> Today, they don't need to in order for the system to function within its

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.

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

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

> marked roles; just like they cannot assign membership in superuser roles

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.

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

> the indirect superuser privileges and assignment thereof by non-superusers

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.

> 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

---

From: "David G. Johnston" <[email-masked]>
Date: Thu, 10 Mar 2022 12:31:30 -0700

On Thu, Mar 10, 2022 at 11:05 AM Stephen Frost <[email-masked]> wrote:

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

> on pg_read_all_stats, or on the postgres role.  That also happens to be

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.

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

> 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

I totally agree.

> 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
> 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
> CREATEROLE as being 'superuser light'.  The goal here is to get rid of

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

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