Re: role self-revocation

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Joshua Brindle <joshua(dot)brindle(at)crunchydata(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: role self-revocation
Date: 2022-03-11 17:31:58
Message-ID: 5FBC013C-A0AA-4730-B7E9-044D5C3031E5@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Mar 11, 2022, at 8:48 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>
> I agree that it would have an impact on backwards compatibility to
> change how WITH ADMIN works- but it would also get us more in line with
> what the SQL standard says for how WITH ADMIN is supposed to work and
> that seems worth the change to me.

I'm fine with giving up some backwards compatibility to get some SQL standard compatibility, as long as we're clear that is what we're doing. What you say about the SQL spec isn't great, though, because too much power is vested in "ADMIN". I see "ADMIN" as at least three separate privileges together. Maybe it would be spec compliant to implement "ADMIN" as a synonym for a set of separate privileges?

> On Mar 11, 2022, at 8:41 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>
> That we aren't discussing the issues with the current GRANT ... WITH
> ADMIN OPTION and how we deviate from what the spec calls for when it
> comes to DROP ROLE, which seems to be the largest thing that's
> 'solved' with this ownership concept, is concerning to me.

Sure, let's discuss that a bit more. Here is my best interpretation of your post about the spec, when applied to postgres with an eye towards not doing any more damage than necessary:

> On Mar 10, 2022, at 11:58 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>
> let's look at what the spec says:
>
> CREATE ROLE
> - Who is allowed to run CREATE ROLE is implementation-defined

This should be anyone with membership in pg_create_role.

> - After creation, this is effictively run:
> GRANT new_role TO creator_role WITH ADMIN, GRANTOR "_SYSTEM"

This should internally be implemented as three separate privileges, one which means you can grant the role, another which means you can drop the role, and a third that means you're a member of the role. That way, they can be independently granted and revoked. We could make "WITH ADMIN" a short-hand for "WITH G, D, M" where G, D, and M are whatever we name the independent privileges Grant, Drop, and Member-of.

Splitting G and D helps with backwards compatibility, because it gives people who want the traditional postgres "admin" a way to get there, by granting "G+M". Splitting M from G and D makes it simpler to implement the "bot" idea, since the bot shouldn't have M. But it does raise a question about always granting G+D+M to the creator, since the bot is the creator and we don't want the bot to have M. This isn't a problem I've invented from thin air, mind you, as G+D+M is just the definition of ADMIN per the SQL spec, if I've understood you correctly. So we need to think a bit more about the pg_create_role built-in role and whether that needs to be further refined to distinguish those who can get membership in roles they create vs. those who cannot. This line of reasoning takes me in the direction of what I think you were calling #5 upthread, but you'd have to elaborate on that, and how it interacts with the spec, for us to have a useful conversation about it.

> DROP ROLE
> - Any user who has been GRANT'd a role with ADMIN option is able to
> DROP that role.

Change this to "Any role who has D on the role". That's spec compliant, because anyone granted ADMIN necessarily has D.

> GRANT ROLE
> - No cycles allowed
> - A role must have ADMIN rights on the role to be able to GRANT it to
> another role.

Change this to "Any role who has G on the role". That's spec compliant, because anyone grant ADMIN necessarily has G.

We should also fix the CREATE ROLE command to require the grantor have G on a role in order to give it to the new role as part of the command. Changing the CREATEROLE, CREATEDB, REPLICATION, and BYPASSRLS attributes into pg_create_role, pg_create_db, pg_replication, and pg_bypassrls, the creator could only give them to the created role if the creator has G on the roles. If we do this, we could keep the historical privilege bits and their syntax support for backward compatibility, or we could rip them out, but the decision between those two options is independent of the rest of the design.


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 Robert Haas 2022-03-11 17:37:34 Re: refactoring basebackup.c
Previous Message Aleksander Alekseev 2022-03-11 17:26:26 Re: Add 64-bit XIDs into PostgreSQL 15