Re: Non-superuser subscription owners

On Wed, Mar 1, 2023 at 7:34 PM Andres Freund <andres@anarazel.de> wrote:
> ISTM that this would require annotating most functions in the system. There's
> many problems besides accessing database contents. Just a few examples:
>
> - dblink functions to access another system / looping back
> - pg_read_file()/pg_file_write() allows almost arbitrary mischief
> - pg_stat_reset[_shared]()
> - creating/dropping logical replication slots
> - use untrusted PL functions
> - many more
>
> A single wrongly annotated function would be sufficient to escape. This
> includes user defined functions.
>
> This basically proposes that we can implement a safe sandbox for executing
> arbitrary code in a privileged context. IMO history suggests that that's a
> hard thing to do.

Yeah, that's true, but I don't think switching users all the time is
going to be great either. And it's not like other people haven't gone
this way: that's what plperl (not plperlu) is all about, and
JavaScript running in your browser, and so on. Those things aren't
problem-free, of course, but we're all using them.

When I was initially thinking about this, I thought that maybe we
could just block access to tables and utility statements. That's got
problems in both directions. On the one hand, there are functions like
the ones you propose here that have side effects which we might not
want to allow, and on the other hand, somebody might have an index
expression that does a lookup in a table that they "never change". The
latter case is problematic for non-security reasons, because there's
an invisible dump-ordering constraint that must be obeyed for
dump/restore to work at all, but there's no security issue. Still, I'm
not sure this idea is completely dead in the water. It doesn't seem
unreasonable to me that if you have that kind of case, you have to
somehow opt into the behavior: yeah, I know that index functions I'm
executing are going to read from tables, and I consent to that. And
similarly, if your index expression calls pg_stat_reset_shared(), that
probably ought to be blocked by default too, and if you want to allow
it, you have to say so. Yes, that does require labelling functions, or
maybe putting run-time checks in them:

RequireAvailableCapability(CAP_MODIFY_DATABASE_STATE);

If that capability isn't available in the present context, the call
errors out. That way, it's possible for the required capabilities to
depend on the arguments to the function, and we can change markings in
minor releases without needing catalog changes.

There's another way of thinking about this problem, which involves
supposing that the invoker should only be induced to do things that
the definer could also have done. Basically do every privilege check
twice, and require that both pass. The problem I have with that is
that there are various operations which depend on your identity, not
just your privileges. For instance, a GRANT statement records a
grantor, and a REVOKE statement infers a grantor whose grant is to be
revoked. The operation isn't just allowed or disallowed based on who
performed it -- it actually does something different depending on who
performs it. I believe we have a number of cases like that, and I
think that they suggest that that whole model is pretty flawed. Even
if that were no issue, this also seems extremely complex to implement,
because we have an absolute crap-ton of places that perform privilege
checks and getting all of those places to check privileges as a second
user seems nightmarish. I also think that it might be lead to
confusing error messages: alice tried to do X but we're not allowing
it because bob isn't allowed to do X. Eh, what? As opposed to the
sandboxing approach, where I think you get something more like:

ERROR: database state cannot be modified now
DETAIL: The database system is evaluating an index expression.
HINT: Do $SOMETHING to allow this.

I don't want to press too hard on my idea here. I'm sure it has a
bunch of problems apart from those already mentioned, and those
already mentioned are not trivial. However, I do think there might be
ways to make it work, and I'm not at all convinced that trying to
switch users all over the place is going to be be better, either for
security or usability. Is there some other whole kind of approach we
can take here that we haven't discussed yet?

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