On Thu, Sep 12, 2024 at 3:40 PM Dominique Devienne <ddevienne@xxxxxxxxx> wrote: > Another way to look at it is this: > > === v14 === > ddevienne=> create role dd_child; > CREATE ROLE > ddevienne=> select pg_has_role(current_role, 'dd_child', 'MEMBER'); > pg_has_role > ------------- > f > (1 row) > > === v16 === > ddevienne=> create role dd_child; > CREATE ROLE > ddevienne=> select pg_has_role(current_role, 'dd_child', 'MEMBER'); > pg_has_role > ------------- > t > (1 row) > > Any existing ROLE graph which had "back-edges" (GRANTs) from a ROLE > back to the ROLE that created it, valid in pre-v16, becomes invalid in v16+. > And there's no work-around. Tough luck, take a hike... > > And our security model and its implementation basically requires such > back-edges. > > My contention is that if this is an ADMIN-only edge, it shouldn't be > deemed circular. > Kind of the same way you break cycles in FKs by making one side DEFERRED, > ADMIN edges should be "weaker" than SET ones, and break cycles. > > Maybe I'm the only one in the world using PostgreSQL in that situation? > Somehow I doubt that. Most people and organization are slow to upgrade, > and v16 is new enough that it wasn't exposed to enough real world usage yet. > So this is issue is only get bigger as time passes IMHO. Hi, I don't normally read pgsql-general, but Tom Lane drew my attention to this thread. I made the changes that you're complaining about here, so everything you're unhappy about is my fault. First of all, I'm sorry that you're frustrated. I was aware that there was a possibility that some people's use cases were going to get broken by these changes, and I tried to minimize the amount of stuff that would get broken, but it's still a bummer to hear that the situation is so frustrating for you. Second, it's over a year too late to think about making any changes to the behavior of v16. Even if everything I did here is terrible, we're kind of stuck with it at this point. We can fix bugs, but it's too late to redefine the semantics of a released branch. However, I don't think that what I did here is terrible, because CREATEROLE is totally insecure in earlier releases. If I understand your setup correctly, dd_user can do this: set role dd_owner; grant pg_execute_server_program to dd_user; grant pg_write_server_files to dd_user; At this point, dd_user should easily be able to make themselves superuser, or just erase/corrupt the entire database cluster. In other words, in v14, every account that has CREATEROLE and every account that can SET ROLE to an account that has CREATEROLE can very easily escalate to superuser. So I guess I would respectfully disagree with the idea that this works on v14 and v16 broke it. It doesn't really work on v14, or at least not any better than just using SUPERUSER everywhere that you're currently using CREATEROLE. And if you choose to do that, I think your example will work pretty much the same way on v16 as it does on v14. But that is not to say that you don't raise a good point here. The prohibition against circular grants is really annoying in your use case. If dd_owner creates dd_user, then dd_user is granted to dd_owner, which means that dd_owner cannot be granted (directly or indirectly) to dd_user. The reason why we have this sort of prohibition is that we don't want to end up with grants that become disconnected from their original source. Suppose the boss grants a privilege to alice, and later revokes it. But suppose that between the time the privilege is granted and the time it is revoked, alice grants the privilege to bob, and bob in turn grants it back to alice. Now, when the boss revokes the privilege, alice still has it, because she's still getting it from bob, who in turn is getting it from her. This would be very bad: the boss has every right to expect that when they revoke a privilege they have previously granted, the former recipient no longer has it. The prohibition against circular grants keeps this from happening. And we now apply that same principle to role grants. Consider this: robert.haas=# create role alice createrole; CREATE ROLE robert.haas=# set session authorization alice; SET robert.haas=> create role accounting; CREATE ROLE robert.haas=> create role bob; CREATE ROLE robert.haas=> grant accounting to bob; GRANT ROLE robert.haas=> set session authorization bob; SET robert.haas=> set role accounting; SET robert.haas=> reset session authorization; RESET robert.haas=# drop role alice; ERROR: role "alice" cannot be dropped because some objects depend on it DETAIL: privileges for membership of role bob in role accounting Privilege grants made by alice cannot survive alice's termination. Prior to v16, this principle applied to grants of everything except role; now, it also applies to role grants. Whether that's correct is an arguable point, but it seems very strange to me to argue that role grants should work differently from every other type of grant in the system, and it does have some nice properties. But that means that the anti-circularity provisions that we apply in other cases also need to be applied to roles. Otherwise, in your example, if the ddevienne role were removed, dd_admin and dd_owner would retain the ability to administer each other even though that grant would now have no source. That administrative authority would have come from ddevienne originally but, by making a set of circular grants, dd_admin and dd_owner could arrange to retain that privilege even after ddevienne was gone. We now forbid that just as we do for other object types. However, it seems like we might be able to fix this by just making the code smarter. Maybe there's a problem that I'm not seeing, but if the boss grants a privilege to alice and alice grants it to bob and bob grants it back to alice and then the boss revokes the privilege, why can't we figure out that alice no longer has a source for that privilege *aside from the one involved in the cycle* and undo the reciprocal grants that bob and alice made to each other? Right now I believe we just ask "is the number of sources that alices has for this privilege still greater than zero" which only works if there are no cycles but maybe we can do better. We'd probably need to think carefully about concurrency issues, though, and whether pg_dump is smart enough to handle this case. Also, there are separate code paths for role grants and non-role grants, and since I went to a lot of trouble to make them work the same way, I'd really prefer it if we didn't go back to having them work differently... -- Robert Haas EDB: http://www.enterprisedb.com
