> On Nov 9, 2021, at 8:50 AM, Stephen Frost <sfr...@snowman.net> wrote:
>
>> If we fix the existing bug that the pg_auth_members.grantor field can end up
>> as a dangling reference, instead making sure that it is always accurate,
>> then perhaps this would be ok if all roles granted into "charlie" had
>> grantor="super_alice". I'm not sure that is really good enough, but it is a
>> lot closer to making this safe than allowing the command to succeed when
>> role "charlie" has been granted away by someone else.
>
> I agree we should fix the issue of the grantor field being a dangling
> reference, that's clearly not a good thing.
Just FYI, I have a patch underway to fix it. I'm not super close to posting
it, though.
> I'm not sure what is meant by making sure they're always 'accurate' or
> why 'accurate' in this case means that the grantor is always
> 'super_alice'..?
I mean that the dangling reference could point at a role that no longer exists,
but if the oid gets recycled, it could point at the *wrong* role rather than
merely at no role. So we'd need that fixed before we could rely on the
"grantor" field for anything. I think Robert mentioned this issue already, on
another thread.
> Are you suggesting that the CREATE OR REPLACE ROLE run
> by super_alice would remove the GRANT that bob made of granting charlie
> to david?
Suppose user "stephen.frost" owns a database and runs a script which creates
roles, schemas, etc:
CREATE OR REPLACE ROLE super_alice SUPERUSER;
SET SESSION AUTHORIZATION super_alice;
CREATE OR REPLACE ROLE charlie;
CREATE OR REPLACE ROLE david IN ROLE charlie;
User "stephen.frost" runs that script again. The system cannot tell, as things
currently are implemented, that "stephen.frost" was the original creator of
role "super_alice", nor that "super_alice" was the original creator of
"charlie" and "david".
The "grantor" field for "david"'s membership in "charlie" points at
"super_alice", so we know enough to allow the "IN ROLE charlie" part, at least
if we fix the dangling reference bug.
If we add an "owner" (or perhaps a "creator") field to pg_authid, the first
time the script runs, it could be set to "stephen.frost" for "super_alice" and
to "super_alice" for "charlie" and "david". When the script gets re-run, the
CREATE OR REPLACE commands can succeed because that field matches.
> I would argue that it's entirely possible that super_alice
> knows exactly what is going on and intends for charlie to have superuser
> access and understands that any role which charlie has been GRANT'd to
> would therefore be able to become charlie, that's not a surprise.
I agree that super_alice might know that, but perhaps we can make this feature
less of a foot-gun and still achieve the goal of making idempotent role
creation scripts work?
> Now, bringing this around to the more general discussion about making it
> possible for folks who aren't superuser to be able to create roles, I
> think there's another way to address this that might satisfy everyone,
> particularly with the CREATE OR REPLACE approach- to wit: if the role
> create isn't one that you've got appropriate rights on, then you
> shouldn't be able to CREATE OR REPLACE it.
Agreed. You shouldn't be able to CREATE OR REPLACE a role that you couldn't
CREATE in the first case.
> This, perhaps, gets to a
> distinction between having ADMIN rights on a role vs. the ability to
> redefine the role (perhaps by virtue of being the 'owner' of that role)
> that's useful.
>
> In other words, in the case outlined above:
>
> bob: CREATE ROLE charlie;
> -- charlie is now a role 'owned' by bob and that isn't able to be
> -- changed by bob to be some other owner, unless bob can become the
> -- role to which they want to change ownership to
> bob: GRANT charlie TO david;
>
> alice: CREATE OR REPLACE ROLE charlie;
> -- This now fails because while alice is able to create roles, alice
> -- can only 'replace' roles which alice owns.
This sounds reasonable. It means, of course, implementing a role ownership
system. I thought you had other concerns about doing so.
> I appreciate that the case of 'super_alice' doing things where they're
> an actual superuser might still be an issue, but running around doing
> things with superuser is already risky business and the point here is to
> get away from doing that by splitting superuser up, ideally in a way
> that privileges can be given out to non-superusers in a manner that's
> safer than doing things as a superuser and where independent
> non-superusers aren't able to do bad things to each other.
I'm not sure what to do about this.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
