Re: CREATE ROLE IF NOT EXISTS

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: David Christensen <david(dot)christensen(at)crunchydata(dot)com>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: CREATE ROLE IF NOT EXISTS
Date: 2021-11-09 17:17:55
Message-ID: 55AA3462-1EFF-4922-9D57-724206900905@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Nov 9, 2021, at 8:50 AM, Stephen Frost <sfrost(at)snowman(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2021-11-09 17:43:20 Re: Commitfest 2021-11 Patch Triage - Part 2
Previous Message Jonathan S. Katz 2021-11-09 17:16:05 Re: 2021-11-11 release announcement draft