Re: pgsql: Add new GUC createrole_self_grant.

From: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgsql: Add new GUC createrole_self_grant.
Date: 2023-01-15 01:12:24
Message-ID: CAKFQuwb2y40CBNawWpruVWyAmbDVUUira9SbQFDZGeyKGo-tjQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Sat, Jan 14, 2023 at 5:31 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Fri, Jan 13, 2023 at 8:29 PM David G. Johnston
> <david(dot)g(dot)johnston(at)gmail(dot)com> wrote:
> >> The point of the security definer section is to explain how to safely
> write
> >> security definer functions that you grant to less privileged users
> >
> > Yeah, we are really good at "how".
> >
> > + If the security definer function intends to create roles, and if it
> > + is running as a non-superuser,
> <varname>createrole_self_grant</varname>
> > + should also be set to a known value using the <literal>SET</literal>
> > + clause.
> >
> > I'd like to know "why". Without knowing why we are adding this I can't
> give it a +1. I want the patch to include the why.
>
> What don't you understand about the "why"? If your security-definer
> function relies on some GUC having some particular value for some
> security-critical purpose, and the caller can substitute some other
> value, they might be able to create a security compromise. Since this
> GUC has some connection to security, there is at least some distant
> possibility of that happening. Reasonable people can, perhaps, differ
> about how likely that is, but I don't really see what's confusing
> about the theory. As a general statement about the human condition, if
> you know that someone may be untrustworthy, you should be careful
> about letting them influence your decisions. If you aren't, something
> bad may happen to you.
>
>
OK, given all of that, I suggest reworking the first paragraph of security
definer functions safely to something like the following:

(Replace: Because a SECURITY DEFINER function is executed with the
privileges of the user that owns it, care is needed to ensure that the
function cannot be misused. For security, search_path should be set to
exclude any schemas writable by untrusted users.) with:

The execution of a SECURITY DEFINER function has two interacting behaviors
that make writing and administering such functions require extra care.
While the privileges that come into play during execution are those of the
function owner, the execution environment is inherited from the calling
context. Therefore, any settings that the function relies upon must be
specified in the SET clause of the CREATE command (or within the body of
the function).

Of particular importance is the search_path setting. The search_path
should be set to the bare minimum required for the function to operate and,
more importantly, not include any schemas writable by untrusted users.

<existing wording>
This prevents malicious users [...]
(existing example)
[...] the function could be subverted by creating a temporary table named
pwds.
</existing wording>

<added note="specifically by this patch">
Another setting of note (at least in the case that the function owner is
not a superuser) is createrole_self_grant. While the function owner has
their own pg_db_role_setting preference for this setting, when wrapping
execution of CREATE ROLE within a function, particularly to be executed by
others, it is the executor's setting that would be in effect, not the
owner's.
</added>

(existing wording regarding revoke from public)
(existing example)

David J.

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message David G. Johnston 2023-01-15 02:19:21 Re: pgsql: Add new GUC createrole_self_grant.
Previous Message Robert Haas 2023-01-15 00:31:30 Re: pgsql: Add new GUC createrole_self_grant.

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2023-01-15 02:16:28 [PATCH] pg_restore: use strerror for errors other than ENOENT
Previous Message Robert Haas 2023-01-15 00:32:20 Re: fixing CREATEROLE