Re: allowing for control over SET ROLE

From: Noah Misch <noah(at)leadboat(dot)com>
To: Robert Haas <robertmhaas(at)gmail(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-12 05:09:32
Message-ID: 20230112050932.GA960241@rfd.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jan 11, 2023 at 03:13:29PM -0500, Robert Haas wrote:
> On Wed, Jan 11, 2023 at 10:16 AM Noah Misch <noah(at)leadboat(dot)com> wrote:
> > I still think docs for the SET option itself should give a sense of the
> > diversity of things it's intended to control. It could be simple. A bunch of
> > the sites you're modifying are near text like "These restrictions enforce that
> > altering the owner doesn't do anything you couldn't do by dropping and
> > recreating the aggregate function." Perhaps the main SET doc could say
> > something about how it restricts other things that would yield equivalent
> > outcomes. (Incidentally, DROP is another case of something one likely doesn't
> > want the WITH SET FALSE member using. I think that reinforces a point I wrote
> > upthread. To achieve the original post's security objective, the role must
> > own no objects whatsoever.)
>
> I spent a while on this. The attached is as well I was able to figure
> out how to do. What do you think?

I think this is good to go modulo one or two things:

> Subject: [PATCH v2] More documentation update for GRANT ... WITH SET OPTION.
>
> Update the reference pages for various ALTER commands that
> mentioned that you must be a member of role that will be the
> new owner to instead say that you must be able to SET ROLE
> to the new owner. Update ddl.sgml's generate statement on this

s/generate/general/

> --- a/doc/src/sgml/ref/grant.sgml
> +++ b/doc/src/sgml/ref/grant.sgml
> @@ -298,6 +298,20 @@ GRANT <replaceable class="parameter">role_name</replaceable> [, ...] TO <replace
> This option defaults to <literal>TRUE</literal>.
> </para>
>
> + <para>
> + To create an object owned by another role or give ownership of an existing
> + object to another role, you must have the ability to <literal>SET
> + ROLE</literal> to that role; otherwise, commands such as <literal>ALTER
> + ... OWNER TO</literal> or <literal>CREATE DATABASE ... OWNER</literal>
> + will fail. However, a user who inherits the privileges of a role but does
> + not have the ability to <literal>SET ROLE</literal> to that role may be
> + able to obtain full access to the role by manipulating existing objects
> + owned by that role (e.g. they could redefine an existing function to act
> + as a Trojan horse). Therefore, if a role's privileges are to be inherited
> + but should not be accessible via <literal>SET ROLE</literal>, it should not
> + own any SQL objects.
> + </para>

I recommend deleting the phrase "are to be inherited but" as superfluous. The
earlier sentence's mention will still be there. WITH SET FALSE + NOINHERIT is
a combination folks should not use or should use only when the role has no
known privileges.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2023-01-12 05:12:43 Re: on placeholder entries in view rule action query's range table
Previous Message Masahiko Sawada 2023-01-12 05:05:44 Re: Spinlock is missing when updating two_phase of ReplicationSlot