Re: Non-superuser subscription owners

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Non-superuser subscription owners
Date: 2023-02-06 19:40:18
Message-ID: CA+TgmoavSQVcvEW3ZgZ7a1Q-TJ-fp0+Nt7W3D7FCawArtTCBCQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Feb 6, 2023 at 2:18 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> 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 don't think that's the same thing at all. A superuser executing a
query interactively can indeed cause all sorts of bad things to
happen, but you don't have to log in as superuser and run DML queries
on tables owned by unprivileged users, and you shouldn't.

But what we're talking about here is -- the superuser comes along and
sets up logical replication in the configuration in what seems to be
exactly the way it was intended to be used, and now any user who can
log into the subscriber node can become superuser for free whenever
they want, without the superuser doing anything at all, even logging
in. Saying it's "not ideal" seems like you're putting it in the same
category as "the cheese got moldy in the fridge" but to me it sounds
more like "the fridge exploded and the house is on fire."

If we were to document this, I assume that the warning we would add to
the documentation would look like this:

<-- begin documentation text -->
Pretty much don't ever use logical replication. In any normal
configuration, it lets every user on your system escalate to superuser
whenever they want. It is possible to make it safe, if you make sure
all the tables on the replica are owned by the superuser and none of
them have any triggers, defaults, expression indexes, or anything else
associated with them that might execute any code while replicating.
But notice that this makes logical replication pretty much useless for
one of its intended purposes, which is high availability, because if
you actually fail over, you're going to then have to change the owners
of all of those tables and apply any missing triggers, defaults,
expression indexes, or anything like that which you may want to have.
And then to fail back you're going to have to remove all of that stuff
again and once again make the tables superuser-owned. That's obviously
pretty impractical, so you probably shouldn't use logical replication
at all until we get around to fixing this. You might wonder why we
implemented a feature that can't be used in any kind of normal way
without completely and totally breaking your system security -- but
don't ask us, we don't know, either!
<-- end documentation text -->

Honestly, this makes the CREATEROLE exploit that I fixed recently in
master look like a walk in the park. Sure, it's a pain for service
providers, who might otherwise use it, but most normal users don't and
wouldn't no matter how it worked, and really are not going to care.
But people do use logical replication, and it seems to me that the
issue you're describing here means that approximately 100% of those
installations have a vulnerability allowing any local user who owns a
table or can create one to escalate to superuser. Far from being not
quite a CVE issue, that seems substantially more serious than most
things we get CVEs for.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2023-02-06 19:57:04 Re: pglz compression performance, take two
Previous Message Andres Freund 2023-02-06 19:33:28 Re: [PATCH] Compression dictionaries for JSONB