Re: running logical replication as the subscription owner

On Thu, Mar 30, 2023 at 2:52 PM Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
> > 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?

So that C can't try to hack into A's account.

I mean the point here is that B already has permissions to get into
A's account whenever they like, without any hacking. So we don't need
to impose SECURITY_RESTRICTED_OPERATION when running as B, because the
only purpose of SECURITY_RESTRICTED_OPERATION is to prevent the role
to which we're switching from attacking the role from which we're
switching. And that's how the patch is currently coded. You proposed
removing that behavior, on the theory that if the
SECURITY_RESTRICTED_OPERATION restrictions were a problem, someone
could activate the run_as_owner option (or whatever we end up calling
it). But the run_as_owner option applies to the entire subscription.
If A activates that option, then B's hypothetical triggers that run
afoul of the SECURITY_RESTRICTED_OPERATION restrictions start working
again (woohoo!) but they're now vulnerable to attacks from C. With the
patch as coded, A doesn't need to use run_as_owner, everything still
just works for B, and A is still protected against C.

> 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.

I don't necessarily find those role grants to be a problem. Obviously
it depends on the use case. If you're hoping to be able to set up an
account whose only purpose in life is to own subscriptions and which
should have as few permissions as possible, then those role grants
suck, and a hypothetical future feature where you can GRANT
REPLICATION ON TABLE t1 TO subscription_owning_user will be far
better. But I imagine CREATE SUBSCRIPTION being used either by
superusers or by people who already have those role grants anyway,
because I imagine replication as something that a highly privileged
user configures on behalf of everyone who uses the system. And in that
case those role grants aren't something new that you do specifically
for logical replication - they're already there because you need them
to administer stuff. Or you're the superuser and don't need them
anyway.

> 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?

Well, the latter is clearly better because it has such an awesome
option name, right?

More seriously, my theory is that there's very little use case for
having a replication trigger, default expression, etc. that is
performing a security restricted operation. And if someone does have
a use case, and it's between users that can't already SET ROLE back
and forth, then the setup is pretty dubious from a security
perspective and maybe the user ought to rethink it. And if they don't
want to rethink it, then they need to throw security out the window,
and I don't really care which of those commands they use to do it, but
the second one would probably break less other stuff for them, so I'd
likely recommend that one.

> > I don't think run_as_owner is terrible, despite the ambiguity.
>
> I won't object but I'm not thrilled.

Let's see if anyone else weighs in.

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