Re: pgsql: Add new GUC createrole_self_grant.

Поиск
Список
Период
Сортировка
От David G. Johnston
Тема Re: pgsql: Add new GUC createrole_self_grant.
Дата
Msg-id CAKFQuwb2y40CBNawWpruVWyAmbDVUUira9SbQFDZGeyKGo-tjQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: pgsql: Add new GUC createrole_self_grant.  (Robert Haas <robertmhaas@gmail.com>)
Ответы Re: pgsql: Add new GUC createrole_self_grant.  ("David G. Johnston" <david.g.johnston@gmail.com>)
Re: pgsql: Add new GUC createrole_self_grant.  (Robert Haas <robertmhaas@gmail.com>)
Список pgsql-hackers
On Sat, Jan 14, 2023 at 5:31 PM Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Jan 13, 2023 at 8:29 PM David G. Johnston
<david.g.johnston@gmail.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.

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Robert Haas
Дата:
Сообщение: Re: fixing CREATEROLE
Следующее
От: Justin Pryzby
Дата:
Сообщение: [PATCH] pg_restore: use strerror for errors other than ENOENT