Re: allowing for control over SET ROLE

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: Jeff Davis <pgsql(at)j-davis(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: allowing for control over SET ROLE
Date: 2023-01-04 20:56:34
Message-ID: CA+TgmoafJ248SfvPvzmeAGy+iRk=1RfDgsR9orF-4kE5rJkOhg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jan 3, 2023 at 5:03 PM Noah Misch <noah(at)leadboat(dot)com> wrote:
> I'd start with locations where the patch already added documentation. In the
> absence of documentation otherwise, a reasonable person could think WITH SET
> controls just SET ROLE. The documentation of WITH SET is a good place to list
> what else you opted for it to control. If the documentation can explain the
> set of principles that would be used to decide whether WITH SET should govern
> another thing in the future, that would provide extra value.

From the point of view of the code, we currently have four different
functions that make inquiries about role membership:
has_privs_of_role, is_member_of_role, is_member_of_role_nosuper, and
member_can_set_role.

I spent a while looking at how has_privs_of_role() is used. Basically,
there are three main patterns. First, in some places, you must have
the privileges of a certain role (typically, either a predefined role
or the role that owns some object) or the operation will fail with an
error indicating that you don't have sufficient permissions. Second,
there are places where having the privileges of a certain role exempts
you from some other permissions check; if you have neither, you'll get
an error. An example is that having the permissions of
pg_read_all_data substitutes for a select privilege. And third, there
are cases where you definitely won't get an error, but the behavior
will vary depending on whether you have the privileges of some role.
For instance, you can see more data in pg_stat_replication,
pg_stat_wal_receiver, and other stats views if you have
pg_read_all_stats. The GUC values reported in EXPLAIN output will
exclude superuser-only values unless you have pg_read_all_settings. It
looks like some maintenance commands like CLUSTER and VACUUM
completely skip over, or just warn about, cases where permission is
lacking. And weirdest of all, having the privileges of a role means
that the RLS policies applied to that role also apply to you. That's
odd because it makes permissions not strictly additive.

member_can_set_role() controls (a) whether you can SET ROLE to some
other role, (b) whether you can alter the owner of an existing object
to that role, and (c) whether you can create an object owned by some
other user in cases where the CREATE command has an option for that,
like CREATE DATABASE ... OWNER.

is_member_of_role_nosuper() is used to prevent creation of role
membership loops, and for pg_hba.conf matching.

The only remaining call to is_member_of_role() is in
pg_role_aclcheck(), which just supports the SQL-callable
pg_has_role(). has_privs_of_role() and member_can_set_role() are used
here, too.

How much of this should we document, do you think? If we're going to
go into the details, I sort of feel like it would be good to somehow
contrast what is attached to membership with what is attached to the
INHERIT option or the SET option. I think it would be slightly
surprising not to mention the way that RLS rules are triggered by
privilege inheritance yet include the fact that the SET option affects
ALTER ... OWNER TO, but maybe I've got the wrong idea. An exhaustive
concordance of what depends on what might be too much, or maybe it
isn't, but it's probably good if the level of detail is pretty
uniform.

Your thoughts?

--
Robert Haas
EDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2023-01-04 21:05:33 Re: pgsql: Delay commit status checks until freezing executes.
Previous Message Tom Lane 2023-01-04 19:59:11 Re: moving extraUpdatedCols out of RangeTblEntry (into ModifyTable)