Re: allowing for control over SET ROLE

On Wed, Jan 04, 2023 at 03:56:34PM -0500, Robert Haas wrote:
> On Tue, Jan 3, 2023 at 5:03 PM Noah Misch <noah@leadboat.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?

Rough thoughts:

Do document:
- For pg_read_all_stats, something like s/Read all pg_stat_/See all rows of all pg_stat_/
- At CREATE POLICY and/or similar places, explain the semantics used to judge
  the applicability of role_name to a given query.

Don't document:
- Mechanism for preventing membership loops.

Already documented adequately:
- "First, in some places, you must have the privileges of a certain role" is
  documented through language like "You must own the table".
- pg_read_all_data
- EXPLAIN.  I'm not seeing any setting that's both GUC_SUPERUSER_ONLY and
  GUC_EXPLAIN.
- SQL-level pg_has_role().

Unsure:
- At INHERIT, cover the not-strictly-additive RLS consequences.

> 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.

Works for me.

> 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.

The CREATE POLICY syntax and docs show the role_name parameter, though they
don't detail how exactly the server determines whether a given role applies at
a given moment.  The docs are silent on the SET / OWNER TO connection.  Hence,
I think the doc gap around SET / OWNER TO is more acute than the doc gap
around this RLS behavior.

Thanks,
nm