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

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

* 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



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

iD8DBQFC5RpSrzgMPqB3kigRAoQ8AKCXHMrCAiV7l8tSWFIyy4 beuIcB0wCgkFyf
diuso5u0oC9XN22ZGHsLHbU=
=mXLy
-----END PGP SIGNATURE-----

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

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

* 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

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

iD8DBQFC5Sg5rzgMPqB3kigRAvlqAJ9LhSx3uYqDxcADNdUK3y VgSGUihgCglXlv
e3B32aGtZ4o77MHDFMTy4f0=
=vcXJ
-----END PGP SIGNATURE-----

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

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

* 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

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

iD8DBQFC5mo7rzgMPqB3kigRAjU4AJ9CWj4hQFv1+LDns/uj5Vj/6EExWgCZAZhc
rhkmkamnmqmklalufpeSQp4=
=ApKa
-----END PGP SIGNATURE-----