Re: role self-revocation
On Mon, Mar 28, 2022 at 10:51 AM Stephen Frost wrote: > > > > So I propose to commit something like what I posted here: > > > > http://postgr.es/m/ca+tgmobgek0jraowqvpqhsxcfbdfitxsomoebhmmmhmj4gl...@mail.gmail.com > > > > > > +1, although the comments might need some more work. In particular, > > > I'm not sure that this bit is well stated: > > Also +1 on this. OK, done using Tom's proposed wording. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
Greetings, * Robert Haas (robertmh...@gmail.com) wrote: > On Thu, Mar 24, 2022 at 1:10 PM Tom Lane wrote: > > > However, it might. And if it does, I think it would be best if > > > removing that exception were the *only* change in this area made by > > > that release. > > > > Good idea, especially since it's getting to be too late to consider > > anything more invasive anyway. > > I'd say it's definitely too late at this point. Agreed. > > > So I propose to commit something like what I posted here: > > > http://postgr.es/m/ca+tgmobgek0jraowqvpqhsxcfbdfitxsomoebhmmmhmj4gl...@mail.gmail.com > > > > +1, although the comments might need some more work. In particular, > > I'm not sure that this bit is well stated: Also +1 on this. Thanks, Stephen signature.asc Description: PGP signature
Re: role self-revocation
On Thu, Mar 24, 2022 at 1:10 PM Tom Lane wrote: > > However, it might. And if it does, I think it would be best if > > removing that exception were the *only* change in this area made by > > that release. > > Good idea, especially since it's getting to be too late to consider > anything more invasive anyway. I'd say it's definitely too late at this point. > > So I propose to commit something like what I posted here: > > http://postgr.es/m/ca+tgmobgek0jraowqvpqhsxcfbdfitxsomoebhmmmhmj4gl...@mail.gmail.com > > +1, although the comments might need some more work. In particular, > I'm not sure that this bit is well stated: > > +* A role cannot have WITH ADMIN OPTION on itself, because that would > +* imply a membership loop. > > We already do consider a role to be a member of itself: > > regression=# create role r; > CREATE ROLE > regression=# grant r to r; > ERROR: role "r" is a member of role "r" > regression=# grant r to r with admin option; > ERROR: role "r" is a member of role "r" > > It might be better to just say "By policy, a role cannot have WITH ADMIN > OPTION on itself". But if you want to write a defense of that policy, > this isn't a very good one. That sentence is present in the current code, along with a bunch of other sentences, which the patch renders irrelevant. So I just deleted all of the other stuff and kept the sentence that is still relevant to the revised code. I think your proposed replacement is an improvement, but let's be careful not to get sucked into too much of a wordsmithing exercise in a patch that's here to make a functional change. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
Robert Haas writes: > Notwithstanding the lack of agreement on that point, I believe that > what we should do for v15 is remove the session user > self-administration exception. We have pretty much established that it > was originally introduced in error. Agreed. > However, it might. And if it does, I think it would be best if > removing that exception were the *only* change in this area made by > that release. Good idea, especially since it's getting to be too late to consider anything more invasive anyway. > So I propose to commit something like what I posted here: > http://postgr.es/m/ca+tgmobgek0jraowqvpqhsxcfbdfitxsomoebhmmmhmj4gl...@mail.gmail.com +1, although the comments might need some more work. In particular, I'm not sure that this bit is well stated: +* A role cannot have WITH ADMIN OPTION on itself, because that would +* imply a membership loop. We already do consider a role to be a member of itself: regression=# create role r; CREATE ROLE regression=# grant r to r; ERROR: role "r" is a member of role "r" regression=# grant r to r with admin option; ERROR: role "r" is a member of role "r" It might be better to just say "By policy, a role cannot have WITH ADMIN OPTION on itself". But if you want to write a defense of that policy, this isn't a very good one. regards, tom lane
Re: role self-revocation
On Fri, Mar 11, 2022 at 11:51 AM Robert Haas wrote: > On Fri, Mar 11, 2022 at 11:34 AM Tom Lane wrote: > > Note that either case would also involve making entries in pg_shdepend; > > although for the case of roles owned by/granted to the bootstrap > > superuser, we could omit those on the usual grounds that we don't need > > to record dependencies on pinned objects. > > That makes sense to me, but it still doesn't solve the problem of > agreeing on role ownership vs. WITH ADMIN OPTION vs. something else. Notwithstanding the lack of agreement on that point, I believe that what we should do for v15 is remove the session user self-administration exception. We have pretty much established that it was originally introduced in error. It later was found to be a security vulnerability, and that resulted in the exception being narrowed without removing it altogether. While there are differences of opinion on what the larger plan here ought to be, nobody's proposal plan involves retaining that exception. Neither has anyone offered a plausible use case for the current behavior, so there's no reason to think that removing it would break anything. However, it might. And if it does, I think it would be best if removing that exception were the *only* change in this area made by that release. If for v16 or v17 or v23 we implement Plan Tom or Plan Stephen or Plan Robert or something else, and along the way we remove that self-administration exception, we're going to have a real fire drill if it turns out that the self-administration exception was important for some reason we're not seeing right now. If, on the other hand, we remove that exception in v15, then if anything breaks, it'll be a lot easier to deal with. Worst case scenario we just revert the removal of that exception, which will be a very localized change if nothing else has been done that depends heavily on its having been removed. So I propose to commit something like what I posted here: http://postgr.es/m/ca+tgmobgek0jraowqvpqhsxcfbdfitxsomoebhmmmhmj4gl...@mail.gmail.com -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
> On Mar 14, 2022, at 7:38 AM, Stephen Frost wrote: > > So ... do you feel like that's now the case? Or were you looking for > more? I don't have any more questions at the moment. Thanks! — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: role self-revocation
Greetings, * Mark Dilger (mark.dil...@enterprisedb.com) wrote: > > On Mar 11, 2022, at 4:56 PM, Stephen Frost wrote: > > First … I outlined a fair bit of further description in the message you > > just responded to but neglected to include in your response, which strikes > > me as odd that you’re now asking for further explanation. > > > When it comes to completing the idea of role ownership- I didn’t come up > > with that idea nor champion it > > Sorry, not "completing", but "competing". It seems we're discussing > different ways to fix how roles and CREATEROLE work, and we have several > ideas competing against each other. (This differs from *people* competing > against each other, as I don't necessarily like the patch I wrote better than > I like your idea.) > > > and therefore I’m not really sure how many of the steps are required to > > fully support that concept..? > > There are problems that the ownership concepts solve. I strongly suspect > that your proposal could also solve those same problems, and just trying to > identify the specific portions of your proposal necessary to do so. I'm happy to help try to identify those, but it seems we'd need to have the exact problems that ownership solves defined first. Robert defined what he's looking for as: Robert Haas wrote: > Probably easier to just say it again: I want to have users that can > create roles and then have superuser-like powers with respect to those > roles. They can freely exercise the privileges of those roles, and > they can do all the things that a superuser can do but only with > respect to those roles. They cannot break out to the OS. I think it's > pretty similar to what you are describing, with a couple of possible > exceptions. For example, would you imagine that being an admin of a > login role would let you change that user's password? Because that > would be desirable behavior from where I sit. Which sure sounds like it's just about covered in step #1 of what I outlined before, except that the above description implies that one can't "get away" from the user who created their role, in which case we do need step #2 also. > > For my part, I would think that those steps necessary to satisfy the spec > > would get us pretty darn close to what true folks advocating for role > > ownership are asking for > > I have little idea what "true folks" means in this context. As for > "advocating for role ownership", I'm not in that group. Whether role > ownership or something else, I just want some solution to a set of problems, > mostly to do with needing superuser to do role management tasks. ... I'm not entirely sure what I meant there either, my hunch is that 'true' was actually just a leftover word from some other framing of that sentence and I had meant to remove it. Apologies for that. What I was trying to get at there is that steps #1 & #2 are the ones that I view as getting us closer to spec compliance and that doing so would get us to where Robert's ask above would be answered. > > , but that doesn’t include the superuser-only alter role attributes piece. > > Is that included in role ownership? I wouldn’t think so, but some might > > argue otherwise, and I don’t know that it is actually useful to divert into > > a discussion about what is or isn’t in that. > > Introducing the idea of role ownership doesn't fix that. But a patch which > introduces role ownership is useless unless CREATEROLE is also fixed. There > isn't any point having non-superusers create and own roles if, to do so, they > need a privilege which can break into superuser. But that argument is no > different with a patch along the lines of what you are proposing. CREATEROLE > needs fixing either way. There's a few ways to have the 'CREATEROLE' role attribute be fixed- - Remove it entirely (replacing with pg_create_role or such) - Remove its ability to GRANT out rights that the role running it doesn't have - Make it superfluous (leave it as-is, but add in pg_create_role which allows a role to create another role but doesn't include the magic GRANT whatever-role TO whatever-role that CREATEROLE has) I agree that we need to do something here to allow roles to create other roles while not having or being able to trivially get superuser themselves. > > If we agree that the role attribute bits are independent > > Yes. Great. > > then I think I agree that we need 1 and 2 to get the capabilities that the > > folks asking for role ownership want > > Yes. Ok. > > as 2 is where we make sure that one admin of a role can’t revoke another > > admin’s rights over that role. > > Exactly, so #2 is part of the competing proposal. (I get the sense you might > not see these as competing proposals, but I find that framing useful for > deciding which approach to pursue.) ... and is also part of getting us closer to the spec. > > Perhaps 2 isn’t strictly necessary in a carefully managed environment > > where
Re: role self-revocation
> On Mar 11, 2022, at 4:56 PM, Stephen Frost wrote: > > First … I outlined a fair bit of further description in the message you just > responded to but neglected to include in your response, which strikes me as > odd that you’re now asking for further explanation. > When it comes to completing the idea of role ownership- I didn’t come up > with that idea nor champion it Sorry, not "completing", but "competing". It seems we're discussing different ways to fix how roles and CREATEROLE work, and we have several ideas competing against each other. (This differs from *people* competing against each other, as I don't necessarily like the patch I wrote better than I like your idea.) > and therefore I’m not really sure how many of the steps are required to fully > support that concept..? There are problems that the ownership concepts solve. I strongly suspect that your proposal could also solve those same problems, and just trying to identify the specific portions of your proposal necessary to do so. > For my part, I would think that those steps necessary to satisfy the spec > would get us pretty darn close to what true folks advocating for role > ownership are asking for I have little idea what "true folks" means in this context. As for "advocating for role ownership", I'm not in that group. Whether role ownership or something else, I just want some solution to a set of problems, mostly to do with needing superuser to do role management tasks. > , but that doesn’t include the superuser-only alter role attributes piece. > Is that included in role ownership? I wouldn’t think so, but some might > argue otherwise, and I don’t know that it is actually useful to divert into a > discussion about what is or isn’t in that. Introducing the idea of role ownership doesn't fix that. But a patch which introduces role ownership is useless unless CREATEROLE is also fixed. There isn't any point having non-superusers create and own roles if, to do so, they need a privilege which can break into superuser. But that argument is no different with a patch along the lines of what you are proposing. CREATEROLE needs fixing either way. > If we agree that the role attribute bits are independent Yes. > then I think I agree that we need 1 and 2 to get the capabilities that the > folks asking for role ownership want Yes. > as 2 is where we make sure that one admin of a role can’t revoke another > admin’s rights over that role. Exactly, so #2 is part of the competing proposal. (I get the sense you might not see these as competing proposals, but I find that framing useful for deciding which approach to pursue.) > Perhaps 2 isn’t strictly necessary in a carefully managed environment where > no one else is given admin rights over the mini-superuser roles, but I’d > rather not have folks depending on that. I think it is necessary, and for the reason you say. > I’d still push back though and ask those advocating for this if it meets > what they’re asking for. I got the impression that it did but maybe I > misunderstood. > > In terms of exactly how things would work with these changes… I thought I > explained it pretty clearly, so it’s kind of hard to answer that further > without a specific question to answer. Did you have something specific in > mind? Perhaps I could answer a specific question and provide more clarity > that way. Your emails contained a lot of "we could do this or that depending on what people want, and maybe this other thing, but that isn't really necessary, and " which left me unclear on the proposal. I don't mean to disparage your communication style; it's just that when trying to distill technical details, high level conversation can be hard to grok. I have the sense that you aren't going to submit a patch, so I wanted this thread to contain enough detail for somebody else to do so. Thanks. — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: role self-revocation
Greetings, On Fri, Mar 11, 2022 at 19:03 Mark Dilger wrote: > > On Mar 11, 2022, at 2:46 PM, Stephen Frost wrote: > > > > I do think that’s reasonable … and believe I suggested it about 3 > messages ago in this thread. ;) (step #3 I think it was? Or maybe 4). > > Yes, and you mentioned it to me off-list. Indeed. I'm soliciting a more concrete specification for what you are proposing. > To me, that means understanding how the SQL spec behavior that you champion > translates into specific changes. You specified some of this in steps #1 > through #5, but I'd like a clearer indication of how many of those (#1 > alone, both #1 and #2, or what?) constitute a competing idea to the idea of > role ownership, and greater detail about how each of those steps translate > into specific behavior changes in postgres. Your initial five-step email > seems to be claiming that #1 by itself is competitive, but to me it seems > at least #1 and #2 would be required. First … I outlined a fair bit of further description in the message you just responded to but neglected to include in your response, which strikes me as odd that you’re now asking for further explanation. When it comes to completing the idea of role ownership- I didn’t come up with that idea nor champion it and therefore I’m not really sure how many of the steps are required to fully support that concept..? For my part, I would think that those steps necessary to satisfy the spec would get us pretty darn close to what true folks advocating for role ownership are asking for, but that doesn’t include the superuser-only alter role attributes piece. Is that included in role ownership? I wouldn’t think so, but some might argue otherwise, and I don’t know that it is actually useful to divert into a discussion about what is or isn’t in that. If we agree that the role attribute bits are independent then I think I agree that we need 1 and 2 to get the capabilities that the folks asking for role ownership want, as 2 is where we make sure that one admin of a role can’t revoke another admin’s rights over that role. Perhaps 2 isn’t strictly necessary in a carefully managed environment where no one else is given admin rights over the mini-superuser roles, but I’d rather not have folks depending on that. I’d still push back though and ask those advocating for this if it meets what they’re asking for. I got the impression that it did but maybe I misunderstood. In terms of exactly how things would work with these changes… I thought I explained it pretty clearly, so it’s kind of hard to answer that further without a specific question to answer. Did you have something specific in mind? Perhaps I could answer a specific question and provide more clarity that way. Thanks, Stephen >
Re: role self-revocation
> On Mar 11, 2022, at 2:46 PM, Stephen Frost wrote: > > I do think that’s reasonable … and believe I suggested it about 3 messages > ago in this thread. ;) (step #3 I think it was? Or maybe 4). Yes, and you mentioned it to me off-list. I'm soliciting a more concrete specification for what you are proposing. To me, that means understanding how the SQL spec behavior that you champion translates into specific changes. You specified some of this in steps #1 through #5, but I'd like a clearer indication of how many of those (#1 alone, both #1 and #2, or what?) constitute a competing idea to the idea of role ownership, and greater detail about how each of those steps translate into specific behavior changes in postgres. Your initial five-step email seems to be claiming that #1 by itself is competitive, but to me it seems at least #1 and #2 would be required. — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: role self-revocation
Greetings, On Fri, Mar 11, 2022 at 12:32 Mark Dilger wrote: > > > > On Mar 11, 2022, at 8:48 AM, Stephen Frost wrote: > > > > I agree that it would have an impact on backwards compatibility to > > change how WITH ADMIN works- but it would also get us more in line with > > what the SQL standard says for how WITH ADMIN is supposed to work and > > that seems worth the change to me. > > I'm fine with giving up some backwards compatibility to get some SQL > standard compatibility, as long as we're clear that is what we're doing. > What you say about the SQL spec isn't great, though, because too much power > is vested in "ADMIN". I see "ADMIN" as at least three separate privileges > together. Maybe it would be spec compliant to implement "ADMIN" as a > synonym for a set of separate privileges? I do think that’s reasonable … and believe I suggested it about 3 messages ago in this thread. ;) (step #3 I think it was? Or maybe 4). > On Mar 11, 2022, at 8:41 AM, Stephen Frost wrote: > > > > That we aren't discussing the issues with the current GRANT ... WITH > > ADMIN OPTION and how we deviate from what the spec calls for when it > > comes to DROP ROLE, which seems to be the largest thing that's > > 'solved' with this ownership concept, is concerning to me. > > Sure, let's discuss that a bit more. Here is my best interpretation of > your post about the spec, when applied to postgres with an eye towards not > doing any more damage than necessary: > > > On Mar 10, 2022, at 11:58 AM, Stephen Frost wrote: > > > > let's look at what the spec says: > > > > CREATE ROLE > > - Who is allowed to run CREATE ROLE is implementation-defined > > This should be anyone with membership in pg_create_role. That could be the case if we wished to go that route. I’d think in such case we’d then also remove CREATEROLE as otherwise the documentation feels like it’d be quite confusing. > - After creation, this is effictively run: > >GRANT new_role TO creator_role WITH ADMIN, GRANTOR "_SYSTEM" > > This should internally be implemented as three separate privileges, one > which means you can grant the role, another which means you can drop the > role, and a third that means you're a member of the role. That way, they > can be independently granted and revoked. We could make "WITH ADMIN" a > short-hand for "WITH G, D, M" where G, D, and M are whatever we name the > independent privileges Grant, Drop, and Member-of. I mean, sure, we can get there, and possibly add more like if you’re allowed to change or reset that role’s password and other things, but I don’t see that this piece is required as part of the very first change in this area. Further, WITH ADMIN already gives grant and member today, so you’re saying the only thing this change does that makes “WITH ADMIN” too powerful is adding DROP to it, yet that’s explicitly what the spec calls for. In short, I disagree that moving the DROP ROLE right from CREATEROLE roles having that across the entire system (excluding superusers) to WITH ADMIN where the role who has that right can: A) already become that role and drop all their objects B) already GRANT that role to some other role is a big issue. Splitting G and D helps with backwards compatibility, because it gives > people who want the traditional postgres "admin" a way to get there, by > granting "G+M". Splitting M from G and D makes it simpler to implement the > "bot" idea, since the bot shouldn't have M. But it does raise a question > about always granting G+D+M to the creator, since the bot is the creator > and we don't want the bot to have M. This isn't a problem I've invented > from thin air, mind you, as G+D+M is just the definition of ADMIN per the > SQL spec, if I've understood you correctly. So we need to think a bit more > about the pg_create_role built-in role and whether that needs to be further > refined to distinguish those who can get membership in roles they create > vs. those who cannot. This line of reasoning takes me in the direction of > what I think you were calling #5 upthread, but you'd have to elaborate on > that, and how it interacts with the spec, for us to have a useful > conversation about it. All that said, as I said before, I’m in favor of splitting things up and so if you want to do that as part of this initial work, sure. Idk that it’s absolutely required as part of this but I’m not going to complain if it’s included either. I agree that would allow folks to get something similar to what they could get today if they want. I agree that the split up helps with the “bot” idea, as we could at least then create a security definer function that the bot runs and which creates roles that the bot then has G for but not M or D. Even better would be to also provide a way for the “bot” to be able to create roles without the need for a security definer function where it doesn’t automatically get all three, and that was indeed what I was thinking about with the template idea. The
Re: role self-revocation
> On Mar 11, 2022, at 8:48 AM, Stephen Frost wrote: > > I agree that it would have an impact on backwards compatibility to > change how WITH ADMIN works- but it would also get us more in line with > what the SQL standard says for how WITH ADMIN is supposed to work and > that seems worth the change to me. I'm fine with giving up some backwards compatibility to get some SQL standard compatibility, as long as we're clear that is what we're doing. What you say about the SQL spec isn't great, though, because too much power is vested in "ADMIN". I see "ADMIN" as at least three separate privileges together. Maybe it would be spec compliant to implement "ADMIN" as a synonym for a set of separate privileges? > On Mar 11, 2022, at 8:41 AM, Stephen Frost wrote: > > That we aren't discussing the issues with the current GRANT ... WITH > ADMIN OPTION and how we deviate from what the spec calls for when it > comes to DROP ROLE, which seems to be the largest thing that's > 'solved' with this ownership concept, is concerning to me. Sure, let's discuss that a bit more. Here is my best interpretation of your post about the spec, when applied to postgres with an eye towards not doing any more damage than necessary: > On Mar 10, 2022, at 11:58 AM, Stephen Frost wrote: > > let's look at what the spec says: > > CREATE ROLE > - Who is allowed to run CREATE ROLE is implementation-defined This should be anyone with membership in pg_create_role. > - After creation, this is effictively run: >GRANT new_role TO creator_role WITH ADMIN, GRANTOR "_SYSTEM" This should internally be implemented as three separate privileges, one which means you can grant the role, another which means you can drop the role, and a third that means you're a member of the role. That way, they can be independently granted and revoked. We could make "WITH ADMIN" a short-hand for "WITH G, D, M" where G, D, and M are whatever we name the independent privileges Grant, Drop, and Member-of. Splitting G and D helps with backwards compatibility, because it gives people who want the traditional postgres "admin" a way to get there, by granting "G+M". Splitting M from G and D makes it simpler to implement the "bot" idea, since the bot shouldn't have M. But it does raise a question about always granting G+D+M to the creator, since the bot is the creator and we don't want the bot to have M. This isn't a problem I've invented from thin air, mind you, as G+D+M is just the definition of ADMIN per the SQL spec, if I've understood you correctly. So we need to think a bit more about the pg_create_role built-in role and whether that needs to be further refined to distinguish those who can get membership in roles they create vs. those who cannot. This line of reasoning takes me in the direction of what I think you were calling #5 upthread, but you'd have to elaborate on that, and how it interacts with the spec, for us to have a useful conversation about it. > DROP ROLE > - Any user who has been GRANT'd a role with ADMIN option is able to >DROP that role. Change this to "Any role who has D on the role". That's spec compliant, because anyone granted ADMIN necessarily has D. > GRANT ROLE > - No cycles allowed > - A role must have ADMIN rights on the role to be able to GRANT it to >another role. Change this to "Any role who has G on the role". That's spec compliant, because anyone grant ADMIN necessarily has G. We should also fix the CREATE ROLE command to require the grantor have G on a role in order to give it to the new role as part of the command. Changing the CREATEROLE, CREATEDB, REPLICATION, and BYPASSRLS attributes into pg_create_role, pg_create_db, pg_replication, and pg_bypassrls, the creator could only give them to the created role if the creator has G on the roles. If we do this, we could keep the historical privilege bits and their syntax support for backward compatibility, or we could rip them out, but the decision between those two options is independent of the rest of the design. — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: role self-revocation
On Fri, Mar 11, 2022 at 11:34 AM Tom Lane wrote: > Note that either case would also involve making entries in pg_shdepend; > although for the case of roles owned by/granted to the bootstrap > superuser, we could omit those on the usual grounds that we don't need > to record dependencies on pinned objects. That makes sense to me, but it still doesn't solve the problem of agreeing on role ownership vs. WITH ADMIN OPTION vs. something else. I find it ironic (and frustrating) that Mark implemented what I think is basically what you're arguing for, it got stuck because Stephen didn't like it, we then said OK so let's try to find out what Stephen would like, only to have you show up and say that it's right the way he already had it. I'm not saying that you're wrong, or for that matter that he's wrong. I'm just saying that if both of you are absolutely bent on having it the way you want it, either one of you is going to be sad, or we're not going to make any progress. Never mind the fact that neither of you seem interested in even giving a hearing to my preferred way of doing it. :-( -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
Greetings, * Robert Haas (robertmh...@gmail.com) wrote: > On Fri, Mar 11, 2022 at 11:12 AM Mark Dilger > wrote: > > This issue of how much backwards compatibility breakage we're willing to > > tolerate is just as important as questions about how we would want roles to > > work in a green-field development project. The sense I got a year ago, on > > this list, was that changing CREATEROLE was acceptable, but changing other > > parts of the system, such as how ADMIN OPTION works, would go too far. That we deviate as far as we do when it comes to the SQL spec is something that I don't feel like I had a good handle on when discussing this previously (that the spec doesn't talk about 'admin option' really but rather 'grantable authorization identifiers' or whatever it is doesn't help... but still, that's on me, sorry about that). > > Role ownership did not yet exist, and that was a big motivation in > > introducing that concept, because you couldn't credibly say it broke other > > existing features. It introduces the new notion that when a superuser > > creates a role, the superuser owns it, which is identical to how things > > implicitly work today; and when a CREATEROLE non-superuser creates a role, > > that role owns the new role, which is different from how it works today, > > arguably breaking CREATEROLE's prior behavior. *But it doesn't break > > anything else*. > > > > If we're going to change how ADMIN OPTION works, or how role membership > > works, or how inherit/noinherit works, let's first be clear that we are > > willing to accept whatever backwards incompatibility that entails. This is > > not a green-field development project. The constant spinning around with > > regard to how much compatibility we need to preserve is giving me vertigo. I agree that it would have an impact on backwards compatibility to change how WITH ADMIN works- but it would also get us more in line with what the SQL standard says for how WITH ADMIN is supposed to work and that seems worth the change to me. > On the other hand, changing ADMIN OPTION does have compatibility and > spec-compliance ramifications. I think Stephen is arguing that we can > solve this problem while coming closer to the spec, and I think we > usually consider getting closer to the spec to be a sufficient reason > for breaking backward compatibility (cf. standard_conforming_strings). Indeed. > But I don't know whether he is correct when he argues that the spec > makes admin option on a role sufficient to drop the role. I've never > had any luck understanding what the SQL specification is saying about > any topic. I'm happy to point you to what the spec says and to discuss it further if that would be helpful, or to get other folks to comment on it. I agree that it's definitely hard to grok at times. In this particular case what I'm looking at is, under DROP ROLE / Access Rules, there's only one sentence: There shall exist at least one grantable role authorization descriptor whose role name is R and whose grantee is an enabled authorization identifier. A bit of decoding: 'grantable role authorization descriptor' is a GRANT of a role WITH ADMIN OPTION. The role name 'R' is the role specified. The 'grantee' is who that role R was GRANT'd to, and 'enabled authorization identifier' is basically "has_privs_of_role()" (note that you can in the spec hvae roles that you're a member of but which are *not* currently enabled). Hopefully that helps. Thanks, Stephen signature.asc Description: PGP signature
Re: role self-revocation
Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Robert Haas writes: > > If we implement the link between the creating role and the created > > role as role ownership, then we are surely just going to add a > > rolowner column to pg_authid, and when the role is owned by nobody, I > > think we should always just store a valid OID in it, rather than > > sometimes storing 0. It just seems simpler. Any time we would store 0, > > store the bootstrap superuser's pg_authid.oid value instead. That way > > the OID is always valid, which probably lets us get by with fewer > > special cases in the code. We haven't got any particularly special cases in the code today for what happens if we run up the role hierarchy to a point that it ends and so I'm not sure why adding in a whole new concept around role ownership, which doesn't exist in the spec, would somehow leave us with fewer such special cases. > +1. > > Note that either case would also involve making entries in pg_shdepend; > although for the case of roles owned by/granted to the bootstrap > superuser, we could omit those on the usual grounds that we don't need > to record dependencies on pinned objects. That we aren't discussing the issues with the current GRANT ... WITH ADMIN OPTION and how we deviate from what the spec calls for when it comes to DROP ROLE, which seems to be the largest thing that's 'solved' with this ownership concept, is concerning to me. If we go down the route of adding role ownership, are we going to document that we explicitly go against the SQL standard when it comes to how DROP ROLE works? Or are we going to fix DROP ROLE? I'd much prefer the latter, but doing so then largely negates the point of this role ownership concept. I don't see how it makes sense to do both. Thanks, Stephen signature.asc Description: PGP signature
Re: role self-revocation
On Fri, Mar 11, 2022 at 11:12 AM Mark Dilger wrote: > This issue of how much backwards compatibility breakage we're willing to > tolerate is just as important as questions about how we would want roles to > work in a green-field development project. The sense I got a year ago, on > this list, was that changing CREATEROLE was acceptable, but changing other > parts of the system, such as how ADMIN OPTION works, would go too far. > > Role ownership did not yet exist, and that was a big motivation in > introducing that concept, because you couldn't credibly say it broke other > existing features. It introduces the new notion that when a superuser > creates a role, the superuser owns it, which is identical to how things > implicitly work today; and when a CREATEROLE non-superuser creates a role, > that role owns the new role, which is different from how it works today, > arguably breaking CREATEROLE's prior behavior. *But it doesn't break > anything else*. > > If we're going to change how ADMIN OPTION works, or how role membership > works, or how inherit/noinherit works, let's first be clear that we are > willing to accept whatever backwards incompatibility that entails. This is > not a green-field development project. The constant spinning around with > regard to how much compatibility we need to preserve is giving me vertigo. I mean, I agree that the backward compatibility ramifications of every idea need to be considered, but I agree even more that the amount of spinning around here is pretty insane. My feeling is that neither role owners nor tenants introduce any real concerns about backward-compatibility or, for that matter, SQL standards compliance, nonwithstanding Stephen's argument to the contrary. Every vendor extends the standard with their own stuff, and we've done that as well, as we can do it in more places. On the other hand, changing ADMIN OPTION does have compatibility and spec-compliance ramifications. I think Stephen is arguing that we can solve this problem while coming closer to the spec, and I think we usually consider getting closer to the spec to be a sufficient reason for breaking backward compatibility (cf. standard_conforming_strings). But I don't know whether he is correct when he argues that the spec makes admin option on a role sufficient to drop the role. I've never had any luck understanding what the SQL specification is saying about any topic. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
Robert Haas writes: > If we implement the link between the creating role and the created > role as role ownership, then we are surely just going to add a > rolowner column to pg_authid, and when the role is owned by nobody, I > think we should always just store a valid OID in it, rather than > sometimes storing 0. It just seems simpler. Any time we would store 0, > store the bootstrap superuser's pg_authid.oid value instead. That way > the OID is always valid, which probably lets us get by with fewer > special cases in the code. +1. Note that either case would also involve making entries in pg_shdepend; although for the case of roles owned by/granted to the bootstrap superuser, we could omit those on the usual grounds that we don't need to record dependencies on pinned objects. regards, tom lane
Re: role self-revocation
On Fri, Mar 11, 2022 at 10:46 AM Tom Lane wrote: > The bootstrap superuser clearly must be a special case in some way. > I'm not convinced that that means there should be other special > cases. Maybe there is a use-case for other "unowned" roles, but in > exactly what way would that be different from deeming such roles > to be owned by the bootstrap superuser? I think that just boils down to how many useless catalog entries you want to make. If we implement the link between the creating role and the created role as role ownership, then we are surely just going to add a rolowner column to pg_authid, and when the role is owned by nobody, I think we should always just store a valid OID in it, rather than sometimes storing 0. It just seems simpler. Any time we would store 0, store the bootstrap superuser's pg_authid.oid value instead. That way the OID is always valid, which probably lets us get by with fewer special cases in the code. If we implement the link between the creating role and the created role as an automatically-granted WITH ADMIN OPTION, then we could choose to put (CREATOR_OID, CREATED_OID, whatever, TRUE) into pg_auth_members for the creating superuser or, indeed, every superuser in the system. Or we can leave it out. The result will be exactly the same. Here, I would favor leaving it out, because extra catalog entries that don't do anything are usually a thing that we do not want. See a49d081235997c67e8aab7a523b17e8d1cb93184, for example. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
> On Mar 11, 2022, at 7:58 AM, Robert Haas wrote: > > This kind of thing is always a judgement call. If we were talking > about breaking 'SELECT * from table', I'm sure it would be hard to > convince anybody to agree to do that at all, let alone with no > deprecation period. Fortunately, CREATEROLE is less used, so breaking > it will inconvenience fewer people. This issue of how much backwards compatibility breakage we're willing to tolerate is just as important as questions about how we would want roles to work in a green-field development project. The sense I got a year ago, on this list, was that changing CREATEROLE was acceptable, but changing other parts of the system, such as how ADMIN OPTION works, would go too far. Role ownership did not yet exist, and that was a big motivation in introducing that concept, because you couldn't credibly say it broke other existing features. It introduces the new notion that when a superuser creates a role, the superuser owns it, which is identical to how things implicitly work today; and when a CREATEROLE non-superuser creates a role, that role owns the new role, which is different from how it works today, arguably breaking CREATEROLE's prior behavior. *But it doesn't break anything else*. If we're going to change how ADMIN OPTION works, or how role membership works, or how inherit/noinherit works, let's first be clear that we are willing to accept whatever backwards incompatibility that entails. This is not a green-field development project. The constant spinning around with regard to how much compatibility we need to preserve is giving me vertigo. — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: role self-revocation
On Fri, Mar 11, 2022 at 10:37 AM David G. Johnston wrote: > I largely agree and am perfectly fine with going with the majority on this > point. My vote would just fall on the conservative side. But as so far no > one else seems to be overly concerned, nerfing CREATEROLE seems to be the > path forward. This kind of thing is always a judgement call. If we were talking about breaking 'SELECT * from table', I'm sure it would be hard to convince anybody to agree to do that at all, let alone with no deprecation period. Fortunately, CREATEROLE is less used, so breaking it will inconvenience fewer people. Moreover, unlike 'SELECT * FROM table', CREATEROLE is kinda broken, and it's less scary to make changes to behavior that sucks in the first place than it is to make changes to the behavior of things that are working well. For all of that, there's no hard-and-fast rule that we couldn't keep the existing behavior around, introduce a substitute, and eventually drop the old thing. I'm just not clear that it's really worth it in this case. It'd certainly be interesting to hear from anyone who is finding some utility in the current system. It looks pretty crap to me, but it's easy to bring too much of one's own bias to such judgements. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
Robert Haas writes: > On Fri, Mar 11, 2022 at 10:27 AM Stephen Frost wrote: >> I agree that there would be a recorded relationship (that is, one that >> we write into the catalog and keep around until and unless it's removed >> by an admin) between creating and created roles and that's probably the >> default when CREATE ROLE is run but, unlike tables or such objects in >> the system, I don't agree that we should require this to exist at >> absolutely all times for every role (what would it be for the bootstrap >> superuser..?). At least today, that's distinct from how ownership in >> the system works. I also don't believe that this is necessarily an >> issue for Robert's use-case, as long as there are appropriate >> restrictions around who is allowed to remove or modify these >> relationships. > I agree. The bootstrap superuser clearly must be a special case in some way. I'm not convinced that that means there should be other special cases. Maybe there is a use-case for other "unowned" roles, but in exactly what way would that be different from deeming such roles to be owned by the bootstrap superuser? regards, tom lane
Re: role self-revocation
On Fri, Mar 11, 2022 at 10:27 AM Stephen Frost wrote: > I agree that there would be a recorded relationship (that is, one that > we write into the catalog and keep around until and unless it's removed > by an admin) between creating and created roles and that's probably the > default when CREATE ROLE is run but, unlike tables or such objects in > the system, I don't agree that we should require this to exist at > absolutely all times for every role (what would it be for the bootstrap > superuser..?). At least today, that's distinct from how ownership in > the system works. I also don't believe that this is necessarily an > issue for Robert's use-case, as long as there are appropriate > restrictions around who is allowed to remove or modify these > relationships. I agree. > > I agree. [ but we need to get consensus ] > > Well ... [ how about we do it my way? ] Repeating the same argument over again isn't necessarily going to help anything here. I read your argument and I can believe there could be a solution along those lines, although you haven't addressed my concern about NOINHERIT. Tom is apparently less convinced, and you know, I think that's OK. Not everybody has to agree with the way you want to do it. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
On Fri, Mar 11, 2022 at 8:32 AM Stephen Frost wrote: > > Such scripts as will break will still > break in a pretty clear way with a clear answer as to how to fix them > and I don't think there's some kind of data corruption or something that > would happen. > > I largely agree and am perfectly fine with going with the majority on this point. My vote would just fall on the conservative side. But as so far no one else seems to be overly concerned, nerfing CREATEROLE seems to be the path forward. David J.
Re: role self-revocation
Greetings, * David G. Johnston (david.g.johns...@gmail.com) wrote: > On Thu, Mar 10, 2022 at 3:01 PM Robert Haas wrote: > > > On Thu, Mar 10, 2022 at 4:00 PM David G. Johnston > > wrote: > > > I dislike changing the documented behavior of CREATEROLE to the degree > > suggested here. However, there are three choices here, only one of which > > can be chosen: > > > > > > 1. Leave CREATEROLE alone entirely > > > 2. Make it so CREATEROLE cannot assign membership to the predefined > > roles or superuser (inheritance included), but leave the rest alone. This > > would be the hard-coded version, not the role attribute one. > > > 3. Make it so CREATEROLE can only assign membership to roles for which > > it has been made an admin; as well as the other things mentioned > > > > > > Moving forward I'd prefer options 1 or 2, leaving the ability to > > create/alter/drop a role to be vested via predefined roles. > > > > It sounds like you prefer a behavior where CREATEROLE gives power over > > all non-superusers, but that seems pretty limiting to me. > > Doh! I edited out the part where I made clear I considered options 1 and 2 > as basically being done for a limited period of time while deprecating the > CREATEROLE attribute altogether in favor of the fine-grained and predefined > role based permission granting. I don't want to nerf CREATEROLE as part of > adding this new feature, instead leave it as close to status quo as > reasonable so as not to mess up existing setups that make use of it. We > can note in the release notes and documentation that we consider CREATEROLE > to be deprecated and that the new predefined role should be used to give a > user the ability to create/alter/drop roles, etc... DBAs should consider > revoking CREATEROLE from their users and granting them proper memberships > in the predefined roles and the groups those roles should be administering. I disagree entirely with the idea that we should push the out however many years it'd take to get through some deprecation period. We are absolutely terrible when it comes to that and what we're talking about here, at this point anyway, is making changes that get us closer to what the spec says. I agree that we can't back-patch these changes, but I don't think we need a deprecation period. If we were just getting rid of CREATEROLE, I don't think we'd have a deprecation period. If we need to get rid of CREATEROLE and introduce something new that more-or-less means the same thing, to make it so that people's scripts break in a more obvious way, maybe we can consider that, but I don't really think that's actually the case here. Such scripts as will break will still break in a pretty clear way with a clear answer as to how to fix them and I don't think there's some kind of data corruption or something that would happen. Thanks, Stephen signature.asc Description: PGP signature
Re: role self-revocation
On Fri, Mar 11, 2022 at 6:55 AM Robert Haas wrote: > On Thu, Mar 10, 2022 at 5:14 PM Tom Lane wrote: > > This seems reasonable in isolation, but > > > > (1) it implies a persistent relationship between creating and created > > roles. Whether you want to call that ownership or not, it sure walks > > and quacks like ownership. > > I like my TENANT idea best, but I'm perfectly willing to call > it ownership as you seem to prefer or WITH ADMIN OPTION as Stephen > seems to prefer if one of those ideas gains consensus. If WITH ADMIN OPTION is sufficient to meet our immediate goals I do not see the benefit of adding an ownership concept where there is not one today. If added, I'd much rather have it be ownership as to fit in with the rest of the existing system rather than introduce an entirely new term. > If Alice creates non-superusers Bob and Charlie, and Charlie creates > Doug, we need the persistent relationship to know that Charlie is > allowed to drop Doug and Bob is not > The interesting question seems to be whether Alice can drop Doug, not whether Bob can. > It's more important > at this point to get agreement on the principles. > What are the principles you want to get agreement on and how do they differ from what we have in place today? What are the proposed changes you would make to enforce the new principles. Which principles are now obsolete and what do you want to do about the features that were built to enforce them (including backward compatibility concerns)? David J.
Re: role self-revocation
Greetings, * Robert Haas (robertmh...@gmail.com) wrote: > On Thu, Mar 10, 2022 at 5:14 PM Tom Lane wrote: > > This seems reasonable in isolation, but > > > > (1) it implies a persistent relationship between creating and created > > roles. Whether you want to call that ownership or not, it sure walks > > and quacks like ownership. I agree that there would be a recorded relationship (that is, one that we write into the catalog and keep around until and unless it's removed by an admin) between creating and created roles and that's probably the default when CREATE ROLE is run but, unlike tables or such objects in the system, I don't agree that we should require this to exist at absolutely all times for every role (what would it be for the bootstrap superuser..?). At least today, that's distinct from how ownership in the system works. I also don't believe that this is necessarily an issue for Robert's use-case, as long as there are appropriate restrictions around who is allowed to remove or modify these relationships. > I agree. It's been obvious to me from the beginning that we needed > such a persistent relationship, and also that it needed to be a > relationship from which the created role couldn't simply walk away. > Yet, more than six months after the first discussions of this topic, > we still don't have any kind of agreement on what that thing should be > called. I like my TENANT idea best, but I'm perfectly willing to call > it ownership as you seem to prefer or WITH ADMIN OPTION as Stephen > seems to prefer if one of those ideas gains consensus. But we've > managed to waste all hope of making any significant progress here for > an entire release cycle for lack of ability to agree on spelling. I > think that's unfair to Mark, who put a lot of work into this area and > got nothing out of it, and I think it sucks for users of PostgreSQL, > too. Well ... one of those actually already exists and also happens to be in the SQL spec. I don't necessarily agree that we should absolutely require that the system always enforce that this relationship exist (I'd like a superuser to be able to get rid of it and to be able to change it too if they want) and that seems a bit saner than having the bootstrap superuser be special in some way here as would seem to otherwise be required. I also feel that it would be generally useful to have more than one of these relationships, if the user wishes, and that's something that ownership doesn't (directly) support today. Further, that's supported and expected by the SQL spec too. Even if we invented some concept of ownership of roles, it seems like we should make most of the other changes discussed here to bring us closer to what the spec says about CREATE ROLE, DROP ROLE, GRANT, REVOKE, etc. At that point though, what's the point of having ownership? > > (2) it seems exactly contradictory to your later point that > > > > > Agree. I also think that it would be a good idea to attribute grants > > > performed by any superuser to the bootstrap superuser, or leave them > > > unattributed somehow. Because otherwise dropping superusers becomes a > > > pain in the tail for no good reason. > > > > Either there's a persistent relationship or there's not. I don't > > think it's sensible to treat superusers differently here. > > > > I think that this argument about the difficulty of dropping superusers > > may in fact be the motivation for the existing behavior that object- > > permissions GRANTs done by superusers are attributed to the object > > owner; something you were unhappy about upthread. > > > > In the end these requirements seem mutually contradictory. Either > > we can have a persistent ownership relationship or not, but I don't > > think we can have it apply in some cases and not others without > > creating worse problems than we solve. I'm inclined to toss overboard > > the requirement that superusers need to be an easy thing to drop. > > Why is that important, anyway? > > Well, I think you're looking at it the wrong way. Compared to getting > useful functionality, the relative ease of dropping users is > completely unimportant. I'm happy to surrender it in exchange for > something else. I just don't see why we should give it up for nothing. > If Alice creates non-superusers Bob and Charlie, and Charlie creates > Doug, we need the persistent relationship to know that Charlie is > allowed to drop Doug and Bob is not. But if Charlie is a superuser > anyway, then the persistent relationship is of no use. I don't see the > point of cluttering up the system with such dependencies. Will I do it > that way, if that's what it takes to get the patch accepted? Sure. But > I can't imagine any end-user actually liking it. We need to know that Charlie is allowed to drop Doug and Bob isn't but that doesn't make it absolutely required that this be tracked permanently or that Alice can't decide later to make it such that Doug can't be dropped by Charlie for whatever reason
Re: role self-revocation
On Thu, Mar 10, 2022 at 5:14 PM Tom Lane wrote: > This seems reasonable in isolation, but > > (1) it implies a persistent relationship between creating and created > roles. Whether you want to call that ownership or not, it sure walks > and quacks like ownership. I agree. It's been obvious to me from the beginning that we needed such a persistent relationship, and also that it needed to be a relationship from which the created role couldn't simply walk away. Yet, more than six months after the first discussions of this topic, we still don't have any kind of agreement on what that thing should be called. I like my TENANT idea best, but I'm perfectly willing to call it ownership as you seem to prefer or WITH ADMIN OPTION as Stephen seems to prefer if one of those ideas gains consensus. But we've managed to waste all hope of making any significant progress here for an entire release cycle for lack of ability to agree on spelling. I think that's unfair to Mark, who put a lot of work into this area and got nothing out of it, and I think it sucks for users of PostgreSQL, too. > (2) it seems exactly contradictory to your later point that > > > Agree. I also think that it would be a good idea to attribute grants > > performed by any superuser to the bootstrap superuser, or leave them > > unattributed somehow. Because otherwise dropping superusers becomes a > > pain in the tail for no good reason. > > Either there's a persistent relationship or there's not. I don't > think it's sensible to treat superusers differently here. > > I think that this argument about the difficulty of dropping superusers > may in fact be the motivation for the existing behavior that object- > permissions GRANTs done by superusers are attributed to the object > owner; something you were unhappy about upthread. > > In the end these requirements seem mutually contradictory. Either > we can have a persistent ownership relationship or not, but I don't > think we can have it apply in some cases and not others without > creating worse problems than we solve. I'm inclined to toss overboard > the requirement that superusers need to be an easy thing to drop. > Why is that important, anyway? Well, I think you're looking at it the wrong way. Compared to getting useful functionality, the relative ease of dropping users is completely unimportant. I'm happy to surrender it in exchange for something else. I just don't see why we should give it up for nothing. If Alice creates non-superusers Bob and Charlie, and Charlie creates Doug, we need the persistent relationship to know that Charlie is allowed to drop Doug and Bob is not. But if Charlie is a superuser anyway, then the persistent relationship is of no use. I don't see the point of cluttering up the system with such dependencies. Will I do it that way, if that's what it takes to get the patch accepted? Sure. But I can't imagine any end-user actually liking it. > I'm a bit disturbed that parts of this discussion seem to be getting > conducted with little understanding of the system's existing behaviors. > We should not be reinventing things we already have perfectly good > solutions for in the object-privileges domain. I did wonder whether that might be the existing behavior, but stopping to check right at that moment didn't seem that important to me. Maybe I should have taken the time, but it's not like we're writing the final patch for commit next Tuesday at this point. It's more important at this point to get agreement on the principles. That said, I do agree that there have been times when we haven't thought hard enough about the existing behavior in proposing new behavior. On the third hand, though, part of the problem here is that neither Stephen nor I are entirely happy with the existing behavior, if for somewhat different reasons. It really isn't "perfectly good." On the one hand, from a purely technical standpoint, a lot of the behavior around roles in particular seems well below the standard that anyone would consider committable today. On the other hand, even the parts of the code that are in reasonable shape from a code quality point of view don't actually do the things that we think users want done. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
> On Mar 10, 2022, at 2:01 PM, Robert Haas wrote: > > It sounds like you prefer a behavior where CREATEROLE gives power over > all non-superusers, but that seems pretty limiting to me. Why can't > someone want to create a user with power over some users but not > others? I agree with Robert on this. Over at [1], I introduced a patch series to (a) change CREATEROLE and (b) introduce role ownership. Part (a) wasn't that controversial. The patch series failed to make it for postgres 15 on account of (b). The patch didn't go quite far enough, but with it applied, this is an example of a min-superuser "lord" operating within database "fiefdom": fiefdom=# -- mini-superuser who can create roles and write all data fiefdom=# CREATE ROLE lord fiefdom-# WITH CREATEROLE fiefdom-# IN ROLE pg_write_all_data; CREATE ROLE fiefdom=# fiefdom=# -- group which "lord" belongs to fiefdom=# CREATE GROUP squire fiefdom-# ROLE lord; CREATE ROLE fiefdom=# fiefdom=# -- group which "lord" has no connection to fiefdom=# CREATE GROUP paladin; CREATE ROLE fiefdom=# fiefdom=# SET SESSION AUTHORIZATION lord; SET fiefdom=> fiefdom=> -- fail, merely a member of "squire" fiefdom=> CREATE ROLE peon IN ROLE squire; ERROR: must have admin option on role "squire" fiefdom=> fiefdom=> -- fail, no privilege to grant CREATEDB fiefdom=> CREATE ROLE peon CREATEDB; ERROR: must have createdb privilege to create createdb users fiefdom=> fiefdom=> RESET SESSION AUTHORIZATION; RESET fiefdom=# fiefdom=# -- grant admin over "squire" to "lord" fiefdom=# GRANT squire fiefdom-# TO lord fiefdom-# WITH ADMIN OPTION; GRANT ROLE fiefdom=# fiefdom=# SET SESSION AUTHORIZATION lord; SET fiefdom=> fiefdom=> -- ok, have both "CREATEROLE" and admin option for "squire" fiefdom=> CREATE ROLE peon IN ROLE squire; CREATE ROLE fiefdom=> fiefdom=> -- fail, no privilege to grant CREATEDB fiefdom=> CREATE ROLE peasant CREATEDB IN ROLE squire; ERROR: must have createdb privilege to create createdb users fiefdom=> fiefdom=> RESET SESSION AUTHORIZATION; RESET fiefdom=# fiefdom=# -- Give lord the missing privilege fiefdom=# GRANT CREATEDB TO lord; ERROR: role "createdb" does not exist fiefdom=# fiefdom=# RESET SESSION AUTHORIZATION; RESET fiefdom=# fiefdom=# -- ok, have "CREATEROLE", "CREATEDB", and admin option for "squire" fiefdom=# CREATE ROLE peasant CREATEDB IN ROLE squire; CREATE ROLE The problem with this is that "lord" needs CREATEDB to grant CREATEDB, but really it should need something like grant option on "CREATEDB". But that's hard to do with the existing system, given the way these privilege bits are represented. If we added a few more built-in pg_* roles, such as pg_create_db, it would just work. CREATEROLE itself could be reimagined as pg_create_role, and then users could be granted into this role with or without admin option, meaning they could/couldn't further give it away. I think that would be a necessary component to Joshua's "bot" use-case, since the bot must itself have the privilege to create roles, but shouldn't necessarily be trusted with the privilege to create additional roles who have it. [1] https://www.postgresql.org/message-id/53c7df4c-8463-4647-9dfd-779b5e186...@amazon.com — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: role self-revocation
Robert Haas writes: > Probably easier to just say it again: I want to have users that can > create roles and then have superuser-like powers with respect to those > roles. They can freely exercise the privileges of those roles, and > they can do all the things that a superuser can do but only with > respect to those roles. This seems reasonable in isolation, but (1) it implies a persistent relationship between creating and created roles. Whether you want to call that ownership or not, it sure walks and quacks like ownership. (2) it seems exactly contradictory to your later point that > Agree. I also think that it would be a good idea to attribute grants > performed by any superuser to the bootstrap superuser, or leave them > unattributed somehow. Because otherwise dropping superusers becomes a > pain in the tail for no good reason. Either there's a persistent relationship or there's not. I don't think it's sensible to treat superusers differently here. I think that this argument about the difficulty of dropping superusers may in fact be the motivation for the existing behavior that object- permissions GRANTs done by superusers are attributed to the object owner; something you were unhappy about upthread. In the end these requirements seem mutually contradictory. Either we can have a persistent ownership relationship or not, but I don't think we can have it apply in some cases and not others without creating worse problems than we solve. I'm inclined to toss overboard the requirement that superusers need to be an easy thing to drop. Why is that important, anyway? > We might also need to think carefully about what happens if for > example the table owner is changed. If bob owns the table and we > change the owner to mary, but bob's previous grants are still > attributed to bob, I'm not sure that's going to be very convenient. That's already handled, is it not? regression=# create user alice; CREATE ROLE regression=# create user bob; CREATE ROLE regression=# create user charlie; CREATE ROLE regression=# \c - alice You are now connected to database "regression" as user "alice". regression=> create table alices_table (f1 int); CREATE TABLE regression=> grant select on alices_table to bob; GRANT regression=> \c - postgres You are now connected to database "regression" as user "postgres". regression=# alter table alices_table owner to charlie; ALTER TABLE regression=# \dp alices_table Access privileges Schema | Name | Type |Access privileges| Column privileges | Policies +--+---+-+---+-- public | alices_table | table | charlie=arwdDxt/charlie+| | | | | bob=r/charlie | | (1 row) I'm a bit disturbed that parts of this discussion seem to be getting conducted with little understanding of the system's existing behaviors. We should not be reinventing things we already have perfectly good solutions for in the object-privileges domain. regards, tom lane
Re: role self-revocation
On Thu, Mar 10, 2022 at 3:01 PM Robert Haas wrote: > On Thu, Mar 10, 2022 at 4:00 PM David G. Johnston > wrote: > > I dislike changing the documented behavior of CREATEROLE to the degree > suggested here. However, there are three choices here, only one of which > can be chosen: > > > > 1. Leave CREATEROLE alone entirely > > 2. Make it so CREATEROLE cannot assign membership to the predefined > roles or superuser (inheritance included), but leave the rest alone. This > would be the hard-coded version, not the role attribute one. > > 3. Make it so CREATEROLE can only assign membership to roles for which > it has been made an admin; as well as the other things mentioned > > > > Moving forward I'd prefer options 1 or 2, leaving the ability to > create/alter/drop a role to be vested via predefined roles. > > It sounds like you prefer a behavior where CREATEROLE gives power over > all non-superusers, but that seems pretty limiting to me. > Doh! I edited out the part where I made clear I considered options 1 and 2 as basically being done for a limited period of time while deprecating the CREATEROLE attribute altogether in favor of the fine-grained and predefined role based permission granting. I don't want to nerf CREATEROLE as part of adding this new feature, instead leave it as close to status quo as reasonable so as not to mess up existing setups that make use of it. We can note in the release notes and documentation that we consider CREATEROLE to be deprecated and that the new predefined role should be used to give a user the ability to create/alter/drop roles, etc... DBAs should consider revoking CREATEROLE from their users and granting them proper memberships in the predefined roles and the groups those roles should be administering. David J.
Re: role self-revocation
On Thu, Mar 10, 2022 at 5:00 PM Bruce Momjian wrote: > On Thu, Mar 10, 2022 at 02:22:05PM -0500, Robert Haas wrote: > > I mean, I didn't design pg_hba.conf, but I think it's part of the > > database doing a reasonable thing, not an external system doing a > > nonsensical thing. > > FYI, I think pg_hba.conf gets away with having negative/reject > permissions only because it is strictly ordered. I agree. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
On Thu, Mar 10, 2022 at 4:00 PM David G. Johnston wrote: > I dislike changing the documented behavior of CREATEROLE to the degree > suggested here. However, there are three choices here, only one of which can > be chosen: > > 1. Leave CREATEROLE alone entirely > 2. Make it so CREATEROLE cannot assign membership to the predefined roles or > superuser (inheritance included), but leave the rest alone. This would be > the hard-coded version, not the role attribute one. > 3. Make it so CREATEROLE can only assign membership to roles for which it has > been made an admin; as well as the other things mentioned > > Moving forward I'd prefer options 1 or 2, leaving the ability to > create/alter/drop a role to be vested via predefined roles. It sounds like you prefer a behavior where CREATEROLE gives power over all non-superusers, but that seems pretty limiting to me. Why can't someone want to create a user with power over some users but not others? For example, the superuser might want to give alice the ability to set up new users in the accounting department, but NOT give alice the right to tinker with the backup user (who is not a superuser, but doesn't have the replication privilege). How would they accomplish that in your view? -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
On Thu, Mar 10, 2022 at 02:22:05PM -0500, Robert Haas wrote: > I mean, I didn't design pg_hba.conf, but I think it's part of the > database doing a reasonable thing, not an external system doing a > nonsensical thing. FYI, I think pg_hba.conf gets away with having negative/reject permissions only because it is strictly ordered. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion.
Re: role self-revocation
On Thu, Mar 10, 2022 at 3:41 PM Stephen Frost wrote:
> > Gosh, I feel like I've spelled that out approximately 463,121 times
> > already. That estimate might be slightly off though; I've been known
> > to make mistakes from time to time
>
> If there's a specific message that details it closely on the lists
> somewhere, I'm happy to go review it. I admit that I didn't go back and
> look for such.
Probably easier to just say it again: I want to have users that can
create roles and then have superuser-like powers with respect to those
roles. They can freely exercise the privileges of those roles, and
they can do all the things that a superuser can do but only with
respect to those roles. They cannot break out to the OS. I think it's
pretty similar to what you are describing, with a couple of possible
exceptions. For example, would you imagine that being an admin of a
login role would let you change that user's password? Because that
would be desirable behavior from where I sit.
> Errr, just to be clear, ALTER ROLE doesn't exist *in the spec*. I
> wasn't suggesting that we get rid of it, just that it doesn't exist in
> the spec and therefore the spec doesn't have anything to say about it.
Oh, OK.
> > Basically, in this sketch, ADMIN OPTION on a role involves the ability
> > to DROP it, which means we don't need a separate role owner concept.
>
> Right. The above doesn't include any specifics about what to do with
> ALTER ROLE, but my thought would be to have it also be under ADMIN
> OPTION rather than under CREATEROLE, as I tried to outline (though not
> very well, I'll admit) below.
This sentence really confused me at first, but I think you're saying
that the right to alter a role would be dependent on having ADMIN
OPTION on the role rather than on having the CREATEROLE attribute.
That seems like a reasonable idea to me.
> > It also involves membership, meaning that you can freely exercise the
> > privileges of the role without SET ROLE. While I'm totally down with
> > having other possible behaviors as options, that particular behavior
> > seems very useful to me, so, sounds great.
>
> Well, yes and no- by default you're right, presuming everything is set
> as inheirited, but I'd wish for us to keep the option of creating roles
> which are noinherit and having that work just as it does today.
Hmm, so if I have membership WITH ADMIN OPTION in a role, but my role
is marked NOINHERIT, that means I can't exercise the privileges of
that role without SET ROLE. But, can I still do other things to that
role, such as dropping it? Given the current coding of
roles_is_member_of(), it seems like I can't. I don't like that, but
then I don't like much of anything about NOINHERIT. Do you have any
suggestions for how this could be improved?
To make this more concrete, suppose the superuser does "CREATE USER
alice CREATEROLE". Alice will have INHERIT, so she'll have control
over any roles she creates. But if she does "CREATE USER bob
CREATEROLE NOINHERIT" then neither she nor Bob will be able to control
the roles bob creates. I'd like to have a way to make it so that
neither Alice nor any other CREATEROLE users she spins up can create
roles over which they no longer have control. Because otherwise people
will do dumb stuff like that and then have to call the superuser to
sort it out, and the superuser won't like that because s/he is a super
busy person.
> > What do you mean by the 'drop role' exception?
>
> 'ability' was probably a better word there. What I'm talking about is
> changing in DropRole:
>
> to be, more or less:
>
> if (!is_admin_of_role(role))
> ereport(ERROR,
> (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> errmsg("permission denied to drop role")));
Sounds good.
> > I don't like removing 'alter role'.
>
> Ditto above but for AlterRole. Taking it away from users with
> CREATEROLE being able to run those commands on $anyrole and instead
> making it so that the role running DROP ROLE or ALTER ROLE needs to have
> ADMIN on the role they're messing with. I do think we may also need to
> make some adjustments in terms of what a regular user WITH ADMIN on a
> given role is able to do when it comes to ALTER ROLE, in particular, I
> don't think we'll want to remove the existing is-superuser checks
> against a user settings bypassrls or replication or superuser on some
> other role. Maybe we can provide a way for a non-superuser to be given
> the ability to set those attributes for roles they create, but that
> would be a separate thing.
This too.
> > > Step #2 is also in-line with the spec: track GRANTORs and care about
> > > them, for everything. We really should have been doing this all along.
> >
> > There are details to work out here, but in general, I like it.
>
> Cool. Note that superusers would still be able to do $anything,
> including removing someone's ADMIN rights on a role even if that
> superuser didn't GRANT it (at least, that's my
Re: role self-revocation
On Thu, Mar 10, 2022 at 12:58 PM Stephen Frost wrote: > I don't think we're that far from having all of these though. To start > with, we remove from CREATEROLE the random things that it does which go > beyond what folks tend to expect- remove the whole 'grant any role to > any other' stuff, remove the 'drop role' exception, remove the > 'alter role' stuff. Do make it so that when you create a role, however, > the above GRANT is effectively done. Now, for the items above where we > removed the checks against have_createrole_privilege() we go back and > add in checks using is_admin_of_role(). Of course, also remove the role > self-administration bug. > > That's step #1, but it gets us more-or-less what you're looking for, I > think, and brings us a lot closer to what the spec has. > That still leaves attribute specification in place: e.g., REPLICATION, CREATEROLE, CREATEDB, etc... (I see BYPASSRLS already is SUPERUSER only) I dislike changing the documented behavior of CREATEROLE to the degree suggested here. However, there are three choices here, only one of which can be chosen: 1. Leave CREATEROLE alone entirely 2. Make it so CREATEROLE cannot assign membership to the predefined roles or superuser (inheritance included), but leave the rest alone. This would be the hard-coded version, not the role attribute one. 3. Make it so CREATEROLE can only assign membership to roles for which it has been made an admin; as well as the other things mentioned Moving forward I'd prefer options 1 or 2, leaving the ability to create/alter/drop a role to be vested via predefined roles. The rest seems fine at an initial glance. David J.
Re: role self-revocation
Greetings,
* Robert Haas (robertmh...@gmail.com) wrote:
> On Thu, Mar 10, 2022 at 2:58 PM Stephen Frost wrote:
> > It'd be useful to have a better definition of exactly what a
> > 'mini-superuser' is, but at least for the moment when it comes to roles,
> > let's look at what the spec says:
>
> Gosh, I feel like I've spelled that out approximately 463,121 times
> already. That estimate might be slightly off though; I've been known
> to make mistakes from time to time
If there's a specific message that details it closely on the lists
somewhere, I'm happy to go review it. I admit that I didn't go back and
look for such.
> > CREATE ROLE
> > - Who is allowed to run CREATE ROLE is implementation-defined
> > - After creation, this is effictively run:
> > GRANT new_role TO creator_role WITH ADMIN, GRANTOR "_SYSTEM"
> >
> > DROP ROLE
> > - Any user who has been GRANT'd a role with ADMIN option is able to
> > DROP that role.
> >
> > GRANT ROLE
> > - No cycles allowed
> > - A role must have ADMIN rights on the role to be able to GRANT it to
> > another role.
> >
> > ALTER ROLE
> > - Doesn't exist
> >
> > This actually looks to me like more-or-less what you're looking for, it
> > just isn't what we have today because CREATEROLE brings along with it a
> > bunch of other stuff, some of which we want and some that we don't, and
> > some things that the SQL spec says ADMIN should be allowed to do (DROP
> > ROLE) we don't allow today.
>
> The above is mostly fine with me, except for the part about ALTER ROLE
> not existing. I think it's always good to be able to change your mind
> post-CREATE.
Errr, just to be clear, ALTER ROLE doesn't exist *in the spec*. I
wasn't suggesting that we get rid of it, just that it doesn't exist in
the spec and therefore the spec doesn't have anything to say about it.
> Basically, in this sketch, ADMIN OPTION on a role involves the ability
> to DROP it, which means we don't need a separate role owner concept.
Right. The above doesn't include any specifics about what to do with
ALTER ROLE, but my thought would be to have it also be under ADMIN
OPTION rather than under CREATEROLE, as I tried to outline (though not
very well, I'll admit) below.
> It also involves membership, meaning that you can freely exercise the
> privileges of the role without SET ROLE. While I'm totally down with
> having other possible behaviors as options, that particular behavior
> seems very useful to me, so, sounds great.
Well, yes and no- by default you're right, presuming everything is set
as inheirited, but I'd wish for us to keep the option of creating roles
which are noinherit and having that work just as it does today.
> > It's also not quite what I want because it requires that membership and
> > ADMIN go together where I'd like to be able to have those be
> > independently GRANT'able- and then some.
> >
> > I don't think we're that far from having all of these though. To start
> > with, we remove from CREATEROLE the random things that it does which go
> > beyond what folks tend to expect- remove the whole 'grant any role to
> > any other' stuff, remove the 'drop role' exception, remove the
> > 'alter role' stuff. Do make it so that when you create a role, however,
> > the above GRANT is effectively done. Now, for the items above where we
> > removed the checks against have_createrole_privilege() we go back and
> > add in checks using is_admin_of_role(). Of course, also remove the role
> > self-administration bug.
>
> What do you mean by the 'drop role' exception?
'ability' was probably a better word there. What I'm talking about is
changing in DropRole:
if (!have_createrole_privilege())
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to drop role")));
to be, more or less:
if (!is_admin_of_role(role))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to drop role")));
> I don't like removing 'alter role'.
Ditto above but for AlterRole. Taking it away from users with
CREATEROLE being able to run those commands on $anyrole and instead
making it so that the role running DROP ROLE or ALTER ROLE needs to have
ADMIN on the role they're messing with. I do think we may also need to
make some adjustments in terms of what a regular user WITH ADMIN on a
given role is able to do when it comes to ALTER ROLE, in particular, I
don't think we'll want to remove the existing is-superuser checks
against a user settings bypassrls or replication or superuser on some
other role. Maybe we can provide a way for a non-superuser to be given
the ability to set those attributes for roles they create, but that
would be a separate thing.
> The rest sounds good.
Great.
> > That's step #1, but it gets us more-or-less what you're looking for, I
> > think, and brings us a lot closer to what the spec has.
>
> Great.
Awesome.
> >
Re: role self-revocation
On Thu, Mar 10, 2022 at 2:58 PM Stephen Frost wrote: > It'd be useful to have a better definition of exactly what a > 'mini-superuser' is, but at least for the moment when it comes to roles, > let's look at what the spec says: Gosh, I feel like I've spelled that out approximately 463,121 times already. That estimate might be slightly off though; I've been known to make mistakes from time to time > CREATE ROLE > - Who is allowed to run CREATE ROLE is implementation-defined > - After creation, this is effictively run: > GRANT new_role TO creator_role WITH ADMIN, GRANTOR "_SYSTEM" > > DROP ROLE > - Any user who has been GRANT'd a role with ADMIN option is able to > DROP that role. > > GRANT ROLE > - No cycles allowed > - A role must have ADMIN rights on the role to be able to GRANT it to > another role. > > ALTER ROLE > - Doesn't exist > > This actually looks to me like more-or-less what you're looking for, it > just isn't what we have today because CREATEROLE brings along with it a > bunch of other stuff, some of which we want and some that we don't, and > some things that the SQL spec says ADMIN should be allowed to do (DROP > ROLE) we don't allow today. The above is mostly fine with me, except for the part about ALTER ROLE not existing. I think it's always good to be able to change your mind post-CREATE. Basically, in this sketch, ADMIN OPTION on a role involves the ability to DROP it, which means we don't need a separate role owner concept. It also involves membership, meaning that you can freely exercise the privileges of the role without SET ROLE. While I'm totally down with having other possible behaviors as options, that particular behavior seems very useful to me, so, sounds great. > It's also not quite what I want because it requires that membership and > ADMIN go together where I'd like to be able to have those be > independently GRANT'able- and then some. > > I don't think we're that far from having all of these though. To start > with, we remove from CREATEROLE the random things that it does which go > beyond what folks tend to expect- remove the whole 'grant any role to > any other' stuff, remove the 'drop role' exception, remove the > 'alter role' stuff. Do make it so that when you create a role, however, > the above GRANT is effectively done. Now, for the items above where we > removed the checks against have_createrole_privilege() we go back and > add in checks using is_admin_of_role(). Of course, also remove the role > self-administration bug. What do you mean by the 'drop role' exception? I don't like removing 'alter role'. The rest sounds good. > That's step #1, but it gets us more-or-less what you're looking for, I > think, and brings us a lot closer to what the spec has. Great. > Step #2 is also in-line with the spec: track GRANTORs and care about > them, for everything. We really should have been doing this all along. > Note that I'm not saying that an owner of a table can't REVOKE some > right that was GRANT'd on that table, but rather that a user who was > GRANT'd ADMIN rights on a table and then GRANT'd that right to some > other user shouldn't have some other user who only has ADMIN rights on > the table be able to remove that GRANT. Same goes for roles, meaning > that you could GRANT rights in a role with ADMIN option and not have to > be afraid that the role you just gave that to will be able to remove > *your* ADMIN rights on that role. In general, I don't think this > would actually have a very large impact on users because most users > don't, today, use the ADMIN option much. There are details to work out here, but in general, I like it. > Step #3 starts going in the direction of what I'd like to see, which > would be to break out membership in a role as a separate thing from > admin rights on that role. This is also what would help with the 'bot' > use-case that Joshua (not David Steele, btw) brought up. Woops, apologies for getting the name wrong. I also said Marc earlier when I meant Mark, because I work with people named Mark, Marc, and Marc, and Mark's spelling got outvoted by some distant corner of my brain. I think this is a fine long-term direction, with the caveat that you've not provided enough specifics here for me to really understand how it would work. I fear the specifics might be hard to get right, both in terms of making it understandable to users and in terms of preserving as much backward-compatibility as we can. However, I am not opposed to the concept. > Step #4 then breaks the 'admin' option on roles into pieces- a 'drop > role' right, a 'reset password' right, maybe separate rights for > different role attributes, etc. We would likely still keep the > 'admin_option' column in pg_auth_members and just check that first > and then check the individual rights (similar to table-level vs. > column-level privileges) so that we stay in line with the spec's > expectation here and with what users are used to.
Re: role self-revocation
On Thu, Mar 10, 2022 at 12:45 PM Stephen Frost wrote: > > * David G. Johnston (david.g.johns...@gmail.com) wrote: > > On Thu, Mar 10, 2022 at 11:05 AM Stephen Frost > wrote: > Why not just look at the admin_option field of pg_auth_members...? I > don't get why that isn't an even more minimal fix than this idea you > have of adding a column to pg_authid and then propagating around "this > user could become a superuser" or writing code that has to go check "is > there some way for this role to become a superuser, either directly or > through some subset of pg_* roles?" > Indeed, maybe I am wrong on the scope of the patch. But at least for the explicit attribute it should be no more difficult than changing: if (grouprole_is_superuser and current_role_is_not_superuser) then error: to be if ((grouoprole_is_superuser OR !groupuser_has_adminattr) AND current_role_is_not_superuser) then error; I have to imagine that given how fundamental inheritance is to our permissions system than doing a similar check up the tree wouldn't be difficult, but I truly don't know with a strong degree of certainty. Assuming we don't actually rip out CREATEROLE when this change goes in...do you propose to prohibit a CREATEROLE user from altering the membership roster of any group which itself is not a member of and also those which it is a member of but where admin_option is false? I don't personally have a problem with the current state where CREATEROLE is an admin for, but not a member of, every non-superuser(-related) role in the system. If the consensus is to change that then I suppose this becomes the minimally invasive fix that accomplishes that goal as well. It seems incomplete though, since you still need superuser to create a group and add the initial WITH ADMIN member to it. So this seems to work in the "avoid using superuser" sense if you've also added something that has what CREATEROLE provides today - admin without membership - but that would have the benefit of not carrying around all the baggage that CREATEROLE has. > > I made the observation that being able to manage the membership of a > group > > without having the ability to create new users seems like a half a loaf > of > > a feature. That's it. I would presume that any redesign of the > > permissions system here would address this adequately. > > If the new design ideas that are being thrown around don't address what > you're thinking they should, it'd be great to point that out. > I mean, you need a Create Role permission in some form, even if it's deprecating the attribute and making it a predefined role. I picked this thread up because it seemed like a limited scope that I could get my head around with the time I have, with the main goal to try to understand this aspect of the system better. I haven't gone and looked into the main thread yet. David J.
Re: role self-revocation
Greetings, * Robert Haas (robertmh...@gmail.com) wrote: > But that's not to say that we couldn't decide to do something else > instead, and that other thing might well be better. Do you want to > sketch out a full proposal, even just what the syntax would look like, > and share that here? And if you could explain how I could use it to > create the mini-superusers that I'm trying to get out of this thing, > even better. It'd be useful to have a better definition of exactly what a 'mini-superuser' is, but at least for the moment when it comes to roles, let's look at what the spec says: CREATE ROLE - Who is allowed to run CREATE ROLE is implementation-defined - After creation, this is effictively run: GRANT new_role TO creator_role WITH ADMIN, GRANTOR "_SYSTEM" DROP ROLE - Any user who has been GRANT'd a role with ADMIN option is able to DROP that role. GRANT ROLE - No cycles allowed - A role must have ADMIN rights on the role to be able to GRANT it to another role. ALTER ROLE - Doesn't exist This actually looks to me like more-or-less what you're looking for, it just isn't what we have today because CREATEROLE brings along with it a bunch of other stuff, some of which we want and some that we don't, and some things that the SQL spec says ADMIN should be allowed to do (DROP ROLE) we don't allow today. It's also not quite what I want because it requires that membership and ADMIN go together where I'd like to be able to have those be independently GRANT'able- and then some. I don't think we're that far from having all of these though. To start with, we remove from CREATEROLE the random things that it does which go beyond what folks tend to expect- remove the whole 'grant any role to any other' stuff, remove the 'drop role' exception, remove the 'alter role' stuff. Do make it so that when you create a role, however, the above GRANT is effectively done. Now, for the items above where we removed the checks against have_createrole_privilege() we go back and add in checks using is_admin_of_role(). Of course, also remove the role self-administration bug. That's step #1, but it gets us more-or-less what you're looking for, I think, and brings us a lot closer to what the spec has. Step #2 is also in-line with the spec: track GRANTORs and care about them, for everything. We really should have been doing this all along. Note that I'm not saying that an owner of a table can't REVOKE some right that was GRANT'd on that table, but rather that a user who was GRANT'd ADMIN rights on a table and then GRANT'd that right to some other user shouldn't have some other user who only has ADMIN rights on the table be able to remove that GRANT. Same goes for roles, meaning that you could GRANT rights in a role with ADMIN option and not have to be afraid that the role you just gave that to will be able to remove *your* ADMIN rights on that role. In general, I don't think this would actually have a very large impact on users because most users don't, today, use the ADMIN option much. Step #3 starts going in the direction of what I'd like to see, which would be to break out membership in a role as a separate thing from admin rights on that role. This is also what would help with the 'bot' use-case that Joshua (not David Steele, btw) brought up. Step #4 then breaks the 'admin' option on roles into pieces- a 'drop role' right, a 'reset password' right, maybe separate rights for different role attributes, etc. We would likely still keep the 'admin_option' column in pg_auth_members and just check that first and then check the individual rights (similar to table-level vs. column-level privileges) so that we stay in line with the spec's expectation here and with what users are used to. In some hyptothetical world, there's even a later step #5 which allows us to define user profiles and then grant the ability for a user to create a role with a certain profile (but not any arbitrary profile), thus making things like the 'bot' even more constrained in terms of what it's able to do (maybe it can then create a role that's a member of a role without itself being a member of that role or explicitly having admin rights in that role, as an example). Thanks, Stephen signature.asc Description: PGP signature
Re: role self-revocation
Greetings, * David G. Johnston (david.g.johns...@gmail.com) wrote: > On Thu, Mar 10, 2022 at 11:05 AM Stephen Frost wrote: > > * David G. Johnston (david.g.johns...@gmail.com) wrote: > > > On Thu, Mar 10, 2022 at 9:19 AM Stephen Frost wrote: > > > > * David G. Johnston (david.g.johns...@gmail.com) wrote: > > > > > On Thu, Mar 10, 2022 at 7:46 AM Robert Haas > > > > > wrote: > > > > > The only indirect way for a role to become superuser is to have been > > > granted membership in a superuser group, then SET ROLE. Non-superusers > > > cannot do this. If a superuser does this I consider the outcome to be no > > > different than if they go and do: > > > > A non-superuser absolutely can be GRANT'd membership in a superuser role > > and then SET ROLE to that user thus becoming a superuser. > > A non-superuser cannot grant a non-superuser membership in a superuser > role. A superuser granting a user membership in a superuser role makes > that user a superuser. This seems sane. > > If a superuser grants a non-superuser membership in a superuser role then > today a non-superuser can grant a user membership in that intermediate > role, thus having a non-superuser make another user a superuser. This is > arguably a bug that needs to be fixed. > > My desired fix is to just require the superuser to mark (or have it marked > by default ideally) the role inheriting superuser and put the > responsibility on the superuser. I agree this is not ideal, but it is > probably quick and low risk. > > I'll let someone else describe the details of the alternative option. I > suspect it will end up being a better option in terms of design. But > depending on time and risk even knowing that we want the better design > eventually doesn't preclude getting the easier fix in now. > > > No, what should matter is if the role doing the GRANT has admin rights > > on pg_read_all_stats, or on the postgres role. That also happens to be > > what the spec says. > > Yes, and superusers implicitly have that right, while CREATEROLE users > implicitly have that right on the pg_* role but not on superuser roles. I > just want to plug that hole and include the pg_* roles (or any role for > that matter) in being able to be denied implied ADMIN rights for > non-superusers. CREATEROLE users implicitly have that right on *all non-superuser roles*. Not just the pg_* ones, which is why the pg_* ones aren't any different in this regard. > > Today a non-superuser cannot "grant postgres to someuser;" > > > > No, but a role can be created like 'admin', which a superuser GRANT's > > 'postgres' to and then that role can be GRANT'd to anyone by anyone who > > has CREATEROLE rights. That's not sane. > > I agree. And I've suggested a minimal fix, adding an attribute to the role > that prohibits non-superusers from granting it to others, that removes the > insane behavior. I disagree that this is a minimal fix as I don't see it as a fix to the actual issue, which is the ability for CREATEROLE users to GRANT role membership to all non-superuser roles on the system. CREATEROLE shouldn't be allowing that. > I'm on board for a hard-coded fix as well - if a superuser is in the > membership chain of a role then non-superusers cannot grant membership in > that role to others. Why not just look at the admin_option field of pg_auth_members...? I don't get why that isn't an even more minimal fix than this idea you have of adding a column to pg_authid and then propagating around "this user could become a superuser" or writing code that has to go check "is there some way for this role to become a superuser, either directly or through some subset of pg_* roles?" > Neither of those really solves the pg_* roles problem. We still need to > indicate that they are somehow special. Whether it is a nice matrix or > roles and permissions or a simple attribute that makes them behave like > they are superuser roles. I disagree that they should be considered special when it comes to role membership and management. They're just roles, like any other. > > I disagree entirely with the idea that we must have some roles who can > > only ever be administered by a superuser. > > I don't think this is a must have. I think that since we do have it today > that fixes that leverage the status quo in order to be done more easily are > perfectly valid solutions. We have a half-way-implemented attempt at this, not something that's actually effective, and therefore I don't agree that we really have it today or that we should keep it. I'd much prefer to throw out nearly everything in the system that's doing an explicit check of "does this role have a superuser bit set on it?" > > If anything, we should be > > moving away (as we have, in fact, been doing), from anything being the > > exclusive purview of the superuser. > > > > I totally agree. Great. > > > In particular, this means CREATEROLE roles cannot assign membership in > > the > > > marked roles; just
Re: role self-revocation
On Thu, Mar 10, 2022 at 11:05 AM Stephen Frost wrote: > Greetings, > > * David G. Johnston (david.g.johns...@gmail.com) wrote: > > On Thu, Mar 10, 2022 at 9:19 AM Stephen Frost > wrote: > > > * David G. Johnston (david.g.johns...@gmail.com) wrote: > > > > On Thu, Mar 10, 2022 at 7:46 AM Robert Haas > > > wrote: > > > The only indirect way for a role to become superuser is to have been > > granted membership in a superuser group, then SET ROLE. Non-superusers > > cannot do this. If a superuser does this I consider the outcome to be no > > different than if they go and do: > > A non-superuser absolutely can be GRANT'd membership in a superuser role > and then SET ROLE to that user thus becoming a superuser. A non-superuser cannot grant a non-superuser membership in a superuser role. A superuser granting a user membership in a superuser role makes that user a superuser. This seems sane. If a superuser grants a non-superuser membership in a superuser role then today a non-superuser can grant a user membership in that intermediate role, thus having a non-superuser make another user a superuser. This is arguably a bug that needs to be fixed. My desired fix is to just require the superuser to mark (or have it marked by default ideally) the role inheriting superuser and put the responsibility on the superuser. I agree this is not ideal, but it is probably quick and low risk. I'll let someone else describe the details of the alternative option. I suspect it will end up being a better option in terms of design. But depending on time and risk even knowing that we want the better design eventually doesn't preclude getting the easier fix in now. > No, what should matter is if the role doing the GRANT has admin rights > on pg_read_all_stats, or on the postgres role. That also happens to be > what the spec says. > Yes, and superusers implicitly have that right, while CREATEROLE users implicitly have that right on the pg_* role but not on superuser roles. I just want to plug that hole and include the pg_* roles (or any role for that matter) in being able to be denied implied ADMIN rights for non-superusers. > Today a non-superuser cannot "grant postgres to someuser;" > > No, but a role can be created like 'admin', which a superuser GRANT's > 'postgres' to and then that role can be GRANT'd to anyone by anyone who > has CREATEROLE rights. That's not sane. > I agree. And I've suggested a minimal fix, adding an attribute to the role that prohibits non-superusers from granting it to others, that removes the insane behavior. I'm on board for a hard-coded fix as well - if a superuser is in the membership chain of a role then non-superusers cannot grant membership in that role to others. Neither of those really solves the pg_* roles problem. We still need to indicate that they are somehow special. Whether it is a nice matrix or roles and permissions or a simple attribute that makes them behave like they are superuser roles. > > I disagree entirely with the idea that we must have some roles who can > only ever be administered by a superuser. I don't think this is a must have. I think that since we do have it today that fixes that leverage the status quo in order to be done more easily are perfectly valid solutions. > If anything, we should be > moving away (as we have, in fact, been doing), from anything being the > exclusive purview of the superuser. > I totally agree. > > > In particular, this means CREATEROLE roles cannot assign membership in > the > > marked roles; just like they cannot assign membership in superuser roles > > today. > > I disagree with the idea that we need to mark some roles as only being > able to be modified by the superuser- why invent this? Because CREATEUSER is a thing and people want to prevent roles with that attribute from assigning membership to the predefined superuser-aspect roles. If I've misunderstood that desire and the scope of delegation given by the superuser to CREATEUSER roles is acceptable, then no change here is needed. What you > seem to be arguing for here is to rip out the ADMIN functionality, which > is defined by spec and not even exclusively by PG, and replace it with a > single per-role flag that says if that role can only be modified by > superusers. I made the observation that being able to manage the membership of a group without having the ability to create new users seems like a half a loaf of a feature. That's it. I would presume that any redesign of the permissions system here would address this adequately. The > short answer is that it's not- which is why we have documented > CREATEROLE as being 'superuser light'. The goal here is to get rid of > that. > Now you tell me. Robert should have led with that goal upfront. > > Yes, we're talking about a new feature- one intended to replace the > broken way that CREATEROLE works, which your proposal doesn't. > > That is correct, I was trying to figure out minimally
Re: role self-revocation
On Thu, Mar 10, 2022 at 2:05 PM Peter Eisentraut wrote: > On 09.03.22 14:02, Robert Haas wrote: > > On Wed, Mar 9, 2022 at 7:55 AM Peter Eisentraut > > wrote: > >> Do we have subtractive permissions today? > > > > Not in the GRANT/REVOKE sense, I think, but you can put a user in a > > group and then mention that group in pg_hba.conf. And that line might > > be "reject" or whatever. > > Well, you can always build an external system that looks at roles and > does nonsensical things with it. But the privilege system itself seems > to be additive only. Personally, I agree with the argument that there > should not be any subtractive permissions. The mental model where > permissions are sort of keys to doors or boxes just doesn't work for that. I mean, I didn't design pg_hba.conf, but I think it's part of the database doing a reasonable thing, not an external system doing a nonsensical thing. I am not sure that I (or anyone) would endorse a system where you can say something like GRANT NOT SELECT ON TABLE foo TO bar, essentially putting a negative ACL into the system dictating that, regardless of any other grants that may exist, foo should not be able to SELECT from that table. But I think it's reasonable to use groups as a way of referencing a defined collection of users for some purpose. The pg_hba.conf thing is an example of that. You put all the users that you want to be treated in a certain way for authentication purposes into a group, and then you mention the group in the file, and it just works. I don't find that an unreasonable design at all. We could've created some other kind of grouping mechanism for such purposes that is separate from the role system, but we didn't choose to do that. I don't know if that was the absolute best possible decision or not, but it doesn't seem like an especially bad choice. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
On 09.03.22 14:02, Robert Haas wrote: On Wed, Mar 9, 2022 at 7:55 AM Peter Eisentraut wrote: Do we have subtractive permissions today? Not in the GRANT/REVOKE sense, I think, but you can put a user in a group and then mention that group in pg_hba.conf. And that line might be "reject" or whatever. Well, you can always build an external system that looks at roles and does nonsensical things with it. But the privilege system itself seems to be additive only. Personally, I agree with the argument that there should not be any subtractive permissions. The mental model where permissions are sort of keys to doors or boxes just doesn't work for that.
Re: role self-revocation
On Thu, Mar 10, 2022 at 12:26 PM Joshua Brindle wrote: > Ownership implies DAC, the ability to grant others rights to an > object. It's not "kooky" to see roles as owned objects, but it isn't > required either. For example most objects on a UNIX system are owned > and subject to DAC but users aren't. I have no issue with anything you write in this paragraph. > Stephen's, and now my, issue with ownership is that, since it implies > DAC, most checks will be bypassed for the owner. We would both prefer > for everyone to be subject to the grants, including whoever created > the role. That sounds like MAC, which is usually something that sits on top of DAC and is enforced in addition to DAC, not a reason for DAC to not exist. > Rather, we'd like to see a "creators of roles get this set of grants > against the role by default" and "as a superuser I can revoke grants > from creators against roles they created" If you create a table, you own it. You get a set of default permissions on the table which can be revoked either by you or by someone else, and you also have certain intrinsic rights over the object as owner which cannot be revoked - including the ability to re-grant yourself any previously-revoked permissions. I am not against the idea of trying to clean things up so that everything you can do with a table is a revocable privilege and you can be the owner without having any rights at all, including the right to give yourself other rights back, but I cannot believe that the idea of removing table ownership as a concept would ever gain consensus on this list. Therefore, I also do not think it is reasonable to say that we shouldn't introduce a similar concept for object types that don't have it yet, such as roles. But that's not to say that we couldn't decide to do something else instead, and that other thing might well be better. Do you want to sketch out a full proposal, even just what the syntax would look like, and share that here? And if you could explain how I could use it to create the mini-superusers that I'm trying to get out of this thing, even better. Thanks, -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
Greetings, * David G. Johnston (david.g.johns...@gmail.com) wrote: > On Thu, Mar 10, 2022 at 9:19 AM Stephen Frost wrote: > > * David G. Johnston (david.g.johns...@gmail.com) wrote: > > > On Thu, Mar 10, 2022 at 7:46 AM Robert Haas > > wrote: > > > > On Wed, Mar 9, 2022 at 4:31 PM Tom Lane wrote: > > > > > I don't think we need syntax to describe it. As I just said in my > > > > > other reply, we have a perfectly good precedent for this already > > > > > in ordinary object permissions. That is: an object owner always, > > > > > implicitly, has GRANT OPTION for all the object's privileges, even > > > > > if she revoked the corresponding plain privilege from herself. > > > > > > > > > > Yeah, this does mean that we're effectively deciding that the creator > > > > > of a role is its owner. What's the problem with that? > > > > > > > > I don't think that's entirely the wrong concept, but it doesn't make a > > > > lot of sense in a world where the creator has to be a superuser. If > > > > alice, bob, and charlie are superusers who take turns creating new > > > > users, and then we let charlie go due to budget cuts, forcing alice > > > > and bob to change the owner of all the users he created to some other > > > > superuser as a condition of dropping his account is a waste of > > > > everyone's time. They can do exactly the same things to every account > > > > on the system after we change the role owner as before. > > > > > > Then maybe we should just implement the idea that if a superuser would > > > become the owner we instead substitute in the bootstrap user. Or give > > the > > > DBA the choice whether they want to retain knowledge of specific roles - > > > and thus are willing to accept the "waste of time". > > > > This doesn't strike me as going in the right direction. Falling back to > > the bootstrap superuser is generally a hack and not a great one. I'll > > also point out that the SQL spec hasn't got a concept of role ownership > > either. > > > > > > But wait, I hear you cry, what about CREATEROLE? Well, CREATEROLE is > > > > generally agreed to be broken right now, and if you don't agree with > > > > that, consider that it can grant pg_execute_server_programs to a > > > > newly-created account and then explain to me how it's functionally > > > > different from superuser. > > > > > > CREATEROLE has long been defined as basically having "with admin option" > > on > > > every role in the system. The failure to special-case the roles that > > grant > > > different aspects of superuser-ness to its members doesn't make > > CREATEROLE > > > itself broken, it makes the implementation of pg_execute_server_programs > > > broken. Only superusers should be considered to have with admin option > > on > > > these roles. They can delegate through the usual membership+admin > > mechanism > > > to a CREATEROLE role if they desire. > > > > No, CREATEROLE having admin option on every role in the system is broken > > and always has been. It's not just an issue for predefined roles like > > pg_execute_server_program, > > > > > it's an issue for any role that could become > > a superuser either directly or indirectly and that extends beyond the > > predefined ones. > > > The only indirect way for a role to become superuser is to have been > granted membership in a superuser group, then SET ROLE. Non-superusers > cannot do this. If a superuser does this I consider the outcome to be no > different than if they go and do: A non-superuser absolutely can be GRANT'd membership in a superuser role and then SET ROLE to that user thus becoming a superuser. Giving users a regular role to log in as and then membership in a role that can become a superuser is akin to having a sudoers group in Unix and is good practice, not something that everyone should have to be super-dooper careful to not do, lest a CREATEROLE user be able to leverage that. > SET allow_system_table_mods TO true; > DROP pg_catalog.pg_class; I don't equate these in the least. > In short, having a CREATEROLE user issuing: > GRANT pg_read_all_stats TO davidj; > should result in the same outcome as them issuing: > GRANT postgres TO davidj; > -- ERROR: must be superuser to alter superusers No, what should matter is if the role doing the GRANT has admin rights on pg_read_all_stats, or on the postgres role. That also happens to be what the spec says. > Superusers can break their system and we don't go to great effort to stop > them. I see no difference here, so arguments of this nature aren't all > that compelling to me. That you don't feel they're compelling don't make them somehow not real, nor even particularly uncommon, nor do I view ignoring that possibility as somehow creating a strong authentication system. > CREATEROLE shouldn't be > > the determining factor in the question of if a role can GRANT a > > predefined (or any other role) to some other role- that should be > > governed by the admin option on that role, and that should
Re: role self-revocation
On Thu, Mar 10, 2022 at 12:11 PM Robert Haas wrote: > > On Thu, Mar 10, 2022 at 11:19 AM Stephen Frost wrote: > > I disagree that ownership is needed that's not what the spec calls for > > either. What we need is more flexibility when it comes to the > > relationships which are allowed to be created between roles and what > > privileges come with them. To that end, I'd argue that we should be > > extending pg_auth_members, first by separating out membership itself > > into an explicitly tracked attribute (instead of being implicit in the > > existance of a row in the table) and then adding on what other > > privileges we see fit to add, such as the ability to DROP a role. We > > do need to remove the ability for a role who hasn't been explicitly > > given the admin right on another role to modify that role's membership > > too, as was originally proposed here. This also seems to more closely > > follow the spec's expectation, something that role ownership doesn't. > > I do not have a problem with more fine-grained kinds of authorization > even though I think there are syntactic issues to work out, but I > strongly disagree with the idea that we can't or shouldn't also have > role ownership. Marc invented it. Now Tom has invented it > independently. All sorts of other objects have it already. Trying to > make it out like this is some kind of kooky idea is not believable. > Yeah, it's not the most sophisticated or elegant model and that's why > it's good for us to also have other things, but for simple cases it is > easy to understand and works great. Ownership implies DAC, the ability to grant others rights to an object. It's not "kooky" to see roles as owned objects, but it isn't required either. For example most objects on a UNIX system are owned and subject to DAC but users aren't. Stephen's, and now my, issue with ownership is that, since it implies DAC, most checks will be bypassed for the owner. We would both prefer for everyone to be subject to the grants, including whoever created the role. Rather, we'd like to see a "creators of roles get this set of grants against the role by default" and "as a superuser I can revoke grants from creators against roles they created"
Re: role self-revocation
On Thu, Mar 10, 2022 at 9:19 AM Stephen Frost wrote: > Greetings, > > * David G. Johnston (david.g.johns...@gmail.com) wrote: > > On Thu, Mar 10, 2022 at 7:46 AM Robert Haas > wrote: > > > On Wed, Mar 9, 2022 at 4:31 PM Tom Lane wrote: > > > > I don't think we need syntax to describe it. As I just said in my > > > > other reply, we have a perfectly good precedent for this already > > > > in ordinary object permissions. That is: an object owner always, > > > > implicitly, has GRANT OPTION for all the object's privileges, even > > > > if she revoked the corresponding plain privilege from herself. > > > > > > > > Yeah, this does mean that we're effectively deciding that the creator > > > > of a role is its owner. What's the problem with that? > > > > > > I don't think that's entirely the wrong concept, but it doesn't make a > > > lot of sense in a world where the creator has to be a superuser. If > > > alice, bob, and charlie are superusers who take turns creating new > > > users, and then we let charlie go due to budget cuts, forcing alice > > > and bob to change the owner of all the users he created to some other > > > superuser as a condition of dropping his account is a waste of > > > everyone's time. They can do exactly the same things to every account > > > on the system after we change the role owner as before. > > > > Then maybe we should just implement the idea that if a superuser would > > become the owner we instead substitute in the bootstrap user. Or give > the > > DBA the choice whether they want to retain knowledge of specific roles - > > and thus are willing to accept the "waste of time". > > This doesn't strike me as going in the right direction. Falling back to > the bootstrap superuser is generally a hack and not a great one. I'll > also point out that the SQL spec hasn't got a concept of role ownership > either. > > > > But wait, I hear you cry, what about CREATEROLE? Well, CREATEROLE is > > > generally agreed to be broken right now, and if you don't agree with > > > that, consider that it can grant pg_execute_server_programs to a > > > newly-created account and then explain to me how it's functionally > > > different from superuser. > > > > CREATEROLE has long been defined as basically having "with admin option" > on > > every role in the system. The failure to special-case the roles that > grant > > different aspects of superuser-ness to its members doesn't make > CREATEROLE > > itself broken, it makes the implementation of pg_execute_server_programs > > broken. Only superusers should be considered to have with admin option > on > > these roles. They can delegate through the usual membership+admin > mechanism > > to a CREATEROLE role if they desire. > > No, CREATEROLE having admin option on every role in the system is broken > and always has been. It's not just an issue for predefined roles like > pg_execute_server_program, > it's an issue for any role that could become > a superuser either directly or indirectly and that extends beyond the > predefined ones. The only indirect way for a role to become superuser is to have been granted membership in a superuser group, then SET ROLE. Non-superusers cannot do this. If a superuser does this I consider the outcome to be no different than if they go and do: SET allow_system_table_mods TO true; DROP pg_catalog.pg_class; In short, having a CREATEROLE user issuing: GRANT pg_read_all_stats TO davidj; should result in the same outcome as them issuing: GRANT postgres TO davidj; -- ERROR: must be superuser to alter superusers Superusers can break their system and we don't go to great effort to stop them. I see no difference here, so arguments of this nature aren't all that compelling to me. CREATEROLE shouldn't be > the determining factor in the question of if a role can GRANT a > predefined (or any other role) to some other role- that should be > governed by the admin option on that role, and that should work exactly > the same for predefined roles as it does for any other. > Never granting the CREATEROLE attribute to anyone will give you this outcome today. > ADMIN option > without membership isn't something the catalogs today can understand > Today, they don't need to in order for the system to function within its existing design specs. > > ALTER ROLE pg_superuser WITH [NO] ADMIN; > > > > Then adding a role membership including the WITH ADMIN OPTION can be > > rejected, as can the non-superuser situation. Setting WITH NO ADMIN > should > > fail if any existing members have admin. You must be a superuser to > > execute WITH ADMIN (maybe WITH NO ADMIN as well...). And possibly even a > > new pg_* role that grants this ability (and maybe some others) for use > by a > > backup/restore user. > > I'm not following this in general or how it helps. Surely we don't want > to limit WITH ADMIN to superusers. Today a non-superuser cannot "grant postgres to someuser;" The point of this attribute is to allow the
Re: role self-revocation
On Thu, Mar 10, 2022 at 11:19 AM Stephen Frost wrote: > I disagree that ownership is needed that's not what the spec calls for > either. What we need is more flexibility when it comes to the > relationships which are allowed to be created between roles and what > privileges come with them. To that end, I'd argue that we should be > extending pg_auth_members, first by separating out membership itself > into an explicitly tracked attribute (instead of being implicit in the > existance of a row in the table) and then adding on what other > privileges we see fit to add, such as the ability to DROP a role. We > do need to remove the ability for a role who hasn't been explicitly > given the admin right on another role to modify that role's membership > too, as was originally proposed here. This also seems to more closely > follow the spec's expectation, something that role ownership doesn't. I do not have a problem with more fine-grained kinds of authorization even though I think there are syntactic issues to work out, but I strongly disagree with the idea that we can't or shouldn't also have role ownership. Marc invented it. Now Tom has invented it independently. All sorts of other objects have it already. Trying to make it out like this is some kind of kooky idea is not believable. Yeah, it's not the most sophisticated or elegant model and that's why it's good for us to also have other things, but for simple cases it is easy to understand and works great. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
> On Mar 10, 2022, at 7:56 AM, David G. Johnston > wrote: > > > I want that because I want mini-superusers, where alice can administer > the users that alice creates just as if she were a superuser, > including having their permissions implicitly and dropping them when > she wants them gone, but where alice cannot break out to the operating > system as a true superuser could do. > > CREATEROLE (once the pg_* with admin rules are fixed) + Ownership and rules > restricting interfering with another role's objects (unless superuser) seems > to handle this. What if one of alice's subordinates also owns roles? Can alice interfere with *that* role's objects? I don't see that a simple rule restricting roles from interfering with another role's objects is quite enough. That raises the question of whether role ownership is transitive, and whether we need a concept similar to inherit/noinherit for ownership. There is also the problem that CREATEROLE currently allows a set of privileges to be granted to created roles, and that set of privileges is hard-coded. You've suggested changing the hard-coded rules to remove pg_* roles from the list of grantable privileges, but that's still an inflexible set of hardcoded privileges. Wouldn't it make more sense for the grantor to need GRANT OPTION on any privilege they give to roles they create? > the bot can stand up > accounts, can grant them membership in a defined set of groups, but > cannot exercise the privileges of those accounts (or hack superuser > either). > > The bot should be provided a security definer procedure that encapsulates all > of this rather than us trying to hack the permission system. This isn't a > user permission concern, it is an unauthorized privilege escalation concern. > Anyone with the bot's credentials can trivially overcome the third > restriction by creating a role with the desired membership and then logging > in as that role - and there is nothing the system can do to prevent that > while also allowing the other two permissions. Doesn't this assume password authentication? If the server uses ldap authentication, for example, wouldn't the bot need valid ldap credentials for at least one user for this attack to work? And if CREATEROLE has been made more configurable, wouldn't the bot only be able to grant that ldap user the limited set of privileges that the bot's database user has been granted ADMIN OPTION for? — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: role self-revocation
Greetings, * David G. Johnston (david.g.johns...@gmail.com) wrote: > On Thu, Mar 10, 2022 at 7:46 AM Robert Haas wrote: > > On Wed, Mar 9, 2022 at 4:31 PM Tom Lane wrote: > > > I don't think we need syntax to describe it. As I just said in my > > > other reply, we have a perfectly good precedent for this already > > > in ordinary object permissions. That is: an object owner always, > > > implicitly, has GRANT OPTION for all the object's privileges, even > > > if she revoked the corresponding plain privilege from herself. > > > > > > Yeah, this does mean that we're effectively deciding that the creator > > > of a role is its owner. What's the problem with that? > > > > I don't think that's entirely the wrong concept, but it doesn't make a > > lot of sense in a world where the creator has to be a superuser. If > > alice, bob, and charlie are superusers who take turns creating new > > users, and then we let charlie go due to budget cuts, forcing alice > > and bob to change the owner of all the users he created to some other > > superuser as a condition of dropping his account is a waste of > > everyone's time. They can do exactly the same things to every account > > on the system after we change the role owner as before. > > Then maybe we should just implement the idea that if a superuser would > become the owner we instead substitute in the bootstrap user. Or give the > DBA the choice whether they want to retain knowledge of specific roles - > and thus are willing to accept the "waste of time". This doesn't strike me as going in the right direction. Falling back to the bootstrap superuser is generally a hack and not a great one. I'll also point out that the SQL spec hasn't got a concept of role ownership either. > > But wait, I hear you cry, what about CREATEROLE? Well, CREATEROLE is > > generally agreed to be broken right now, and if you don't agree with > > that, consider that it can grant pg_execute_server_programs to a > > newly-created account and then explain to me how it's functionally > > different from superuser. > > CREATEROLE has long been defined as basically having "with admin option" on > every role in the system. The failure to special-case the roles that grant > different aspects of superuser-ness to its members doesn't make CREATEROLE > itself broken, it makes the implementation of pg_execute_server_programs > broken. Only superusers should be considered to have with admin option on > these roles. They can delegate through the usual membership+admin mechanism > to a CREATEROLE role if they desire. No, CREATEROLE having admin option on every role in the system is broken and always has been. It's not just an issue for predefined roles like pg_execute_server_program, it's an issue for any role that could become a superuser either directly or indirectly and that extends beyond the predefined ones. As this issue with CREATEROLE existed way before predefined roles were added to PG, claiming that it's an issue with predefined roles doesn't make a bit of sense. > > The whole area needs a rethink. I believe > > everyone involved in the discussion on the other threads agrees that > > some reform of CREATEROLE is necessary, and more generally with the > > idea that it's useful for non-superusers to be able to create roles. > > As the documentation says, using SUPERUSER for day-to-day administration is > contrary to good security practices. Role management is considered to be a > day-to-day administration activity. I agree with this principle. It was > designed to neither be a superuser nor grant superuser, so removing the > ability to grant the pg_* role memberships remains consistent with its > original intent. That would not be sufficient to make CREATEROLE safe. Far, far from it. > > I want that because I want mini-superusers, where alice can administer > > the users that alice creates just as if she were a superuser, > > including having their permissions implicitly and dropping them when > > she wants them gone, but where alice cannot break out to the operating > > system as a true superuser could do. > > CREATEROLE (once the pg_* with admin rules are fixed) + Ownership and rules > restricting interfering with another role's objects (unless superuser) > seems to handle this. This is not sufficient- roles can be not-superuser themselves but have the ability to become superuser if GRANT'd a superuser role and therefore we can't have a system where CREATEROLE allows arbitrary GRANT'ing of roles to each other. I'm a bit confused too as anything where we are curtailing what CREATEROLE roles are able to do in a manner that means they're only able to modify some subset of roles should equally apply to predefined roles too- that is, CREATEROLE shouldn't be the determining factor in the question of if a role can GRANT a predefined (or any other role) to some other role- that should be governed by the admin option on that role, and that should work exactly the same for
Re: role self-revocation
On Thu, Mar 10, 2022 at 7:46 AM Robert Haas wrote: > On Wed, Mar 9, 2022 at 4:31 PM Tom Lane wrote: > > I don't think we need syntax to describe it. As I just said in my > > other reply, we have a perfectly good precedent for this already > > in ordinary object permissions. That is: an object owner always, > > implicitly, has GRANT OPTION for all the object's privileges, even > > if she revoked the corresponding plain privilege from herself. > > > > Yeah, this does mean that we're effectively deciding that the creator > > of a role is its owner. What's the problem with that? > > I don't think that's entirely the wrong concept, but it doesn't make a > lot of sense in a world where the creator has to be a superuser. If > alice, bob, and charlie are superusers who take turns creating new > users, and then we let charlie go due to budget cuts, forcing alice > and bob to change the owner of all the users he created to some other > superuser as a condition of dropping his account is a waste of > everyone's time. They can do exactly the same things to every account > on the system after we change the role owner as before. > Then maybe we should just implement the idea that if a superuser would become the owner we instead substitute in the bootstrap user. Or give the DBA the choice whether they want to retain knowledge of specific roles - and thus are willing to accept the "waste of time". > But wait, I hear you cry, what about CREATEROLE? Well, CREATEROLE is > generally agreed to be broken right now, and if you don't agree with > that, consider that it can grant pg_execute_server_programs to a > newly-created account and then explain to me how it's functionally > different from superuser. CREATEROLE has long been defined as basically having "with admin option" on every role in the system. The failure to special-case the roles that grant different aspects of superuser-ness to its members doesn't make CREATEROLE itself broken, it makes the implementation of pg_execute_server_programs broken. Only superusers should be considered to have with admin option on these roles. They can delegate through the usual membership+admin mechanism to a CREATEROLE role if they desire. > The whole area needs a rethink. I believe > everyone involved in the discussion on the other threads agrees that > some reform of CREATEROLE is necessary, and more generally with the > idea that it's useful for non-superusers to be able to create roles. > As the documentation says, using SUPERUSER for day-to-day administration is contrary to good security practices. Role management is considered to be a day-to-day administration activity. I agree with this principle. It was designed to neither be a superuser nor grant superuser, so removing the ability to grant the pg_* role memberships remains consistent with its original intent. > I want that because I want mini-superusers, where alice can administer > the users that alice creates just as if she were a superuser, > including having their permissions implicitly and dropping them when > she wants them gone, but where alice cannot break out to the operating > system as a true superuser could do. CREATEROLE (once the pg_* with admin rules are fixed) + Ownership and rules restricting interfering with another role's objects (unless superuser) seems to handle this. > the bot can stand up > accounts, can grant them membership in a defined set of groups, but > cannot exercise the privileges of those accounts (or hack superuser > either). The bot should be provided a security definer procedure that encapsulates all of this rather than us trying to hack the permission system. This isn't a user permission concern, it is an unauthorized privilege escalation concern. Anyone with the bot's credentials can trivially overcome the third restriction by creating a role with the desired membership and then logging in as that role - and there is nothing the system can do to prevent that while also allowing the other two permissions. > And that's why I'm not sure it's really the right idea to say that we > don't need syntax for this admin-without-member concept. We already have this syntax in the form of CREATEROLE. But we do need a fix, just on the group side. We need a way to define a group as having no ADMINS. ALTER ROLE pg_superuser WITH [NO] ADMIN; Then adding a role membership including the WITH ADMIN OPTION can be rejected, as can the non-superuser situation. Setting WITH NO ADMIN should fail if any existing members have admin. You must be a superuser to execute WITH ADMIN (maybe WITH NO ADMIN as well...). And possibly even a new pg_* role that grants this ability (and maybe some others) for use by a backup/restore user. Or just special-case pg_* roles. The advantage of exposing this to the DBA is that they can then package pg_* roles into a custom group and still have the benefit of superuser only administration. In the special-case implementation the presence of a
Re: role self-revocation
On Wed, Mar 9, 2022 at 4:31 PM Tom Lane wrote: > I don't think we need syntax to describe it. As I just said in my > other reply, we have a perfectly good precedent for this already > in ordinary object permissions. That is: an object owner always, > implicitly, has GRANT OPTION for all the object's privileges, even > if she revoked the corresponding plain privilege from herself. > > Yeah, this does mean that we're effectively deciding that the creator > of a role is its owner. What's the problem with that? I don't think that's entirely the wrong concept, but it doesn't make a lot of sense in a world where the creator has to be a superuser. If alice, bob, and charlie are superusers who take turns creating new users, and then we let charlie go due to budget cuts, forcing alice and bob to change the owner of all the users he created to some other superuser as a condition of dropping his account is a waste of everyone's time. They can do exactly the same things to every account on the system after we change the role owner as before. But wait, I hear you cry, what about CREATEROLE? Well, CREATEROLE is generally agreed to be broken right now, and if you don't agree with that, consider that it can grant pg_execute_server_programs to a newly-created account and then explain to me how it's functionally different from superuser. The whole area needs a rethink. I believe everyone involved in the discussion on the other threads agrees that some reform of CREATEROLE is necessary, and more generally with the idea that it's useful for non-superusers to be able to create roles. But the reasons why people want that vary. I want that because I want mini-superusers, where alice can administer the users that alice creates just as if she were a superuser, including having their permissions implicitly and dropping them when she wants them gone, but where alice cannot break out to the operating system as a true superuser could do. I want this because the lack of meaningful privilege separation that led to CVE-2019-9193 being filed spuriously is a very real problem. It's a thing a lot of people want, and I want to give it to them. David Steele, on the other hand, wants to build a user-creating bot that can create accounts but otherwise conforms to the principle of least privilege: the bot can stand up accounts, can grant them membership in a defined set of groups, but cannot exercise the privileges of those accounts (or hack superuser either). Other people may well want other things. And that's why I'm not sure it's really the right idea to say that we don't need syntax for this admin-without-member concept. If we just want to bolt role ownership onto the existing framework without really changing anything else, we can do that without extra syntax and, as you say here, make it an implicit property of role ownership. But I don't see that as has having much value; we just end up with a bunch of superuser owners. Whatever. Now Stephen made the argument that we ought to actually have admin-without-member as a first class concept, something that could be assigned to arbitrary users. Actually, I think he wanted it even more fine grained with that. And I think that could make the concept a lot more useful, but then it needs some kind of understandable syntax. There's a lot of moving parts here. It's not just about coming up with something that sounds generally logical, but about creating a system that has some real-world utility. > > But do we really have to solve this problem before we can clean up > > this session exception? > > I think we need a plan for where we're going. I don't see "clean up > the session exception" as an end in itself; it's part of re-examining > how all of this ought to work. I don't say that we have to have a > complete patch right away, only that we need a coherent end goal. I'd like to have a plan, too, but if this behavior is accidental, I still think we can remove it without making big decisions about future direction. The perfect is the enemy of the good. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
"David G. Johnston" writes:
> So CREATE ROLE will assign ownership of AND membership in the newly created
> role to the session_user
I would NOT have it automatically assign membership in the new role,
even though the SQL spec says so. We've not done that historically
and it doesn't seem desirable. In particular, it's *really* not
desirable for a user (role with LOGIN).
> I'm fine with this. It does introduce an OWNER concept to roles and so at
> minimum we need to add:
> ALTER ROLE foo OWNER TO { new_owner | CURRENT_ROLE | CURRENT_USER |
> SESSION_USER }
Agreed.
regards, tom lane
Re: role self-revocation
On Wed, Mar 9, 2022 at 2:31 PM Tom Lane wrote:
> Robert Haas writes:
> > Well, the problem is that as far as I can see, the admin option is an
> > optional feature of membership. You can grant someone membership
> > without admin option, or with admin option, but you can't grant them
> > the admin option without membership, just like you can't purchase an
> > upgrade to first class without the underlying plane ticket. What would
> > the syntax look even like for this? GRANT foo TO bar WITH ADMIN OPTION
> > BUT WITHOUT MEMBERSHIP? Yikes.
>
> I don't think we need syntax to describe it. As I just said in my
> other reply, we have a perfectly good precedent for this already
> in ordinary object permissions. That is: an object owner always,
> implicitly, has GRANT OPTION for all the object's privileges, even
> if she revoked the corresponding plain privilege from herself.
>
So CREATE ROLE will assign ownership of AND membership in the newly created
role to the session_user UNLESS the OWNER clause is present in which case
the named role, so long as the session_user can SET ROLE to the named role,
becomes the owner & member. Subsequent to that the owner can issue: REVOKE
new_role FROM role_name where role_name is again the session_user role or
one that can be SET ROLE to.
> Yeah, this does mean that we're effectively deciding that the creator
> of a role is its owner. What's the problem with that?
>
I'm fine with this. It does introduce an OWNER concept to roles and so at
minimum we need to add:
ALTER ROLE foo OWNER TO { new_owner | CURRENT_ROLE | CURRENT_USER |
SESSION_USER }
And similar for CREATE ROLE
And keep the USER alias commands in sync.
GROUP commands are only present for backward compatibility and so don't get
updated with new features by design.
Obviously a superuser can change ownership.
Playing with table ownership I find this behavior:
-- superuser
CREATE ROLE tblowner;
CREATE TABLE tblowner_test (id serial primary key);
ALTER TABLE tblowner_test OWNER TO tblowner;
CREATE ROLE boss;
GRANT boss TO tblowner;
SET SESSION AUTHORIZATION tblowner;
ALTER TABLE tblowner_test OWNER TO boss; --works
So tblowner can push their ownership attribute to any group they are a
member of. Is that the behavior we want for roles as well?
David J.
Re: role self-revocation
Robert Haas writes: > Well, the problem is that as far as I can see, the admin option is an > optional feature of membership. You can grant someone membership > without admin option, or with admin option, but you can't grant them > the admin option without membership, just like you can't purchase an > upgrade to first class without the underlying plane ticket. What would > the syntax look even like for this? GRANT foo TO bar WITH ADMIN OPTION > BUT WITHOUT MEMBERSHIP? Yikes. I don't think we need syntax to describe it. As I just said in my other reply, we have a perfectly good precedent for this already in ordinary object permissions. That is: an object owner always, implicitly, has GRANT OPTION for all the object's privileges, even if she revoked the corresponding plain privilege from herself. Yeah, this does mean that we're effectively deciding that the creator of a role is its owner. What's the problem with that? > But do we really have to solve this problem before we can clean up > this session exception? I think we need a plan for where we're going. I don't see "clean up the session exception" as an end in itself; it's part of re-examining how all of this ought to work. I don't say that we have to have a complete patch right away, only that we need a coherent end goal. regards, tom lane
Re: role self-revocation
I wrote: > This seems like a reasonable answer to me too: the creating role has admin > option implicitly, and can then choose to grant that to other roles. > Obviously some work needs to be done to make that happen (and we should > see whether the SQL spec has some different idea). Ah, here we go: it's buried under CREATE ROLE. SQL:2021 12.4 saith that when role A executes CREATE ROLE , then 1) A grantable role authorization descriptor is created whose role name is , whose grantor is "_SYSTEM", and whose grantee is A. Since nobody is _SYSTEM, this grant can't be deleted except by dropping the new role (or, maybe, dropping A?). So that has nearly the same end result as "the creating role has admin option implicitly". The main difference I can see is that it also means the creating role is a *member* implicitly, which is something I'd argue we don't want to enforce. This is analogous to the way we let an object owner revoke her own ordinary permissions, which the SQL model doesn't allow since those permissions were granted to her by _SYSTEM. regards, tom lane
Re: role self-revocation
Greetings, * Robert Haas (robertmh...@gmail.com) wrote: > On Wed, Mar 9, 2022 at 4:01 PM Tom Lane wrote: > > > In my opinion, the right to > > > administer a role - regardless of whether or not it is a login role - > > > most naturally vests in the role that created it, or something in that > > > direction at least, if not that exact thing. > > > > This seems like a reasonable answer to me too: the creating role has admin > > option implicitly, and can then choose to grant that to other roles. > > Obviously some work needs to be done to make that happen (and we should > > see whether the SQL spec has some different idea). > > Well, the problem is that as far as I can see, the admin option is an > optional feature of membership. You can grant someone membership > without admin option, or with admin option, but you can't grant them > the admin option without membership, just like you can't purchase an > upgrade to first class without the underlying plane ticket. What would > the syntax look even like for this? GRANT foo TO bar WITH ADMIN OPTION > BUT WITHOUT MEMBERSHIP? Yikes. I've been meaning to reply to your other email regarding this, but I don't really agree that the syntax ends up being so terrible or difficult to deal with, considering we have these same general things for ALTER ROLE already and there hasn't been all that much complaining. That is, we have LOGIN and NOLOGIN, CREATEROLE and NOCREATEROLE, and we could have MEMBERSHIP and NOMEMBERSHIP pretty easily here if we wanted to. > But do we really have to solve this problem before we can clean up > this session exception? I hope not, because I think that's a much > bigger can of worms than this is. I do believe we can deal with the above independently and at a later time and go ahead and clean up the session excepton bit without dealing with the above at the same time. Thanks, Stephen signature.asc Description: PGP signature
Re: role self-revocation
Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Robert Haas writes: > > On Mar 7, 2022, at 12:16 PM, Tom Lane wrote: > > tgl> Having said that, one thing that I find fishy is that it's not clear > > tgl> where the admin privilege for a role originates. After "CREATE ROLE > > tgl> alice", alice has no members, therefore none that have admin privilege, > > tgl> therefore the only way that the first member could be added is via > > tgl> superuser deus ex machina. This does not seem clean. > > > I agree with that, but I don't think it's a sufficient reason for > > keeping the self-admin exception, because the same problem exists for > > non-login roles. I don't even think it's the right idea conceptually > > to suppose that the power to administer a role originates from the > > role itself. > > Actually, that's the same thing I was trying to say. But if it doesn't > originate from the role itself, where does it originate from? > > > In my opinion, the right to > > administer a role - regardless of whether or not it is a login role - > > most naturally vests in the role that created it, or something in that > > direction at least, if not that exact thing. > > This seems like a reasonable answer to me too: the creating role has admin > option implicitly, and can then choose to grant that to other roles. I agree that this has some appeal, but it's not desirable in all cases and so I wouldn't want it to be fully baked into the system ala the role 'owner' concept. > Obviously some work needs to be done to make that happen (and we should > see whether the SQL spec has some different idea). Agreed on this, though I don't recall it having much to say on it. Thanks, Stephen signature.asc Description: PGP signature
Re: role self-revocation
On Wed, Mar 9, 2022 at 4:01 PM Tom Lane wrote: > > In my opinion, the right to > > administer a role - regardless of whether or not it is a login role - > > most naturally vests in the role that created it, or something in that > > direction at least, if not that exact thing. > > This seems like a reasonable answer to me too: the creating role has admin > option implicitly, and can then choose to grant that to other roles. > Obviously some work needs to be done to make that happen (and we should > see whether the SQL spec has some different idea). Well, the problem is that as far as I can see, the admin option is an optional feature of membership. You can grant someone membership without admin option, or with admin option, but you can't grant them the admin option without membership, just like you can't purchase an upgrade to first class without the underlying plane ticket. What would the syntax look even like for this? GRANT foo TO bar WITH ADMIN OPTION BUT WITHOUT MEMBERSHIP? Yikes. But do we really have to solve this problem before we can clean up this session exception? I hope not, because I think that's a much bigger can of worms than this is. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
Robert Haas writes: > On Mar 7, 2022, at 12:16 PM, Tom Lane wrote: > tgl> Having said that, one thing that I find fishy is that it's not clear > tgl> where the admin privilege for a role originates. After "CREATE ROLE > tgl> alice", alice has no members, therefore none that have admin privilege, > tgl> therefore the only way that the first member could be added is via > tgl> superuser deus ex machina. This does not seem clean. > I agree with that, but I don't think it's a sufficient reason for > keeping the self-admin exception, because the same problem exists for > non-login roles. I don't even think it's the right idea conceptually > to suppose that the power to administer a role originates from the > role itself. Actually, that's the same thing I was trying to say. But if it doesn't originate from the role itself, where does it originate from? > In my opinion, the right to > administer a role - regardless of whether or not it is a login role - > most naturally vests in the role that created it, or something in that > direction at least, if not that exact thing. This seems like a reasonable answer to me too: the creating role has admin option implicitly, and can then choose to grant that to other roles. Obviously some work needs to be done to make that happen (and we should see whether the SQL spec has some different idea). regards, tom lane
Re: role self-revocation
On Mon, Mar 7, 2022 at 11:14 PM Mark Dilger wrote: > > On Mar 7, 2022, at 12:16 PM, Tom Lane wrote: > > What would be > > lost if we drop it? > > I looked into this a bit. Removing that bit of code, the only regression > test changes for "check-world" are the expected ones, with nothing else > breaking. Running installcheck+pg_upgrade to the patched version of HEAD > from each of versions 11, 12, 13 and 14 doesn't turn up anything untoward. I looked into this a bit, too. I attach a draft patch for removing the self-admin exception. I found that having is_admin_of_role() return true matters in three ways: (1) It lets you grant membership in the role to some other role. (2) It lets you revoke membership in the role from some other role. (3) It changes the return value of pg_role_aclcheck(), which is used in the implementation of various SQL-callable functions all invoked via the name pg_has_role(). We've mostly been discussing (2) as an issue, but (1) and (3) are pretty interesting too. Regarding (3), there is a comment in the code indicating that Noah considered the self-admin exception something of a wart as far as pg_has_role() is concerned. As to (1), I discovered that today you can do this: rhaas=# create user foo; CREATE ROLE rhaas=# create user bar; CREATE ROLE rhaas=# \q [rhaas ~]$ psql -U foo rhaas psql (15devel) Type "help" for help. rhaas=> grant foo to bar with admin option; GRANT ROLE I don't know why I didn't realize that before. It's a natural result of treating the logged-in user as if they had admin option. But it's weird that you can't even be granted WITH ADMIN OPTION on your own login role, but at the same time without having it you can grant it to someone else! I believe there are three other points worth some consideration here. First, in the course of my investigation I re-discovered what Tom already did a good job articulating: tgl> Having said that, one thing that I find fishy is that it's not clear tgl> where the admin privilege for a role originates. After "CREATE ROLE tgl> alice", alice has no members, therefore none that have admin privilege, tgl> therefore the only way that the first member could be added is via tgl> superuser deus ex machina. This does not seem clean. I agree with that, but I don't think it's a sufficient reason for keeping the self-admin exception, because the same problem exists for non-login roles. I don't even think it's the right idea conceptually to suppose that the power to administer a role originates from the role itself. If that were so, then it would be inherited by all members of the role along with all the rest of the role's privileges, which is so clearly not right that we've already prohibited a role from having WITH ADMIN OPTION on itself. In my opinion, the right to administer a role - regardless of whether or not it is a login role - most naturally vests in the role that created it, or something in that direction at least, if not that exact thing. Today, that means the superuser or a CREATEROLE user who could hack superuser if they wished. In the future, I hope for other alternatives, as recently argued on other threads. But we need not resolve the question of how that should work exactly in order to agree (as I hope we do) that doubling down on the self-administration exception is not the answer. Second, it occured to me to wonder what implications a change like this might have for dump and restore. If privilege restoration somehow relied on this behavior, then we'd have a problem. But I don't think it does, because (a) pg_dump can SET ROLE but can't change the session user without reconnecting, so it's unclear how we could be relying on it; (b) it wouldn't work for non-login roles, and it's unlikely that we would treat login and non-login roles different in terms of restoring privileges, and (c) when I execute the example shown above and then run pg_dump, there's no attempt to change the current user, it just dumps "GRANT foo TO bar WITH ADMIN OPTION GRANTED BY foo". Third, it occurred to me to wonder whether some users might be using and relying upon this behavior. It's certainly possible, and it does suck that we'd be removing it without providing a workable substitute. But it's probably not a LOT of users because most people who have commented on this topic on this mailing list seem to find granting membership in a login role a super-weird thing to do, because a lot of people really seem to want every role to be a user or a group, and a login role with members feels like it's blurring that line. I'm inclined to think that the small number of people who may be unhappy is an acceptable price to pay for removing this wart, but it's a judgement call and if someone has information to suggest that I'm wrong, it'd be good to hear about that. Thanks, -- Robert Haas EDB: http://www.enterprisedb.com remove-self-admin.patch Description: Binary data
Re: role self-revocation
On Wed, Mar 9, 2022 at 7:55 AM Peter Eisentraut wrote: > Do we have subtractive permissions today? Not in the GRANT/REVOKE sense, I think, but you can put a user in a group and then mention that group in pg_hba.conf. And that line might be "reject" or whatever. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
On 07.03.22 19:18, Robert Haas wrote: That all said, permissions SHOULD BE strictly additive. If boss doesn't want to be a member of pg_read_all_files allowing them to revoke themself from that role seems like it should be acceptable. If there is fear in allowing someone to revoke (not add) themselves as a member of a different role that suggests we have a design issue in another feature of the system. Today, they neither grant nor revoke, and the self-revocation doesn't seem that important to add. I disagree with this on principle, and I also think that's not how it works today. On the general principle, I do not see a compelling reason why we should have two systems for maintaining groups of users, one of which is used for additive things and one of which is used for subtractive things. Do we have subtractive permissions today?
Re: role self-revocation
> On Mar 7, 2022, at 12:16 PM, Tom Lane wrote: > > What would be > lost if we drop it? I looked into this a bit. Removing that bit of code, the only regression test changes for "check-world" are the expected ones, with nothing else breaking. Running installcheck+pg_upgrade to the patched version of HEAD from each of versions 11, 12, 13 and 14 doesn't turn up anything untoward. The change I used (for reference) is attached: v1-0001-WIP-Removing-role-self-administration.patch.WIP Description: Binary data — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: role self-revocation
On Mon, Mar 7, 2022 at 1:16 PM Tom Lane wrote: > Based on Robert's archaeological dig, it now seems that the fact that > we have any such behavior at all was just a mistake. What would be > lost if we drop it? > Probably nothing that couldn't be replaced, and with a better model, but I do have a concern that there are setups in the wild inadvertently using this behavior. Enough so that I would vote to change it but include a migration GUC to restore the current behavior, probably with a deprecation warning. Kinda depends on the post-change dump/restore mechanics. But just tearing it out wouldn't seem extraordinary for us. > > Having said that, one thing that I find fishy is that it's not clear > where the admin privilege for a role originates. I do not see a problem with there being no inherent admin privilege for a role. A superuser or CREATEROLE user holds admin privilege on all roles in the cluster. They can delegate the privilege to administer a role to yet another role in the system. The necessitates creating two roles - the one being administered and the one being delegated to. I don't see a benefit to saving which specific superuser or CREATEROLE user "owns" the role that is to be administered. Not unless non-owner CREATEROLE users are prevented from exercising admin privileges on the role. That all said, I'd accept the choice to include such ownership information as a requirement for meeting the auditing needs of DBAs. But I would argue that such auditing probably needs to be external to the working system - the fact that ownership can be changed reduces the benefit of an in-database value. > If we recorded which user created the role, we could act as though that user has > admin privilege (whether or not it's a member). I suppose we could record the current owner of a role but that seems unnecessary. I dislike using the "created" concept by virtue of the fact that, for routines, "security definer" implies creator but it actually means "security owner". David J.
Re: role self-revocation
Mark Dilger writes: >> On Mar 7, 2022, at 12:01 PM, Robert Haas wrote: >> >> It's been pointed out upthread that this would have undesirable >> security implications, because the admin option would be inherited, >> and the implicit permission isn't. > Right, but with a reflexive self-admin-option, we could document that it > works in a non-inherited way. We'd just be saying the current hard-coded > behavior is an option which can be revoked rather than something you're stuck > with. After reflection, I think that role self-admin is probably a bad idea that we should stay away from. It could perhaps be reasonable given some other system design and/or syntax than what SQL gives us, but we're dealing in SQL. It doesn't make sense to GRANT a role to itself, and therefore it likewise doesn't make sense to GRANT WITH ADMIN OPTION. Based on Robert's archaeological dig, it now seems that the fact that we have any such behavior at all was just a mistake. What would be lost if we drop it? Having said that, one thing that I find fishy is that it's not clear where the admin privilege for a role originates. After "CREATE ROLE alice", alice has no members, therefore none that have admin privilege, therefore the only way that the first member could be added is via superuser deus ex machina. This does not seem clean. If we recorded which user created the role, we could act as though that user has admin privilege (whether or not it's a member). Perhaps I'm reinventing something that was already discussed upthread. I wonder what the SQL spec has to say on this point, too. regards, tom lane
Re: role self-revocation
> On Mar 7, 2022, at 12:03 PM, Mark Dilger wrote: > > Right, but with a reflexive self-admin-option, we could document that it > works in a non-inherited way. We'd just be saying the current hard-coded > behavior is an option which can be revoked rather than something you're stuck > with. We could also say that the default is to not have admin option on yourself, with that being something grantable, but that is a larger change from the historical behavior and might have more consequences for dump/restore, etc. My concern about just nuking self-admin is that there may be sites which use self-admin and we'd be leaving them without a simple work-around after upgrade, because they couldn't restore the behavior by executing a grant. They'd have to more fundamentally restructure their role relationships to not depend on self-admin, something which might be harder for them to do. Perhaps nobody is using self-admin, or very few people are using it, and I'm being overly concerned. — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: role self-revocation
> On Mar 7, 2022, at 12:01 PM, Robert Haas wrote: > > It's been pointed out upthread that this would have undesirable > security implications, because the admin option would be inherited, > and the implicit permission isn't. Right, but with a reflexive self-admin-option, we could document that it works in a non-inherited way. We'd just be saying the current hard-coded behavior is an option which can be revoked rather than something you're stuck with. — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: role self-revocation
On Mon, Mar 7, 2022 at 2:59 PM Mark Dilger wrote: > This test failure is just a manifestation of the intended change, but > assuming we make no other changes, the error message would clearly need to be > updated, because it suggests the role should have admin_option on itself, a > situation which is not currently supported. It's been pointed out upthread that this would have undesirable security implications, because the admin option would be inherited, and the implicit permission isn't. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
> On Mar 7, 2022, at 10:28 AM, Tom Lane wrote: > > Does anything interesting break if you do just take it out? SET SESSION AUTHORIZATION regress_priv_group2; GRANT regress_priv_group2 TO regress_priv_user5; -- ok: a role can self-admin -NOTICE: role "regress_priv_user5" is already a member of role "regress_priv_group2" +ERROR: must have admin option on role "regress_priv_group2" This test failure is just a manifestation of the intended change, but assuming we make no other changes, the error message would clearly need to be updated, because it suggests the role should have admin_option on itself, a situation which is not currently supported. Perhaps we should support that, though, by adding a reflexive aclitem[] to pg_authid (meaning it tracks which privileges a role has on itself) with tracking of who granted it, so that revocation can be handled properly. The aclitem could start out null, meaning the role has by default the traditional limited self-admin which the code comments discuss: /* * A role can admin itself when it matches the session user and we're * outside any security-restricted operation, SECURITY DEFINER or * similar context. SQL-standard roles cannot self-admin. However, * SQL-standard users are distinct from roles, and they are not * grantable like roles: PostgreSQL's role-user duality extends the * standard. Checking for a session user match has the effect of * letting a role self-admin only when it's conspicuously behaving * like a user. Note that allowing self-admin under a mere SET ROLE * would make WITH ADMIN OPTION largely irrelevant; any member could * SET ROLE to issue the otherwise-forbidden command. * * Withholding self-admin in a security-restricted operation prevents * object owners from harnessing the session user identity during * administrative maintenance. Suppose Alice owns a database, has * issued "GRANT alice TO bob", and runs a daily ANALYZE. Bob creates * an alice-owned SECURITY DEFINER function that issues "REVOKE alice * FROM carol". If he creates an expression index calling that * function, Alice will attempt the REVOKE during each ANALYZE. * Checking InSecurityRestrictedOperation() thwarts that attack. * * Withholding self-admin in SECURITY DEFINER functions makes their * behavior independent of the calling user. There's no security or * SQL-standard-conformance need for that restriction, though. * * A role cannot have actual WITH ADMIN OPTION on itself, because that * would imply a membership loop. Therefore, we're done either way. */ For non-null aclitem[], we could support REVOKE ADMIN OPTION FOR joe FROM joe, and for explicit re-grants, we could track who granted it, such that further revocations could properly refuse if the revoker doesn't have sufficient privileges vis-a-vis the role that granted it in the first place. I have not yet tried to implement this, and might quickly hit problems with the idea, but will take a stab at a proof-of-concept patch unless you suggest a better approach. Thoughts? — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: role self-revocation
On Mon, Mar 7, 2022 at 2:29 PM David G. Johnston wrote: > You mean the one that was based upon your "ooops"...I discounted that > immediately because members cannot revoke their own membership in a group > unless they were given WITH ADMIN OPTION on that group. Oh, hmm. That example might be backwards from the case I'm talking about. > The mere fact that the pg_hba.conf concern raised there hasn't been reported > as a live issue suggests the lack of any meaningful design flaw here. Not really. The system is full of old bugs, just as all software system are, and the particular role self-administration behavior that is at issue here appears to be something that was accidentally introduced 16 years years ago in a commit that did something else and never scrutinized from a design perspective since then. Personally, I've been shocked by the degree to which this entire area seems to be full of design flaws and half-baked code. I mean, just the fact that the pg_auth_members.grantor can be left pointing to a role OID that no longer exists is pretty crazy, right? I don't think anyone today would consider something with that kind of wart committable. > That isn't to say that having a LOGIN role get an automatic temporary WITH > ADMIN OPTION on itself is a good thing - but there isn't any privilege > escalation vector here to be squashed. There is just a "DBAs should treat > LOGIN roles as leaf nodes" expectation in which case there would be no > superuser granted memberships to be removed. Well, we may not have found one yet, but that doesn't prove none exists. In any case, if we can agree that it's not necessarily a desirable behavior, that's good enough for me. (I still disagree with the idea that LOGIN roles have to be leaf nodes. We could have a system where that's true, but that's not how the system we actually have is designed.) -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
On Mon, Mar 7, 2022 at 11:18 AM Robert Haas wrote: > In terms of how > things work today, see Joshua Brindle's email about the use of groups > in pg_hba.conf. That is an excellent example of how removing oneself > from a group could enable one to bypass security restrictions intended > by the DBA. > > You mean the one that was based upon your "ooops"...I discounted that immediately because members cannot revoke their own membership in a group unless they were given WITH ADMIN OPTION on that group. The mere fact that the pg_hba.conf concern raised there hasn't been reported as a live issue suggests the lack of any meaningful design flaw here. That isn't to say that having a LOGIN role get an automatic temporary WITH ADMIN OPTION on itself is a good thing - but there isn't any privilege escalation vector here to be squashed. There is just a "DBAs should treat LOGIN roles as leaf nodes" expectation in which case there would be no superuser granted memberships to be removed. David J.
Re: role self-revocation
Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > I'm not quite following this bit. Where would SET ROLE come into play > > when we're talking about old dump scripts and how the commands in those > > scripts might be interpreted by newer versions of PG..? > > No, the concern there is the other way around: what if you take a > script made by newer pg_dump and try to load it into an older server > that doesn't have the GRANTED BY option? Wow. No, I really don't think I can agree that we need to care about this. > We're accustomed to saying that that doesn't work if you use a > database feature that didn't exist in the old server, but > privilege grants are hardly that. I don't want us to change the > pg_dump output in such a way that the grants can't be restored at all > to an older server, just because of a syntax choice that we could > make backwards-compatibly instead of not-backwards-compatibly. GRANTED BY is clearly such a feature that exists in the newer version and doesn't exist in the older and I can't agree that we should complicate things for ourselves and bend over backwards to try and make it work to take a dump from a newer version of PG and make it work on random older versions. Folks are also able to exclude privileges from dumps if they want to. Where do we document that we are going to put in effort to make these kinds of things work? What other guarantees are we supposed to be providing regarding using output from a newer pg_dump against older servers? What about newer custom format dumps? Surely you're not suggesting that we need to back-patch support for them to released versions of pg_restore. Thanks, Stephen signature.asc Description: PGP signature
Re: role self-revocation
On Mon, Mar 7, 2022 at 1:58 PM Tom Lane wrote: > Stephen Frost writes: > > I'm not quite following this bit. Where would SET ROLE come into play > > when we're talking about old dump scripts and how the commands in those > > scripts might be interpreted by newer versions of PG..? > > No, the concern there is the other way around: what if you take a > script made by newer pg_dump and try to load it into an older server > that doesn't have the GRANTED BY option? > > We're accustomed to saying that that doesn't work if you use a > database feature that didn't exist in the old server, but > privilege grants are hardly that. I don't want us to change the > pg_dump output in such a way that the grants can't be restored at all > to an older server, just because of a syntax choice that we could > make backwards-compatibly instead of not-backwards-compatibly. Are you absolutely positive that it's that simple? I mean, what if the SET ROLE command has other side effects, or if the GRANT command behaves differently in some way as a result of the SET ROLE having been done? I feel like a solution that involves explicitly specifying the behavior that we want (i.e. GRANTED BY) is likely to be more reliable and more secure than a solution which involves absorbing a key value from a session property (i.e. the role established by SET ROLE). Even if we decide that SET ROLE is the way to go for compatibility reasons, I would personally say that it's an inferior hack only worth accepting for that reason than a truly desirable design. See CVE-2018-1058 for an example of what I'm talking about. The prevailing search_path turned out to affect not only the creation schema, as intended, but also the resolution of references to other objects mentioned in the CREATE COMMAND, as not intended. I don't see a similar hazard here, but I'm worried that there might be one. Declarative syntax is a very powerful tool for avoiding those kinds of mishaps, and I think we should make as much use of it as we can. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > * Tom Lane (t...@sss.pgh.pa.us) wrote: > >> Agreed, this is not something to move on quickly. We might want > >> to think about adjusting pg_dump to use explicit GRANTED BY > >> options in GRANT/REVOKE a release or two before making incompatible > >> changes. > > > I'm with Robert on this though- folks should know already that they need > > to use the pg_dump of the version of PG that they want to move to and > > not try to re-use older pg_dump output with newer versions, for a number > > of reasons and this is just another. > > Yeah, in an ideal world you'd do that, but our users don't always have > the luxury of living in an ideal world. Sometimes all you've got is > an old pg_dump file. Perhaps this behavior wouldn't mess things up > enough to make the restored database unusable, but we need to think > about (and test) that case while we're considering changes. I agree it's something to consider and deal with if we're able to do so sanely, but I disagree that we should be beholden to old dump files when considering how to move the project forward. Further, they can surely build and install the version of PG that goes with that dump file in a great many cases and then dump the data out using a newer version of pg_dump. For 5 years they could do that with a completely supported version of PG, but we've recently agreed to make an effort to do more here by supporting the building of even older versions on modern systems. Thanks, Stephen signature.asc Description: PGP signature
Re: role self-revocation
Stephen Frost writes: > I'm not quite following this bit. Where would SET ROLE come into play > when we're talking about old dump scripts and how the commands in those > scripts might be interpreted by newer versions of PG..? No, the concern there is the other way around: what if you take a script made by newer pg_dump and try to load it into an older server that doesn't have the GRANTED BY option? We're accustomed to saying that that doesn't work if you use a database feature that didn't exist in the old server, but privilege grants are hardly that. I don't want us to change the pg_dump output in such a way that the grants can't be restored at all to an older server, just because of a syntax choice that we could make backwards-compatibly instead of not-backwards-compatibly. regards, tom lane
Re: role self-revocation
Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> Agreed, this is not something to move on quickly. We might want >> to think about adjusting pg_dump to use explicit GRANTED BY >> options in GRANT/REVOKE a release or two before making incompatible >> changes. > I'm with Robert on this though- folks should know already that they need > to use the pg_dump of the version of PG that they want to move to and > not try to re-use older pg_dump output with newer versions, for a number > of reasons and this is just another. Yeah, in an ideal world you'd do that, but our users don't always have the luxury of living in an ideal world. Sometimes all you've got is an old pg_dump file. Perhaps this behavior wouldn't mess things up enough to make the restored database unusable, but we need to think about (and test) that case while we're considering changes. regards, tom lane
Re: role self-revocation
Greetings, * Robert Haas (robertmh...@gmail.com) wrote: > On Sun, Mar 6, 2022 at 11:34 AM Tom Lane wrote: > > I was thinking the former ... however, after a bit of experimentation > > I see that we accept "grant foo to bar granted by baz" a VERY long > > way back, but the "granted by" option for object privileges is > > (a) pretty new and (b) apparently restrictively implemented: > > > > regression=# grant delete on alices_table to bob granted by alice; > > ERROR: grantor must be current user > > > > That's ... surprising. I guess whoever put that in was only > > interested in pro-forma SQL syntax compliance and not in making > > a usable feature. > > It appears so: > https://www.postgresql.org/message-id/2073b6a9-7f79-5a00-5f26-cd19589a52c7%402ndquadrant.com > > It doesn't seem like that would be hard to fix. Maybe we should just do that. Yeah, that seems like something that should be fixed. Superusers should be allowed to set GRANTED BY to whatever they feel like, and I'd argue that a role who wants a GRANT to actually be GRANTED BY some other role they're a member of should also be allowed to (as they could anyway by doing a SET ROLE), provided that role also has the privileges to do the GRANT itself, of course. > > So if we decide to extend this change into object privileges > > it would be advisable to use SET ROLE, else we'd be giving up > > an awful lot of backwards compatibility in dump scripts. > > But if we're only talking about role grants then I think > > GRANTED BY would work fine. > > OK. I'm not quite following this bit. Where would SET ROLE come into play when we're talking about old dump scripts and how the commands in those scripts might be interpreted by newer versions of PG..? Thanks, Stephen signature.asc Description: PGP signature
Re: role self-revocation
On Mon, Mar 7, 2022 at 11:18 AM Robert Haas wrote: > On Sun, Mar 6, 2022 at 11:01 PM David G. Johnston > wrote: > > The example, which you moved here, then attempts to demonstrate this > "fact" but gets it wrong. Boss became a member of peon so if you want to > demonstrate self-administration of a role's membership in a different group > you have to login as boss, not peon. Doing that, and then revoking peon > from boss, yields "ERROR: must have admin option on role "peon"". > > This doesn't seem to me to be making a constructive argument. I showed > an example with certain names demonstrating a certain behavior that I > find problematic. Whether you choose the wording of the original thread: "This is because we allow 'self administration' of roles, meaning that they can decide what other roles they are a member of." https://www.postgresql.org/message-id/flat/20211005025746.GN20998%40tamriel.snowman.net Or you quote at the top of this one: > The ability of a role to revoke itself from some other role is just > something we need to accept as being a change that needs to be made, This example: rhaas=# create user boss; CREATE ROLE rhaas=# create user peon; CREATE ROLE rhaas=# grant peon to boss; GRANT ROLE rhaas=# \c - peon You are now connected to database "rhaas" as user "peon". rhaas=> revoke peon from boss; -- i don't like being bossed around! REVOKE ROLE Fails to demonstrate the boss "can revoke itself from peon" / "boss can decide what other roles they are a member of." You are logged in as peon when you do the revoke, not boss, so the extent of what "boss" can or cannot do has not been shown. boss is a member of peon, not the other way around. That the wording "grant peon to boss" makes you think otherwise is unfortunate. David J.
Re: role self-revocation
Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Robert Haas writes: > > 1. What should be the exact rule for whether A can remove a grant made > > by B? Is it has_privs_of_role()? is_member_of_role()? Something else? > > No strong opinion here, but I'd lean slightly to the more restrictive > option. > > > 2. What happens if the same GRANT is enacted by multiple users? For > > example, suppose peon does "GRANT peon to boss" and then the superuser > > does the same thing afterwards, or vice versa? One design would be to > > try to track those as two separate grants, but I'm not sure if we want > > to add that much complexity, since that's not how we do it now and it > > would, for example, implicate the choice of PK on the pg_auth_members > > table. > > As you note later, we *do* track such grants separately in ordinary > ACLs, and I believe this is clearly required by the SQL spec. Agreed. > It says (for privileges on objects): > > Each privilege is represented by a privilege descriptor. > A privilege descriptor contains: > — The identification of the object on which the privilege is granted. > — The of the grantor of the privilege. > ^^^ > — The of the grantee of the privilege. > — Identification of the action that the privilege allows. > — An indication of whether or not the privilege is grantable. > — An indication of whether or not the privilege has the WITH HIERARCHY > OPTION specified. > > Further down (4.42.3 in SQL:2021), the granting of roles is described, > and that says: > > Each role authorization is described by a role authorization descriptor. > A role authorization descriptor includes: > — The role name of the role. > — The authorization identifier of the grantor. > > — The authorization identifier of the grantee. > — An indication of whether or not the role authorization is grantable. > > If we are not tracking the grantors of role authorizations, > then we are doing it wrong and we ought to fix that. Yup, and as noted elsewhere, we are tracking it but not properly dealing with dependencies nor are we considering the grantor when REVOKE is run. Looking at the spec for REVOKE is quite useful when trying to understand how this is all supposed to work (and, admittedly, isn't something I did enough of when I did the original work on roles... sorry about that, was early on). In particular, a REVOKE only works when it finds something to revoke/remove, and part of that search includes basically "was it the current role who was the grantor?" The specific language here being: A role authorization descriptor is said to be identified if it defines the grant of any of the specified roles revoked to grantee with grantor A. Basically, a role authorization descriptor isn't identified unless it's one that this user/role had previously granted. > > 3. What happens if a user is dropped after being recorded as a > > grantor? > > Should work the same as it does now for ordinary ACLs, ie, you > gotta drop the grant first. Agreed. > > 4. Should we apply this rule to other types of grants, rather than > > just to role membership? > > I am not sure about the reasoning behind the existing rule that > superuser-granted privileges are recorded as being granted by the > object owner. It does feel more like a wart than something we want. > It might have been a hack to deal with the lack of GRANTED BY > options in GRANT/REVOKE back in the day. Yeah, that doesn't seem right and isn't great. > Changing it could have some bad compatibility consequences though. > In particular, I believe it would break existing pg_dump files, > in that after restore all privileges would be attributed to the > restoring superuser, and there'd be no very easy way to clean that > up. Ugh, that's pretty grotty, certainly. > > Please note that it is not really my intention to try to shove > > anything into v15 here. > > Agreed, this is not something to move on quickly. We might want > to think about adjusting pg_dump to use explicit GRANTED BY > options in GRANT/REVOKE a release or two before making incompatible > changes. I'm with Robert on this though- folks should know already that they need to use the pg_dump of the version of PG that they want to move to and not try to re-use older pg_dump output with newer versions, for a number of reasons and this is just another. Thanks, Stephen signature.asc Description: PGP signature
Re: role self-revocation
On Mon, Mar 7, 2022 at 1:28 PM Tom Lane wrote: > Ugh, I think you are right. It's been a long time of course, but it sure > looks like that was copied-and-pasted without recognizing that it was > wrong in this function because of the need to check the admin_option flag. > And then in the later security discussion we didn't realize that the > problematic behavior was a flat-out thinko, so we narrowed it as much as > we could instead of just taking it out. > > Does anything interesting break if you do just take it out? That is an excellent question, but I haven't had time yet to investigate the matter. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
Robert Haas writes: > It appears to me that the actual behavior of having is_admin_of_role() > return true when member == role dates to > f9fd1764615ed5d85fab703b0ffb0c323fe7dfd5 (Tom Lane, 2005). If I'm not > reading this code wrong, prior to that commit, it seems to me that we > only searched the roles that were members of that role, directly or > indirectly, and you had to have admin_option on the last hop of the > membership chain in order to get a "true" result. But that commit, > among other changes, made member == role a special case, but the > comment just says /* Fast path for simple case */ which makes it > appear that it wasn't thought to be a behavior change at all, but it > looks to me like it was. Am I confused? Ugh, I think you are right. It's been a long time of course, but it sure looks like that was copied-and-pasted without recognizing that it was wrong in this function because of the need to check the admin_option flag. And then in the later security discussion we didn't realize that the problematic behavior was a flat-out thinko, so we narrowed it as much as we could instead of just taking it out. Does anything interesting break if you do just take it out? regards, tom lane
Re: role self-revocation
On Sun, Mar 6, 2022 at 11:01 PM David G. Johnston wrote: > The example, which you moved here, then attempts to demonstrate this "fact" > but gets it wrong. Boss became a member of peon so if you want to > demonstrate self-administration of a role's membership in a different group > you have to login as boss, not peon. Doing that, and then revoking peon from > boss, yields "ERROR: must have admin option on role "peon"". This doesn't seem to me to be making a constructive argument. I showed an example with certain names demonstrating a certain behavior that I find problematic. You don't have to think it's problematic, and you can show other examples that demonstrate things you want to show. But please don't tell me that when I literally cut and paste the output from my terminal into an email window, what I'm showing is somehow counterfactual. The behavior as it exists today is surely a fact, and an easily demonstrable one at that. It's not a "fact'" in quotes, and it doesn't "get it wrong". It is the actual behavior and the example with the names I picked demonstrates precisely what I want to demonstrate. When you say that I should have chosen a different example or used different identifier names or talked about it in different way, *that* is an opinion. I believe that you are wholly entitled to that opinion, even if (as in this case) I disagree, but I believe that it is not right at all to make it sound as if I don't have the right to pick the examples I care about, or as if terminal output is not a factual representation of how things work today. > So no, without "WITH ADMIN OPTION" a role cannot decide what other roles they > are a member of. It clearly can in some limited cases, because I showed an example demonstrating *exactly that thing*. > I don't necessarily have an issue changing self-administration but if the > motivating concern is that all these new pg_* roles we are creating are > something a normal user can opt-out of/revoke that simply isn't the case > today, unless they are added to the pg_* role WITH ADMIN OPTION. I agree with this, but that's not my concern, because that's a different use case from the one that I complained about. Since the session user exception only applies to login roles, the problem that I'm talking about only occurs when a login role is granted to some other role. > That all said, permissions SHOULD BE strictly additive. If boss doesn't want > to be a member of pg_read_all_files allowing them to revoke themself from > that role seems like it should be acceptable. If there is fear in allowing > someone to revoke (not add) themselves as a member of a different role that > suggests we have a design issue in another feature of the system. Today, > they neither grant nor revoke, and the self-revocation doesn't seem that > important to add. I disagree with this on principle, and I also think that's not how it works today. On the general principle, I do not see a compelling reason why we should have two systems for maintaining groups of users, one of which is used for additive things and one of which is used for subtractive things. That is a lot of extra machinery for little gain, especially given how close we are to having it sorted out so that the same mechanism can serve both purposes. It presently appears to me that if we either remove the session user exception OR do the grantor-tracking thing discussed earlier, we can get to a place where the same facility can be used for either purpose. That would, I think, be a significant step forward over the status quo. In terms of how things work today, see Joshua Brindle's email about the use of groups in pg_hba.conf. That is an excellent example of how removing oneself from a group could enable one to bypass security restrictions intended by the DBA. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
On Mon, Mar 7, 2022 at 11:04 AM Tom Lane wrote: > > Is there some use case for the behavior described in that last > > sentence? > > Good question. You might try figuring out when that text was added > and then see if there's relevant discussion in the archives. Apparently the permission used to be broader, and commit fea164a72a7bfd50d77ba5fb418d357f8f2bb7d0 (February 2014, Noah, CVE-2014-0060) restricted it by requiring that (a) the user had to be the logged-in user, rather than an identity assumed via SET ROLE (so maybe my bogus example from before would have worked in 2013) and (b) that we're not in a security-restricted operation at the time. Interestingly, it appears to me that the behavior wasn't documented prior to that commit. The previous text read simply: If WITH ADMIN OPTION is specified, the member can in turn grant membership in the role to others, and revoke membership in the role as well. Without the admin option, ordinary users cannot do that. That doesn't give any hint that self-administration is a special case. I reviewed the (private) discussion of this vulnerability on the pgsql-security mailing list where various approaches were considered. I think it's safe to share a few broad details about that conversation publicly now, since it was many years ago and the fix has long since been published. There was discussion of making this self-administration behavior something that could be turned off, but such a change was deemed too large for the back-branches. There was no discussion that I could find about removing the behavior altogether. It was noted that having a special case for this was different than granting WITH ADMIN OPTION because WITH ADMIN OPTION is inherited and being logged in as a certain user is not. It appears to me that the actual behavior of having is_admin_of_role() return true when member == role dates to f9fd1764615ed5d85fab703b0ffb0c323fe7dfd5 (Tom Lane, 2005). If I'm not reading this code wrong, prior to that commit, it seems to me that we only searched the roles that were members of that role, directly or indirectly, and you had to have admin_option on the last hop of the membership chain in order to get a "true" result. But that commit, among other changes, made member == role a special case, but the comment just says /* Fast path for simple case */ which makes it appear that it wasn't thought to be a behavior change at all, but it looks to me like it was. Am I confused? -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
On Mon, Mar 7, 2022 at 9:04 AM Tom Lane wrote: > Just looking at it now, without having done any historical research, > I wonder why it is that we don't attach significance to WITH ADMIN > OPTION being granted to the role itself. It seems like the second > part of that sentence is effectively saying that a role DOES have > admin option on itself, contradicting the first part. > > WITH ADMIN OPTION is inheritable which is really bad if the group has WITH ADMIN OPTION on itself. The session_user exception temporarily grants WITH ADMIN OPTION to the group but it is done in such a way so that it is not inheritable. There is no possible way to even assign WITH ADMIN OPTION on a role to itself since pg_auth_members doesn't record a self-relationship and admin_option only exists there. David J. P.S. Feature request; modify \du+ to show which "Member of" roles a given role has the WITH ADMIN OPTION privilege on.
Re: role self-revocation
Robert Haas writes: > Hmm. I think the real issue is what David Johnson calls the session > user exception. I hadn't quite understood how that played into this. > According to the documentation: "If WITH ADMIN OPTION is specified, > the member can in turn grant membership in the role to others, and > revoke membership in the role as well. Without the admin option, > ordinary users cannot do that. A role is not considered to hold WITH > ADMIN OPTION on itself, but it may grant or revoke membership in > itself from a database session where the session user matches the > role." > Is there some use case for the behavior described in that last > sentence? Good question. You might try figuring out when that text was added and then see if there's relevant discussion in the archives. Just looking at it now, without having done any historical research, I wonder why it is that we don't attach significance to WITH ADMIN OPTION being granted to the role itself. It seems like the second part of that sentence is effectively saying that a role DOES have admin option on itself, contradicting the first part. regards, tom lane
Re: role self-revocation
On Mon, Mar 7, 2022 at 8:37 AM Robert Haas wrote: > A role is not considered to hold WITH > ADMIN OPTION on itself, but it may grant or revoke membership in > itself from a database session where the session user matches the > role." > > Is there some use case for the behavior described in that last > sentence? I can imagine, in particular combined with CREATEROLE, that this allows for any user to delegate their personal permissions to a separate newly created user. Like an assistant. I'm not all that sure whether CREATEROLE is presently safe enough to give to a normal user in order to make this use case work but it seems reasonable. I would be concerned about changing the behavior at this point. But I would be in favor of at least removing the hard-coded exception and linking it to a role attribute. That attribute can default to "SELFADMIN" to match the existing behavior but then "NOSELFADMIN" would exist to disable that behavior on the per-role basis. Still tied to session_user as opposed to current_user. David J. P.S. create role selfadmin admin selfadmin; -- ERROR: role "selfadmin" is a member of role "selfadmin" create role selfadmin; grant selfadmin to selfadmin with admin option; -- ERROR: role "selfadmin" is a member of role "selfadmin" The error message seems odd. I tried this because instead of a "SELFADMIN" attribute adding a role to itself WITH ADMIN OPTION could be defined to have the same effect. You cannot change WITH ADMIN OPTION independently of the adding of the role to the group.
Re: role self-revocation
On Sun, Mar 6, 2022 at 2:09 PM David G. Johnston wrote: > So, do we really want to treat every single login role as a potential group > by keeping the session_user exception? I think that we DO want to continue to treat login roles as potentially grantable privileges. That feels fundamentally useful to me. The superuser is essentially granted the privileges of all users on the system, and has all the rights they have, including the right to drop tables owned by those users as if they were the owner of those tables. If it's useful for the superuser to implicitly have the rights of all users on the system, why should it not be useful for some non-superuser to implicitly have the rights of some other users on the system? I think it pretty clearly is. If one of my colleagues leaves the company, the DBA can say "grant jdoe to rhaas" and let me mess around with this stuff. Or, the DBA can grant me the privileges of all my direct reports even when they're not leaving so that I can sort out anything I need to do without superuser involvement. That all seems cool and OK to me. Now I think it is fair to say that we could have chosen a different design, and MAYBE that would have been better. Nobody forced us to conflate users and groups into a unified thing called roles, and I think there's pretty good evidence that it's confusing and counterintuitive in some ways. There's also no intrinsic reason why the superuser has to be able to directly exercise the privileges of every role rather than, say, having a way to become any given role. But at this point, those design decisions are pretty well baked into the system design, and I don't really think it's likely that we want to change them. To put that another way, just because you don't like the idea of granting one login role to another login role, that doesn't mean that the feature doesn't exist, and as long as that feature does exist, trying to make it work better or differently is fair game. But I think that's separate from your other question about whether we should remove the session user exception. That looks tempting to me at first glance, because we have exchanged several hundred, and it really feels more like several million, emails on this list about how much of a problem it is that an unprivileged user can just log in and run a REVOKE. It breaks the idea that the people WITH ADMIN OPTION on a role are the ones who control membership in that role. Joshua Brindle's note upthread about the interaction of this with pg_hba.conf is another example of that, and I think there are more. Any idea that a role is a general-purpose way of designating a group of users for some security critical purpose is threatened if people can make changes to the membership of that group without being specifically authorized to do so. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
On Sun, Mar 6, 2022 at 11:53 AM Tom Lane wrote: > Really? > > regression=# grant admin to joe; > GRANT ROLE > regression=# grant admin to sally; > GRANT ROLE > regression=# \c - joe > You are now connected to database "regression" as user "joe". > regression=> revoke admin from sally; > ERROR: must have admin option on role "admin" > regression=> set role admin; > SET > regression=> revoke admin from sally; > ERROR: must have admin option on role "admin" Oops. I stand corrected. > I think there is an issue here around exactly what the admin option > means, but if it doesn't grant you the ability to remove grants > made by other people, it's pretty hard to see what it's for. Hmm. I think the real issue is what David Johnson calls the session user exception. I hadn't quite understood how that played into this. According to the documentation: "If WITH ADMIN OPTION is specified, the member can in turn grant membership in the role to others, and revoke membership in the role as well. Without the admin option, ordinary users cannot do that. A role is not considered to hold WITH ADMIN OPTION on itself, but it may grant or revoke membership in itself from a database session where the session user matches the role." Is there some use case for the behavior described in that last sentence? If that exception is the only case in which an unprivileged user can revoke a grant made by someone else, then getting rid of it seems pretty appealing from where I sit. I can't speak to the standards compliance end of things, but it doesn't intrinsically seem bothersome that having "WITH ADMIN OPTION" on a role lets you control who has membership in said role. And certainly it's not bothersome that the superuser can change whatever they want. The problem here is just that a user with NO special privileges on any role, including their own, can make changes that more privileged users might not like. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
On Sun, Mar 6, 2022 at 11:34 AM Tom Lane wrote: > I was thinking the former ... however, after a bit of experimentation > I see that we accept "grant foo to bar granted by baz" a VERY long > way back, but the "granted by" option for object privileges is > (a) pretty new and (b) apparently restrictively implemented: > > regression=# grant delete on alices_table to bob granted by alice; > ERROR: grantor must be current user > > That's ... surprising. I guess whoever put that in was only > interested in pro-forma SQL syntax compliance and not in making > a usable feature. It appears so: https://www.postgresql.org/message-id/2073b6a9-7f79-5a00-5f26-cd19589a52c7%402ndquadrant.com It doesn't seem like that would be hard to fix. Maybe we should just do that. > So if we decide to extend this change into object privileges > it would be advisable to use SET ROLE, else we'd be giving up > an awful lot of backwards compatibility in dump scripts. > But if we're only talking about role grants then I think > GRANTED BY would work fine. OK. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
On Sun, Mar 6, 2022 at 8:19 AM Robert Haas wrote: > The choice of names in my example wasn't accidental. If the granted > role is a login role, then the superuser's intention was to vest the > privileges of that role in some other role, and it is surely not right > for that role to be able to decide that it doesn't want it's > privileges to be so granted. That's why I chose the name "peon". >> rhaas [as peon] => revoke peon from boss; -- i don't like being bossed around! Well, the peon is not getting bossed around, the boss is getting peoned around and the peon has decided that they like boss too much and don't need to do that anymore. When you grant a group "to" a role you place the role under the group - and inheritance flows downward. In the original thread Stephen wrote: "This is because we allow 'self administration' of roles, meaning that they can decide what other roles they are a member of.: The example, which you moved here, then attempts to demonstrate this "fact" but gets it wrong. Boss became a member of peon so if you want to demonstrate self-administration of a role's membership in a different group you have to login as boss, not peon. Doing that, and then revoking peon from boss, yields "ERROR: must have admin option on role "peon"". So no, without "WITH ADMIN OPTION" a role cannot decide what other roles they are a member of. I don't necessarily have an issue changing self-administration but if the motivating concern is that all these new pg_* roles we are creating are something a normal user can opt-out of/revoke that simply isn't the case today, unless they are added to the pg_* role WITH ADMIN OPTION. That all said, permissions SHOULD BE strictly additive. If boss doesn't want to be a member of pg_read_all_files allowing them to revoke themself from that role seems like it should be acceptable. If there is fear in allowing someone to revoke (not add) themselves as a member of a different role that suggests we have a design issue in another feature of the system. Today, they neither grant nor revoke, and the self-revocation doesn't seem that important to add. David J.
Re: role self-revocation
On Sun, Mar 6, 2022 at 9:53 AM Tom Lane wrote: > Robert Haas writes: > > ... Suppose the superuser grants "admin" to both "joe" and "sally". > > Now "joe" can SET ROLE to "admin" and revoke it from "sally", and the > > superuser has no tool to prevent this. > > Really? > > regression=# grant admin to joe; > GRANT ROLE > regression=# grant admin to sally; > GRANT ROLE > regression=# \c - joe > You are now connected to database "regression" as user "joe". > regression=> revoke admin from sally; > ERROR: must have admin option on role "admin" > regression=> set role admin; > SET > regression=> revoke admin from sally; > ERROR: must have admin option on role "admin" > > I think there is an issue here around exactly what the admin option > means, but if it doesn't grant you the ability to remove grants > made by other people, it's pretty hard to see what it's for. > > Precisely. The current system, with the session_user exception, basically guides a superuser to define two kinds of roles. Groups: No login, permission grants Users: Login, inherits permissions from groups, can manage group membership if given WITH ADMIN OPTION. The original example using only users is not all that compelling to me. IMO, DBAs should not be setting up their system that way. Two questions remain: 1. Are we willing to get rid of the session_user exception? 2. Do we want to track who the grantor is for role membership grants and institute a requirement that non-superusers can only revoke the grants that they personally made? I'm personally in favor of getting rid of the session_user exception, which nicely prevents the problem at the beginning of this thread and further encourages the DBA to define groups and roles with a greater separation-of-concerns design. WITH ADMIN OPTION is sufficient. I think tracking grantor information for role membership would allow for greater auditing capabilities and a better degree of control in the permissions system. In short, I am in favor of both options. The grantor tracking seems to be headed for acceptance. So, do we really want to treat every single login role as a potential group by keeping the session_user exception? David J.
Re: role self-revocation
Robert Haas writes: > ... Suppose the superuser grants "admin" to both "joe" and "sally". > Now "joe" can SET ROLE to "admin" and revoke it from "sally", and the > superuser has no tool to prevent this. Really? regression=# grant admin to joe; GRANT ROLE regression=# grant admin to sally; GRANT ROLE regression=# \c - joe You are now connected to database "regression" as user "joe". regression=> revoke admin from sally; ERROR: must have admin option on role "admin" regression=> set role admin; SET regression=> revoke admin from sally; ERROR: must have admin option on role "admin" I think there is an issue here around exactly what the admin option means, but if it doesn't grant you the ability to remove grants made by other people, it's pretty hard to see what it's for. regards, tom lane
Re: role self-revocation
On Sun, Mar 6, 2022 at 10:19 AM Robert Haas wrote: > > On Fri, Mar 4, 2022 at 5:20 PM David G. Johnston > wrote: > > I think I disagree. Or, at least, the superuser has full control of > > dictating how role membership is modified and that seems sufficient. > > The point is that the superuser DOES NOT have full control. The > superuser cannot prevent relatively low-privileged users from undoing > things that the superuser did intentionally and doesn't want reversed. > > The choice of names in my example wasn't accidental. If the granted > role is a login role, then the superuser's intention was to vest the > privileges of that role in some other role, and it is surely not right > for that role to be able to decide that it doesn't want it's > privileges to be so granted. That's why I chose the name "peon". In > your example, where you chose the name "admin", the situation is less > clear. If we imagine the granted role as a container for a bundle of > privileges, giving it the ability to administer itself feels more > reasonable. However, I am very much unconvinced that it's correct even > there. Suppose the superuser grants "admin" to both "joe" and "sally". > Now "joe" can SET ROLE to "admin" and revoke it from "sally", and the > superuser has no tool to prevent this. > > Now you can imagine a situation where the superuser is totally OK with > either "joe" or "sally" having the ability to lock the other one out, > but I don't think it's right to say that this will be true in all > cases. > Another example here is usage of groups in pg_hba.conf, if the admin has a group of users with stronger authentication requirements: e.g., hostssl all +certonlyusers all cert map=certmap clientcert=1 and one can remove their membership, they can change their authentication requirements.
Re: role self-revocation
Robert Haas writes: > On Fri, Mar 4, 2022 at 4:34 PM Tom Lane wrote: >> Agreed, this is not something to move on quickly. We might want >> to think about adjusting pg_dump to use explicit GRANTED BY >> options in GRANT/REVOKE a release or two before making incompatible >> changes. > Uggh. I really want to make some meaningful progress here before the > heat death of the universe, and I'm not sure that this manner of > proceeding is really going in that direction. That said, I do entirely > see your point. Are you thinking we'd actually add a GRANTED BY clause > to GRANT/REVOKE, vs. just wrapping it in SET ROLE incantations of some > sort? I was thinking the former ... however, after a bit of experimentation I see that we accept "grant foo to bar granted by baz" a VERY long way back, but the "granted by" option for object privileges is (a) pretty new and (b) apparently restrictively implemented: regression=# grant delete on alices_table to bob granted by alice; ERROR: grantor must be current user That's ... surprising. I guess whoever put that in was only interested in pro-forma SQL syntax compliance and not in making a usable feature. So if we decide to extend this change into object privileges it would be advisable to use SET ROLE, else we'd be giving up an awful lot of backwards compatibility in dump scripts. But if we're only talking about role grants then I think GRANTED BY would work fine. regards, tom lane
Re: role self-revocation
On Fri, Mar 4, 2022 at 4:34 PM Tom Lane wrote: > If we are not tracking the grantors of role authorizations, > then we are doing it wrong and we ought to fix that. Hmm, so maybe that's the place to start. We are tracking it in the sense that we record an OID in the catalog, but nothing that happens after that makes a lot of sense. > > 3. What happens if a user is dropped after being recorded as a > > grantor? > > Should work the same as it does now for ordinary ACLs, ie, you > gotta drop the grant first. OK, that makes sense to me. > Changing it could have some bad compatibility consequences though. > In particular, I believe it would break existing pg_dump files, > in that after restore all privileges would be attributed to the > restoring superuser, and there'd be no very easy way to clean that > up. I kind of wonder whether we ought to attribute all privileges granted by any superuser to the bootstrap superuser. That doesn't seem to have any meaningful downside, and it could avoid a lot of annoying dependencies that serve no real purpose. > Agreed, this is not something to move on quickly. We might want > to think about adjusting pg_dump to use explicit GRANTED BY > options in GRANT/REVOKE a release or two before making incompatible > changes. Uggh. I really want to make some meaningful progress here before the heat death of the universe, and I'm not sure that this manner of proceeding is really going in that direction. That said, I do entirely see your point. Are you thinking we'd actually add a GRANTED BY clause to GRANT/REVOKE, vs. just wrapping it in SET ROLE incantations of some sort? -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
On Fri, Mar 4, 2022 at 5:20 PM David G. Johnston wrote: > I think I disagree. Or, at least, the superuser has full control of > dictating how role membership is modified and that seems sufficient. The point is that the superuser DOES NOT have full control. The superuser cannot prevent relatively low-privileged users from undoing things that the superuser did intentionally and doesn't want reversed. The choice of names in my example wasn't accidental. If the granted role is a login role, then the superuser's intention was to vest the privileges of that role in some other role, and it is surely not right for that role to be able to decide that it doesn't want it's privileges to be so granted. That's why I chose the name "peon". In your example, where you chose the name "admin", the situation is less clear. If we imagine the granted role as a container for a bundle of privileges, giving it the ability to administer itself feels more reasonable. However, I am very much unconvinced that it's correct even there. Suppose the superuser grants "admin" to both "joe" and "sally". Now "joe" can SET ROLE to "admin" and revoke it from "sally", and the superuser has no tool to prevent this. Now you can imagine a situation where the superuser is totally OK with either "joe" or "sally" having the ability to lock the other one out, but I don't think it's right to say that this will be true in all cases. -- Robert Haas EDB: http://www.enterprisedb.com
Re: role self-revocation
On Fri, Mar 4, 2022 at 1:50 PM Robert Haas wrote: > On Mon, Feb 28, 2022 at 2:09 PM Stephen Frost wrote: > > The ability of a role to revoke itself from some other role is just > > something we need to accept as being a change that needs to be made, and > > I do believe that such a change is supported by the standard, in that a > > REVOKE will only work if you have the right to make it as the user who > > performed the GRANT in the first place. > > First, a quick overview of the > issue for those who have not followed the earlier threads in their > grueling entirety: > > rhaas=# create user boss; > CREATE ROLE > rhaas=# create user peon; > CREATE ROLE > rhaas=# grant peon to boss; > GRANT ROLE > rhaas=# \c - peon > You are now connected to database "rhaas" as user "peon". > rhaas=> revoke peon from boss; -- i don't like being bossed around! > REVOKE ROLE > > The wording for this example is hurting my brain. GRANT admin TO joe; \c admin REVOKE admin FROM joe; > I argue (and Stephen seems to agree) that the peon shouldn't be able > to undo the superuser's GRANT. I think I disagree. Or, at least, the superuser has full control of dictating how role membership is modified and that seems sufficient. The example above works because of: "A role is not considered to hold WITH ADMIN OPTION on itself, but it may grant or revoke membership in itself from a database session where the session user matches the role." If a superuser doesn't want "admin" to modify its own membership then they can prevent anyone but a superuser from being able to have a session_user of "admin". If that happens then the only way a non-superuser can modify group membership is by being added to said group WITH ADMIN OPTION. Now, if two people and a superuser are all doing membership management on the same group, and we want to add permission checks and multiple grants as tools, instead of having them just communicate with each other, then by all means let us do so. In that case, in answer to questions 2 and 3, we should indeed track which session_user made the grant and only allow the same session_user or the superuser to revoke it (we'd want to stop "ignoring" the GRANTED BY clause of REVOKE ROLE FROM so the superuser at least could remove grants made via WITH ADMIN OPTION). 4. Should we apply this rule to other types of grants, rather than > just to role membership? Why or why not? Consider this: > > The fact that the accountant may not be not in favor > of the auditor seeing what the accountant is doing with the money is > precisely the reason why we have auditors. [...] > However, when the superuser > performs the GRANT as in the above example, the grantor is recorded as > the table owner, not the superuser! So if we really want role > membersip and other kinds of grants to behave in the same way, we have > our work cut out for us here. > Yes, this particular choice seems unfortunate, but also not something that I think it is necessarily mandatory for us to improve. If the accountant is the owner then yes they get to decide permissions. In the presence of an auditor role either you trust the accountant role to keep the permissions in place or you define a superior authority to both the auditor and accountant to be the owner. Or let the superuser manage everything by witholding login and WITH ADMIN OPTION privileges from the ownership role. If we do extend role membership tracking I suppose the design question is whether the new role grantor dependency tracking will have a superuser be the recorded grantor instead of some owner. Given that roles don't presently have an owner concept, consistency with existing permissions in this manner would be trickier. Because of this, I would probably leave role grantor tracking at the session_user level while database objects continue to emanate from the object owner. The global vs database differences seem like a sufficient theoretical justification for the difference in implementation. David J.
Re: role self-revocation
Robert Haas writes: > 1. What should be the exact rule for whether A can remove a grant made > by B? Is it has_privs_of_role()? is_member_of_role()? Something else? No strong opinion here, but I'd lean slightly to the more restrictive option. > 2. What happens if the same GRANT is enacted by multiple users? For > example, suppose peon does "GRANT peon to boss" and then the superuser > does the same thing afterwards, or vice versa? One design would be to > try to track those as two separate grants, but I'm not sure if we want > to add that much complexity, since that's not how we do it now and it > would, for example, implicate the choice of PK on the pg_auth_members > table. As you note later, we *do* track such grants separately in ordinary ACLs, and I believe this is clearly required by the SQL spec. It says (for privileges on objects): Each privilege is represented by a privilege descriptor. A privilege descriptor contains: — The identification of the object on which the privilege is granted. — The of the grantor of the privilege. ^^^ — The of the grantee of the privilege. — Identification of the action that the privilege allows. — An indication of whether or not the privilege is grantable. — An indication of whether or not the privilege has the WITH HIERARCHY OPTION specified. Further down (4.42.3 in SQL:2021), the granting of roles is described, and that says: Each role authorization is described by a role authorization descriptor. A role authorization descriptor includes: — The role name of the role. — The authorization identifier of the grantor. — The authorization identifier of the grantee. — An indication of whether or not the role authorization is grantable. If we are not tracking the grantors of role authorizations, then we are doing it wrong and we ought to fix that. > 3. What happens if a user is dropped after being recorded as a > grantor? Should work the same as it does now for ordinary ACLs, ie, you gotta drop the grant first. > 4. Should we apply this rule to other types of grants, rather than > just to role membership? I am not sure about the reasoning behind the existing rule that superuser-granted privileges are recorded as being granted by the object owner. It does feel more like a wart than something we want. It might have been a hack to deal with the lack of GRANTED BY options in GRANT/REVOKE back in the day. Changing it could have some bad compatibility consequences though. In particular, I believe it would break existing pg_dump files, in that after restore all privileges would be attributed to the restoring superuser, and there'd be no very easy way to clean that up. > Please note that it is not really my intention to try to shove > anything into v15 here. Agreed, this is not something to move on quickly. We might want to think about adjusting pg_dump to use explicit GRANTED BY options in GRANT/REVOKE a release or two before making incompatible changes. regards, tom lane
