Blocking execution of SECURITY INVOKER - Mailing list pgsql-hackers

Motivation
==========

SECURITY INVOKER is dangerous, particularly for administrators. There
are numerous ways to put code in a place that's likely to be executed:
triggers, views calling functions, logically-replicated tables, casts,
search path and function resolution tricks, etc. If this code is run
with the privileges of the invoker, then it provides an easy path to
privilege escalation.

We've addressed some of these risks, i.e. by offering better ways to
control the search path, and by ignoring SECURITY INVOKER in some
contexts (like maintenance commands). But it still leaves a lot of
risks for administrators who want to do a SELECT or INSERT. And it
limits major use cases, like logical replication (where the
subscription owner must trust all table owners).

Note that, in the SQL spec, SECURITY DEFINER is the default, which may
be due to some of the dangers of SECURITY INVOKER. (SECURITY DEFINER
carries its own risks, of course, especially if the definer is highly
privileged.)

Prior work
==========

https://www.postgresql.org/message-id/19327.1533748538%40sss.pgh.pa.us

The above thread came up with a couple solutions to express a trust
relationship between users (via GUC or DDL). I'm happy if that
discussion continues, but it appeared to trail off.

My new proposal is different (and simpler, I believe) in two ways:

First, my proposal is only concerned with SECURITY INVOKER functions
and executing arbitrary code written by untrusted users. There's still
the potential for mischief without using SECURITY INVOKER (e.g. if the
search path isn't properly controlled), but it's a different kind of
problem. This narrower problem scope makes my proposal less invasive.

Second, my proposal doesn't establish a new trust relationship. If the
SECURITY INVOKER function is owned by a user that can SET ROLE to you,
you trust it; otherwise not.

Proposal
========

New boolean GUC check_function_owner_trust, default false.

If check_function_owner_trust=true, throw an error if you try to
execute a function that is SECURITY INVOKER and owned by a user other
than you or someone that can SET ROLE to you.

Use Cases
=========

1. If you are a superuser/admin working on a problem interactively, you
can protect yourself against accidentally executing malicious code with
your privileges.

2. You can set up logical replication subscriptions into tables owned
by users you don't trust, as long as triggers (if needed) can be safely
written as SECURITY DEFINER.

3. You can ensure that running an extension script doesn't somehow
execute malicious code with superuser privileges.

4. Users can protect themselves from executing malicious code in cases
where:
  a. role membership accurately describes the trust relationship
already
  b. triggers, views-calling-UDFs, etc., (if any) can be safely written
as SECURITY DEFINER

While that may not be every conceivable use case, it feels very useful
to me.

Even if you really don't like SECURITY DEFINER, points 1, 3, and 4(a)
seem like wins. And there are a lot of cases where the user simply
doesn't need triggers (etc.).

Extensions
==========

Some extensions might create and extension-specific user that owns lots
of SECURITY INVOKER functions. If this GUC is set, other users wouldn't
be able to call those functions.

Our contrib extensions don't seem do that, and all the tests for them
pass without modification (even when the GUC is true).

For extensions that do create extension-specific users that own
SECURITY INVOKER functions, this GUC alone won't work. Trying to
capture that use case as well could involve more discussion (involving
extension authors) and may result in an extension-specific trust
proposal, so I'm considering that out of scope.

Loose Ends
==========

Do we need to check security-invoker views? I don't think it's nearly
as important, because views can't write data. A security-invoker view
read from a security definer function uses the privileges of the
function owner, so I don't see an obvious way to abuse a security
invoker view, but perhaps I'm not creative enough.

Also, Noah's patch did things differently from mine in a few places,
and I need to work out whether I missed something. I may have to add a
check for the range types "subtype_diff" function, for instance.

Future Work
===========

In some cases, we should consider defaulting (or even forcing) this GUC
to be true, such as in a subscription apply worker.

This proposal may offer a path to allowing non-superusers to create
event triggers.

We may want to provide SECURITY PUBLIC or SECURITY NONE (or even
"SECURITY AS <role>"?), which would execute a function with minimal
privileges, and further reduce the need for executing untrusted
SECURITY INVOKER code.

Another idea is to have READ ONLY functions which would be another way
to make SECURITY INVOKER safer.

--
Jeff Davis
PostgreSQL Contributor Team - AWS