Re: pgsql: Add new GUC createrole_self_grant.

Hi,

On 2023-01-12 18:15:50 -0700, David G. Johnston wrote:
> On Thu, Jan 12, 2023 at 8:11 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> > On Wed, Jan 11, 2023 at 7:53 PM David G. Johnston
> > <david(dot)g(dot)johnston(at)gmail(dot)com> wrote:
> > > Justed wanted to chime in and say Robert has eloquently put into words
> > much of what I have been thinking here, and that I concur that guiding the
> > DBA to use care with the power they have been provided is a sane position
> > to take.
> > >
> > > +1, and thank you.
> >
> > Thanks!
> >
> > Here's a patch. In it I make three changes, only one of which is
> > directly relevant to the topic at hand:
> >
> > 1. Add a sentence to the documentation on writing SECURITY FUNCTIONS
> > safely concerning createrole_self_grant.
> > 2. Add a sentence to the documentation on SECURITY DEFINER referring
> > to the section about writing such functions safely.
> > 3. Remove a note discussing the fact that pre-8.3 versions did not
> > have SET clauses for functions.
> >
> > I can separate this into multiple patches if desired. And of course
> > you, Tom, or others may have suggestions on which of these changes
> > should be included at all or how to word them better.
> >
> >
> I'm still not really feeling how security definer is the problematic option
> here. Security invoker scares me much more.

I don't really see what that has to do with the topic at hand, unless you want
to suggest removing the entire section about how to write secure security
definer functions?

> postgres=# create role unpriv login;
> CREATE ROLE
> postgres=# create role creator createrole login;
> CREATE ROLE
> postgres=# grant pg_read_all_data to creator with admin option;
> postgres=#
> create procedure makeunprivpowerful() LANGUAGE sql AS $$
> grant pg_read_all_data to unpriv;
> $$;
> CREATE PROCEDURE
> postgres=# alter procedure makeunprivpowerful() owner to unpriv;
> ALTER PROCEDURE
>
> postgres=# \c postgres unpriv
> You are now connected to database "postgres" as user "unpriv".
> postgres=> call makeunprivpowerful();
> ERROR: must have admin option on role "pg_read_all_data"
> CONTEXT: SQL function "makeunprivpowerful" statement 1
>
> postgres=# \c postgres creator
> You are now connected to database "postgres" as user "creator".
> postgres=> call makeunprivpowerful();
> CALL
>
> Personally I think the best advice for a CREATEROLE granted user is to
> never call routines they themselves don't own; or at least only those have
> reviewed thoroughly and understand the consequences of. Regardless of
> security definer/invoker.
>
> In short, a low-privilege user can basically run anything without much fear
> because they can't do harm anyway. Thus security invoker applies to them,
> and having security definer gives them the ability to do some things
> without needing to have permanent higher privileges. These are useful,
> security related attributes with respect to them.
>
> For a high-privilege I would argue that neither security invoker nor
> definer are relevant considerations from a security point-of-view. If you
> have high-privilege you must know what it is you are executing before you
> execute it and anything bad it causes you to do using your privileges, or
> higher if you run a security definer function blindly, is an administrative
> failure, not a security hole.

The point of the security definer section is to explain how to safely write
security definer functions that you grant to less privileged users. It's not
about whether it's safe to call a security invoker / definer function -
indeed, if you don't trust the function author / owner, it's never safe to
call the function.

Greetings,

Andres Freund