Unix Technical Forum

Re: [PATCHES] Roles - SET ROLE Updated

This is a discussion on Re: [PATCHES] Roles - SET ROLE Updated within the pgsql Hackers forums, part of the PostgreSQL category; --> * Tom Lane (tgl@sss.pgh.pa.us) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > * Tom Lane (tgl@sss.pgh.pa.us) wrote: > >> ...


Go Back   Unix Technical Forum > Database Server Software > PostgreSQL > pgsql Hackers

Register FAQ Members List Calendar Search Today's Posts Mark Forums Read
  #1 (permalink)  
Old 04-11-2008, 04:56 AM
Stephen Frost
 
Posts: n/a
Default 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

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFC3/1QrzgMPqB3kigRApB/AJ40xJmNhCSHpkoywrN2/M1C5AtIlACePsPw
r2ni+917pZf6+hOgVKRw030=
=36F+
-----END PGP SIGNATURE-----

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #2 (permalink)  
Old 04-11-2008, 04:56 AM
Tom Lane
 
Posts: n/a
Default Re: [PATCHES] Roles - SET ROLE Updated

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

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #3 (permalink)  
Old 04-11-2008, 04:56 AM
Tom Lane
 
Posts: n/a
Default Re: [PATCHES] Roles - SET ROLE Updated

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 authorization descriptors
whose grantee is that <authorization identifier> or PUBLIC together
with all other roles they contain.

The set of user privileges for a <user identifier> consists of all
privileges defined by the privilege descriptors whose 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 descriptors whose 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

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #4 (permalink)  
Old 04-11-2008, 04:56 AM
Stephen Frost
 
Posts: n/a
Default Re: [PATCHES] Roles - SET ROLE Updated

* 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

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFC4AwwrzgMPqB3kigRAk/WAJ9dLK/9a+UqmBqNQ8IegU1zK9B/YgCaA1Q9
K1NEmTYIdCHygBkZnAyfMxg=
=OOYy
-----END PGP SIGNATURE-----

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #5 (permalink)  
Old 04-11-2008, 04:56 AM
Stephen Frost
 
Posts: n/a
Default Re: [PATCHES] Roles - SET ROLE Updated

* 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



-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFC4A5/rzgMPqB3kigRAkFAAKCX33BUoxfl/IfArqH/iHsb/Iu8fgCggv5s
HLWR5ocDjCNxyNeQbXB+3H8=
=9r2u
-----END PGP SIGNATURE-----

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #6 (permalink)  
Old 04-11-2008, 04:56 AM
Tom Lane
 
Posts: n/a
Default Re: [PATCHES] Roles - SET ROLE Updated

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

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #7 (permalink)  
Old 04-11-2008, 04:56 AM
Stephen Frost
 
Posts: n/a
Default Re: [PATCHES] Roles - SET ROLE Updated

* 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

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFC4A8zrzgMPqB3kigRAnbbAKCHa3RVb/6EN5mJtB8JnPept8LJygCaA6Ul
BHSi7ghAggLyV1Wqn453MJQ=
=3I1C
-----END PGP SIGNATURE-----

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #8 (permalink)  
Old 04-11-2008, 04:56 AM
Tom Lane
 
Posts: n/a
Default Re: [PATCHES] Roles - SET ROLE Updated

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

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #9 (permalink)  
Old 04-11-2008, 04:56 AM
Stephen Frost
 
Posts: n/a
Default Re: [PATCHES] Roles - SET ROLE Updated

* 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

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFC4BJ+rzgMPqB3kigRAvjEAJ4jndLenpDNQzIXZ7hRfS M9JqHpywCggU4U
OMMyBcGGt66XmZlSEeSeuAI=
=OUKa
-----END PGP SIGNATURE-----

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #10 (permalink)  
Old 04-11-2008, 04:57 AM
Stephen Frost
 
Posts: n/a
Default Re: [PATCHES] Roles - SET ROLE Updated

* 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

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFC4BXCrzgMPqB3kigRAiR6AJ9dkGNnO5IP8HG2jGnGJL 3a1Wi+swCfQsNu
Qq3oGqNE1xd95xAgJ1nyBbs=
=JtJ+
-----END PGP SIGNATURE-----

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
Reply


Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are On
Forum Jump


All times are GMT. The time now is 05:38 AM.


Powered by vBulletin® Version 3.6.5
Copyright ©2000 - 2008, Jelsoft Enterprises Ltd.
SEO by vBSEO 3.2.0
www.UnixAdminTalk.com