Re: pg_auth_members.grantor is bunk - Mailing list pgsql-hackers
From | Robert Haas |
---|---|
Subject | Re: pg_auth_members.grantor is bunk |
Date | |
Msg-id | CA+TgmoY=wR7Q9XZR_-yJusuMD+TboKL2HWEREst6NC_dZLhwbQ@mail.gmail.com Whole thread Raw |
In response to | Re: pg_auth_members.grantor is bunk (Robert Haas <robertmhaas@gmail.com>) |
List | pgsql-hackers |
On Fri, Jun 24, 2022 at 4:46 PM Robert Haas <robertmhaas@gmail.com> wrote: > Interesting. I hadn't thought about changing the behavior of DROP > OWNED BY and REASSIGN OWNED BY. A quick experiment supports your > interpretation: Here is a minimal patch fixing exactly $SUBJECT. Granting a role to another role now creates a dependency on the grantor, so if you try to drop the grantor you get an ERROR. You can resolve that by revoking the grant, or by using DROP OWNED BY or REASSIGN OWNED BY. To make this work, I had to make role memberships participate in the dependency system, which means pg_auth_members gains an OID column. The tricky part is that removing either of the two roles directly involved in a grant currently does, and should still, silently remove the grant. So, if you do "GRANT foo TO bar GRANTED BY baz", and then try to "DROP ROLE baz", that should fail, but if you instead try to "DROP ROLE baz, bar", that should work, because when bar is removed, the grant is silently removed, and then it's OK to drop baz. If these were database-local objects I think this could have all been sorted out quite easily by creating dependencies on all three roles involved in the GRANT and using the right deptype for each, but shared objects have their own set of deptypes which seemed to present no easy solution to this problem. I resolved the issue by having DropRole() make two loops over the list of roles to be dropped rather than one; see patch for details. There are several things that I think ought to be changed which this patch does not change. Most likely, I'll try to write separate patches for those things rather than making this one bigger. First, as discussed upthread, I think we ought to change things so that you can have multiple simultaneous grants of role A to role B each with a different grantor. That is what we do for other types of grants and Stephen at least thinks it's what the SQL standard specifies. Second, I think we ought to enforce that the grantor has to be a role which has the ability to perform the grant, just as we do for other object types. This is a little thorny, though, because we play some tricks with other types of objects that don't work for roles. If superuser alice executes "GRANT SELECT ON bobs_table TO fred" we record the owner of the grant as being the table owner and update the ownership of the grant each time the table owner is changed. That way, even if alice ceases to be a superuser, we maintain the invariant that the grantor of record must have privileges to perform the grant. But if superuser alice executes "GRANT accounting TO fred", we can't use the same trick, because the "accounting" role doesn't have an owner. If we attribute the grant to alice and she ceases to be a superuser (and also doesn't have CREATEROLE) then the invariant is violated. Attributing the grant to the bootstrap superuser doesn't help, as that user can also be made not a superuser. Attributing the grant to accounting is no good, as accounting doesn't and can't have ADMIN OPTION on itself; and fred doesn't have to have ADMIN OPTION on accounting either. One way to fix this problem would be to prohibit the removal of superuser privileges from the booststrap superuser. Then, we could attribute grants made by users who lack ADMIN OPTION on the granted role to the bootstrap superuser. Grants made by users who do possess ADMIN OPTION would be attributed to the actual grantor (unless GRANTED BY was used) and removing ADMIN OPTION from such a user could be made to fail if they had outstanding role grants. I think that's probably the nearest analogue of what we do for other object types, but if you've got another idea what to do here, I'd love to hear it. Thoughts on this patch would be great, too. Thanks, -- Robert Haas EDB: http://www.enterprisedb.com
Attachment
pgsql-hackers by date: