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

Greetings,

* Jeff Davis (pgsql@j-davis.com) wrote:
> On Tue, 2023-02-28 at 08:37 -0500, Robert Haas wrote:
> > The existing SECURITY_RESTRICTED_OPERATION flag basically prevents
> > you
> > from tinkering with the session state.
>
> Currently, every time we set that flag we also run all the code as the
> table owner.
>
> You're suggesting using the SECURITY_RESTRICTED_OPERATION flag, along
> with the new security flags, but not switch to the table owner, right?

I'm having trouble following this too, I have to admit.  If we aren't
changing who we're running the code under.. but making it so that the
code isn't actually able to do anything then that doesn't strike me as
likely to actually be useful?  Surely things like triggers which are
used to update another table or insert into another table what happened
on the table with the trigger need to be allowed to modify the database-
how do we make that possible while the code runs as the invoker and not
the table owner when the table owner is the one who gets to write the
code?

> >  If we also had a similar flags
> > like DATABASE_READS_PROHIBITED and DATABASE_WRITES_PROHIBITED (or
> > just
> > a combined DATABASE_ACCESS_PROHIBITED flag) I think that would be
> > pretty close to what we need. The idea would be that, when a user
> > executes a function or procedure 
>
> 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).

Would need to apply to functions in views and functions in RLS too,
along wth default expressions and everything else that could be defined
by one person and run by another.

> > owned by a user that they don't trust
> > completely, we'd set
> > SECURITY_RESTRICTED_OPERATION|DATABASE_READS_PROHIBITED|DATABASE_WRIT
> > ES_PROHIBITED.
>
> 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.

While there's certainly some use-cases where a completely unprivileged
user would work, there's certainly an awful lot where it wouldn't.
Having that as an option might be interesting for those much more
limited use-cases and maybe you could even say "only run functions which
are owned by a superuser or X roles" but it's certainly not a general
solution to the problem.

> > And we could provide a user with a way to express the degree of trust
> > they have in some other user or perhaps even some specific function,
> > e.g.
> >
> > SET trusted_roles='alice:read';
> >
> > ...could mean that I trust alice to read from the database with my
> > permissions, should I happen to run code provided by her in SECURITY
> > INVOKER modacke.
>
> 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 pretty far down the path of "wow that looks really difficult to work
with", to put it nicely.

> > I'm sure there's some details to sort out here, e.g. around security
> > related to the trusted_roles GUC itself. But I don't really see a
> > fundamental problem. We can invent arbitrary flags that prohibit
> > classes of operations that are of concern, set them by default in
> > cases where concern is justified, and then give users who want the
> > current behavior some kind of escape hatch that causes those flags to
> > not get set after all. Not only does such a solution not seem
> > impossible, I can possibly even imagine back-patching it, depending
> > on
> > exactly what the shape of the final solution is, how important we
> > think it is to get a fix out there, and how brave I'm feeling that
> > day.
>
> Unless the trusted roles defaults to '*', then I think it will still
> break some things.

Defaulting to an option that is "don't break anything" while giving
users flexibility to test out other, more secure, options seems like it
would be a pretty reasonable way forward, generally.  That said.. I
don't really think this particular approach ends up being a good
direction to go in...

> One of my key tests for user-facing proposals is whether the
> documentation will be reasonable or not. Most of these proposals to
> make SECURITY INVOKER less bad fail that test.

and this is certainly a very good point as to why.

Thanks,

Stephen