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

On Thu, 2023-03-30 at 09:41 -0400, Robert Haas wrote:
> On Thu, Mar 30, 2023 at 1:19 AM Jeff Davis <pgsql@j-davis.com> wrote:
> > I say just take the special case out of 0001. If the trigger
> > doesn't
> > work as a SECURITY_RESTRICTED_OPERATION, and is also ENABLED
> > ALWAYS,
> > then the user can just use the new option in 0002 to get the old
> > behavior. I don't see a reason to implicitly give them the old
> > behavior, as 0001 does.
>
> Mmm, I don't agree. Suppose A can SET ROLE to B or C, and B can SET
> ROLE to A. With the patch as written, actions on B's tables are not
> confined by the SECURITY_RESTRICTED_OPERATION flag, but actions on
> C's
> tables are.

It's interesting that it's not transitive, but I'm not sure whether
that's an argument for or against the current approach, or where it
fits (or doesn't fit) with my suggestion. Why do you consider it
important that C's actions are SECURITY_RESTRICTED_OPERATIONs?

> I think we want to do everything possible to avoid people feeling
> like
> they need to turn on this new option. I'm not sure we'll ever be able
> to get rid of it, but we certainly should avoid doing things that
> make
> it more likely that it will be needed.

I don't think it helps much, though. While I previously said that the
special-case behavior is implicit (which is true), it still almost
certainly requires a manual step:

  GRANT subscription_owner TO table_owner WITH SET;

In version 16, the subscription owner is almost certainly a superuser,
and the table owner almost certainly is not, so there's little chance
that it just happens that the table owner has that privilege already.

I don't think we want to encourage such grants to proliferate any more
than we want the option you introduce in 0002 to proliferate. Arguably,
it's worse.

And let's say a user says "I upgraded and my trigger broke logical
replication with message about a security-restricted operation... how
do I get up and running again?". With the patches as-written, we will
have two answers to that question:

  * GRANT subscription_owner TO table_owner WITH SET TRUE
  * ALTER SUBSCRIPTION ... ($awesome_option_name=false)

Under what circumstances would we recommend the former vs. the latter?

> >
> I agree that the naming is somewhat problematic, but I don't like
> trust_table_owners. It's not clear enough about what actually
> happens.
> I want the name to describe behavior, not sentiment.

On reflection, I agree here. We want it to communicate something about
the behavior or mechanism.

> run_as_subscription_owner removes the ambiguity, but is long.

Then fewer people will use it, which might be a good thing.

> I can think of other alternatives, like user_switching or
> switch_to_table_owner or no_user_switching or various other things,
> but none of them seem very good to me.

I like the idea of using "switch" (or some synonym) because it's
technically more correct. The subscription always runs as the
subscription owner; we are just switching temporarily while applying a
change.

> Another idea could be to make the option non-Boolean. This is
> comically long and I can't seriously recommend it, but just to
> illustrate the point, if you type CREATE SUBSCRIPTION ... WITH
> (execute_code_as_owner_of_which_object = subscription) then you
> certainly should know what you've signed up for! If there were a
> shorter version that were still clear, I could go for that, but I'm
> having trouble coming up with exact wording.

I don't care for that -- it communicates the options as co-equal and
maybe something that would live forever (or even have more options in
the future). I'd prefer that nobody uses the non-switching behavior
except for migration purposes or weird use cases we don't really
understand.

> I don't think run_as_owner is terrible, despite the ambiguity.

I won't object but I'm not thrilled.

>
Regards,
    Jeff Davis