allowing for control over SET ROLE
Hi,
There are two ways in which a role can exercise the privileges of some
other role which has been granted to it. First, it can implicitly
inherit the privileges of the granted role. Second, it can assume the
identity of the granted role using the SET ROLE command. It is
possible to control the former behavior, but not the latter. In v15
and prior release, we had a role-level [NO]INHERIT property which
controlled whether a role automatically inherited the privileges of
any role granted to it. This was all-or-nothing. Beginning in
e3ce2de09d814f8770b2e3b3c152b7671bcdb83f, the inheritance behavior of
role-grants can be overridden for individual grants, so that some
grants are inherited and others are not. However, there is no similar
facility for controlling whether a role can SET ROLE to some other
role of which it is a member. At present, if role A is a member of
role B, then A can SET ROLE B, and that's it.
In some circumstances, it may be desirable to control this behavior.
For example, if we GRANT pg_read_all_settings TO seer, we do want the
seer to be able to read all the settings, else we would not have
granted the role. But we might not want the seer to be able to do
this:
You are now connected to database "rhaas" as user "seer".
rhaas=> set role pg_read_all_settings;
SET
rhaas=> create table artifact (a int);
CREATE TABLE
rhaas=> \d
List of relations
Schema | Name | Type | Owner
--------+----------+-------+----------------------
public | artifact | table | pg_read_all_settings
(1 row)
I have attached a rather small patch which makes it possible to
control this behavior:
You are now connected to database "rhaas" as user "rhaas".
rhaas=# grant pg_read_all_settings to seer with set false;
GRANT ROLE
rhaas=# \c - seer
You are now connected to database "rhaas" as user "seer".
rhaas=> set role pg_read_all_settings;
ERROR: permission denied to set role "pg_read_all_settings"
I think that this behavior is generally useful, and not just for the
predefined roles that we ship as part of PostgreSQL. I don't think
it's too hard to imagine someone wanting to use some locally created
role as a container for privileges but not wanting the users who
possess this role to run around creating new objects owned by it. To
some extent that can be controlled by making sure the role in question
doesn't have any excess privileges, but that's not really sufficient:
all you need is one schema anywhere in the system that grants CREATE
to PUBLIC. You could avoid creating such a schema, which might be a
good idea for other reasons anyway, but it feels like approaching the
problem from the wrong end. What you really want is to allow the users
to inherit the privileges of the role but not use SET ROLE to become
that role, so that's what this patch lets you do.
There's one other kind of case in which this sort of thing might be
somewhat useful, although it's more dubious. Suppose you have an
oncall group where you regularly add and remove members according to
who is on call. Naturally, you have an on-call bot which performs this
task automatically. The on-call bot has the ability to manage
memberships in the oncall group, but should not have the ability to
access any of its privileges, either by inheritance or via SET ROLE.
This patch KIND OF lets you accomplish this:
rhaas=# create role oncall;
CREATE ROLE
rhaas=# create role oncallbot login;
CREATE ROLE
rhaas=# grant oncall to oncallbot with inherit false, set false, admin true;
GRANT ROLE
rhaas=# create role anna;
CREATE ROLE
rhaas=# create role eliza;
CREATE ROLE
rhaas=# \c - oncallbot
You are now connected to database "rhaas" as user "oncallbot".
rhaas=> grant oncall to anna;
GRANT ROLE
rhaas=> revoke oncall from anna;
REVOKE ROLE
rhaas=> grant oncall to eliza;
GRANT ROLE
rhaas=> set role oncall;
ERROR: permission denied to set role "oncall"
The problem here is that if a nasty evil hacker takes over the
oncallbot role, nothing whatsoever prevents them from executing "grant
oncall to oncallbot with set true" after which they can then "SET ROLE
oncall" using the privileges they just granted themselves. And even if
under some theory we blocked that, they could still maliciously grant
the sought-after on-call privileges to some other role i.e. "grant
oncall to accomplice". It's fundamentally difficult to allow people to
administer a set of privileges without giving them the ability to
usurp those privileges, and I wouldn't like to pretend that this patch
is in any way sufficient to accomplish such a thing. Nevertheless, I
think there's some chance it might be useful to someone building such
a system, in combination with other safeguards. Or maybe not: this
isn't the main reason I'm interested in this, and it's just an added
benefit if it turns out that someone can do something like this with
it.
In order to apply this patch, we'd need to reach a conclusion about
the matters mentioned in
/messages/by-id/CA+TgmobhEYYnW9vrHvoLvD8ODsPBJuU9CbK6tms6Owd70hFMTw@mail.gmail.com
-- and thinking about this patch might shed some light on what we'd
want to do over there.
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachments:
v1-0001-Add-a-SET-option-to-the-GRANT-command.patchapplication/octet-stream; name=v1-0001-Add-a-SET-option-to-the-GRANT-command.patchDownload+199-48
On Wed, Aug 31, 2022 at 08:56:31AM -0400, Robert Haas wrote:
In some circumstances, it may be desirable to control this behavior.
For example, if we GRANT pg_read_all_settings TO seer, we do want the
seer to be able to read all the settings, else we would not have
granted the role. But we might not want the seer to be able to do
this:You are now connected to database "rhaas" as user "seer".
rhaas=> set role pg_read_all_settings;
SET
rhaas=> create table artifact (a int);
CREATE TABLE
rhaas=> \d
List of relations
Schema | Name | Type | Owner
--------+----------+-------+----------------------
public | artifact | table | pg_read_all_settings
(1 row)
+1
The problem here is that if a nasty evil hacker takes over the
oncallbot role, nothing whatsoever prevents them from executing "grant
oncall to oncallbot with set true" after which they can then "SET ROLE
oncall" using the privileges they just granted themselves. And even if
under some theory we blocked that, they could still maliciously grant
the sought-after on-call privileges to some other role i.e. "grant
oncall to accomplice". It's fundamentally difficult to allow people to
administer a set of privileges without giving them the ability to
usurp those privileges, and I wouldn't like to pretend that this patch
is in any way sufficient to accomplish such a thing. Nevertheless, I
think there's some chance it might be useful to someone building such
a system, in combination with other safeguards. Or maybe not: this
isn't the main reason I'm interested in this, and it's just an added
benefit if it turns out that someone can do something like this with
it.
Yeah, if you have ADMIN for a role, you would effectively have SET. I'm
tempted to suggest that ADMIN roles should be restricted from granting SET
unless they have it themselves. However, that seems like it'd create a
weird discrepancy. If you have ADMIN but not INHERIT or SET, you'd still
be able to grant membership with or without INHERIT, but you wouldn't be
able to grant SET. In the end, I guess I agree with you that it's
"fundamentally difficult to allow people to administer a set of privileges
without giving them the ability to usurp those privileges..."
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Robert Haas:
Beginning in
e3ce2de09d814f8770b2e3b3c152b7671bcdb83f, the inheritance behavior of
role-grants can be overridden for individual grants, so that some
grants are inherited and others are not.
That's a great thing to have!
However, there is no similar
facility for controlling whether a role can SET ROLE to some other
role of which it is a member. At present, if role A is a member of
role B, then A can SET ROLE B, and that's it.In some circumstances, it may be desirable to control this behavior.
+1
rhaas=# grant oncall to oncallbot with inherit false, set false, admin true;
Looking at the syntax here, I'm not sure whether adding more WITH
options is the best way to do this. From a user perspective WITH SET
TRUE looks more like a privilege granted on how to use this database
object (role). Something like this would be more consistent with the
other GRANT variants:
GRANT SET ON ROLE oncall TO oncallbot WITH GRANT OPTION;
This is obviously not exactly the same as the command above, because
oncallbot would be able to use SET ROLE directly. But as discussed, this
is more cosmetic anyway, because they could GRANT it to themselves.
The full syntax could look like this:
GRANT { INHERIT | SET | ALL [ PRIVILEGES ] }
ON ROLE role_name [, ...]
TO role_specification [, ...] WITH GRANT OPTION
[ GRANTED BY role_specification ]
With this new syntax, the existing
GRANT role_name TO role_specification [WITH ADMIN OPTION];
would be the same as
GRANT ALL ON role_name TO role_specification [WITH GRANT OPTION];
This would slightly change the way INHERIT works: As a privilege, it
would not override the member's role INHERIT attribute, but would
control whether that attribute is applied. This means:
- INHERIT attribute + INHERIT granted -> inheritance (same)
- INHERIT attribute + INHERIT not granted -> no inheritance (different!)
- NOINHERIT attribute + INHERIT not granted -> no inheritance (same)
- NOINHERIT attribute + INHERIT granted -> no inheritance (different!)
This would allow us to do the following:
GRANT INHERIT ON ROLE pg_read_all_settings TO seer_bot WITH GRANT OPTION;
seer_bot would now be able to GRANT pg_read_all_settings to other users,
too - but without the ability to use or grant SET ROLE to anyone. As
long as seer_bot has the NOINHERIT attribute set, they wouldn't use that
privilege, though - which might be desired for the bot.
Similary, it would be possible for the oncallbot in the example above to
be able to grant SET ROLE only - and not INHERIT.
I realize that there has been a lot of discussion about roles and
privileges in the past year. I have tried to follow those discussions,
but it's likely that I missed some good arguments against my proposal above.
Best
Wolfgang
On Fri, Sep 2, 2022 at 3:20 AM Wolfgang Walther <walther@technowledgy.de> wrote:
The full syntax could look like this:
GRANT { INHERIT | SET | ALL [ PRIVILEGES ] }
ON ROLE role_name [, ...]
TO role_specification [, ...] WITH GRANT OPTION
[ GRANTED BY role_specification ]With this new syntax, the existing
GRANT role_name TO role_specification [WITH ADMIN OPTION];
would be the same as
GRANT ALL ON role_name TO role_specification [WITH GRANT OPTION];
This would be a pretty significant rework. Right now, there's only one
ADMIN OPTION on a role, and you either have it or you don't. Changing
things around so that you can have each individual privilege with or
without grant option would be a fair amount of work. I don't think
it's completely crazy, but I'm not very sold on the idea, either,
because giving somebody *either* the ability to grant INHERIT option
*or* the ability to grant SET option is largely equivalent from a
security point of view. Either way, the grantees will be able to
access the privileges of the role in some fashion. This is different
from table privileges, where SELECT and INSERT are clearly distinct
rights that do not overlap, and thus separating the ability to
administer one of those things from the ability to administer the
other one has more utility.
The situation might look different in the future if we added more role
options and if each of those were clearly severable rights. For
instance, if we had a DROP option on a role grant that conferred the
right to drop the role, that would be distinct from SET and INHERIT
and it might make sense to allow someone to administer SET and/or
INHERIT but not DROP. However, I don't have any current plans to add
such an option, and TBH I find it a little hard to come up with a
compelling list of things that would be worth adding as separate
permissions here. There are a bunch of things that one role can do to
another using ALTER ROLE, and right now you have to be SUPERUSER or
have CREATEROLE to do that stuff. In
theory, you could turn that into a big list of individual rights so
that you can e.g. GRANT CHANGE PASSWORD ON role1 TO role2 WITH GRANT
OPTION.
However, I really don't see a lot of utility in slicing things up at
that level of granularity. There isn't in my view a lot of use case
for giving a user the right to change some other user's password but
not giving them the right to set the connection limit for that same
other user -- and there's even less use case for giving some user the
ability to grant one of those rights but not the other.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Wed, 2022-08-31 at 08:56 -0400, Robert Haas wrote:
In some circumstances, it may be desirable to control this behavior.
For example, if we GRANT pg_read_all_settings TO seer, we do want the
seer to be able to read all the settings, else we would not have
granted the role. But we might not want the seer to be able to do
this:You are now connected to database "rhaas" as user "seer".
rhaas=> set role pg_read_all_settings;
SET
rhaas=> create table artifact (a int);
CREATE TABLE
rhaas=> \d
List of relations
Schema | Name | Type | Owner
--------+----------+-------+----------------------
public | artifact | table | pg_read_all_settings
(1 row)
Interesting case.
I have attached a rather small patch which makes it possible to
control this behavior:You are now connected to database "rhaas" as user "rhaas".
rhaas=# grant pg_read_all_settings to seer with set false;
GRANT ROLE
You've defined this in terms of the mechanics -- allow SET ROLE or not
-- but I assume you intend it as a security feature to allow/forbid
some capabilities.
Is this only about the capability to create objects owned by a role
you're a member of? Or are there other implications?
Regards,
Jeff Davis
On Sat, Sep 3, 2022 at 3:46 PM Jeff Davis <pgsql@j-davis.com> wrote:
You are now connected to database "rhaas" as user "rhaas".
rhaas=# grant pg_read_all_settings to seer with set false;
GRANT ROLEYou've defined this in terms of the mechanics -- allow SET ROLE or not
-- but I assume you intend it as a security feature to allow/forbid
some capabilities.Is this only about the capability to create objects owned by a role
you're a member of? Or are there other implications?
I think there are some other implications, but I don't think they're
anything super-dramatic. For example, you could create a group that's
just for purposes of pg_hba.conf matching and make the grants both SET
FALSE and INHERIT FALSE, with the idea that the members shouldn't have
any access to the role; it's just there for grouping purposes. I
mentioned one other possible scenario, with oncallbot, in the original
post.
I'm not sure whether thinking about this in terms of security
capabilities is the most helpful way to view it. My view was, as you
say, more mechanical. I think sometimes you grant somebody a role and
you want them to be able to use SET ROLE to assume the privileges of
the target role, and sometimes you don't. I think that primarily
depends on the reason why you made the grant. In the case of a
predefined role, you're almost certainly granting membership so that
the privileges of the predefined role can be inherited. In other
cases, you may be doing it so that the member can SET ROLE to the
target role, or you may be doing it so that the member can administer
the role (because you give them ADMIN OPTION), or you may even be
doing it for pg_hba.conf matching.
And because of this, I think it follows that there may be some
capabilities conferred by role membership that you don't really want
to convey in particular cases, so I think it makes sense to have a way
to avoid conveying the ones that aren't necessary for the grant to
fulfill its purpose. I'm not exactly sure how far that gets you in
terms of building a system that is more secure than what you could
build otherwise, but it feels like a useful capability regardless.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Tue, 2022-09-06 at 10:42 -0400, Robert Haas wrote:
I think there are some other implications, but I don't think they're
anything super-dramatic. For example, you could create a group that's
just for purposes of pg_hba.conf matching and make the grants both
SET
FALSE and INHERIT FALSE, with the idea that the members shouldn't
have
any access to the role; it's just there for grouping purposes. I
mentioned one other possible scenario, with oncallbot, in the
original
post.
Interesting. All of those seem like worthwhile use cases to me.
I'm not sure whether thinking about this in terms of security
capabilities is the most helpful way to view it. My view was, as you
say, more mechanical. I think sometimes you grant somebody a role and
you want them to be able to use SET ROLE to assume the privileges of
the target role, and sometimes you don't.
By denying the ability to "SET ROLE pg_read_all_settings", I assumed
that we'd deny the ability to create objects owned by that
pg_read_all_settings. But on closer inspection:
grant all privileges on schema public to public;
create user u1;
grant pg_read_all_settings to u1 with set false;
\c - u1
create table foo(i int);
set role pg_read_all_settings;
ERROR: permission denied to set role "pg_read_all_settings"
alter table foo owner to pg_read_all_settings;
\d
List of relations
Schema | Name | Type | Owner
--------+------+-------+----------------------
public | foo | table | pg_read_all_settings
(1 row)
Users will reasonably interpret any feature of GRANT to be a security
feature that allows or prevents certain users from causing certain
outcomes. But here, I was initially fooled, and the outcome is still
possible.
So I believe we do need to think in terms of what capabilities we are
really restricting with this feature rather than solely the mechanics.
Regards,
Jeff Davis
On Tue, Sep 6, 2022 at 2:45 PM Jeff Davis <pgsql@j-davis.com> wrote:
I'm not sure whether thinking about this in terms of security
capabilities is the most helpful way to view it. My view was, as you
say, more mechanical. I think sometimes you grant somebody a role and
you want them to be able to use SET ROLE to assume the privileges of
the target role, and sometimes you don't.By denying the ability to "SET ROLE pg_read_all_settings", I assumed
that we'd deny the ability to create objects owned by that
pg_read_all_settings. But on closer inspection:grant all privileges on schema public to public;
create user u1;
grant pg_read_all_settings to u1 with set false;
\c - u1
create table foo(i int);
set role pg_read_all_settings;
ERROR: permission denied to set role "pg_read_all_settings"
alter table foo owner to pg_read_all_settings;
\d
List of relations
Schema | Name | Type | Owner
--------+------+-------+----------------------
public | foo | table | pg_read_all_settings
(1 row)
Yeah. Please note this paragraph in my original post:
"In order to apply this patch, we'd need to reach a conclusion about
the matters mentioned in
/messages/by-id/CA+TgmobhEYYnW9vrHvoLvD8ODsPBJuU9CbK6tms6Owd70hFMTw@mail.gmail.com
-- and thinking about this patch might shed some light on what we'd
want to do over there."
I hadn't quite gotten around to updating that thread based on posting
this, but this scenario was indeed on my mind.
--
Robert Haas
EDB: http://www.enterprisedb.com
On 31.08.22 14:56, Robert Haas wrote:
In some circumstances, it may be desirable to control this behavior.
For example, if we GRANT pg_read_all_settings TO seer, we do want the
seer to be able to read all the settings, else we would not have
granted the role. But we might not want the seer to be able to do
this:You are now connected to database "rhaas" as user "seer".
rhaas=> set role pg_read_all_settings;
SET
rhaas=> create table artifact (a int);
CREATE TABLE
rhaas=> \d
List of relations
Schema | Name | Type | Owner
--------+----------+-------+----------------------
public | artifact | table | pg_read_all_settings
(1 row)
I think this is because we have (erroneously) make SET ROLE to be the
same as SET SESSION AUTHORIZATION. If those two were separate (i.e.,
there is a current user and a separate current role, as in the SQL
standard), then this would be more straightforward.
I don't know if it's possible to untangle that at this point.
On Mon, Sep 12, 2022 at 11:41 AM Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
I think this is because we have (erroneously) make SET ROLE to be the
same as SET SESSION AUTHORIZATION. If those two were separate (i.e.,
there is a current user and a separate current role, as in the SQL
standard), then this would be more straightforward.I don't know if it's possible to untangle that at this point.
I think that it already works as you describe:
rhaas=# create role foo;
CREATE ROLE
rhaas=# create role bar;
CREATE ROLE
rhaas=# grant bar to foo;
GRANT ROLE
rhaas=# set session authorization foo;
SET
rhaas=> set role bar;
SET
rhaas=> select current_user;
current_user
--------------
bar
(1 row)
rhaas=> select session_user;
session_user
--------------
foo
(1 row)
There may well be problems here, but this example shows that the
current_user and session_user concepts are different in PostgreSQL.
It's also true that the privileges required to execute the commands
are different: SET SESSION AUTHORIZATION requires that the session
user is a superuser, and SET ROLE requires that the identity
established via SET SESSION AUTHORIZATION has the target role granted
to it.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Wed, Aug 31, 2022 at 8:56 AM Robert Haas <robertmhaas@gmail.com> wrote:
In order to apply this patch, we'd need to reach a conclusion about
the matters mentioned in
/messages/by-id/CA+TgmobhEYYnW9vrHvoLvD8ODsPBJuU9CbK6tms6Owd70hFMTw@mail.gmail.com
-- and thinking about this patch might shed some light on what we'd
want to do over there.
That thread has not reached an entirely satisfying conclusion.
However, the behavior that was deemed outright buggy over there has
been fixed. The remaining question is what to do about commands that
allow you to give objects to other users (like ALTER <whatever> ..
OWNER TO) or commands that allow you to create objects owned by other
users (like CREATE DATABASE ... OWNER). I have, in this version,
adopted the proposal by Wolfgang Walther on the other thread to make
this controlled by the new SET option. This essentially takes the view
that the ability to create objects owned by another user is not
precisely a privilege, and is thus not inherited just because the
INHERIT option is set on the GRANT, but it is something you can do if
you could SET ROLE to that role, so we make it dependent on the SET
option. This logic is certainly debatable, but it does have the
practical advantage of making INHERIT TRUE, SET FALSE a useful
combination of settings for predefined roles. It's also 100%
backward-compatible, whereas if we made the behavior dependent on the
INHERIT option, users could potentially notice behavior changes after
upgrading to v16.
So I do like this behavior ... but it's definitely arguable whether
it's the best thing. At any rate, here's an updated patch that
implements it, and to which I've also added a test case.
Review appreciated.
Thanks,
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachments:
v2-0001-Add-a-SET-option-to-the-GRANT-command.patchapplication/octet-stream; name=v2-0001-Add-a-SET-option-to-the-GRANT-command.patchDownload+315-92
On Fri, Sep 30, 2022 at 04:34:32PM -0400, Robert Haas wrote:
That thread has not reached an entirely satisfying conclusion.
However, the behavior that was deemed outright buggy over there has
been fixed. The remaining question is what to do about commands that
allow you to give objects to other users (like ALTER <whatever> ..
OWNER TO) or commands that allow you to create objects owned by other
users (like CREATE DATABASE ... OWNER). I have, in this version,
adopted the proposal by Wolfgang Walther on the other thread to make
this controlled by the new SET option. This essentially takes the view
that the ability to create objects owned by another user is not
precisely a privilege, and is thus not inherited just because the
INHERIT option is set on the GRANT, but it is something you can do if
you could SET ROLE to that role, so we make it dependent on the SET
option. This logic is certainly debatable, but it does have the
practical advantage of making INHERIT TRUE, SET FALSE a useful
combination of settings for predefined roles. It's also 100%
backward-compatible, whereas if we made the behavior dependent on the
INHERIT option, users could potentially notice behavior changes after
upgrading to v16.
I'm not sure about tying the ownership stuff to this new SET privilege.
While you noted some practical advantages, I'd expect users to find it kind
of surprising. Also, for predefined roles, I think you need to be careful
about distributing ADMIN, as anyone with ADMIN on a predefined role can
just GRANT SET to work around the restrictions. I don't have a better
idea, though, so perhaps neither of these things is a deal-breaker. I was
tempted to suggest using ADMIN instead of SET for the ownership stuff, but
that wouldn't be backward-compatible, and you'd still be able to work
around it to some extent with SET (e.g., SET ROLE followed by CREATE
DATABASE).
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Greetings,
* Nathan Bossart (nathandbossart@gmail.com) wrote:
On Fri, Sep 30, 2022 at 04:34:32PM -0400, Robert Haas wrote:
That thread has not reached an entirely satisfying conclusion.
However, the behavior that was deemed outright buggy over there has
been fixed. The remaining question is what to do about commands that
allow you to give objects to other users (like ALTER <whatever> ..
OWNER TO) or commands that allow you to create objects owned by other
users (like CREATE DATABASE ... OWNER). I have, in this version,
adopted the proposal by Wolfgang Walther on the other thread to make
this controlled by the new SET option. This essentially takes the view
that the ability to create objects owned by another user is not
precisely a privilege, and is thus not inherited just because the
INHERIT option is set on the GRANT, but it is something you can do if
you could SET ROLE to that role, so we make it dependent on the SET
option. This logic is certainly debatable, but it does have the
practical advantage of making INHERIT TRUE, SET FALSE a useful
combination of settings for predefined roles. It's also 100%
backward-compatible, whereas if we made the behavior dependent on the
INHERIT option, users could potentially notice behavior changes after
upgrading to v16.I'm not sure about tying the ownership stuff to this new SET privilege.
While you noted some practical advantages, I'd expect users to find it kind
of surprising. Also, for predefined roles, I think you need to be careful
about distributing ADMIN, as anyone with ADMIN on a predefined role can
just GRANT SET to work around the restrictions. I don't have a better
idea, though, so perhaps neither of these things is a deal-breaker. I was
tempted to suggest using ADMIN instead of SET for the ownership stuff, but
that wouldn't be backward-compatible, and you'd still be able to work
around it to some extent with SET (e.g., SET ROLE followed by CREATE
DATABASE).
As we work through splitting up the privileges and managing them in a
more fine-grained way, it seems clear that we'll need to have a similar
split for ADMIN rights on roles- that is, we'll need to be able to
say "role X is allowed to GRANT INHERIT for role Y to other roles, but
not SET".
I'm still half-tempted to say that predefined roles should just be dealt
with as a special case.. but if we split ADMIN in the manner as
described above then maybe we could get away with not having to, but it
would depend a great deal of people actually reading the documentation
and I'm concerned that's a bit too much to ask in this case.
That is- the first person who is likely to GRANT out ADMIN rights in a
predefined role is going to be a superuser. To avoid breaking backwards
compatibility, GRANT'ing of ADMIN needs to GRANT all the partial-ADMIN
rights that exist, or at least exist today, which includes both SET and
INHERIT. Unless we put some kind of special case for predefined roles
where we throw an error or at least a warning when a superuser
(presumably) inadvertantly does a simple GRANT ADMIN for $predefined
role, we're going to end up in the situation where folks can SET ROLE to
a predefined role and do things that they really shouldn't be allowed
to.
We could, of course, very clearly document that the way to GRANT ADMIN
rights for a predefined role is to always make sure to *only* GRANT
ADMIN/INHERIT, but again I worry that it simply wouldn't be followed in
many cases. Perhaps we could arrange for the bootstrap superuser to
only be GRANT'd ADMIN/INHERIT for predefined roles and then not have an
explicit cut-out for superuser doing a GRANT on predefined roles or
perhaps having such be protected under allow_system_table_mods under the
general consideration that modifying of predefined roles isn't something
that folks should be doing post-initdb.
Just a few thoughts on this, not sure any of these ideas are great but
perhaps this helps move us forward.
Thanks,
Stephen
On Wed, Oct 12, 2022 at 4:59 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
I'm not sure about tying the ownership stuff to this new SET privilege.
While you noted some practical advantages, I'd expect users to find it kind
of surprising. Also, for predefined roles, I think you need to be careful
about distributing ADMIN, as anyone with ADMIN on a predefined role can
just GRANT SET to work around the restrictions. I don't have a better
idea, though, so perhaps neither of these things is a deal-breaker.
Right, I think if you give ADMIN away to someone, that's it: they can
grant that role to whoever they want in whatever mode they want,
including themselves. That seems more or less intentional to me,
though. Giving someone ADMIN OPTION on a role is basically making them
an administrator of that role, and then it is not surprising that they
can access its privileges.
I agree with your other caveat about it being potentially surprising,
but I think it's not worse than a lot of other somewhat surprising
things that we handle by documenting them. And I don't have a better
idea either.
I was
tempted to suggest using ADMIN instead of SET for the ownership stuff, but
that wouldn't be backward-compatible, and you'd still be able to work
around it to some extent with SET (e.g., SET ROLE followed by CREATE
DATABASE).
I think that would be way worse. Giving ADMIN OPTION on a role is like
making someone the owner of the object, whereas giving someone INHERIT
or SET on a role is just a privilege to use the object.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Sun, Oct 16, 2022 at 12:34 PM Stephen Frost <sfrost@snowman.net> wrote:
As we work through splitting up the privileges and managing them in a
more fine-grained way, it seems clear that we'll need to have a similar
split for ADMIN rights on roles- that is, we'll need to be able to
say "role X is allowed to GRANT INHERIT for role Y to other roles, but
not SET".
I don't think this is clear at all, actually. I see very little
advantage in splitting up ADMIN OPTION this way. I did think about
this, because it would be more consistent with what we do for table
privileges, but INHERIT and SET overlap enough from a permissions
point of view that there doesn't seem to be a lot of value in it. Now,
if we invent a bunch more per-grant options, things might look
different, but in my opinion that has dubious value. Right now, all
role privileges other than the ones that are controlled by ADMIN
OPTION, INHERIT, and what I'm proposing to make controlled by SET, are
gated by CREATEROLE or by SUPERUSER. The list looks something like
this: change the INHERIT flag on a role, change the CREATEROLE flag on
a role, change the CREATEDB flag on a role, change the connection
limit for a role, change the VALID UNTIL time for a role, change the
password for a role other than your own, drop the role.
And that's a pretty obscure list of things. I do think we need better
ways to control who can do those things, but I don't think making them
all role privileges and then on top of that giving them all separate
admin options is the right way to go. It's slicing things incredibly
finely to give alice the right to grant to some other user the right
to set only the VALID UNTIL time on role bob, but not the right to
modify role bob in any other way or the right to confer the ability to
set VALID UNTIL for any other user. I can't believe we want to go
there. It's not worth the permissions bits, and even if we had
infinite privilege bits available, it's not worth the complexity from
a user perspective. Maybe you have some less-obscure list of things
that you think should be grantable privileges on roles?
Another thing to consider is that, since ADMIN OPTION is, as I
understand it, part of the SQL specification, I think it would move us
further from the SQL specification. I think we will be better off
thinking of ADMIN OPTION on a role as roughly equivalent to being the
owner of that role, which is an indivisible privilege, rather than
thinking of it as equivalent to GRANT OPTION on each of N rights,
which could then be subdivided.
I'm still half-tempted to say that predefined roles should just be dealt
with as a special case.. but if we split ADMIN in the manner as
described above then maybe we could get away with not having to, but it
would depend a great deal of people actually reading the documentation
and I'm concerned that's a bit too much to ask in this case.
I don't think any splitting of ADMIN would be required to solve the
predefined roles problem. Doesn't the patch I proposed do that?
--
Robert Haas
EDB: http://www.enterprisedb.com
Bump.
Discussion has trailed off here, but I still don't see that we have a
better way forward here than what I proposed on September 30th. Two
people have commented. Nathan said that he wasn't sure this was best
(neither am I) but that he didn't have a better idea either (neither
do I). Stephen proposed decomposing ADMIN OPTION, which is not my
preference, but even if it turns out that we want to pursue that
approach, I do not think it would make sense to bundle that into this
patch, because there isn't enough overlap between that change and this
change to justify that treatment.
If anyone else wants to comment, or if either of those people want to
comment further, please speak up soon. Otherwise, I am going to press
forward with committing this. If we do not, we will continue to have
no way of restricting of SET ROLE, and we will continue to have no way
of preventing the creation of objects owned by predefined roles by
users who have been granted those roles. As far as I am aware, no one
is opposed to those goals, and in fact I think everyone who has
commented thinks that it would be good to do something. If a better
idea than what I've implemented comes along, I'm happy to defer to it,
but I think this is one of those cases in which there probably isn't
any totally satisfying solution, and yet doing nothing is not a
superior alternative.
Thanks,
--
Robert Haas
EDB: http://www.enterprisedb.com
On Tue, Nov 15, 2022 at 12:07:06PM -0500, Robert Haas wrote:
If anyone else wants to comment, or if either of those people want to
comment further, please speak up soon. Otherwise, I am going to press
forward with committing this.
I don't think I have any further thoughts about the approach, so I won't
balk if you proceed with this change. It might be worth starting a new
thread to discuss whether to treat predefined roles as a special case, but
IMO that needn't hold up this patch.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Tue, 2022-11-15 at 12:07 -0500, Robert Haas wrote:
If anyone else wants to comment, or if either of those people want to
comment further, please speak up soon.
Did you have some thoughts on:
/messages/by-id/a41d606daaaa03b629c2ef0ed274ae3b04a2c266.camel@j-davis.com
Regards,
Jeff Davis
On Tue, Nov 15, 2022 at 7:23 PM Jeff Davis <pgsql@j-davis.com> wrote:
On Tue, 2022-11-15 at 12:07 -0500, Robert Haas wrote:
If anyone else wants to comment, or if either of those people want to
comment further, please speak up soon.Did you have some thoughts on:
/messages/by-id/a41d606daaaa03b629c2ef0ed274ae3b04a2c266.camel@j-davis.com
I mean, I think what we were discussing there could be done, but it's
not the approach I like best. That's partly because that was just a
back-of-the-envelope sketch of an idea, not a real proposal for
something with a clear implementation path. But I think the bigger
reason is that, in my opinion, this proposal is more generally useful,
because it takes no position on why you wish to disallow SET ROLE. You
can just disallow it in some cases and allow it in others, and that's
fine. That proposal targets a specific use case, which may make it a
better solution to that particular problem, but it makes it unworkable
as a solution to any other problem, I believe.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Thu, 2022-11-17 at 16:52 -0500, Robert Haas wrote:
But I think the bigger
reason is that, in my opinion, this proposal is more generally
useful,
because it takes no position on why you wish to disallow SET ROLE.
You
can just disallow it in some cases and allow it in others, and that's
fine.
I agree that the it's more flexible in the sense that it does what it
does, and administrators can use it if it's useful for them. That means
we don't need to understand the actual goals as well; but it also means
that it's harder to determine the consequences if we tweak the behavior
(or any related behavior) later.
I'll admit that I don't have an example of a likely problem here,
though.
That proposal targets a specific use case, which may make it a
better solution to that particular problem, but it makes it
unworkable
as a solution to any other problem, I believe.
Yeah, that's the flip side: "virtual" roles (for lack of a better name)
are a more narrow fix for the problem as I understand it; but it might
leave related problems unfixed. You and Stephen[2]/messages/by-id/YzIAGCrxoXibAKOD@tamriel.snowman.net both seemed to
consider this approach, and I happened to like it, so I wanted to make
sure that it wasn't dismissed too quickly.
But I'm fine if you'd like to move on with the SET ROLE privilege
instead, as long as we believe it grants a stable set of capabilities
(and conversely, that if the SET ROLE privilege is revoked, that it
revokes a stable set of capabilities).
[2]: /messages/by-id/YzIAGCrxoXibAKOD@tamriel.snowman.net
/messages/by-id/YzIAGCrxoXibAKOD@tamriel.snowman.net
--
Jeff Davis
PostgreSQL Contributor Team - AWS