On 9/15/21 10:38 AM, Mark Dilger wrote: >> On Aug 31, 2021, at 6:41 PM, Mark Dilger <mark.dil...@enterprisedb.com> >> wrote: >> >> <v6-0019-Giving-role-owners-control-over-owned-roles.patch> > Synopsis: > > The major change in version 7 is a reworking of role ownership and the > CREATEROLE attribute to make it no longer a near-superuser privilege. This > new functionality is in v7-0021. > > Details: > > The changes in version 7 of the patchset are: > > v7-0001 is a new patch that introduces a single new regression test covering > various aspects of the permissions system surrounding creating, altering, > dropping and granting membership in roles. The functional changes in v7-0021 > do not cause pre-existing regression test failures, not even when running > check-world, despite fundamentally changing how much of this works. This new > test adds coverage for create role, and as each patch in the series > introduces changes, is modified to reflect them. > > v6-0001 through v6-0019 correspond to v7-0002 through v7-0020 and are mostly > unchanged, but are updated to apply cleanly to the current git master, to fix > a bug that was present in the v6 patch set, to update the regression tests > for security labels where CREATEROLE is used, and to update the create_role > regression test from v7-0001 as needed per patch. > > v7-0021 redesigns the CREATEROLE attribute to no longer bestow nearly so much > power. The ability to alter or drop a role no longer flows from having the > CREATEROLE attribute, but rather from being the role's owner. The ADMIN > option works as before, but role owners implicitly have ADMIN on roles which > they own. > > Roles with the CREATEROLE attribute may create new roles, but those new roles > may not be created with privileges which the creating role lacks. > Specifically, SUPERUSER, REPLICATION, BYPASSRLS, CREATEDB, CREATEROLE and > LOGIN privilege may not be granted the new role unless the creating role has > them. (This rule is adhered to but trivial in the case of the CREATEROLE > privilege, since the creator must necessarily have that one.) When creating > a new role using the IN ROLE, ROLE, or ADMIN clauses, the creating role must > have sufficient privileges on the roles named by these clauses to perform the > GRANTs these roles entail. Merely having the CREATEROLE attribute is > insufficient to perform arbitrary grants of role memberships. > > The INHERIT, VALID UNTIL, and CONNECTION LIMIT attributes are not thought > about as privileges in the patch; perhaps they should be? It would be quite > reasonable to say that a role with a finite connection limit should have that > limit thought about as a "pool" and should have to assign connection rights > from that pool to other roles it creates. Likewise, a role with a VALID > UNTIL limit could be constrained to only create roles with VALID UNTIL less > than or equal to its own limit. Perhaps a NOINHERIT role should only be able > to create NOINHERIT roles? The patch does none of these things, but feedback > is much appreciated. > > The docs are adjusted, but drop_role.sgml may need to be further adjusted: > > <para> > The SQL standard defines <command>DROP ROLE</command>, but it allows > only one role to be dropped at a time, and it specifies different > privilege requirements than <productname>PostgreSQL</productname> uses. > </para> > > I lack a copy of the SQL standard, so I'm uncertain if this patch has, by > chance, changed the privilege requirements to match that of the spec? > >
This patch set is failing to apply for me - it fails on patch 2. I haven't dug terribly deeply into it yet, but I notice that there is a very large increase in test volume, which appears to account for much of the 44635 lines of the patch set. I think we're probably going to want to reduce that. We've had complaints in the past from prominent hackers about adding too much volume to the regression tests. I do like the basic thrust of reducing the power of CREATEROLE. There's an old legal maxim I learned in my distant youth that says "nemo dat quod non habet" - Nobody can give something they don't own. This seems to be in that spirit, and I approve :-) cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
