Re: Commitfest 2021-11 Patch Triage - Part 3

> On Nov 11, 2021, at 2:12 PM, Daniel Gustafsson <daniel@yesql.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