Thread: has_privs_of_role vs. is_member_of_role, redux
Hi,
We've had some previous discussions about when to use
has_privs_of_role and when to use is_member_of_role, and
has_privs_of_role has mostly won the fight. That means that, if role
"robert" is set to NOINHERIT and you "GRANT stuff TO robert", for the
most part "robert" will not actually be able to do things that "stuff"
could do. Now, robert will be able TO "SET ROLE stuff" and then do all
and only those things that "stuff" can do, but he won't be able to do
those things as "robert". For example:
rhaas=# set role robert;
SET
rhaas=> select * from stuff_table;
ERROR: permission denied for table stuff_table
So far, so good. But it's clearly not the case that "GRANT stuff TO
robert" has conferred no privileges at all on robert. At the very
least, it's enabled him to "SET ROLE stuff", but what else? I decided
to go through the code and make a list of the things that robert can
now do that he couldn't do before. Here it is:
1. robert can create new objects of various types owned by stuff:
rhaas=> create schema stuff_by_robert authorization stuff;
CREATE SCHEMA
rhaas=> create schema unrelated_by_robert authorization unrelated;
ERROR: must be member of role "unrelated"
2. robert can change the owner of objects he owns to instead be owned by stuff:
rhaas=> alter table robert_table owner to unrelated;
ERROR: must be member of role "unrelated"
rhaas=> alter table robert_table owner to stuff;
ALTER TABLE
3. robert can change the default privileges for stuff:
rhaas=> alter default privileges for role unrelated grant select on
tables to public;
ERROR: must be member of role "unrelated"
rhaas=> alter default privileges for role stuff grant select on tables
to public;
ALTER DEFAULT PRIVILEGES
4. robert can execute "SET ROLE stuff".
That's it. There are two other behaviors that change -- the return
value of pg_has_role('robert', 'stuff', 'MEMBER') and pg_hba.conf
matching to groups -- but those aren't things that robert gains the
ability to do. The above is an exhaustive list of the things robert
gains the ability to do.
I argue that #3 is a clear bug. robert can't select from stuff's
tables or change privileges on stuff's objects, so why can he change
stuff's default privileges? is_member_of_role() has a note that it is
not to be used for privilege checking, and this seems like it's pretty
clearly a privilege check.
On the flip side, #4 is pretty clearly correct. Presumably, allowing
that to happen was the whole point of executing "GRANT stuff TO
robert" in the first place.
The other two are less clear, in my opinion. We don't want users to
end up owning objects that they didn't intend to own; in particular,
if any user could make a security-definer function and then gift it to
the superuser, it would be a disaster. So, arguably, the ability to
make some other role the owner of an object represents a privilege
that your role holds with respect to their role. Under that theory,
the is_member_of_role() checks that are performed in cases #1 and #2
are privilege checks, and we ought to be using has_privis_of_role()
instead, so that a non-inherited role grant doesn't confer those
privileges. But I don't find this very clear cut, because except when
the object you're gifting is a Trojan horse, giving stuff away helps
the recipient, not the donor.
Also, from a practical point of view, changing the owner of an object
is different from other things that robert might want to do. If robert
wants to create a table as user stuff or read some data from tables
user stuff can access or change privileges on objects that role stuff
owns, he can just execute "SET ROLE stuff" and then do any of that
stuff. But he can't give away his own objects by assuming stuff's
privileges. Either he can do it as himself, or he can't do it at all.
It wouldn't be crazy IMHO to decide that a non-inherited grant isn't
sufficient to donate objects to the granted role, and thus an
inherited grant is required in such cases. However, the current system
doesn't seem insane either, and in fact might be convenient in some
situations.
In short, my proposal is to change the ALTER DEFAULT PRIVILEGES code
so that you have to have the privileges of the target role, not jut
membership in the target role, and leave everything else unchanged.
Thoughts?
--
Robert Haas
EDB: http://www.enterprisedb.com
On 8/25/22 12:12, Robert Haas wrote: > So far, so good. But it's clearly not the case that "GRANT stuff TO > robert" has conferred no privileges at all on robert. At the very > least, it's enabled him to "SET ROLE stuff", but what else? I decided > to go through the code and make a list of the things that robert can > now do that he couldn't do before. Here it is: > > 1. robert can create new objects of various types owned by stuff: > 2. robert can change the owner of objects he owns to instead be owned by stuff: > 3. robert can change the default privileges for stuff: > 4. robert can execute "SET ROLE stuff". Nice analysis, and surprising (to me) > I argue that #3 is a clear bug. robert can't select from stuff's > tables or change privileges on stuff's objects, so why can he change > stuff's default privileges? is_member_of_role() has a note that it is > not to be used for privilege checking, and this seems like it's pretty > clearly a privilege check. +1 this feels very wrong to me > On the flip side, #4 is pretty clearly correct. Presumably, allowing > that to happen was the whole point of executing "GRANT stuff TO > robert" in the first place. Exactly > The other two are less clear, in my opinion. We don't want users to > end up owning objects that they didn't intend to own; in particular, > if any user could make a security-definer function and then gift it to > the superuser, it would be a disaster. So, arguably, the ability to > make some other role the owner of an object represents a privilege > that your role holds with respect to their role. Under that theory, > the is_member_of_role() checks that are performed in cases #1 and #2 > are privilege checks, and we ought to be using has_privis_of_role() > instead, so that a non-inherited role grant doesn't confer those > privileges. But I don't find this very clear cut, because except when > the object you're gifting is a Trojan horse, giving stuff away helps > the recipient, not the donor. > > Also, from a practical point of view, changing the owner of an object > is different from other things that robert might want to do. If robert > wants to create a table as user stuff or read some data from tables > user stuff can access or change privileges on objects that role stuff > owns, he can just execute "SET ROLE stuff" and then do any of that > stuff. But he can't give away his own objects by assuming stuff's > privileges. Either he can do it as himself, or he can't do it at all. > It wouldn't be crazy IMHO to decide that a non-inherited grant isn't > sufficient to donate objects to the granted role, and thus an > inherited grant is required in such cases. However, the current system > doesn't seem insane either, and in fact might be convenient in some > situations. > > In short, my proposal is to change the ALTER DEFAULT PRIVILEGES code > so that you have to have the privileges of the target role, not jut > membership in the target role, and leave everything else unchanged. > > Thoughts? I'm not sure about these last two. Does it matter that object creation is being logged, maybe for auditing purposes, under a different user than the owner of the object? -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
On Thu, Aug 25, 2022 at 3:03 PM Joe Conway <mail@joeconway.com> wrote: > Nice analysis, and surprising (to me) Thanks. > > I argue that #3 is a clear bug. robert can't select from stuff's > > tables or change privileges on stuff's objects, so why can he change > > stuff's default privileges? is_member_of_role() has a note that it is > > not to be used for privilege checking, and this seems like it's pretty > > clearly a privilege check. > > +1 this feels very wrong to me Cool. I'll prepare a patch for that, unless someone else beats me to it. I really hate back-patching this kind of change but it's possible that it's the right thing to do. There's no real security exposure because the member could always SET ROLE and then do the exact same thing, so back-patching feels to me like it has a significantly higher chance of turning happy users into unhappy ones than the reverse. On the other hand, it's pretty hard to defend the current behavior once you stop to think about it, so perhaps it should be back-patched on those grounds. On the third hand, the fact that this has gone undiscovered for a decade makes you wonder whether we've really had clear enough ideas about this to justify calling it a bug rather than, say, an elevation of our thinking on this topic. > I'm not sure about these last two. Does it matter that object creation > is being logged, maybe for auditing purposes, under a different user > than the owner of the object? I'd be inclined to say that it doesn't matter, because the grant could have just as well been inheritable, or the action could have been performed by a superuser. Also, as a rule of thumb, I don't think we should choose to prohibit things on the grounds that some auditing regime might not be able to understand what happened. If that's an issue, we should address it by making the logging better, or including better logging hooks, or what have you. I think that the focus should be on the permissions model: what is the "right thing" from a security standpoint? -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
> I really hate back-patching this kind of change but it's possible that
> it's the right thing to do. There's no real security exposure because
> the member could always SET ROLE and then do the exact same thing, so
> back-patching feels to me like it has a significantly higher chance of
> turning happy users into unhappy ones than the reverse. On the other
> hand, it's pretty hard to defend the current behavior once you stop to
> think about it, so perhaps it should be back-patched on those grounds.
> On the third hand, the fact that this has gone undiscovered for a
> decade makes you wonder whether we've really had clear enough ideas
> about this to justify calling it a bug rather than, say, an elevation
> of our thinking on this topic.
Yeah, I'd lean against back-patching. This is the sort of behavioral
change that users tend not to like finding in minor releases.
regards, tom lane
On Thu, Aug 25, 2022 at 4:41 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Yeah, I'd lean against back-patching. This is the sort of behavioral > change that users tend not to like finding in minor releases. Here's a small patch. Despite the small size of the patch, there are a couple of debatable points here: 1. Should we have a code comment? I feel it isn't necessary, because there's a comment just a few lines earlier saying "Look up the role OIDs and do permissions checks" and that seems like sufficient justification for what follows. 2. What about the error message? Personally, I'm not very excited about "permission denied to whatever" as a way to phrase an error message. It doesn't sound like particularly good grammar to me. But it's the phrasing we use elsewhere, so I guess we should do the same here. 3. Should we have a test case? We are extremely thin on test cases for NOINHERIT behavior, it seems, and testing this one thing when we don't test anything else seems relatively useless. Also, privileges.sql is a giant mess. It's a 1700+ line test file that tests many fairly unrelated things. I am inclined to think that this file badly needs to be split up into a bunch of smaller files, because it's practically unmaintainable as is. For instance, the stuff at the top of the file is testing a bunch of things about role privileges, but then check some stuff about leakproof functions before coming back to test stuff about groups, which logically probably belongs with the role privileges stuff. Perhaps a reasonable starting split would be something like: - Privileges on roles. - Privileges on relations. - Privileges on other kinds of objects. - Default privileges. - Security barriers and leakproof functions. - Security-restricted operations. Some of those files might be fairly small initially, but they might be get bigger later, especially because it'd be a heck of a lot easier to add new test cases if you didn't have to worry that some change you make is going to break a test 1000 lines down in the file. -- Robert Haas EDB: http://www.enterprisedb.com
Attachment
Jeff Davis's comment in http://postgr.es/m/4f8d536a9221bccc5a33bb784dace0ef2310ec4a.camel@j-davis.com reminds me that I need to update this thread based on the patch posted over there. That patch allows you to grant membership in one role to another while withholding the ability to SET ROLE to the target role. And it's already possible to grant membership in one role to another while not allowing for inheritance of privileges. And I think that sheds new light on the two debatable points from my original email: On Thu, Aug 25, 2022 at 12:12 PM Robert Haas <robertmhaas@gmail.com> wrote: > 1. robert can create new objects of various types owned by stuff: > > rhaas=> create schema stuff_by_robert authorization stuff; > CREATE SCHEMA > rhaas=> create schema unrelated_by_robert authorization unrelated; > ERROR: must be member of role "unrelated" > > 2. robert can change the owner of objects he owns to instead be owned by stuff: > > rhaas=> alter table robert_table owner to unrelated; > ERROR: must be member of role "unrelated" > rhaas=> alter table robert_table owner to stuff; > ALTER TABLE It now seems to me that we should disallow these, because if we adopt the patch from that other thread, and then you GRANT pg_read_all_settings TO alice WITH INHERIT false, SET false, you might reasonably expect that alice is not going to be able to clutter the system with a bunch of objects owned by pg_read_all_settings, but because of (1) and (2), alice can do exactly that. To be more precise, I propose that in order for alice to create objects owned by bob or to change one of her objects to be owned by bob, she must not only be a member of role bob, but also inherit bob's privileges. If she has the ability to SET ROLE bob but not does not inherit his privileges, she can create new objects owned by bob only if she first does SET ROLE bob, and she cannot reassign ownership of her objects to bob at all. Meanwhile, the patch that I posted previously to fix point (3) from the original email, that ALTER DEFAULT PRIVILEGES is allowed for no good reason, still seems like a good idea. Any reviews appreciated. Thanks, -- Robert Haas EDB: http://www.enterprisedb.com
Greetings, * Robert Haas (robertmhaas@gmail.com) wrote: > Jeff Davis's comment in > http://postgr.es/m/4f8d536a9221bccc5a33bb784dace0ef2310ec4a.camel@j-davis.com > reminds me that I need to update this thread based on the patch posted > over there. That patch allows you to grant membership in one role to > another while withholding the ability to SET ROLE to the target role. > And it's already possible to grant membership in one role to another > while not allowing for inheritance of privileges. And I think that > sheds new light on the two debatable points from my original email: > > On Thu, Aug 25, 2022 at 12:12 PM Robert Haas <robertmhaas@gmail.com> wrote: > > 1. robert can create new objects of various types owned by stuff: > > > > rhaas=> create schema stuff_by_robert authorization stuff; > > CREATE SCHEMA > > rhaas=> create schema unrelated_by_robert authorization unrelated; > > ERROR: must be member of role "unrelated" > > > > 2. robert can change the owner of objects he owns to instead be owned by stuff: > > > > rhaas=> alter table robert_table owner to unrelated; > > ERROR: must be member of role "unrelated" > > rhaas=> alter table robert_table owner to stuff; > > ALTER TABLE > > It now seems to me that we should disallow these, because if we adopt > the patch from that other thread, and then you GRANT > pg_read_all_settings TO alice WITH INHERIT false, SET false, you might > reasonably expect that alice is not going to be able to clutter the > system with a bunch of objects owned by pg_read_all_settings, but > because of (1) and (2), alice can do exactly that. Err, that shouldn't be allowed and if it is then that's my fault for not implementing something to avoid having that happen. imv, predefined roles shouldn't be able to end up with objects they own except in cases where we declare that a predefined role owns X. I do think that the above two are correct and am fairly confident that they were intentional as implemented as, otherwise, as noted in your original message, you can't actually change the ownership of the existing object/table and instead end up having to copy the whole thing, which seems quite inefficient. In other words, the same result could be accomplished but in a much less efficient way and therefore it makes sense to provide a way for it to be done that is efficient. > To be more precise, I propose that in order for alice to create > objects owned by bob or to change one of her objects to be owned by > bob, she must not only be a member of role bob, but also inherit bob's > privileges. If she has the ability to SET ROLE bob but not does not > inherit his privileges, she can create new objects owned by bob only > if she first does SET ROLE bob, and she cannot reassign ownership of > her objects to bob at all. ... which means that to get a table owned by bob which is currently owned by alice, alice has to: set role bob; create table; grant insert on table to alice; reset role; insert into table select * from table; That's pretty sucky and is the case that had been contemplated at the time that was written to allow it (at least, if memory serves). iirc, that's also why we check the *bob* has CREATE rights in the place where this is happening (as otherwise the above process wouldn't work either). > Meanwhile, the patch that I posted previously to fix point (3) from > the original email, that ALTER DEFAULT PRIVILEGES is allowed for no > good reason, still seems like a good idea. Any reviews appreciated. Haven't looked at the patch, +1 on the general change though, that looks like incorrect usage of is_member_of_role to me. Thanks, Stephen
Attachment
On Wed, Sep 7, 2022 at 5:51 PM Stephen Frost <sfrost@snowman.net> wrote: > > To be more precise, I propose that in order for alice to create > > objects owned by bob or to change one of her objects to be owned by > > bob, she must not only be a member of role bob, but also inherit bob's > > privileges. If she has the ability to SET ROLE bob but not does not > > inherit his privileges, she can create new objects owned by bob only > > if she first does SET ROLE bob, and she cannot reassign ownership of > > her objects to bob at all. > > ... which means that to get a table owned by bob which is currently > owned by alice, alice has to: > > set role bob; > create table; > grant insert on table to alice; > reset role; > insert into table select * from table; > > That's pretty sucky and is the case that had been contemplated at the > time that was written to allow it (at least, if memory serves). iirc, > that's also why we check the *bob* has CREATE rights in the place where > this is happening (as otherwise the above process wouldn't work either). Sure. I think it comes down to what you think that the system administrator intended to block by not allowing alice to inherit bob's permissions. In existing releases, there's no facility to prevent alice from doing SET ROLE bob, so the system administrator can't have intended this as a security measure. But the system administrator might have intended that alice shouldn't do anything that relies on bob's permissions by accident, else she should have SET ROLE. And in that case the intention is defeated by allowing the operation. Now, you may well have in mind some other intention that the system administrator could have had where allowing alice to perform this operation without needing to inherit bob's permissions is sensible; I'm not trying to say there is no such case. I don't know what it is, though. My first reaction was in the same ballpark as yours: what's the big deal? But as I think about it more, I struggle to reconcile that instinct with any specific use case. 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. 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? -- Robert Haas EDB: http://www.enterprisedb.com
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. > > 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? I think to change the owner of an object from role A to role B, you just need a different "privilege" on that role B to "use" the role that way, which is distinct from INHERIT or SET ROLE "privileges". When you are allowed to INHERIT a role, you are allowed to use the GRANTs that have been given to this role. When you are allowed to SET ROLE, then you are allowed to switch into this role. You could think of another "privilege", USAGE on a role, which would allow you to "use" this role as a target in a statement to change the owner of an object. To change the owner for an object from role A to role B, you need: - the privilege to ALTER the object, which is implied when you are A - the privilege to "use" role B as a target So basically the privilege to use role B as the new owner, is a privilege you have **on** the role object B, while the privilege to change the owner of an object is something you have **through** your membership in role A. Up to v15, there were no separate privileges for this. You were either a member of a role or you were not. Now with INHERIT and maybe SET ROLE privileges/grant options, we can do two things: - Keep the ability to use a role as a target in those statements as the most basic privilege on a role, that is implied by membership in that role and can't be taken away (currently the case), or - invent a new privilege or grant option to allow changing that. But mixing this with either INHERIT or SET ROLE doesn't make sense, imho. Best Wolfgang
On Thu, Sep 8, 2022 at 11:45 AM Wolfgang Walther <walther@technowledgy.de> wrote: > I think to change the owner of an object from role A to role B, you just > need a different "privilege" on that role B to "use" the role that way, > which is distinct from INHERIT or SET ROLE "privileges". It's not distinct, though, because if you can transfer ownership of a table to another user, you can use that ability to gain the privileges of that user. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas: >> I think to change the owner of an object from role A to role B, you just >> need a different "privilege" on that role B to "use" the role that way, >> which is distinct from INHERIT or SET ROLE "privileges". > > It's not distinct, though, because if you can transfer ownership of a > table to another user, you can use that ability to gain the privileges > of that user. Right, but the inverse is not neccessarily true, so you could have SET ROLE privileges, but not "USAGE" - and then couldn't change the owner of an object to this role. USAGE is not a good term, because it implies "least amount of privileges", but in this case it's quite the opposite. In any case, adding a grant option for SET ROLE, while keeping the required privileges for a transfer of ownership at the minimum (membership only), doesn't really make sense. I guess both threads should be discussed together? Best Wolfgang
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
On Fri, Aug 26, 2022 at 10:11 AM Robert Haas <robertmhaas@gmail.com> wrote: > Here's a small patch. Despite the small size of the patch, there are a > couple of debatable points here: Nobody's commented on this patch specifically, but it seemed like we had consensus that ALTER DEFAULT PRIVILEGES was doing The Wrong Thing, so I've pushed the patch I posted previously for that issue. -- Robert Haas EDB: http://www.enterprisedb.com
On Thu, Sep 8, 2022 at 1:06 PM <walther@technowledgy.de> wrote: > 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. I do not accept the argument that we've already made the decision that this should not depend on inheritance. It's pretty clear that we haven't thought carefully enough about which checks should depend only on membership, and which ones should depend on inheritance. The patch I committed just now to fix ALTER DEFAULT PRIVILEGES is one clear example of where we've gotten that wrong. We also changed the way predefined roles worked with inheritance not too long ago, so that they started using has_privs_of_role() rather than is_member_of_role(). Our past thinking on this topic has been fuzzy enough that we can't really conclude that because something uses is_member_of_role() now that's what it should continue to do in the future. We are working to get from a messy situation where the rules aren't consistent or understandable to one where they are, and that may mean changing some things. > 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. Now, having said that, this choice of behavior might have some advantages. It would mean that you could GRANT pg_read_all_settings TO someone WITH INHERIT TRUE, SET FALSE and that user would be able to read all settings but would not be able to create objects owned by pg_read_all_settings. It would also be upward-compatible with the existing behavior, which is nice. Well, maybe. Suppose that role A has been granted pg_read_all_settings WITH INHERIT TRUE, SET TRUE and role B has been granted pg_read_all_settings WITH INHERIT TRUE, SET FALSE. A can create a table owned by pg_read_all_settings. If A does that, then B can now create a trigger on that table and usurp the privileges of pg_read_all_settings, after which B can now create any number of objects owned by pg_read_all_settings. If A does not do that, though, I think that with the proposed rule, B would have no way to create objects owned by A. This is a bit unsatisfying. It seems like B should either have the right to usurp pg_read_all_settings's privileges or not, rather than maybe having that right depending on what some other user chooses to do. But maybe it's OK. It's hard to come up with perfect solutions here. One could take the view that the issue here is that pg_read_all_settings shouldn't have the right to create objects in the first place, and that this INHERIT vs. SET ROLE distinction is just a distraction. However, that would require accepting the idea that it's possible for a role to lack privileges granted to PUBLIC, which also sounds pretty unsatisfying. On the whole, I'm inclined to think it's reasonable to suppose that if you want to grant a role to someone without letting them create objects owned by that role, it should be a role that doesn't own any existing objects either. Essentially, that's legislating that predefined roles should be minimally privileged: they should hold the ability to do whatever it is that they are there to do (like read all settings) but not have any other privileges (like the ability to do stuff to objects they own). But maybe there's a better answer. Ideas/opinions welcome. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas: > Well, maybe. Suppose that role A has been granted pg_read_all_settings > WITH INHERIT TRUE, SET TRUE and role B has been granted > pg_read_all_settings WITH INHERIT TRUE, SET FALSE. A can create a > table owned by pg_read_all_settings. If A does that, then B can now > create a trigger on that table and usurp the privileges of > pg_read_all_settings, after which B can now create any number of > objects owned by pg_read_all_settings. I'm not seeing how this is possible. A trigger function would run with the invoking user's privileges by default, right? So B would have to create a trigger with a SECURITY DEFINER function, which is owned by pg_read_all_settings to actually usurp the privileges of that role. But creating objects with that owner is exactly the thing B can't do. What am I missing? Best Wolfgang
On Sun, Sep 25, 2022 at 5:08 AM Wolfgang Walther <walther@technowledgy.de> wrote: > Robert Haas: > > Well, maybe. Suppose that role A has been granted pg_read_all_settings > > WITH INHERIT TRUE, SET TRUE and role B has been granted > > pg_read_all_settings WITH INHERIT TRUE, SET FALSE. A can create a > > table owned by pg_read_all_settings. If A does that, then B can now > > create a trigger on that table and usurp the privileges of > > pg_read_all_settings, after which B can now create any number of > > objects owned by pg_read_all_settings. > > I'm not seeing how this is possible. A trigger function would run with > the invoking user's privileges by default, right? So B would have to > create a trigger with a SECURITY DEFINER function, which is owned by > pg_read_all_settings to actually usurp the privileges of that role. But > creating objects with that owner is exactly the thing B can't do. Yeah, my statement before wasn't correct. It appears that alice can't just usurp the privileges of pg_read_all_settings trivially, but she can create a trigger on any preexisting table owned by pg_read_all_settings and then anyone who performs an operation that causes that trigger to fire is at risk: rhaas=# create role alice; CREATE ROLE rhaas=# create table foo (a int, b text); CREATE TABLE rhaas=# alter table foo owner to pg_read_all_settings; ALTER TABLE rhaas=# grant pg_read_all_settings to alice; GRANT ROLE rhaas=# grant create on schema public to alice; GRANT rhaas=# set session authorization alice; SET rhaas=> create or replace function alice_function () returns trigger as $$begin raise notice 'this trigger is running as %', current_user; return null; end$$ language plpgsql; CREATE FUNCTION rhaas=> create trigger t1 before insert or update or delete on foo for each row execute function alice_function(); CREATE TRIGGER rhaas=> begin; BEGIN rhaas=*> insert into foo values (1, 'stuff'); NOTICE: this trigger is running as alice INSERT 0 0 rhaas=*> rollback; ROLLBACK rhaas=> reset session authorization; RESET rhaas=# begin; BEGIN rhaas=*# insert into foo values (1, 'stuff'); NOTICE: this trigger is running as rhaas INSERT 0 0 rhaas=*# rollback; ROLLBACK This shows that if rhaas (or whoever) performs DML on a table owned by pg_read_all_settings, he might trigger arbitrary code written by alice to run under his own user ID. Now, that hazard would exist anyway for tables owned by alice, but now it also exists for any tables owned by pg_read_all_settings. I'm not really sure how significant that is. If you can create triggers as some other user and that user ever does stuff as themselves, you can probably steal their privileges, because they will probably eventually do DML on one of their own tables and thereby execute your Trojan trigger. However, in the particular case of pg_read_all_settings, the intent is probably that nobody would ever run as that user, and there is probably also no reason to create tables or other objects owned by that user. So maybe we really can say that just blocking SET ROLE is enough. I'm slightly skeptical of that conclusion because the whole thing just feels a bit flimsy. Like, the whole idea that you can compromise your account by inserting a row into somebody else's table feels a little nuts to me. Triggers and row-level security policies make it easy to do things that look safe and are actually very dangerous. I think anyone would reasonably expect that calling a function owned by some other user might be risky, because who knows what that function might do, but it seems less obvious that accessing a table could execute arbitrary code, yet it can. And it is even less obvious that creating a table owned by one role might give some other role who inherits that user's privileges to booby-trap that table in a way that might fool a third user into doing something unsafe. But I have no idea what we could reasonably do to improve the situation. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas: > This shows that if rhaas (or whoever) performs DML on a table owned by > pg_read_all_settings, he might trigger arbitrary code written by alice > to run under his own user ID. Now, that hazard would exist anyway for > tables owned by alice, but now it also exists for any tables owned by > pg_read_all_settings. This hazard exists for all tables that alice has been granted the TRIGGER privilege on. While we prevent alice from creating tables that are owned by pg_read_all_settings, we do not prevent inheriting the TRIGGER privilege. > I'm slightly skeptical of that conclusion because the whole thing just > feels a bit flimsy. Like, the whole idea that you can compromise your > account by inserting a row into somebody else's table feels a little > nuts to me. Triggers and row-level security policies make it easy to > do things that look safe and are actually very dangerous. I think > anyone would reasonably expect that calling a function owned by some > other user might be risky, because who knows what that function might > do, but it seems less obvious that accessing a table could execute > arbitrary code, yet it can. And it is even less obvious that creating > a table owned by one role might give some other role who inherits that > user's privileges to booby-trap that table in a way that might fool a > third user into doing something unsafe. But I have no idea what we > could reasonably do to improve the situation. Right. This will always be the case when giving out the TRIGGER privilege on one of your tables to somebody else. There is two kind of TRIGGER privileges: An explicitly GRANTed privilege and an implicit privilege, that is given to the table owner. I think, when WITH INHERIT TRUE, SET FALSE is set, we should: - Inherit all explicitly granted privileges - Not inherit any DDL privileges implicitly given through ownership: CREATE, REFERENCES, TRIGGER. - Inherit all other privileges implicitly given through ownership (DML + others) Those implicit DDL privileges should be considered part of WITH SET TRUE. When you can't do SET ROLE x, then you can't act as the owner of any object owned by x. Or to put it the other way around: Only allow implicit ownership privileges to be executed when the CURRENT_USER is the owner. But provide a shortcut, when you have the WITH SET TRUE option on a role, so that you don't need to do SET ROLE + CREATE TRIGGER, but can just do CREATE TRIGGER instead. This is similar to the mental model of "requesting and accepting a transfer of ownership" with an implicit SET ROLE built-in, that I used before. Best Wolfgang
On Mon, Sep 26, 2022 at 12:16 PM Wolfgang Walther <walther@technowledgy.de> wrote: > I think, when WITH INHERIT TRUE, SET FALSE is set, we should: > - Inherit all explicitly granted privileges > - Not inherit any DDL privileges implicitly given through ownership: > CREATE, REFERENCES, TRIGGER. > - Inherit all other privileges implicitly given through ownership (DML + > others) I don't think we're going to be very happy if we redefine inheriting the privileges of another role to mean inheriting only some of them. That seems pretty counterintuitive to me. I also think that this particular definition is pretty fuzzy. Your previous proposal was to make the SET attribute of a GRANT control not only the ability to SET ROLE to the target role but also the ability to create objects owned by that role and/or transfer objects to that role. I think some people might find that behavior a little bit surprising - certainly, it goes beyond what the name SET implies - but it is at least simple enough to explain in one sentence, and the consequences don't seem too difficult to reason about. Here, though, it doesn't really seem simple enough to explain in one sentence, nor does it seem easy to reason about. I'm not sure that there's any firm distinction between DML privileges and DDL privileges. I'm not sure that the privileges that you mention are all and only those that are security-relevant, nor that it would necessarily remain true in the future even if it's true today. There are many operations which are permitted or declined just using an owner-check. One example is commenting on an object. That sure sounds like it would fit within your proposed "DDL privileges implicitly given through ownership" category, but it doesn't really present any security hazard, so I do not think there is a good reason to restrict that from a user who has INHERIT TRUE, SET FALSE. Another is renaming an object, which is a little more murky. You can't directly usurp someone's privileges by renaming an object that they own, but you could potentially rename an object out of the way and replace it with one that you own and thus induce a user to do something dangerous. I don't really want to make even small exceptions to the idea that inheriting a role's privileges means inheriting all of them, and I especially don't want to make large exceptions, or exceptions that involve judgement calls about the relative degree of risk of each possible operation. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas:
> I don't think we're going to be very happy if we redefine inheriting
> the privileges of another role to mean inheriting only some of them.
> That seems pretty counterintuitive to me. I also think that this
> particular definition is pretty fuzzy.
Scratch my previous suggestion. A new, less fuzyy definition would be:
Ownership is not a privilege itself and as such not inheritable.
When role A is granted to role B, two things happen:
1. Role B now has the right to use the GRANTed privileges of role A.
2. Role B now has the right to become role A via SET ROLE.
WITH SET controls whether point 2 is the case or not.
WITH INHERIT controls whether role B actually executes their right to
use those privileges ("inheritance") **and** whether the set role is
done implicitly for anything that requires ownership, but of course only
WITH SET TRUE.
This is the same way that the role attributes INHERIT / NOINHERIT behave.
> Your previous proposal was to make the SET attribute of a GRANT
> control not only the ability to SET ROLE to the target role but also
> the ability to create objects owned by that role and/or transfer
> objects to that role. I think some people might find that behavior a
> little bit surprising - certainly, it goes beyond what the name SET
> implies - but it is at least simple enough to explain in one sentence,
> and the consequences don't seem too difficult to reason about.
This would be included in the above.
> Here, though, it doesn't really seem simple enough to explain in one
> sentence, nor does it seem easy to reason about.
I think the "ownership is not inheritable" idea is easy to explain.
> There are many operations which are permitted or declined just using
> an owner-check. One example is commenting on an object. That sure
> sounds like it would fit within your proposed "DDL privileges
> implicitly given through ownership" category, but it doesn't really
> present any security hazard, so I do not think there is a good reason
> to restrict that from a user who has INHERIT TRUE, SET FALSE. Another
> is renaming an object, which is a little more murky. You can't
> directly usurp someone's privileges by renaming an object that they
> own, but you could potentially rename an object out of the way and
> replace it with one that you own and thus induce a user to do
> something dangerous. I don't really want to make even small exceptions
> to the idea that inheriting a role's privileges means inheriting all
> of them, and I especially don't want to make large exceptions, or
> exceptions that involve judgement calls about the relative degree of
> risk of each possible operation.
I would not make this about security-risks only. We didn't distinguish
between privileges and ownership that much before, because we didn't
have WITH INHERIT or WITH SET. Now that we have both, we could do so.
The ideas of "inherited GRANTs" and "a shortcut to avoid SET ROLE to do
owner-things" should be better to explain.
No judgement required.
All of this is to find a way to make WITH INHERIT TRUE, SET FALSE a
"real", risk-free thing - and not just some syntactic sugar. And if that
comes with the inability to COMMENT ON TABLE
owned_by_pg_read_all_settings... fine. No need for that at all.
However, it would come with the inability to do SELECT * FROM
owned_by_pg_read_all_settings, **unless** explicitly GRANTed to the
owner, too. This might feel strange at first, but should not be a
problem either. WITH INHERIT TRUE, SET FALSE is designed for built-in
roles or other container roles that group a set of privileges. Those
roles should not have objects they own anyway. And if they still do,
denying access to those objects unless explicitly granted is the safe way.
Best
Wolfgang
Greetings, * Robert Haas (robertmhaas@gmail.com) wrote: > On Thu, Sep 8, 2022 at 1:06 PM <walther@technowledgy.de> wrote: > > 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. > > I do not accept the argument that we've already made the decision that > this should not depend on inheritance. It's pretty clear that we > haven't thought carefully enough about which checks should depend only > on membership, and which ones should depend on inheritance. The patch > I committed just now to fix ALTER DEFAULT PRIVILEGES is one clear > example of where we've gotten that wrong. We also changed the way > predefined roles worked with inheritance not too long ago, so that > they started using has_privs_of_role() rather than > is_member_of_role(). Our past thinking on this topic has been fuzzy > enough that we can't really conclude that because something uses > is_member_of_role() now that's what it should continue to do in the > future. We are working to get from a messy situation where the rules > aren't consistent or understandable to one where they are, and that > may mean changing some things. Agreed that we haven't been good about the distinction between these, but that the recent work by Joshua and yourself has been moving us in the right direction. > One could take the view that the issue here is that > pg_read_all_settings shouldn't have the right to create objects in the > first place, and that this INHERIT vs. SET ROLE distinction is just a > distraction. However, that would require accepting the idea that it's > possible for a role to lack privileges granted to PUBLIC, which also > sounds pretty unsatisfying. On the whole, I'm inclined to think it's > reasonable to suppose that if you want to grant a role to someone > without letting them create objects owned by that role, it should be a > role that doesn't own any existing objects either. Essentially, that's > legislating that predefined roles should be minimally privileged: they > should hold the ability to do whatever it is that they are there to do > (like read all settings) but not have any other privileges (like the > ability to do stuff to objects they own). Predefined roles are special in that they should GRANT just the privileges that the role is described to GRANT and that users really shouldn't be able to SET ROLE to them nor should they be allowed to own objects, or at least that's my general feeling on them. If an administrator doesn't wish for a user to have the privileges provided by the predefined role by default, they should be able to set that up by creating another role who has that privilege which the user is able to SET ROLE to. That is: CREATE ROLE admin WITH INHERIT FALSE; GRANT pg_read_all_settings TO admin; GRANT admin TO alice; Would allow 'alice' to log in without the privileges associated with pg_read_all_settings but 'alice' is able to SET ROLE admin; and gain those privileges. It wasn't intended that 'alice' be able to SET ROLE to pg_read_all_settings itself though. * Robert Haas (robertmhaas@gmail.com) wrote: > Yeah, my statement before wasn't correct. It appears that alice can't > just usurp the privileges of pg_read_all_settings trivially, but she > can create a trigger on any preexisting table owned by > pg_read_all_settings and then anyone who performs an operation that > causes that trigger to fire is at risk: Triggers aren't the only thing to be worried about in this area- functions defined inside of views are also run with the privileges of the user running the SELECT and not as the owner of the view. The same is true of running SELECT against tables with RLS too, of course. Generally speaking, it's always been very risky to access the objects of users who you don't trust in any way and we don't currently provide any particularly easy way to make that kind of access safe. RLS at least provides an escape by allowing a user to turn it off, but the same isn't available for setting a search_path and then running queries or accessing views or running DML against tables with triggers. > I'm slightly skeptical of that conclusion because the whole thing just > feels a bit flimsy. Like, the whole idea that you can compromise your > account by inserting a row into somebody else's table feels a little > nuts to me. Triggers and row-level security policies make it easy to > do things that look safe and are actually very dangerous. I think > anyone would reasonably expect that calling a function owned by some > other user might be risky, because who knows what that function might > do, but it seems less obvious that accessing a table could execute > arbitrary code, yet it can. And it is even less obvious that creating > a table owned by one role might give some other role who inherits that > user's privileges to booby-trap that table in a way that might fool a > third user into doing something unsafe. But I have no idea what we > could reasonably do to improve the situation. Just to reiterate- this is not only about DML/triggers or RLS but also includes SELECT statements against views and setting of the search_path to that owned by someone trying to compromise your account (though the latter does require a bit more than just the SET itself). One approach to dealing with this would be to have a mechanism to define exactly what code you feel comfortable running and set that to be only the bootstrap superuser (or perhaps we'd have this as a superuser-set GUC list) and the current role and then fail any queries that end up calling code owned by any other role. * Wolfgang Walther (walther@technowledgy.de) wrote: > Robert Haas: > > This shows that if rhaas (or whoever) performs DML on a table owned by > > pg_read_all_settings, he might trigger arbitrary code written by alice > > to run under his own user ID. Now, that hazard would exist anyway for > > tables owned by alice, but now it also exists for any tables owned by > > pg_read_all_settings. > > This hazard exists for all tables that alice has been granted the TRIGGER > privilege on. While we prevent alice from creating tables that are owned by > pg_read_all_settings, we do not prevent inheriting the TRIGGER privilege. The issue here is that we don't prevent alice from issuing a 'SET' to pg_read_all_settings nor do we prevent predefined roles from creating objects. I'd be inclined to change the system to explicitly prevent both of those things from being allowed- for the special case of predefined roles and not as some general role capability. Maybe there's an argument that we should allow administrators to create roles that no user is allowed to SET to or which aren't allowed to create/own objects but I'm not sure that there's a strong use-case for that. I do think it's useful to allow administrators to create roles that *some* users are allowed to have the privileges are but aren't allowed to SET to, but the whole point of predefined roles is to use the role GRANT system as a more flexible way to give out certain distinct privileges to certain roles and that's it. > > I'm slightly skeptical of that conclusion because the whole thing just > > feels a bit flimsy. Like, the whole idea that you can compromise your > > account by inserting a row into somebody else's table feels a little > > nuts to me. Triggers and row-level security policies make it easy to > > do things that look safe and are actually very dangerous. I think > > anyone would reasonably expect that calling a function owned by some > > other user might be risky, because who knows what that function might > > do, but it seems less obvious that accessing a table could execute > > arbitrary code, yet it can. And it is even less obvious that creating > > a table owned by one role might give some other role who inherits that > > user's privileges to booby-trap that table in a way that might fool a > > third user into doing something unsafe. But I have no idea what we > > could reasonably do to improve the situation. > > Right. This will always be the case when giving out the TRIGGER privilege on > one of your tables to somebody else. Giving out of the TRIGGER privilege is really just a ill-conceived idea that we should acknowledge only exists because it's part of the standard. > There is two kind of TRIGGER privileges: An explicitly GRANTed privilege and > an implicit privilege, that is given to the table owner. That's really only half-right: TRIGGER is just one of the privileges that the owner has if there's no ACL on the table. If the ACL is defined and the owner's entry has the TRIGGER privilege removed then they'll lose the ability to create triggers on that table. Of course, the owner can simply GRANT that ability back to themselves if they wish but it's a useful distinction to be aware of. * Robert Haas (robertmhaas@gmail.com) wrote: > On Mon, Sep 26, 2022 at 12:16 PM Wolfgang Walther > <walther@technowledgy.de> wrote: > > I think, when WITH INHERIT TRUE, SET FALSE is set, we should: > > - Inherit all explicitly granted privileges > > - Not inherit any DDL privileges implicitly given through ownership: > > CREATE, REFERENCES, TRIGGER. > > - Inherit all other privileges implicitly given through ownership (DML + > > others) > > I don't think we're going to be very happy if we redefine inheriting > the privileges of another role to mean inheriting only some of them. > That seems pretty counterintuitive to me. I also think that this > particular definition is pretty fuzzy. I agree with Robert on this part. Inheriting the privileges of another role should generally mean exactly that and not some awkward subset of the privileges. > Your previous proposal was to make the SET attribute of a GRANT > control not only the ability to SET ROLE to the target role but also > the ability to create objects owned by that role and/or transfer > objects to that role. I think some people might find that behavior a > little bit surprising - certainly, it goes beyond what the name SET > implies - but it is at least simple enough to explain in one sentence, > and the consequences don't seem too difficult to reason about. I still feel it's useful to allow users to transfer object to roles that they can SET to even if they don't inherit the privileges of that role. I don't feel that should be allowed for predefined roles, however. One thing that I don't want to miss mentioning is that I'm not against the idea of predefined roles having ownership of some objects- but should that happen (tho I tend to doubt it will, because we usually use the bootstrap superuser for objects that admins can use but shouldn't be mucking around with and changing), those objects shouldn't be ones that are able to be messed with by anyone except a superuser running around with allow_system_table_mods or such. Thanks, Stephen
Attachment
Greetings, * Wolfgang Walther (walther@technowledgy.de) wrote: > Robert Haas: > > I don't think we're going to be very happy if we redefine inheriting > > the privileges of another role to mean inheriting only some of them. > > That seems pretty counterintuitive to me. I also think that this > > particular definition is pretty fuzzy. > > Scratch my previous suggestion. A new, less fuzyy definition would be: > Ownership is not a privilege itself and as such not inheritable. One of the reasons the role system was brought into being was explicitly to allow other roles to have ownership-level rights on objects that they didn't directly own. I don't see us changing that. Thanks, Stephen
Attachment
On Mon, Sep 26, 2022 at 3:16 PM Wolfgang Walther
<walther@technowledgy.de> wrote:
> Robert Haas:
> > I don't think we're going to be very happy if we redefine inheriting
> > the privileges of another role to mean inheriting only some of them.
> > That seems pretty counterintuitive to me. I also think that this
> > particular definition is pretty fuzzy.
>
> Scratch my previous suggestion. A new, less fuzyy definition would be:
> Ownership is not a privilege itself and as such not inheritable.
>
> When role A is granted to role B, two things happen:
> 1. Role B now has the right to use the GRANTed privileges of role A.
> 2. Role B now has the right to become role A via SET ROLE.
>
> WITH SET controls whether point 2 is the case or not.
>
> WITH INHERIT controls whether role B actually executes their right to
> use those privileges ("inheritance") **and** whether the set role is
> done implicitly for anything that requires ownership, but of course only
> WITH SET TRUE.
If I'm understanding correctly, this would amount to a major
redefinition of what it means to inherit privileges, and I think the
chances of such a change being accepted are approximately zero.
Inheriting privileges needs to keep meaning what it means now, namely,
you inherit all the rights of the granted role.
> > Here, though, it doesn't really seem simple enough to explain in one
> > sentence, nor does it seem easy to reason about.
>
> I think the "ownership is not inheritable" idea is easy to explain.
I don't. And even if I did think it were easy to explain, I don't
think it would be a good idea. One of my first patches to PostgreSQL
added a grantable TRUNCATE privilege to tables. I think that, under
your proposed definitions, the addition of this privilege would have
had the result that a role grant would cease to allow the recipient to
truncate tables owned by the granted role. There is currently a
proposal on the table to make VACUUM and ANALYZE grantable permissions
on tables, which would have the same issue. I think that if I made it
so that adding such privileges resulted in role inheritance not
working for those operations any more, people would come after me with
pitchforks. And I wouldn't blame them: that sounds terrible.
I think the only thing we should be discussing here is how to tighten
up the tests for operations in categories (1) and (2) in my original
email. The options so far proposed are: (a) do nothing, which makes
the proposed SET option on grants a lot less useful; (b) restrict
those operations by has_privs_of_role(), basically making them
dependent on the INHERIT option, (c) restrict them by
has_privs_of_role() || member_can_set_role(), requiring either the
INHERIT option or the SET option, or (d) restrict them by
member_can_set_role() only, i.e. making them depend on the SET option
alone. A broader reworking of what the INHERIT option means is not on
the table: I don't want to write a patch for it, I don't think it's a
good idea, and I don't think the community would accept it even if I
did want to write a patch for it and even if I did think it was a good
idea.
I would like to hear more opinions on that topic. I understand your
vote from among those four options to be (d). I do not know what
anyone else thinks.
Thanks,
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas: >> Scratch my previous suggestion. A new, less fuzyy definition would be: >> Ownership is not a privilege itself and as such not inheritable. >> [...] > If I'm understanding correctly, this would amount to a major > redefinition of what it means to inherit privileges, and I think the > chances of such a change being accepted are approximately zero. > Inheriting privileges needs to keep meaning what it means now, namely, > you inherit all the rights of the granted role. No. Inheriting stays the same, it's just WITH SET that's different from what it is "now". > I don't. And even if I did think it were easy to explain, I don't > think it would be a good idea. One of my first patches to PostgreSQL > added a grantable TRUNCATE privilege to tables. I think that, under > your proposed definitions, the addition of this privilege would have > had the result that a role grant would cease to allow the recipient to > truncate tables owned by the granted role. There is currently a > proposal on the table to make VACUUM and ANALYZE grantable permissions > on tables, which would have the same issue. I think that if I made it > so that adding such privileges resulted in role inheritance not > working for those operations any more, people would come after me with > pitchforks. And I wouldn't blame them: that sounds terrible. No, there is a misunderstanding. In my proposal, when you do WITH SET TRUE everything stays exactly the same as it is right now. I'm just saying WITH SET FALSE should take away more of the things you can do (all the ownership things) to a point where it's safe to GRANT .. WITH INHERIT TRUE, SET FALSE and still be useful for pre-defined or privilege-container roles. Could be discussed in the WITH SET thread, but it's a natural extension of the categories (1) and (2) in your original email. It's all about ownership. Best Wolfgang
On Tue, Sep 27, 2022 at 2:05 AM Wolfgang Walther <walther@technowledgy.de> wrote: > I'm just saying WITH SET FALSE should take away more of the things you > can do (all the ownership things) to a point where it's safe to GRANT .. > WITH INHERIT TRUE, SET FALSE and still be useful for pre-defined or > privilege-container roles. I don't see that as viable, either. It's too murky what you'd have to take away to make it safe, and it sounds like stuff that naturally falls under INHERIT rather than SET. -- Robert Haas EDB: http://www.enterprisedb.com
On Mon, 2022-09-19 at 15:32 -0400, Robert Haas wrote: > One could take the view that the issue here is that > pg_read_all_settings shouldn't have the right to create objects in > the > first place, and that this INHERIT vs. SET ROLE distinction is just a > distraction. However, that would require accepting the idea that it's > possible for a role to lack privileges granted to PUBLIC, which also > sounds pretty unsatisfying. On the whole, I'm inclined to think it's > reasonable to suppose that if you want to grant a role to someone > without letting them create objects owned by that role, it should be > a > role that doesn't own any existing objects either. Essentially, > that's > legislating that predefined roles should be minimally privileged: > they > should hold the ability to do whatever it is that they are there to > do > (like read all settings) but not have any other privileges (like the > ability to do stuff to objects they own). I like this approach -- the idea that you can create a role that can't own anything, can't create anything, and to which nobody else can "SET ROLE". Creating a "virtual" role like that feels much more declarative and easy to document: "this isn't a real user, it's just a collection of inheritable privileges". Even superusers couldn't "SET ROLE pg_read_all_settings" or "OWNER TO pg_signal_backend". I wouldn't call it "minimally privileged" (which feels wrong because it wouldn't even have privileges on PUBLIC, as you say); I'd just say that it's a type of role where those things just don't make sense. -- Jeff Davis PostgreSQL Contributor Team - AWS
On Mon, 2022-09-26 at 15:40 -0400, Stephen Frost wrote: > Predefined roles are special in that they should GRANT just the > privileges that the role is described to GRANT and that users really > shouldn't be able to SET ROLE to them nor should they be allowed to > own > objects, or at least that's my general feeling on them. What about granting privileges to others? I don't think that makes sense for a predefined role, either, because then they'd own a bunch of grants, which is as awkward as owning objects. > If an administrator doesn't wish for a user to have the privileges > provided by the predefined role by default, they should be able to > set > that up by creating another role who has that privilege which the > user > is able to SET ROLE to. And that other role could be used for grants, if needed, too. But I don't think we need to special-case predefined roles though. I think a lot of administrators would like to declare some roles that are just a collection of inheritable privileges. -- Jeff Davis PostgreSQL Contributor Team - AWS