Re: Non-superuser subscription owners - Mailing list pgsql-hackers

On Tue, Feb 28, 2023 at 4:01 PM Jeff Davis <pgsql@j-davis.com> wrote:
> You're suggesting using the SECURITY_RESTRICTED_OPERATION flag, along
> with the new security flags, but not switch to the table owner, right?

Correct.

> Or default expressions, I presume. If we at least agree on this point,
> then I think we should try to find a way to treat these other hunks of
> code in a secure way (which I think is what Andres was suggesting).

Yeah, or any other expressions. Basically impose restrictions when the
user running the code is not the same as the user who provided the
code.

> It seems like you're saying to basically just keep the user ID the
> same, and maybe keep USAGE privileges, but not be able to do anything
> else? Might be useful. Kind of like running it as a nobody user but
> without the problems you mentioned. Some details to think about, I'm
> sure.

Yep.

> I'm not very excited about inventing a new privilege language inside a
> GUC, but perhaps a simpler form could be a reasonable mitigation (or at
> least a starting place).

I'm not very sure about this part, either. I think we need some way of
shutting off whatever new controls we impose, but the shape of it is
unclear to me and I think there are a bunch of problems.

> Unless the trusted roles defaults to '*', then I think it will still
> break some things.

Definitely. IMHO, it's OK to break some things, certainly in a major
release and maybe even in a minor release. But we don't want to break
more things that we really need to break. And as you say, we want the
restrictions to be comprehensible.

> Each of these ideas and sub-ideas affect the semantics, and should be
> documented. But how do we document that some code runs as you, some as
> the person who wrote it, sometimes we obey SECURITY INVOKER and
> sometimes we ignore it and use DEFINER semantics, some code is outside
> a function and always executes as the invoker, some code has some
> security flags, and some code has more security flags, code can change
> between the time you look at it and the time it runs, and it's all
> filtered through GUCs with their own privilege sub-language?
>
> OK, let's assume that we have all of that documented, then how do we
> guide users on what reasonable best practices are for the GUC settings,
> etc.? Or do we just say "this is mechanically how all these parts work,
> good luck assembling it into a secure system!". [ Note: I feel like
> this is the state we are in now. Even if technically we don't have live
> security bugs that I'm aware of, we are setting users up for security
> problems. ]
>
> On the other hand, if we focus on executing code as the user who wrote
> it in most places, then the documentation will be something like: "you
> defined the table, you wrote the code, it runs as you, here are some
> best practices for writing secure code". And we have some different
> documentation for writing a cool SECURITY INVOKER function and how to
> get other users to trust you enough to run it. That sounds a LOT more
> understandable for users.

What I was imagining is that we would document something like: A table
can have executable code associated with it in a variety of ways. For
example, it can have triggers, default expressions, check constraints,
or row-level security filters. In most cases, these expressions are
executed with the privileges of the user performing the operation on
the table, except when SECURITY DEFINER functions are used. Because
these expressions are set by the table owner and executed by the users
accessing the table, there is a risk that the table owner could
include malicious code that usurps the privileges of the user
accessing the table. For this reason, these expressions are, by
default, restricted from doing <things>. If you want to allow those
operations, you can <something>.

I agree that running code as the table owner is helpful in a bunch of
scenarios, but I also don't think it fixes everything. 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.
And I think that's right. 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? I also think
there are plenty of cases where that could just replace the current
set of security problems with a new set of security problems. If the
trigger function is SECURITY INVOKER, then the user who wrote it
doesn't have to worry about securing it against attacks by users
accessing the table; it's just running with the permissions of the
user performing the DML. Maybe there are correctness issues if you
don't lock down search_path etc., but there's no security compromise
because there's no user ID switching. 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.

--
Robert Haas
EDB: http://www.enterprisedb.com