Re: Commitfest 2021-11 Patch Triage - Part 3

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: Daniel Gustafsson <daniel(at)yesql(dot)se>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Jeff Davis <pgsql(at)j-davis(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Joshua Brindle <joshua(dot)brindle(at)crunchydata(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: Commitfest 2021-11 Patch Triage - Part 3
Date: 2021-12-13 19:20:38
Message-ID: 7EA351A8-CB2B-4D86-A40B-8931D3C65BA0@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Nov 11, 2021, at 2:12 PM, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>
> 3223: Delegating superuser tasks to new security roles
> ======================================================
> There is a lot of ongoing discussion on this one, but skimming the thread it's
> not really clear (to me) where it leaves the patch in question.

This set of patches got a fair amount of in-person off-list discussion at the
PGConf NYC with folks who had concerns about some or all parts of the patch.
Most of the known contentious design details were resolved, and I've been
updating the patches to repost this week. The subscriptions patch and guc
patch should be close to ready. The CREATEROLE patch may take a bit longer.

Conversation Highlights:

- The logical replication subscription patch needs to consider RLS.
(Jeff Davis)

This is implemented but I still need to update the documentation before
posting.

- The patch for granting SET VALUE and/or ALTER SYSTEM on guc settings should
be extended to allow revoking privilege to SET VALUE on userset gucs.
(Joshua Brindle, Stephen Frost).

This is implemented for core by means of converting PGC_USERSET variables
to PGC_SUSET, with a corresponding "GRANT SET VALUE ON .. TO public" added
to the new catalog/setting_privileges.sql file. By making it an explicit
grant, it is revokable without the need for any special logic. This would allow, for
example, REVOKE SET VALUE ON work_mem FROM public; followed by explicit
grants to whichever roles you want to allow to set work_mem.

I still need to adjust gucs defined in some contrib modules, update
documentation, and verify pg_upgrade from older versions before posting.

External projects are not harmed by this implementation. The only issue
for them is that SET VALUE cannot be revoked from any PGC_USERSET gucs they
define until they update their module to use PGC_SUSET plus a GRANT
somewhere in their extension creation or upgrade sql script.

- The CREATEROLE patch can go forward with roles having owners, but other
spec compliant behavior for GRANT/REVOKE should be implemented.
(Stephen Frost)

The bug wherein the grantor field in pg_auth_members may point to a dropped
role should be fixed. Probably this field should simply be dropped. We
don't use it anywhere, and on upgrade we can't know if existing values are
correct or bogus. Values that do not refer to any existing role are
obviously bogus, but if a role Oid was reassigned, a grantor entry may be
pointing wrongly at the new role, and there is no way to know that.

Stephen indicated that, for all privileges, only the role which GRANTed a
privilege should be able to REVOKE it. This differs from the current
implementation and is likely controversial. Robert Haas, at least,
described that idea as a non-starter. I don't plan to include that in this
patch, as it doesn't seem necessary to combine the two features even if we
eventually intend to do both. (And certainly unhelpful to include it if
the idea is rejected.)

Whether the owner of a role is implicitly a member (or admin) of the owned
role is still disputed. Stephen Frost would rather the owner has the right
to grant itself into the role, or grant itself ADMIN on the role, but not
be a member nor an admin by default. Robert Haas preferred that the owner
is both a member and an admin by default, making role ownership analogous
to how superuser works. I lean slightly in Stephen's favor, as any role
"foo" with CREATEROLE could always run:

CREATE ROLE bar [ ROLE foo | ADMIN foo];

and thereby give themselves the privilege they want in the same command
that creates the role. Going with Robert's proposal, a creator who does
not want to have privileges in the role would need to run:

CREATE ROLE bar;
REVOKE [ADMIN OPTION FOR ] bar FROM foo;

which requires two separate commands, and creates a short window inbetween
where a (possibly undesireable) configuration exists.

Jeff Davis and Andrew Dunstan have volunteered as committers.


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 Andres Freund 2021-12-13 19:33:24 Re: port conflicts when running tests concurrently on windows.
Previous Message Tom Lane 2021-12-13 18:57:52 Re: isolationtester: add session name to application name