Re: Non-superuser subscription owners

Hi,

On 2023-02-06 14:07:39 -0500, Robert Haas wrote:
> On Fri, Feb 3, 2023 at 3:47 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > On 2023-02-02 09:28:03 -0500, Robert Haas wrote:
> > > I don't know what you mean by this. DML doesn't confer privileges. If
> > > code gets executed and runs with the replication user's credentials,
> > > that could lead to privilege escalation, but just moving rows around
> > > doesn't, at least not in the database sense.
> >
> > Executing DML ends up executing code. Think predicated/expression
> > indexes, triggers, default expressions etc. If a badly written trigger
> > etc can be tricked to do arbitrary code exec, an attack will be able to
> > run with the privs of the run-as user. How bad that is is influenced to
> > some degree by the amount of privileges that user has.
>
> I spent some time studying this today. I think you're right. What I'm
> confused about is: why do we consider this situation even vaguely
> acceptable? Isn't this basically an admission that our logical
> replication security model is completely and totally broken and we
> need to fix it somehow and file for a CVE number? Like, in released
> branches, you can't even have a subscription owned by a non-superuser.
> But any non-superuser can set a default expression or create an enable
> always trigger and sure enough, if that table is replicated, the
> system will run that trigger as the subscription owner, who is a
> superuser. Which AFAICS means that if a non-superuser owns a table
> that is part of a subscription, they can instantly hack superuser.
> Which seems, uh, extremely bad. Am I missing something?

It's decidedly not great, yes. I don't know if it's quite a CVE type issue,
after all, the same is true for any other type of query the superuser
executes. But at the very least the documentation needs to be better, with a
big red box making sure the admin is aware of the problem.

I think we need some more fundamental ways to deal with this issue, including
but not restricted to the replication context. Some potentially relevant
discussion is in this thread:
https://postgr.es/m/75b0dbb55e9febea54c441efff8012a6d2cb5bd7.camel%40j-davis.com

I don't agree with Jeff's proposal, but I think there's some worthwhile ideas
in the idea + followups.

> And then, in master, where there's some provision for non-superuser
> subscription owners, we maybe need to re-think the privileges required
> to replicate into a table in the first place. I don't think that
> having I/U/D permissions on a table is really sufficient to justify
> performing those operations *as the table owner*; perhaps the check
> ought to be whether you have the privileges of the table owner.

Yes, I think we ought to check role membership, including non-inherited
memberships.

Greetings,

Andres Freund