Re: Non-superuser subscription owners

From: Jeff Davis <pgsql(at)j-davis(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Non-superuser subscription owners
Date: 2023-03-01 18:13:55
Message-ID: 505559e0ae8b6c78e35e1039d2439a1c605d0597.camel@j-davis.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 2023-03-01 at 10:05 -0500, Robert Haas wrote:
> For this reason, these expressions are, by
> default, restricted from doing <things>.

The hard part is defining <things> without resorting to a bunch of
special cases, and also in a way that doesn't break a bunch of stuff.

> You earlier
> mentioned that switching to the table owner seems to be just a way of
> turning SECURITY INVOKER into SECURITY DEFINER in random places, or
> maybe that's not exactly what you said but that's what I took from
> it.

Yeah, though I glossed over some details, see below.

> If we just slather user context switches
> everywhere, I'm not actually very sure that's going to be
> comprehensible behavior: if my trigger function is SECURITY INVOKER,
> why is it getting executed as me, not the inserting user?

Let's consider other expressions first. The proposal is that all
expressions attached to a table should run as the table owner (set
aside compatibility concerns for a moment). If those expressions call a
SECURITY INVOKER function, the invoker would need to be the table owner
as well. Users could get confused by that, but I think it's
documentable and understandable; and it's really the only thing that
makes sense -- otherwise changing the user is completely useless.

We should consider triggers as just another expression being executed,
and the invoker is the table owner. The problem is that it's a little
annoying to users because they probably defined the function for the
sole purpose of being a trigger function for a single table, and it
might look as though the SECURITY INVOKER label was ignored.

But there is a difference, which I glossed over before: SECURITY
INVOKER on a trigger function would still have significance, because
the function owner (definer) and table owner (invoker) could still be
different in the case of a trigger, just like in an expression.

This goes back to my point that SECURITY INVOKER is more complex for us
to document and for users to understand. The user *must* understand who
the invoker is in various contexts. That's the situation today and
there's no escaping it. We aren't making things any worse, at least as
long as we can sort out compatibility in a reasonable way.

(Aside: I'm having some serious concerns about how the invoker of a
function called in a view is not the view definer. That's another thing
we'll need to fix, because it's another way of putting SECURITY INVOKER
code in front of someone without them knowing.)

(Aside: We should take a second look at the security invoker views
before we release them. I know that I learned some things during this
discussion and a fresh look might be useful.)

> As soon as you magically turn
> that into a SECURITY DEFINER function, you've provided a way for the
> users performing DML to attack the table owner.

I don't think it's magic, as I said above. But I assume that your more
general point is that if we take some responsibility away from the
invoker and place it on the definer, then it creates room for new kinds
of problems. And I agree.

The point of moving responsibility to the definer is that the definer
can actually do something to protect themselves (nail down search_path,
restrict USAGE privs, and avoid dynamic SQL); whereas the invoker is
nearly helpless.

Regards,
Jeff Davis

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2023-03-01 18:33:01 Re: PATCH: Using BRIN indexes for sorted output
Previous Message Alvaro Herrera 2023-03-01 18:09:53 Re: add PROCESS_MAIN to VACUUM