Thread: Re: [PATCHES] Roles - SET ROLE Updated
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > * Tom Lane (tgl@sss.pgh.pa.us) wrote: > >> After rereading SQL99 4.31, I don't think there is any need to > >> distinguish CURRENT_USER from CURRENT_ROLE, mainly because our > >> implementation does not distinguish users from roles at all. > > > CURRENT_USER and CURRENT_ROLE can have different values, as I understand > > SQL2003, and there are places where one is used instead of the other > > It's possible for CURRENT_ROLE to be null according to the spec; if you > like we could implement that as returning what the current outer-level > SET ROLE value is (which would then make it semantically more like > SESSION_USER than CURRENT_USER). I don't think CURRENT_USER should ever > be allowed to be null, or to be different from the active authorization > identifier, first because it's silly and second because it will break > existing applications that depend on CURRENT_USER for authorization > checking. Sorry about the existing applications, but this does go directly against the SQL2003 specification. At least from my reading of SQL2003 5.37 ROLE_COLUMN_GRANTS view, which 'Identifies the privileges on columns defined in this catalog that are available to or granted by the currently enabled roles': WHERE ( GRANTEE IN ( SELECT ROLE_NAME FROM ENABLED_ROLES ) Where the ENABLED_ROLES view operates specifically off of the 'CURRENT_ROLE' value. > Given that we don't really distinguish users and roles, I would be > inclined to make the same argument for CURRENT_ROLE too, leaving > SHOW ROLE (and its function equivalent) as the only way to see what > you SET ROLE to. But it's less likely to break existing apps if we > don't. I don't quite follow this- the point of SET ROLE is to change your authorization identifier to be a specific role instead of the current role. What I had thought you were suggesting was to make it so that after a SET ROLE the CURRENT_USER shows what you SET ROLE to. This sounds like SET ROLE is just there for looks and completely ignored for authorization purposes, making it next to useless. > > (such as with the 'grantor' in grants, according to SQL2003 the > > 'grantor' should be the CURRENT_USER, regardless of if CURRENT_ROLE is > > set or not). > > Exactly. CURRENT_USER has to be the active authorization identifier. No, that's an exception, and only for what ends up in the table recorded as the 'grantor'. Re-reading 4.34 it's apparently actually supposed to be a "last-in, first-out" mechanism, though I don't see any way for a user (beyond a connect statement) to actually change CURRENT_USER, unlike SET ROLE which can be used to change CURRENT_ROLE (and in so doing put it at the top of the 'stack'). Technically I believe this actually allows multiple levels of 'SET ROLE's to be done and for 'SET ROLE NONE's to only pull off the top-level. My patch didn't handle such multi-level SET ROLE's, but it's certainly something which could be done. Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > * Tom Lane (tgl@sss.pgh.pa.us) wrote: >> It's possible for CURRENT_ROLE to be null according to the spec; if you >> like we could implement that as returning what the current outer-level >> SET ROLE value is (which would then make it semantically more like >> SESSION_USER than CURRENT_USER). I don't think CURRENT_USER should ever >> be allowed to be null, or to be different from the active authorization >> identifier, first because it's silly and second because it will break >> existing applications that depend on CURRENT_USER for authorization >> checking. > Sorry about the existing applications, but this does go directly against > the SQL2003 specification. The spec isn't sufficiently well-designed in this area to make me willing to insert security holes into existing apps in order to follow it slavishly. They clearly failed to think through the grant-role-to-PUBLIC business, and the whole distinction between users and roles is pretty artificial anyway. > At least from my reading of SQL2003 5.37 > ROLE_COLUMN_GRANTS view, which 'Identifies the privileges on columns > defined in this catalog that are available to or granted by the > currently enabled roles': > WHERE ( GRANTEE IN ( SELECT ROLE_NAME FROM ENABLED_ROLES ) > Where the ENABLED_ROLES view operates specifically off of the > 'CURRENT_ROLE' value. OK, so we make CURRENT_ROLE return the SET ROLE value (possibly NULL). I notice that the privilege-related info schema views consistently check privileges via locutions like WHERE ( SCHEMA_OWNER = CURRENT_USER OR SCHEMA_OWNER IN ( SELECT ROLE_NAME FROM ENABLED_ROLES ) ) which is a tad odd if it's intended to model the privileges you currently have; the implication of that is that you cannot drop any of your "login ID"'s privileges by doing SET ROLE, which surely is not the intended behavior (else you might as well not have SET ROLE at all; the only possible use of SET ROLE is to *restrict* your privileges, since any role you can become represents privileges you'd have anyway without SET ROLE). So I'm pretty unconvinced that the spec is being self-consistent here. > Technically I believe this > actually allows multiple levels of 'SET ROLE's to be done and for 'SET > ROLE NONE's to only pull off the top-level. I don't see anything in the spec that suggests that reading to me. regards, tom lane
I wrote: > ... the implication of that is that you cannot drop any of > your "login ID"'s privileges by doing SET ROLE, which surely is not > the intended behavior (else you might as well not have SET ROLE at all; > the only possible use of SET ROLE is to *restrict* your privileges, > since any role you can become represents privileges you'd have anyway > without SET ROLE). So I'm pretty unconvinced that the spec is being > self-consistent here. After some further study I see where the disconnect is coming from: what we've implemented isn't quite what the spec has in mind. Look at SQL99 4.31.4: 4.31.4 Security model definitions The set of applicable roles for an <authorization identifier> consists of all roles defined by the role authorizationdescriptors whose grantee is that <authorization identifier> or PUBLIC together with all otherroles they contain. The set of user privileges for a <user identifier> consists of all privileges defined by the privilege descriptorswhose grantee is either that <user identifier> or PUBLIC. The set of role privileges for a <role name> consists of all privileges defined by the privilege descriptorswhose grantee is either that <role name>, PUBLIC, or one of the applicable roles of that <role name>. What this says is that when a role A is a member of another role B, A automatically has all of B's privileges. But when a user U is a member of role R, U does *not* have R's privileges automatically. What he has is the right to do SET ROLE R, after which he has R's privileges in addition to his own (see the rest of 4.31.4). This is ... um ... a pretty bizarre way of looking at security. U can in fact do whatever his roles allow him to do, he just needs to say "Mother may I?" first. I suppose the fact that the spec only allows SET ROLE at the outer level (outside any transaction) provides some veneer of security against Trojan-horse functions, but it sure looks lame. But anyway, it seems that the spec sees SET ROLE as an operation that gets you additional privileges, not as an operation that restricts your privileges. I don't think we can possibly emulate this definition unless we make some pretty fundamental changes in the way the ROLE patch works. In particular, is_member_of_role isn't in general the right way to check applicability of privileges. regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > Sorry about the existing applications, but this does go directly against > > the SQL2003 specification. > > The spec isn't sufficiently well-designed in this area to make me > willing to insert security holes into existing apps in order to follow > it slavishly. They clearly failed to think through the > grant-role-to-PUBLIC business, and the whole distinction between users > and roles is pretty artificial anyway. Perhaps the specification isn't but I'm pretty sure other implementations follow the SET ROLE -> current authorization identifier (and thus dropping other rights granted to the CURRENT_USER). Having thought about this a bit more I'd like to know what security holes you're thinking would be introduced by this change. CURRENT_USER was always required to be set in my original patch, and SET ROLE didn't exist before and only ever dropped privileges anyway. A current app is rather unlikely I'd think to use SET ROLE and *then* base authorization decisions off the value of CURRENT_USER... I suppose I'm being dense but I'd like to get a better explanation of the specific problem before trying to come up with an acceptable solution. > > At least from my reading of SQL2003 5.37 > > ROLE_COLUMN_GRANTS view, which 'Identifies the privileges on columns > > defined in this catalog that are available to or granted by the > > currently enabled roles': > > > WHERE ( GRANTEE IN ( SELECT ROLE_NAME FROM ENABLED_ROLES ) > > > Where the ENABLED_ROLES view operates specifically off of the > > 'CURRENT_ROLE' value. > > OK, so we make CURRENT_ROLE return the SET ROLE value (possibly NULL). > > I notice that the privilege-related info schema views consistently check > privileges via locutions like > > WHERE ( SCHEMA_OWNER = CURRENT_USER > OR > SCHEMA_OWNER IN > ( SELECT ROLE_NAME > FROM ENABLED_ROLES ) ) > > which is a tad odd if it's intended to model the privileges you > currently have; the implication of that is that you cannot drop any of > your "login ID"'s privileges by doing SET ROLE, which surely is not > the intended behavior (else you might as well not have SET ROLE at all; > the only possible use of SET ROLE is to *restrict* your privileges, > since any role you can become represents privileges you'd have anyway > without SET ROLE). So I'm pretty unconvinced that the spec is being > self-consistent here. Looking back on it I'd have to agree that there does seem something a bit odd here. There are some places where it's limited to the current role (the ROLE_*_GRANTS that I had originally been looking at) but other places indicate cases where the 'user' is the 'owner', or is in the role of the 'owner'. The grantee cases tend to have 'public', CURRENT_USER or an enabled_role. Interestingly, there *is* a distinction that's made here, when you think about it: This lists things which the CURRENT_USER or the ENABLED_ROLES (via a SET ROLE) has access to. This does *not* list objects in the APPLICABLE_ROLES set. This indicates that SET ROLE *does* drop privileges, but you may still see objects which the underlying user can directly, but not things which the underlying user can see indirectly through other roles (unless those other roles are available under ENABLED_ROLES). The odd bit is that this doesn't seem to handle the case where CURRENT_ROLE is NULL very cleanly- if you've not SET ROLE then it's expected you have access to anything which a role you've been granted has access to, instead you only see those things which you directly own or which are available to 'public'. I recall you telling me to go back and look at the spec at one point regarding what a given user could see via information_schema and to submit a patch if something in information_schema was wrong. Well, seems like perhaps information_schema might have been following the spec (since this isn't what I would have expected). > > Technically I believe this > > actually allows multiple levels of 'SET ROLE's to be done and for 'SET > > ROLE NONE's to only pull off the top-level. > > I don't see anything in the spec that suggests that reading to me. It's in 4.34.1.1, at least in the SQL2003 specification, and it reads: "This stack is maintained using a "last-in, first-out" discipline, and effectively only the top cell is visible. When an SQL-session is started, by explicit or implicit execution of a <connect statement>, the authorization stack is initialized with one cell, which contains only the user identifier known as the SQL-session user identifier, a role name, known as the SQL-session role name may be added subsequently." It also says: "The <set session user identifier statement> changes the value of the current user identifier and of the SQL session user identifier. The <set role statement> changes the value of the current role name." Which does seem to conflict. Were it meaning that SET ROLE pushes onto the stack I'd expect the wording to reflect that instead of saying "chagnes". This stack-like behaviour of multiple set-role statements isn't something I can currently think I'd have any need for, but it does more closely follow how 'su's in Unix work. Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > Perhaps the specification isn't but I'm pretty sure other > implementations follow the SET ROLE -> current authorization > identifier (and thus dropping other rights granted to the CURRENT_USER). My current reading of 4.31 is that SET ROLE *doesn't* drop rights, which means we need to rethink all of this. However, on this point: >>> Technically I believe this >>> actually allows multiple levels of 'SET ROLE's to be done and for 'SET >>> ROLE NONE's to only pull off the top-level. >> >> I don't see anything in the spec that suggests that reading to me. > It's in 4.34.1.1, at least in the SQL2003 specification, and it reads: > "This stack is maintained using a "last-in, first-out" discipline, and > effectively only the top cell is visible. Yes, but the only events that push or pop stack entries are entry/exit of an external procedure (think SECURITY DEFINER procedure). SET ROLE doesn't push or pop anything, it just alters the current top entry. (Which must in fact be the *only* entry, given that SET ROLE is only allowed at outer level...) regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > What this says is that when a role A is a member of another role B, A > automatically has all of B's privileges. But when a user U is a member > of role R, U does *not* have R's privileges automatically. What he has > is the right to do SET ROLE R, after which he has R's privileges in > addition to his own (see the rest of 4.31.4). Indeed, when I was looking through the information_schema views more closely I was starting to realize something like this was going on. > This is ... um ... a pretty bizarre way of looking at security. > U can in fact do whatever his roles allow him to do, he just needs to > say "Mother may I?" first. I suppose the fact that the spec only allows > SET ROLE at the outer level (outside any transaction) provides some > veneer of security against Trojan-horse functions, but it sure looks > lame. > > But anyway, it seems that the spec sees SET ROLE as an operation that > gets you additional privileges, not as an operation that restricts your > privileges. Yeah, myself, and at least one other person that I recall asking after this stuff, felt it was the opposite. > I don't think we can possibly emulate this definition unless we make > some pretty fundamental changes in the way the ROLE patch works. > In particular, is_member_of_role isn't in general the right way to > check applicability of privileges. It is, and it isn't... It's correct for checking role-privileges, just not for user-privileges. That is to say, is_member_of_role works for when CURRENT_ROLE is set, and should be started based off of whatever CURRENT_ROLE is set to. If CURRENT_ROLE is not set then I don't think it can be used. Thanks, Stephen
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > Perhaps the specification isn't but I'm pretty sure other > > implementations follow the SET ROLE -> current authorization > > identifier (and thus dropping other rights granted to the CURRENT_USER). > > My current reading of 4.31 is that SET ROLE *doesn't* drop rights, which > means we need to rethink all of this. However, on this point: Yeah, it seems to add them. Honestly, my recollection from working on Oracle is that you have access to all the roles you've been granted when you connect as a user. If it'd be useful I'd be happy to see about finding out exactly what Oracle does in this regard and if it actually follows the specification or what. > > It's in 4.34.1.1, at least in the SQL2003 specification, and it reads: > > "This stack is maintained using a "last-in, first-out" discipline, and > > effectively only the top cell is visible. > > Yes, but the only events that push or pop stack entries are entry/exit > of an external procedure (think SECURITY DEFINER procedure). SET ROLE > doesn't push or pop anything, it just alters the current top entry. > (Which must in fact be the *only* entry, given that SET ROLE is only > allowed at outer level...) Alright, that would make the later language make more sense. Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > * Tom Lane (tgl@sss.pgh.pa.us) wrote: >> My current reading of 4.31 is that SET ROLE *doesn't* drop rights, which >> means we need to rethink all of this. However, on this point: > Yeah, it seems to add them. Honestly, my recollection from working on > Oracle is that you have access to all the roles you've been granted when > you connect as a user. If it'd be useful I'd be happy to see about > finding out exactly what Oracle does in this regard and if it actually > follows the specification or what. Yeah, let's take a look. This wouldn't be the first part of the spec we've come across that is mostly honored in the breach... regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > Perhaps the specification isn't but I'm pretty sure other > > implementations follow the SET ROLE -> current authorization > > identifier (and thus dropping other rights granted to the CURRENT_USER). > > My current reading of 4.31 is that SET ROLE *doesn't* drop rights, which > means we need to rethink all of this. However, on this point: Reviewing: http://www.psoug.org/reference/roles.html (Top link in Google - Oracle Roles): Oracle allows a 'SET ROLE all;' syntax, which is essentially what we're currently automatically doing. You can't deactivate a specific role, but you can deactivate all roles using 'SET ROLE none;'. Interestingly, on at least one Oracle setup it appears that it also has an implicit 'SET ROLE all;'. Check this out: ----------------------------------------------------------------- melkor> sqlplus SQL> select * from session_roles; ROLE ------------------------------ CONNECT NORMAL SQL> SET ROLE none; Role set. SQL> select * from session_roles; no rows selected SQL> ----------------------------------------------------------------- Doing this doesn't seem entirely unreasonable but we don't currently have a way of handling 'SET ROLE none;'. We'd need to make some changes but I think we could handle it, and correctly handle a specific 'SET ROLE <role>', which under Oracle does appear to drop any other roles you currently have. Thanks, Stephen
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Yeah, let's take a look. This wouldn't be the first part of the spec > we've come across that is mostly honored in the breach... Oracle appears to mostly follow it, except there's an implicit 'SET ROLE all;', at least in the instance I'm looking at. I'm guessing it's probably something which is tunable. We don't currently have a way of having certain roles turned on and certain ones turned off for a given session. That doesn't seem like it'd be a very hard thing to add though. I have to apologize for not realizing this sooner and implementing it correctly the first time, sorry about that.. Thanks, Stephen
* Stephen Frost (sfrost@snowman.net) wrote: > Doing this doesn't seem entirely unreasonable but we don't currently > have a way of handling 'SET ROLE none;'. We'd need to make some changes > but I think we could handle it, and correctly handle a specific > 'SET ROLE <role>', which under Oracle does appear to drop any other > roles you currently have. Thinking about this a bit more.. Basically what we have is: An implicit 'SET ROLE all;' on session connect, like Oracle does. Support from the patch for an explicit 'SET ROLE <role>;', which drops privileges for all other roles except the role set. The only change to correctly support that would be to add 'CURRENT_USER' back into the resulting set of 'enabled_roles' (but not doing so recursively or we're back to 'SET ROLE all;'). You don't appear to be able to drop rights which you have via CURRENT_USER. To support having certain roles turned on and certain roles turned off would be some additional effort. I think we'd need a list of 'ENABLED_ROLES' and then correct recursion based off of that list instead of just starting from a single point like we do now. Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > To support having certain roles turned on and certain roles turned off > would be some additional effort. I think we'd need a list of > 'ENABLED_ROLES' and then correct recursion based off of that list > instead of just starting from a single point like we do now. That seems fairly ugly and messy though, because it wouldn't readily translate to the case of SECURITY DEFINER procedures. Let's review the bidding. As I presently understand it: 1. At session start, you are the SESSION_USER, which is also CURRENT_USER, and (per spec anyway) CURRENT_ROLE should beNULL. You have only the privileges granted directly to SESSION_USER or PUBLIC. 2. You are allowed to SET ROLE to any role that the SESSION_USER is (directly or indirectly) a member of. After you doso, the spec says that CURRENT_USER is still SESSION_USER, CURRENT_ROLE is now the selected role, and you have the unionof the rights of those two authorization identifiers. 3. If you enter a module (call a SECURITY DEFINER procedure), then the current authorization identifier becomes that ofthe owner of the module/procedure. Per spec this identifier is either a user or a role, not both, and either CURRENT_USERor CURRENT_ROLE reflects that with the other becoming NULL. (The authorization stack will pop back to theprior state when you exit the call.) Note: this is the case that allows CURRENT_USER to become null. 4. I'm not totally clear about whether the spec allows a module to be entered without starting a transaction. If so, itwould be legal for a module owned by a user (not a role) to SET ROLE to one of the roles that user is a member of. Againthis would have effect only till module exit. Now #4 doesn't currently apply to us, since we have neither modules nor procedures that can be entered outside a transaction. And frankly I don't see the point in it anyway --- you may as well let the module or procedure be owned by the desired role in the first place. ISTM most of the messiness here comes from the spec's hard-and-fast distinction between users and roles, which they then have to blur again in order to talk about "authorization identifiers". Our current implementation treats these as a single kind of entity with some optional attributes (eg rolcanlogin), and I think that that's a much cleaner way of doing it. What I would like to do is say that there is exactly one active authorization identifier at any instant. That means that if you SET ROLE then you no longer have the privileges attached to SESSION_USER (except the right to SET ROLE to a different one of his roles). This is contrary to spec but it seems to be a pretty widely accepted way of doing things; and it makes SET ROLE effectively equivalent to the change in privileges associated with entering a SECURITY DEFINER procedure owned by that role, so there's a much cleaner conceptual model. Furthermore, I still feel that both CURRENT_USER and CURRENT_ROLE should reflect this single active authorization id. The spec's distinction is artificial and doesn't gain anything, except the ability to break code that assumes CURRENT_USER is always meaningful. If we don't do it that way then we have a bunch of API that breaks down: all of the has_foo_privilege functions stop working, because they don't have a signature that allows both a user and a role to be passed to them. I think we do need to invent the concept of roles (pg_authid entries) that either do or do not inherit privileges from roles they are members of. A non-inheriting role can do SET ROLE to gain the privileges of a role it is a member of, but it does not have those privileges without SET ROLE. We could drive this off rolcanlogin but it's probably better to invent an additional flag column in pg_authid (call it rolinherit, maybe). Users by default would not inherit, which puts us a great deal closer to the spec's semantics. Thoughts? regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > To support having certain roles turned on and certain roles turned off > > would be some additional effort. I think we'd need a list of > > 'ENABLED_ROLES' and then correct recursion based off of that list > > instead of just starting from a single point like we do now. > > That seems fairly ugly and messy though, because it wouldn't readily > translate to the case of SECURITY DEFINER procedures. SECURITY DEFINER is just another level in the stack, one which is above all others. At least, in my thinking anyway. We don't actually need a stack, just need to save off the old user/role list and reinstate it after the SECURITY DEFINER function. > Let's review the bidding. As I presently understand it: [...] That all looks right. > What I would like to do is say that there is exactly one active > authorization identifier at any instant. That means that if you SET > ROLE then you no longer have the privileges attached to SESSION_USER > (except the right to SET ROLE to a different one of his roles). > This is contrary to spec but it seems to be a pretty widely accepted > way of doing things; and it makes SET ROLE effectively equivalent to > the change in privileges associated with entering a SECURITY DEFINER > procedure owned by that role, so there's a much cleaner conceptual > model. I'd rather get away from the idea of only one 'active authorization identifier'. That's rather limiting and I don't really see the point. All that's changing is what seeds the initial list of roles prior to doing the full hierarchical resolution to populate the full list of roles. The original patch used CURRENT_USER or CURRENT_ROLE if it was set. This just adds CURRENT_USER back in at the end if we used CURRENT_ROLE initially. Except in a 'SET ROLE all' case, when we seed with CURRENT_USER, as I had originally. > Furthermore, I still feel that both CURRENT_USER and CURRENT_ROLE should > reflect this single active authorization id. The spec's distinction is > artificial and doesn't gain anything, except the ability to break code > that assumes CURRENT_USER is always meaningful. There is a distinction there though, at least as the SQL spec works, and really there is when you consider that CURRENT_USER is really the SESSION_USER generally. > If we don't do it that way then we have a bunch of API that breaks down: > all of the has_foo_privilege functions stop working, because they don't > have a signature that allows both a user and a role to be passed to > them. It seems like there might be a way to solve this better but I'm not coming up with it yet. > I think we do need to invent the concept of roles (pg_authid entries) > that either do or do not inherit privileges from roles they are members > of. A non-inheriting role can do SET ROLE to gain the privileges of a > role it is a member of, but it does not have those privileges without > SET ROLE. We could drive this off rolcanlogin but it's probably better > to invent an additional flag column in pg_authid (call it rolinherit, > maybe). Users by default would not inherit, which puts us a great deal > closer to the spec's semantics. I really don't care for this idea as an alternative to what the spec calls for. I don't think it puts us much closer to the spec's semantics unless you let a user change pg_authid wrt this and that seems like quite a bad idea. > Thoughts? Sorry if it's not entirely coherent, I've been thinking about this alot over the past couple of hours. I'm going to sleep on it and hopefully write up something better tommorow. Basically my thinking is that in general the 'list of roles current enabled' is what we've currently got already and that works. We're just changing how and what gets added/removed from it. Thanks, Stephen
Am Donnerstag, 21. Juli 2005 22:55 schrieb Tom Lane: > What this says is that when a role A is a member of another role B, A > automatically has all of B's privileges. But when a user U is a member > of role R, U does *not* have R's privileges automatically. What he has > is the right to do SET ROLE R, after which he has R's privileges in > addition to his own (see the rest of 4.31.4). > > This is ... um ... a pretty bizarre way of looking at security. > U can in fact do whatever his roles allow him to do, he just needs to > say "Mother may I?" first. In some circles, this is considered the standard behavior of role security models. (There is a NIST standard somewhere.) It allows (with additional work) dynamic separation of concerns, namely that you could be a member of roles A and B but cannot use both at the same time. This is admittedly a fairly advanced feature, but should nevertheless be kept in mind. -- Peter Eisentraut http://developer.postgresql.org/~petere/
* Peter Eisentraut (peter_e@gmx.net) wrote: > Am Donnerstag, 21. Juli 2005 22:55 schrieb Tom Lane: > > What this says is that when a role A is a member of another role B, A > > automatically has all of B's privileges. But when a user U is a member > > of role R, U does *not* have R's privileges automatically. What he has > > is the right to do SET ROLE R, after which he has R's privileges in > > addition to his own (see the rest of 4.31.4). > > > > This is ... um ... a pretty bizarre way of looking at security. > > U can in fact do whatever his roles allow him to do, he just needs to > > say "Mother may I?" first. > > In some circles, this is considered the standard behavior of role security > models. (There is a NIST standard somewhere.) It allows (with additional > work) dynamic separation of concerns, namely that you could be a member of > roles A and B but cannot use both at the same time. This is admittedly a > fairly advanced feature, but should nevertheless be kept in mind. It sounds like this is essentially if 'SET ROLE all;' is allowed or not. If you disallow 'SET ROLE all;' (and therefore not do it on session start) then you would get this behaviour. I certainly see that as a reasonable option though I think we'd want to allow 'SET ROLE all;' for backwards compatibility to group-based systems. This doesn't change that even after a SET ROLE <role>; you would have the permissions available via CURRENT_USER. Thinking about it a bit more, in our context a 'SET ROLE all;' is really a 'SET ROLE <login-role-name>;', which I'd think would therefore mean that upon login in a 'default' setup we'd have CURRENT_USER and CURRENT_ROLE set to the same thing. My patch would need to be modified to add CURRENT_USER to the role-cache when CURRENT_USER != CURRENT_ROLE, *after* the rest of the resolution, of course. We'd then need an option for if the 'SET ROLE all;' is done on connect or not, perhaps seperately an option saying if it's allowed at all, and syntax modifications to all for 'SET ROLE all;', etc. Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > It sounds like this is essentially if 'SET ROLE all;' is allowed or not. > If you disallow 'SET ROLE all;' (and therefore not do it on session > start) then you would get this behaviour. I certainly see that as a > reasonable option though I think we'd want to allow 'SET ROLE all;' for > backwards compatibility to group-based systems. 'SET ROLE all' is nonstandard; it will complicate the implementation a great deal; and it creates a problem with the permissions environment of a SECURITY DEFINER function being different from that seen at the outer level by the same user. I think a better answer is to have a "rolinherit" flag in pg_authid, which people can set "off" for spec compatibility or "on" for backwards compatibility to the GROUP feature. In either setting, the permissions given to a particular authid are clear from pg_authid and don't vary depending on magic SET variables. regards, tom lane
* Stephen Frost (sfrost@snowman.net) wrote: > * Tom Lane (tgl@sss.pgh.pa.us) wrote: > > If we don't do it that way then we have a bunch of API that breaks down: > > all of the has_foo_privilege functions stop working, because they don't > > have a signature that allows both a user and a role to be passed to > > them. > > It seems like there might be a way to solve this better but I'm not > coming up with it yet. Alright, I've thought about this some more. I think there's two ways to deal with this. One is the solution you proposed below where we have per-role 'rolinherit' and answers are based off of that. The other is to just base it off of the question of if 'SET ROLE all;' is allowed or not. If not then we basically use 'rolcanlogin' as a proxy for 'rolinherit'. Otherwise the logic stays the same as it is currently. I like your solution more but I see it as something to do to go above what the spec calls for, not as a replacement for doing what the spec requires. I *think* with your solution we wouldn't need the explicit 'SET ROLE all;'-allowed option, as that would just correspond to setting rolinherit = !rolcanlogin;. > > I think we do need to invent the concept of roles (pg_authid entries) > > that either do or do not inherit privileges from roles they are members > > of. A non-inheriting role can do SET ROLE to gain the privileges of a > > role it is a member of, but it does not have those privileges without > > SET ROLE. We could drive this off rolcanlogin but it's probably better > > to invent an additional flag column in pg_authid (call it rolinherit, > > maybe). Users by default would not inherit, which puts us a great deal > > closer to the spec's semantics. This sounds like a good addition, though to retain backwards compatibility to group-based systems I'd think we'd either have to have rolinherit on by default or cause pg_dump to turn it on when coming from group-based systems (the latter would probably be better, if it's possible). > Sorry if it's not entirely coherent, I've been thinking about this alot > over the past couple of hours. I'm going to sleep on it and hopefully > write up something better tommorow. Basically my thinking is that in > general the 'list of roles current enabled' is what we've currently got > already and that works. We're just changing how and what gets > added/removed from it. In the end I think we can have just one top-level to work from, we just have to add CURRENT_USER to the list of roles after the resolution if it's not in it already and it wasn't used as the base. Otherwise I think what I had pretty much works except that we do 'SET ROLE all;' initially, which just sets CURRENT_ROLE = CURRENT_USER and does the role-cache generation off that. When a SET ROLE is done we change CURRENT_ROLE to the new role and regenerate the role-cache based off the CURRENT_ROLE and add CURRENT_USER to the list at the end. This does mean that SET ROLE can't work off the cache, but that doesn't seem all that terrible to me. Thanks, Stephen
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > It sounds like this is essentially if 'SET ROLE all;' is allowed or not. > > If you disallow 'SET ROLE all;' (and therefore not do it on session > > start) then you would get this behaviour. I certainly see that as a > > reasonable option though I think we'd want to allow 'SET ROLE all;' for > > backwards compatibility to group-based systems. > > 'SET ROLE all' is nonstandard; it will complicate the implementation a > great deal; and it creates a problem with the permissions environment of > a SECURITY DEFINER function being different from that seen at the outer > level by the same user. 'SET ROLE all' is nonstandard but done in practice. > I think a better answer is to have a "rolinherit" flag in pg_authid, > which people can set "off" for spec compatibility or "on" for backwards > compatibility to the GROUP feature. In either setting, the permissions > given to a particular authid are clear from pg_authid and don't vary > depending on magic SET variables. This is nonstandard and not done in practice. Authorization changes being allowed by 'SET ROLE' is what the spec calls for. Not supporting that ability would be unfortunate and it seems there'd be no point to having 'SET ROLE' at all. Stephen
[ getting back to this thread... ] Stephen Frost <sfrost@snowman.net> writes: > * Tom Lane (tgl@sss.pgh.pa.us) wrote: >> I think a better answer is to have a "rolinherit" flag in pg_authid, >> which people can set "off" for spec compatibility or "on" for backwards >> compatibility to the GROUP feature. In either setting, the permissions >> given to a particular authid are clear from pg_authid and don't vary >> depending on magic SET variables. > This is nonstandard and not done in practice. Authorization changes > being allowed by 'SET ROLE' is what the spec calls for. Not supporting > that ability would be unfortunate and it seems there'd be no point to > having 'SET ROLE' at all. I think maybe you misunderstood what I was suggesting. The function of the flag as I imagine it is: * rolinherit = false: role does not automatically have the privileges of roles it is a member of. It must do SET ROLE to gain the privileges of a role it is a member of. (This emulates the spec behavior for users.) * rolinherit = true: role has the privileges of all roles it is a member of, without needing to do SET ROLE. (This handles the spec behavior for roles, and is also needed for users when backwards compatibility with our old behavior for groups is wanted, and also provides an approximate equivalent to Oracle's SET ROLE ALL.) If users have rolinherit = false and roles have rolinherit = true, everything behaves per spec, except that I don't want to support the aspect of the spec that says you can SET ROLE at the outer level and still have the privileges of the SESSION_USER. I think SET ROLE should effectively drop the SESSION_USER's privileges (except that subsequent SET ROLE commands will be checked against the SESSION_USER's role memberships, not the current effective role). If both users and roles have rolinherit = true, we have a good emulation of the old group-based behavior. For backwards compatibility we probably have to have CREATE USER defaulting to rolinherit = true. Is it sufficient to say "if you want the spec-compatible behavior you always have to say CREATE USER ... NOINHERIT"? Since the spec doesn't actually define a CREATE USER command, this is not a spec violation in a technical sense. But people who are migrating towards using SET ROLE might wish it defaulted to NOINHERIT. We could (either now or in a future release) add a GUC variable to control the default, I suppose. regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > [ getting back to this thread... ] Happy to, was getting worried you'd forgotten or ignored it. ;) > * rolinherit = false: role does not automatically have the privileges of > roles it is a member of. It must do SET ROLE to gain the privileges of > a role it is a member of. (This emulates the spec behavior for users.) > > * rolinherit = true: role has the privileges of all roles it is a member > of, without needing to do SET ROLE. (This handles the spec behavior for > roles, and is also needed for users when backwards compatibility with our > old behavior for groups is wanted, and also provides an approximate > equivalent to Oracle's SET ROLE ALL.) > > If users have rolinherit = false and roles have rolinherit = true, > everything behaves per spec, except that I don't want to support the > aspect of the spec that says you can SET ROLE at the outer level and > still have the privileges of the SESSION_USER. I think SET ROLE should > effectively drop the SESSION_USER's privileges (except that subsequent > SET ROLE commands will be checked against the SESSION_USER's role > memberships, not the current effective role). I don't particularly like deviating from the spec in this regard (since I don't think it'd be all that hard to implement what the spec calls for), but it doesn't bother me that much. > If both users and roles have rolinherit = true, we have a good emulation > of the old group-based behavior. For backwards compatibility we > probably have to have CREATE USER defaulting to rolinherit = true. While I agree that this is what Oracle's SET ROLE ALL does initially, it's possible for a user to 'SET ROLE <a>' and drop the permissions given by the other roles in which the user is in. Will that still be possible with your proposed solution, or will doing 'SET ROLE <a>' have no effect when 'rolinherit = true'? That's really my main concern. For my systems I expect to want to do 'rolinherit = true' generally but I really don't like the idea that 'SET ROLE <a>' has no effect then. Thinking about it a bit more I suppose I could live with it since it's per-role and I tend to set up unprivileged accounts, which is where I'd really be more concerned about 'SET ROLE <a>' working. We should probably issue a warning or something if my hypothosis on 'SET ROLE' above is correct in the 'rolinherit = true' case so that people don't get the wrong idea that they've dropped privileges in cases when they actually havn't. > Is it sufficient to say "if you want the spec-compatible behavior you > always have to say CREATE USER ... NOINHERIT"? Since the spec doesn't > actually define a CREATE USER command, this is not a spec violation in a > technical sense. But people who are migrating towards using SET ROLE > might wish it defaulted to NOINHERIT. We could (either now or in a > future release) add a GUC variable to control the default, I suppose. Being able to control the default would be nice but I don't believe it would be a requirement. I would actually like to have a variable to control if SESSION_USER privileges are kept across a SET ROLE or not, though primairly to conform to the spec than expectation that I'd personally use it much. Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > * Tom Lane (tgl@sss.pgh.pa.us) wrote: >> everything behaves per spec, except that I don't want to support the >> aspect of the spec that says you can SET ROLE at the outer level and >> still have the privileges of the SESSION_USER. I think SET ROLE should >> effectively drop the SESSION_USER's privileges (except that subsequent >> SET ROLE commands will be checked against the SESSION_USER's role >> memberships, not the current effective role). > I don't particularly like deviating from the spec in this regard (since > I don't think it'd be all that hard to implement what the spec calls > for), but it doesn't bother me that much. The problem I have with the spec's way is that it creates a disconnect between the privilege environment seen at the outer level and the environment seen within SECURITY DEFINER functions --- unless you want to allow SET ROLE to have the union behavior within SECURITY DEFINER functions too, which I don't want to support (and it's not legal per spec anyway to do SET ROLE inside a function). > While I agree that this is what Oracle's SET ROLE ALL does initially, > it's possible for a user to 'SET ROLE <a>' and drop the permissions > given by the other roles in which the user is in. Will that still be > possible with your proposed solution, or will doing 'SET ROLE <a>' have > no effect when 'rolinherit = true'? That's really my main concern. According to my proposal "SET ROLE x" would drop the user's privileges and thus be a privilege restriction operation, never a privilege addition operation, if the user has rolinherit = true. If we don't say that SET ROLE drops the session user's privileges then indeed SET ROLE would be a no-op when the session user has rolinherit = true... regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > The problem I have with the spec's way is that it creates a disconnect > between the privilege environment seen at the outer level and the > environment seen within SECURITY DEFINER functions --- unless you want > to allow SET ROLE to have the union behavior within SECURITY DEFINER > functions too, which I don't want to support (and it's not legal per > spec anyway to do SET ROLE inside a function). Essentially the union behavior is what the spec seems to say- except that only one or the other is valid inside a SECURITY DEFINER, as I understand it. So, you make everything do the union, but when you go into a SECURITY DEFINER function you set the one-not-set to NULL and handle that correctly in the union. I'm not advocating allowing SET ROLE inside a function, no. Again, this is more about the spec than an actual use-case that I have for it, so we can ignore it until someone with a more concrete problem with it comes along. > > While I agree that this is what Oracle's SET ROLE ALL does initially, > > it's possible for a user to 'SET ROLE <a>' and drop the permissions > > given by the other roles in which the user is in. Will that still be > > possible with your proposed solution, or will doing 'SET ROLE <a>' have > > no effect when 'rolinherit = true'? That's really my main concern. > > According to my proposal "SET ROLE x" would drop the user's privileges > and thus be a privilege restriction operation, never a privilege > addition operation, if the user has rolinherit = true. If we don't say > that SET ROLE drops the session user's privileges then indeed SET ROLE > would be a no-op when the session user has rolinherit = true... Right, I would expect it to drop privileges when rolinherit = true. The second issue is one reason I don't particularly care for locking it into the catalog- it means we're building the system in such a way as to be unable to support what Oracle (at least) does today. If we end up needing to support it later, or wanting to, perhaps because the spec follows Oracle's lead and adds SET ROLE ALL, then we've got alot that would need to be changed because things have become dependent on the catalog directly. Otherwise, I think your proposal is fine. :) Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > * Tom Lane (tgl@sss.pgh.pa.us) wrote: >> The problem I have with the spec's way is that it creates a disconnect >> between the privilege environment seen at the outer level and the >> environment seen within SECURITY DEFINER functions > Essentially the union behavior is what the spec seems to say- except > that only one or the other is valid inside a SECURITY DEFINER, as I > understand it. So, you make everything do the union, but when you go > into a SECURITY DEFINER function you set the one-not-set to NULL and > handle that correctly in the union. My understanding of things is that per spec, a SECURITY DEFINER function can be owned by either a user or a role, and so within the function either CURRENT_USER or CURRENT_ROLE would return the owner and the other would return NULL. Emulating this would require a hard distinction between users and roles that is simply not there in our implementation, which is why I think they should both return the owner. > Right, I would expect it to drop privileges when rolinherit = true. The > second issue is one reason I don't particularly care for locking it into > the catalog- it means we're building the system in such a way as to be > unable to support what Oracle (at least) does today. If we end up > needing to support it later, or wanting to, perhaps because the spec > follows Oracle's lead and adds SET ROLE ALL, then we've got alot that > would need to be changed because things have become dependent on the > catalog directly. To some extent SET ROLE ALL can be emulated by ALTER USER ... INHERIT. I'm of two minds about whether an unprivileged user should be allowed to adjust his own rolinherit flag --- in most cases it seems pretty harmless (and Oracle evidently thinks it is) --- but one could imagine that the roles have been set up on the assumption that you can't get more than one role's privileges at a time. INHERIT (or SET ROLE ALL) would break that assumption, and perhaps allow people to do unwanted stuff. regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > My understanding of things is that per spec, a SECURITY DEFINER function > can be owned by either a user or a role, and so within the function > either CURRENT_USER or CURRENT_ROLE would return the owner and the other > would return NULL. Emulating this would require a hard distinction > between users and roles that is simply not there in our implementation, > which is why I think they should both return the owner. I would have been more inclined to just pick one and always set it and leave the other always null. For that, CURRENT_USER would be more backwards-compatible, but for our implementation I'd tend to think CURRENT_ROLE is more appropriate. That'd follow the spec closer and would be closer to what functions written to the spec would expect. I don't use SECURITY DEFINER functions much though so perhaps others have a stronger opinion. I've been a bit suprised at the lack of commentary from other people, perhaps they're just waiting to destroy whatever we come up with once it's actually been implemented. :) > To some extent SET ROLE ALL can be emulated by ALTER USER ... INHERIT. Yeah, but that affects all sessions too, not just a single one, which makes it quite a different thing. > I'm of two minds about whether an unprivileged user should be allowed > to adjust his own rolinherit flag --- in most cases it seems pretty > harmless (and Oracle evidently thinks it is) --- but one could imagine > that the roles have been set up on the assumption that you can't get > more than one role's privileges at a time. INHERIT (or SET ROLE ALL) > would break that assumption, and perhaps allow people to do unwanted > stuff. This is actually what I was thinking about when I was saying at some point prior in this thread that we should have an option to indicate if SET ROLE ALL is allowed or not. I don't think that users should be allowed to adjust their own rolinherit flag. I think the default should probably be 'true', even for users, but if an admin sets it to false then I think that should be enforced and users shouldn't be allowed to change it. I suspect it's possible to disable 'SET ROLE ALL' in Oracle, and to turn off having it done upon connection. I'd be somewhat suprised if it wasn't possible but I havn't really investigated it either way. I don't know if Oracle has a way to let you do it per-user/per-role though. Thanks, Stephen
I've committed changes to add a "rolinherit" flag to pg_authid as per discussion. The pg_has_role function now distinguishes USAGE rights on a role (do you currently have the privileges of that role) from MEMBER rights (do you have the ability to SET ROLE to that role). A couple of loose ends remain: * Should is_admin_of_role pay attention to rolinherit? I suspect it should but can't quite face going through the SQL spec again to be sure. This only affects the right to GRANT role membership to someone else. * The information_schema needs another pass to see which pg_has_role usages ought to be testing USAGE instead of MEMBER. I think most of them should, but in and around applicable_roles and enabled_roles some more thought and spec-reading is needed. I'm planning on doing some documentation work next, and was hoping someone else would look at the above items. regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > I've committed changes to add a "rolinherit" flag to pg_authid as per > discussion. The pg_has_role function now distinguishes USAGE rights > on a role (do you currently have the privileges of that role) from > MEMBER rights (do you have the ability to SET ROLE to that role). Great, thanks. > A couple of loose ends remain: > > * Should is_admin_of_role pay attention to rolinherit? I suspect it > should but can't quite face going through the SQL spec again to be sure. > This only affects the right to GRANT role membership to someone else. > > * The information_schema needs another pass to see which pg_has_role > usages ought to be testing USAGE instead of MEMBER. I think most of > them should, but in and around applicable_roles and enabled_roles > some more thought and spec-reading is needed. I'll look into what the spec says for these, hopefully anoncvs is working now... > I'm planning on doing some documentation work next, and was hoping > someone else would look at the above items. Will do. I'll be using the SQL2003 draft. Should be able to run these down later today. Thanks, Stephen