From: | walther(at)technowledgy(dot)de |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net> |
Cc: | "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: has_privs_of_role vs. is_member_of_role, redux |
Date: | 2022-09-08 17:06:24 |
Message-ID: | b580a773-1e4a-6a91-49f5-c98ce976de4d@technowledgy.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Robert Haas:
> Fairly obviously, my thinking here is biased by having written the
> patch to allow restricting SET ROLE. If alice can neither inherit
> bob's privileges nor SET ROLE bob, she had better not be able to
> create objects owned by bob, because otherwise she can make a table,
> add an expression index that calls a user-defined function, do stuff
> until it needs to be autovacuumed, and then give it to bob, and boom,
> exploit. But that doesn't mean that the is_member_of_role() tests here
> have to be changed to has_privs_of_role(). They could be changed to
> has_privs_of_role() || member_can_set_role(). And if the consensus is
> to do it that way, I'm OK with that.
A different line of thought (compared to the "USAGE" privilege I
discussed earlier), would be:
To transfer ownership of an object, you need two sets of privileges:
- You need to have the privilege to initiate a request to transfer
ownership.
- You need to have the privilege to accept a request to transfer ownership.
Let's imagine there'd be such a request created temporarily, then when I
start the process of changing ownership, I would have to change to the
other role and then accept that request.
In theory, I could also inherit that privilege, but that's not how the
system works today. By using is_member_of_role, the decision was already
made that this should not depend on inheritance. What is left, is the
ability to do it via SET ROLE only.
So it should not be has_privs_of_role() nor has_privs_of_role() ||
member_can_set_role(), as you suggested above, but rather just
member_can_set_role() only. Of course, only in the context of the SET
ROLE patch.
Basically, with that patch is_member_of_role() has to become
member_can_set_role().
> I'm just a little unconvinced that it's actually the best route. I
> think that logic of the form "well Alice could just SET ROLE and do it
> anyway" is weak -- and not only because of the patch to allow
> restricting SET ROLE, but because AFAICT there is no point to the
> INHERIT option in the first place unless it is to force you to issue
> SET ROLE. That is literally the only thing it does. If we're going to
> have weird exceptions where you don't have to SET ROLE after all, why
> even have INHERIT in the first place?
As stated above, I don't think this is about INHERIT. INHERIT works fine
both without the SET ROLE patch (and keeping is_member_of_role) and with
the SET ROLE patch (and changing to member_can_set_role).
The exception is made, because there is no formal two-step process for
requesting and accepting a transfer of ownership. Or alternatively:
There is no exception, it's just that during the command to transfer
ownership, the current role has to be changed temporarily to the
accepting role. And that's the same as checking is_member_of_role or
member_can_set_role, respectively.
Best
Wolfgang
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2022-09-08 17:07:40 | Re: Minimum bison and flex versions |
Previous Message | Andres Freund | 2022-09-08 17:01:11 | Re: [RFC] building postgres with meson - v12 |