Re: CREATE FUNCTION ... SEARCH { DEFAULT | SYSTEM | SESSION }

On Mon, Sep 18, 2023 at 4:51 PM Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
> I don't want to make an argument of the form "the status quo is really
> bad, and therefore my proposal is good". That line of argument is
> suspect for good reason.

+1.

> But if my proposal isn't good enough, and we don't have a clear
> alternative, we need to think seriously about how much we've
> collectively over-promised and under-delivered on the concept of
> privilege separation.
>
> Absent a better idea, we need to figure out a way to un-promise what we
> can't do and somehow guide users towards safe practices. For instance,
> don't grant the INSERT or UPDATE privilege if the table uses functions
> in index expressions or constraints. Also don't touch any table unless
> the onwer has SET ROLE privileges on your role already, or the
> operation is part of a special carve out (logical replication or a
> maintenance command). And don't use the predefined role
> pg_write_all_data, because that's unsafe for most imaginable use cases.

I agree this is a mess, and that documenting the mess better would be
good. But instead of saying not to do something, we need to say what
will happen if you do the thing. I'm regularly annoyed when somebody
reports that "I tried to do X and it didn't work," instead of saying
what happened when they tried, and this situation is another form of
the same thing. "If you do X, then Y will or can occur" is much better
than "do not do X". And I think better documentation of this area
would be useful regardless of any other improvements that we may or
may not make. Indeed, really good documentation of this area might
facilitate making further improvements by highlighting some of the
problems so that they can more easily be understood by a wider
audience. I fear it will be hard to come up with something that is
clear, that highlights the severity of the problems, and that does not
veer off into useless vitriol against the status quo, but if we can
get there, that would be good.

But, leaving that to one side, what technical options do we have on
the table, supposing that we want to do something that is useful but
not this exact thing?

I think one option is to somehow change the behavior around
search_path but in a different way than you've proposed. The most
radical option would be to make it not be a GUC any more. I think the
backward-compatibility implications of that would likely be
unpalatable to many, and the details of what we'd actually do are also
not clear, at least to me. For a function, I think there is a
reasonable argument that you just make it a function property, like
IMMUTABLE, as you said before. But for code that goes directly into
the session, where's the search_path supposed to come from? It's got
to be configured somewhere, and somehow that somewhere feels a lot
like a GUC. That leads to a second idea, which is having it continue
to be a GUC but only affect directly-entered SQL, with all
indirectly-entered SQL either being stored as a node tree or having a
search_path property attached somewhere. Or, as a third idea, suppose
we leave it a GUC but start breaking semantics around where and how
that GUC gets set, e.g. by changing CREATE FUNCTION to capture the
prevailing search_path by default unless instructed otherwise.
Personally I feel like we'd need pretty broad consensus for any of
these kinds of changes because it would break a lot of stuff for a lot
of people, but if we could get that then I think we could maybe emerge
in a better spot once the pain of the compatibility break receded.

Another option is something around sandboxing and/or function trust.
The idea here is to not care too much about the search_path behavior
itself, and instead focus on the consequences, namely what code is
getting executed as which user and perhaps what kinds of operations
it's performing. To me, this seems like a possibly easier answer way
forward at least in the short to medium term, because I think it will
break fewer things for fewer people, and if somebody doesn't like the
new behavior they can just say "well I trust everyone completely" and
it all goes back to the way it was. That said, I think there are
problems with my previous proposals on the other thread so I believe
some adjustments would be needed there, and then there's the problem
of actually implementing anything. I'll try to respond to your
comments on that thread soon.

Are there any other categories of things we can do? More specific
kinds of things we can do in each category? I don't really see an
option other than (1) "change something in the system design so that
people use search_path wrongly less often" or (2) "make it so that it
doesn't matter as much if people using the wrong search_path" but
maybe I'm missing a clever idea.

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