Re: running logical replication as the subscription owner - Mailing list pgsql-hackers

On Mon, 2023-03-27 at 12:53 -0400, Robert Haas wrote:
> We do know that an empty search_path is secure,
> but it's probably also going to cause any code we run to fail, unless
> that code schema-qualifies all references outside of pg_catalog, or
> unless it sets search_path itself.

I am confused.

If the trigger function requires schema "xyz" to be in the search_path,
and the function itself doesn't set it, how will it ever get set in the
subscription process? Won't such a function be simply broken for all
logical subscriptions (in current code or with any of the proposals
active right now)?

And if the trigger function requires object "abc" (regardless of
schema) to be somehow accessible without qualification, and if it
doesn't set the search path itself, and it's not in pg_catalog; then
again I think that function would be broken today.

It feels like we are reaching to say that a trigger function might be
broken based on some contrived cases, but we can already contrive cases
that are broken for logical replication today. It might not be exactly
the same set, but unless I'm missing something it would be a very
similar set.

>  search_path also isn't necessarily
> the only problem. As a hypothetical example, suppose I create a table
> with one text column, revoke all public access to that table, and
> then
> create an on-insert trigger that executes as an SQL command any text
> value inserted into the table. This is perfectly secure as long as
> I'm
> the only one who can access the table, but if someone else gets
> access
> to insert things into that table using my credentials then they can
> easily break into my account. Real examples aren't necessarily that
> dramatically bad, but that doesn't mean they don't exist.

Are you suggesting that we require the subscription owner to have SET
ROLE on everybody so that everyone is equally insecure and there's no
way to ever fix it even if you don't have any triggers on your tables
at all?

And all of this is to protect a user that writes a trigger function
that executes arbitrary SQL based on the input data?

>
> Well, I continue to feel that if you can compromise the subscription
> owner's account, we haven't really fixed anything yet.

I'm trying to reconcile the following two points:

  A. That you are proposing a patch to allow non-superuser
subscriptions; and
  B. That you are arguing that the subscription owner basically needs
to be superuser and we should just accept that.

Granted, there's some nuance here and I won't say that those two are
mutually exclusive. But I think it would be helpful to explain how
those two ideas fit together.

> We just ... fixed it so that no
> compromise was possible. And I think that's also the right approach
> here.

If you are saying that we fix the subscription process so that it can't
easily be compromised by the table owner, then that appears to be the
entire purpose of this patch and I agree.

But I think you're saying something slightly different about the
subscription process compromising the table owner, and assuming that
problem exists, your patch doesn't do anything to stop it. In fact,
your patch requires the subscription owner can SET ROLE to each of the
table owners, guaranteeing that the table owners can never do anything
at all to protect themselves from the subscription owner.

> I do agree with you that allowing SET ROLE on tons of accounts is not
> great, though.

Regardless of security concerns, the UI is bad.


Regards,
    Jeff Davis